From e9bcdf4d8132b99356035b35465b69c3011e9549 Mon Sep 17 00:00:00 2001 From: Bruno Silva Date: Tue, 28 Jul 2026 11:41:13 +1000 Subject: [PATCH 1/3] fix(auth): retry the OAuth token exchange on transient failures exchangeCodeForTokens (shared by loginWithPopup and the redirect handleLoginCallback) retries on network error / HTTP >= 500, not on 4xx. 2 retries, 1s->2s backoff. No public API change. Refs GAM-509. Co-Authored-By: Claude Opus 4.8 --- packages/auth/src/login/standalone.test.ts | 104 +++++++++++++++++++++ packages/auth/src/login/standalone.ts | 76 +++++++++++---- 2 files changed, 162 insertions(+), 18 deletions(-) create mode 100644 packages/auth/src/login/standalone.test.ts diff --git a/packages/auth/src/login/standalone.test.ts b/packages/auth/src/login/standalone.test.ts new file mode 100644 index 0000000000..db8c5953e4 --- /dev/null +++ b/packages/auth/src/login/standalone.test.ts @@ -0,0 +1,104 @@ +/** + * Tests for the token-exchange retry in exchangeCodeForTokens, exercised through + * handleLoginCallback (the shared path used by both popup and redirect flows). + */ + +const trackMock = jest.fn(); + +jest.mock('@imtbl/metrics', () => ({ + track: (...args: unknown[]) => trackMock(...args), + getDetail: jest.fn(), + Detail: { RUNTIME_ID: 'runtime_id' }, +})); + +jest.mock('../utils/jwt', () => ({ + decodeJwtPayload: jest.fn(() => ({})), +})); + +// eslint-disable-next-line import/first +import { handleLoginCallback, type LoginConfig } from './standalone'; + +const CONFIG: LoginConfig = { + clientId: 'client-123', + redirectUri: 'https://app.example.com/callback', + authenticationDomain: 'https://auth.example.com', +}; + +const PKCE = { + state: 'state-xyz', + verifier: 'verifier-abc', + redirectUri: 'https://app.example.com/callback', +}; + +const TOKENS = { access_token: 'access-tok', refresh_token: 'refresh-tok' }; + +function mockResponse(status: number, body: unknown): Response { + return { + ok: status >= 200 && status < 300, + status, + json: async () => body, + text: async () => JSON.stringify(body), + } as unknown as Response; +} + +describe('exchangeCodeForTokens retry (via handleLoginCallback)', () => { + let fetchMock: jest.Mock; + + beforeEach(() => { + jest.useFakeTimers(); + window.sessionStorage.setItem('imtbl_pkce_data', JSON.stringify(PKCE)); + window.history.replaceState(null, '', '/?code=auth-code&state=state-xyz'); + fetchMock = jest.fn(); + global.fetch = fetchMock as unknown as typeof fetch; + }); + + afterEach(() => { + jest.runOnlyPendingTimers(); + jest.useRealTimers(); + window.sessionStorage.clear(); + }); + + it('retries a 5xx and resolves on the next success', async () => { + fetchMock + .mockResolvedValueOnce(mockResponse(503, { error: 'unavailable' })) + .mockResolvedValueOnce(mockResponse(200, TOKENS)); + + const promise = handleLoginCallback(CONFIG); + await jest.advanceTimersByTimeAsync(1000); + const result = await promise; + + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(result?.accessToken).toBe('access-tok'); + }); + + it('retries a network error and resolves on the next success', async () => { + fetchMock + .mockRejectedValueOnce(new Error('network down')) + .mockResolvedValueOnce(mockResponse(200, TOKENS)); + + const promise = handleLoginCallback(CONFIG); + await jest.advanceTimersByTimeAsync(1000); + const result = await promise; + + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(result?.accessToken).toBe('access-tok'); + }); + + it('does not retry a 4xx', async () => { + fetchMock.mockResolvedValueOnce(mockResponse(400, { error: 'invalid_grant' })); + + await expect(handleLoginCallback(CONFIG)).rejects.toThrow('invalid_grant'); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('gives up after exhausting retries', async () => { + fetchMock.mockResolvedValue(mockResponse(500, { error: 'boom' })); + + const promise = handleLoginCallback(CONFIG); + promise.catch(() => {}); // avoid an unhandled rejection while advancing timers + await jest.advanceTimersByTimeAsync(3000); // 1s + 2s backoffs + + await expect(promise).rejects.toThrow('boom'); + expect(fetchMock).toHaveBeenCalledTimes(3); // initial + 2 retries + }); +}); diff --git a/packages/auth/src/login/standalone.ts b/packages/auth/src/login/standalone.ts index 8a23a3bb4b..4da488cf1e 100644 --- a/packages/auth/src/login/standalone.ts +++ b/packages/auth/src/login/standalone.ts @@ -430,6 +430,36 @@ async function buildAuthorizationUrl( // Token Exchange // ============================================================================ +// A transient failure on the token exchange (network error, CloudFront 5xx, rate +// limit) would otherwise drop an otherwise-valid login. The auth code is not +// consumed on a 5xx, so re-POSTing it is safe; a 4xx is permanent (bad/expired code). +const TOKEN_EXCHANGE_MAX_RETRIES = 2; +const TOKEN_EXCHANGE_RETRY_DELAY_MS = 1000; + +function delay(ms: number): Promise { + return new Promise((resolve) => { + setTimeout(resolve, ms); + }); +} + +async function parseTokenErrorMessage(response: Response): Promise { + const errorText = await response.text(); + let errorMessage = `Token exchange failed with status ${response.status}`; + try { + const errorData = JSON.parse(errorText); + if (errorData.error_description) { + errorMessage = errorData.error_description; + } else if (errorData.error) { + errorMessage = errorData.error; + } + } catch { + if (errorText) { + errorMessage = errorText; + } + } + return errorMessage; +} + async function exchangeCodeForTokens( config: LoginConfig, code: string, @@ -438,8 +468,7 @@ async function exchangeCodeForTokens( ): Promise { const authDomain = getAuthDomain(config); const tokenUrl = `${authDomain}${TOKEN_ENDPOINT}`; - - const response = await fetch(tokenUrl, { + const requestInit: RequestInit = { method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded', @@ -451,28 +480,39 @@ async function exchangeCodeForTokens( code, redirect_uri: redirectUri, }), - }); + }; + + const attempt = async (retriesLeft: number): Promise => { + const backoffMs = TOKEN_EXCHANGE_RETRY_DELAY_MS * (TOKEN_EXCHANGE_MAX_RETRIES - retriesLeft + 1); - if (!response.ok) { - const errorText = await response.text(); - let errorMessage = `Token exchange failed with status ${response.status}`; + let response: Response; try { - const errorData = JSON.parse(errorText); - if (errorData.error_description) { - errorMessage = errorData.error_description; - } else if (errorData.error) { - errorMessage = errorData.error; - } - } catch { - if (errorText) { - errorMessage = errorText; + response = await fetch(tokenUrl, requestInit); + } catch (networkError) { + // Network/transport failure — transient, retry. + if (retriesLeft > 0) { + await delay(backoffMs); + return attempt(retriesLeft - 1); } + throw networkError instanceof Error ? networkError : new Error('Token exchange network error'); + } + + if (response.ok) { + const tokenData = await response.json(); + return mapTokenResponseToResult(tokenData); + } + + const errorMessage = await parseTokenErrorMessage(response); + + // Only 5xx is transient; a 4xx (bad/expired/consumed code) is permanent. + if (response.status >= 500 && retriesLeft > 0) { + await delay(backoffMs); + return attempt(retriesLeft - 1); } throw new Error(errorMessage); - } + }; - const tokenData = await response.json(); - return mapTokenResponseToResult(tokenData); + return attempt(TOKEN_EXCHANGE_MAX_RETRIES); } // ============================================================================ From 36fbe88c6b17e421b2714bed095787c9a357f160 Mon Sep 17 00:00:00 2001 From: Bruno Silva Date: Tue, 28 Jul 2026 12:06:57 +1000 Subject: [PATCH 2/3] fix(auth): bound token-exchange attempts with a timeout and jitter the backoff Per-attempt AbortController timeout so a hung request becomes a retryable failure instead of an indefinite wait; full jitter on the backoff so mass 5xx retries don't synchronise into a thundering herd. Body reuse across attempts documented as deliberate/safe. Refs GAM-509. Co-Authored-By: Claude Opus 4.8 --- packages/auth/src/login/standalone.test.ts | 21 ++++++++++++++ packages/auth/src/login/standalone.ts | 32 +++++++++++++++++----- 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/packages/auth/src/login/standalone.test.ts b/packages/auth/src/login/standalone.test.ts index db8c5953e4..2131901759 100644 --- a/packages/auth/src/login/standalone.test.ts +++ b/packages/auth/src/login/standalone.test.ts @@ -84,6 +84,27 @@ describe('exchangeCodeForTokens retry (via handleLoginCallback)', () => { expect(result?.accessToken).toBe('access-tok'); }); + it('aborts a hung attempt at the timeout and retries', async () => { + // First attempt never responds; the per-attempt AbortController should fire at + // TOKEN_EXCHANGE_TIMEOUT_MS and reject it, which is treated as transient. + fetchMock.mockImplementationOnce( + (_url: string, init: RequestInit) => new Promise((_resolve, reject) => { + init.signal?.addEventListener('abort', () => { + reject(Object.assign(new Error('The operation was aborted'), { name: 'AbortError' })); + }); + }), + ); + fetchMock.mockResolvedValueOnce(mockResponse(200, TOKENS)); + + const promise = handleLoginCallback(CONFIG); + await jest.advanceTimersByTimeAsync(10000); // fire the attempt timeout -> abort + await jest.advanceTimersByTimeAsync(1000); // backoff before the retry + const result = await promise; + + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(result?.accessToken).toBe('access-tok'); + }); + it('does not retry a 4xx', async () => { fetchMock.mockResolvedValueOnce(mockResponse(400, { error: 'invalid_grant' })); diff --git a/packages/auth/src/login/standalone.ts b/packages/auth/src/login/standalone.ts index 4da488cf1e..23db4c2984 100644 --- a/packages/auth/src/login/standalone.ts +++ b/packages/auth/src/login/standalone.ts @@ -430,11 +430,14 @@ async function buildAuthorizationUrl( // Token Exchange // ============================================================================ -// A transient failure on the token exchange (network error, CloudFront 5xx, rate -// limit) would otherwise drop an otherwise-valid login. The auth code is not +// A transient failure on the token exchange (network error, timeout, CloudFront 5xx, +// rate limit) would otherwise drop an otherwise-valid login. The auth code is not // consumed on a 5xx, so re-POSTing it is safe; a 4xx is permanent (bad/expired code). const TOKEN_EXCHANGE_MAX_RETRIES = 2; const TOKEN_EXCHANGE_RETRY_DELAY_MS = 1000; +// Bound each attempt so a stalled request becomes a retryable failure rather than an +// indefinite hang (this path has no other timeout around it). +const TOKEN_EXCHANGE_TIMEOUT_MS = 10000; function delay(ms: number): Promise { return new Promise((resolve) => { @@ -442,6 +445,13 @@ function delay(ms: number): Promise { }); } +// Full jitter on the backoff so that when auth returns 5xx to many clients at once, +// their retries spread out instead of synchronising into a thundering herd. +function backoffWithJitter(attemptNumber: number): number { + const base = TOKEN_EXCHANGE_RETRY_DELAY_MS * attemptNumber; + return base * (0.5 + Math.random() * 0.5); +} + async function parseTokenErrorMessage(response: Response): Promise { const errorText = await response.text(); let errorMessage = `Token exchange failed with status ${response.status}`; @@ -468,6 +478,10 @@ async function exchangeCodeForTokens( ): Promise { const authDomain = getAuthDomain(config); const tokenUrl = `${authDomain}${TOKEN_ENDPOINT}`; + // Deliberately built once and reused across retry attempts: the body is a + // URLSearchParams, which fetch re-serialises on every call (it is not a one-shot + // consumable stream), so re-sending it is safe. Only the per-attempt AbortSignal + // differs, and that is merged in at the fetch call below. const requestInit: RequestInit = { method: 'POST', headers: { @@ -483,18 +497,22 @@ async function exchangeCodeForTokens( }; const attempt = async (retriesLeft: number): Promise => { - const backoffMs = TOKEN_EXCHANGE_RETRY_DELAY_MS * (TOKEN_EXCHANGE_MAX_RETRIES - retriesLeft + 1); + const attemptNumber = TOKEN_EXCHANGE_MAX_RETRIES - retriesLeft + 1; + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), TOKEN_EXCHANGE_TIMEOUT_MS); let response: Response; try { - response = await fetch(tokenUrl, requestInit); + response = await fetch(tokenUrl, { ...requestInit, signal: controller.signal }); } catch (networkError) { - // Network/transport failure — transient, retry. + // Network error or a timed-out (aborted) attempt — both transient, retry. if (retriesLeft > 0) { - await delay(backoffMs); + await delay(backoffWithJitter(attemptNumber)); return attempt(retriesLeft - 1); } throw networkError instanceof Error ? networkError : new Error('Token exchange network error'); + } finally { + clearTimeout(timeoutId); } if (response.ok) { @@ -506,7 +524,7 @@ async function exchangeCodeForTokens( // Only 5xx is transient; a 4xx (bad/expired/consumed code) is permanent. if (response.status >= 500 && retriesLeft > 0) { - await delay(backoffMs); + await delay(backoffWithJitter(attemptNumber)); return attempt(retriesLeft - 1); } throw new Error(errorMessage); From fc3c341c66ad0691d69a91d8825510e1628dc843 Mon Sep 17 00:00:00 2001 From: Bruno Silva Date: Tue, 28 Jul 2026 13:53:17 +1000 Subject: [PATCH 3/3] fix(auth): keep the abort armed across the body read and track retries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fetch resolves on headers, so clearing the timeout before reading the body left response.json()/text() unbounded — a server that stalls the body hung forever. Body reads now happen inside the abort window, and each attempt's outcome is classified as data so a permanent 4xx isn't retried. Adds standaloneTokenExchangeFailed / standaloneTokenExchangeRecovered telemetry so retry frequency and recovery rate are observable. Refs GAM-509. Co-Authored-By: Claude Opus 4.8 --- packages/auth/src/login/standalone.test.ts | 53 +++++++++++++++++-- packages/auth/src/login/standalone.ts | 59 ++++++++++++++++------ 2 files changed, 93 insertions(+), 19 deletions(-) diff --git a/packages/auth/src/login/standalone.test.ts b/packages/auth/src/login/standalone.test.ts index 2131901759..f25b11eb7d 100644 --- a/packages/auth/src/login/standalone.test.ts +++ b/packages/auth/src/login/standalone.test.ts @@ -64,7 +64,52 @@ describe('exchangeCodeForTokens retry (via handleLoginCallback)', () => { .mockResolvedValueOnce(mockResponse(200, TOKENS)); const promise = handleLoginCallback(CONFIG); - await jest.advanceTimersByTimeAsync(1000); + await jest.advanceTimersByTimeAsync(2000); + const result = await promise; + + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(result?.accessToken).toBe('access-tok'); + }); + + it('emits telemetry for the retry and the recovery', async () => { + fetchMock + .mockResolvedValueOnce(mockResponse(503, { error: 'unavailable' })) + .mockResolvedValueOnce(mockResponse(200, TOKENS)); + + const promise = handleLoginCallback(CONFIG); + await jest.advanceTimersByTimeAsync(2000); + await promise; + + expect(trackMock).toHaveBeenCalledWith( + 'passport', + 'standaloneTokenExchangeFailed', + expect.objectContaining({ attempt: 1, reason: '503', willRetry: true }), + ); + expect(trackMock).toHaveBeenCalledWith( + 'passport', + 'standaloneTokenExchangeRecovered', + expect.objectContaining({ attempt: 2 }), + ); + }); + + it('aborts a stalled response body and retries', async () => { + // Headers arrive but the body never completes — the abort must still be armed, or the + // read hangs forever (fetch resolves on headers, not on the full body). + fetchMock.mockImplementationOnce((_url: string, init: RequestInit) => Promise.resolve({ + ok: true, + status: 200, + json: () => new Promise((_resolve, reject) => { + init.signal?.addEventListener('abort', () => { + reject(Object.assign(new Error('The operation was aborted'), { name: 'AbortError' })); + }); + }), + text: async () => '', + } as unknown as Response)); + fetchMock.mockResolvedValueOnce(mockResponse(200, TOKENS)); + + const promise = handleLoginCallback(CONFIG); + await jest.advanceTimersByTimeAsync(10000); // body-read timeout fires + await jest.advanceTimersByTimeAsync(2000); // backoff before the retry const result = await promise; expect(fetchMock).toHaveBeenCalledTimes(2); @@ -77,7 +122,7 @@ describe('exchangeCodeForTokens retry (via handleLoginCallback)', () => { .mockResolvedValueOnce(mockResponse(200, TOKENS)); const promise = handleLoginCallback(CONFIG); - await jest.advanceTimersByTimeAsync(1000); + await jest.advanceTimersByTimeAsync(2000); const result = await promise; expect(fetchMock).toHaveBeenCalledTimes(2); @@ -98,7 +143,7 @@ describe('exchangeCodeForTokens retry (via handleLoginCallback)', () => { const promise = handleLoginCallback(CONFIG); await jest.advanceTimersByTimeAsync(10000); // fire the attempt timeout -> abort - await jest.advanceTimersByTimeAsync(1000); // backoff before the retry + await jest.advanceTimersByTimeAsync(2000); // backoff before the retry const result = await promise; expect(fetchMock).toHaveBeenCalledTimes(2); @@ -117,7 +162,7 @@ describe('exchangeCodeForTokens retry (via handleLoginCallback)', () => { const promise = handleLoginCallback(CONFIG); promise.catch(() => {}); // avoid an unhandled rejection while advancing timers - await jest.advanceTimersByTimeAsync(3000); // 1s + 2s backoffs + await jest.advanceTimersByTimeAsync(6000); // both backoffs, generous for jitter await expect(promise).rejects.toThrow('boom'); expect(fetchMock).toHaveBeenCalledTimes(3); // initial + 2 retries diff --git a/packages/auth/src/login/standalone.ts b/packages/auth/src/login/standalone.ts index 23db4c2984..01032bb428 100644 --- a/packages/auth/src/login/standalone.ts +++ b/packages/auth/src/login/standalone.ts @@ -452,6 +452,13 @@ function backoffWithJitter(attemptNumber: number): number { return base * (0.5 + Math.random() * 0.5); } +// Outcome of a single token-exchange attempt: transient failures are retried, permanent +// ones (4xx) are thrown straight through. +type TokenAttemptOutcome = + | { kind: 'ok'; tokens: TokenResponse } + | { kind: 'transient'; error: Error; reason: string } + | { kind: 'permanent'; error: Error }; + async function parseTokenErrorMessage(response: Response): Promise { const errorText = await response.text(); let errorMessage = `Token exchange failed with status ${response.status}`; @@ -500,34 +507,56 @@ async function exchangeCodeForTokens( const attemptNumber = TOKEN_EXCHANGE_MAX_RETRIES - retriesLeft + 1; const controller = new AbortController(); const timeoutId = setTimeout(() => controller.abort(), TOKEN_EXCHANGE_TIMEOUT_MS); + const startTime = Date.now(); + + // Classified as data rather than thrown, so a permanent failure isn't confused with a + // transient one by the catch below. + let outcome: TokenAttemptOutcome; - let response: Response; try { - response = await fetch(tokenUrl, { ...requestInit, signal: controller.signal }); - } catch (networkError) { - // Network error or a timed-out (aborted) attempt — both transient, retry. - if (retriesLeft > 0) { - await delay(backoffWithJitter(attemptNumber)); - return attempt(retriesLeft - 1); + const response = await fetch(tokenUrl, { ...requestInit, signal: controller.signal }); + + // Body reads stay inside the try so the abort above still covers them: fetch resolves + // once headers arrive, so a server that stalls the body would otherwise hang unbounded. + if (response.ok) { + outcome = { kind: 'ok', tokens: mapTokenResponseToResult(await response.json()) }; + } else { + const error = new Error(await parseTokenErrorMessage(response)); + // Only 5xx is transient; a 4xx (bad/expired/consumed code) is permanent. + outcome = response.status >= 500 + ? { kind: 'transient', error, reason: String(response.status) } + : { kind: 'permanent', error }; } - throw networkError instanceof Error ? networkError : new Error('Token exchange network error'); + } catch (caught) { + // Network failure, or this attempt timing out mid-request or mid-body — all transient. + const error = caught instanceof Error ? caught : new Error('Token exchange network error'); + outcome = { kind: 'transient', error, reason: error.name === 'AbortError' ? 'timeout' : 'network' }; } finally { clearTimeout(timeoutId); } - if (response.ok) { - const tokenData = await response.json(); - return mapTokenResponseToResult(tokenData); + if (outcome.kind === 'ok') { + if (attemptNumber > 1) { + track('passport', 'standaloneTokenExchangeRecovered', { attempt: attemptNumber }); + } + return outcome.tokens; + } + if (outcome.kind === 'permanent') { + throw outcome.error; } - const errorMessage = await parseTokenErrorMessage(response); + track('passport', 'standaloneTokenExchangeFailed', { + attempt: attemptNumber, + reason: outcome.reason, + willRetry: retriesLeft > 0, + timeToFailureMs: Date.now() - startTime, + }); - // Only 5xx is transient; a 4xx (bad/expired/consumed code) is permanent. - if (response.status >= 500 && retriesLeft > 0) { + if (retriesLeft > 0) { await delay(backoffWithJitter(attemptNumber)); return attempt(retriesLeft - 1); } - throw new Error(errorMessage); + throw outcome.error; }; return attempt(TOKEN_EXCHANGE_MAX_RETRIES);