Merge pull request 'Refactor network request handling by introducing AuthInterceptor for improved authentication management. Removed the previous request interceptor logic to streamline the code and enhance maintainability.' (#227) from auth-interceptor into develop

Reviewed-on: https://repo.ravanertebat.com/TM/tm_app/pulls/227
Reviewed-by: Hadi Barzegar <barzagarhadi@gmail.com>

lib/core/constants/pref_keys.dart

@@ -0,0 +1,12 @@
+/// Keys used for values persisted in [SharedPreferences].
+///
+/// Centralised here so the auth flow, the network layer and the router all
+/// agree on the same key. Renaming a key in one place would otherwise silently
+/// diverge from the others (e.g. the interceptor's defensive logout fallback).
+class PrefKeys {
+  PrefKeys._();
+
+  static const String bearer = 'bearer';
+  static const String refreshToken = 'refresh_token';
+  static const String customerData = 'customer_data';
+}

lib/core/network/auth_interceptor.dart

@@ -0,0 +1,189 @@
+import 'package:dio/dio.dart';
+import 'package:shared_preferences/shared_preferences.dart';
+
+import '../constants/pref_keys.dart';
+import '../utils/logger.dart';
+
+/// Interceptor responsible for everything authentication-related on the wire:
+///
+/// * injects the stored bearer token into every outgoing request, and
+/// * reacts to `401 Unauthorized` responses by attempting a single token
+///   refresh, replaying the original request, and—if that fails—logging the
+///   user out and navigating them to the login screen via [onAuthFailed].
+///
+/// The refresh / login endpoints are skipped so a failed login or a dead
+/// refresh token does not recurse back into the refresh flow.
+///
+/// When several in-flight requests `401` at once after the token expires, only
+/// the first one performs the refresh; the rest await the same in-flight
+/// [_refreshTokenOnce] future. This is important because [refreshToken] rotates
+/// the refresh token, so two concurrent refreshes would have the second post an
+/// already-consumed token, fail, and spuriously log the user out.
+///
+/// (Dio's [QueuedInterceptor] would serialize error handling, but it deadlocks
+/// with the replay-via-the-same-Dio pattern used here: the suspended `onError`
+/// keeps the error queue busy while it awaits `_dio.fetch`, so a replay that
+/// also 401s can never be dequeued. Sharing the refresh future avoids that.)
+class AuthInterceptor extends Interceptor {
+  AuthInterceptor({
+    required Dio dio,
+    required SharedPreferences prefs,
+    required Future<String?> Function() refreshToken,
+    required Future<void> Function()? Function() onAuthFailed,
+  }) : _dio = dio,
+       _prefs = prefs,
+       _refreshToken = refreshToken,
+       _onAuthFailed = onAuthFailed;
+
+  final Dio _dio;
+  final SharedPreferences _prefs;
+
+  /// Refreshes the access token, returning the new token or `null` on failure.
+  final Future<String?> Function() _refreshToken;
+
+  /// Resolves the (lazily-wired) logout/navigation callback. It is a getter so
+  /// that the callback can be registered after the interceptor is created,
+  /// without leaving the interceptor holding a stale `null` reference.
+  final Future<void> Function()? Function() _onAuthFailed;
+
+  /// Endpoints for which a 401 must NOT trigger a token refresh.
+  static const List<String> _authPaths = <String>[
+    '/api/v1/profile/login',
+    '/api/v1/profile/refresh-token',
+  ];
+
+  /// Marks a request that has already been replayed once after a refresh, so a
+  /// second 401 on the replay does not recurse into another refresh + replay.
+  static const String _retriedFlag = 'auth_retried';
+
+  /// The refresh currently in flight, shared across all requests that 401 while
+  /// it is running so the token is only refreshed (and rotated) once. Cleared
+  /// when the refresh completes, allowing a later expiry to refresh again.
+  Future<String?>? _ongoingRefresh;
+
+  /// Guards against the auth-failed callback running once per concurrent caller.
+  /// When the shared refresh fails, every request awaiting it falls through to
+  /// [_logout]; only the first should fire the (possibly non-idempotent)
+  /// callback. Set synchronously before the first `await` so the rest of the
+  /// burst short-circuits, and reset afterwards so a later session can log out.
+  bool _loggingOut = false;
+
+  /// Refreshes the token, coalescing concurrent callers onto a single refresh.
+  Future<String?> _refreshTokenOnce() {
+    return _ongoingRefresh ??= _refreshToken().whenComplete(() {
+      _ongoingRefresh = null;
+    });
+  }
+
+  @override
+  void onRequest(RequestOptions options, RequestInterceptorHandler handler) {
+    appLogger.info('🌐 Request: ${options.method} ${options.path}');
+    final String? token = _prefs.getString(PrefKeys.bearer);
+    if (token != null) {
+      options.headers['Authorization'] = 'Bearer $token';
+    }
+    handler.next(options);
+  }
+
+  @override
+  void onResponse(Response response, ResponseInterceptorHandler handler) {
+    appLogger.info(
+      '✅ Response: ${response.statusCode} ${response.requestOptions.path}',
+    );
+    handler.next(response);
+  }
+
+  @override
+  Future<void> onError(
+    DioException error,
+    ErrorInterceptorHandler handler,
+  ) async {
+    appLogger.error('❌ Error: ${error.message} - ${error.requestOptions.path}');
+    appLogger.error('❌ Error type: ${error.type}');
+
+    final bool isUnauthorized = error.response?.statusCode == 401;
+    final String path = error.requestOptions.path;
+    final bool isAuthEndpoint = _authPaths.any(path.contains);
+    final bool alreadyRetried =
+        error.requestOptions.extra[_retriedFlag] == true;
+
+    // Only the first 401 on a protected endpoint is interesting here. A second
+    // 401 on the already-replayed request means refreshing won't help (revoked
+    // permissions, disabled account, clock skew), so just propagate the error.
+    // The outer onError that issued the replay catches it below and performs the
+    // logout once — handling it here too would log the user out twice.
+    if (!isUnauthorized || isAuthEndpoint || alreadyRetried) {
+      if (isAuthEndpoint) {
+        appLogger.info('🔄 Skipping token refresh for authentication endpoint');
+      } else if (alreadyRetried) {
+        appLogger.error('❌ Still 401 after refresh + replay, giving up');
+      }
+      return handler.next(error);
+    }
+
+    // Attempt a single refresh and replay the original request. The replay is
+    // tagged so a fresh 401 on it short-circuits the block above rather than
+    // refreshing again. The Authorization header is intentionally not set here:
+    // [onRequest] re-injects the (now refreshed) bearer token on the replay.
+    //
+    // Refresh-window straggler guard: a request sent with the old token can 401
+    // just after a concurrent refresh already finished (so [_ongoingRefresh] is
+    // cleared and the single-flight no longer coalesces it). If the stored token
+    // no longer matches the one this request was sent with, it was already
+    // rotated by someone else — replay with the fresh token instead of starting
+    // a second refresh that would needlessly rotate the just-issued token again.
+    final String? storedToken = _prefs.getString(PrefKeys.bearer);
+    final String? sentAuthHeader =
+        error.requestOptions.headers['Authorization'] as String?;
+    final bool alreadyRefreshed =
+        storedToken != null && sentAuthHeader != 'Bearer $storedToken';
+
+    final String? newAccessToken =
+        alreadyRefreshed ? storedToken : await _refreshTokenOnce();
+    if (newAccessToken != null) {
+      try {
+        final RequestOptions options = error.requestOptions
+          ..extra[_retriedFlag] = true;
+        final Response<dynamic> response = await _dio.fetch(options);
+        return handler.resolve(response);
+      } on DioException catch (e) {
+        // Only a fresh 401 means refreshing didn't help (token revoked, account
+        // disabled) — fall through to logout. Anything else (timeout, connection
+        // drop, 5xx) is a transient/server failure unrelated to auth: propagate
+        // it so a flaky network doesn't wipe a perfectly valid session, and so
+        // the caller sees the real error instead of the stale pre-refresh 401.
+        if (e.response?.statusCode != 401) {
+          return handler.next(e);
+        }
+        appLogger.error('❌ Retry after token refresh failed: ${e.message}');
+      }
+    }
+
+    // Refresh (or the replay) failed → force logout and go to login.
+    appLogger.error('❌ Authentication failed, logging out');
+    await _logout();
+    return handler.next(error);
+  }
+
+  Future<void> _logout() async {
+    if (_loggingOut) return;
+    _loggingOut = true;
+    try {
+      final Future<void> Function()? callback = _onAuthFailed();
+      if (callback != null) {
+        await callback();
+        return;
+      }
+
+      // Defensive fallback: if no logout/navigation callback has been wired yet,
+      // at least clear the stored credentials so the router's auth guard can
+      // redirect the user away from protected routes.
+      appLogger.error('⚠️ No auth-failed callback registered; clearing tokens');
+      await _prefs.remove(PrefKeys.bearer);
+      await _prefs.remove(PrefKeys.refreshToken);
+      await _prefs.remove(PrefKeys.customerData);
+    } finally {
+      _loggingOut = false;
+    }
+  }
+}

lib/core/network/network_manager.dart

@@ -6,9 +6,11 @@ import 'package:tm_app/data/services/model/error/error_model.dart';
 import 'package:tm_app/data/services/model/login_response/login_response_model.dart';
 
 import '../config/app_config.dart';
+import '../constants/pref_keys.dart';
 import '../utils/logger.dart';
 import '../utils/result.dart';
 import 'app_exceptions.dart';
+import 'auth_interceptor.dart';
 import 'network_connectivity.dart';
 
 class NetworkManager {
@@ -45,54 +47,15 @@ class NetworkManager {
       return;
     }
 
-    final requestInterceptor = InterceptorsWrapper(
-      onRequest: (options, handler) {
-        appLogger.info('🌐 Request: ${options.method} ${options.path}');
-        String? token = _prefs.getString('bearer');
-        if (token != null) {
-          options.headers['Authorization'] = 'Bearer $token';
-        }
-        return handler.next(options);
-      },
-      onResponse: (response, handler) {
-        appLogger.info(
-          '✅ Response: ${response.statusCode} ${response.requestOptions.path}',
+    mainDio.interceptors.add(
+      AuthInterceptor(
+        dio: mainDio,
+        prefs: _prefs,
+        refreshToken: refreshToken,
+        // Resolved lazily so the callback can be wired up after construction.
+        onAuthFailed: () => _onAuthFailed,
+      ),
     );
-
-        return handler.next(response);
-      },
-      onError: (error, handler) async {
-        appLogger.error(
-          '❌ Error: ${error.message} - ${error.requestOptions.path}',
-        );
-        appLogger.error('❌ Error type: ${error.type}');
-
-        if (error.response?.statusCode == 401) {
-          // Skip refresh token for login and refresh token endpoints
-          final path = error.requestOptions.path;
-          if (path.contains('/api/v1/profile/login') ||
-              path.contains('/api/v1/profile/refresh-token')) {
-            appLogger.info(
-              '🔄 Skipping token refresh for authentication endpoints',
-            );
-            return handler.next(error);
-          }
-
-          final newAccessToken = await refreshToken();
-          if (newAccessToken != null) {
-            mainDio.options.headers['Authorization'] = 'Bearer $newAccessToken';
-            return handler.resolve(await mainDio.fetch(error.requestOptions));
-          }
-
-          // Refresh token failed – force logout
-          appLogger.error('❌ Refresh token failed, triggering logout');
-          await _onAuthFailed!();
-        }
-        return handler.next(error);
-      },
-    );
-
-    mainDio.interceptors.add(requestInterceptor);
     _interceptorsAdded = true;
   }
 
@@ -167,7 +130,7 @@ class NetworkManager {
         }
       }
 
-      String? bearerToken = _prefs.getString('bearer');
+      String? bearerToken = _prefs.getString(PrefKeys.bearer);
       // String? url = prefs.getString('url');
 
       // appLogger.info('🔗 Base URL: ${url ?? 'NOT SET'}');
@@ -344,7 +307,7 @@ class NetworkManager {
   /// Refresh the access token using the refresh token
   Future<String?> refreshToken() async {
     try {
-      final refreshToken = _prefs.getString('refresh_token');
+      final refreshToken = _prefs.getString(PrefKeys.refreshToken);
       if (refreshToken == null) {
         appLogger.error('❌ No refresh token available');
         return null;
@@ -363,9 +326,9 @@ class NetworkManager {
         final newRefreshToken = loginResponse.data.refreshToken;
 
         if (newAccessToken != null) {
-          await _prefs.setString('bearer', newAccessToken);
+          await _prefs.setString(PrefKeys.bearer, newAccessToken);
           if (newRefreshToken != null) {
-            await _prefs.setString('refresh_token', newRefreshToken);
+            await _prefs.setString(PrefKeys.refreshToken, newRefreshToken);
           }
           appLogger.info('✅ Token refreshed successfully');
           return newAccessToken;

lib/core/routes/app_routes.dart

@@ -18,6 +18,7 @@ import '../../views/profile/pages/profile_screen.dart';
 import '../../views/splash/pages/splash_screen.dart';
 import '../../views/tenders/pages/tenders_screen.dart';
 import '../../views/your_tenders/pages/your_tenders_screen.dart';
+import '../constants/pref_keys.dart';
 import '../providers/board_provider.dart';
 import '../providers/final_completion_provider.dart';
 import '../providers/forgot_password_provider.dart';
@@ -51,7 +52,7 @@ final GoRouter appRouter = GoRouter(
   navigatorKey: rootNavigatorKey,
   redirect: (context, state) {
     final prefs = context.read<SharedPreferences>();
-    final hasToken = prefs.getString('bearer') != null;
+    final hasToken = prefs.getString(PrefKeys.bearer) != null;
     final path = state.uri.path;
     final location = state.matchedLocation;
     final isSplash = path == '/splash' || location == '/splash';

lib/data/services/auth_service.dart

@@ -4,6 +4,7 @@ import 'dart:convert';
 
 import 'package:firebase_messaging/firebase_messaging.dart';
 import 'package:shared_preferences/shared_preferences.dart';
+import 'package:tm_app/core/constants/pref_keys.dart';
 import 'package:tm_app/core/utils/logger.dart';
 import 'package:tm_app/data/services/api/auth_api.dart';
 import 'package:tm_app/data/services/model/customer/customer.dart';
@@ -58,13 +59,13 @@ class AuthService {
       final refreshToken = result.value.data.refreshToken;
       final customer = result.value.data.customer;
 
-      await prefs.setString('bearer', token!);
-      await prefs.setString('refresh_token', refreshToken!);
+      await prefs.setString(PrefKeys.bearer, token!);
+      await prefs.setString(PrefKeys.refreshToken, refreshToken!);
 
       // Save customer data as JSON string
       if (customer != null) {
         final customerJson = jsonEncode(customer.toJson());
-        await prefs.setString('customer_data', customerJson);
+        await prefs.setString(PrefKeys.customerData, customerJson);
         AppLogger().info('👤 Customer data saved to preferences');
       }
     }
@@ -102,9 +103,9 @@ class AuthService {
 
   Future<void> localLogout() async {
     final prefs = await SharedPreferences.getInstance();
-    await prefs.remove('bearer');
-    await prefs.remove('refresh_token');
-    await prefs.remove('customer_data');
+    await prefs.remove(PrefKeys.bearer);
+    await prefs.remove(PrefKeys.refreshToken);
+    await prefs.remove(PrefKeys.customerData);
     appLogger.info('tokens and customer data cleared!');
   }
 
@@ -141,7 +142,7 @@ class AuthService {
 
   Future<Result<bool>> checkIsLoggedIn() async {
     final prefs = await SharedPreferences.getInstance();
-    final token = prefs.getString('bearer');
+    final token = prefs.getString(PrefKeys.bearer);
     if (token == null) {
       return const Result.ok(false);
     }
@@ -150,7 +151,7 @@ class AuthService {
 
   Future<Customer?> getStoredCustomer() async {
     final prefs = await SharedPreferences.getInstance();
-    final customerJson = prefs.getString('customer_data');
+    final customerJson = prefs.getString(PrefKeys.customerData);
     if (customerJson == null) {
       return null;
     }

test/core/network/auth_interceptor_test.dart

@@ -0,0 +1,166 @@
+import 'dart:convert';
+import 'dart:typed_data';
+
+import 'package:dio/dio.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:shared_preferences/shared_preferences.dart';
+import 'package:tm_app/core/constants/pref_keys.dart';
+import 'package:tm_app/core/network/auth_interceptor.dart';
+import 'package:tm_app/core/utils/logger.dart';
+
+/// A configurable [HttpClientAdapter] that lets each test decide, per request,
+/// what the wire should return. Avoids a mocking dependency the repo doesn't
+/// pull in while still exercising the real Dio request/replay machinery.
+class _FakeAdapter implements HttpClientAdapter {
+  _FakeAdapter(this.responder);
+
+  Future<ResponseBody> Function(RequestOptions options) responder;
+  int requestCount = 0;
+
+  @override
+  Future<ResponseBody> fetch(
+    RequestOptions options,
+    Stream<Uint8List>? requestStream,
+    Future<void>? cancelFuture,
+  ) {
+    requestCount++;
+    return responder(options);
+  }
+
+  @override
+  void close({bool force = false}) {}
+}
+
+ResponseBody _json(int status, [Map<String, dynamic> body = const {}]) {
+  return ResponseBody.fromString(
+    jsonEncode(body),
+    status,
+    headers: <String, List<String>>{
+      Headers.contentTypeHeader: <String>[Headers.jsonContentType],
+    },
+  );
+}
+
+void main() {
+  TestWidgetsFlutterBinding.ensureInitialized();
+  // The interceptor logs through the app-wide singleton, which is `late`-init.
+  appLogger.init();
+
+  late Dio dio;
+  late SharedPreferences prefs;
+  late int refreshCalls;
+  late int logoutCalls;
+
+  setUp(() async {
+    SharedPreferences.setMockInitialValues(<String, Object>{
+      PrefKeys.bearer: 'old-token',
+      PrefKeys.refreshToken: 'old-refresh',
+    });
+    prefs = await SharedPreferences.getInstance();
+    refreshCalls = 0;
+    logoutCalls = 0;
+    dio = Dio(BaseOptions(baseUrl: 'https://example.test'));
+  });
+
+  /// Refresh that succeeds: rotates the stored bearer and returns the new token,
+  /// mirroring the production refresh which persists before replaying.
+  Future<String?> refreshSucceeds() async {
+    refreshCalls++;
+    // A small delay so concurrent 401s both register on the single-flight
+    // future before it resolves, exercising the coalescing path for real.
+    await Future<void>.delayed(const Duration(milliseconds: 10));
+    await prefs.setString(PrefKeys.bearer, 'new-token');
+    return 'new-token';
+  }
+
+  void attachInterceptor({Future<String?> Function()? refreshToken}) {
+    dio.interceptors.add(
+      AuthInterceptor(
+        dio: dio,
+        prefs: prefs,
+        refreshToken: refreshToken ?? refreshSucceeds,
+        onAuthFailed: () => () async {
+          logoutCalls++;
+          await prefs.clear();
+        },
+      ),
+    );
+  }
+
+  test(
+    'retry guard: a replay that still 401s does not trigger a second refresh',
+    () async {
+      // Every request — original and replay — comes back 401.
+      dio.httpClientAdapter = _FakeAdapter(
+        (RequestOptions options) async => _json(401, <String, dynamic>{
+          'message': 'unauthorized',
+        }),
+      );
+      attachInterceptor();
+
+      await expectLater(
+        dio.get<dynamic>('/api/v1/protected'),
+        throwsA(isA<DioException>()),
+      );
+
+      // The refresh + replay happens exactly once; the second 401 short-circuits
+      // via the auth_retried flag instead of refreshing again.
+      expect(refreshCalls, 1);
+      expect(logoutCalls, 1);
+    },
+  );
+
+  test('single-flight: concurrent 401s share a single refresh', () async {
+    // First attempt 401s; the tagged replay succeeds.
+    dio.httpClientAdapter = _FakeAdapter((RequestOptions options) async {
+      final bool isReplay = options.extra['auth_retried'] == true;
+      return _json(isReplay ? 200 : 401, <String, dynamic>{'ok': isReplay});
+    });
+    attachInterceptor();
+
+    final List<Response<dynamic>> responses = await Future.wait(
+      <Future<Response<dynamic>>>[
+        dio.get<dynamic>('/api/v1/a'),
+        dio.get<dynamic>('/api/v1/b'),
+      ],
+    );
+
+    expect(refreshCalls, 1);
+    expect(logoutCalls, 0);
+    expect(
+      responses.every((Response<dynamic> r) => r.statusCode == 200),
+      isTrue,
+    );
+  });
+
+  test(
+    'non-401 replay failure propagates and does not log the user out',
+    () async {
+      // Refresh succeeds, but the replay hits a transient/server error (a 500
+      // here stands in for the timeout/connection-drop case): authentication is
+      // fine, so the error must surface to the caller rather than wipe the
+      // session.
+      dio.httpClientAdapter = _FakeAdapter((RequestOptions options) async {
+        final bool isReplay = options.extra['auth_retried'] == true;
+        return _json(isReplay ? 500 : 401);
+      });
+      attachInterceptor();
+
+      await expectLater(
+        dio.get<dynamic>('/api/v1/protected'),
+        throwsA(
+          isA<DioException>().having(
+            (DioException e) => e.response?.statusCode,
+            'replay status code',
+            500,
+          ),
+        ),
+      );
+
+      expect(refreshCalls, 1);
+      expect(logoutCalls, 0);
+      // Session left intact for the retry — credentials were not cleared.
+      expect(prefs.getString(PrefKeys.bearer), 'new-token');
+    },
+  );
+}