diff --git a/docs/TODO.md b/docs/TODO.md index 929c450..1067436 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -23,6 +23,26 @@ updated: 2026-05-23 ## UX +### Authentication + +- Coalesce app-level auth recovery and restore paths so repeated refresh failures do not + cause noisy auth state transitions. +- Make the OAuth callback screen distinguish success from failure instead of always + returning through the login screen. +- Replace raw auth exception snackbars with better user-facing messages for invalid + identifiers, denied authorization, network/provider failures, and timeouts; keep + detailed errors in logs. + +--- + +- Add a lightweight OAuth cancel/retry state while waiting for browser authorization. + - Show "Cancel sign in" during the pending OAuth flow. + - After timeout, cancellation, or callback failure, show a concise retry message. +- Do not block sign-in if persisting the selected AppView provider fails; continue with + the current/default provider and show a non-blocking warning. +- Represent expired sessions that could not refresh at startup as a degraded/retrying + auth state instead of normal authenticated state. + ### Notifications - Foreground push messages are processed through the standalone background notification diff --git a/lib/features/account/cubit/account_switcher_cubit.dart b/lib/features/account/cubit/account_switcher_cubit.dart index 51f7a21..1bc28f7 100644 --- a/lib/features/account/cubit/account_switcher_cubit.dart +++ b/lib/features/account/cubit/account_switcher_cubit.dart @@ -100,6 +100,8 @@ class AccountSwitcherCubit extends Cubit { service: account.service, oauthService: account.oauthService, oauthClientId: account.oauthClientId, + oauthTokenType: account.oauthTokenType, + oauthScope: account.oauthScope, dpopNonce: account.dpopNonce, dpopPublicKey: account.dpopPublicKey, dpopPrivateKey: account.dpopPrivateKey, @@ -143,6 +145,8 @@ class AccountSwitcherCubit extends Cubit { service: tokens.service != null ? Value(tokens.service!) : const Value.absent(), oauthService: tokens.oauthService != null ? Value(tokens.oauthService!) : const Value.absent(), oauthClientId: tokens.oauthClientId != null ? Value(tokens.oauthClientId!) : const Value.absent(), + oauthTokenType: tokens.oauthTokenType != null ? Value(tokens.oauthTokenType!) : const Value.absent(), + oauthScope: tokens.oauthScope != null ? Value(tokens.oauthScope!) : const Value.absent(), accessToken: Value(tokens.accessToken), refreshToken: tokens.refreshToken != null ? Value(tokens.refreshToken!) : const Value.absent(), dpopPublicKey: tokens.dpopPublicKey != null ? Value(tokens.dpopPublicKey!) : const Value.absent(), diff --git a/lib/features/auth/data/auth_repository.dart b/lib/features/auth/data/auth_repository.dart index 7e1981b..365cd10 100644 --- a/lib/features/auth/data/auth_repository.dart +++ b/lib/features/auth/data/auth_repository.dart @@ -69,6 +69,67 @@ final class _OAuthRefreshAttemptFailure { final String? oauthErrorCode; } +final class _PendingOAuthState { + const _PendingOAuthState({ + required this.handle, + required this.service, + required this.redirectUri, + required this.context, + required this.createdAt, + required this.clientId, + }); + + final String handle; + final String service; + final String redirectUri; + final OAuthContext context; + final DateTime createdAt; + final String clientId; + + Map toJson() => { + 'handle': handle, + 'service': service, + 'redirectUri': redirectUri, + 'createdAt': createdAt.toUtc().toIso8601String(), + 'clientId': clientId, + 'context': {'codeVerifier': context.codeVerifier, 'state': context.state, 'dpopNonce': context.dpopNonce}, + }; + + static _PendingOAuthState? fromJson(Map json) { + final contextJson = json['context']; + final handle = json['handle']; + final service = json['service']; + final redirectUri = json['redirectUri']; + final createdAtRaw = json['createdAt']; + final clientId = json['clientId']; + if (contextJson is! Map || + handle is! String || + service is! String || + redirectUri is! String || + createdAtRaw is! String || + clientId is! String) { + return null; + } + + final codeVerifier = contextJson['codeVerifier']; + final state = contextJson['state']; + final dpopNonce = contextJson['dpopNonce']; + final createdAt = DateTime.tryParse(createdAtRaw); + if (codeVerifier is! String || state is! String || dpopNonce is! String || createdAt == null) { + return null; + } + + return _PendingOAuthState( + handle: handle, + service: service, + redirectUri: redirectUri, + context: OAuthContext(codeVerifier: codeVerifier, state: state, dpopNonce: dpopNonce), + createdAt: createdAt.toUtc(), + clientId: clientId, + ); + } +} + class AuthRepository { AuthRepository({ required AppDatabase database, @@ -123,6 +184,8 @@ class AuthRepository { static const Duration _refreshLockLease = Duration(seconds: 30); static const Duration _refreshLockPollInterval = Duration(milliseconds: 100); static const Duration _refreshLockWait = Duration(seconds: 5); + static const Duration _pendingOAuthStateTtl = Duration(minutes: 10); + static const String _pendingOAuthStateSettingKey = 'auth_pending_oauth_state'; static final Uri _mobileOAuthRedirectUri = Uri.parse('$_mobileOAuthRedirectScheme:$_mobileOAuthRedirectPath'); static final Uri _httpsOAuthRedirectUri = Uri.https(_httpsOAuthRedirectHost, _httpsOAuthRedirectPath); @@ -312,9 +375,20 @@ class AuthRepository { _pendingOAuthClient = oauthClient; _pendingOAuthContext = context; callbackCompleter = _oauthCompleter!; + await _persistPendingOAuthState( + _PendingOAuthState( + handle: _pendingHandle!, + service: oauthService, + redirectUri: redirectUri.toString(), + context: context, + createdAt: DateTime.now().toUtc(), + clientId: oauthClient.metadata.clientId, + ), + ); log.i('AuthRepository: OAuth PAR completed, launching browser to ${_sanitizeUriForLog(authorizationUrl)}'); await _launchUrl(authorizationUrl); } catch (error, stackTrace) { + await _clearPersistedPendingOAuthState(); _resetPendingOAuthAttemptState(clearHandle: false); lastAttemptError = error; lastAttemptStackTrace = stackTrace; @@ -345,6 +419,7 @@ class AuthRepository { rethrow; } catch (error, stackTrace) { log.e('AuthRepository: OAuth login failed', error: error, stackTrace: stackTrace); + await _clearPersistedPendingOAuthState(); _resetPendingOAuthState(); throw Exception('Failed to login with OAuth: $error'); } finally { @@ -672,11 +747,12 @@ class AuthRepository { /// Entry point for app links/routes that deliver OAuth callbacks. Duplicate /// deliveries are joined so a single-use authorization code is redeemed once. Future completeOAuthCallbackFromUri(Uri callbackUri) async { - final pendingOAuthFlow = - _pendingOAuthClient != null && - _pendingOAuthContext != null && - _pendingHandle != null && - _pendingService != null; + var pendingOAuthFlow = _hasPendingOAuthFlow; + if (!pendingOAuthFlow) { + await _restorePendingOAuthStateFromStorage(); + pendingOAuthFlow = _hasPendingOAuthFlow; + } + if (!pendingOAuthFlow) { log.w( 'AuthRepository: Ignoring OAuth callback without active flow ' @@ -707,11 +783,73 @@ class AuthRepository { return false; } finally { if (!joiningInFlightExchange) { + await _clearPersistedPendingOAuthState(); _resetPendingOAuthState(clearLaunchMode: false); } } } + bool get _hasPendingOAuthFlow => + _pendingOAuthClient != null && _pendingOAuthContext != null && _pendingHandle != null && _pendingService != null; + + Future _persistPendingOAuthState(_PendingOAuthState state) async { + await _database.setSetting(_pendingOAuthStateSettingKey, jsonEncode(state.toJson())); + } + + Future _clearPersistedPendingOAuthState() async { + await _database.deleteSetting(_pendingOAuthStateSettingKey); + } + + Future _restorePendingOAuthStateFromStorage() async { + final raw = await _database.getSetting(_pendingOAuthStateSettingKey); + if (raw == null) { + return; + } + + try { + final decoded = jsonDecode(raw); + if (decoded is! Map) { + await _clearPersistedPendingOAuthState(); + return; + } + + final state = _PendingOAuthState.fromJson(decoded); + if (state == null) { + await _clearPersistedPendingOAuthState(); + return; + } + + final age = DateTime.now().toUtc().difference(state.createdAt); + if (age.isNegative || age > _pendingOAuthStateTtl) { + log.w('AuthRepository: Ignoring stale persisted OAuth state for ${state.handle}'); + await _clearPersistedPendingOAuthState(); + return; + } + + final metadata = _pendingOAuthClientMetadata(state); + _pendingHandle = state.handle; + _pendingService = state.service; + _pendingOAuthContext = state.context; + _pendingOAuthClient = OAuthClient(metadata, service: state.service); + log.i('AuthRepository: Restored pending OAuth state for ${state.handle}'); + } catch (error, stackTrace) { + log.w('AuthRepository: Failed to restore pending OAuth state', error: error, stackTrace: stackTrace); + await _clearPersistedPendingOAuthState(); + } + } + + OAuthClientMetadata _pendingOAuthClientMetadata(_PendingOAuthState state) => OAuthClientMetadata( + clientId: _resolveOauthClientId(state.clientId), + applicationType: 'native', + clientName: 'Lazurite', + clientUri: 'https://lazurite.stormlightlabs.org', + redirectUris: [state.redirectUri], + responseTypes: const ['code'], + grantTypes: const ['authorization_code', 'refresh_token'], + scope: 'atproto transition:generic transition:chat.bsky', + tokenEndpointAuthMethod: 'none', + ); + /// Joins concurrent callback deliveries to the first exchange future. This is /// intentionally not a retry: OAuth authorization codes are single-use. @visibleForTesting diff --git a/lib/features/auth/presentation/oauth_callback_screen.dart b/lib/features/auth/presentation/oauth_callback_screen.dart index d858ca2..dbd6ed2 100644 --- a/lib/features/auth/presentation/oauth_callback_screen.dart +++ b/lib/features/auth/presentation/oauth_callback_screen.dart @@ -45,6 +45,7 @@ class _OAuthCallbackScreenState extends State { return; } + context.read().add(const CheckSessionRequested()); _submitted = true; context.go('/login'); } diff --git a/test/features/account/cubit/account_switcher_cubit_test.dart b/test/features/account/cubit/account_switcher_cubit_test.dart index 9263c18..323abf2 100644 --- a/test/features/account/cubit/account_switcher_cubit_test.dart +++ b/test/features/account/cubit/account_switcher_cubit_test.dart @@ -38,12 +38,20 @@ void main() { DateTime? expiresAt, String? dpopPrivateKey, String? dpopPublicKey, + String? oauthService, + String? oauthClientId, + String? oauthTokenType, + String? oauthScope, }) { return Account( did: did, handle: handle, displayName: null, service: null, + oauthService: oauthService, + oauthClientId: oauthClientId, + oauthTokenType: oauthTokenType, + oauthScope: oauthScope, accessToken: accessToken, refreshToken: refreshToken, dpopPublicKey: dpopPublicKey, @@ -158,6 +166,32 @@ void main() { verifyNever(() => mockAuthRepository.refreshSession(any())); }); + test('preserves complete OAuth metadata when switching accounts', () async { + final account = makeAccount( + did: 'did:plc:user1', + oauthService: 'bsky.social', + oauthClientId: 'client-id', + oauthTokenType: 'DPoP', + oauthScope: 'atproto transition:generic', + dpopPublicKey: 'public-key', + dpopPrivateKey: 'private-key', + ); + when(() => mockDatabase.setSetting(any(), any())).thenAnswer((_) async => 1); + when(() => mockDatabase.getAccount('did:plc:user1')).thenAnswer((_) async => account); + + final cubit = buildCubit(); + cubit.emit(AccountSwitcherState.ready(accounts: [account], activeDid: 'did:plc:user1')); + + final tokens = await cubit.switchAccount('did:plc:user1'); + + expect(tokens, isNotNull); + expect(tokens!.authMethod, AuthMethod.oauth); + expect(tokens.oauthService, 'bsky.social'); + expect(tokens.oauthClientId, 'client-id'); + expect(tokens.oauthTokenType, 'DPoP'); + expect(tokens.oauthScope, 'atproto transition:generic'); + }); + test('calls refreshSession when account is expired with refresh token', () async { final expiredAt = DateTime.now().subtract(const Duration(hours: 1)); final refreshedTokens = AuthTokens( @@ -289,13 +323,16 @@ void main() { }, ); - test('persists OAuth private keys when adding an account', () async { + test('persists complete OAuth metadata when adding an account', () async { final captured = []; const tokens = AuthTokens( accessToken: 'token', did: 'did:plc:newuser', handle: 'new.bsky.social', oauthService: 'bsky.social', + oauthClientId: 'client-id', + oauthTokenType: 'DPoP', + oauthScope: 'atproto transition:generic', dpopPublicKey: 'public-key', dpopPrivateKey: 'private-key', authMethod: AuthMethod.oauth, @@ -320,6 +357,9 @@ void main() { expect(captured.single.dpopPublicKey.value, 'public-key'); expect(captured.single.dpopPrivateKey.value, 'private-key'); expect(captured.single.oauthService.value, 'bsky.social'); + expect(captured.single.oauthClientId.value, 'client-id'); + expect(captured.single.oauthTokenType.value, 'DPoP'); + expect(captured.single.oauthScope.value, 'atproto transition:generic'); }); blocTest( diff --git a/test/features/auth/data/auth_repository_test.dart b/test/features/auth/data/auth_repository_test.dart index fb25747..44319e8 100644 --- a/test/features/auth/data/auth_repository_test.dart +++ b/test/features/auth/data/auth_repository_test.dart @@ -1,4 +1,5 @@ import 'dart:async'; +import 'dart:convert'; import 'package:flutter/foundation.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -37,6 +38,9 @@ void main() { mockDatabase = MockAppDatabase(); mockSlingshotClient = MockSlingshotClient(); when(() => mockDatabase.getAccount(any())).thenAnswer((_) async => null); + when(() => mockDatabase.getSetting(any())).thenAnswer((_) async => null); + when(() => mockDatabase.setSetting(any(), any())).thenAnswer((_) async => 1); + when(() => mockDatabase.deleteSetting(any())).thenAnswer((_) async => 1); when( () => mockDatabase.acquireAuthRefreshLock( any(), @@ -1329,6 +1333,73 @@ void main() { expect(service, equals('auth.example.com')); }); + test('restores persisted pending OAuth state before redeeming a cold-start callback', () async { + final callbackServices = []; + final pendingState = jsonEncode({ + 'handle': 'alice.bsky.social', + 'service': 'bsky.social', + 'redirectUri': 'org.stormlightlabs.lazurite:/oauth/callback', + 'createdAt': DateTime.now().toUtc().toIso8601String(), + 'clientId': AuthRepository.kClientId, + 'context': {'codeVerifier': 'verifier', 'state': 'state', 'dpopNonce': 'nonce'}, + }); + when(() => mockDatabase.getSetting('auth_pending_oauth_state')).thenAnswer((_) async => pendingState); + when(() => mockDatabase.insertAccount(any())).thenAnswer((_) async => 1); + + authRepository = AuthRepository( + database: mockDatabase, + loadClientMetadata: (_) async => _testClientMetadata(), + oauthCallbackSession: (client, callbackUrl, context) async { + callbackServices.add(client.service); + expect(context.codeVerifier, equals('verifier')); + expect(context.state, equals('state')); + expect(context.dpopNonce, equals('nonce')); + expect(Uri.parse(callbackUrl).queryParameters['code'], equals('abc')); + return OAuthSession( + accessToken: 'access', + refreshToken: 'refresh', + tokenType: 'DPoP', + scope: 'atproto', + expiresAt: DateTime.now().add(const Duration(hours: 1)), + sub: 'did:plc:alice', + $dPoPNonce: 'next-nonce', + $publicKey: 'public-key', + $privateKey: 'private-key', + ); + }, + oauthTokenBuilder: + ( + session, { + required fallbackHandle, + required fallbackPdsHost, + required oauthService, + oauthClientId, + }) async => testAuthTokens( + accessToken: session.accessToken, + refreshToken: session.refreshToken, + expiresAt: session.expiresAt, + did: session.sub, + handle: fallbackHandle, + service: fallbackPdsHost, + oauthService: oauthService, + oauthClientId: oauthClientId, + dpopNonce: session.$dPoPNonce, + dpopPublicKey: session.$publicKey, + dpopPrivateKey: session.$privateKey, + authMethod: AuthMethod.oauth, + ), + ); + + final handled = await authRepository.completeOAuthCallbackFromUri( + Uri.parse('org.stormlightlabs.lazurite:/oauth/callback?code=abc&state=state'), + ); + + expect(handled, isTrue); + expect(callbackServices, equals(['bsky.social'])); + verify(() => mockDatabase.insertAccount(any())).called(1); + verify(() => mockDatabase.deleteSetting('auth_pending_oauth_state')).called(1); + }); + test('redeems callback with issuer host when it differs from launched auth service', () async { final authorizeServices = []; final callbackServices = [];