Update ADR for OIDC token refresh
This commit is contained in:
@@ -82,7 +82,9 @@ void _internalLog(
|
|||||||
|
|
||||||
if (shouldSentry) {
|
if (shouldSentry) {
|
||||||
if (level == Level.trace) {
|
if (level == Level.trace) {
|
||||||
SentryManager.instance.captureMessage(rawMessage, extras: extras);
|
unawaited(
|
||||||
|
SentryManager.instance.captureMessage(rawMessage, extras: extras),
|
||||||
|
);
|
||||||
} else {
|
} else {
|
||||||
unawaited(
|
unawaited(
|
||||||
SentryManager.instance.captureException(
|
SentryManager.instance.captureException(
|
||||||
@@ -208,11 +210,13 @@ void logDebug(
|
|||||||
void logTrace(
|
void logTrace(
|
||||||
String? message, {
|
String? message, {
|
||||||
bool webConsoleEnabled = false,
|
bool webConsoleEnabled = false,
|
||||||
|
Map<String, dynamic>? extras,
|
||||||
}) {
|
}) {
|
||||||
_internalLog(
|
_internalLog(
|
||||||
message,
|
message,
|
||||||
level: Level.trace,
|
level: Level.trace,
|
||||||
webConsoleEnabled: webConsoleEnabled,
|
webConsoleEnabled: webConsoleEnabled,
|
||||||
|
extras: extras,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -4,9 +4,9 @@ Date: 2023-09-11
|
|||||||
|
|
||||||
## Status
|
## Status
|
||||||
|
|
||||||
- Issue:
|
Superseded by [ADR-0035](0035-error-handling-on-no-longer-valid-oidc-token.md)
|
||||||
|
|
||||||
[1974](https://github.com/linagora/tmail-flutter/issues/1974)
|
- Issue: [1974](https://github.com/linagora/tmail-flutter/issues/1974)
|
||||||
|
|
||||||
## Context
|
## Context
|
||||||
|
|
||||||
|
|||||||
@@ -1,19 +1,37 @@
|
|||||||
# 35. Error handling on no longer valid OIDC token (Issue #2592)
|
# 35. OIDC Token Refresh Mechanism in AuthorizationInterceptors
|
||||||
|
|
||||||
Date: 2024-02-15
|
Date: 2024-02-15
|
||||||
|
Updated: 2026-02-09
|
||||||
|
|
||||||
## Status
|
## Status
|
||||||
|
|
||||||
Accepted
|
Accepted (supersedes [ADR-0031](0031-fix-refresh-token-with-oidc.md))
|
||||||
|
|
||||||
## Context
|
## Context
|
||||||
|
|
||||||
- When my OIDC token is expired, I see an awful red error message while being redirected
|
Multiple issues drove the evolution of the refresh token logic:
|
||||||
|
|
||||||
|
1. **Concurrent 401s caused logout** ([#1974](https://github.com/linagora/tmail-flutter/issues/1974)) — parallel requests with stale headers all triggered independent refresh attempts.
|
||||||
|
2. **Ugly error on expired token** ([#2592](https://github.com/linagora/tmail-flutter/issues/2592)) — users saw a red error instead of a silent redirect.
|
||||||
|
3. **400 on refresh not handled** — when the refresh token itself was revoked/expired, the server returned 400 (invalid_grant) but the interceptor did not distinguish this from transient errors, leading to confusing failures.
|
||||||
|
4. **Server-side revocation before local expiry** — relying on local `isExpired` check caused the interceptor to skip refresh when the server had already revoked the token.
|
||||||
|
|
||||||
## Decision
|
## Decision
|
||||||
|
|
||||||
- When my OIDC token is expired, I am just redirected to the OIDC provider portal. No need to show an awful error.
|
`AuthorizationInterceptors` extends `QueuedInterceptorsWrapper` (Dio). The `onError` handler processes errors one at a time with the following ordered checks:
|
||||||
|
|
||||||
|
1. **`_refreshAttemptedKey` guard** — if the request already attempted a refresh/retry, skip everything and propagate the error. Prevents infinite loops.
|
||||||
|
2. **`validateToRetryTheRequestWithNewToken`** (checked first) — only on **401** responses: if `_token` was already updated by a preceding queued request, retry immediately with the new token. No refresh call needed. Non-401 errors (500, 403, etc.) are never retried here — they are server errors unrelated to authentication.
|
||||||
|
3. **`validateToRefreshToken`** — if status is 401, auth type is OIDC, and access/refresh tokens are present, attempt refresh. The local `isExpired` check was **removed**; we trust the server's 401.
|
||||||
|
- **Success (different token):** update `_token`, persist to cache, mark `_refreshAttemptedKey`, retry.
|
||||||
|
- **Success (same token — duplicate):** propagate original error, no retry.
|
||||||
|
- **`DioException` 400:** call `clear()` (wipe auth state) and reject with `RefreshTokenFailedException` — triggers silent logout.
|
||||||
|
- **`DioException` other:** propagate original error, preserve OIDC state.
|
||||||
|
4. **Outer catch:** `ServerError`/`TemporarilyUnavailable` wrapped in `DioException`; other exceptions forwarded via `err.copyWith`.
|
||||||
|
|
||||||
## Consequences
|
## Consequences
|
||||||
|
|
||||||
- In case of receiving a `BadCredentials` error, the system automatically logs out and returns to the login screen without any notification to the user.
|
- Queued requests after a successful refresh retry without redundant refresh calls.
|
||||||
|
- 400 from the OIDC provider causes immediate session cleanup and silent redirect to login.
|
||||||
|
- No infinite retry loops thanks to the `_refreshAttemptedKey` per-request flag.
|
||||||
|
- Server-side token revocation is handled correctly regardless of local expiry time.
|
||||||
|
|||||||
@@ -91,13 +91,10 @@ class AuthorizationInterceptors extends QueuedInterceptorsWrapper {
|
|||||||
final extraInRequest = requestOptions.extra;
|
final extraInRequest = requestOptions.extra;
|
||||||
bool isRetryRequest = false;
|
bool isRetryRequest = false;
|
||||||
|
|
||||||
// Check if this request has already attempted a refresh/retry
|
|
||||||
final hasAttemptedRefresh = extraInRequest[_refreshAttemptedKey] == true;
|
final hasAttemptedRefresh = extraInRequest[_refreshAttemptedKey] == true;
|
||||||
|
|
||||||
// FIRST: Check if token was already updated by another request in the queue
|
|
||||||
// If so, just retry with the new token - no refresh needed
|
|
||||||
// But skip if we've already attempted (to prevent infinite loops)
|
|
||||||
if (!hasAttemptedRefresh && validateToRetryTheRequestWithNewToken(
|
if (!hasAttemptedRefresh && validateToRetryTheRequestWithNewToken(
|
||||||
|
responseStatusCode: err.response?.statusCode,
|
||||||
authHeader: requestOptions.headers[HttpHeaders.authorizationHeader],
|
authHeader: requestOptions.headers[HttpHeaders.authorizationHeader],
|
||||||
tokenOIDC: _token
|
tokenOIDC: _token
|
||||||
)) {
|
)) {
|
||||||
@@ -107,7 +104,6 @@ class AuthorizationInterceptors extends QueuedInterceptorsWrapper {
|
|||||||
responseStatusCode: err.response?.statusCode,
|
responseStatusCode: err.response?.statusCode,
|
||||||
tokenOIDC: _token
|
tokenOIDC: _token
|
||||||
)) {
|
)) {
|
||||||
// SECOND: Check if we should attempt to refresh the token
|
|
||||||
try {
|
try {
|
||||||
log('AuthorizationInterceptors::onError: Perform get New Token');
|
log('AuthorizationInterceptors::onError: Perform get New Token');
|
||||||
final newTokenOidc = PlatformInfo.isIOS
|
final newTokenOidc = PlatformInfo.isIOS
|
||||||
@@ -126,7 +122,6 @@ class AuthorizationInterceptors extends QueuedInterceptorsWrapper {
|
|||||||
await _iosSharingManager.saveKeyChainSharingSession(personalAccount);
|
await _iosSharingManager.saveKeyChainSharingSession(personalAccount);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Mark that we've attempted refresh for this request
|
|
||||||
requestOptions.extra[_refreshAttemptedKey] = true;
|
requestOptions.extra[_refreshAttemptedKey] = true;
|
||||||
isRetryRequest = true;
|
isRetryRequest = true;
|
||||||
} on DioException catch (refreshError, st) {
|
} on DioException catch (refreshError, st) {
|
||||||
@@ -149,12 +144,30 @@ class AuthorizationInterceptors extends QueuedInterceptorsWrapper {
|
|||||||
return handler.reject(sessionExpiredError);
|
return handler.reject(sessionExpiredError);
|
||||||
}
|
}
|
||||||
|
|
||||||
return super.onError(err, handler);
|
logError(
|
||||||
|
'AuthorizationInterceptors: Refresh token failed with '
|
||||||
|
'statusCode=${refreshError.response?.statusCode}',
|
||||||
|
exception: refreshError,
|
||||||
|
stackTrace: st,
|
||||||
|
);
|
||||||
|
|
||||||
|
if (refreshError is ServerError ||
|
||||||
|
refreshError is TemporarilyUnavailable) {
|
||||||
|
return super.onError(
|
||||||
|
DioException(
|
||||||
|
requestOptions: err.requestOptions,
|
||||||
|
error: refreshError,
|
||||||
|
),
|
||||||
|
handler,
|
||||||
|
);
|
||||||
|
} else {
|
||||||
|
return super.onError(err.copyWith(error: refreshError), handler);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
logTrace(
|
logTrace(
|
||||||
'AuthorizationInterceptors::onError: '
|
'AuthorizationInterceptors::onError: '
|
||||||
'401 received but refresh skipped. '
|
'No retry or refresh applicable. '
|
||||||
'statusCode = ${err.response?.statusCode} | '
|
'statusCode = ${err.response?.statusCode} | '
|
||||||
'authType = $_authenticationType | '
|
'authType = $_authenticationType | '
|
||||||
'hasConfig = ${_configOIDC != null} | '
|
'hasConfig = ${_configOIDC != null} | '
|
||||||
@@ -249,9 +262,6 @@ class AuthorizationInterceptors extends QueuedInterceptorsWrapper {
|
|||||||
final hasAccessToken = _isTokenNotEmpty(tokenOIDC);
|
final hasAccessToken = _isTokenNotEmpty(tokenOIDC);
|
||||||
final hasRefreshToken = _isRefreshTokenNotEmpty(tokenOIDC);
|
final hasRefreshToken = _isRefreshTokenNotEmpty(tokenOIDC);
|
||||||
|
|
||||||
// Note: We removed isExpired check. If server returns 401, we trust it
|
|
||||||
// and attempt refresh regardless of local expiry time. This handles cases
|
|
||||||
// where server clock is ahead or token was revoked server-side.
|
|
||||||
final canProceedRefresh = isStatusCode401 &&
|
final canProceedRefresh = isStatusCode401 &&
|
||||||
isLoginWithOIDC &&
|
isLoginWithOIDC &&
|
||||||
hasAccessToken &&
|
hasAccessToken &&
|
||||||
@@ -271,21 +281,25 @@ class AuthorizationInterceptors extends QueuedInterceptorsWrapper {
|
|||||||
}
|
}
|
||||||
|
|
||||||
bool validateToRetryTheRequestWithNewToken(
|
bool validateToRetryTheRequestWithNewToken(
|
||||||
{required String? authHeader, required TokenOIDC? tokenOIDC}) {
|
{required int? responseStatusCode,
|
||||||
|
required String? authHeader,
|
||||||
|
required TokenOIDC? tokenOIDC}) {
|
||||||
|
final isStatusCode401 = responseStatusCode == 401;
|
||||||
final hasAuthHeader = authHeader != null;
|
final hasAuthHeader = authHeader != null;
|
||||||
final hasAccessToken = _isTokenNotEmpty(tokenOIDC);
|
final hasAccessToken = _isTokenNotEmpty(tokenOIDC);
|
||||||
final isTokenStillValid = !_isTokenExpired(tokenOIDC);
|
final isTokenStillValid = !_isTokenExpired(tokenOIDC);
|
||||||
final isTokenUpdated =
|
final isTokenUpdated =
|
||||||
tokenOIDC != null && authHeader?.contains(tokenOIDC.token) != true;
|
tokenOIDC != null && authHeader?.contains(tokenOIDC.token) != true;
|
||||||
|
|
||||||
// Note: We don't check isTokenExpired here. If another request already
|
final shouldRetry = isStatusCode401 &&
|
||||||
// refreshed the token, we should retry with the new token regardless of
|
hasAuthHeader &&
|
||||||
// its expiry status. The key check is isTokenUpdated.
|
hasAccessToken &&
|
||||||
final shouldRetry =
|
isTokenStillValid &&
|
||||||
hasAuthHeader && hasAccessToken && isTokenStillValid && isTokenUpdated;
|
isTokenUpdated;
|
||||||
|
|
||||||
logTrace(
|
logTrace(
|
||||||
'AuthorizationInterceptors::validateToRetryWithNewToken: '
|
'AuthorizationInterceptors::validateToRetryWithNewToken: '
|
||||||
|
'isStatusCode401 = $isStatusCode401 | '
|
||||||
'hasHeader = $hasAuthHeader | '
|
'hasHeader = $hasAuthHeader | '
|
||||||
'hasAccessToken = $hasAccessToken | '
|
'hasAccessToken = $hasAccessToken | '
|
||||||
'isTokenValid = $isTokenStillValid | '
|
'isTokenValid = $isTokenStillValid | '
|
||||||
|
|||||||
@@ -225,10 +225,11 @@ void main() {
|
|||||||
// ============================================================
|
// ============================================================
|
||||||
group('validateToRetryTheRequestWithNewToken', () {
|
group('validateToRetryTheRequestWithNewToken', () {
|
||||||
test(
|
test(
|
||||||
'should return TRUE when auth header present, token updated, and token not expired',
|
'should return TRUE when 401, auth header present, token updated, and token not expired',
|
||||||
() {
|
() {
|
||||||
final result =
|
final result =
|
||||||
authorizationInterceptors.validateToRetryTheRequestWithNewToken(
|
authorizationInterceptors.validateToRetryTheRequestWithNewToken(
|
||||||
|
responseStatusCode: 401,
|
||||||
authHeader: 'Bearer old_token',
|
authHeader: 'Bearer old_token',
|
||||||
tokenOIDC: OIDCFixtures.newTokenOidc,
|
tokenOIDC: OIDCFixtures.newTokenOidc,
|
||||||
);
|
);
|
||||||
@@ -237,9 +238,32 @@ void main() {
|
|||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
|
test('should return FALSE when status code is not 401 (e.g. 500)', () {
|
||||||
|
final result =
|
||||||
|
authorizationInterceptors.validateToRetryTheRequestWithNewToken(
|
||||||
|
responseStatusCode: 500,
|
||||||
|
authHeader: 'Bearer old_token',
|
||||||
|
tokenOIDC: OIDCFixtures.newTokenOidc,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(result, false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('should return FALSE when status code is null', () {
|
||||||
|
final result =
|
||||||
|
authorizationInterceptors.validateToRetryTheRequestWithNewToken(
|
||||||
|
responseStatusCode: null,
|
||||||
|
authHeader: 'Bearer old_token',
|
||||||
|
tokenOIDC: OIDCFixtures.newTokenOidc,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(result, false);
|
||||||
|
});
|
||||||
|
|
||||||
test('should return FALSE when auth header is null', () {
|
test('should return FALSE when auth header is null', () {
|
||||||
final result =
|
final result =
|
||||||
authorizationInterceptors.validateToRetryTheRequestWithNewToken(
|
authorizationInterceptors.validateToRetryTheRequestWithNewToken(
|
||||||
|
responseStatusCode: 401,
|
||||||
authHeader: null,
|
authHeader: null,
|
||||||
tokenOIDC: OIDCFixtures.newTokenOidc,
|
tokenOIDC: OIDCFixtures.newTokenOidc,
|
||||||
);
|
);
|
||||||
@@ -252,6 +276,7 @@ void main() {
|
|||||||
() {
|
() {
|
||||||
final result =
|
final result =
|
||||||
authorizationInterceptors.validateToRetryTheRequestWithNewToken(
|
authorizationInterceptors.validateToRetryTheRequestWithNewToken(
|
||||||
|
responseStatusCode: 401,
|
||||||
authHeader: 'Bearer ${OIDCFixtures.newTokenOidc.token}',
|
authHeader: 'Bearer ${OIDCFixtures.newTokenOidc.token}',
|
||||||
tokenOIDC: OIDCFixtures.newTokenOidc,
|
tokenOIDC: OIDCFixtures.newTokenOidc,
|
||||||
);
|
);
|
||||||
@@ -263,6 +288,7 @@ void main() {
|
|||||||
test('should return FALSE when token is expired', () {
|
test('should return FALSE when token is expired', () {
|
||||||
final result =
|
final result =
|
||||||
authorizationInterceptors.validateToRetryTheRequestWithNewToken(
|
authorizationInterceptors.validateToRetryTheRequestWithNewToken(
|
||||||
|
responseStatusCode: 401,
|
||||||
authHeader: 'Bearer some_other_token',
|
authHeader: 'Bearer some_other_token',
|
||||||
tokenOIDC: OIDCFixtures.tokenOidcExpiredTime,
|
tokenOIDC: OIDCFixtures.tokenOidcExpiredTime,
|
||||||
);
|
);
|
||||||
@@ -273,6 +299,7 @@ void main() {
|
|||||||
test('should return FALSE when token is empty', () {
|
test('should return FALSE when token is empty', () {
|
||||||
final result =
|
final result =
|
||||||
authorizationInterceptors.validateToRetryTheRequestWithNewToken(
|
authorizationInterceptors.validateToRetryTheRequestWithNewToken(
|
||||||
|
responseStatusCode: 401,
|
||||||
authHeader: 'Bearer some_token',
|
authHeader: 'Bearer some_token',
|
||||||
tokenOIDC: OIDCFixtures.tokenOidcExpiredTimeAndTokenEmpty,
|
tokenOIDC: OIDCFixtures.tokenOidcExpiredTimeAndTokenEmpty,
|
||||||
);
|
);
|
||||||
@@ -283,6 +310,7 @@ void main() {
|
|||||||
test('should return FALSE when tokenOIDC is null', () {
|
test('should return FALSE when tokenOIDC is null', () {
|
||||||
final result =
|
final result =
|
||||||
authorizationInterceptors.validateToRetryTheRequestWithNewToken(
|
authorizationInterceptors.validateToRetryTheRequestWithNewToken(
|
||||||
|
responseStatusCode: 401,
|
||||||
authHeader: 'Bearer some_token',
|
authHeader: 'Bearer some_token',
|
||||||
tokenOIDC: null,
|
tokenOIDC: null,
|
||||||
);
|
);
|
||||||
|
|||||||
Reference in New Issue
Block a user