From 45135d9185fb1ad092ca40bb875f1785c14c617e Mon Sep 17 00:00:00 2001 From: cogwheel0 <172976095+cogwheel0@users.noreply.github.com> Date: Wed, 12 Nov 2025 13:23:58 +0530 Subject: [PATCH] fix(auth): Improve auth error handling without token clearing --- lib/core/auth/api_auth_interceptor.dart | 30 +++- lib/core/auth/auth_state_manager.dart | 156 ++++++++++++++---- lib/core/providers/app_providers.dart | 13 +- lib/core/router/app_router.dart | 17 +- lib/core/services/api_service.dart | 23 +-- lib/core/services/settings_service.dart | 5 +- .../providers/unified_auth_providers.dart | 1 + .../auth/views/connection_issue_page.dart | 47 +++++- .../profile/views/app_customization_page.dart | 22 ++- tool/validate_arb_locales.dart | 4 +- 10 files changed, 239 insertions(+), 79 deletions(-) diff --git a/lib/core/auth/api_auth_interceptor.dart b/lib/core/auth/api_auth_interceptor.dart index 5bb3fbf..b9c46d9 100644 --- a/lib/core/auth/api_auth_interceptor.dart +++ b/lib/core/auth/api_auth_interceptor.dart @@ -1,3 +1,5 @@ +import 'dart:async'; + import 'package:dio/dio.dart'; import '../utils/debug_logger.dart'; @@ -136,6 +138,7 @@ class ApiAuthInterceptor extends Interceptor { final path = err.requestOptions.path; // Handle authentication errors consistently + // IMPORTANT: Never auto-logout. Instead, notify the app to show connection issue page if (statusCode == 401) { // Do not clear the token for public or optional-auth endpoints. // A 401 here may indicate endpoint-level permission or server config, @@ -143,8 +146,9 @@ class ApiAuthInterceptor extends Interceptor { final requiresAuth = _requiresAuth(path); final optionalAuth = _hasOptionalAuth(path); if (requiresAuth && !optionalAuth) { - DebugLogger.auth('401 Unauthorized on $path - clearing auth token'); - _clearAuthToken(); + _notifyAuthFailure( + '401 Unauthorized on $path - notifying app without clearing token', + ); } else { DebugLogger.auth( '401 on public/optional endpoint $path - keeping auth token', @@ -155,10 +159,9 @@ class ApiAuthInterceptor extends Interceptor { final requiresAuth = _requiresAuth(path); final optionalAuth = _hasOptionalAuth(path); if (requiresAuth && !optionalAuth) { - DebugLogger.auth( - '403 Forbidden on protected endpoint $path - clearing auth token', + _notifyAuthFailure( + '403 Forbidden on protected endpoint $path - notifying app without clearing token', ); - _clearAuthToken(); } else { DebugLogger.auth( '403 Forbidden on public/optional endpoint $path - keeping auth token', @@ -169,9 +172,24 @@ class ApiAuthInterceptor extends Interceptor { handler.next(err); } + /// Clear auth token and notify callbacks + /// Note: This should only be called for explicit logout, not for connection errors + void _clearAuthToken() { _authToken = null; + final future = onTokenInvalidated?.call(); + if (future != null) { + unawaited(future); + } + } + + void _notifyAuthFailure(String message) { + DebugLogger.auth(message); onAuthTokenInvalid?.call(); - onTokenInvalidated?.call(); + } + + /// Explicitly clear auth token for logout scenarios + void clearAuthTokenForLogout() { + _clearAuthToken(); } } diff --git a/lib/core/auth/auth_state_manager.dart b/lib/core/auth/auth_state_manager.dart index f697451..681565a 100644 --- a/lib/core/auth/auth_state_manager.dart +++ b/lib/core/auth/auth_state_manager.dart @@ -79,6 +79,7 @@ enum AuthStatus { unauthenticated, tokenExpired, error, + credentialError, // Invalid credentials - need re-login } /// Unified auth state manager - single source of truth for all auth operations @@ -87,6 +88,11 @@ class AuthStateManager extends _$AuthStateManager { final AuthCacheManager _cacheManager = AuthCacheManager(); Future? _silentLoginFuture; + // Prevent infinite retry loops + int _retryCount = 0; + static const int _maxRetries = 3; + DateTime? _lastRetryTime; + AuthState get _current => state.asData?.value ?? const AuthState(status: AuthStatus.initial); @@ -496,13 +502,23 @@ class AuthStateManager extends _$AuthStateManager { String errorMessage = e.toString(); - // Clear invalid credentials on auth errors - if (e.toString().contains('401') || - e.toString().contains('403') || - e.toString().contains('authentication') || - e.toString().contains('unauthorized')) { + // Don't clear credentials on connection errors - only clear on actual auth failures + // Check if this is a genuine auth failure vs network issue + final isNetworkError = + e.toString().contains('SocketException') || + e.toString().contains('Connection') || + e.toString().contains('timeout') || + e.toString().contains('NetworkImage'); + + if (!isNetworkError && + (e.toString().contains('401') || + e.toString().contains('403') || + e.toString().contains('authentication') || + e.toString().contains('unauthorized'))) { + // Only clear credentials if this is a real auth failure, not a network issue final storage = ref.read(optimizedStorageServiceProvider); try { + DebugLogger.auth('Clearing invalid credentials after auth failure'); await storage.deleteSavedCredentials(); } catch (deleteError, deleteStack) { DebugLogger.error( @@ -516,72 +532,148 @@ class AuthStateManager extends _$AuthStateManager { 'credentials; please clear Conduit credentials from ' 'system settings.'; } + + // Set credential error status to trigger login page + _update( + (current) => current.copyWith( + status: AuthStatus.credentialError, + error: errorMessage, + isLoading: false, + clearToken: true, + ), + ); + return false; + } else if (isNetworkError) { + DebugLogger.auth( + 'Silent login failed due to network error - keeping credentials', + ); + errorMessage = 'Connection issue - please check your network'; + + // Set general error status to trigger connection issue page + _update( + (current) => current.copyWith( + status: AuthStatus.error, + error: errorMessage, + isLoading: false, + ), + ); + return false; } + // Unknown error type - treat as connection issue but keep credentials + if (errorMessage.trim().isEmpty) { + errorMessage = 'Connection issue - please try again shortly'; + } + DebugLogger.auth( + 'Silent login failed with unknown error - keeping credentials', + ); _update( (current) => current.copyWith( - status: AuthStatus.unauthenticated, + status: AuthStatus.error, error: errorMessage, isLoading: false, - clearToken: true, ), ); return false; } } - /// Handle token invalidation (called by API service) + /// Reset retry counter (called when user manually retries) + void resetRetryCounter() { + _retryCount = 0; + _lastRetryTime = null; + DebugLogger.auth('Retry counter reset for manual retry'); + } + + /// Handle auth issues (called by API service) + /// This shows connection issue page instead of logging out + void onAuthIssue() { + DebugLogger.auth('Auth issue detected - showing connection issue page'); + // Don't clear token or user data - just set error state + // The router will show connection issue page + _update( + (current) => current.copyWith( + status: AuthStatus.error, + error: 'Connection issue - please check your connection', + clearError: false, + ), + ); + } + + /// Handle token invalidation (called by API service for explicit token expiry) + /// This is only used when we need to clear the token for re-login attempts Future onTokenInvalidated() async { + // Prevent infinite retry loops + final now = DateTime.now(); + if (_lastRetryTime != null && + now.difference(_lastRetryTime!).inSeconds < 5) { + _retryCount++; + if (_retryCount >= _maxRetries) { + DebugLogger.auth( + 'Max retry attempts reached - stopping silent re-login', + ); + _update( + (current) => current.copyWith( + status: AuthStatus.error, + error: 'Connection issue - please retry manually', + clearError: false, + ), + ); + // Reset after 30 seconds to allow manual retry + Future.delayed(const Duration(seconds: 30), () { + _retryCount = 0; + _lastRetryTime = null; + }); + return; + } + } else { + // Reset counter if enough time has passed + _retryCount = 0; + } + _lastRetryTime = now; + // Avoid spamming logs if multiple requests invalidate at once final reloginInProgress = _silentLoginFuture != null; if (!reloginInProgress) { - DebugLogger.auth('Auth token invalidated'); + DebugLogger.auth( + 'Auth token invalidated - attempting silent re-login (attempt ${_retryCount + 1}/$_maxRetries)', + ); } - // Clear token from storage final storage = ref.read(optimizedStorageServiceProvider); try { await storage.deleteAuthToken(); - _updateApiServiceToken(null); - } catch (error, stack) { + DebugLogger.auth('Cleared invalidated token from secure storage'); + } catch (e, stack) { DebugLogger.error( 'token-delete-failed', scope: 'auth/state', - error: error, + error: e, stackTrace: stack, ); - _updateApiServiceToken(null); - _update( - (current) => current.copyWith( - status: AuthStatus.error, - error: - 'Failed to clear secure token. Please clear Conduit ' - 'credentials from your device keychain and sign in again.', - clearToken: true, - clearUser: true, - clearError: false, - ), - ); - return; } + _updateApiServiceToken(null); - // Update state _update( (current) => current.copyWith( status: AuthStatus.tokenExpired, + error: 'Session expired - please sign in again', clearToken: true, clearUser: true, - clearError: true, + isLoading: false, ), ); // Attempt silent re-login if credentials are available final hasCredentials = await storage.getSavedCredentials() != null; - if (hasCredentials) { - if (!reloginInProgress) { - DebugLogger.auth('Attempting silent re-login after token invalidation'); + if (hasCredentials && !reloginInProgress) { + DebugLogger.auth('Attempting silent re-login after token invalidation'); + final success = await silentLogin(); + if (success) { + // Reset retry counter on success + _retryCount = 0; + _lastRetryTime = null; } - await silentLogin(); } } diff --git a/lib/core/providers/app_providers.dart b/lib/core/providers/app_providers.dart index c39dfb6..c5d4f0a 100644 --- a/lib/core/providers/app_providers.dart +++ b/lib/core/providers/app_providers.dart @@ -275,8 +275,14 @@ final apiServiceProvider = Provider((ref) { // Keep callbacks in sync so interceptor can notify auth manager apiService.setAuthCallbacks( - onAuthTokenInvalid: () {}, + onAuthTokenInvalid: () { + // Called when auth errors occur (401/403) + // Show connection issue page instead of logging out + final authManager = ref.read(authStateManagerProvider.notifier); + authManager.onAuthIssue(); + }, onTokenInvalidated: () async { + // Called for token expiry - attempt silent re-login final authManager = ref.read(authStateManagerProvider.notifier); await authManager.onTokenInvalidated(); }, @@ -291,8 +297,9 @@ final apiServiceProvider = Provider((ref) { // Keep legacy callback for backward compatibility during transition apiService.onAuthTokenInvalid = () { - // This will be removed once migration is complete - DebugLogger.auth('legacy-token-callback', scope: 'auth/api'); + // Show connection issue page instead of logging out + final authManager = ref.read(authStateManagerProvider.notifier); + authManager.onAuthIssue(); }; return apiService; diff --git a/lib/core/router/app_router.dart b/lib/core/router/app_router.dart index 7b4acf5..dcef7d4 100644 --- a/lib/core/router/app_router.dart +++ b/lib/core/router/app_router.dart @@ -3,6 +3,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; +import '../auth/auth_state_manager.dart'; import '../providers/app_providers.dart'; import '../services/connectivity_service.dart'; import '../services/navigation_service.dart'; @@ -129,8 +130,20 @@ class RouterNotifier extends ChangeNotifier { if (location == Routes.connectionIssue) return null; return null; case AuthNavigationState.error: - if (location == Routes.connectionIssue) return null; - return null; + final authSnapshot = ref + .read(authStateManagerProvider) + .maybeWhen(data: (state) => state, orElse: () => null); + final hasValidToken = authSnapshot?.hasValidToken ?? false; + final isAuthFormRoute = + location == Routes.login || location == Routes.authentication; + if (!hasValidToken && isAuthFormRoute) { + // Keep user on the login/authentication flow to show inline errors + return null; + } + // Otherwise show connection issue page for recoverable auth errors + return location == Routes.connectionIssue + ? null + : Routes.connectionIssue; case AuthNavigationState.authenticated: // Avoid unnecessary redirects if already on a non-auth route if (_isAuthLocation(location) || diff --git a/lib/core/services/api_service.dart b/lib/core/services/api_service.dart index 0fa5063..2859748 100644 --- a/lib/core/services/api_service.dart +++ b/lib/core/services/api_service.dart @@ -1170,9 +1170,7 @@ class ApiService { data, debugLabel: 'parse_file_search', ); - return normalized - .map(FileInfo.fromJson) - .toList(growable: false); + return normalized.map(FileInfo.fromJson).toList(growable: false); } return const []; } @@ -1186,9 +1184,7 @@ class ApiService { data, debugLabel: 'parse_file_all', ); - return normalized - .map(FileInfo.fromJson) - .toList(growable: false); + return normalized.map(FileInfo.fromJson).toList(growable: false); } return const []; } @@ -1599,10 +1595,7 @@ class ApiService { if (data is Map) { final voices = data['voices']; if (voices is List) { - return _normalizeList( - voices, - debugLabel: 'parse_voice_list', - ); + return _normalizeList(voices, debugLabel: 'parse_voice_list'); } } if (data is List) { @@ -1795,10 +1788,7 @@ class ApiService { final response = await _dio.get('/api/v1/images/models'); final data = response.data; if (data is List) { - return _normalizeList( - data, - debugLabel: 'parse_image_models', - ); + return _normalizeList(data, debugLabel: 'parse_image_models'); } return []; } @@ -3213,10 +3203,7 @@ class ApiService { final data = response.data; if (data is List) { - return _normalizeList( - data, - debugLabel: 'parse_message_search', - ); + return _normalizeList(data, debugLabel: 'parse_message_search'); } if (data is Map) { final list = (data['items'] ?? data['results'] ?? data['messages']); diff --git a/lib/core/services/settings_service.dart b/lib/core/services/settings_service.dart index e04f210..afeddf8 100644 --- a/lib/core/services/settings_service.dart +++ b/lib/core/services/settings_service.dart @@ -161,7 +161,10 @@ class SettingsService { box.get(PreferenceKeys.voiceSttPreference) as String?, ), voiceSilenceDuration: - (box.get(_voiceSilenceDurationKey) as int? ?? 2000).clamp(300, 3000), + (box.get(_voiceSilenceDurationKey) as int? ?? 2000).clamp( + 300, + 3000, + ), ), ); } diff --git a/lib/features/auth/providers/unified_auth_providers.dart b/lib/features/auth/providers/unified_auth_providers.dart index 29e9abe..37ca30f 100644 --- a/lib/features/auth/providers/unified_auth_providers.dart +++ b/lib/features/auth/providers/unified_auth_providers.dart @@ -142,6 +142,7 @@ final authNavigationStateProvider = Provider((ref) { return AuthNavigationState.authenticated; case AuthStatus.unauthenticated: case AuthStatus.tokenExpired: + case AuthStatus.credentialError: return AuthNavigationState.needsLogin; case AuthStatus.error: return AuthNavigationState.error; diff --git a/lib/features/auth/views/connection_issue_page.dart b/lib/features/auth/views/connection_issue_page.dart index 62ae029..3fc0468 100644 --- a/lib/features/auth/views/connection_issue_page.dart +++ b/lib/features/auth/views/connection_issue_page.dart @@ -3,12 +3,11 @@ import 'dart:io' show Platform; import 'package:flutter/cupertino.dart'; import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; -import 'package:go_router/go_router.dart'; +import '../../../core/auth/auth_state_manager.dart'; import '../../../core/models/server_config.dart'; import '../../../core/providers/app_providers.dart'; import '../../../core/services/connectivity_service.dart'; -import '../../../core/services/navigation_service.dart'; import '../../../core/widgets/error_boundary.dart'; import '../../../l10n/app_localizations.dart'; import '../../../shared/theme/theme_extensions.dart'; @@ -25,6 +24,7 @@ class ConnectionIssuePage extends ConsumerStatefulWidget { class _ConnectionIssuePageState extends ConsumerState { bool _isLoggingOut = false; + bool _isRetrying = false; String? _statusMessage; @override @@ -174,9 +174,8 @@ class _ConnectionIssuePageState extends ConsumerState { children: [ ConduitButton( text: l10n.retry, - onPressed: _isLoggingOut - ? null - : () => context.go(Routes.serverConnection), + onPressed: (_isLoggingOut || _isRetrying) ? null : _retryConnection, + isLoading: _isRetrying, icon: Platform.isIOS ? CupertinoIcons.refresh : Icons.refresh_rounded, @@ -185,7 +184,9 @@ class _ConnectionIssuePageState extends ConsumerState { const SizedBox(height: Spacing.sm), ConduitButton( text: l10n.signOut, - onPressed: _isLoggingOut ? null : () => _logout(l10n), + onPressed: (_isLoggingOut || _isRetrying) + ? null + : () => _logout(l10n), isLoading: _isLoggingOut, isSecondary: true, icon: Platform.isIOS @@ -212,6 +213,40 @@ class _ConnectionIssuePageState extends ConsumerState { ); } + Future _retryConnection() async { + setState(() { + _isRetrying = true; + _statusMessage = null; + }); + + try { + // Clear the error state and attempt to re-establish connection + final authManager = ref.read(authStateManagerProvider.notifier); + + // Reset retry counter for manual retry attempts + authManager.resetRetryCounter(); + + await authManager.silentLogin(); + + // If successful, router will automatically navigate to chat + if (!mounted) return; + + // Small delay to show loading state + await Future.delayed(const Duration(milliseconds: 500)); + } catch (_) { + if (!mounted) return; + setState(() { + _statusMessage = 'Connection failed. Please try again.'; + }); + } finally { + if (mounted) { + setState(() { + _isRetrying = false; + }); + } + } + } + Future _logout(AppLocalizations l10n) async { setState(() { _isLoggingOut = true; diff --git a/lib/features/profile/views/app_customization_page.dart b/lib/features/profile/views/app_customization_page.dart index f560c3b..c3e71b7 100644 --- a/lib/features/profile/views/app_customization_page.dart +++ b/lib/features/profile/views/app_customization_page.dart @@ -698,7 +698,8 @@ class AppCustomizationPage extends ConsumerWidget { children: [ Text( l10n.sttSilenceDuration, - style: theme.bodyMedium?.copyWith( + style: + theme.bodyMedium?.copyWith( color: theme.sidebarForeground, fontWeight: FontWeight.w600, ) ?? @@ -711,13 +712,16 @@ class AppCustomizationPage extends ConsumerWidget { const SizedBox(height: Spacing.xs), Text( '${settings.voiceSilenceDuration}ms', - style: theme.bodySmall?.copyWith( - color: theme.sidebarForeground - .withValues(alpha: 0.7), + style: + theme.bodySmall?.copyWith( + color: theme.sidebarForeground.withValues( + alpha: 0.7, + ), ) ?? TextStyle( - color: theme.sidebarForeground - .withValues(alpha: 0.7), + color: theme.sidebarForeground.withValues( + alpha: 0.7, + ), fontSize: 12, ), ), @@ -726,7 +730,8 @@ class AppCustomizationPage extends ConsumerWidget { ), Text( '${(settings.voiceSilenceDuration / 1000).toStringAsFixed(1)}s', - style: theme.bodyMedium?.copyWith( + style: + theme.bodyMedium?.copyWith( color: theme.buttonPrimary, fontWeight: FontWeight.w600, ) ?? @@ -752,7 +757,8 @@ class AppCustomizationPage extends ConsumerWidget { ), Text( l10n.sttSilenceDurationDescription, - style: theme.bodySmall?.copyWith( + style: + theme.bodySmall?.copyWith( color: theme.sidebarForeground.withValues(alpha: 0.7), ) ?? TextStyle( diff --git a/tool/validate_arb_locales.dart b/tool/validate_arb_locales.dart index 95771b0..8157f45 100644 --- a/tool/validate_arb_locales.dart +++ b/tool/validate_arb_locales.dart @@ -147,9 +147,7 @@ Future> _scanUsedLocalizationKeys(Set baseKeys) async { } return false; } catch (e) { - stderr.writeln( - 'warning: failed to search for key "$key": $e', - ); + stderr.writeln('warning: failed to search for key "$key": $e'); return false; } }