From 47476797cf9c5005209232b02caa3ef0c606c8e7 Mon Sep 17 00:00:00 2001 From: Owais Jamil Date: Fri, 22 May 2026 14:56:27 -0500 Subject: [PATCH] docs: add comments to oauth related code * fix kotlin compat * fix discover text overflow --- android/build.gradle.kts | 19 ++++++++--- android/gradle.properties | 4 +++ lib/app/lazurite_app.dart | 8 +++++ .../network/unauthorized_recovery_runner.dart | 8 +++++ lib/features/auth/bloc/auth_bloc.dart | 8 +++++ lib/features/auth/bloc/auth_event.dart | 4 +++ lib/features/auth/data/auth_repository.dart | 33 +++++++++++++++++++ .../auth/data/models/auth_models.dart | 6 ++++ .../presentation/oauth_callback_screen.dart | 6 ++++ .../presentation/public_home_screen.dart | 9 +++-- .../presentation/public_home_screen_test.dart | 6 ++++ 11 files changed, 105 insertions(+), 6 deletions(-) diff --git a/android/build.gradle.kts b/android/build.gradle.kts index b95b87c..e927b8a 100644 --- a/android/build.gradle.kts +++ b/android/build.gradle.kts @@ -14,6 +14,14 @@ fun Project.configureAndroidJvmCompatibility() { } } +fun Project.configureKotlinJvmCompatibility() { + tasks.withType().configureEach { + compilerOptions { + jvmTarget.set(org.jetbrains.kotlin.gradle.dsl.JvmTarget.JVM_17) + } + } +} + val newBuildDir: Directory = rootProject.layout.buildDirectory .dir("../../build") @@ -30,12 +38,15 @@ subprojects { } } - tasks.withType().configureEach { - compilerOptions { - jvmTarget.set(org.jetbrains.kotlin.gradle.dsl.JvmTarget.JVM_17) - } + configureKotlinJvmCompatibility() +} + +gradle.projectsEvaluated { + subprojects { + configureKotlinJvmCompatibility() } } + subprojects { project.evaluationDependsOn(":app") } diff --git a/android/gradle.properties b/android/gradle.properties index fbee1d8..d5da727 100644 --- a/android/gradle.properties +++ b/android/gradle.properties @@ -1,2 +1,6 @@ org.gradle.jvmargs=-Xmx8G -XX:MaxMetaspaceSize=4G -XX:ReservedCodeCacheSize=512m -XX:+HeapDumpOnOutOfMemoryError android.useAndroidX=true +# This builtInKotlin flag was added automatically by Flutter migrator +android.builtInKotlin=false +# This newDsl flag was added automatically by Flutter migrator +android.newDsl=false diff --git a/lib/app/lazurite_app.dart b/lib/app/lazurite_app.dart index 6ea9fd2..eb21d83 100644 --- a/lib/app/lazurite_app.dart +++ b/lib/app/lazurite_app.dart @@ -219,6 +219,8 @@ class _LazuriteAppState extends State with WidgetsBindingObserver { } } + /// Resume is a cheap chance to refresh expired tokens before the first visible + /// request hits a 401. Valid sessions are left untouched. Future _refreshExpiredSessionOnResume() async { final authState = widget.authBloc.state; final tokens = authState.tokens; @@ -228,6 +230,9 @@ class _LazuriteAppState extends State with WidgetsBindingObserver { await _recoverAuthSession(trigger: 'app_resumed'); } + /// Shared auth recovery entry point for app resume, router/repository 401s, + /// push registration, and background-triggered work. Recovery is coalesced by + /// DID so simultaneous failures spend at most one rotating refresh token. Future _recoverAuthSession({required String trigger}) async { String? refreshingDid; Completer? completer; @@ -273,6 +278,7 @@ class _LazuriteAppState extends State with WidgetsBindingObserver { } } + /// Guard against publishing refreshed tokens after logout or account switch. bool _canPublishRecoveryForDid(String? refreshingDid) { if (!mounted || refreshingDid == null) { return false; @@ -290,6 +296,8 @@ class _LazuriteAppState extends State with WidgetsBindingObserver { ).router; } + /// Router state is keyed by DID so authenticated route providers are rebuilt + /// when accounts change, but not for token rotations within the same account. String _sessionKeyFor(AuthState state) => state.tokens?.did ?? 'guest'; void _handleSessionKeyChanged(String sessionKey) { diff --git a/lib/core/network/unauthorized_recovery_runner.dart b/lib/core/network/unauthorized_recovery_runner.dart index ae51c76..3194795 100644 --- a/lib/core/network/unauthorized_recovery_runner.dart +++ b/lib/core/network/unauthorized_recovery_runner.dart @@ -6,6 +6,10 @@ typedef UnauthorizedClientFactory = TClient? Function(AuthTokens tokens typedef UnauthorizedRecoveryLogger = void Function(Object error, StackTrace stackTrace); /// Centralized helper for retry-on-unauthorized with token refresh. +/// +/// Repositories keep their current client locally. On a 401, this asks the app +/// shell to refresh the session, rebuilds the client from fresh tokens, and +/// retries the original request once. final class UnauthorizedRecoveryRunner { UnauthorizedRecoveryRunner({ required TClient initialClient, @@ -26,6 +30,8 @@ final class UnauthorizedRecoveryRunner { TClient get client => _client; + /// Runs [request] with the current client. Only UnauthorizedException is + /// recoverable; all other failures belong to the caller's normal error path. Future run(Future Function(TClient client) request) async { try { return await request(_client); @@ -50,6 +56,8 @@ final class UnauthorizedRecoveryRunner { return false; } + // Account-scoped repositories must not switch to a refreshed client for a + // different DID when account switching or background work overlaps recovery. if (_expectedDid != null && refreshedTokens.did != _expectedDid) { return false; } diff --git a/lib/features/auth/bloc/auth_bloc.dart b/lib/features/auth/bloc/auth_bloc.dart index ac216d1..579c3d5 100644 --- a/lib/features/auth/bloc/auth_bloc.dart +++ b/lib/features/auth/bloc/auth_bloc.dart @@ -21,6 +21,8 @@ class AuthBloc extends Bloc { final AuthRepository _authRepository; + /// Routes browser/app-link OAuth callbacks to the repository because the + /// repository owns the pending PAR context needed to redeem the code. Future handleOAuthRedirectUri(Uri uri) => _authRepository.completeOAuthCallbackFromUri(uri); Future _onLoginRequested(LoginRequested event, Emitter emit) async { @@ -40,6 +42,8 @@ class AuthBloc extends Bloc { } } + /// Starts OAuth login and leaves completion to the callback route. The + /// repository future resolves only after the browser returns with a code. Future _onOAuthLoginRequested(OAuthLoginRequested event, Emitter emit) async { emit(const AuthState.authenticating()); @@ -75,10 +79,14 @@ class AuthBloc extends Bloc { } } + /// Publishes tokens refreshed outside this bloc, such as unauthorized recovery + /// in the app shell or account switching. Future _onSessionRestored(SessionRestored event, Emitter emit) async { emit(AuthState.authenticated(event.tokens)); } + /// Re-reads persisted auth state during startup or recovery fallback. This is + /// intentionally separate from explicit logout/clear events. Future _onCheckSessionRequested(CheckSessionRequested event, Emitter emit) async { emit(const AuthState.authenticating()); diff --git a/lib/features/auth/bloc/auth_event.dart b/lib/features/auth/bloc/auth_event.dart index 4eb9fe9..40fd4f2 100644 --- a/lib/features/auth/bloc/auth_event.dart +++ b/lib/features/auth/bloc/auth_event.dart @@ -32,6 +32,7 @@ class LocalAuthDataClearRequested extends AuthEvent { const LocalAuthDataClearRequested(); } +/// Tokens produced outside the main login handlers, usually by refresh/recovery. class SessionRestored extends AuthEvent { const SessionRestored({required this.tokens}); final AuthTokens tokens; @@ -40,10 +41,13 @@ class SessionRestored extends AuthEvent { List get props => [tokens]; } +/// Ask the repository to restore persisted session state without implying a +/// user-requested logout when restoration fails. class CheckSessionRequested extends AuthEvent { const CheckSessionRequested(); } +/// Clears in-memory auth state after external session removal. class SessionCleared extends AuthEvent { const SessionCleared(); } diff --git a/lib/features/auth/data/auth_repository.dart b/lib/features/auth/data/auth_repository.dart index c01d4bd..c74f3ef 100644 --- a/lib/features/auth/data/auth_repository.dart +++ b/lib/features/auth/data/auth_repository.dart @@ -224,6 +224,9 @@ class AuthRepository { await _database.deleteSetting(AppDatabase.activeAccountDidSettingKey); } + /// Starts ATProto OAuth by resolving the account's PDS, selecting an + /// authorization server, launching PAR, then waiting for the app callback. + /// The pending OAuth fields are the bridge between browser launch and callback. Future loginWithOAuth(String handle) async { try { _oauthCompleter = Completer(); @@ -373,6 +376,9 @@ class AuthRepository { } } + /// Refreshes the current session while coalescing same-DID refreshes in this + /// isolate. The refresh token is rotating, so callers must not refresh the + /// same account independently. Future refreshSession(AuthTokens currentSession) async { if (currentSession.refreshToken == null) { throw Exception('No refresh token available for session refresh'); @@ -394,6 +400,9 @@ class AuthRepository { return refresh; } + /// Extends refresh coalescing across foreground/background isolates using a + /// short database lease. A loser waits for the winner's persisted token before + /// attempting to spend the refresh token itself. Future _refreshSessionWithPersistentLock(AuthTokens currentSession) async { final owner = _refreshLockOwner(currentSession); @@ -447,6 +456,8 @@ class AuthRepository { return '${DateTime.now().toUtc().microsecondsSinceEpoch}-$hashCode-${identityHashCode(session)}'; } + /// Performs the network refresh. Before spending a refresh token, check + /// storage because another worker may already have rotated and persisted it. Future _refreshSession(AuthTokens currentSession) async { var session = currentSession; final storedReplacement = await _storedSessionIfRefreshTokenChanged(currentSession); @@ -621,6 +632,9 @@ class AuthRepository { log.i('AuthRepository: Logout complete'); } + /// Redeems the authorization code from the browser callback. If the callback + /// includes an issuer, exchange at that authorization server because the PAR + /// host and final issuer can differ for account-routed OAuth. Future _handleOAuthCallback(String callbackUrl) async { final oauthClient = _pendingOAuthClient; final oauthContext = _pendingOAuthContext; @@ -664,6 +678,8 @@ class AuthRepository { return tokens; } + /// 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 && @@ -705,6 +721,8 @@ class AuthRepository { } } + /// Joins concurrent callback deliveries to the first exchange future. This is + /// intentionally not a retry: OAuth authorization codes are single-use. @visibleForTesting Future runOAuthCallbackExchangeOnce( Uri normalizedCallbackUri, @@ -724,6 +742,8 @@ class AuthRepository { return exchange; } + /// Converts the OAuth library session into persisted app tokens, enriching it + /// with the resolved handle/profile when possible without failing login. Future _buildOAuthTokens( OAuthSession session, { required String fallbackHandle, @@ -783,6 +803,8 @@ class AuthRepository { ); } + /// Resolves a handle or DID to the account PDS. OAuth uses this to discover + /// the protected resource before choosing the authorization server. @visibleForTesting Future resolveServiceForIdentifier(String identifier) async { log.d('AuthRepository: Resolving AT Protocol service for $identifier'); @@ -1043,6 +1065,8 @@ class AuthRepository { return path == _httpsOAuthRedirectPath || path == '$_httpsOAuthRedirectPath/'; } + /// Accepts only callbacks we registered, normalizing Android/iOS route shapes + /// and the hosted callback's trailing-slash redirect into one exchange URL. @visibleForTesting Uri? normalizeOAuthCallbackUri(Uri callbackUri) { if (_isSupportedCustomSchemeRedirect(callbackUri)) { @@ -1085,6 +1109,9 @@ class AuthRepository { (queryParameters.containsKey('code') || queryParameters.containsKey('error')); } + /// Chooses the registered redirect URI for the platform. Android defaults to + /// the custom scheme to avoid browser/app-link relay ambiguity; HTTPS remains + /// available behind a build flag and for iOS. @visibleForTesting Uri selectOAuthRedirectUriTemplate( List redirectUris, { @@ -1137,6 +1164,8 @@ class AuthRepository { } } + /// Persists rotated tokens with compare-and-swap on the previous refresh + /// token. Losing the race means another worker won or the account was removed. Future _persistRefreshedSession({ required AuthTokens previousSession, required AuthTokens refreshedSession, @@ -1202,6 +1231,8 @@ class AuthRepository { ); } + /// Deletes credentials only if storage still contains the rejected refresh + /// token. This avoids logging out a session already refreshed by another path. Future _invalidateSessionIfStillCurrent(AuthTokens tokens) async { final storedAccount = await _database.getAccount(tokens.did); if (storedAccount == null) { @@ -1282,6 +1313,8 @@ class AuthRepository { return null; } + /// Treat invalid_grant as terminal only when every candidate rejects the token + /// or the authoritative issuer does. Other host failures remain retryable. bool _shouldInvalidateOAuthSessionAfterRefreshFailures( List<_OAuthRefreshAttemptFailure> failures, { required String? issuerHost, diff --git a/lib/features/auth/data/models/auth_models.dart b/lib/features/auth/data/models/auth_models.dart index 9514977..a5c1709 100644 --- a/lib/features/auth/data/models/auth_models.dart +++ b/lib/features/auth/data/models/auth_models.dart @@ -2,6 +2,10 @@ import 'package:equatable/equatable.dart'; enum AuthMethod { appPassword, oauth } +/// Persisted credentials plus routing metadata for either auth method. +/// +/// [service] is the user's PDS. [oauthService] is the OAuth authorization +/// server. They can differ, so network clients must choose carefully. class AuthTokens extends Equatable { const AuthTokens({ required this.accessToken, @@ -66,6 +70,8 @@ class AuthTokens extends Equatable { bool get usesOAuth => authMethod == AuthMethod.oauth; + /// Treat tokens as expired slightly early so foreground requests do not race + /// the server-side expiration boundary. bool get isExpired { if (expiresAt == null) return false; return DateTime.now().isAfter(expiresAt!.subtract(const Duration(minutes: 5))); diff --git a/lib/features/auth/presentation/oauth_callback_screen.dart b/lib/features/auth/presentation/oauth_callback_screen.dart index 8212be8..63ae9b6 100644 --- a/lib/features/auth/presentation/oauth_callback_screen.dart +++ b/lib/features/auth/presentation/oauth_callback_screen.dart @@ -5,6 +5,10 @@ import 'package:flutter_bloc/flutter_bloc.dart'; import 'package:go_router/go_router.dart'; import 'package:lazurite/features/auth/bloc/auth_bloc.dart'; +/// Thin route used by custom-scheme and HTTPS app-link callbacks. +/// +/// It must hand the raw callback URI to AuthBloc; the repository validates the +/// state/code against the pending OAuth flow created before browser launch. class OAuthCallbackScreen extends StatefulWidget { const OAuthCallbackScreen({required this.callbackUri, super.key}); @@ -26,6 +30,8 @@ class _OAuthCallbackScreenState extends State { unawaited(_consumeCallback()); } + /// Consume once on route creation. The repository joins duplicate deliveries, + /// which matters because OAuth codes are single-use. Future _consumeCallback() async { final handled = await context.read().handleOAuthRedirectUri(widget.callbackUri); if (!mounted) { diff --git a/lib/features/public/presentation/public_home_screen.dart b/lib/features/public/presentation/public_home_screen.dart index 128503e..503fb3f 100644 --- a/lib/features/public/presentation/public_home_screen.dart +++ b/lib/features/public/presentation/public_home_screen.dart @@ -93,12 +93,17 @@ class _PublicHomeScreenState extends State { ? Icons.trending_up : Icons.travel_explore_outlined, ), - label: Text(widget.providerKey == AppViewProviders.blackskyKey ? 'Trending' : 'Discover'), + label: Text( + widget.providerKey == AppViewProviders.blackskyKey ? 'Trending' : 'Discover', + maxLines: 1, + softWrap: false, + overflow: TextOverflow.fade, + ), ), const ButtonSegment( value: PublicContentTab.feeds, icon: Icon(Icons.rss_feed_outlined), - label: Text('Feeds'), + label: Text('Feeds', maxLines: 1, softWrap: false, overflow: TextOverflow.fade), ), ], selected: {widget.contentTab}, diff --git a/test/features/public/presentation/public_home_screen_test.dart b/test/features/public/presentation/public_home_screen_test.dart index 245b5a2..bac56e8 100644 --- a/test/features/public/presentation/public_home_screen_test.dart +++ b/test/features/public/presentation/public_home_screen_test.dart @@ -99,6 +99,12 @@ void main() { ); expect(contentSwitch.segments[0].icon, isA()); expect(contentSwitch.segments[1].icon, isA()); + final discoverLabel = tester.widget(find.text('Discover')); + final feedsLabel = tester.widget(find.text('Feeds')); + expect(discoverLabel.softWrap, isFalse); + expect(discoverLabel.maxLines, 1); + expect(feedsLabel.softWrap, isFalse); + expect(feedsLabel.maxLines, 1); }); testWidgets('renders BlackSky Trending from public trend data', (tester) async { -- 2.51.2