fix(auth): stop refreshing with a superseded refresh token - #106
Merged
Conversation
Refresh tokens are single-use: the server rotates on every refresh and revokes the whole family when a superseded one is presented outside its 5 minute grace window (auth.refresh_reuse_revoked). Users saw this as a sudden "login expired" with no way to tell it from ordinary expiry. The interceptor treated its in-memory copy as the source of truth and never re-read the store before refreshing. Every command builds its own interceptor and `drive watch` rebuilds one per reconnect, so a second interceptor could hold a token another had already rotated and present it minutes later -- matching the ~8 minute gap_ms on the reported event. Re-read the persisted tokens immediately before refreshing and adopt them when they are newer, skipping the rotation entirely if the adopted access token is still usable. Also single-flight the refresh so a Promise.all of expired requests shares one rotation, and skip the reactive refresh when a concurrent one already replaced the token the 401 came back for. Covers both with regression tests: a concurrent double refresh and a stale-copy refresh, each driven through a mock that rotates and revokes the way the server does.
Re-reading the config before every refresh means reads now happen constantly while other `wspc` processes are writing, and `write` was truncating the file in place. A reader could observe a half-written config and take a JSON parse error out through `loadPersisted`, killing the command instead of refreshing. Write a sibling file and rename over the target so a reader always sees one whole config or the other. Covered by a test that reads in a loop against concurrent writes: it fails on every run without the rename. Also share the token-endpoint fake between the two reuse tests and route the proactive-refresh check through the same freshness helper the adopt path uses, so the skew arithmetic exists once.
The server now names the reason in `error_description` rather than sending a constant, so a revoked session no longer has to be reported as "token refresh failed (invalid_grant)". Map the reasons a user acts on differently — a reuse revocation means something is holding a stale copy of the credentials, not that the session lapsed — and keep echoing the raw code for anything this build doesn't recognise, so an older or newer server still produces a usable message.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
為什麼
同事回報 CLI 突然「登入已過期」（WSPC todo
tod_01KYXPN96YQRJ2DJAEG2Q4ZYN5，Sentry WSPC-AUTH-WORKER-1）。不是 token 自然過期，是 server 的 refresh token 重用偵測撤銷了整個 family。Sentry 事件的
gap_ms是 501465（約 8.4 分鐘），遠超 server 的 5 分鐘 grace window。所以是某個 interceptor 拿著 8 分鐘前就已經被 rotate 掉的 token 去 refresh，不是毫秒級的並發競態（那個 grace window 本來就會吸收，回grace_tolerated不撤銷）。interceptor 把 token 存在 closure 裡當 source of truth，refresh 前從不重讀 config。每個指令各建一個 interceptor，
drive watch每次重連還會再建一個——所以第二個 interceptor 很容易握著別人已經 rotate 掉的 token，幾分鐘後才拿去用。改了什麼
ConfigStore.write()改成 write-then-rename：因為 refresh 現在每次都重讀，讀取會頻繁跟別的 process 的寫入重疊，而原本是 truncate-in-place，reader 可能讀到寫到一半的檔案。error_description，把 reuse 撤銷跟一般過期分開講。修改全部在
src/handwritten/，沒有碰任何 generated code（interceptor 是透過createConfig({ fetch })注入 generated client 的，npm run generate不會洗掉）。需要搭配 sadcoderlabs/wspc 的對應 PR 才會有分辨得出來的錯誤訊息；沒有它也能運作（會 fall back 成原本的格式）。
Test plan
npm run typecheck— 乾淨npm test— 62 檔 630 測試通過load-sdk-client的接線測試——實際把loadPersisted接線拿掉跑過，確認轉紅（只測sdk-auth.ts的話，接線被刪掉測試仍會全綠而 bug 回歸）SyntaxError: Unterminated string in JSON