test(auth): prove two tabs refreshing at once stay signed in (#700)

backend/internal/handler/auth/handler.go

@@ -457,8 +457,11 @@ func (h *Handler) RefreshToken(c *fiber.Ctx) error {
 	})
 
 	if err != nil {
-		// Reuse of a rotated token (or a lost concurrent-rotation race) has already
-		// revoked the whole family server-side. Clear the browser's cookies and
+		// Reuse of a rotated token has already revoked the whole family
+		// server-side. A concurrent refresh never lands here: inside the grace
+		// window it is served the same successor as the request it raced (D-048,
+		// #700), so the cookies are only cleared once the session is really dead,
+		// never under a winner in another tab. Clear the browser's cookies and
 		// record it as the distinct security event it is, so a leaked token shows
 		// up in the audit trail rather than as one more "refresh failed".
 		if errors.Is(err, coreauth.ErrRefreshTokenReuse) {

backend/internal/handler/auth/refresh_concurrency_e2e_test.go

@@ -0,0 +1,218 @@
+// Copyright (c) 2026 OpenDefender Contributors
+// SPDX-License-Identifier: AGPL-3.0-only
+// This program is free software: you can redistribute it and/or modify it under
+// the terms of the GNU Affero General Public License v3.0 (see LICENSE).
+
+package auth_test
+
+import (
+	"context"
+	"encoding/json"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/gofiber/fiber/v2"
+	"github.com/google/uuid"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	appauth "github.com/opendefender/openrisk/internal/application/auth"
+	coreauth "github.com/opendefender/openrisk/internal/auth"
+	"github.com/opendefender/openrisk/internal/domain"
+	authhandler "github.com/opendefender/openrisk/internal/handler/auth"
+	"github.com/opendefender/openrisk/internal/infrastructure/repository"
+	"github.com/opendefender/openrisk/internal/middleware"
+)
+
+// #700 — two tabs of one browser share a cookie jar. When both renew the session
+// at once, the losing refresh used to answer REFRESH_REUSE_DETECTED and clear the
+// session cookies, wiping the ones the winner had just set. D-048 (#777) made a
+// replay inside the grace window idempotent in the token manager; these tests
+// hold the HTTP contract the browser actually sees.
+
+type refreshStack struct {
+	*deferredFixture
+}
+
+func newRefreshStack(t *testing.T) *refreshStack {
+	t.Helper()
+	f := newDeferredFixture(t)
+	// ":memory:" is per connection: a burst must not open a second, empty one.
+	sqlDB, err := f.db.DB()
+	require.NoError(t, err)
+	sqlDB.SetMaxOpenConns(1)
+
+	tokens := coreauth.NewTokenManager(f.db, f.keys)
+	tokens.SetSessionResolver(func(_ context.Context, _ uuid.UUID) (*coreauth.SessionClaims, error) {
+		return &coreauth.SessionClaims{
+			TenantID: f.tenantA,
+			OrgRoles: map[uuid.UUID]string{f.tenantA: string(domain.RoleUser)},
+		}, nil
+	})
+	loginUC := appauth.NewLoginUseCase(repository.NewGormUserRepository(f.db), tokens, deferredHasher{})
+	h := authhandler.NewHandler(loginUC, nil, appauth.NewRefreshTokenUseCase(tokens), nil, deferredHasher{}, nil)
+
+	app := fiber.New()
+	api := app.Group("/api/v1")
+	api.Post("/auth/login", h.Login)
+	api.Post("/auth/refresh", h.RefreshToken)
+	f.app = app
+	return &refreshStack{deferredFixture: f}
+}
+
+// loginRefreshCookie signs in and returns the refresh cookie the browser holds.
+func (s *refreshStack) loginRefreshCookie(t *testing.T) string {
+	t.Helper()
+	req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/login",
+		jsonReader(t, map[string]string{"email": s.memberA.User.Email, "password": deferredPassword}))
+	req.Header.Set("Content-Type", "application/json")
+	resp, err := s.app.Test(req, -1)
+	require.NoError(t, err)
+	_ = resp.Body.Close()
+	require.Equal(t, http.StatusOK, resp.StatusCode)
+	refresh := cookieNamed(resp, middleware.RefreshTokenCookie)
+	require.NotNil(t, refresh, "login must set the refresh cookie")
+	return refresh.Value
+}
+
+type refreshResult struct {
+	status int
+	code   string
+	resp   *http.Response
+}
+
+// send replays what the SPA sends: the refresh cookie and no body. It touches
+// no *testing.T, so a burst can call it from its own goroutines.
+func (s *refreshStack) send(token string) (refreshResult, error) {
+	req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/refresh", nil)
+	req.AddCookie(&http.Cookie{Name: middleware.RefreshTokenCookie, Value: token})
+	resp, err := s.app.Test(req, -1)
+	if err != nil {
+		return refreshResult{}, err
+	}
+	defer func() { _ = resp.Body.Close() }()
+	raw, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return refreshResult{}, err
+	}
+	var body struct {
+		Code string `json:"code"`
+	}
+	_ = json.Unmarshal(raw, &body)
+	return refreshResult{status: resp.StatusCode, code: body.Code, resp: resp}, nil
+}
+
+func (s *refreshStack) refresh(t *testing.T, token string) refreshResult {
+	t.Helper()
+	r, err := s.send(token)
+	require.NoError(t, err)
+	return r
+}
+
+func cookieNamed(resp *http.Response, name string) *http.Cookie {
+	for _, c := range resp.Cookies() {
+		if c.Name == name {
+			return c
+		}
+	}
+	return nil
+}
+
+// clearedCookies lists the session cookies a response tells the browser to drop.
+func clearedCookies(resp *http.Response) []string {
+	var out []string
+	for _, c := range resp.Cookies() {
+		if c.MaxAge < 0 || c.Value == "" {
+			out = append(out, c.Name)
+		}
+	}
+	return out
+}
+
+func jsonReader(t *testing.T, v any) io.Reader {
+	t.Helper()
+	raw, err := json.Marshal(v)
+	require.NoError(t, err)
+	return strings.NewReader(string(raw))
+}
+
+// TestRefreshHandler_ConcurrentBurst_Success — every request of a burst on one
+// refresh cookie succeeds, re-issues the SAME refresh cookie, and none clears the
+// session the others are setting.
+func TestRefreshHandler_ConcurrentBurst_Success(t *testing.T) {
+	s := newRefreshStack(t)
+	token := s.loginRefreshCookie(t)
+
+	const n = 6
+	results := make([]refreshResult, n)
+	errs := make([]error, n)
+	var wg sync.WaitGroup
+	start := make(chan struct{})
+	for i := 0; i < n; i++ {
+		wg.Add(1)
+		go func(i int) {
+			defer wg.Done()
+			<-start
+			results[i], errs[i] = s.send(token)
+		}(i)
+	}
+	close(start)
+	wg.Wait()
+	for i, err := range errs {
+		require.NoError(t, err, "request %d", i)
+	}
+
+	first := cookieNamed(results[0].resp, middleware.RefreshTokenCookie)
+	require.NotNil(t, first)
+	for i, r := range results {
+		require.Equal(t, http.StatusOK, r.status, "request %d (%s): a burst is not a theft", i, r.code)
+		assert.Empty(t, clearedCookies(r.resp), "request %d cleared the session the others are setting", i)
+		got := cookieNamed(r.resp, middleware.RefreshTokenCookie)
+		require.NotNil(t, got, "request %d set no refresh cookie", i)
+		assert.Equal(t, first.Value, got.Value, "request %d was handed a token of its own: the lineage forked", i)
+		assert.NotNil(t, cookieNamed(r.resp, middleware.AccessTokenCookie), "request %d set no access cookie", i)
+	}
+
+	// Whatever order the responses landed in, the jar holds a live session.
+	assert.Equal(t, http.StatusOK, s.refresh(t, first.Value).status)
+}
+
+// TestRefreshHandler_UnknownToken_NotFound — a token the server never issued is
+// refused as a plain failure, not as reuse.
+func TestRefreshHandler_UnknownToken_NotFound(t *testing.T) {
+	s := newRefreshStack(t)
+	r := s.refresh(t, "not-a-token-we-issued")
+	assert.Equal(t, http.StatusUnauthorized, r.status)
+	assert.Empty(t, r.code)
+}
+
+// TestRefreshHandler_ReplayAfterWindow_Unauthorized — the grace window is not a
+// loophole: a rotated token replayed after it is still reuse. The family dies,
+// the cookies are cleared and the code tells the client why (criterion 2).
+func TestRefreshHandler_ReplayAfterWindow_Unauthorized(t *testing.T) {
+	s := newRefreshStack(t)
+	original := s.loginRefreshCookie(t)
+
+	rotated := s.refresh(t, original)
+	require.Equal(t, http.StatusOK, rotated.status)
+	successor := cookieNamed(rotated.resp, middleware.RefreshTokenCookie).Value
+
+	require.NoError(t, s.db.Model(&coreauth.RefreshToken{}).
+		Where("rotated_at IS NOT NULL").
+		Update("rotated_at", time.Now().Add(-coreauth.RotationGracePeriod-time.Minute)).Error)
+
+	replay := s.refresh(t, original)
+	assert.Equal(t, http.StatusUnauthorized, replay.status)
+	assert.Equal(t, "REFRESH_REUSE_DETECTED", replay.code)
+	assert.ElementsMatch(t,
+		[]string{middleware.AccessTokenCookie, middleware.RefreshTokenCookie, middleware.CSRFCookie},
+		clearedCookies(replay.resp))
+
+	// The family is gone: the legitimate holder of the successor is signed out too.
+	assert.Equal(t, http.StatusUnauthorized, s.refresh(t, successor).status)
+}

docs/700_CONCURRENT_REFRESH.md

@@ -0,0 +1,141 @@
+# Spec — #700 two tabs refreshing at once must not sign the user out
+
+Status: approved by owner 2026-10-07 (proof and tests only, no client lock) · done, see Results · Branch: `fix/700-concurrent-refresh-two-tabs` · Milestone: trust-v1
+
+## Objective
+
+A user with OpenRisk open in two tabs of the same browser was signed out of every
+tab when both renewed their session at the same instant. The losing
+`POST /auth/refresh` answered `401 REFRESH_REUSE_DETECTED` and cleared the
+session cookies, and because tabs share one cookie jar, that wiped the cookies
+the winning request had just set.
+
+Success means that two tabs (or a retry, or an app waking up) refreshing together
+both stay signed in. A refresh token replayed after the chain has moved on is
+still treated as theft.
+
+## What changed since the issue was filed
+
+The backend fix exists already. The owner decided **D-048** on 2026-09-23 (grace
+window plus derived successor), and #777 implemented it. It is merged on master
+(`e6cff71c`, `635dd9e4`).
+
+| Piece | File | Fact (read 2026-10-07) |
+|---|---|---|
+| Grace window | `backend/internal/auth/token.go:62` | `RotationGracePeriod = 10s` |
+| Rotation | `backend/internal/auth/token.go` `RefreshTokenPair` | Inside the window, a replay gets the **same** successor, `HMAC(key, family_id ‖ presented)`. Outside the window, on a fingerprint mismatch, or once the successor has been spent, the family is revoked and the call returns `ErrRefreshTokenReuse` |
+| Unit proof | `backend/internal/auth/token_test.go:187` `TestRefresh_ConcurrentRotation_WithinGraceAllSucceed` | 8 goroutines, one token: all succeed with one successor and the lineage does not fork. Past the window the original token still revokes the family |
+| Reuse proof | `token_test.go:131` `TestRefresh_Reuse_RevokesFamily`, `:239` `TestRefresh_ReplayAfterChainMovedOn` | unchanged security semantics |
+| Handler | `backend/internal/handler/auth/handler.go:427-438` | still clears cookies on `ErrRefreshTokenReuse`. That is correct now, because the error only fires on genuine reuse. **The comment still says "or a lost concurrent-rotation race", which is stale** |
+| Frontend | `frontend/src/lib/api.ts:82-108` | one in-flight refresh per tab; a failed refresh goes to `/login`. CSRF is read from the `or_csrf` cookie, so the last writer wins consistently. The access token is per tab and in memory, and each tab's token is a valid JWT |
+
+No issue comment and no PR on #700 records any of this. Nobody has shown the
+issue's criteria 1, 3 and 5 against the running product.
+
+## Assumptions (correct me or I proceed with these)
+
+1. **No new design.** D-048 is the decision the issue's design note asked for. There is nothing new to escalate, and `docs/DECISIONS.md` is not touched.
+2. **No client-side cross-tab lock** (Web Locks / `BroadcastChannel`). The server already makes concurrent refresh idempotent. A lock would add auth-client surface for no change a user can see. It stays available as defence in depth if the live pass shows a gap.
+3. **Real concurrency is proved on Postgres through the live stack, not by a new pg test harness.** The handler tests run on sqlite, which serialises writes, so the `RowsAffected != 1 → re-read` branch never really races there. The issue's own evidence was taken at the API level against the local stack, and the same method serves as proof here.
+4. **Criterion 4's backend test is added at the HTTP level** (`app.Test`, sqlite). The existing tests cover the token manager. What they do not cover is that the *handler* answers 200 to every request in a burst, re-issues the same `or_refresh`, and never sends a clearing `Set-Cookie`.
+
+## Scope
+
+In:
+- **T1:** a handler-level test of a concurrent refresh burst plus a reuse-after-window regression test. Fix the stale comment at `handler.go:427`.
+- **T2:** a Playwright test with one browser context, two tabs, the `or_access` cookie removed, and both tabs reloading at once. Both must stay signed in.
+- **T3:** a live pass on Postgres: the issue's API burst and two-tab scenario, 3 runs each, with output pasted into the issue.
+
+Out:
+- client-side cross-tab lock (assumption 2);
+- turning a network error or 5xx on `/auth/refresh` into something other than `/login` (`api.ts` `.catch(() => false)`). This is a separate defect; I open an issue if the live pass reproduces it;
+- residual CSRF race: tab A reads `or_csrf`=X just as tab B's refresh lands Y, and a *mutation* sent in that microsecond window gets a 403. It is noted here but not fixed unless the live pass hits it.
+
+## Commands
+
+```
+Backend unit:   cd backend && go test ./internal/auth/ ./internal/handler/auth/ -run 'Refresh' -count=1 -race
+Backend full:   cd backend && go test ./... -count=1           # route/authz ratchets
+Frontend types: cd frontend && npx tsc -b --noEmit
+Playwright:     cd frontend && OPENRISK_BASE_URL=<vite> E2E_API_URL=<api>/api/v1 npx playwright test e2e/session-tabs.spec.ts --repeat-each 3 --workers 1
+Live stack:     throwaway pg/redis on spare ports, server from repo root (live-boot recipe)
+```
+
+## Project structure touched
+
+```
+backend/internal/handler/auth/refresh_concurrency_e2e_test.go   new — T1
+backend/internal/handler/auth/handler.go                        comment only — T1
+frontend/e2e/session-tabs.spec.ts                               new — T2
+docs/700_CONCURRENT_REFRESH.md                                  this spec
+```
+
+## Code style
+
+Follow the existing handler e2e tests (`logout_e2e_test.go`, `mfa_challenge_e2e_test.go`): a
+fixture constructor, `app.Test(req, -1)`, `require`. Fire the burst from goroutines released
+by a closed channel, as in `TestRefresh_ConcurrentRotation_WithinGraceAllSucceed`.
+
+```go
+for i, resp := range responses {
+	require.Equal(t, fiber.StatusOK, resp.StatusCode, "request %d: a burst is not a theft", i)
+	require.Equal(t, refreshCookie(responses[0]), refreshCookie(resp), "request %d forked the lineage", i)
+	require.False(t, clearsSession(resp), "request %d cleared the session cookies", i)
+}
+```
+
+The Playwright test lives in its own file, `e2e/session-tabs.spec.ts`, and not in `e2e/auth.spec.ts`.
+That suite probes `/health`, while the route is `/api/v1/health`, so it skips itself on every
+stack (#895). The new file follows the same pattern: it creates an account over the API, skips when
+the API is unreachable, and uses `context.clearCookies({ name: 'or_access' })`. It then runs
+`Promise.all([a.reload(), b.reload()])` and asserts that neither URL is `/login`, that
+`/auth/me` returns 200 from both tabs, and that the jar holds `or_access` and `or_refresh`.
+
+## Testing strategy
+
+| Criterion | Proof |
+|---|---|
+| 1. Both tabs keep a valid session | T2 (Playwright) and T3 (live, 3 runs) |
+| 2. Old-token replay still revokes the family and answers `REFRESH_REUSE_DETECTED` | existing `TestRefresh_Reuse_RevokesFamily` and `TestRefresh_ReplayAfterChainMovedOn` stay green and unmodified. T1 adds the HTTP-level check (age the rotation, then 401 with the code and clearing cookies) |
+| 3. A losing request never clears the winner's cookies | T1 asserts no clearing `Set-Cookie` in a burst. T3 checks the cookie jar after the API burst |
+| 4. Backend concurrent and reuse tests | T1 with `-race` |
+| 5. Playwright two tabs | T2, with output pasted |
+
+The tests per CLAUDE.md rule 4 (`_Success` / `_NotFound` / `_Unauthorized`) map onto
+the refresh handler as: burst succeeds, unknown token gives 401, reused token gives
+401 `REFRESH_REUSE_DETECTED`.
+
+## Boundaries
+
+- **Always:** keep reuse-detection tests unchanged and green; run the full `go test ./...` before the PR; do the live pass with figures; post the resume-anchor comment on #700.
+- **Ask first:** any change to `RotationGracePeriod`, to the successor derivation, or to when cookies are cleared. These are D-048 territory.
+- **Never:** weaken reuse detection to make a test pass, or merge the PR.
+
+## Success criteria
+
+- [x] T1 is green under `-race`, and the existing reuse tests pass unchanged.
+- [x] T2 is green against the local stack, with output pasted.
+- [x] T3: the API burst gives every response 200 with the same `or_refresh` and the jar is intact afterwards, 3 out of 3 runs. The two-tab reload passes 3 out of 3.
+- [x] Old-token replay outside the window, checked live: 401 `REFRESH_REUSE_DETECTED`, and the family is gone.
+- [ ] PR open with `Closes #700`, and the issue carries `status:in-review`.
+
+## Results (2026-10-07, throwaway Postgres 16 + Redis 7, master `cefe453d`)
+
+| Check | master (with #777) | before #777 (`9dd1c3ee`) |
+|---|---|---|
+| API, 2 concurrent refreshes, 3 runs | 3/3: both 200, one `or_refresh`, no clearing, jar intact, `/auth/me` 200, next refresh 200 | one 200 and one `401 REFRESH_REUSE_DETECTED`; the jar ends **empty**, `/auth/me` 401 |
+| API, 8 concurrent refreshes | 8 × 200, one `or_refresh` | — |
+| Replay of the original token 11 s later | 401 `REFRESH_REUSE_DETECTED`, clears all 3 cookies; the legitimate successor then gets 401 (family revoked) | — |
+| Family integrity in DB | each family holds exactly 1 live token (no fork) | — |
+| Playwright `session-tabs.spec.ts`, `--repeat-each 3` | 6/6 passed | 6/6 failed (`REFRESH_REUSE_DETECTED`) |
+| `TestRefreshHandler_*` (`-race`) | 3/3 pass | the burst test fails; the reuse test passes |
+| `go test ./...` | 80 packages ok | — |
+
+Found on the way, outside this issue:
+
+- the login screen crashes on master (`ReferenceError: notice is not defined`, `AuthScreen.tsx`). A merge of master into #872 (`990b93f3`) dropped the prop. The Playwright pass ran with the one-line fix applied locally; it ships in #893 (PR #894);
+- `e2e/auth.spec.ts` probes `/health` instead of `/api/v1/health`, so the whole auth suite skips silently (#895).
+
+## Open questions
+
+None. The owner approved proof and tests only. The client-side Web Lock was not built.