refactor(storage): remove remember-credentials and improve error logging and handling

2025-10-30 14:28:00 +05:30
parent 7fb199b2e4
commit 0e98f2ab2a
5 changed files with 106 additions and 64 deletions
--- a/lib/core/auth/auth_state_manager.dart
+++ b/lib/core/auth/auth_state_manager.dart
@@ -244,7 +244,6 @@ class AuthStateManager extends _$AuthStateManager {
                  'api_key_user', // Special username to indicate API key auth
              password: tokenStr, // Store API key in password field
            );
-            await storage.setRememberCredentials(true);
          }
        }

@@ -273,8 +272,13 @@ class AuthStateManager extends _$AuthStateManager {
        // If user fetch fails, the API key might be invalid
        throw Exception('Invalid API key or insufficient permissions');
      }
-    } catch (e) {
-      DebugLogger.error('api-key-login-failed', scope: 'auth/state', error: e);
+    } catch (e, stack) {
+      DebugLogger.error(
+        'api-key-login-failed',
+        scope: 'auth/state',
+        error: e,
+        stackTrace: stack,
+      );
      _update(
        (current) => current.copyWith(
          status: AuthStatus.error,
@@ -283,7 +287,7 @@ class AuthStateManager extends _$AuthStateManager {
          clearToken: true,
        ),
      );
-      return false;
+      rethrow;
    }
  }

@@ -336,7 +340,6 @@ class AuthStateManager extends _$AuthStateManager {
            username: username,
            password: password,
          );
-          await storage.setRememberCredentials(true);
        }
      }

@@ -360,8 +363,13 @@ class AuthStateManager extends _$AuthStateManager {

      DebugLogger.auth('Login successful');
      return true;
-    } catch (e) {
-      DebugLogger.error('login-failed', scope: 'auth/state', error: e);
+    } catch (e, stack) {
+      DebugLogger.error(
+        'login-failed',
+        scope: 'auth/state',
+        error: e,
+        stackTrace: stack,
+      );
      _update(
        (current) => current.copyWith(
          status: AuthStatus.error,
@@ -370,7 +378,7 @@ class AuthStateManager extends _$AuthStateManager {
          clearToken: true,
        ),
      );
-      return false;
+      rethrow;
    }
  }

@@ -478,8 +486,15 @@ class AuthStateManager extends _$AuthStateManager {
        // Normal username/password credentials
        return await login(username, password, rememberCredentials: false);
      }
-    } catch (e) {
-      DebugLogger.error('silent-login-failed', scope: 'auth/state', error: e);
+    } catch (e, stack) {
+      DebugLogger.error(
+        'silent-login-failed',
+        scope: 'auth/state',
+        error: e,
+        stackTrace: stack,
+      );
+
+      String errorMessage = e.toString();

      // Clear invalid credentials on auth errors
      if (e.toString().contains('401') ||
@@ -487,13 +502,26 @@ class AuthStateManager extends _$AuthStateManager {
          e.toString().contains('authentication') ||
          e.toString().contains('unauthorized')) {
        final storage = ref.read(optimizedStorageServiceProvider);
-        await storage.deleteSavedCredentials();
+        try {
+          await storage.deleteSavedCredentials();
+        } catch (deleteError, deleteStack) {
+          DebugLogger.error(
+            'silent-login-credential-clear-failed',
+            scope: 'auth/state',
+            error: deleteError,
+            stackTrace: deleteStack,
+          );
+          errorMessage =
+              '$errorMessage. Also failed to clear saved '
+              'credentials; please clear Conduit credentials from '
+              'system settings.';
+        }
      }

      _update(
        (current) => current.copyWith(
          status: AuthStatus.unauthenticated,
-          error: e.toString(),
+          error: errorMessage,
          isLoading: false,
          clearToken: true,
        ),
@@ -512,8 +540,30 @@ class AuthStateManager extends _$AuthStateManager {

    // Clear token from storage
    final storage = ref.read(optimizedStorageServiceProvider);
-    await storage.deleteAuthToken();
-    _updateApiServiceToken(null);
+    try {
+      await storage.deleteAuthToken();
+      _updateApiServiceToken(null);
+    } catch (error, stack) {
+      DebugLogger.error(
+        'token-delete-failed',
+        scope: 'auth/state',
+        error: error,
+        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;
+    }

    // Update state
    _update(
@@ -578,9 +628,14 @@ class AuthStateManager extends _$AuthStateManager {
      );

      DebugLogger.auth('Logout complete');
-    } catch (e) {
-      DebugLogger.error('logout-failed', scope: 'auth/state', error: e);
-      // Even if logout fails, clear local state
+    } catch (e, stack) {
+      DebugLogger.error(
+        'logout-failed',
+        scope: 'auth/state',
+        error: e,
+        stackTrace: stack,
+      );
+      // Even if logout fails, clear local state where possible
      final storage = ref.read(optimizedStorageServiceProvider);
      await storage.setActiveServerId(null);
      ref.invalidate(activeServerProvider);
@@ -591,7 +646,9 @@ class AuthStateManager extends _$AuthStateManager {
          isLoading: false,
          clearToken: true,
          clearUser: true,
-          error: 'Logout error: $e',
+          error:
+              'Logout error: $e. Secure credentials may remain stored; '
+              'please clear them from your device keychain.',
        ),
      );
      _updateApiServiceToken(null);
--- a/lib/core/persistence/persistence_keys.dart
+++ b/lib/core/persistence/persistence_keys.dart
@@ -14,7 +14,6 @@ final class PreferenceKeys {
  static const String socketTransportMode = 'socket_transport_mode';
  static const String quickPills = 'quick_pills';
  static const String sendOnEnterKey = 'send_on_enter';
-  static const String rememberCredentials = 'remember_credentials';
  static const String activeServerId = 'active_server_id';
  static const String themeMode = 'theme_mode';
  static const String themePalette = 'theme_palette_v1';
--- a/lib/core/persistence/persistence_migrator.dart
+++ b/lib/core/persistence/persistence_migrator.dart
@@ -93,7 +93,6 @@ class PersistenceMigrator {
    copyString(PreferenceKeys.socketTransportMode);
    copyStringList(PreferenceKeys.quickPills);
    copyBool(PreferenceKeys.sendOnEnterKey);
-    copyBool(PreferenceKeys.rememberCredentials);
    copyString(PreferenceKeys.activeServerId);
    copyString(PreferenceKeys.themeMode);
    copyString(PreferenceKeys.themePalette);
@@ -198,7 +197,6 @@ class PersistenceMigrator {
      PreferenceKeys.socketTransportMode,
      PreferenceKeys.quickPills,
      PreferenceKeys.sendOnEnterKey,
-      PreferenceKeys.rememberCredentials,
      PreferenceKeys.activeServerId,
      PreferenceKeys.themeMode,
      PreferenceKeys.themePalette,
--- a/lib/core/services/optimized_storage_service.dart
+++ b/lib/core/services/optimized_storage_service.dart
@@ -32,8 +32,6 @@ class OptimizedStorageService {

  static const String _authTokenKey = 'auth_token_v3';
  static const String _activeServerIdKey = PreferenceKeys.activeServerId;
-  static const String _rememberCredentialsKey =
-      PreferenceKeys.rememberCredentials;
  static const String _themeModeKey = PreferenceKeys.themeMode;
  static const String _themePaletteKey = PreferenceKeys.themePalette;
  static const String _localeCodeKey = PreferenceKeys.localeCode;
@@ -101,10 +99,12 @@ class OptimizedStorageService {
        scope: 'storage/optimized',
      );
    } catch (error) {
-      DebugLogger.log(
-        'Failed to delete auth token: $error',
+      DebugLogger.error(
+        'Failed to delete auth token',
        scope: 'storage/optimized',
+        error: error,
      );
+      rethrow;
    }
  }

@@ -164,10 +164,12 @@ class OptimizedStorageService {
        scope: 'storage/optimized',
      );
    } catch (error) {
-      DebugLogger.log(
-        'Failed to delete credentials: $error',
+      DebugLogger.error(
+        'Failed to delete credentials',
        scope: 'storage/optimized',
+        error: error,
      );
+      rethrow;
    }
  }

@@ -182,14 +184,6 @@ class OptimizedStorageService {
  // ---------------------------------------------------------------------------
  // Preference helpers (Hive-backed)
  // ---------------------------------------------------------------------------
-  Future<void> setRememberCredentials(bool remember) async {
-    await _preferencesBox.put(_rememberCredentialsKey, remember);
-  }
-
-  bool getRememberCredentials() {
-    return (_preferencesBox.get(_rememberCredentialsKey) as bool?) ?? false;
-  }
-
  Future<void> saveServerConfigs(List<ServerConfig> configs) async {
    try {
      final jsonString = jsonEncode(configs.map((c) => c.toJson()).toList());
@@ -352,37 +346,29 @@ class OptimizedStorageService {
  // Batch operations
  // ---------------------------------------------------------------------------
  Future<void> clearAuthData() async {
-    try {
-      await Future.wait([
-        deleteAuthToken(),
-        deleteSavedCredentials(),
-        _preferencesBox.delete(_rememberCredentialsKey),
-        _preferencesBox.delete(_activeServerIdKey),
-      ]);
+    await Future.wait([
+      deleteAuthToken(),
+      deleteSavedCredentials(),
+      _preferencesBox.delete(_activeServerIdKey),
+    ]);

-      _cache.removeWhere(
-        (key, _) =>
-            key.contains('auth') ||
-            key.contains('credentials') ||
-            key.contains('server'),
-      );
-      _cacheTimestamps.removeWhere(
-        (key, _) =>
-            key.contains('auth') ||
-            key.contains('credentials') ||
-            key.contains('server'),
-      );
+    _cache.removeWhere(
+      (key, _) =>
+          key.contains('auth') ||
+          key.contains('credentials') ||
+          key.contains('server'),
+    );
+    _cacheTimestamps.removeWhere(
+      (key, _) =>
+          key.contains('auth') ||
+          key.contains('credentials') ||
+          key.contains('server'),
+    );

-      DebugLogger.log(
-        'Auth data cleared in batch operation',
-        scope: 'storage/optimized',
-      );
-    } catch (error) {
-      DebugLogger.log(
-        'Failed to clear auth data: $error',
-        scope: 'storage/optimized',
-      );
-    }
+    DebugLogger.log(
+      'Auth data cleared in batch operation',
+      scope: 'storage/optimized',
+    );
  }

  Future<void> clearAll() async {
--- a/lib/core/services/secure_credential_storage.dart
+++ b/lib/core/services/secure_credential_storage.dart
@@ -171,6 +171,7 @@ class SecureCredentialStorage {
      DebugLogger.storage('delete-ok', scope: 'credentials');
    } catch (e) {
      DebugLogger.error('delete-failed', scope: 'credentials', error: e);
+      rethrow;
    }
  }

@@ -216,6 +217,7 @@ class SecureCredentialStorage {
        scope: 'credentials/token',
        error: e,
      );
+      rethrow;
    }
  }