From 45a3ba8829ac6be768f09e2e2563a2121a1fd5b1 Mon Sep 17 00:00:00 2001 From: elky Date: Fri, 4 Sep 2026 13:14:16 +0800 Subject: [PATCH] fix(frontend): avoid blocking public routes on invalid session --- .../utils/__tests__/crossTabRefresh.spec.ts | 51 ++++++++++++++++--- frontend/src/utils/crossTabRefresh.ts | 22 +++++++- 2 files changed, 64 insertions(+), 9 deletions(-) diff --git a/frontend/src/utils/__tests__/crossTabRefresh.spec.ts b/frontend/src/utils/__tests__/crossTabRefresh.spec.ts index a36935dae..197f8968d 100644 --- a/frontend/src/utils/__tests__/crossTabRefresh.spec.ts +++ b/frontend/src/utils/__tests__/crossTabRefresh.spec.ts @@ -53,6 +53,10 @@ function createDeferred() { return { promise, resolve, reject } } +function createHttpError(status: number, message: string): Error & { response: { status: number } } { + return Object.assign(new Error(message), { response: { status } }) +} + function createRacyStorage(): Storage { let hiddenInitialLockReads = 2 let ownLockWrite: string | null = null @@ -169,16 +173,49 @@ describe('CrossTabRefreshCoordinator', () => { second.destroy() }) - it('surfaces a genuine single-tab refresh failure after the coordination window', async () => { - const refreshError = new Error('authoritative refresh rejection') + it('surfaces a genuine single-tab refresh failure without waiting for the request timeout', async () => { + const refreshError = createHttpError(401, 'authoritative refresh rejection') const executor = vi.fn(() => Promise.reject(refreshError)) const coordinator = new CrossTabRefreshCoordinator({ storage: localStorage, channelFactory: createChannel, - waitTimeoutMs: 10, + waitTimeoutMs: 500, }) - await expect(coordinator.run(executor)).rejects.toBe(refreshError) + const nextTimerTick = Symbol('next-timer-tick') + let timerId: ReturnType | undefined + const outcome = await Promise.race([ + coordinator.run(executor).catch((error: unknown) => error), + new Promise((resolve) => { + timerId = setTimeout(() => resolve(nextTimerTick), 0) + }), + ]) + if (timerId !== undefined) clearTimeout(timerId) + + expect(outcome).toBe(refreshError) + expect(executor).toHaveBeenCalledTimes(1) + + coordinator.destroy() + }) + + it('keeps the coordination window for a refresh-token rotation conflict', async () => { + const refreshError = createHttpError(409, 'refresh token was rotated concurrently') + const executor = vi.fn(() => Promise.reject(refreshError)) + const coordinator = new CrossTabRefreshCoordinator({ + storage: localStorage, + channelFactory: createChannel, + waitTimeoutMs: 20, + }) + + let settled = false + const outcome = coordinator.run(executor).catch((error: unknown) => error) + void outcome.then(() => { + settled = true + }) + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(settled).toBe(false) + await expect(outcome).resolves.toBe(refreshError) expect(executor).toHaveBeenCalledTimes(1) coordinator.destroy() @@ -208,7 +245,7 @@ describe('CrossTabRefreshCoordinator', () => { expect(firstExecutor).toHaveBeenCalledTimes(1) expect(secondExecutor).toHaveBeenCalledTimes(1) - failedAttempt.reject(new Error('lost refresh-token rotation')) + failedAttempt.reject(createHttpError(409, 'lost refresh-token rotation')) await Promise.resolve() successfulAttempt.resolve('access-from-winner') @@ -244,7 +281,7 @@ describe('CrossTabRefreshCoordinator', () => { successfulAttempt.resolve('access-from-first-winner') await expect(firstRun).resolves.toBe('access-from-first-winner') - failedAttempt.reject(new Error('stale rotated cookie')) + failedAttempt.reject(createHttpError(409, 'stale rotated cookie')) await expect(secondRun).resolves.toBe('verified-after-failure-hint') expect(secondExecutor).toHaveBeenCalledTimes(2) expect(localStorage.getItem('aether_auth_refresh_result')).not.toContain('access-from') @@ -274,7 +311,7 @@ describe('CrossTabRefreshCoordinator', () => { const firstRun = first.run(firstExecutor) const secondRun = second.run(secondExecutor) - failedAttempt.reject(new Error('stale rotated cookie')) + failedAttempt.reject(createHttpError(409, 'stale rotated cookie')) await new Promise((resolve) => setTimeout(resolve, 300)) successfulAttempt.resolve('access-from-delayed-winner') diff --git a/frontend/src/utils/crossTabRefresh.ts b/frontend/src/utils/crossTabRefresh.ts index 42969943a..2093fc722 100644 --- a/frontend/src/utils/crossTabRefresh.ts +++ b/frontend/src/utils/crossTabRefresh.ts @@ -81,6 +81,18 @@ function defaultChannelFactory(name: string): BroadcastChannelLike | null { return new BroadcastChannel(name) } +function isDefinitiveRefreshRejection(error: unknown): boolean { + if (!error || typeof error !== 'object') { + return false + } + const response = (error as { response?: unknown }).response + if (!response || typeof response !== 'object') { + return false + } + const status = (response as { status?: unknown }).status + return status === 401 || status === 403 +} + export class CrossTabRefreshCoordinator { private readonly storage: Storage | null private readonly waitTimeoutMs: number @@ -184,6 +196,13 @@ export class CrossTabRefreshCoordinator { }) } + // The refresh endpoint reserves 409 for a concurrent token rotation. + // A direct 401/403 is authoritative, so waiting for the HTTP timeout + // cannot recover the session and would block initial navigation. + if (isDefinitiveRefreshRejection(error)) { + throw error + } + // A failure is a hint, never a cross-tab verdict. Give a concurrent // winner a bounded opportunity to publish success before returning the // local error. If one does, retry using this tab's shared HttpOnly cookie. @@ -296,8 +315,7 @@ export class CrossTabRefreshCoordinator { } this.successObservers.add(onSuccess) // A competing request can legitimately run until the HTTP client timeout. - // The coordinator timeout includes that budget, so do not surface a local - // 401 while an undetectable best-effort lock contender may still succeed. + // The coordinator timeout includes that budget for retryable failures. const timeoutId = setTimeout(() => finish(hasSuccess()), this.waitTimeoutMs) // A result may have arrived between the initial check and observer // registration, so check once more synchronously.