Skip to content

Commit 690edf2

Browse files
authored
fix(js): keep a Device Intelligence result that arrives after the deadline (#10276)
1 parent 870b9a7 commit 690edf2

3 files changed

Lines changed: 298 additions & 6 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@clerk/clerk-js': patch
3+
---
4+
5+
Keep a Device Intelligence result that arrives after ClerkJS has stopped waiting for it, and use it for the next sign-in or sign-up instead of discarding it. Users on slow connections who retry are less likely to have sign-ups blocked by protections that require a Device Intelligence result.

‎packages/clerk-js/src/core/__tests__/protectSession.test.ts‎

Lines changed: 238 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -741,6 +741,244 @@ describe('ProtectSession deadlines', () => {
741741
});
742742
});
743743

744+
describe('ProtectSession late token', () => {
745+
const STORE_KEY = '__clerk_protect_st';
746+
747+
beforeEach(() => {
748+
vi.useFakeTimers();
749+
});
750+
751+
afterEach(() => {
752+
vi.useRealTimers();
753+
delete (navigator as unknown as Record<string, unknown>).locks;
754+
});
755+
756+
const timedOut = async () => {
757+
const h = session([loader({ token_timeout_ms: 5_000 })]);
758+
h.session?.start();
759+
const first = h.session?.getRequestParams();
760+
await vi.advanceTimersByTimeAsync(5_000);
761+
await expect(first).resolves.toMatchObject({ __clerk_protect_status: 'timeout' });
762+
return h;
763+
};
764+
765+
it('keeps a token whose loader finishes after the deadline for the next sign-in', async () => {
766+
const { session: created, elements } = await timedOut();
767+
768+
await vi.advanceTimersByTimeAsync(2_000);
769+
serveInline(elements[0], { cid: created?.placeholders().cid });
770+
await vi.advanceTimersByTimeAsync(0);
771+
772+
await expect(created?.getRequestParams()).resolves.toMatchObject({
773+
__clerk_protect_token: 'v1.payload.mac',
774+
__clerk_protect_status: 'ok',
775+
});
776+
// Still inside the cooldown, so the token can only have come from the late load.
777+
expect(elements).toHaveLength(1);
778+
});
779+
780+
it('keeps a token whose ready settles after the deadline', async () => {
781+
const { session: created, elements } = session([loader({ token_timeout_ms: 5_000 })]);
782+
created?.start();
783+
const first = created?.getRequestParams();
784+
785+
let release: (value: unknown) => void = () => undefined;
786+
await vi.advanceTimersByTimeAsync(1_000);
787+
serveInline(elements[0], {
788+
cid: created?.placeholders().cid,
789+
ready: new Promise(resolve => {
790+
release = resolve;
791+
}),
792+
});
793+
await vi.advanceTimersByTimeAsync(4_000);
794+
await expect(first).resolves.toMatchObject({ __clerk_protect_status: 'timeout' });
795+
796+
// Well past a second deadline window, so only an unbounded late read can still catch it.
797+
await vi.advanceTimersByTimeAsync(7_000);
798+
release({ token: 'v1.late.mac', exp: nowSeconds() + 43_200 });
799+
await vi.advanceTimersByTimeAsync(0);
800+
801+
await expect(created?.getRequestParams()).resolves.toMatchObject({
802+
__clerk_protect_token: 'v1.late.mac',
803+
__clerk_protect_status: 'ok',
804+
});
805+
});
806+
807+
it('does not replace a stored token that lasts longer', async () => {
808+
const { session: created, elements } = await timedOut();
809+
810+
localStorage.setItem(
811+
STORE_KEY,
812+
storedEntry({ token: 'v1.leader.mac', rid: 'c'.repeat(26), exp: nowSeconds() + 43_260 }),
813+
);
814+
serveInline(elements[0], { cid: created?.placeholders().cid });
815+
await vi.advanceTimersByTimeAsync(0);
816+
817+
await expect(created?.getRequestParams()).resolves.toMatchObject({ __clerk_protect_token: 'v1.leader.mac' });
818+
});
819+
820+
it('replaces a stored token that is still usable but expires sooner', async () => {
821+
const { session: created, elements } = await timedOut();
822+
823+
localStorage.setItem(STORE_KEY, storedEntry({ token: 'v1.old.mac', rid: 'c'.repeat(26), exp: nowSeconds() + 45 }));
824+
serveInline(elements[0], { cid: created?.placeholders().cid });
825+
await vi.advanceTimersByTimeAsync(0);
826+
827+
await expect(created?.getRequestParams()).resolves.toMatchObject({ __clerk_protect_token: 'v1.payload.mac' });
828+
});
829+
830+
it('does not replace a stored token stamped with a newer invalidation floor', async () => {
831+
const floor = nowSeconds() - 60;
832+
const created = session([loader({ token_timeout_ms: 5_000 })], floor);
833+
created.session?.start();
834+
const first = created.session?.getRequestParams();
835+
await vi.advanceTimersByTimeAsync(5_000);
836+
await expect(first).resolves.toMatchObject({ __clerk_protect_status: 'timeout' });
837+
838+
localStorage.setItem(
839+
STORE_KEY,
840+
storedEntry({ token: 'v1.newer.mac', rid: 'c'.repeat(26), exp: nowSeconds() + 3_600, floor: floor + 30 }),
841+
);
842+
serveInline(created.elements[0], { cid: created.session?.placeholders().cid });
843+
await vi.advanceTimersByTimeAsync(0);
844+
845+
await expect(created.session?.getRequestParams()).resolves.toMatchObject({ __clerk_protect_token: 'v1.newer.mac' });
846+
});
847+
848+
it('waits for the cross-tab lock before storing a late token', async () => {
849+
let tail: Promise<unknown> = Promise.resolve();
850+
Object.defineProperty(navigator, 'locks', {
851+
value: {
852+
request: vi.fn((_key: string, _options: unknown, callback: () => Promise<unknown>) => {
853+
const result = tail.then(() => callback());
854+
tail = result.catch(() => undefined);
855+
return result;
856+
}),
857+
},
858+
configurable: true,
859+
});
860+
861+
const a = await timedOut();
862+
863+
// Another tab takes the lock and starts minting.
864+
const b = session([loader({ token_timeout_ms: 5_000 })]);
865+
b.session?.start();
866+
await vi.advanceTimersByTimeAsync(0);
867+
expect(b.elements).toHaveLength(1);
868+
869+
serveInline(a.elements[0], { cid: a.session?.placeholders().cid });
870+
await vi.advanceTimersByTimeAsync(0);
871+
expect(localStorage.getItem(STORE_KEY)).toBeNull();
872+
873+
serveInline(b.elements[0], {
874+
cid: b.session?.placeholders().cid,
875+
ready: Promise.resolve({ token: 'v1.b.mac', exp: nowSeconds() + 43_260 }),
876+
});
877+
await vi.advanceTimersByTimeAsync(0);
878+
879+
expect(JSON.parse(localStorage.getItem(STORE_KEY) as string)).toMatchObject({ token: 'v1.b.mac' });
880+
});
881+
882+
it.each([
883+
{ name: 'an empty store', stored: undefined, token: 'v1.payload.mac' },
884+
{ name: 'a longer-lived stored token', stored: { exp: 43_260, floor: 0 }, token: 'v1.leader.mac' },
885+
{ name: 'a newer invalidation floor', stored: { exp: 3_600, floor: 30 }, token: 'v1.leader.mac' },
886+
])('retries a timed-out lock and respects $name', async ({ stored, token }) => {
887+
let calls = 0;
888+
const request = vi.fn((_key: string, options: { signal: AbortSignal }, callback: () => Promise<unknown>) => {
889+
if (++calls === 2) {
890+
return new Promise((_resolve, reject) => {
891+
options.signal.addEventListener('abort', () => reject(new DOMException('aborted', 'AbortError')));
892+
});
893+
}
894+
return callback();
895+
});
896+
Object.defineProperty(navigator, 'locks', { value: { request }, configurable: true });
897+
898+
const { session: created, elements } = await timedOut();
899+
await vi.advanceTimersByTimeAsync(2_000);
900+
serveInline(elements[0], { cid: created?.placeholders().cid });
901+
await vi.advanceTimersByTimeAsync(4_999);
902+
903+
expect(request).toHaveBeenCalledTimes(2);
904+
expect(localStorage.getItem(STORE_KEY)).toBeNull();
905+
await expect(created?.getRequestParams()).resolves.toMatchObject({ __clerk_protect_status: 'timeout' });
906+
907+
if (stored) {
908+
localStorage.setItem(
909+
STORE_KEY,
910+
storedEntry({ token, rid: 'c'.repeat(26), exp: nowSeconds() + stored.exp, floor: stored.floor }),
911+
);
912+
}
913+
await vi.advanceTimersByTimeAsync(1_000);
914+
915+
expect(request).toHaveBeenCalledTimes(3);
916+
await expect(created?.getRequestParams()).resolves.toMatchObject({
917+
__clerk_protect_token: token,
918+
__clerk_protect_status: 'ok',
919+
});
920+
expect(elements).toHaveLength(1);
921+
});
922+
923+
it('stops retrying the lock when the late token is no longer usable', async () => {
924+
let calls = 0;
925+
const request = vi.fn((_key: string, options: { signal: AbortSignal }, callback: () => Promise<unknown>) => {
926+
if (++calls > 1) {
927+
return new Promise((_resolve, reject) => {
928+
options.signal.addEventListener('abort', () => reject(new DOMException('aborted', 'AbortError')));
929+
});
930+
}
931+
return callback();
932+
});
933+
Object.defineProperty(navigator, 'locks', { value: { request }, configurable: true });
934+
935+
const { session: created, elements } = await timedOut();
936+
serveInline(elements[0], {
937+
cid: created?.placeholders().cid,
938+
ready: Promise.resolve({ token: 'v1.late.mac', exp: nowSeconds() + 12 }),
939+
});
940+
await vi.advanceTimersByTimeAsync(20_000);
941+
942+
expect(request).toHaveBeenCalledTimes(3);
943+
expect(localStorage.getItem(STORE_KEY)).toBeNull();
944+
});
945+
946+
it('revalidates a late token after waiting for the lock', async () => {
947+
let calls = 0;
948+
const request = vi.fn((_key: string, _options: unknown, callback: () => Promise<unknown>) => {
949+
if (++calls === 2) {
950+
return new Promise(resolve => setTimeout(resolve, 4_000)).then(callback);
951+
}
952+
return callback();
953+
});
954+
Object.defineProperty(navigator, 'locks', { value: { request }, configurable: true });
955+
956+
const { session: created, elements } = await timedOut();
957+
serveInline(elements[0], {
958+
cid: created?.placeholders().cid,
959+
ready: Promise.resolve({ token: 'v1.late.mac', exp: nowSeconds() + 7 }),
960+
});
961+
await vi.advanceTimersByTimeAsync(4_000);
962+
963+
expect(request).toHaveBeenCalledTimes(2);
964+
expect(localStorage.getItem(STORE_KEY)).toBeNull();
965+
await expect(created?.getRequestParams()).resolves.toMatchObject({ __clerk_protect_status: 'timeout' });
966+
});
967+
968+
it.each([
969+
['minted for another page', { cid: 'someone-else' }],
970+
['carrying no token', { ready: Promise.resolve({ status: 'no_token' }) }],
971+
])('ignores a late result %s', async (_name, overrides) => {
972+
const { session: created, elements } = await timedOut();
973+
974+
serveInline(elements[0], { cid: created?.placeholders().cid, ...overrides });
975+
await vi.advanceTimersByTimeAsync(0);
976+
977+
await expect(created?.getRequestParams()).resolves.toMatchObject({ __clerk_protect_status: 'timeout' });
978+
expect(localStorage.getItem(STORE_KEY)).toBeNull();
979+
});
980+
});
981+
744982
describe('ProtectSession cross-tab single flight', () => {
745983
/**
746984
* A serialising Web Locks stand-in. jsdom has no `navigator.locks`, and the `browser-tabs-lock`

‎packages/clerk-js/src/core/protectSession.ts‎

Lines changed: 55 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ const TOKEN_SEND_MARGIN_MS = 5 * 1_000;
5656
/** Bounds we hold the server-supplied `retry_in_ms` to. */
5757
const MIN_RETRY_DELAY_MS = 50;
5858
const MAX_RETRY_DELAY_MS = 1_000;
59+
const LATE_TOKEN_RETRY_DELAY_MS = 1_000;
5960

6061
/** Lowercase RFC 4648 base32 — a `cid` has to survive case-insensitive, alphanumeric-only contexts. */
6162
const BASE32_ALPHABET = 'abcdefghijklmnopqrstuvwxyz234567';
@@ -366,7 +367,7 @@ function settleWithin(ready: PromiseLike<unknown>, deadline: number): Promise<un
366367
* `timeout`: until the server half deploys it is the correct answer for every load, and calling
367368
* that a timeout would make a normal rollout look like an outage.
368369
*/
369-
async function readInlineToken(cid: string, rid: string, deadline: number): Promise<MintedToken | ProtectStatus> {
370+
async function readInlineToken(cid: string, rid: string, deadline?: number): Promise<MintedToken | ProtectStatus> {
370371
const inline = (globalThis as unknown as Record<string, unknown>)[INLINE_TOKEN_GLOBAL];
371372
if (!inline || typeof inline !== 'object') {
372373
return 'no_token';
@@ -379,7 +380,8 @@ async function readInlineToken(cid: string, rid: string, deadline: number): Prom
379380
return 'no_token';
380381
}
381382

382-
const resolved = await settleWithin(ready, deadline);
383+
const resolved =
384+
deadline === undefined ? await Promise.resolve(ready).catch(() => null) : await settleWithin(ready, deadline);
383385
if (resolved === DEADLINE) {
384386
return 'timeout';
385387
}
@@ -688,7 +690,7 @@ export class ProtectSession {
688690
async #mint(): Promise<ProtectStatus> {
689691
const deadline = Date.now() + this.#timeoutMs;
690692

691-
const outcome = await this.#runTokenLoader(deadline);
693+
const outcome = await this.#runTokenLoader(deadline, () => void this.#keepLateToken());
692694
if (outcome === 'error') {
693695
return 'script_error';
694696
}
@@ -701,6 +703,9 @@ export class ProtectSession {
701703
}
702704

703705
const inline = await readInlineToken(this.#cid as string, this.#rid as string, deadline);
706+
if (inline === 'timeout') {
707+
void this.#keepLateToken();
708+
}
704709
if (typeof inline === 'string') {
705710
return inline;
706711
}
@@ -709,11 +714,38 @@ export class ProtectSession {
709714
return 'ok';
710715
}
711716

717+
async #keepLateToken(): Promise<void> {
718+
if (this.#tokenUrl) {
719+
return;
720+
}
721+
const late = await readInlineToken(this.#cid as string, this.#rid as string);
722+
if (typeof late === 'string') {
723+
return;
724+
}
725+
while (validateToken(late.token, late.exp, TOKEN_SEND_MARGIN_MS)) {
726+
const saved = await this.#lock.acquireLockAndRun(() => {
727+
if (!validateToken(late.token, late.exp, TOKEN_SEND_MARGIN_MS)) {
728+
return Promise.resolve(true);
729+
}
730+
const stored = readStoredToken(this.#tokenStorageKey, TOKEN_SEND_MARGIN_MS, this.#floor);
731+
if (!stored || (stored.floor <= this.#floor && stored.exp < late.exp)) {
732+
writeStoredToken(this.#tokenStorageKey, late, this.#floor);
733+
this.#consecutiveFailures = 0;
734+
}
735+
return Promise.resolve(true);
736+
});
737+
if (saved === true) {
738+
return;
739+
}
740+
await new Promise(resolve => setTimeout(resolve, LATE_TOKEN_RETRY_DELAY_MS));
741+
}
742+
}
743+
712744
/**
713745
* Injects the token loader and settles on whichever of `load`, `error` or the deadline comes
714746
* first, or immediately when the element has nothing to fetch.
715747
*/
716-
#runTokenLoader(deadline: number): Promise<LoaderOutcome> {
748+
#runTokenLoader(deadline: number, onLateLoad?: () => void): Promise<LoaderOutcome> {
717749
const loader = this.#tokenLoader;
718750
if (!loader) {
719751
return Promise.resolve('error');
@@ -739,12 +771,29 @@ export class ProtectSession {
739771
}
740772

741773
return new Promise<LoaderOutcome>(resolve => {
742-
const timer = setTimeout(() => resolve('timeout'), Math.max(0, deadline - Date.now()));
774+
let timedOut = false;
775+
const timer = setTimeout(
776+
() => {
777+
timedOut = true;
778+
resolve('timeout');
779+
},
780+
Math.max(0, deadline - Date.now()),
781+
);
743782
const settle = (outcome: LoaderOutcome) => () => {
744783
clearTimeout(timer);
745784
resolve(outcome);
746785
};
747-
element.addEventListener('load', settle('loaded'), { once: true });
786+
element.addEventListener(
787+
'load',
788+
() => {
789+
if (timedOut) {
790+
onLateLoad?.();
791+
return;
792+
}
793+
settle('loaded')();
794+
},
795+
{ once: true },
796+
);
748797
element.addEventListener('error', settle('error'), { once: true });
749798
});
750799
}

0 commit comments

Comments
 (0)