fix(security): apply token-store operations in the order they are issued

lib/core/contracts/storage_contracts.dart

@@ -24,6 +24,14 @@ abstract class IKeyValueStore {
 }
 
 /// Dedicated token/session storage contract.
+///
+/// An implementation must apply its operations in the order they are issued:
+/// a clear issued after a write lands after that write. The session guards in
+/// `AuthRepositoryImpl.refreshToken()` and `AuthInterceptor` check the session
+/// immediately before a write and rely on a logout's later clear removing it.
+/// `SecureTokenStore` queues its operations to guarantee this, because the
+/// platform store does not on every target. An in-memory store whose methods
+/// complete synchronously meets it trivially.
 abstract class ITokenStore {
   Future<String?> getAccessToken();
   Future<bool> setAccessToken(String token);

lib/core/storage/adapters/secure_token_store.dart

@@ -1,45 +1,90 @@
+import 'dart:async';
+
 import 'package:flutter_starter/core/constants/app_constants.dart';
 import 'package:flutter_starter/core/contracts/storage_contracts.dart';
 import 'package:flutter_starter/core/errors/exceptions.dart';
 import 'package:flutter_starter/core/storage/secure_storage_service.dart';
 
 /// Secure token store implementation backed by [SecureStorageService].
+///
+/// ## Every operation runs in the order it was issued
+///
+/// Each read, write and delete waits for the ones issued before it to settle.
+/// The session guards in `AuthRepositoryImpl.refreshToken()` and
+/// `AuthInterceptor` check the session immediately before a write, which only
+/// helps if a logout's clear, issued after that check, also *lands* after the
+/// write. flutter_secure_storage does not promise that on every platform: on
+/// web, a write awaits WebCrypto encryption before it reaches `localStorage`,
+/// while a delete removes the key at once, so a clear issued after a pending
+/// write ran before it and the write then re-stored a token on a signed-out
+/// browser. Queuing here makes the order the code issues the order the store
+/// applies, on every platform, without clearing anything a newer session
+/// wrote.
+///
+/// The cost: one platform call that never completes holds every later token
+/// operation behind it, the token read on each request and a logout's clear
+/// included. Keychain and Keystore failures surface as errors, which do not
+/// hold the queue; a call that hangs outright would.
 class SecureTokenStore implements ITokenStore {
   /// Creates a token store backed by secure platform storage.
   SecureTokenStore(this._secureStorageService);
 
   final SecureStorageService _secureStorageService;
 
-  @override
-  Future<String?> getAccessToken() {
-    return _secureStorageService.getString(AppConstants.tokenKey);
+  /// Tail of the operation queue; null while idle.
+  Future<void>? _queue;
+
+  /// Runs [operation] once every operation issued before it has settled. A
+  /// failed operation does not wedge the queue.
+  Future<T> _inOrder<T>(Future<T> Function() operation) {
+    final previous = _queue;
+    final done = Completer<void>();
+    _queue = done.future;
+    Future<T> run() async {
+      try {
+        if (previous != null) await previous;
+        return await operation();
+      } finally {
+        done.complete();
+        if (identical(_queue, done.future)) _queue = null;
+      }
+    }
+
+    return run();
   }
 
   @override
-  Future<bool> setAccessToken(String token) async {
-    return await _secureStorageService.setString(AppConstants.tokenKey, token);
-  }
+  Future<String?> getAccessToken() => _inOrder(
+    () => _secureStorageService.getString(AppConstants.tokenKey),
+  );
+
+  @override
+  Future<bool> setAccessToken(String token) => _inOrder(
+    () => _secureStorageService.setString(AppConstants.tokenKey, token),
+  );
 
   /// Throws [CacheException] if the Keychain/Keystore delete fails.
   @override
-  Future<void> clearAccessToken() => _remove(AppConstants.tokenKey);
+  Future<void> clearAccessToken() =>
+      _inOrder(() => _remove(AppConstants.tokenKey));
 
   @override
-  Future<String?> getRefreshToken() {
-    return _secureStorageService.getString(AppConstants.refreshTokenKey);
-  }
+  Future<String?> getRefreshToken() => _inOrder(
+    () => _secureStorageService.getString(AppConstants.refreshTokenKey),
+  );
 
   @override
-  Future<bool> setRefreshToken(String token) async {
-    return await _secureStorageService.setString(
+  Future<bool> setRefreshToken(String token) => _inOrder(
+    () => _secureStorageService.setString(
       AppConstants.refreshTokenKey,
       token,
-    );
-  }
+    ),
+  );
 
   /// Throws [CacheException] if the Keychain/Keystore delete fails.
   @override
-  Future<void> clearRefreshToken() => _remove(AppConstants.refreshTokenKey);
+  Future<void> clearRefreshToken() =>
+      _inOrder(() => _remove(AppConstants.refreshTokenKey));
 
   /// Deletes both tokens, attempting the second even when the first fails.
   ///

lib/features/auth/data/repositories/auth_repository_impl.dart

@@ -316,11 +316,12 @@ class AuthRepositoryImpl implements AuthRepository {
       );
       // Each check sits directly before the write it guards, with no await
       // between them, so a logout that starts after the check issues its clear
-      // after that write. Where the platform store handles calls in order
-      // (flutter_secure_storage on Android and iOS), the clear also lands after
-      // it; elsewhere this narrows the window rather than closing it. A single
-      // check before both writes let a logout landing during the first write
-      // clear the store, and the refresh token was then written after it.
+      // after that write - and the token store applies its operations in the
+      // order they are issued (SecureTokenStore queues them; on web the
+      // plugin alone would let the clear land first), so the clear also lands
+      // after it. A single check before both writes let a logout landing
+      // during the first write clear the store, and the refresh token was then
+      // written after it.
       if (!sessionGeneration.isCurrent(generation)) return stale;
       await localDataSource.cacheToken(authResponse.token);
       if (authResponse.refreshToken != null) {

test/core/storage/adapters/secure_token_store_order_test.dart

@@ -0,0 +1,153 @@
+// flutter_secure_storage on web awaits WebCrypto encryption before a write
+// reaches localStorage, but removes a key at once. So a logout's clear, issued
+// after a token refresh had checked the session and started writing, ran
+// before that write landed - and the write then put a live access token back
+// on a signed-out browser. SecureTokenStore now applies its operations in the
+// order they are issued. These run it over a storage that reorders like the
+// web plugin.
+
+import 'dart:async';
+
+import 'package:flutter_starter/core/constants/app_constants.dart';
+import 'package:flutter_starter/core/errors/failures.dart';
+import 'package:flutter_starter/core/storage/adapters/secure_token_store.dart';
+import 'package:flutter_starter/core/storage/secure_storage_service.dart';
+import 'package:flutter_starter/core/utils/result.dart';
+import 'package:flutter_starter/features/auth/data/datasources/auth_local_datasource.dart';
+import 'package:flutter_starter/features/auth/data/datasources/auth_remote_datasource.dart';
+import 'package:flutter_starter/features/auth/data/models/auth_response_model.dart';
+import 'package:flutter_starter/features/auth/data/repositories/auth_repository_impl.dart';
+import 'package:flutter_starter/features/auth/domain/auth_error_codes.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:mocktail/mocktail.dart';
+
+import '../../../helpers/in_memory_stores.dart';
+
+class _MockSecureStorage extends Mock implements SecureStorageService {}
+
+/// A secure storage whose writes land after an asynchronous step (the web
+/// plugin's encryption) and whose deletes land at once.
+class _WebLikeStorage {
+  _WebLikeStorage() {
+    when(() => mock.getString(any())).thenAnswer(
+      (i) async => values[i.positionalArguments[0] as String],
+    );
+    when(() => mock.setString(any(), any())).thenAnswer((i) async {
+      final key = i.positionalArguments[0] as String;
+      final value = i.positionalArguments[1] as String;
+      writeStarted.remove(key)?.complete();
+      await Future<void>.delayed(const Duration(milliseconds: 20));
+      values[key] = value;
+      return true;
+    });
+    when(() => mock.remove(any())).thenAnswer((i) async {
+      values.remove(i.positionalArguments[0] as String);
+      return true;
+    });
+  }
+
+  final mock = _MockSecureStorage();
+  final Map<String, String> values = {};
+
+  /// Completes when a write to the key starts.
+  final Map<String, Completer<void>> writeStarted = {};
+
+  Future<void> whenWriteStarts(String key) =>
+      (writeStarted[key] = Completer<void>()).future;
+}
+
+class _Remote implements AuthRemoteDataSource {
+  static AuthResponseModel _pair(String prefix) => AuthResponseModel.fromJson({
+    'user': {'id': '42', 'email': 'a@b.c'},
+    'token': '$prefix-access',
+    'refresh_token': '$prefix-refresh',
+  });
+
+  @override
+  Future<AuthResponseModel> login(String email, String password) async =>
+      _pair('login');
+
+  @override
+  Future<AuthResponseModel> register(
+    String email,
+    String password,
+    String name,
+  ) => throw UnimplementedError();
+
+  /// Offline: the remote logout fails at once, so the local clear follows
+  /// immediately.
+  @override
+  Future<void> logout() async => throw Exception('offline');
+
+  @override
+  Future<AuthResponseModel> refreshToken(String refreshToken) async =>
+      _pair('new');
+}
+
+void main() {
+  late _WebLikeStorage storage;
+  late SecureTokenStore tokens;
+  late AuthRepositoryImpl repository;
+
+  setUp(() {
+    storage = _WebLikeStorage()
+      ..values[AppConstants.tokenKey] = 'old-access'
+      ..values[AppConstants.refreshTokenKey] = 'old-refresh';
+    tokens = SecureTokenStore(storage.mock);
+    repository = AuthRepositoryImpl(
+      remoteDataSource: _Remote(),
+      localDataSource: AuthLocalDataSourceImpl(
+        storageService: InMemoryKeyValueStore(),
+        tokenStore: tokens,
+      ),
+    );
+  });
+
+  void expectStale(Result<String> result) {
+    expect(
+      (result as ResultFailure<String>).failure,
+      isA<Failure>().having(
+        (f) => f.code,
+        'code',
+        AuthErrorCodes.sessionTerminated,
+      ),
+    );
+  }
+
+  test('a clear issued after a pending write lands after it', () async {
+    final write = tokens.setAccessToken('late');
+    final clear = tokens.clearAllTokens();
+    await (write, clear).wait;
+
+    expect(storage.values[AppConstants.tokenKey], isNull);
+    expect(storage.values[AppConstants.refreshTokenKey], isNull);
+  });
+
+  test('a logout during the refresh write leaves no token behind', () async {
+    final started = storage.whenWriteStarts(AppConstants.tokenKey);
+    final refresh = repository.refreshToken();
+    await started;
+    final logout = repository.logout();
+
+    final (result, _) = await (refresh, logout).wait;
+
+    expect(await tokens.getAccessToken(), isNull);
+    expect(await tokens.getRefreshToken(), isNull);
+    expectStale(result);
+  });
+
+  test('a login completing after the stale refresh write keeps its own '
+      'tokens', () async {
+    final started = storage.whenWriteStarts(AppConstants.tokenKey);
+    final refresh = repository.refreshToken();
+    await started;
+    final login = repository.login('a@b.c', 'pw');
+
+    final (result, signedIn) = await (refresh, login).wait;
+
+    expect(signedIn, isA<Success<Object?>>());
+    expect(await tokens.getAccessToken(), 'login-access');
+    expect(await tokens.getRefreshToken(), 'login-refresh');
+    expectStale(result);
+  });
+}