feat(clerk-js): Monotonic token replacement based on oiat#8097
Conversation
🦋 Changeset detectedLatest commit: e157d4d The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
dc3c6e9 to
cf6803d
Compare
f0b2a14 to
cbc83a0
Compare
Session Minter uses oiat (original_issued_at) in the JWT header to track when token claims were last assembled from the DB. Edge re-mints copy this value forward, so consumers can determine claim freshness regardless of how many times the token was re-signed. Marked @internal so developers don't depend on this field.
Prevent multi-tab race conditions where an edge-minted token with stale claims overwrites a fresher DB-minted token. Uses `oiat ?? iat` as the claim freshness metric. A token with oiat (JWT header) uses oiat as its claim freshness. A token without oiat is origin-minted (coupled FF), so iat represents claim freshness. Four guard points: 1. tokenCache handleBroadcastMessage - replaces old iat comparison 2. tokenCache setInternal - async compare-and-swap at resolve time 3. Session #dispatchTokenEvents - before token:update emit 4. AuthCookieService updateSessionCookie - cookie chokepoint with session scoping (different sessions always allowed through) Guard 4 catches the sleeping tab edge case where in-memory guards pass (stale baseline) but the cookie has a fresher value from another tab.
cf6803d to
43ad1e4
Compare
43ad1e4 to
91b664b
Compare
ESLint simple-import-sort/imports flagged pickFreshestJwt as misplaced. Move it to the correct alphabetical slot (after resources/Environment, before ./cookies/...).
The cookie write guard at AuthCookieService.updateSessionCookie was causing integration test failures across handshake, sessions, and multiple framework matrices. The guard would reject token writes when oiat+iat matched, but two tokens with identical timestamps can still differ in OTHER claims (azp added in a recent token-format rollout, org_id, etc.). Backend then logged 'Session token from cookie is missing the azp claim' and treated the session as invalid, redirecting to /sign-in. The broadcast handler (tokenCache.ts:292) and Session resource (Session.ts:463, :526) keep the monotonic enforcement at the layers where it works correctly. The cookie chokepoint was too aggressive. The cookie path deserves a guard but with a different shape (e.g., raw-string equality or signature compare), not the claim-timestamp shape.
00f872b to
78b3328
Compare
| return; | ||
| } | ||
|
|
||
| if (this.lastActiveToken && pickFreshestJwt(this.lastActiveToken, token) === this.lastActiveToken) { |
There was a problem hiding this comment.
The suppression test at Session.test.ts:103 only proves the guard can suppress, not that it does so for the right reason.
And every test that asserts dispatch=2 (lines 186, 242, 60) constructs the session without last_active_token, so the this.lastActiveToken && … short-circuit means the comparator is never actually called.
We should probably add a test where lastActiveToken is set with stale oiat, a fresher-oiat token arrives via cache (broadcast from another tab) or fetch, and assert token:update fires with the fresher token
bratsos
left a comment
There was a problem hiding this comment.
Minor comment re:tests, LGTM!
Two tokens with identical oiat+iat may still differ in other claims (azp, org_id, etc.) added in a token-format rollout. The previous 'no churn on tie' rule suppressed legitimate updates and caused the backend to read stale claim sets, redirecting to /sign-in. Only suppress when existing is strictly fresher.
3b558c3 to
7efa1af
Compare
The Session.ts guards at #_getToken cache-hit emit and #dispatchTokenEvents were suppressing token:update events that AuthCookieService needs to write the session cookie. Backend then saw an empty/stale cookie and treated the session as unauthenticated. Keep only the broadcast handler guard in tokenCache.ts, which covers the original motivation: cross-tab races where a background tab's stale edge-minted token can clobber a fresher DB-minted token via the BroadcastChannel.
77d5f09 to
64ab4ae
Compare
…er cached one Inverse of the existing 'older broadcast does not overwrite newer' test. Confirms the monotonic guard is direction-correct: a fresher oiat replaces an older cached entry rather than being suppressed.
The handler is async (awaits the existing tokenResolver), so reading the cache synchronously after broadcastListener() captures the pre-await state. Type the listener as returning void | Promise<void> and await it so the second broadcast finishes processing before we assert the new createdAt.
…ehavior Doc said 'on a tie, returns existing (no churn)' but the implementation returns incoming on full ties. Rewrite the doc to match: only suppress when existing is strictly fresher; on a tie, hand through to incoming since the two tokens may differ in claims that don't affect freshness. Also await the broadcast handler in the older-rejected monotonicity test so it doesn't pass vacuously when the async handler runs after the assertion. Same shape as the newer-replaces-older test.
The cache.get path drops entries whose iat+ttl is past the current test clock (nowSec=1666648260, POLLER_INTERVAL=5s). With the default ttl=60, the older token (iat=1666648190, exp=1666648250) was 10s expired and got purged before the assertion could read it. Use ttl=120 so both older (exp=1666648310) and newer (exp=1666648370) stay valid against the fixed test clock.
…avascript into pr-8097-fix
Why
With Session Minter, edge-minted tokens can have fresh
iat(just minted) but stale claims (copied from an old parent). In multi-tab BroadcastChannel scenarios, a background tab's stale edge token can clobber a fresher DB-minted token in another tab's cache because the old broadcast guard comparediatonly, which doesn't reflect claim freshness for edge-minted tokens.What
Introduces
oiat(origin-issued-at, JWT header) as the claim freshness metric for the cross-tab broadcast comparison:oiat: oiat = when claims were last read from DBoiat: pre-feature legacy token, by definition staler than any oiat-bearing tokenDecision table
pickFreshestJwt(existing, incoming)returns whichever side wins. The broadcast handler uses=== existingto detect "skip this broadcast".A1-A5 is the main path under universal oiat. B1-B3 is the legacy safety net for pre-rollout tokens. On a full tie (A5), incoming wins rather than existing: two tokens with identical oiat+iat may still differ in other claims (azp, org_id, etc.) added in a token-format rollout, so we only suppress when existing is strictly fresher.
Guard site
The shared comparator (
tokenFreshness.ts) is invoked at one site:tokenCache.ts handleBroadcastMessage- replaces the old iat comparison on cross-tab broadcastsThe Session resource and AuthCookieService originally also added guards but those caused regressions on the auth flow (suppressing legitimate token-update events that AuthCookieService needs to write the cookie) and have been removed. The broadcast handler is the only site where a stale edge-minted token could realistically race with a fresher one without other in-flight protections.
Test plan