Skip to content

Commit af45f26

Browse files
pgbezerraclaude
andcommitted
fix(auth): handle NaN clockDrift to fix token auto-refresh (#14618)
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 <noreply@anthropic.com>
1 parent df5b60e commit af45f26

File tree

5 files changed

+171
-4
lines changed

5 files changed

+171
-4
lines changed

packages/auth/__tests__/providers/cognito/tokenProvider/tokenOrchestrator.test.ts

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -450,5 +450,115 @@ describe('tokenOrchestrator', () => {
450450
}),
451451
);
452452
});
453+
454+
describe('clockDrift edge cases', () => {
455+
it('should trigger refresh when clockDrift is NaN and token is expired', async () => {
456+
const now = Math.floor(Date.now() / 1000);
457+
const pastExp = now - 3600; // 1 hour ago
458+
const expiredTokensWithNaNClockDrift: CognitoAuthTokens = {
459+
accessToken: {
460+
payload: {
461+
exp: pastExp,
462+
iat: pastExp - 3600,
463+
},
464+
toString: () => 'mock-expired-access-token',
465+
},
466+
idToken: {
467+
payload: {
468+
exp: pastExp,
469+
iat: pastExp - 3600,
470+
},
471+
toString: () => 'mock-expired-id-token',
472+
},
473+
refreshToken: 'mock-refresh-token',
474+
clockDrift: NaN,
475+
username: 'testuser',
476+
};
477+
const newTokens = createTokens();
478+
mockTokenStore.loadTokens.mockResolvedValue(
479+
expiredTokensWithNaNClockDrift,
480+
);
481+
mockTokenStore.getLastAuthUser.mockResolvedValue('testuser');
482+
mockTokenRefresher.mockResolvedValue(newTokens);
483+
484+
const result = await tokenOrchestrator.getTokens();
485+
486+
expect(mockTokenRefresher).toHaveBeenCalledWith(
487+
expect.objectContaining({
488+
tokens: expiredTokensWithNaNClockDrift,
489+
username: 'testuser',
490+
}),
491+
);
492+
expect(result?.accessToken).toEqual(newTokens.accessToken);
493+
});
494+
495+
it('should not trigger refresh when clockDrift is NaN but token is still valid', async () => {
496+
const now = Math.floor(Date.now() / 1000);
497+
const futureExp = now + 3600; // 1 hour from now
498+
const validTokensWithNaNClockDrift: CognitoAuthTokens = {
499+
accessToken: {
500+
payload: {
501+
exp: futureExp,
502+
iat: now,
503+
},
504+
toString: () => 'mock-access-token',
505+
},
506+
idToken: {
507+
payload: {
508+
exp: futureExp,
509+
iat: now,
510+
},
511+
toString: () => 'mock-id-token',
512+
},
513+
refreshToken: 'mock-refresh-token',
514+
clockDrift: NaN,
515+
username: 'testuser',
516+
};
517+
mockTokenStore.loadTokens.mockResolvedValue(
518+
validTokensWithNaNClockDrift,
519+
);
520+
mockTokenStore.getLastAuthUser.mockResolvedValue('testuser');
521+
522+
const result = await tokenOrchestrator.getTokens();
523+
524+
expect(mockTokenRefresher).not.toHaveBeenCalled();
525+
expect(result?.accessToken).toBeDefined();
526+
});
527+
528+
it('should trigger refresh when clockDrift is undefined and token is expired', async () => {
529+
const now = Math.floor(Date.now() / 1000);
530+
const pastExp = now - 3600; // 1 hour ago
531+
const expiredTokensWithUndefinedClockDrift = {
532+
accessToken: {
533+
payload: {
534+
exp: pastExp,
535+
iat: pastExp - 3600,
536+
},
537+
toString: () => 'mock-expired-access-token',
538+
},
539+
idToken: {
540+
payload: {
541+
exp: pastExp,
542+
iat: pastExp - 3600,
543+
},
544+
toString: () => 'mock-expired-id-token',
545+
},
546+
refreshToken: 'mock-refresh-token',
547+
clockDrift: undefined,
548+
username: 'testuser',
549+
} as unknown as CognitoAuthTokens;
550+
const newTokens = createTokens();
551+
mockTokenStore.loadTokens.mockResolvedValue(
552+
expiredTokensWithUndefinedClockDrift,
553+
);
554+
mockTokenStore.getLastAuthUser.mockResolvedValue('testuser');
555+
mockTokenRefresher.mockResolvedValue(newTokens);
556+
557+
const result = await tokenOrchestrator.getTokens();
558+
559+
expect(mockTokenRefresher).toHaveBeenCalled();
560+
expect(result?.accessToken).toEqual(newTokens.accessToken);
561+
});
562+
});
453563
});
454564
});

packages/auth/src/providers/cognito/tokenProvider/TokenOrchestrator.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,11 +126,11 @@ export class TokenOrchestrator implements AuthTokenOrchestrator {
126126
!!tokens?.idToken &&
127127
isTokenExpired({
128128
expiresAt: (tokens.idToken?.payload?.exp ?? 0) * 1000,
129-
clockDrift: tokens.clockDrift ?? 0,
129+
clockDrift: tokens.clockDrift || 0,
130130
});
131131
const accessTokenExpired = isTokenExpired({
132132
expiresAt: (tokens.accessToken?.payload?.exp ?? 0) * 1000,
133-
clockDrift: tokens.clockDrift ?? 0,
133+
clockDrift: tokens.clockDrift || 0,
134134
});
135135

136136
if (options?.forceRefresh || idTokenExpired || accessTokenExpired) {

packages/auth/src/providers/cognito/tokenProvider/TokenStore.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ export class DefaultTokenStore implements AuthTokenStore {
7070

7171
const clockDriftString =
7272
(await this.getKeyValueStorage().getItem(authKeys.clockDrift)) ?? '0';
73-
const clockDrift = Number.parseInt(clockDriftString);
73+
const parsedClockDrift = Number.parseInt(clockDriftString);
74+
const clockDrift = Number.isNaN(parsedClockDrift) ? 0 : parsedClockDrift;
7475

7576
const signInDetails = await this.getKeyValueStorage().getItem(
7677
authKeys.signInDetails,

packages/core/__tests__/utils/isTokenExpired.test.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,4 +53,58 @@ describe('isTokenExpired', () => {
5353

5454
expect(result).toBe(false);
5555
});
56+
57+
describe('clockDrift edge cases', () => {
58+
it('should treat NaN clockDrift as 0 and correctly detect expired token', () => {
59+
const result = isTokenExpired({
60+
expiresAt: Date.now() - 1000, // expired 1 second ago
61+
clockDrift: NaN,
62+
tolerance: 0,
63+
});
64+
65+
expect(result).toBe(true);
66+
});
67+
68+
it('should treat NaN clockDrift as 0 and correctly detect valid token', () => {
69+
const result = isTokenExpired({
70+
expiresAt: Date.now() + 10000, // expires in 10 seconds
71+
clockDrift: NaN,
72+
tolerance: 0,
73+
});
74+
75+
expect(result).toBe(false);
76+
});
77+
78+
it('should handle positive clockDrift correctly', () => {
79+
// With positive clockDrift, effective time is ahead
80+
const result = isTokenExpired({
81+
expiresAt: Date.now() + 1000, // expires in 1 second
82+
clockDrift: 2000, // but we're 2 seconds ahead
83+
tolerance: 0,
84+
});
85+
86+
expect(result).toBe(true);
87+
});
88+
89+
it('should handle negative clockDrift correctly', () => {
90+
// With negative clockDrift, effective time is behind
91+
const result = isTokenExpired({
92+
expiresAt: Date.now() - 1000, // expired 1 second ago
93+
clockDrift: -2000, // but we're 2 seconds behind
94+
tolerance: 0,
95+
});
96+
97+
expect(result).toBe(false);
98+
});
99+
100+
it('should handle zero clockDrift correctly', () => {
101+
const result = isTokenExpired({
102+
expiresAt: Date.now() + 1000,
103+
clockDrift: 0,
104+
tolerance: 0,
105+
});
106+
107+
expect(result).toBe(false);
108+
});
109+
});
56110
});

packages/core/src/utils/isTokenExpired.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ export function isTokenExpired({
1111
tolerance?: number;
1212
}): boolean {
1313
const currentTime = Date.now();
14+
// Treat NaN clockDrift as 0 for safety (NaN comparisons always return false)
15+
const safeClockDrift = Number.isNaN(clockDrift) ? 0 : clockDrift;
1416

15-
return currentTime + clockDrift + tolerance > expiresAt;
17+
return currentTime + safeClockDrift + tolerance > expiresAt;
1618
}

0 commit comments

Comments
 (0)