From 71730cbd6bbcb66bc0e7fca1298e1ee204c97161 Mon Sep 17 00:00:00 2001 From: jR4dh3y Date: Thu, 15 Oct 2026 11:41:42 +0530 Subject: [PATCH] Fix sign-outs caused by overlapping session refreshes --- .github/workflows/ci.yml | 4 + backend/internal/handler/auth_test.go | 49 +++++++- backend/internal/handler/errors.go | 1 + backend/internal/model/error.go | 1 + backend/internal/service/auth.go | 3 +- .../internal/service/auth_sessions_test.go | 16 +-- backend/internal/service/auth_test.go | 10 +- docs/api.md | 2 + docs/development.md | 4 + docs/security.md | 2 +- docs/troubleshooting.md | 2 + frontend/scripts/auth-session-browser.ts | 111 ++++++++++++++++++ frontend/src/lib/api/client.test.ts | 37 ++++++ frontend/src/lib/api/client.ts | 39 +++--- frontend/src/lib/api/earlyStart.test.ts | 9 ++ scripts/auth-smoke/http.ts | 22 ++++ scripts/auth-smoke/scenarios/sessions.ts | 18 +-- 17 files changed, 285 insertions(+), 45 deletions(-) create mode 100644 frontend/scripts/auth-session-browser.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7e177bd..ac77a0f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -108,6 +108,10 @@ jobs: working-directory: frontend run: bunx playwright install --with-deps chromium + - name: Check shared-cookie session recovery + working-directory: frontend + run: bun scripts/auth-session-browser.ts + - name: Guard real browser cold and warm folder loads working-directory: frontend env: diff --git a/backend/internal/handler/auth_test.go b/backend/internal/handler/auth_test.go index 953823c..34c1f09 100644 --- a/backend/internal/handler/auth_test.go +++ b/backend/internal/handler/auth_test.go @@ -279,8 +279,8 @@ func TestAuthPersistenceFailuresPreserveOrClearCookieByOutcome(t *testing.T) { if rotated.Value == cookie.Value { t.Fatal("successful retry did not rotate refresh cookie") } - if _, err := auth.ValidateRefreshToken(cookie.Value); !errors.Is(err, service.ErrTokenRevoked) { - t.Fatalf("previous refresh token after retry = %v, want revoked", err) + if _, err := auth.ValidateRefreshToken(cookie.Value); !errors.Is(err, service.ErrRefreshSuperseded) { + t.Fatalf("previous refresh token after retry = %v, want superseded", err) } }) @@ -316,8 +316,8 @@ func TestAuthPersistenceFailuresPreserveOrClearCookieByOutcome(t *testing.T) { if err != nil { t.Fatalf("restart auth service: %v", err) } - if _, err := restarted.ValidateRefreshToken(cookie.Value); !errors.Is(err, service.ErrTokenRevoked) { - t.Fatalf("published refresh rotation after restart = %v, want revoked", err) + if _, err := restarted.ValidateRefreshToken(cookie.Value); !errors.Is(err, service.ErrRefreshSuperseded) { + t.Fatalf("published refresh rotation after restart = %v, want superseded", err) } }) @@ -541,3 +541,44 @@ func onlyCookie(t *testing.T, response *httptest.ResponseRecorder) *http.Cookie } return cookies[0] } + +func TestSupersededRefreshAndLogoutPreserveSharedCookie(t *testing.T) { + auth, handler := newAuthHandler(t, filesystem.NewOsFS(), t.TempDir()) + original, err := auth.Login(context.Background(), "admin", "correct-password") + if err != nil { + t.Fatal(err) + } + replacement, err := auth.Refresh(context.Background(), original.RefreshToken) + if err != nil { + t.Fatal(err) + } + for _, action := range []struct { + name string + serve http.HandlerFunc + }{ + {"refresh", handler.Refresh}, {"logout", handler.Logout}, + } { + t.Run(action.name, func(t *testing.T) { + request := httptest.NewRequest(http.MethodPost, "http://boxbox.example/api/v1/auth/"+action.name, nil) + request.AddCookie(&http.Cookie{Name: refreshCookieName, Value: original.RefreshToken}) + response := httptest.NewRecorder() + action.serve(response, request) + if response.Code != http.StatusConflict { + t.Fatalf("status = %d, want 409", response.Code) + } + if response.Header().Get("Set-Cookie") != "" { + t.Fatal("stale request overwrote the shared cookie") + } + var body model.ErrorResponse + if err := json.Unmarshal(response.Body.Bytes(), &body); err != nil { + t.Fatal(err) + } + if body.Code != model.ErrCodeRefreshSuperseded { + t.Fatalf("code = %s", body.Code) + } + if _, err := auth.ValidateRefreshToken(replacement.RefreshToken); err != nil { + t.Fatal(err) + } + }) + } +} diff --git a/backend/internal/handler/errors.go b/backend/internal/handler/errors.go index 3c81b8b..8a057db 100644 --- a/backend/internal/handler/errors.go +++ b/backend/internal/handler/errors.go @@ -55,6 +55,7 @@ var serviceErrorMappings = []ErrorMapping{ {service.ErrInvalidWallpaper, "Wallpaper must be a JPEG, PNG, GIF, or WebP image up to 20 MB with a valid display mode", model.ErrCodeValidationError, http.StatusBadRequest}, // Auth service errors + {service.ErrRefreshSuperseded, "Another request refreshed this session. Retry with the current cookie.", model.ErrCodeRefreshSuperseded, http.StatusConflict}, {service.ErrInvalidCredentials, "Invalid credentials", model.ErrCodeUnauthorized, http.StatusUnauthorized}, {service.ErrInvalidToken, "Invalid token", model.ErrCodeTokenInvalid, http.StatusUnauthorized}, {service.ErrTokenExpired, "Token has expired", model.ErrCodeTokenInvalid, http.StatusUnauthorized}, diff --git a/backend/internal/model/error.go b/backend/internal/model/error.go index aa13786..5c8d05f 100644 --- a/backend/internal/model/error.go +++ b/backend/internal/model/error.go @@ -15,6 +15,7 @@ const ( ErrCodeInvalidPath = "INVALID_PATH" ErrCodeUnauthorized = "UNAUTHORIZED" ErrCodeTokenInvalid = "TOKEN_INVALID" + ErrCodeRefreshSuperseded = "REFRESH_SUPERSEDED" ErrCodePermissionDenied = "PERMISSION_DENIED" ErrCodeConflict = "CONFLICT" ErrCodeValidationError = "VALIDATION_ERROR" diff --git a/backend/internal/service/auth.go b/backend/internal/service/auth.go index cf9cac2..3cfb9ad 100644 --- a/backend/internal/service/auth.go +++ b/backend/internal/service/auth.go @@ -25,6 +25,7 @@ var ( ErrInvalidToken = errors.New("invalid token") ErrTokenExpired = errors.New("token expired") ErrTokenRevoked = errors.New("token revoked") + ErrRefreshSuperseded = errors.New("refresh token superseded") ErrSessionRecoveryRequired = errors.New("session recovery required") ) @@ -332,7 +333,7 @@ func (s *authService) validateSessionLocked(claims *Claims, tokenType TokenType, return ErrInvalidToken } if tokenType == TokenTypeRefresh && record.RefreshHash != tokenHash { - return ErrTokenRevoked + return ErrRefreshSuperseded } return nil } diff --git a/backend/internal/service/auth_sessions_test.go b/backend/internal/service/auth_sessions_test.go index 5469ef9..4d8fb91 100644 --- a/backend/internal/service/auth_sessions_test.go +++ b/backend/internal/service/auth_sessions_test.go @@ -89,8 +89,8 @@ func TestRefreshRotationPersistsCurrentTokenAndStableSession(t *testing.T) { } second := newTestAuthServiceAt(t, dataDir) - if _, err := second.ValidateRefreshToken(original.RefreshToken); !errors.Is(err, ErrTokenRevoked) { - t.Fatalf("old refresh after restart = %v, want rejected", err) + if _, err := second.ValidateRefreshToken(original.RefreshToken); !errors.Is(err, ErrRefreshSuperseded) { + t.Fatalf("old refresh after restart = %v, want superseded", err) } if _, err := second.ValidateRefreshToken(replacement.RefreshToken); err != nil { t.Fatalf("current refresh after restart: %v", err) @@ -278,15 +278,15 @@ func TestConcurrentRefreshHasOneWinnerAndReplayCannotKillWinner(t *testing.T) { winner = current.pair continue } - if !errors.Is(current.err, ErrTokenRevoked) { + if !errors.Is(current.err, ErrRefreshSuperseded) { t.Fatalf("concurrent refresh error = %v, want stale refresh rejection", current.err) } } if winner == nil { t.Fatal("concurrent refresh produced no winner") } - if _, err := service.Refresh(context.Background(), original.RefreshToken); !errors.Is(err, ErrTokenRevoked) { - t.Fatalf("replay after winning rotation = %v, want rejected", err) + if _, err := service.Refresh(context.Background(), original.RefreshToken); !errors.Is(err, ErrRefreshSuperseded) { + t.Fatalf("replay after winning rotation = %v, want superseded", err) } for _, token := range []string{winner.AccessToken, winner.RefreshToken} { if _, err := service.ValidateToken(token); err != nil { @@ -534,15 +534,15 @@ func exerciseSessionPersistenceFailure(t *testing.T, stage authPersistenceFailur if operation == "refresh" { if published { for _, currentService := range []*authService{service, restarted} { - if _, err := currentService.ValidateRefreshToken(current.RefreshToken); !errors.Is(err, ErrTokenRevoked) { + if _, err := currentService.ValidateRefreshToken(current.RefreshToken); !errors.Is(err, ErrRefreshSuperseded) { t.Fatalf("old refresh after published failure = %v, want rotated", err) } if _, err := currentService.ValidateAccessToken(current.AccessToken); err != nil { t.Fatalf("active session lost access after published refresh failure: %v", err) } } - if _, err := service.Refresh(context.Background(), current.RefreshToken); !errors.Is(err, ErrTokenRevoked) { - t.Fatalf("replay after published refresh failure = %v, want rejected", err) + if _, err := service.Refresh(context.Background(), current.RefreshToken); !errors.Is(err, ErrRefreshSuperseded) { + t.Fatalf("replay after published refresh failure = %v, want superseded", err) } } else { for _, currentService := range []*authService{service, restarted} { diff --git a/backend/internal/service/auth_test.go b/backend/internal/service/auth_test.go index 8ade104..69b5b6b 100644 --- a/backend/internal/service/auth_test.go +++ b/backend/internal/service/auth_test.go @@ -77,10 +77,10 @@ func TestRefreshKeepsSessionAndOldAccessUntilLogout(t *testing.T) { if oldAccess.SessionID != newAccess.SessionID || newAccess.SessionID != newRefresh.SessionID { t.Fatal("refresh rotation changed the active session ID") } - if _, err := svc.ValidateRefreshToken(original.RefreshToken); !errors.Is(err, ErrTokenRevoked) { - t.Fatalf("rotated refresh token validation = %v, want rejected replay", err) + if _, err := svc.ValidateRefreshToken(original.RefreshToken); !errors.Is(err, ErrRefreshSuperseded) { + t.Fatalf("rotated refresh token validation = %v, want superseded replay", err) } - if _, err := svc.ValidateToken(original.RefreshToken); !errors.Is(err, ErrTokenRevoked) { + if _, err := svc.ValidateToken(original.RefreshToken); !errors.Is(err, ErrRefreshSuperseded) { t.Fatalf("generic validation admitted a rotated refresh token: %v", err) } } @@ -100,8 +100,8 @@ func TestLogoutIsSessionWideAndReplayCannotTerminateWinner(t *testing.T) { t.Fatalf("refresh: %v", err) } - if err := svc.Logout(context.Background(), original.RefreshToken); !errors.Is(err, ErrTokenRevoked) { - t.Fatalf("replayed logout error = %v, want rejected stale refresh", err) + if err := svc.Logout(context.Background(), original.RefreshToken); !errors.Is(err, ErrRefreshSuperseded) { + t.Fatalf("replayed logout error = %v, want superseded stale refresh", err) } for _, token := range []string{winner.AccessToken, winner.RefreshToken} { if _, err := svc.ValidateToken(token); err != nil { diff --git a/docs/api.md b/docs/api.md index 4df9d4d..ab3fae2 100644 --- a/docs/api.md +++ b/docs/api.md @@ -62,6 +62,8 @@ POST /api/v1/auth/logout The request has no body. Logout needs a valid current refresh cookie. It does not need an access-token header. Logout removes the entire session, so all access tokens issued by that session stop working, including tokens from earlier refresh rotations. Other logins for the same account remain active. A missing or invalid refresh cookie returns `401 TOKEN_INVALID` and clears the cookie. +A still-unexpired refresh token that another request has replaced returns `409 REFRESH_SUPERSEDED` from refresh or logout. The response contains no token pair and no `Set-Cookie` header, preserving the replacement cookie shared by browser tabs. Retry with the current cookie. A repeated conflict is retryable, not proof that the session has ended. + Login, refresh, and logout save session state before reporting success. Publication occurs at the file rename. Success requires syncing and closing the file, renaming it, and syncing the parent directory. Before rename, a failed write returns `500 INTERNAL_ERROR` without publishing a token pair or a `Set-Cookie` header. Refresh and logout preserve the existing cookie so the client can retry. Login does not clear an unrelated cookie. If a durability step fails after rename, the server keeps the installed in-memory state and returns `401 SESSION_RECOVERY_REQUIRED` without a token pair. Refresh and logout clear the cookie. Login sends no new cookie and does not clear an unrelated existing cookie. Sign in again after this error. Logout did not report success. diff --git a/docs/development.md b/docs/development.md index c072fc6..d9debcb 100644 --- a/docs/development.md +++ b/docs/development.md @@ -172,6 +172,10 @@ Omit `--base` for a browser-only candidate check. In stacked PRs CI compares aga CI uploads raw samples, benchmark summaries, browser byte/request counts and failure diagnostics as 14-day artifacts, even when a gate fails. Local results are ignored under `perf-results/`. No automatic review requests, budget resets or baseline commits are performed. +## Check concurrent browser refresh + +From `frontend/`, run `bun scripts/auth-session-browser.ts` with Playwright Chromium installed. This builds the real API client and Go server, sends overlapping refreshes from two pages sharing one cookie jar, and delivers the stale response after the replacement cookie. Both pages must recover without losing their cookie. It also verifies that explicit logout still ends the other page's session. + ## Check active-session authentication On Linux, run the production-mode auth smoke check from the repository root: diff --git a/docs/security.md b/docs/security.md index c055206..4c56af4 100644 --- a/docs/security.md +++ b/docs/security.md @@ -25,7 +25,7 @@ Before exposing BoxBox beyond your own machine: - A write failure before rename returns `500`. The service restores only the session changed by that request. Login, refresh, and logout do not publish a token pair or `Set-Cookie`; refresh and logout preserve the existing cookie for retry. Login does not clear an unrelated existing cookie. - Rename publishes the new registry. Success also requires the temporary file to be synced and closed and the parent directory to be synced. If a durability step fails after rename, the service keeps the installed in-memory state and returns `401 SESSION_RECOVERY_REQUIRED` without a token pair. Refresh and logout clear the cookie. Login does not clear an unrelated cookie. - Keep `data_dir` on persistent storage and run one server process per directory. The service does not coordinate processes that share a directory. Deleting the registry while the server runs does not clear its in-memory sessions, and a later successful write can persist them again. This is not immediate disk-loss revalidation. -- Browser refresh coalescing applies within one tab only. Concurrent refreshes in different tabs share the cookie but are not coordinated; a losing request can clear the replacement cookie and require another sign-in. +- Browser refresh coalescing applies within one tab. A replaced refresh token for an active session returns `409 REFRESH_SUPERSEDED` without changing the shared cookie or issuing tokens. Refresh and logout retry once with the browser's current cookie. Repeated conflicts remain retryable and do not end the session. Expired tokens and removed sessions still return `401`. - `auth-revocations.json` is ignored and is not deleted. Its denylist cannot identify sessions to load. Legacy tokens without a session ID are rejected on first use and never auto-enrolled; users sign in again. A missing registry file or valid empty registry after restart rejects old tokens without requiring JWT-secret rotation solely because the registry was lost. - Restoring an older valid `auth-sessions.json` backup can revive sessions, access tokens, and their current refresh tokens. Rotate `BOXBOX_JWT_SECRET` after restoring a backup. An older validator that ignores session IDs and the registry may accept tokens after downgrade; rotate the secret if you downgrade. - After startup with a user removed from configuration, that user's tokens fail validation. Session deadlines preserve the existing sliding refresh lifetime and retain the longest existing deadline when configured token TTLs shrink. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index d12f5f0..c0fe358 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -18,6 +18,8 @@ docker compose up -d ## Login, Logout, or Refresh Fails +`409 REFRESH_SUPERSEDED` means another request replaced the refresh cookie. The browser retries once automatically. If another refresh is still in flight, retry the action; this response does not clear the cookie or end the session. + If login, refresh, or logout returns `500` before rename, the server did not publish the session update. It sends no new token pair or `Set-Cookie` header. Refresh and logout leave the existing cookie unchanged so you can retry. Login does not clear an unrelated existing cookie. Check that `data_dir` is writable and on persistent storage, then retry. If the browser cannot restore a session, it preserves the refresh cookie on retryable errors and shows **Retry session**. Repair storage or connectivity, then use that button without reloading. While restoration is pending, the retry and sign-in controls are disabled. An expired or invalid cookie requires a fresh sign-in; the stale retry warning is removed. diff --git a/frontend/scripts/auth-session-browser.ts b/frontend/scripts/auth-session-browser.ts new file mode 100644 index 0000000..391bd7d --- /dev/null +++ b/frontend/scripts/auth-session-browser.ts @@ -0,0 +1,111 @@ +import assert from 'node:assert/strict'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { join, resolve } from 'node:path'; +import { tmpdir } from 'node:os'; +import { chromium, type Page, type Route } from 'playwright'; +import { type Subprocess } from 'bun'; +import { createFixture } from '../../scripts/auth-smoke/fixtures'; +import { + buildBinary, + reservePort, + startServer, + killAndWait +} from '../../scripts/auth-smoke/process'; +import { clearCredentials } from '../../scripts/auth-smoke/credentials'; + +type Client = Pick< + typeof import('../src/lib/api/client'), + 'refreshSession' | 'getAccessToken' | 'logoutSession' +>; +const repo = resolve(import.meta.dir, '../..'); +const root = mkdtempSync(join(tmpdir(), 'boxbox-session-browser-')); +const processes: Subprocess[] = []; +const browser = await chromium.launch({ headless: true }); +const moduleUrl = '/auth-test-client.js'; + +async function refresh(page: Page) { + return page.evaluate(async (url) => { + const client: Client = await import(url); + await client.refreshSession(); + return Boolean(client.getAccessToken()); + }, moduleUrl); +} + +try { + const port = await reservePort(); + const fixture = await createFixture(root, port); + const binary = await buildBinary(root, join(repo, 'backend'), processes); + await startServer( + binary, + fixture.configPath, + join(repo, 'backend'), + port, + processes, + 'browser auth' + ); + const bundle = await Bun.build({ + entrypoints: [join(repo, 'frontend/src/lib/api/client.ts')], + target: 'browser' + }); + assert.ok(bundle.success, 'API client bundle must compile'); + const source = await bundle.outputs[0].text(); + const context = await browser.newContext(); + await context.route('**' + moduleUrl, (route) => + route.fulfill({ contentType: 'text/javascript', body: source }) + ); + const first = await context.newPage(); + const second = await context.newPage(); + await Promise.all([ + first.goto(fixture.baseUrl + '/health'), + second.goto(fixture.baseUrl + '/health') + ]); + const login = await context.request.post(fixture.baseUrl + '/api/v1/auth/login', { + data: { username: 'auth-smoke-user', password: fixture.password } + }); + assert.equal(login.status(), 200); + assert.equal(await refresh(first), true); + assert.equal(await refresh(second), true); + + // Both requests leave with the same cookie; deliver the stale rejection after the new cookie. + const waiting: Route[] = []; + const statuses: number[] = []; + await context.route('**/api/v1/auth/refresh', async (route) => { + waiting.push(route); + if (waiting.length !== 2) return; + const replies = await Promise.all( + waiting.map(async (held) => ({ held, response: await held.fetch() })) + ); + replies.sort((a, b) => a.response.status() - b.response.status()); + await context.unroute('**/api/v1/auth/refresh'); + for (const { held, response } of replies) { + statuses.push(response.status()); + if (response.status() === 409) assert.equal(response.headers()['set-cookie'], undefined); + await held.fulfill({ response }); + } + }); + assert.deepEqual(await Promise.all([refresh(first), refresh(second)]), [true, true]); + assert.deepEqual(statuses, [200, 409]); + assert.ok((await context.cookies()).some((cookie) => cookie.name === 'boxbox_refresh')); + assert.equal(await refresh(first), true); + assert.equal(await refresh(second), true); + await first.evaluate(async (url) => { + const client: Client = await import(url); + await client.logoutSession(); + }, moduleUrl); + await assert.rejects(refresh(second)); + assert.equal( + await second.evaluate(async (url) => { + const client: Client = await import(url); + return Boolean(client.getAccessToken()); + }, moduleUrl), + false + ); + console.log( + 'PASS: overlapping refreshes preserve the shared cookie and both tabs; logout still ends the session.' + ); +} finally { + await browser.close(); + await Promise.all(processes.map(killAndWait)); + clearCredentials(); + rmSync(root, { recursive: true, force: true }); +} diff --git a/frontend/src/lib/api/client.test.ts b/frontend/src/lib/api/client.test.ts index 4f70fe9..02afebb 100644 --- a/frontend/src/lib/api/client.test.ts +++ b/frontend/src/lib/api/client.test.ts @@ -280,6 +280,43 @@ describe('auth persistence in the API client', () => { } }); + for (const action of ['refresh', 'logout'] as const) { + for (const persistent of [false, true]) { + test(`${action} retries a superseded cookie once and preserves auth on repeated conflict: ${persistent}`, async () => { + let calls = 0; + const server = Bun.serve({ + port: 0, + fetch() { + calls++; + if (persistent || calls === 1) + return Response.json( + { error: 'Rotated', code: 'REFRESH_SUPERSEDED' }, + { status: 409 } + ); + return Response.json({ accessToken: 'replacement', expiresAt: '2030-01-01T00:00:00Z' }); + } + }); + const { previousWindow, previousLocalStorage } = installWindow(server.url.origin); + setAccessToken('original-token'); + try { + const operation = action === 'refresh' ? refreshSession() : logoutSession(); + if (persistent) { + await assert.rejects(operation, { status: 409, code: 'REFRESH_SUPERSEDED' }); + assert.equal(getAccessToken(), 'original-token'); + } else { + await operation; + assert.equal(getAccessToken(), action === 'refresh' ? 'replacement' : null); + } + assert.equal(calls, 2); + } finally { + clearTokens(); + Object.assign(globalThis, { window: previousWindow, localStorage: previousLocalStorage }); + server.stop(true); + } + }); + } + } + test('logout 401 never starts an automatic refresh', async () => { let refreshCalls = 0; const server = Bun.serve({ diff --git a/frontend/src/lib/api/client.ts b/frontend/src/lib/api/client.ts index 383d4cd..cd0645a 100644 --- a/frontend/src/lib/api/client.ts +++ b/frontend/src/lib/api/client.ts @@ -96,6 +96,22 @@ function isTerminalAuthFailure(error: unknown): error is ApiRequestError { return error instanceof ApiRequestError && error.status === 401; } +// A parallel tab may have rotated the shared cookie. Retry once without ending the session. +async function requestSessionMutation(endpoint: '/auth/refresh' | '/auth/logout'): Promise { + const request = () => apiRequest(endpoint, { method: 'POST', skipAuth: true }); + try { + return await request(); + } catch (error) { + if ( + !(error instanceof ApiRequestError) || + error.status !== 409 || + error.code !== 'REFRESH_SUPERSEDED' + ) + throw error; + return request(); + } +} + /** * Refresh the session once for all same-tab callers. */ @@ -107,10 +123,7 @@ export async function refreshSession(): Promise { const operation = (async () => { try { - const response = await apiRequest('/auth/refresh', { - method: 'POST', - skipAuth: true - }); + const response = await requestSessionMutation('/auth/refresh'); setAccessToken(response.accessToken); if (typeof window !== 'undefined') { window.dispatchEvent(new AuthRefreshEvent({ kind: 'succeeded' })); @@ -155,10 +168,7 @@ export function logoutSession(): Promise { } try { - await apiRequest('/auth/logout', { - method: 'POST', - skipAuthRefresh: true - }); + await requestSessionMutation('/auth/logout'); } catch (error) { if (isTerminalAuthFailure(error)) { sessionEnded = true; @@ -188,7 +198,6 @@ export interface RequestOptions { body?: unknown; headers?: Record; skipAuth?: boolean; - skipAuthRefresh?: boolean; params?: Record; signal?: AbortSignal; } @@ -252,15 +261,7 @@ export async function parseResponse(response: Response): Promise { * Main API request function with automatic token injection and refresh */ export async function apiRequest(endpoint: string, options: RequestOptions = {}): Promise { - const { - method = 'GET', - body, - headers = {}, - skipAuth = false, - skipAuthRefresh = false, - params, - signal - } = options; + const { method = 'GET', body, headers = {}, skipAuth = false, params, signal } = options; const url = buildUrl(endpoint, params); @@ -298,7 +299,7 @@ export async function apiRequest(endpoint: string, options: RequestOptions = let response = await fetch(url, fetchOptions); // A logout response is terminal for this request; never rotate after its 401. - if (response.status === 401 && !skipAuth && !skipAuthRefresh) { + if (response.status === 401 && !skipAuth) { try { const refreshed = await refreshSession(); requestHeaders['Authorization'] = `Bearer ${refreshed.accessToken}`; diff --git a/frontend/src/lib/api/earlyStart.test.ts b/frontend/src/lib/api/earlyStart.test.ts index cdd9198..a8f933a 100644 --- a/frontend/src/lib/api/earlyStart.test.ts +++ b/frontend/src/lib/api/earlyStart.test.ts @@ -351,6 +351,15 @@ describe('restoreSession', () => { assert.deepEqual(requests, []); }); + test('a superseded shell refresh falls back to the current shared cookie', async () => { + const { win } = runShell({ + refresh: () => Response.json({ code: 'REFRESH_SUPERSEDED' }, { status: 409 }) + }); + assert.equal((await restoreSession(createEarlyStart(() => win))).accessToken, 'app-token'); + assert.equal(requests.length, 1); + assert.match(requests[0], /\/auth\/refresh$/); + }); + test('refreshes by itself when the shell started nothing or its refresh failed', async () => { const failed = runShell({ refresh: () => new Response('no', { status: 503 }) }); assert.equal( diff --git a/scripts/auth-smoke/http.ts b/scripts/auth-smoke/http.ts index bd73645..d995b3c 100644 --- a/scripts/auth-smoke/http.ts +++ b/scripts/auth-smoke/http.ts @@ -157,6 +157,28 @@ export async function expectRefreshRejected( await response.arrayBuffer(); } +export async function expectRefreshSuperseded( + baseUrl: string, + refreshToken: string, + label: string, +): Promise { + const response = await request(baseUrl, label, refreshPath, { + method: "POST", + headers: { Cookie: `${refreshCookieName}=${refreshToken}` }, + }); + expectStatus(response, 409, label); + if (response.headers.has("Set-Cookie")) + throw new Error(`${label}: stale request changed the cookie`); + const body: unknown = await response.json(); + if ( + typeof body !== "object" || + body === null || + !("code" in body) || + body.code !== "REFRESH_SUPERSEDED" + ) + throw new Error(`${label}: missing conflict code`); +} + export async function logout( baseUrl: string, refreshToken: string, diff --git a/scripts/auth-smoke/scenarios/sessions.ts b/scripts/auth-smoke/scenarios/sessions.ts index 8cb2545..059da71 100644 --- a/scripts/auth-smoke/scenarios/sessions.ts +++ b/scripts/auth-smoke/scenarios/sessions.ts @@ -6,6 +6,7 @@ import { cookieValue, expectCookieCleared, expectRefreshRejected, + expectRefreshSuperseded, login, logout, protectedFiles, @@ -78,7 +79,7 @@ export async function checkSessions(context: SmokeContext, server: Subprocess) { (response) => response.status === 200, ); const losers = concurrentRefreshes.filter( - (response) => response.status === 401, + (response) => response.status === 409, ); expect(winners.length === 1, "concurrent refresh did not have one winner"); expect(losers.length === 1, "concurrent refresh did not reject one replay"); @@ -86,7 +87,10 @@ export async function checkSessions(context: SmokeContext, server: Subprocess) { const loserResponse = losers[0]; expect(winnerResponse !== undefined, "concurrent refresh winner was missing"); expect(loserResponse !== undefined, "concurrent refresh replay was missing"); - expectCookieCleared(loserResponse, "concurrent refresh replay"); + expect( + !loserResponse.headers.has("Set-Cookie"), + "concurrent refresh replay changed the shared cookie", + ); await loserResponse.arrayBuffer(); const replayWinner = { accessToken: await responseAccessToken( @@ -108,7 +112,7 @@ export async function checkSessions(context: SmokeContext, server: Subprocess) { 200, "winner replacement access", ); - await expectRefreshRejected( + await expectRefreshSuperseded( baseUrl, replayInitial.refreshToken, "losing concurrent refresh replay", @@ -126,12 +130,12 @@ export async function checkSessions(context: SmokeContext, server: Subprocess) { "winner access after replay", ); - await expectRefreshRejected( + await expectRefreshSuperseded( baseUrl, logoutInitial.refreshToken, "first rotated refresh before logout", ); - await expectRefreshRejected( + await expectRefreshSuperseded( baseUrl, logoutRotated.refreshToken, "second rotated refresh before logout", @@ -211,7 +215,7 @@ export async function checkSessions(context: SmokeContext, server: Subprocess) { `${label} access after restart`, ); } - await expectRefreshRejected( + await expectRefreshSuperseded( baseUrl, replayInitial.refreshToken, "old concurrent refresh after restart", @@ -329,7 +333,7 @@ export async function checkSessions(context: SmokeContext, server: Subprocess) { scenario: "Refresh replay and private transports", outcome: "PASS", evidence: - "Two concurrent uses of one refresh token produced one HTTP 200 winner and one HTTP 401 replay; the winning session refreshed before and after restart. Active stream header/query returned exact marker bytes, active WebSocket header/query returned 101, and removed-session stream/WebSocket header/query returned 401.", + "Two concurrent uses of one refresh token produced one HTTP 200 winner and one HTTP 409 replay without Set-Cookie; the winning session refreshed before and after restart. Active stream header/query returned exact marker bytes, active WebSocket header/query returned 101, and removed-session stream/WebSocket header/query returned 401.", }); return { server, activeAfterRestart }; }