From cb1422f34f258a20a5216be5cebc2a0047ef345d Mon Sep 17 00:00:00 2001 From: Juan Denis Date: Mon, 26 Oct 2026 00:46:13 -0400 Subject: [PATCH] Log out only when the refresh token is rejected Any failed refresh (offline, 5xx, 429) cleared both tokens, and each expired request started its own refresh, so a burst of 401s could trip the refresh rate limit and log the tab out. Page load also wiped tokens on any /auth/me failure. Share one in-flight refresh, end the session only on a 401/422 from /auth/refresh or /auth/me, and let transient failures surface as errors with the tokens intact. --- frontend/src/contexts/AuthContext.jsx | 9 +-- .../services/api/__tests__/refresh.test.mjs | 71 +++++++++++++++++++ frontend/src/services/api/client.js | 45 +++++++----- 3 files changed, 104 insertions(+), 21 deletions(-) create mode 100644 frontend/src/services/api/__tests__/refresh.test.mjs diff --git a/frontend/src/contexts/AuthContext.jsx b/frontend/src/contexts/AuthContext.jsx index 7e71f98e..48b9ca65 100644 --- a/frontend/src/contexts/AuthContext.jsx +++ b/frontend/src/contexts/AuthContext.jsx @@ -47,10 +47,11 @@ export function AuthProvider({ children }) { const data = await api.getCurrentUser(); setUser(data.user); } catch (error) { - if (error.status !== 401) { - console.error('Auth check failed:', error); - } - api.clearTokens(); + // Only a 401 means the session is gone. A restarting backend + // (502, "database is locked") or a dropped connection must not + // delete a refresh token that is still good. + if (error.status === 401) api.clearTokens(); + else console.error('Auth check failed:', error); } } setLoading(false); diff --git a/frontend/src/services/api/__tests__/refresh.test.mjs b/frontend/src/services/api/__tests__/refresh.test.mjs new file mode 100644 index 00000000..898a6dc7 --- /dev/null +++ b/frontend/src/services/api/__tests__/refresh.test.mjs @@ -0,0 +1,71 @@ +import test, { beforeEach } from 'node:test'; +import assert from 'node:assert/strict'; +import ApiClient from '../client.js'; + +// A browser-ish world: localStorage, window events, and a scripted fetch. +const store = new Map(); +globalThis.localStorage = { + getItem: (k) => (store.has(k) ? store.get(k) : null), + setItem: (k, v) => store.set(k, String(v)), + removeItem: (k) => store.delete(k), +}; +const events = []; +globalThis.window ??= globalThis; +globalThis.window.dispatchEvent = (e) => events.push(e.type); + +const json = (status, body) => new Response(JSON.stringify(body), { + status, headers: { 'Content-Type': 'application/json' }, +}); +const accessKey = () => [...store.keys()].find((k) => k.endsWith('access_token')); +const refreshKey = () => [...store.keys()].find((k) => k.endsWith('refresh_token')); + +function client() { + const c = new ApiClient(); + c.setTokens('expired', 'refresh-ok'); + return c; +} + +beforeEach(() => { store.clear(); events.length = 0; }); + +test('parallel expired requests share one refresh', async () => { + const c = client(); + let refreshes = 0; + globalThis.fetch = async (url, init) => { + if (url.endsWith('/auth/refresh')) { + refreshes += 1; + await new Promise((r) => setTimeout(r, 10)); + return json(200, { access_token: 'fresh' }); + } + return init.headers.Authorization === 'Bearer fresh' + ? json(200, { ok: true }) + : json(401, { msg: 'Token has expired' }); + }; + const results = await Promise.all([1, 2, 3].map((n) => c._request(`/things/${n}`))); + assert.deepEqual(results, [{ ok: true }, { ok: true }, { ok: true }]); + assert.equal(refreshes, 1); +}); + +for (const failure of ['offline', 500, 429]) { + test(`a transient refresh failure (${failure}) keeps the session`, async () => { + const c = client(); + globalThis.fetch = async (url) => { + if (!url.endsWith('/auth/refresh')) return json(401, { msg: 'Token has expired' }); + if (failure === 'offline') throw new TypeError('Failed to fetch'); + return json(failure, { error: 'busy' }); + }; + await assert.rejects(c._request('/things')); + assert.equal(store.get(refreshKey()), 'refresh-ok'); + assert.deepEqual(events, []); + }); +} + +test('a rejected refresh token ends the session', async () => { + const c = client(); + globalThis.fetch = async (url) => (url.endsWith('/auth/refresh') + ? json(401, { msg: 'Token has been revoked' }) + : json(401, { msg: 'Token has expired' })); + await assert.rejects(c._request('/things'), (err) => err.status === 401); + assert.equal(accessKey(), undefined); + assert.equal(refreshKey(), undefined); + assert.deepEqual(events, ['serverkit:auth-expired']); +}); diff --git a/frontend/src/services/api/client.js b/frontend/src/services/api/client.js index eef60a5c..9b2c0f47 100644 --- a/frontend/src/services/api/client.js +++ b/frontend/src/services/api/client.js @@ -174,6 +174,8 @@ class ApiClient { const isJwtIssue = probe && probe.msg && !probe.error; if (isJwtIssue) { + // Throws on a transient refresh failure (offline, 5xx, 429): + // the refresh token is still good, so the session must survive. const refreshed = await this.refreshToken(); if (refreshed) { config.headers.Authorization = `Bearer ${this.getToken()}`; @@ -223,28 +225,37 @@ class ApiClient { return data; } - async refreshToken() { + // True once a new access token is stored; false only when the server + // rejected the refresh token (401/422), which is the one case that ends the + // session. Anything else throws and leaves the tokens alone. Concurrent + // callers share one in-flight refresh, so a burst of expired requests costs + // one POST /auth/refresh instead of one each (and can't trip its limit). + refreshToken() { + this._refreshing ??= this._refreshOnce().finally(() => { this._refreshing = null; }); + return this._refreshing; + } + + async _refreshOnce() { const refreshToken = localStorage.getItem(storageKey('refresh_token')); if (!refreshToken) return false; - try { - const response = await fetch(`${this.baseUrl}/auth/refresh`, { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - Authorization: `Bearer ${refreshToken}`, - }, - }); + const response = await fetch(`${this.baseUrl}/auth/refresh`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Authorization: `Bearer ${refreshToken}`, + }, + }); - if (response.ok) { - const data = await response.json(); - localStorage.setItem(storageKey('access_token'), data.access_token); - return true; - } - return false; - } catch { - return false; + if (response.ok) { + const data = await response.json(); + localStorage.setItem(storageKey('access_token'), data.access_token); + return true; } + if (response.status === 401 || response.status === 422) return false; + const err = new Error(`Could not refresh the session (${response.status})`); + err.status = response.status; + throw err; } }