From 45f97d4afe08bf8e75002ea41d2d2e736f6e71db Mon Sep 17 00:00:00 2001 From: Brion Date: Wed, 12 Aug 2026 22:19:41 +0530 Subject: [PATCH] Stop signOut() blocking on access token revocation requestAccessTokenRevocation() was awaited directly in signOut()'s critical path before the redirect, with a 10s timeout as its upper bound. A slow or unreachable revocation_endpoint (or CORS-unfriendly OP) stalled the entire sign-out flow, showing a blank page between the auth UI tearing down and the actual redirect firing. Fires the revocation request without awaiting it instead, so it can't delay sign out. Since it's no longer awaited, the access token is now snapshotted before firing the request so a concurrent clearSession can't race it out of storage first. --- .../browser/src/ThunderIDBrowserClient.ts | 19 ++-- .../__tests__/ThunderIDBrowserClient.test.ts | 96 +++++++++++++++++++ .../src/ThunderIDJavaScriptClient.ts | 10 +- .../ThunderIDJavaScriptClient.test.ts | 57 +++++++++++ 4 files changed, 172 insertions(+), 10 deletions(-) create mode 100644 packages/browser/src/__tests__/ThunderIDBrowserClient.test.ts diff --git a/packages/browser/src/ThunderIDBrowserClient.ts b/packages/browser/src/ThunderIDBrowserClient.ts index 0ddc7554..73f15ec7 100644 --- a/packages/browser/src/ThunderIDBrowserClient.ts +++ b/packages/browser/src/ThunderIDBrowserClient.ts @@ -393,18 +393,21 @@ class ThunderIDBrowserClient extends ThunderIDJavaScriptC } } - // Revoke the access token at the OP before clearing the session. Disabled by default; set - // tokenLifecycle.revokeToken.revokeOnSignOut to true to enable. Best-effort: revocation can - // fail (no revocation_endpoint advertised, network error, non-200 response, or a stalled request) - // without blocking sign out, since the local session must be cleared regardless. Uses the + // Revoke the access token at the OP. Disabled by default; set + // tokenLifecycle.revokeToken.revokeOnSignOut to true to enable. Fire-and-forget: not awaited, so a + // slow or unreachable revocation_endpoint can't delay the redirect below. Best-effort: revocation + // can fail (no revocation_endpoint advertised, network error, non-200 response, or a stalled + // request) without affecting sign out, since the local session is cleared regardless. Uses the // request-only core method so the session (and the ID token read above) isn't cleared twice or // ahead of the RP-Initiated Logout URL resolution. if (config?.tokenLifecycle?.revokeToken?.revokeOnSignOut === true) { - try { - await this.requestAccessTokenRevocation(sessionId); - } catch (error) { + // Snapshot the access token now, before firing the revocation request without awaiting it — + // clearSession(Async) below can otherwise remove it from storage before this request reads it. + const accessTokenToRevoke = (await sm.getSessionData(sessionId))?.access_token; + + this.requestAccessTokenRevocation(sessionId, accessTokenToRevoke).catch((error) => { logger.debug('Could not revoke the access token before signing out.', error); - } + }); } if (signOutUrl) { diff --git a/packages/browser/src/__tests__/ThunderIDBrowserClient.test.ts b/packages/browser/src/__tests__/ThunderIDBrowserClient.test.ts new file mode 100644 index 00000000..1fac582c --- /dev/null +++ b/packages/browser/src/__tests__/ThunderIDBrowserClient.test.ts @@ -0,0 +1,96 @@ +// Copyright 2026 The ThunderID Authors +// SPDX-License-Identifier: Apache-2.0 + +import {describe, expect, it, vi, beforeEach, afterEach} from 'vitest'; +import ThunderIDBrowserClient from '../ThunderIDBrowserClient'; + +vi.mock('../utils/navigate', () => ({default: vi.fn()})); + +const BASE_CONFIG = { + baseUrl: 'https://example.com', + clientId: 'test-client', + rpInitiatedLogout: false, + signInUrl: 'https://example.com/sign-in', + storage: 'browserMemory', +} as any; + +const REVOCATION_ENDPOINT = 'https://example.com/oauth2/revoke'; + +async function initClient(overrides: Record = {}): Promise { + const client = new ThunderIDBrowserClient(); + await client.initialize({...BASE_CONFIG, ...overrides}); + const sm = (client as any).getStorageManager(); + await sm.setOIDCProviderMetaData({revocation_endpoint: REVOCATION_ENDPOINT}); + await sm.setTemporaryDataParameter('op_config_initiated', true); + await sm.setSessionData({access_token: 'stored-access-token'}); + + return client; +} + +function mockFetchOnce(ok = true, status = 200): {resolve: () => void} { + let resolveFetch: () => void = () => {}; + const pending = new Promise((resolve) => { + resolveFetch = resolve; + }); + + vi.stubGlobal( + 'fetch', + vi.fn().mockImplementation( + () => + new Promise((resolve) => { + pending.then(() => + resolve({ + json: () => Promise.resolve({}), + ok, + status, + statusText: ok ? 'OK' : 'Bad Request', + }), + ); + }), + ), + ); + + return {resolve: resolveFetch}; +} + +describe('ThunderIDBrowserClient signOut()', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it('completes sign out without waiting for a slow revocation_endpoint to respond', async () => { + const client = await initClient({tokenLifecycle: {revokeToken: {revokeOnSignOut: true}}}); + // Never resolved during this test — if signOut() awaited the revocation response, it would hang. + mockFetchOnce(); + + const afterSignOutUrl = await client.signOut(); + + expect(afterSignOutUrl).toBe(window.location.origin); + await vi.waitFor(() => expect(fetch).toHaveBeenCalledTimes(1)); + }); + + it('sends the access token captured before session clearing, not a post-clear read', async () => { + const client = await initClient({tokenLifecycle: {revokeToken: {revokeOnSignOut: true}}}); + const {resolve} = mockFetchOnce(); + + await client.signOut(); + resolve(); + await vi.waitFor(() => expect((fetch as any).mock.calls.length).toBe(1)); + + const [, requestInit] = (fetch as any).mock.calls[0]; + expect(requestInit.body).toContain('token=stored-access-token'); + }); + + it('does not call the revocation endpoint when revokeOnSignOut is not enabled', async () => { + const client = await initClient(); + mockFetchOnce(); + + await client.signOut(); + + expect(fetch).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/javascript/src/ThunderIDJavaScriptClient.ts b/packages/javascript/src/ThunderIDJavaScriptClient.ts index ee601857..2c43f009 100644 --- a/packages/javascript/src/ThunderIDJavaScriptClient.ts +++ b/packages/javascript/src/ThunderIDJavaScriptClient.ts @@ -886,8 +886,12 @@ class ThunderIDJavaScriptClient implements ThunderIDClient { * Sends the access token revocation request to the OP's `revocation_endpoint`. Unlike * {@link revokeAccessToken}, this does not clear the local session, so callers that need to read * session data (e.g. the ID token for RP-Initiated Logout) after revoking can do so. + * + * `accessToken` can be passed explicitly to snapshot it ahead of time (e.g. before firing this off + * without awaiting it, so a concurrent session clear can't race the token being read from storage). + * When omitted, it's read from storage at call time. */ - protected async requestAccessTokenRevocation(userId?: string): Promise { + protected async requestAccessTokenRevocation(userId?: string, accessToken?: string): Promise { const revokeTokenEndpoint: string | undefined = (await this.oidcProviderMetaDataProvider()).revocation_endpoint; const configData = await this.configProvider(); @@ -899,9 +903,11 @@ class ThunderIDJavaScriptClient implements ThunderIDClient { ); } + const resolvedAccessToken = accessToken ?? (await this.storageManager.getSessionData(userId)).access_token; + const body: string[] = [ `client_id=${configData.clientId}`, - `token=${(await this.storageManager.getSessionData(userId)).access_token}`, + `token=${resolvedAccessToken}`, 'token_type_hint=access_token', ]; diff --git a/packages/javascript/src/__tests__/ThunderIDJavaScriptClient.test.ts b/packages/javascript/src/__tests__/ThunderIDJavaScriptClient.test.ts index 426ce73f..edbd3129 100644 --- a/packages/javascript/src/__tests__/ThunderIDJavaScriptClient.test.ts +++ b/packages/javascript/src/__tests__/ThunderIDJavaScriptClient.test.ts @@ -488,4 +488,61 @@ describe('ThunderIDJavaScriptClient', () => { expect(url.searchParams.has('client_id')).toBe(false); }); }); + + describe('requestAccessTokenRevocation()', () => { + const REVOCATION_ENDPOINT = 'https://example.com/oauth2/revoke'; + + async function initForRevocation(): Promise { + const client = new ThunderIDJavaScriptClient(store, {} as any); + await client.initialize(BASE_CONFIG); + const sm = (client as any).storageManager; + await sm.setOIDCProviderMetaData({revocation_endpoint: REVOCATION_ENDPOINT}); + await sm.setTemporaryDataParameter('op_config_initiated', true); + await sm.setSessionData({access_token: 'stored-access-token'}); + return client; + } + + it('reads the access token from storage when no override is passed', async () => { + const client = await initForRevocation(); + mockFetchOnce({}, true, 200); + + await (client as any).requestAccessTokenRevocation(); + + const [, requestInit] = (fetch as any).mock.calls[0]; + expect(requestInit.body).toContain('token=stored-access-token'); + }); + + it('uses the passed accessToken instead of reading storage, so a concurrent session clear cannot race it', async () => { + const client = await initForRevocation(); + // Simulate the session having already been cleared by the time the request body is built. + await (client as any).storageManager.setSessionData({access_token: undefined}); + mockFetchOnce({}, true, 200); + + await (client as any).requestAccessTokenRevocation(undefined, 'snapshotted-access-token'); + + const [, requestInit] = (fetch as any).mock.calls[0]; + expect(requestInit.body).toContain('token=snapshotted-access-token'); + }); + + it('throws when the OP advertises no revocation_endpoint', async () => { + const client = new ThunderIDJavaScriptClient(store, {} as any); + await client.initialize(BASE_CONFIG); + const sm = (client as any).storageManager; + await sm.setOIDCProviderMetaData({token_endpoint: 'https://example.com/oauth2/token'}); + await sm.setTemporaryDataParameter('op_config_initiated', true); + + await expect((client as any).requestAccessTokenRevocation()).rejects.toMatchObject({ + code: 'JS-AUTH_CORE-RAT3-NF01', + }); + }); + + it('throws when the revocation request receives a non-200 response', async () => { + const client = await initForRevocation(); + mockFetchOnce({error: 'invalid_token'}, false, 400); + + await expect((client as any).requestAccessTokenRevocation()).rejects.toMatchObject({ + code: 'JS-AUTH_CORE-RAT3-HE03', + }); + }); + }); });