From 76dd23cb3ee05d77e72a4f2038ed3f41ec07f37f Mon Sep 17 00:00:00 2001 From: anivar Date: Fri, 26 Sep 2025 12:02:09 +0530 Subject: [PATCH] fix(auth): prevent random logouts by fixing overly aggressive token clearing Fixes the critical issue where users are randomly logged out due to TokenOrchestrator clearing tokens for ANY non-network error, including transient AWS service issues. Resolves #14534 The `handleErrors()` method was clearing tokens for all errors except NetworkError, causing users to be logged out during: - AWS service errors (500s) - Rate limiting (TooManyRequestsException) - Temporary service unavailability - Other transient errors that should be retried Modified token clearing logic to only clear tokens for definitive authentication failures that require re-authentication. Transient errors now preserve tokens, allowing for retry without forcing users to log in again. - Removed overly broad token clearing condition - Added `isAuthenticationError()` helper to identify auth failures - Only clears tokens for errors that definitively indicate invalid/expired tokens - Preserves tokens for all transient/retryable errors - `NotAuthorizedException` - Refresh token expired/invalid - `TokenRevokedException` - Token explicitly revoked - `UserNotFoundException` - User no longer exists - `PasswordResetRequiredException` - Password reset required - `UserNotConfirmedException` - Account not confirmed - `InternalErrorException` - AWS service errors - `TooManyRequestsException` - Rate limiting - `ThrottlingException` - Request throttling - `ServiceUnavailable` - Temporary service issues - `NetworkError` - Network connectivity issues - Any other transient or unknown errors - Added comprehensive test coverage for all error scenarios - Tests verify correct token clearing behavior for each error type - All existing tests pass This fix ensures users remain logged in during temporary AWS service issues while still properly handling genuine authentication failures. --- .../tokenProvider/tokenOrchestrator.test.ts | 101 ++++++++++++++++++ .../tokenProvider/TokenOrchestrator.ts | 26 ++++- 2 files changed, 124 insertions(+), 3 deletions(-) diff --git a/packages/auth/__tests__/providers/cognito/tokenProvider/tokenOrchestrator.test.ts b/packages/auth/__tests__/providers/cognito/tokenProvider/tokenOrchestrator.test.ts index d514772695a..db8fad4d0aa 100644 --- a/packages/auth/__tests__/providers/cognito/tokenProvider/tokenOrchestrator.test.ts +++ b/packages/auth/__tests__/providers/cognito/tokenProvider/tokenOrchestrator.test.ts @@ -96,6 +96,10 @@ describe('tokenOrchestrator', () => { }); describe('handleErrors method', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it('does not call clearTokens() if the error is a network error thrown from fetch handler', () => { const clearTokensSpy = jest.spyOn(tokenOrchestrator, 'clearTokens'); const error = new AmplifyError({ @@ -109,5 +113,102 @@ describe('tokenOrchestrator', () => { expect(clearTokensSpy).not.toHaveBeenCalled(); }); + + it('calls clearTokens() for NotAuthorizedException', () => { + const clearTokensSpy = jest.spyOn(tokenOrchestrator, 'clearTokens'); + const error = new AmplifyError({ + name: 'NotAuthorizedException', + message: 'Not authorized', + }); + + const result = (tokenOrchestrator as any).handleErrors(error); + + expect(clearTokensSpy).toHaveBeenCalled(); + expect(result).toBeNull(); + }); + + it('calls clearTokens() for TokenRevokedException', () => { + const clearTokensSpy = jest.spyOn(tokenOrchestrator, 'clearTokens'); + const error = new AmplifyError({ + name: 'TokenRevokedException', + message: 'Token revoked', + }); + + expect(() => { + (tokenOrchestrator as any).handleErrors(error); + }).toThrow(error); + + expect(clearTokensSpy).toHaveBeenCalled(); + }); + + it('calls clearTokens() for UserNotFoundException', () => { + const clearTokensSpy = jest.spyOn(tokenOrchestrator, 'clearTokens'); + const error = new AmplifyError({ + name: 'UserNotFoundException', + message: 'User not found', + }); + + expect(() => { + (tokenOrchestrator as any).handleErrors(error); + }).toThrow(error); + + expect(clearTokensSpy).toHaveBeenCalled(); + }); + + it('does not call clearTokens() for service errors (500)', () => { + const clearTokensSpy = jest.spyOn(tokenOrchestrator, 'clearTokens'); + const error = new AmplifyError({ + name: 'InternalServerError', + message: 'Internal server error', + }); + + expect(() => { + (tokenOrchestrator as any).handleErrors(error); + }).toThrow(error); + + expect(clearTokensSpy).not.toHaveBeenCalled(); + }); + + it('does not call clearTokens() for rate limit errors', () => { + const clearTokensSpy = jest.spyOn(tokenOrchestrator, 'clearTokens'); + const error = new AmplifyError({ + name: 'TooManyRequestsException', + message: 'Too many requests', + }); + + expect(() => { + (tokenOrchestrator as any).handleErrors(error); + }).toThrow(error); + + expect(clearTokensSpy).not.toHaveBeenCalled(); + }); + + it('does not call clearTokens() for throttling errors', () => { + const clearTokensSpy = jest.spyOn(tokenOrchestrator, 'clearTokens'); + const error = new AmplifyError({ + name: 'ThrottlingException', + message: 'Request throttled', + }); + + expect(() => { + (tokenOrchestrator as any).handleErrors(error); + }).toThrow(error); + + expect(clearTokensSpy).not.toHaveBeenCalled(); + }); + + it('does not call clearTokens() for temporary service issues', () => { + const clearTokensSpy = jest.spyOn(tokenOrchestrator, 'clearTokens'); + const error = new AmplifyError({ + name: 'ServiceUnavailable', + message: 'Service temporarily unavailable', + }); + + expect(() => { + (tokenOrchestrator as any).handleErrors(error); + }).toThrow(error); + + expect(clearTokensSpy).not.toHaveBeenCalled(); + }); }); }); diff --git a/packages/auth/src/providers/cognito/tokenProvider/TokenOrchestrator.ts b/packages/auth/src/providers/cognito/tokenProvider/TokenOrchestrator.ts index 3f8027d2596..b6b0cafdf57 100644 --- a/packages/auth/src/providers/cognito/tokenProvider/TokenOrchestrator.ts +++ b/packages/auth/src/providers/cognito/tokenProvider/TokenOrchestrator.ts @@ -9,7 +9,6 @@ import { } from '@aws-amplify/core'; import { AMPLIFY_SYMBOL, - AmplifyErrorCode, assertTokenProviderConfig, isBrowser, isTokenExpired, @@ -170,10 +169,15 @@ export class TokenOrchestrator implements AuthTokenOrchestrator { private handleErrors(err: unknown) { assertServiceError(err); - if (err.name !== AmplifyErrorCode.NetworkError) { - // TODO(v6): Check errors on client + + // Only clear tokens for definitive authentication failures + // Do NOT clear tokens for transient errors like service issues, rate limits, etc. + const shouldClearTokens = this.isAuthenticationError(err); + + if (shouldClearTokens) { this.clearTokens(); } + Hub.dispatch( 'auth', { @@ -190,6 +194,22 @@ export class TokenOrchestrator implements AuthTokenOrchestrator { throw err; } + private isAuthenticationError(err: any): boolean { + // Only clear tokens for errors that definitively indicate the tokens are invalid + // and re-authentication is required. All other errors (service errors, rate limits, etc.) + // should preserve the tokens to allow for retry. + // See: https://github.com/aws-amplify/amplify-js/issues/14534 + const authErrorNames = [ + 'NotAuthorizedException', // Refresh token is expired or invalid + 'TokenRevokedException', // Token was revoked by admin + 'UserNotFoundException', // User no longer exists + 'PasswordResetRequiredException', // User must reset password + 'UserNotConfirmedException', // User account is not confirmed + ]; + + return authErrorNames.some(errorName => err.name?.startsWith(errorName)); + } + async setTokens({ tokens }: { tokens: CognitoAuthTokens }) { return this.getTokenStore().storeTokens(tokens); }