diff --git a/.changeset/protect-token-timeout-10s.md b/.changeset/protect-token-timeout-10s.md new file mode 100644 index 00000000000..6d514b66cc2 --- /dev/null +++ b/.changeset/protect-token-timeout-10s.md @@ -0,0 +1,10 @@ +--- +'@clerk/clerk-js': patch +'@clerk/shared': patch +--- + +Wait up to 10 seconds, instead of 5, for the Device Intelligence check to finish before sending a sign-in or sign-up without its result. This applies when an instance hasn't configured its own limit. Users on slow connections are less likely to have sign-ups blocked by protections that require a Device Intelligence result. + +Keep the wait bounded when the device clock changes so later attempts can recover after another tab's lock is released. + +The shared loader type reference now documents the same 10-second default. diff --git a/packages/clerk-js/src/core/__tests__/protectSession.test.ts b/packages/clerk-js/src/core/__tests__/protectSession.test.ts index bb8056d59e9..0ae8ee6dfeb 100644 --- a/packages/clerk-js/src/core/__tests__/protectSession.test.ts +++ b/packages/clerk-js/src/core/__tests__/protectSession.test.ts @@ -686,7 +686,7 @@ describe('ProtectSession deadlines', () => { const startedAt = Date.now(); await expect(created?.getRequestParams()).resolves.toMatchObject({ __clerk_protect_status: 'timeout' }); - // Misread, this would fall back to the 5s default and be two orders of magnitude out. + // Misread, this would fall back to the 10s default and be two orders of magnitude out. expect(Date.now() - startedAt).toBeLessThan(1_000); }); @@ -733,11 +733,204 @@ describe('ProtectSession deadlines', () => { // `tokenTimeoutMs` is server config with no upper bound of its own, so the ceiling is ours. expect(clampTimeout(120_000)).toBe(10_000); expect(clampTimeout(1_500)).toBe(1_500); - expect(clampTimeout(undefined)).toBe(5_000); - expect(clampTimeout(0)).toBe(5_000); - expect(clampTimeout(-1)).toBe(5_000); - expect(clampTimeout('soon')).toBe(5_000); - expect(clampTimeout(Number.POSITIVE_INFINITY)).toBe(5_000); + expect(clampTimeout(undefined)).toBe(10_000); + expect(clampTimeout(0)).toBe(10_000); + expect(clampTimeout(-1)).toBe(10_000); + expect(clampTimeout('soon')).toBe(10_000); + expect(clampTimeout(Number.POSITIVE_INFINITY)).toBe(10_000); + }); +}); + +describe('ProtectSession default deadline', () => { + const defaultLoader = () => loader({ token_timeout_ms: undefined }); + + const track = (params: Promise | undefined) => { + const state = { settled: false }; + void params?.then(() => { + state.settled = true; + }); + return state; + }; + + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + delete (navigator as unknown as Record).locks; + }); + + it('accepts a token that arrives after 7s', async () => { + const { session: created, elements } = session([defaultLoader()]); + created?.start(); + const params = created?.getRequestParams(); + + await vi.advanceTimersByTimeAsync(7_000); + serveInline(elements[0], { cid: created?.placeholders().cid }); + + await expect(params).resolves.toMatchObject({ __clerk_protect_status: 'ok' }); + }); + + it('gives up at 10s when the loader never loads', async () => { + const { session: created } = session([defaultLoader()]); + created?.start(); + const params = created?.getRequestParams(); + const state = track(params); + + await vi.advanceTimersByTimeAsync(9_999); + expect(state.settled).toBe(false); + await vi.advanceTimersByTimeAsync(1); + await expect(params).resolves.toMatchObject({ __clerk_protect_status: 'timeout' }); + }); + + it('holds a sign-in made mid-acquisition only for what is left of it', async () => { + const { session: created } = session([defaultLoader()]); + created?.start(); + await vi.advanceTimersByTimeAsync(6_000); + + const params = created?.getRequestParams(); + const state = track(params); + + await vi.advanceTimersByTimeAsync(3_999); + expect(state.settled).toBe(false); + await vi.advanceTimersByTimeAsync(1); + await expect(params).resolves.toMatchObject({ __clerk_protect_status: 'timeout' }); + }); + + it('never holds one sign-in past the deadline when the lock is granted late', async () => { + Object.defineProperty(navigator, 'locks', { + value: { + request: vi.fn((_key: string, _options: unknown, callback: () => Promise) => + new Promise(resolve => setTimeout(resolve, 4_900)).then(() => callback()), + ), + }, + configurable: true, + }); + + const { session: created } = session([defaultLoader()]); + created?.start(); + const params = created?.getRequestParams(); + const state = track(params); + + await vi.advanceTimersByTimeAsync(9_999); + expect(state.settled).toBe(false); + await vi.advanceTimersByTimeAsync(1); + await expect(params).resolves.toMatchObject({ __clerk_protect_status: 'timeout' }); + }); + + it('keeps waiting for a leader tab after SafeLock gives up, until its own deadline', async () => { + Object.defineProperty(navigator, 'locks', { + value: { + request: vi.fn( + (_key: string, options: { signal: AbortSignal }) => + new Promise((_resolve, reject) => { + options.signal.addEventListener('abort', () => reject(new DOMException('aborted', 'AbortError'))); + }), + ), + }, + configurable: true, + }); + + const { session: created, elements } = session([defaultLoader()]); + created?.start(); + const params = created?.getRequestParams(); + + await vi.advanceTimersByTimeAsync(7_000); + localStorage.setItem('__clerk_protect_st', storedEntry({ token: 'v1.leader.mac', rid: 'c'.repeat(26) })); + await vi.advanceTimersByTimeAsync(100); + + await expect(params).resolves.toMatchObject({ + __clerk_protect_token: 'v1.leader.mac', + __clerk_protect_status: 'ok', + }); + expect(elements).toHaveLength(0); + }); + + it('answers at once when the browser refuses the cross-tab lock', async () => { + Object.defineProperty(navigator, 'locks', { + value: { request: vi.fn(() => Promise.reject(new DOMException('denied', 'SecurityError'))) }, + configurable: true, + }); + + const { session: created, elements } = session([defaultLoader()]); + created?.start(); + const params = created?.getRequestParams(); + const state = track(params); + + await vi.advanceTimersByTimeAsync(50); + expect(state.settled).toBe(true); + await expect(params).resolves.toMatchObject({ __clerk_protect_status: 'timeout' }); + expect(elements).toHaveLength(0); + }); + + it('stops at its own deadline when that comes before SafeLock gives up', async () => { + Object.defineProperty(navigator, 'locks', { + value: { + request: vi.fn( + (_key: string, options: { signal: AbortSignal }) => + new Promise((_resolve, reject) => { + options.signal.addEventListener('abort', () => reject(new DOMException('aborted', 'AbortError'))); + }), + ), + }, + configurable: true, + }); + + const { session: created, elements } = session([loader({ token_timeout_ms: 3_000 })]); + created?.start(); + const params = created?.getRequestParams(); + const state = track(params); + + await vi.advanceTimersByTimeAsync(2_999); + expect(state.settled).toBe(false); + await vi.advanceTimersByTimeAsync(1); + await expect(params).resolves.toMatchObject({ __clerk_protect_status: 'timeout' }); + + localStorage.setItem('__clerk_protect_st', storedEntry({ token: 'v1.leader.mac', rid: 'c'.repeat(26) })); + await vi.advanceTimersByTimeAsync(2_000); + expect(vi.getTimerCount()).toBe(0); + await expect(created?.getRequestParams()).resolves.toMatchObject({ __clerk_protect_token: 'v1.leader.mac' }); + expect(elements).toHaveLength(0); + }); + + it.each([ + [2_000, -3_600_000], + [6_000, -3_600_000], + [2_000, 3_600_000], + [6_000, 3_600_000], + ])('settles follower polling after a clock change at %s ms by %s ms', async (changeAt, correction) => { + let available = false; + const request = vi.fn((_key: string, options: { signal: AbortSignal }, callback: () => Promise) => { + if (available) { + return callback(); + } + return new Promise((_resolve, reject) => { + options.signal.addEventListener('abort', () => reject(new DOMException('aborted', 'AbortError'))); + }); + }); + Object.defineProperty(navigator, 'locks', { configurable: true, value: { request } }); + const { session: created, elements } = session([defaultLoader()]); + created?.start(); + const params = created?.getRequestParams(); + const state = track(params); + + await vi.advanceTimersByTimeAsync(changeAt); + vi.setSystemTime(Date.now() + correction); + await vi.advanceTimersByTimeAsync(9_999 - changeAt); + expect(state.settled).toBe(false); + await vi.advanceTimersByTimeAsync(1); + await expect(params).resolves.toMatchObject({ __clerk_protect_status: 'timeout' }); + expect(vi.getTimerCount()).toBe(0); + + available = true; + await vi.advanceTimersByTimeAsync(30_000); + const retry = created?.getRequestParams(); + await vi.advanceTimersByTimeAsync(0); + expect(request).toHaveBeenCalledTimes(2); + expect(elements).toHaveLength(1); + serveInline(elements[0], { cid: created?.placeholders().cid }); + await expect(retry).resolves.toMatchObject({ __clerk_protect_status: 'ok' }); }); }); diff --git a/packages/clerk-js/src/core/protectSession.ts b/packages/clerk-js/src/core/protectSession.ts index ee36a82b0e9..2be0731c856 100644 --- a/packages/clerk-js/src/core/protectSession.ts +++ b/packages/clerk-js/src/core/protectSession.ts @@ -31,9 +31,11 @@ const INLINE_TOKEN_GLOBAL = '__clerk_specter'; /** A token this close to expiry is treated as absent, so a fresh run starts before it lapses. */ const TOKEN_REFRESH_MARGIN_MS = 60 * 1_000; /** Acquisition deadline when the instance does not configure one. */ -const DEFAULT_TOKEN_TIMEOUT_MS = 5 * 1_000; +const DEFAULT_TOKEN_TIMEOUT_MS = 10 * 1_000; /** Ceiling on the instance-configured deadline, so no server value can stall a sign-in. */ const MAX_TOKEN_TIMEOUT_MS = 10 * 1_000; +const STORE_POLL_INTERVAL_MS = 100; +const LOCK_REFUSAL_WINDOW_MS = 4_000; /** A stored token claiming more remaining life than this was not minted by us. */ const MAX_TOKEN_LIFETIME_MS = 24 * 60 * 60 * 1_000; /** Longest token we will hand back, so a planted store entry cannot bloat a sign-in body. */ @@ -404,16 +406,17 @@ function isRetryableStatus(status: number): boolean { } function sleep(ms: number, signal: AbortSignal): Promise { + if (signal.aborted) { + return Promise.resolve(); + } return new Promise(resolve => { - const timer = setTimeout(resolve, ms); - signal.addEventListener( - 'abort', - () => { - clearTimeout(timer); - resolve(); - }, - { once: true }, - ); + const settle = () => { + clearTimeout(timer); + signal.removeEventListener('abort', settle); + resolve(); + }; + const timer = setTimeout(settle, ms); + signal.addEventListener('abort', settle, { once: true }); }); } @@ -622,7 +625,8 @@ export class ProtectSession { if (readStoredToken(this.#tokenStorageKey, TOKEN_SEND_MARGIN_MS, this.#floor)) { return; } - if (Date.now() - this.#lastAttemptAt < this.#backoffMs()) { + const elapsed = Date.now() - this.#lastAttemptAt; + if (elapsed >= 0 && elapsed < this.#backoffMs()) { return; } this.#startAcquisition(); @@ -666,22 +670,47 @@ export class ProtectSession { return 'ok'; } - const result = await this.#lock.acquireLockAndRun(async () => { - // Double-checked inside the lock. Without this re-read every tab runs the loader in turn, - // which is the exact failure this design exists to prevent. - if (this.#freshStoredToken()) { - return 'ok' satisfies ProtectStatus; + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), this.#timeoutMs); + let lockWaitedOut = false; + const lockTimer = setTimeout(() => { + lockWaitedOut = true; + }, LOCK_REFUSAL_WINDOW_MS); + try { + const result = await this.#lock.acquireLockAndRun(async () => { + // Double-checked inside the lock. Without this re-read every tab runs the loader in turn, + // which is the exact failure this design exists to prevent. + if (this.#freshStoredToken()) { + return 'ok' satisfies ProtectStatus; + } + return await this.#mint(); + }); + + if (typeof result === 'string') { + return result as ProtectStatus; } - return await this.#mint(); - }); - if (typeof result === 'string') { - return result as ProtectStatus; + // Refused: nobody is minting for us. Waited out: a leader may still be, so poll the store until our deadline. + if (!lockWaitedOut) { + return this.#freshStoredToken() ? 'ok' : 'timeout'; + } + return (await this.#awaitStoredToken(controller.signal)) ? 'ok' : 'timeout'; + } finally { + clearTimeout(timer); + clearTimeout(lockTimer); } + } - // The lock was never held — `SafeLock` gives up waiting after ~5s so a wedged leader delays - // nobody. Whatever a leader did manage to write is still ours to use. - return this.#freshStoredToken() ? 'ok' : 'timeout'; + async #awaitStoredToken(signal: AbortSignal): Promise { + for (;;) { + if (this.#freshStoredToken()) { + return true; + } + if (signal.aborted) { + return false; + } + await sleep(STORE_POLL_INTERVAL_MS, signal); + } } /** Runs the loader and takes the token it was served with. Never rejects. */ diff --git a/packages/shared/src/types/protectConfig.ts b/packages/shared/src/types/protectConfig.ts index 45ea9c20f01..c6b1b637656 100644 --- a/packages/shared/src/types/protectConfig.ts +++ b/packages/shared/src/types/protectConfig.ts @@ -34,7 +34,7 @@ export interface ProtectLoader { token_url?: string; /** * How long to wait for the token before giving up and reporting a status instead. Defaults to - * 5000 and is capped at 10000 by the SDK, so this cannot stall a sign-in. + * 10000 and is capped at 10000 by the SDK, so this cannot stall a sign-in. */ token_timeout_ms?: number; /**