feat(auth): SPA session-refresh driver — cross-tab /auth/refresh timer (#1854) - #218
Conversation
The session model is deliberately non-sliding: session_ttl (default 600 s) is extended only by an explicit POST /auth/refresh, so without a driver every user is force-logged-out ~10 min after login regardless of activity (constructorfabric/insight#1854, PRD 5.4 "SPA contract"). - Session gains expires_at/refresh_at (unix seconds), read from /auth/me at boot; the driver always schedules from the server-supplied refresh_at (re-jittered on every response), never a client-computed one. - Single refresher across tabs: leadership is a heartbeat-renewed localStorage lease (stale leases are stolen; pagehide releases early); results fan out over BroadcastChannel with a localStorage storage-event fallback. Races are tolerated by the backend's rotation grace + CAS. - POST carries X-CSRF-Token (fail-closed server middleware, #212) and an abort timeout so a hung request can't wedge the driver. - 401 → clear store, fan out "expired", bounce to /auth/login. 429 → honor Retry-After. 403 → re-prime csrf/schedule via /auth/me with a retry floor (no zero-delay loop if the 403 persists), or expire if the re-probe fails closed. Transient errors retry on a 15 s cadence. - visibilitychange re-evaluates on wake so a sleep-overdue refresh fires (or a dead leader is replaced) immediately. - MSW: MOCK_SESSION now carries csrf_token (loadSession's fail-closed guard was rejecting mock sessions) + timestamps and a /auth/refresh handler. - Tests build sessions via a shared makeSession() factory so the next Session field lands in one place instead of ten fixture literals. EPIC: constructorfabric/insight#1583 Signed-off-by: Anton Zelenov <antonz@constructor.tech>
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughChangesThe PR adds server-provided session timing fields, a cross-tab SPA refresh driver with retry and expiration handling, startup integration, refreshed authentication mocks, and shared authenticated test fixtures. Session refresh lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant main.tsx
participant loadSession
participant RefreshDriver
participant AuthAPI
main.tsx->>loadSession: load initial session
loadSession-->>main.tsx: authenticated status and refreshAt
main.tsx->>RefreshDriver: startSessionRefresh()
RefreshDriver->>AuthAPI: POST /auth/refresh with CSRF token
AuthAPI-->>RefreshDriver: expires_at and refresh_at
RefreshDriver->>RefreshDriver: update store and schedule next refresh
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/auth/refresh.test.ts (1)
33-349: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCover request timeout aborts and visibility-change re-ticks in this test suite.
The test file does not exercise
AbortSignal.timeout(REQUEST_TIMEOUT_MS)or thevisibilitychangere-tick path. For request timeouts, Vitest fake timers do not mockAbortSignal.timeout, so the test should mock the signal itself/inject it or run with real timers/worker setup. Add assertions that a 10sfetchabort is retried instead of leaving the driver on the stale refresh path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/auth/refresh.test.ts` around lines 33 - 349, The session refresh driver tests need coverage for request-timeout retries and visibilitychange re-ticking. Extend the “session refresh driver” suite with a timeout case that controls or injects the request abort signal (since fake timers do not advance AbortSignal.timeout), verifies the 10-second fetch is aborted, and confirms the driver retries rather than remaining on the stale refresh schedule. Add a visibilitychange test that dispatches the event and asserts the refresh scheduling is re-ticked.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/auth/refresh.ts`:
- Around line 173-246: In the 403 branch of refresh(), when loadSession()
returns a non-authenticated result, broadcast the expired event before calling
expire(), matching the ordering in the 401 branch so other tabs receive the
session-death notification before the channel closes. Update the corresponding
refresh test to verify the expired fan-out.
In `@src/auth/session.test.ts`:
- Around line 28-29: Update the session test’s mock response and expectation
timestamps for expires_at and refresh_at to use non-expiring relative values or
shared far-future constants, ensuring the refresh driver does not schedule an
immediate refresh.
In `@src/auth/session.ts`:
- Around line 27-28: Validate expires_at and refresh_at at the session-ingestion
boundary before storing or using them; do not rely on the JSON cast. Accept
finite numeric timestamps, preserve 0 only when a field is missing, and reject
or disable the session when a supplied value is non-numeric or non-finite,
covering the assignments around the session field definitions and the related
lines 48-51.
---
Nitpick comments:
In `@src/auth/refresh.test.ts`:
- Around line 33-349: The session refresh driver tests need coverage for
request-timeout retries and visibilitychange re-ticking. Extend the “session
refresh driver” suite with a timeout case that controls or injects the request
abort signal (since fake timers do not advance AbortSignal.timeout), verifies
the 10-second fetch is aborted, and confirms the driver retries rather than
remaining on the stale refresh schedule. Add a visibilitychange test that
dispatches the event and asserts the refresh scheduling is re-ticked.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dbffcb89-1871-4f95-bea3-fefc9c9af108
📒 Files selected for processing (18)
src/api/fetch-with-auth.test.tssrc/api/use-catalog.test.tsxsrc/api/view-configs.test.tsxsrc/auth/index.tssrc/auth/refresh.test.tssrc/auth/refresh.tssrc/auth/session.test.tssrc/auth/session.tssrc/auth/types.tssrc/auth/use-auth.test.tssrc/auth/use-viewer.test.tssrc/components/auth-gate.test.tsxsrc/components/widgets/v2/counters-block.test.tsxsrc/components/widgets/v2/distribution-strip.test.tsxsrc/components/widgets/v2/section-card.test.tsxsrc/main.tsxsrc/mocks/handlers.tssrc/test/session.ts
…s (review) - The 403 -> dead-session path now broadcasts "expired" like the 401 path. stop() has already closed the BroadcastChannel by then, but broadcast() falls through to the localStorage transport, which every tab listens to regardless of its own transport - the earlier comment claiming fan-out was impossible was wrong. - loadSession() validates expires_at/refresh_at off the wire (finite positive number, else 0 = "never schedule"), matching the validation refresh() and onMessage() already do; the JSON cast is compile-time only. Signed-off-by: Anton Zelenov <antonz@constructor.tech>
2 participants
Closes constructorfabric/insight#1854 — the FE half of the nginx+auth session model (EPIC constructorfabric/insight#1583). Backend
POST /auth/refreshlanded in step 10 (insight#1593); without this driver every user is force-logged-out ~10 min after login because the session is deliberately non-sliding (PRD 5.3/5.4).What it does
SessiongainsexpiresAt/refreshAt(unix seconds), read fromGET /auth/meat boot (loadSession).src/auth/refresh.ts— the driver, started frommain.tsxonce boot resolves authenticated:POST /auth/refreshat the server-suppliedrefresh_at; on success the next timer is armed from the response's freshrefresh_at(never client-computed — the server re-jitters it every time).localStoragelease — stale leases are stolen,pagehidereleases early — and results fan out overBroadcastChannelwith alocalStoragestorage-event fallback. Occasional double-refresh races are tolerated by the backend's rotation grace + CAS.X-CSRF-Tokenon the POST (pairs with feat(auth): send X-CSRF-Token on state-changing /auth/* (nginx+auth step 10.5) #212), plus an abort timeout so a hung request can't wedge the driver/lease.401→ clear store, fan outexpired, bounce to/auth/login.429→ honorRetry-After.403→ re-prime csrf + schedule via/auth/mewith a 15 s retry floor (a persistent 403 cannot become a zero-delay loop), or expire if the re-probe fails closed. Transient errors retry every 15 s.visibilitychangere-evaluates on wake, so a refresh that came due during laptop sleep fires immediately and a dead leader is replaced without waiting out the heartbeat.MOCK_SESSIONnow carriescsrf_token—loadSession's fail-closed guard (from feat(auth): send X-CSRF-Token on state-changing /auth/* (nginx+auth step 10.5) #212) was rejecting mock sessions inVITE_ENABLE_MOCKSruns — plus timestamps and an/auth/refreshhandler.makeSession()factory, so the nextSessionfield is a one-line change instead of the ten-literal sweep that broke main after feat(auth): send X-CSRF-Token on state-changing /auth/* (nginx+auth step 10.5) #212 (fixed by test(auth): add csrfToken to Session fixtures (fix main build) #216).Known trade-offs (deliberate)
authenticatedbetween retries; the first round-trip that reaches the backend after expiry lands on the 401 → login path.Testing
refresh_atfails the suite), immediate overdue refresh, 401/429/403/network paths, 403 hot-loop floor, lease takeover, follower fan-out (both transports), malformed-message hardening, signOut mid-flight, stop-on-unauthenticated.pnpm test: 663 passed (main: 654).tsc -b, eslint clean on touched files (theeslint-plugin-local/distfailure pre-exists on main).EPIC: constructorfabric/insight#1583
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Enhancements
Tests