Fix sign-outs caused by overlapping session refreshes

.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:

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)
+			}
+		})
+	}
+}

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},

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"

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
 }

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} {

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 {

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.

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:

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.

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.

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 });
+}