Harden AuthInterceptor: retry guard, single-flight refresh, shared pref keys
Addresses PR review on the auth interceptor: - Guard against unbounded retry recursion: tag the replayed request with extra['auth_retried'] and skip the refresh/replay path on a second 401 so it can't recurse refresh -> replay indefinitely. Drop the redundant manual Authorization header on the replay (onRequest re-injects it). - Fix the concurrent-refresh race: coalesce concurrent 401s onto a single in-flight refresh future so the rotating refresh token is only consumed once, instead of later refreshes posting an already-consumed token and spuriously logging out. (Chose the shared-future approach over QueuedInterceptor, which deadlocks with the replay-via-same-Dio pattern.) - Centralise the 'bearer' / 'refresh_token' / 'customer_data' pref keys in a PrefKeys constants class used by the interceptor, network manager, auth service and router so they can't silently diverge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -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';
|
||||||
|
}
|
||||||
@@ -1,6 +1,7 @@
|
|||||||
import 'package:dio/dio.dart';
|
import 'package:dio/dio.dart';
|
||||||
import 'package:shared_preferences/shared_preferences.dart';
|
import 'package:shared_preferences/shared_preferences.dart';
|
||||||
|
|
||||||
|
import '../constants/pref_keys.dart';
|
||||||
import '../utils/logger.dart';
|
import '../utils/logger.dart';
|
||||||
|
|
||||||
/// Interceptor responsible for everything authentication-related on the wire:
|
/// Interceptor responsible for everything authentication-related on the wire:
|
||||||
@@ -12,6 +13,17 @@ import '../utils/logger.dart';
|
|||||||
///
|
///
|
||||||
/// The refresh / login endpoints are skipped so a failed login or a dead
|
/// The refresh / login endpoints are skipped so a failed login or a dead
|
||||||
/// refresh token does not recurse back into the refresh flow.
|
/// 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 {
|
class AuthInterceptor extends Interceptor {
|
||||||
AuthInterceptor({
|
AuthInterceptor({
|
||||||
required Dio dio,
|
required Dio dio,
|
||||||
@@ -40,10 +52,26 @@ class AuthInterceptor extends Interceptor {
|
|||||||
'/api/v1/profile/refresh-token',
|
'/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;
|
||||||
|
|
||||||
|
/// Refreshes the token, coalescing concurrent callers onto a single refresh.
|
||||||
|
Future<String?> _refreshTokenOnce() {
|
||||||
|
return _ongoingRefresh ??= _refreshToken().whenComplete(() {
|
||||||
|
_ongoingRefresh = null;
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
@override
|
@override
|
||||||
void onRequest(RequestOptions options, RequestInterceptorHandler handler) {
|
void onRequest(RequestOptions options, RequestInterceptorHandler handler) {
|
||||||
appLogger.info('🌐 Request: ${options.method} ${options.path}');
|
appLogger.info('🌐 Request: ${options.method} ${options.path}');
|
||||||
final String? token = _prefs.getString('bearer');
|
final String? token = _prefs.getString(PrefKeys.bearer);
|
||||||
if (token != null) {
|
if (token != null) {
|
||||||
options.headers['Authorization'] = 'Bearer $token';
|
options.headers['Authorization'] = 'Bearer $token';
|
||||||
}
|
}
|
||||||
@@ -69,21 +97,32 @@ class AuthInterceptor extends Interceptor {
|
|||||||
final bool isUnauthorized = error.response?.statusCode == 401;
|
final bool isUnauthorized = error.response?.statusCode == 401;
|
||||||
final String path = error.requestOptions.path;
|
final String path = error.requestOptions.path;
|
||||||
final bool isAuthEndpoint = _authPaths.any(path.contains);
|
final bool isAuthEndpoint = _authPaths.any(path.contains);
|
||||||
|
final bool alreadyRetried =
|
||||||
|
error.requestOptions.extra[_retriedFlag] == true;
|
||||||
|
|
||||||
// Only 401s on protected endpoints are interesting here.
|
// Only the first 401 on a protected endpoint is interesting here. A second
|
||||||
if (!isUnauthorized || isAuthEndpoint) {
|
// 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) {
|
if (isAuthEndpoint) {
|
||||||
appLogger.info('🔄 Skipping token refresh for authentication endpoint');
|
appLogger.info('🔄 Skipping token refresh for authentication endpoint');
|
||||||
|
} else if (alreadyRetried) {
|
||||||
|
appLogger.error('❌ Still 401 after refresh + replay, giving up');
|
||||||
}
|
}
|
||||||
return handler.next(error);
|
return handler.next(error);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Attempt a single refresh and replay the original request.
|
// Attempt a single refresh and replay the original request. The replay is
|
||||||
final String? newAccessToken = await _refreshToken();
|
// 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.
|
||||||
|
final String? newAccessToken = await _refreshTokenOnce();
|
||||||
if (newAccessToken != null) {
|
if (newAccessToken != null) {
|
||||||
try {
|
try {
|
||||||
final RequestOptions options = error.requestOptions
|
final RequestOptions options = error.requestOptions
|
||||||
..headers['Authorization'] = 'Bearer $newAccessToken';
|
..extra[_retriedFlag] = true;
|
||||||
final Response<dynamic> response = await _dio.fetch(options);
|
final Response<dynamic> response = await _dio.fetch(options);
|
||||||
return handler.resolve(response);
|
return handler.resolve(response);
|
||||||
} on DioException catch (e) {
|
} on DioException catch (e) {
|
||||||
@@ -109,8 +148,8 @@ class AuthInterceptor extends Interceptor {
|
|||||||
// at least clear the stored credentials so the router's auth guard can
|
// at least clear the stored credentials so the router's auth guard can
|
||||||
// redirect the user away from protected routes.
|
// redirect the user away from protected routes.
|
||||||
appLogger.error('⚠️ No auth-failed callback registered; clearing tokens');
|
appLogger.error('⚠️ No auth-failed callback registered; clearing tokens');
|
||||||
await _prefs.remove('bearer');
|
await _prefs.remove(PrefKeys.bearer);
|
||||||
await _prefs.remove('refresh_token');
|
await _prefs.remove(PrefKeys.refreshToken);
|
||||||
await _prefs.remove('customer_data');
|
await _prefs.remove(PrefKeys.customerData);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ 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 'package:tm_app/data/services/model/login_response/login_response_model.dart';
|
||||||
|
|
||||||
import '../config/app_config.dart';
|
import '../config/app_config.dart';
|
||||||
|
import '../constants/pref_keys.dart';
|
||||||
import '../utils/logger.dart';
|
import '../utils/logger.dart';
|
||||||
import '../utils/result.dart';
|
import '../utils/result.dart';
|
||||||
import 'app_exceptions.dart';
|
import 'app_exceptions.dart';
|
||||||
@@ -129,7 +130,7 @@ class NetworkManager {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
String? bearerToken = _prefs.getString('bearer');
|
String? bearerToken = _prefs.getString(PrefKeys.bearer);
|
||||||
// String? url = prefs.getString('url');
|
// String? url = prefs.getString('url');
|
||||||
|
|
||||||
// appLogger.info('🔗 Base URL: ${url ?? 'NOT SET'}');
|
// appLogger.info('🔗 Base URL: ${url ?? 'NOT SET'}');
|
||||||
@@ -306,7 +307,7 @@ class NetworkManager {
|
|||||||
/// Refresh the access token using the refresh token
|
/// Refresh the access token using the refresh token
|
||||||
Future<String?> refreshToken() async {
|
Future<String?> refreshToken() async {
|
||||||
try {
|
try {
|
||||||
final refreshToken = _prefs.getString('refresh_token');
|
final refreshToken = _prefs.getString(PrefKeys.refreshToken);
|
||||||
if (refreshToken == null) {
|
if (refreshToken == null) {
|
||||||
appLogger.error('❌ No refresh token available');
|
appLogger.error('❌ No refresh token available');
|
||||||
return null;
|
return null;
|
||||||
@@ -325,9 +326,9 @@ class NetworkManager {
|
|||||||
final newRefreshToken = loginResponse.data.refreshToken;
|
final newRefreshToken = loginResponse.data.refreshToken;
|
||||||
|
|
||||||
if (newAccessToken != null) {
|
if (newAccessToken != null) {
|
||||||
await _prefs.setString('bearer', newAccessToken);
|
await _prefs.setString(PrefKeys.bearer, newAccessToken);
|
||||||
if (newRefreshToken != null) {
|
if (newRefreshToken != null) {
|
||||||
await _prefs.setString('refresh_token', newRefreshToken);
|
await _prefs.setString(PrefKeys.refreshToken, newRefreshToken);
|
||||||
}
|
}
|
||||||
appLogger.info('✅ Token refreshed successfully');
|
appLogger.info('✅ Token refreshed successfully');
|
||||||
return newAccessToken;
|
return newAccessToken;
|
||||||
|
|||||||
@@ -18,6 +18,7 @@ import '../../views/profile/pages/profile_screen.dart';
|
|||||||
import '../../views/splash/pages/splash_screen.dart';
|
import '../../views/splash/pages/splash_screen.dart';
|
||||||
import '../../views/tenders/pages/tenders_screen.dart';
|
import '../../views/tenders/pages/tenders_screen.dart';
|
||||||
import '../../views/your_tenders/pages/your_tenders_screen.dart';
|
import '../../views/your_tenders/pages/your_tenders_screen.dart';
|
||||||
|
import '../constants/pref_keys.dart';
|
||||||
import '../providers/board_provider.dart';
|
import '../providers/board_provider.dart';
|
||||||
import '../providers/final_completion_provider.dart';
|
import '../providers/final_completion_provider.dart';
|
||||||
import '../providers/forgot_password_provider.dart';
|
import '../providers/forgot_password_provider.dart';
|
||||||
@@ -51,7 +52,7 @@ final GoRouter appRouter = GoRouter(
|
|||||||
navigatorKey: rootNavigatorKey,
|
navigatorKey: rootNavigatorKey,
|
||||||
redirect: (context, state) {
|
redirect: (context, state) {
|
||||||
final prefs = context.read<SharedPreferences>();
|
final prefs = context.read<SharedPreferences>();
|
||||||
final hasToken = prefs.getString('bearer') != null;
|
final hasToken = prefs.getString(PrefKeys.bearer) != null;
|
||||||
final path = state.uri.path;
|
final path = state.uri.path;
|
||||||
final location = state.matchedLocation;
|
final location = state.matchedLocation;
|
||||||
final isSplash = path == '/splash' || location == '/splash';
|
final isSplash = path == '/splash' || location == '/splash';
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import 'dart:convert';
|
|||||||
|
|
||||||
import 'package:firebase_messaging/firebase_messaging.dart';
|
import 'package:firebase_messaging/firebase_messaging.dart';
|
||||||
import 'package:shared_preferences/shared_preferences.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/core/utils/logger.dart';
|
||||||
import 'package:tm_app/data/services/api/auth_api.dart';
|
import 'package:tm_app/data/services/api/auth_api.dart';
|
||||||
import 'package:tm_app/data/services/model/customer/customer.dart';
|
import 'package:tm_app/data/services/model/customer/customer.dart';
|
||||||
@@ -58,13 +59,13 @@ class AuthService {
|
|||||||
final refreshToken = result.value.data.refreshToken;
|
final refreshToken = result.value.data.refreshToken;
|
||||||
final customer = result.value.data.customer;
|
final customer = result.value.data.customer;
|
||||||
|
|
||||||
await prefs.setString('bearer', token!);
|
await prefs.setString(PrefKeys.bearer, token!);
|
||||||
await prefs.setString('refresh_token', refreshToken!);
|
await prefs.setString(PrefKeys.refreshToken, refreshToken!);
|
||||||
|
|
||||||
// Save customer data as JSON string
|
// Save customer data as JSON string
|
||||||
if (customer != null) {
|
if (customer != null) {
|
||||||
final customerJson = jsonEncode(customer.toJson());
|
final customerJson = jsonEncode(customer.toJson());
|
||||||
await prefs.setString('customer_data', customerJson);
|
await prefs.setString(PrefKeys.customerData, customerJson);
|
||||||
AppLogger().info('👤 Customer data saved to preferences');
|
AppLogger().info('👤 Customer data saved to preferences');
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -102,9 +103,9 @@ class AuthService {
|
|||||||
|
|
||||||
Future<void> localLogout() async {
|
Future<void> localLogout() async {
|
||||||
final prefs = await SharedPreferences.getInstance();
|
final prefs = await SharedPreferences.getInstance();
|
||||||
await prefs.remove('bearer');
|
await prefs.remove(PrefKeys.bearer);
|
||||||
await prefs.remove('refresh_token');
|
await prefs.remove(PrefKeys.refreshToken);
|
||||||
await prefs.remove('customer_data');
|
await prefs.remove(PrefKeys.customerData);
|
||||||
appLogger.info('tokens and customer data cleared!');
|
appLogger.info('tokens and customer data cleared!');
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -141,7 +142,7 @@ class AuthService {
|
|||||||
|
|
||||||
Future<Result<bool>> checkIsLoggedIn() async {
|
Future<Result<bool>> checkIsLoggedIn() async {
|
||||||
final prefs = await SharedPreferences.getInstance();
|
final prefs = await SharedPreferences.getInstance();
|
||||||
final token = prefs.getString('bearer');
|
final token = prefs.getString(PrefKeys.bearer);
|
||||||
if (token == null) {
|
if (token == null) {
|
||||||
return const Result.ok(false);
|
return const Result.ok(false);
|
||||||
}
|
}
|
||||||
@@ -150,7 +151,7 @@ class AuthService {
|
|||||||
|
|
||||||
Future<Customer?> getStoredCustomer() async {
|
Future<Customer?> getStoredCustomer() async {
|
||||||
final prefs = await SharedPreferences.getInstance();
|
final prefs = await SharedPreferences.getInstance();
|
||||||
final customerJson = prefs.getString('customer_data');
|
final customerJson = prefs.getString(PrefKeys.customerData);
|
||||||
if (customerJson == null) {
|
if (customerJson == null) {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user