fix(frontend): sequence auth refresh against session invalidation (P3-4) - #82
Merged
Merged
Conversation
`refresh()` wrote the `fetchAuthStatus` result into auth state unconditionally, with no sequencing against the `scrye:auth-invalidated` event or `logout()`. A status request answered before the credential was revoked but resolving after the invalidation restored the stale `user`, flashing the authenticated shell back over the login screen. Enforce the invariant that a logged-out session is never restored by a late-arriving refresh: a `sessionGeneration` ref is bumped by every invalidation (the 401 event handler, and `logout()` before it awaits the request), and `refresh()` compares the generation across its `await` — applying the session-independent facts (`needs_setup`, `oidc`, loading cleared) but forcing `user: null` when the session died mid-flight. `refresh()` also takes a `createLatestGuard()` token so two overlapping refreshes cannot resolve out of order. Adds a jsdom test covering both races and the happy path. See `docs/ARCHIVE.md` § Deviations for the dated entry.
4 tasks
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.
Summary
Closes P3-4, the last open finding in the frontend-review Priority-3 batch (
docs/reviews/frontend-review.md§ Priority 3, tracked indocs/reviews/STATUS.md§ 1).AuthContext.refresh()wrote thefetchAuthStatusresult into auth state unconditionally, with no sequencing against thescrye:auth-invalidatedevent (dispatched byapi/client.tson any 401) or againstlogout(). A status request answered by the backend before the credential was revoked, but resolving after the invalidation, wrote its staleuserback — flashing the authenticated shell over the login screen until the next 401.Invariant now enforced: a logged-out session is never restored by a late-arriving refresh. Once the session is invalidated, no auth-state write from a request that was already in flight at that moment may restore
user; the session can only be re-entered by a fresh authentication.What changed
frontend/src/auth/AuthContext.tsxsessionGenerationref is bumped by every invalidation: thescrye:auth-invalidatedhandler, andlogout()— the latter before awaiting the request, so a refresh already in flight when sign-out starts is covered too.refresh()captures the generation before fetching and re-checks it after theawait. If it changed, the session-independent facts (needs_setup,oidc,loading: false) are still applied butuseris forced tonull. Keeping those fields is what prevents an invalidation during the initial load from stranding the app on the loading spinner.refresh()additionally takes a token from the existingcreateLatestGuard()helper (lib/latest.ts— the same latest-wins guard the history/findings fetches use), so two overlapping refreshes cannot resolve out of order and clobber each other.frontend/src/auth/AuthContext.test.tsx(new, jsdom) — holds the status request open and drives resolution order: (1) invalidation event fires mid-refresh → stays signed out; (2)logout()lands mid-refresh → stays signed out; (3) no invalidation → the refresh applies normally. Both race tests were confirmed to fail against the pre-fix provider and pass with it.frontend/src/test/render.tsx— re-exportsactso tests keep importing the whole Testing Library surface from one place.docs/ARCHIVE.md§ 14 entry;docs/reviews/STATUS.mdmoves P3-4 from open to Resolved (the "LOW with a correctness/security edge" bucket is now empty; only P3-8's deferred TS-strictness flags remain).A generation counter was chosen over an
AbortControllerbecause the invalidation must also invalidate a response that has already been received but not yet applied, which aborting does not cover — and it reuses the guard idiom already in the codebase rather than adding a new one.Verification
npm test— 36 tests / 11 files pass (3 new).npm run lint,npx prettier --check src,npm run build(tsc -b && vite build) — all clean.See
docs/ARCHIVE.md§ Deviations for the dated entry.