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
Wrap the b64 arrow expression in createJwtWithOiat over two lines to match the project's prettier printWidth. CI flagged the original one-liner as a format violation.
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