V-3499 Implement session sync with JWT refresh on tab focus - #171
Merged
Conversation
A logged-in customer whose Store API JWT expired (1h) kept an authenticated-looking UI while every authenticated request silently failed: My Account rendered but order history and addresses were blank, and checkout showed a blank, non-interactable email field so no order could be placed. Recovery required a manual logout/login. Root cause was in the storefront's token lifecycle, not the SDK (a stateless client that only attaches the token it is handed): - "Authenticated" was inferred from cookie presence (7-day cookie) rather than JWT validity (1-hour token), so a dead session still looked logged in. isAuthenticated now treats an expired token as logged out. - Token refresh persists via cookie writes, which Next.js forbids during a Server Component render, so account/checkout page fetches could never renew the session and fell back to empty data. Refresh now runs only where cookies are writable and skips rotating a refresh token it cannot store, avoiding the rotation-loss that wedged the session. - The session is now transparently refreshed on the client (a Server Action on mount) and server components are re-rendered so their data reflects the renewed token; a genuinely expired session is treated as logged out. - The checkout email field is never left both disabled and blank, and falls back to the account email. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X9CjDU8j8MvtR2Wtxpobfa
The transparent session refresh only fired when the auth provider mounted (a fresh load or reload). A customer who left an already-open tab idle past the JWT lifetime and then kept navigating in-app never triggered a refresh, so pages could still render blank until a manual reload. Re-run the session sync when the tab regains visibility/focus — the exact "return to the tab" motion — so the expired JWT is renewed (or the session is treated as logged out) without a reload. Throttled to avoid re-syncing on ordinary focus churn. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X9CjDU8j8MvtR2Wtxpobfa
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughJWT freshness and cookie persistence handling now support session reconciliation. Auth context synchronization responds to initial loading, visibility, and focus events. Checkout email state uses the authenticated user email when needed and conditionally enables editing. ChangesSession synchronization
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant AuthProvider
participant syncSession
participant ensureFreshSession
participant CustomerAPI
Browser->>AuthProvider: mount or regain visibility
AuthProvider->>syncSession: reconcile session
syncSession->>ensureFreshSession: inspect or refresh tokens
ensureFreshSession-->>syncSession: session state
syncSession->>CustomerAPI: fetch customer when authenticated
CustomerAPI-->>syncSession: customer
syncSession-->>AuthProvider: customer and refreshed flag
AuthProvider-->>Browser: update auth state and refresh route when renewed
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/checkout/AddressSection.tsx (1)
277-287: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winFix input disabling mid-typing.
Because
To fix this and accurately match the intent of locking to the account email, derive the condition from the
userprop instead of the live input state.🐛 Proposed fix
// Lock to the account email only when we actually have one — never // leave the field both disabled and blank (a stale session would // otherwise make checkout impossible). - disabled={isAuthenticated && !!email} + disabled={isAuthenticated && !!user?.email} placeholder={t("emailAddress")} /> - {isAuthenticated && !!email && ( + {isAuthenticated && !!user?.email && ( <p className="text-xs text-gray-500 mt-1.5"> {t("usingAccountEmail")} </p>🤖 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/components/checkout/AddressSection.tsx` around lines 277 - 287, Update the email field’s disabled condition and account-email helper visibility in AddressSection to use the authenticated user’s account email from the user prop, not the live email state. Preserve the behavior of leaving the field enabled when the user lacks an account email, so typing does not disable it after the first character.src/lib/spree/auth-helpers.ts (1)
95-123: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftSerialize token refreshes before clearing cookies
src/lib/spree/auth-helpers.ts:104-121
tryRefresh()is reachable fromgetAuthOptions(),withAuthRefresh(), andensureFreshSession(). In writable requests, two concurrent refreshes can race on the same rotated refresh token: the first persists the new pair, the second fails on the stale token andclearAuthCookies()wipes the newly valid session. Use a single-flight guard per session/request so only one refresh runs at a time.🤖 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/lib/spree/auth-helpers.ts` around lines 95 - 123, The tryRefresh function can concurrently rotate the same refresh token, allowing a losing request to clear the newly refreshed cookies. Add a per-session or per-request single-flight guard around tryRefresh so concurrent callers await the same refresh operation, ensuring only one network refresh and cookie update occurs while preserving the existing success and failure behavior.src/lib/data/customer.ts (1)
65-101: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDon’t collapse transient customer fetch failures into a logged-out state
getCustomer()returnsnullfor any non-auth error, andAuthContext.refreshUser()turns that intosetUser(null)on mount/focus. A brief network/5xx failure can therefore clear a valid session in the UI; keep the existing user unless the error is a confirmed 401/403.🤖 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/lib/data/customer.ts` around lines 65 - 101, Update getCustomer so transient network or server failures are not converted to the same null result as an unauthenticated customer. Preserve null for confirmed 401/403 or missing tokens, but propagate or otherwise distinguish non-auth errors so AuthContext.refreshUser does not call setUser(null); keep the existing authenticated user on those failures.
🧹 Nitpick comments (3)
src/lib/spree/auth-helpers.ts (1)
125-132: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
clearAuthCookies()isn't exported, forcing a duplicate implementation incustomer.ts.See consolidated comment for the cross-file duplication with
src/lib/data/customer.ts'sclearAuthTokens().🤖 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/lib/spree/auth-helpers.ts` around lines 125 - 132, Export clearAuthCookies from auth-helpers.ts and update customer.ts to reuse this shared function instead of maintaining the duplicate clearAuthTokens implementation, preserving the existing best-effort cookie-clearing behavior.src/lib/data/__tests__/customer.test.ts (1)
128-177: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a test for
getCustomer()throwing a non-auth error after a successful refresh.The current suite covers
ensureFreshSession()'s four states well, but doesn't exercise the case wherestateis"valid"/"refreshed"and the subsequentgetClient().customer.get(...)call throws a non-401/403 error. That's the scenario behind the reliability concern raised insrc/lib/data/customer.ts(transient failure surfacing as a false logout); a regression test here would guard any future fix.🤖 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/lib/data/__tests__/customer.test.ts` around lines 128 - 177, Extend the syncSession tests to cover a non-authentication error from mockClient.customer.get after ensureFreshSession returns both "valid" and "refreshed" (or the relevant shared path). Assert the error is propagated or handled according to syncSession’s intended behavior, and verify the result does not incorrectly indicate an expired or anonymous session.src/lib/data/customer.ts (1)
22-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicates
clearAuthCookies()fromauth-helpers.ts; also missing an explicit return type.This best-effort clear logic is identical to the private
clearAuthCookies()insrc/lib/spree/auth-helpers.ts— see consolidated comment for the cross-file fix. Separately,clearAuthTokens()is declared without an explicit return type (Promise<void>), unlike the other functions touched in this diff.As per coding guidelines, "Use strict TypeScript type checking. Always define explicit return types for functions... and avoid 'any' (use 'unknown' instead)."
♻️ Proposed fix
-async function clearAuthTokens() { +async function clearAuthTokens(): Promise<void> { try { await clearAccessToken(); await clearRefreshToken(); } catch { // Best-effort — not writable during a Server Component render. } }🤖 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/lib/data/customer.ts` around lines 22 - 34, Remove the duplicated clearAuthTokens implementation and reuse the existing clearAuthCookies helper from auth-helpers.ts for best-effort token clearing. Ensure the affected function has an explicit Promise<void> return type, preserving the current behavior in read-only contexts.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@src/components/checkout/AddressSection.tsx`:
- Around line 277-287: Update the email field’s disabled condition and
account-email helper visibility in AddressSection to use the authenticated
user’s account email from the user prop, not the live email state. Preserve the
behavior of leaving the field enabled when the user lacks an account email, so
typing does not disable it after the first character.
In `@src/lib/data/customer.ts`:
- Around line 65-101: Update getCustomer so transient network or server failures
are not converted to the same null result as an unauthenticated customer.
Preserve null for confirmed 401/403 or missing tokens, but propagate or
otherwise distinguish non-auth errors so AuthContext.refreshUser does not call
setUser(null); keep the existing authenticated user on those failures.
In `@src/lib/spree/auth-helpers.ts`:
- Around line 95-123: The tryRefresh function can concurrently rotate the same
refresh token, allowing a losing request to clear the newly refreshed cookies.
Add a per-session or per-request single-flight guard around tryRefresh so
concurrent callers await the same refresh operation, ensuring only one network
refresh and cookie update occurs while preserving the existing success and
failure behavior.
---
Nitpick comments:
In `@src/lib/data/__tests__/customer.test.ts`:
- Around line 128-177: Extend the syncSession tests to cover a
non-authentication error from mockClient.customer.get after ensureFreshSession
returns both "valid" and "refreshed" (or the relevant shared path). Assert the
error is propagated or handled according to syncSession’s intended behavior, and
verify the result does not incorrectly indicate an expired or anonymous session.
In `@src/lib/data/customer.ts`:
- Around line 22-34: Remove the duplicated clearAuthTokens implementation and
reuse the existing clearAuthCookies helper from auth-helpers.ts for best-effort
token clearing. Ensure the affected function has an explicit Promise<void>
return type, preserving the current behavior in read-only contexts.
In `@src/lib/spree/auth-helpers.ts`:
- Around line 125-132: Export clearAuthCookies from auth-helpers.ts and update
customer.ts to reuse this shared function instead of maintaining the duplicate
clearAuthTokens implementation, preserving the existing best-effort
cookie-clearing behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 83e638b7-1a82-4cb7-b778-8959d6bbaa00
📒 Files selected for processing (11)
src/components/checkout/AddressSection.tsxsrc/contexts/AuthContext.tsxsrc/contexts/__tests__/AuthContext.test.tsxsrc/lib/data/__tests__/customer.test.tssrc/lib/data/cookies.tssrc/lib/data/customer.tssrc/lib/spree/__tests__/jwt.test.tssrc/lib/spree/auth-helpers.tssrc/lib/spree/cookies.tssrc/lib/spree/index.tssrc/lib/spree/jwt.ts
damianlegawiec
changed the title
damianlegawiec
pushed a commit
that referenced
this pull request
Jul 14, 2026
Follow-up to the JWT session-sync work (#171), addressing review findings: - Checkout email input is keyed off the account email rather than the live input value, so an authenticated user without an email on file can still type instead of being disabled after the first keystroke. - Transient customer-fetch failures no longer collapse a valid session to logged-out: syncSession flags them as `stale` so the client keeps the current user, clearing only on a confirmed 401/403. - Concurrent token refreshes sharing the same rotated refresh token are coalesced into a single in-flight request, so a losing race can no longer wipe the freshly persisted session. - Deduplicate best-effort cookie clearing behind an exported clearAuthCookies helper. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X9CjDU8j8MvtR2Wtxpobfa
damianlegawiec
added a commit
that referenced
this pull request
Jul 15, 2026
* Harden session sync against false logouts and refresh races Follow-up to the JWT session-sync work (#171), addressing review findings: - Checkout email input is keyed off the account email rather than the live input value, so an authenticated user without an email on file can still type instead of being disabled after the first keystroke. - Transient customer-fetch failures no longer collapse a valid session to logged-out: syncSession flags them as `stale` so the client keeps the current user, clearing only on a confirmed 401/403. - Concurrent token refreshes sharing the same rotated refresh token are coalesced into a single in-flight request, so a losing race can no longer wipe the freshly persisted session. - Deduplicate best-effort cookie clearing behind an exported clearAuthCookies helper. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X9CjDU8j8MvtR2Wtxpobfa * Harden token refresh against transient failures The session-sync work made the customer-fetch path resilient to transient errors but left the token-refresh path clearing auth cookies on any failure, so a network or 5xx blip during refresh logged the user out — the exact failure this change set out to prevent. Only drop the session on a confirmed-invalid refresh token; treat other refresh failures as stale and preserve the session. Carry the rotation signal through a stale follow-up fetch so server components still re-render under the renewed session. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X9CjDU8j8MvtR2Wtxpobfa * Clarify email-lock guard comment in checkout AddressSection The comment justified the `!!user?.email` guard with an impossible state (a saved customer always has an email). The guard actually protects the window where the server-derived `isAuthenticated` is true while the AuthContext `user` is still null — during hydration or after a transient sync preserved as stale — where keying off `isAuthenticated` alone would leave the email field disabled and blank, blocking checkout. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X9CjDU8j8MvtR2Wtxpobfa --------- Co-authored-by: Claude <noreply@anthropic.com>
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
2 participants
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
Adds robust session management to the authentication system by implementing JWT expiry detection, transparent token refresh, and session re-sync when the browser tab regains focus. This ensures users remain authenticated across tab switches and prevents stale sessions from blocking checkout.
Key Changes
JWT utilities (
src/lib/spree/jwt.ts): New module for decoding JWTexpclaims and checking expiry with configurable skew windows, without signature verification.Session refresh logic (
src/lib/spree/auth-helpers.ts):isJwtExpired()helperensureFreshSession()Server Action to refresh expired JWTs and return session state (valid,refreshed,expired,anonymous)tryRefresh()to skip token rotation when cookies can't be persisted (Server Component context), deferring to client-side re-synccanPersistCookies()probe to detect writable execution contextsSession sync endpoint (
src/lib/data/customer.ts):syncSession()Server Action that reconciles client session state: refreshes expired JWTs and returns the current customer plus arefreshedflagclearAuthTokens()helper with best-effort error handlingAuthContext enhancements (
src/contexts/AuthContext.tsx):syncSession()for initial auth state and transparent JWT refreshvisibilitychangeandfocusevents to refresh session when tab regains visibilityrouter.refresh()after transparent token refresh to re-render server components with renewed sessionCookie authentication (
src/lib/data/cookies.ts):isAuthenticated()to check JWT expiry (30-second skew) in addition to token presence, preventing stale tokens from appearing authenticatedCheckout UX fix (
src/components/checkout/AddressSection.tsx):Comprehensive test coverage:
AuthContext.test.tsx: Tests session sync on mount, focus-driven re-sync with throttling, transparent refresh triggering server re-render, and session expiry handlingjwt.test.ts: Tests JWT decoding, expiry detection with skew windows, and malformed token handlingcustomer.test.ts: TestssyncSession()behavior across all session statesImplementation Details
router.refresh()to ensure server components re-fetch data with the new sessionhttps://claude.ai/code/session_01X9CjDU8j8MvtR2Wtxpobfa
Summary by CodeRabbit
Bug Fixes
Tests