From d9659a127c5944c1d489290cf94cfe1be43fd406 Mon Sep 17 00:00:00 2001 From: Bezerra Date: Mon, 26 Jan 2026 16:27:24 -0300 Subject: [PATCH 1/2] test(auth): add comprehensive token refresh test coverage (#14618) Add test cases for the getTokens() method in TokenOrchestrator to verify token refresh behavior when tokens expire. These tests prove that: - Expired access tokens trigger automatic refresh - Expired ID tokens trigger automatic refresh - forceRefresh option works correctly with valid tokens - signInDetails are preserved after token refresh - NotAuthorizedException returns null and clears tokens - Network errors are thrown (not swallowed) - clientMetadata is passed to the token refresher - New tokens are stored after successful refresh All 12 new tests pass, confirming the core token refresh logic works as expected. This suggests issues reported in #14618 may be related to specific user configurations rather than the refresh mechanism itself. Co-Authored-By: Claude Opus 4.5 --- .../tokenProvider/tokenOrchestrator.test.ts | 226 ++++++++++++++++++ 1 file changed, 226 insertions(+) diff --git a/packages/auth/__tests__/providers/cognito/tokenProvider/tokenOrchestrator.test.ts b/packages/auth/__tests__/providers/cognito/tokenProvider/tokenOrchestrator.test.ts index 6f287d1f6c1..64526406227 100644 --- a/packages/auth/__tests__/providers/cognito/tokenProvider/tokenOrchestrator.test.ts +++ b/packages/auth/__tests__/providers/cognito/tokenProvider/tokenOrchestrator.test.ts @@ -18,6 +18,44 @@ jest.mock('@aws-amplify/core', () => ({ })); jest.mock('../../../../src/providers/cognito/utils/oauth/oAuthStore'); +// Helper function for creating test tokens with configurable expiration +function createTokens({ + accessTokenExpired = false, + idTokenExpired = false, + overrides = {}, +}: { + accessTokenExpired?: boolean; + idTokenExpired?: boolean; + overrides?: Partial; +} = {}): CognitoAuthTokens { + const now = Math.floor(Date.now() / 1000); + const pastExp = now - 3600; // 1 hour ago + const futureExp = now + 3600; // 1 hour from now + + return { + accessToken: { + payload: { + exp: accessTokenExpired ? pastExp : futureExp, + iat: accessTokenExpired ? pastExp - 3600 : now, + }, + toString: () => + accessTokenExpired ? 'mock-expired-access-token' : 'mock-access-token', + }, + idToken: { + payload: { + exp: idTokenExpired ? pastExp : futureExp, + iat: idTokenExpired ? pastExp - 3600 : now, + }, + toString: () => + idTokenExpired ? 'mock-expired-id-token' : 'mock-id-token', + }, + refreshToken: 'mock-refresh-token', + clockDrift: 0, + username: 'testuser', + ...overrides, + }; +} + describe('tokenOrchestrator', () => { const mockTokenRefresher = jest.fn(); const mockTokenStore = { @@ -225,4 +263,192 @@ describe('tokenOrchestrator', () => { expect(clearTokensSpy).not.toHaveBeenCalled(); }); }); + + describe('getTokens method', () => { + const mockAuthConfig = { + Cognito: { + userPoolId: 'us-east-1_testpool', + userPoolClientId: 'testclientid', + }, + }; + + beforeEach(() => { + tokenOrchestrator.setAuthConfig(mockAuthConfig); + jest.clearAllMocks(); + (oAuthStore.loadOAuthInFlight as jest.Mock).mockResolvedValue(false); + }); + + it('should return null when no tokens are stored', async () => { + mockTokenStore.loadTokens.mockResolvedValue(null); + + const result = await tokenOrchestrator.getTokens(); + + expect(result).toBeNull(); + expect(mockTokenRefresher).not.toHaveBeenCalled(); + }); + + it('should return tokens without refresh when tokens are valid', async () => { + const validTokens = createTokens(); + mockTokenStore.loadTokens.mockResolvedValue(validTokens); + mockTokenStore.getLastAuthUser.mockResolvedValue('testuser'); + + const result = await tokenOrchestrator.getTokens(); + + expect(mockTokenRefresher).not.toHaveBeenCalled(); + expect(result?.accessToken).toBeDefined(); + expect(result?.idToken).toBeDefined(); + }); + + it.each([ + [ + 'access token is expired', + { accessTokenExpired: true, idTokenExpired: false }, + ], + [ + 'ID token is expired', + { accessTokenExpired: false, idTokenExpired: true }, + ], + [ + 'both tokens are expired', + { accessTokenExpired: true, idTokenExpired: true }, + ], + ])('should trigger refresh when %s', async (_scenario, tokenConfig) => { + const expiredTokens = createTokens(tokenConfig); + const newTokens = createTokens(); + mockTokenStore.loadTokens.mockResolvedValue(expiredTokens); + mockTokenStore.getLastAuthUser.mockResolvedValue('testuser'); + mockTokenRefresher.mockResolvedValue(newTokens); + + const result = await tokenOrchestrator.getTokens(); + + expect(mockTokenRefresher).toHaveBeenCalledWith( + expect.objectContaining({ + tokens: expiredTokens, + username: 'testuser', + }), + ); + expect(result?.accessToken).toEqual(newTokens.accessToken); + }); + + it('should trigger refresh when forceRefresh is true even with valid tokens', async () => { + const validTokens = createTokens(); + const newTokens = createTokens(); + mockTokenStore.loadTokens.mockResolvedValue(validTokens); + mockTokenStore.getLastAuthUser.mockResolvedValue('testuser'); + mockTokenRefresher.mockResolvedValue(newTokens); + + const result = await tokenOrchestrator.getTokens({ forceRefresh: true }); + + expect(mockTokenRefresher).toHaveBeenCalledWith( + expect.objectContaining({ + tokens: validTokens, + username: 'testuser', + }), + ); + expect(result?.accessToken).toEqual(newTokens.accessToken); + }); + + it('should preserve signInDetails after token refresh', async () => { + const expiredTokens = createTokens({ + accessTokenExpired: true, + overrides: { + signInDetails: { + authFlowType: 'USER_SRP_AUTH', + loginId: 'testuser', + }, + }, + }); + const newTokens = createTokens(); + + mockTokenStore.loadTokens.mockResolvedValue(expiredTokens); + mockTokenStore.getLastAuthUser.mockResolvedValue('testuser'); + mockTokenRefresher.mockResolvedValue(newTokens); + + const result = await tokenOrchestrator.getTokens(); + + expect(result?.signInDetails?.authFlowType).toBe('USER_SRP_AUTH'); + expect(result?.signInDetails?.loginId).toBe('testuser'); + }); + + it('should return null when refresh fails with NotAuthorizedException', async () => { + const expiredTokens = createTokens({ accessTokenExpired: true }); + mockTokenStore.loadTokens.mockResolvedValue(expiredTokens); + mockTokenStore.getLastAuthUser.mockResolvedValue('testuser'); + mockTokenRefresher.mockRejectedValue( + new AmplifyError({ + name: 'NotAuthorizedException', + message: 'Refresh token has expired', + }), + ); + + const result = await tokenOrchestrator.getTokens(); + + expect(result).toBeNull(); + expect(mockTokenStore.clearTokens).toHaveBeenCalled(); + }); + + it('should throw error when refresh fails with network error', async () => { + const expiredTokens = createTokens({ accessTokenExpired: true }); + mockTokenStore.loadTokens.mockResolvedValue(expiredTokens); + mockTokenStore.getLastAuthUser.mockResolvedValue('testuser'); + mockTokenRefresher.mockRejectedValue( + new AmplifyError({ + name: AmplifyErrorCode.NetworkError, + message: 'Network Error', + }), + ); + + await expect(tokenOrchestrator.getTokens()).rejects.toThrow( + 'Network Error', + ); + expect(mockTokenStore.clearTokens).not.toHaveBeenCalled(); + }); + + it('should not refresh tokens when idToken is missing but accessToken is valid', async () => { + const tokensWithoutIdToken = createTokens(); + delete (tokensWithoutIdToken as any).idToken; + mockTokenStore.loadTokens.mockResolvedValue(tokensWithoutIdToken); + mockTokenStore.getLastAuthUser.mockResolvedValue('testuser'); + + const result = await tokenOrchestrator.getTokens(); + + expect(mockTokenRefresher).not.toHaveBeenCalled(); + expect(result?.accessToken).toBeDefined(); + expect(result?.idToken).toBeUndefined(); + }); + + it('should pass clientMetadata to token refresher', async () => { + const expiredTokens = createTokens({ accessTokenExpired: true }); + const newTokens = createTokens(); + const clientMetadata = { customKey: 'customValue' }; + mockTokenStore.loadTokens.mockResolvedValue(expiredTokens); + mockTokenStore.getLastAuthUser.mockResolvedValue('testuser'); + mockTokenRefresher.mockResolvedValue(newTokens); + + await tokenOrchestrator.getTokens({ clientMetadata }); + + expect(mockTokenRefresher).toHaveBeenCalledWith( + expect.objectContaining({ + clientMetadata, + }), + ); + }); + + it('should store new tokens after successful refresh', async () => { + const expiredTokens = createTokens({ accessTokenExpired: true }); + const newTokens = createTokens(); + mockTokenStore.loadTokens.mockResolvedValue(expiredTokens); + mockTokenStore.getLastAuthUser.mockResolvedValue('testuser'); + mockTokenRefresher.mockResolvedValue(newTokens); + + await tokenOrchestrator.getTokens(); + + expect(mockTokenStore.storeTokens).toHaveBeenCalledWith( + expect.objectContaining({ + accessToken: newTokens.accessToken, + idToken: newTokens.idToken, + }), + ); + }); + }); }); From 45db6c316543927e03bac73db16f80a9e9abc153 Mon Sep 17 00:00:00 2001 From: Bezerra Date: Mon, 26 Jan 2026 17:55:41 -0300 Subject: [PATCH 2/2] fix(auth): handle NaN clockDrift to fix token auto-refresh (#14618) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix automatic token refresh failing silently when clockDrift is NaN. This addresses the issue where fetchAuthSession() does not auto-refresh expired tokens for CUSTOM_WITHOUT_SRP auth flow. ## Root Cause When clockDrift is NaN, the token expiration check always returns false: ```javascript // isTokenExpired.ts (before) return currentTime + clockDrift + tolerance > expiresAt; // currentTime + NaN + tolerance = NaN // NaN > expiresAt = false (always) ``` This causes expired tokens to never be detected as expired, preventing automatic refresh. ## How clockDrift becomes NaN In TokenStore.loadTokens(), clockDrift is parsed from storage: ```javascript const clockDriftString = (await storage.getItem(key)) ?? '0'; const clockDrift = Number.parseInt(clockDriftString); ``` The nullish coalescing operator (??) only handles null/undefined, not empty strings. If clockDrift is stored as "" (empty string): | Stored Value | ?? '0' result | parseInt() result | |--------------|---------------|-------------------| | null | '0' | 0 ✓ | | '123' | '123' | 123 ✓ | | '' | '' (falsy!) | NaN ✗ | This can happen with custom KeyValueStorage implementations or when certain auth flows don't properly initialize the clockDrift value. ## Fix Added three-layer defense against NaN clockDrift: 1. **TokenStore.ts**: Sanitize at load time ```javascript const parsedClockDrift = Number.parseInt(clockDriftString); const clockDrift = Number.isNaN(parsedClockDrift) ? 0 : parsedClockDrift; ``` 2. **TokenOrchestrator.ts**: Use || instead of ?? for fallback ```javascript clockDrift: tokens.clockDrift || 0 // NaN || 0 = 0 // vs clockDrift: tokens.clockDrift ?? 0 // NaN ?? 0 = NaN ``` 3. **isTokenExpired.ts**: Final safety net ```javascript const safeClockDrift = Number.isNaN(clockDrift) ? 0 : clockDrift; ``` ## Test Coverage Added comprehensive tests for clockDrift edge cases: - NaN clockDrift with expired tokens triggers refresh - NaN clockDrift with valid tokens does not trigger refresh - undefined clockDrift with expired tokens triggers refresh - Positive/negative/zero clockDrift handled correctly Co-Authored-By: Claude Opus 4.5 --- .../tokenProvider/tokenOrchestrator.test.ts | 110 ++++++++++++++++++ .../tokenProvider/TokenOrchestrator.ts | 4 +- .../cognito/tokenProvider/TokenStore.ts | 3 +- .../__tests__/utils/isTokenExpired.test.ts | 54 +++++++++ packages/core/src/utils/isTokenExpired.ts | 4 +- 5 files changed, 171 insertions(+), 4 deletions(-) diff --git a/packages/auth/__tests__/providers/cognito/tokenProvider/tokenOrchestrator.test.ts b/packages/auth/__tests__/providers/cognito/tokenProvider/tokenOrchestrator.test.ts index 64526406227..64cb17404a1 100644 --- a/packages/auth/__tests__/providers/cognito/tokenProvider/tokenOrchestrator.test.ts +++ b/packages/auth/__tests__/providers/cognito/tokenProvider/tokenOrchestrator.test.ts @@ -450,5 +450,115 @@ describe('tokenOrchestrator', () => { }), ); }); + + describe('clockDrift edge cases', () => { + it('should trigger refresh when clockDrift is NaN and token is expired', async () => { + const now = Math.floor(Date.now() / 1000); + const pastExp = now - 3600; // 1 hour ago + const expiredTokensWithNaNClockDrift: CognitoAuthTokens = { + accessToken: { + payload: { + exp: pastExp, + iat: pastExp - 3600, + }, + toString: () => 'mock-expired-access-token', + }, + idToken: { + payload: { + exp: pastExp, + iat: pastExp - 3600, + }, + toString: () => 'mock-expired-id-token', + }, + refreshToken: 'mock-refresh-token', + clockDrift: NaN, + username: 'testuser', + }; + const newTokens = createTokens(); + mockTokenStore.loadTokens.mockResolvedValue( + expiredTokensWithNaNClockDrift, + ); + mockTokenStore.getLastAuthUser.mockResolvedValue('testuser'); + mockTokenRefresher.mockResolvedValue(newTokens); + + const result = await tokenOrchestrator.getTokens(); + + expect(mockTokenRefresher).toHaveBeenCalledWith( + expect.objectContaining({ + tokens: expiredTokensWithNaNClockDrift, + username: 'testuser', + }), + ); + expect(result?.accessToken).toEqual(newTokens.accessToken); + }); + + it('should not trigger refresh when clockDrift is NaN but token is still valid', async () => { + const now = Math.floor(Date.now() / 1000); + const futureExp = now + 3600; // 1 hour from now + const validTokensWithNaNClockDrift: CognitoAuthTokens = { + accessToken: { + payload: { + exp: futureExp, + iat: now, + }, + toString: () => 'mock-access-token', + }, + idToken: { + payload: { + exp: futureExp, + iat: now, + }, + toString: () => 'mock-id-token', + }, + refreshToken: 'mock-refresh-token', + clockDrift: NaN, + username: 'testuser', + }; + mockTokenStore.loadTokens.mockResolvedValue( + validTokensWithNaNClockDrift, + ); + mockTokenStore.getLastAuthUser.mockResolvedValue('testuser'); + + const result = await tokenOrchestrator.getTokens(); + + expect(mockTokenRefresher).not.toHaveBeenCalled(); + expect(result?.accessToken).toBeDefined(); + }); + + it('should trigger refresh when clockDrift is undefined and token is expired', async () => { + const now = Math.floor(Date.now() / 1000); + const pastExp = now - 3600; // 1 hour ago + const expiredTokensWithUndefinedClockDrift = { + accessToken: { + payload: { + exp: pastExp, + iat: pastExp - 3600, + }, + toString: () => 'mock-expired-access-token', + }, + idToken: { + payload: { + exp: pastExp, + iat: pastExp - 3600, + }, + toString: () => 'mock-expired-id-token', + }, + refreshToken: 'mock-refresh-token', + clockDrift: undefined, + username: 'testuser', + } as unknown as CognitoAuthTokens; + const newTokens = createTokens(); + mockTokenStore.loadTokens.mockResolvedValue( + expiredTokensWithUndefinedClockDrift, + ); + mockTokenStore.getLastAuthUser.mockResolvedValue('testuser'); + mockTokenRefresher.mockResolvedValue(newTokens); + + const result = await tokenOrchestrator.getTokens(); + + expect(mockTokenRefresher).toHaveBeenCalled(); + expect(result?.accessToken).toEqual(newTokens.accessToken); + }); + }); }); }); diff --git a/packages/auth/src/providers/cognito/tokenProvider/TokenOrchestrator.ts b/packages/auth/src/providers/cognito/tokenProvider/TokenOrchestrator.ts index 7077e42c483..7e16e775fca 100644 --- a/packages/auth/src/providers/cognito/tokenProvider/TokenOrchestrator.ts +++ b/packages/auth/src/providers/cognito/tokenProvider/TokenOrchestrator.ts @@ -126,11 +126,11 @@ export class TokenOrchestrator implements AuthTokenOrchestrator { !!tokens?.idToken && isTokenExpired({ expiresAt: (tokens.idToken?.payload?.exp ?? 0) * 1000, - clockDrift: tokens.clockDrift ?? 0, + clockDrift: tokens.clockDrift || 0, }); const accessTokenExpired = isTokenExpired({ expiresAt: (tokens.accessToken?.payload?.exp ?? 0) * 1000, - clockDrift: tokens.clockDrift ?? 0, + clockDrift: tokens.clockDrift || 0, }); if (options?.forceRefresh || idTokenExpired || accessTokenExpired) { diff --git a/packages/auth/src/providers/cognito/tokenProvider/TokenStore.ts b/packages/auth/src/providers/cognito/tokenProvider/TokenStore.ts index dae3ec26ff7..f2a101f7936 100644 --- a/packages/auth/src/providers/cognito/tokenProvider/TokenStore.ts +++ b/packages/auth/src/providers/cognito/tokenProvider/TokenStore.ts @@ -70,7 +70,8 @@ export class DefaultTokenStore implements AuthTokenStore { const clockDriftString = (await this.getKeyValueStorage().getItem(authKeys.clockDrift)) ?? '0'; - const clockDrift = Number.parseInt(clockDriftString); + const parsedClockDrift = Number.parseInt(clockDriftString); + const clockDrift = Number.isNaN(parsedClockDrift) ? 0 : parsedClockDrift; const signInDetails = await this.getKeyValueStorage().getItem( authKeys.signInDetails, diff --git a/packages/core/__tests__/utils/isTokenExpired.test.ts b/packages/core/__tests__/utils/isTokenExpired.test.ts index 04a61b297ae..e18e3c508b4 100644 --- a/packages/core/__tests__/utils/isTokenExpired.test.ts +++ b/packages/core/__tests__/utils/isTokenExpired.test.ts @@ -53,4 +53,58 @@ describe('isTokenExpired', () => { expect(result).toBe(false); }); + + describe('clockDrift edge cases', () => { + it('should treat NaN clockDrift as 0 and correctly detect expired token', () => { + const result = isTokenExpired({ + expiresAt: Date.now() - 1000, // expired 1 second ago + clockDrift: NaN, + tolerance: 0, + }); + + expect(result).toBe(true); + }); + + it('should treat NaN clockDrift as 0 and correctly detect valid token', () => { + const result = isTokenExpired({ + expiresAt: Date.now() + 10000, // expires in 10 seconds + clockDrift: NaN, + tolerance: 0, + }); + + expect(result).toBe(false); + }); + + it('should handle positive clockDrift correctly', () => { + // With positive clockDrift, effective time is ahead + const result = isTokenExpired({ + expiresAt: Date.now() + 1000, // expires in 1 second + clockDrift: 2000, // but we're 2 seconds ahead + tolerance: 0, + }); + + expect(result).toBe(true); + }); + + it('should handle negative clockDrift correctly', () => { + // With negative clockDrift, effective time is behind + const result = isTokenExpired({ + expiresAt: Date.now() - 1000, // expired 1 second ago + clockDrift: -2000, // but we're 2 seconds behind + tolerance: 0, + }); + + expect(result).toBe(false); + }); + + it('should handle zero clockDrift correctly', () => { + const result = isTokenExpired({ + expiresAt: Date.now() + 1000, + clockDrift: 0, + tolerance: 0, + }); + + expect(result).toBe(false); + }); + }); }); diff --git a/packages/core/src/utils/isTokenExpired.ts b/packages/core/src/utils/isTokenExpired.ts index 67b5b00f915..8ae5b4ff034 100644 --- a/packages/core/src/utils/isTokenExpired.ts +++ b/packages/core/src/utils/isTokenExpired.ts @@ -11,6 +11,8 @@ export function isTokenExpired({ tolerance?: number; }): boolean { const currentTime = Date.now(); + // Treat NaN clockDrift as 0 for safety (NaN comparisons always return false) + const safeClockDrift = Number.isNaN(clockDrift) ? 0 : clockDrift; - return currentTime + clockDrift + tolerance > expiresAt; + return currentTime + safeClockDrift + tolerance > expiresAt; }