Repository navigation
fix: follow server time in the client token store when the local clock is behind - #516
theluckystrike wants to merge 1 commit into
Conversation
[Medium impact] The change reaches the package’s client-side access-token workflow. The PR should address late refreshes for older session tokens before merging. Findings
Fix with agent prompt### Issue 1
src/components/tokenStore.ts:221
`updateClockSkew` treats the token's issue time as the current server time. But the eager cookie can contain an older token: `respondWithCurrentToken` copies the existing `session.accessToken`.
With a browser 600 seconds behind and a 300-second token issued 120 seconds ago, this records only 480 seconds of clock difference. The store schedules refresh in 270 seconds, although the token expires in 180 seconds. During that gap, `getAccessToken()` returns the expired token, so API requests can still fail.
Use a current server-time reference or a freshly issued token before calculating the refresh time, and add a test with an older cookie token.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryAdds a stored clock difference based on each new token's
Reviews (1) · Last reviewed commit: "fix: follow server time in the client to..." · Reviewed by Greptile |
| try { | ||
| const { payload } = decodeJwt(token); | ||
| const localNow = Math.floor(Date.now() / 1000); | ||
| this.clockSkewSeconds = typeof payload.iat === 'number' ? Math.max(0, payload.iat - localNow) : 0; |
There was a problem hiding this comment.
updateClockSkew treats the token's issue time as the current server time. But the eager cookie can contain an older token: respondWithCurrentToken copies the existing session.accessToken.
With a browser 600 seconds behind and a 300-second token issued 120 seconds ago, this records only 480 seconds of clock difference. The store schedules refresh in 270 seconds, although the token expires in 180 seconds. During that gap, getAccessToken() returns the expired token, so API requests can still fail.
Use a current server-time reference or a freshly issued token before calculating the refresh time, and add a test with an older cookie token.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/tokenStore.ts
Line: 221
Comment:
**Older tokens refresh too late**
`updateClockSkew` treats the token's issue time as the current server time. But the eager cookie can contain an older token: `respondWithCurrentToken` copies the existing `session.accessToken`.
With a browser 600 seconds behind and a 300-second token issued 120 seconds ago, this records only 480 seconds of clock difference. The store schedules refresh in 270 seconds, although the token expires in 180 seconds. During that gap, `getAccessToken()` returns the expired token, so API requests can still fail.
Use a current server-time reference or a freshly issued token before calculating the refresh time, and add a test with an older cookie token.
**Knowledge Base Used:**
- [AuthKit provider and token state](https://app.greptile.com/-/switch-tenant?tenant=workos&next=%2Fcustom-context%2Fknowledge-base%2Fworkos%2Fauthkit-nextjs%2F-%2Fdocs%2Fauthkit-provider-and-token-state.md)
- [Restore continuous access-token refresh scheduling](https://app.greptile.com/-/switch-tenant?tenant=workos&next=%2Fcustom-context%2Fknowledge-base%2Fworkos%2Fauthkit-nextjs%2F-%2Freverts%2Frollback_489-20261001-access-token-refresh-scheduling-93f7b3b.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
The client tokenStore decides whether a token is expiring with Date.now() alone. On a browser whose clock runs behind the server, getAccessToken() keeps returning a token the server has already rejected, and the refresh timer is scheduled from the same skewed arithmetic. So every token lifetime repeats the failure for as long as the page stays open. With a 5 minute token and a clock 10 minutes behind, the store treats a fresh token as valid for 15 minutes and refreshes it 10 minutes too late.
A token's iat claim proves the server clock had already reached that time. The store now records how far its local clock is behind the iat of each new token it receives (from the eager cookie, the server actions or a refresh), never less than zero, and adds that difference to the current time in parseToken. Expiry checks and the refresh timer then follow server time. The difference is recomputed on every new token, so when the local clock is corrected the next token brings it back to zero. A clock that runs ahead is not touched and behaves as before, and a token without iat leaves the store on the local clock as it does today.
This does not add a way for the app to report a 401 to the store, the second option in the issue. That is a public API choice, so I left it out.
Three tests in src/components/tokenStore.scheduling.spec.ts cover a client 600 seconds behind the server. The first checks that a 300 second token is refreshed after 270 seconds for three cycles in a row. The second checks that getAccessToken() refreshes instead of returning the cached token once the server expiry has passed. The third checks that the store goes back to the local clock once that clock catches up. With the new tests on the unfixed tokenStore.ts,
npx vitest run src/components/tokenStore.scheduling.spec.tsgives 18 passed and 2 failed (the first two). With the fix it gives 20 passed.pnpm testpasses all 553 tests in 27 files, andpnpm typecheck,pnpm build,oxlintandoxfmt --checkon both touched files report no problems.Fixes #470