Persist OAuth login across tab close (T426515)

Daanvrrequested to merge
feat/T426515-persistent-login into main

Summary

  • Move OAuth access token + expiry from sessionStorage to localStorage so the login survives a tab/browser close. The long-lived refresh token (~365 days) already lived in localStorage; the storage choice was inconsistent.
  • isAuthenticated() now treats "refresh token present in localStorage" as authenticated, so <AuthGate> renders <Bootstrap/> on cold start and the first fetchUserProfile() call transparently exchanges the refresh token for a fresh access token.
  • One-time migration on module load: any leftover sessionStorage values from older builds are moved into localStorage and the legacy slots cleared. Existing logged-in users don't get bumped to the login screen on the first visit after this lands.
  • <Bootstrap> now detects "refresh actually failed" (token revoked / expired) and routes to <Login/> instead of showing a generic boot-error panel.

Phabricator

Root cause

The task description said "the login is now lost on closing the page" and asked for IndexedDB. Investigation showed the symptom was not a missing-persistence problem — localStorage (which already held the refresh token) persists across tab close fine. The actual bug:

src/api/oauth.js:58-66 (storeTokens)
  sessionStorage.setItem(KEYS.accessToken, …)   // <- wipes on tab close
  sessionStorage.setItem(KEYS.tokenExpires, …)  // <- wipes on tab close
  if (tokenData.refresh_token) {
    localStorage.setItem(KEYS.refreshToken, …)  // <- persists, but isAuthenticated() never checks this
  }

src/api/oauth.js:204-210 (isAuthenticated, synchronous)
  const token = sessionStorage.getItem(KEYS.accessToken);   // <- gone after tab close → returns false
  const expires = sessionStorage.getItem(KEYS.tokenExpires);
  return Boolean(token && expires && Date.now() < Number(expires));

On the next visit, <AuthGate> calls isAuthenticated() synchronously, sees no access token, and renders <Login/> — never giving the refresh-token path in getAccessToken() a chance to run. The refresh token sitting in localStorage was effectively dead weight.

Storage choice — security analysis (the maintainer asked for "better more secure ways if they exist")

For OAuth bearer tokens in a single-page app with no backend, the comparison is:

Storage Persists across tab close XSS readable Notes
localStorage yes yes Same-origin readable by any script.
sessionStorage no yes Same XSS surface as localStorage, but loses login on tab close. Worse than localStorage for this use case.
IndexedDB yes yes Same XSS surface — any same-origin script can indexedDB.open(...) and read. Not meaningfully more secure than localStorage, only slightly higher friction for naive attackers.
httpOnly cookies yes no Genuinely more secure, but requires a backend to set. We have no backend (PKCE public client). Out of scope.
WebCrypto + IndexedDB w/ derived key yes yes (in memory) Defeats the seamless-login UX the maintainer wants.

Honest answer to the task's "better more secure ways" question: for this threat model (browser-only PKCE client, no backend), localStorage ↔️ IndexedDB is a wash. The persistence symptom was the bug; the storage backend doesn't need to change. Filed in the task comment when this moves to Reviewing so future me / future Claude doesn't redo the analysis.

Why not IndexedDB

The maintainer specifically suggested IndexedDB. We considered it and decided against:

  • No security improvement (same XSS surface, same origin scope).
  • Async API — would require either polluting <AuthGate>'s synchronous initial-state hook with a Suspense boundary, or duplicating an in-memory cache that re-reads from IndexedDB on bootstrap. Both add code without fixing the bug.
  • Bigger surgery, more places to regress.

Sticking to localStorage is the minimal correct fix.

Files touched

  • src/api/oauth.js — storeTokens / clearTokens / getAccessToken / isAuthenticated switched to localStorage; added migrateSessionToLocal() one-time migration on module load. PKCE per-redirect state (codeVerifier, oauthState) intentionally left in sessionStorage.
  • src/main.jsx — <Bootstrap> now distinguishes "transient profile-fetch failure" from "refresh-token rejected" and calls logout() (reload → login screen) in the latter case instead of showing a boot-error panel.
  • CHANGELOG.md — [Unreleased] Fixed entry.

Relationship to T426435

T426435 (Backlog) asks for a 30s expiry margin on the validity check to avoid a separate race condition. It's orthogonal to this fix (the expiry timestamp itself is correct; the issue here is storage, not arithmetic). Picking that up is a separate task.

Test plan

  • Log in fresh via the preview URL. Confirm localStorage has uwb_access_token, uwb_token_expires, uwb_refresh_token (DevTools → Application → Local Storage).
  • Confirm sessionStorage does not have those keys (only uwb_code_verifier / uwb_oauth_state may appear briefly during the redirect dance and clear afterwards).
  • Close the tab. Reopen the preview URL. Confirm you're still logged in — no login screen, items load.
  • Close the whole browser. Reopen. Confirm still logged in.
  • Migration test: in DevTools manually copy the localStorage uwb_access_token value into sessionStorage under the same key, remove it from localStorage, then reload. Confirm the migration moves it back to localStorage (the user stays logged in).
  • Simulated refresh-token rejection: in DevTools, set localStorage.uwb_refresh_token = 'bogus' and clear uwb_access_token + uwb_token_expires. Reload. Confirm <Login/> appears (not a boot-error panel).

CLI test note: real "close tab and reopen 4 hours later" cannot be exercised from CLI. Manual browser test on the MR preview is required.

🤖 Generated with Claude Code

Merge request reports