security(auth): a refresh-token reuse can leave the winner’s new session alive (race)

[alex-dembele]

Problem

TestRefresh_ConcurrentRotation_OneWinner (backend/internal/auth/token_test.go) fails
about one run in five, in isolation, with -count=1:

token_test.go:208: Error: Not equal: expected: 0, actual: 1
                   Messages: concurrent reuse revokes the family

The flake is real and it points at the code, not at the test.

Why

TokenManager.RefreshTokenPair (backend/internal/auth/token.go) does, in order:

1. claim the row atomically (UPDATE … WHERE id = ? AND rotated_at IS NULL);
2. resolve the session scope;
3. GenerateTokenPair, which inserts a new refresh row in the same family.

A concurrent loser takes the RowsAffected != 1 branch and calls
revokeFamily(familyID) immediately. Nothing orders these two: when the loser's
DELETE … WHERE family_id = ? runs before the winner's INSERT, the winner's
brand-new refresh token is written after the family was revoked and survives.

So a token-reuse event — the signal that a refresh token leaked — can leave a live
session behind, which is exactly what family revocation exists to prevent. The
window is small, but it is the window an attacker replaying a stolen token races.

Acceptance criteria

1. Given N concurrent rotations of one refresh token, when exactly one wins and at least one is flagged as reuse, then no refresh row of that family survives — asserted by the existing test, run with -count=50 -race and green every time.
2. The winner's new token is not written after a revocation of its family (for instance: claim + insert in one transaction, and revoke by family in the same serialized path; or re-check the family after insert and delete).
3. No behaviour change for the ordinary, uncontended refresh path; internal/auth tests stay green.

Definition of Done

• go test ./internal/auth/ -run TestRefresh -count=50 -race pasted in the issue comment.
• The ordering argument written down in the code, next to the claim.

Context

Found while verifying #718/#719/#720 in the running app: the full go test ./... failed
here once, and the test then failed 1 run in 5 in isolation on an untouched checkout.
This is not caused by those branches; none of them touch internal/auth.

[alex-dembele]

backend — 2026-10-02

Done

• Re-scoped with the owner. The symptom described here was already fixed by security(auth): family revocation can miss the token the winning rotation is about to insert #775, and the test it names was replaced in security(auth): a grace window so a concurrent refresh is not read as token theft #777. A gap remained on Postgres only: a revoking DELETE misses a successor committed after its snapshot, and the rotation still sees its witness because the DELETE has not committed.
• backend/internal/auth/token.go: sweepRefreshTokens repeats the DELETE until a pass removes nothing (at most 5). It is used by revokeFamily, RevokeAllUserTokens and RevokeUserTokensInTenant. The ordering argument is written next to the sweep and next to the witness check.
• backend/internal/auth/token_pg_test.go (new): forces the interleaving on Postgres for all three revokers.
• backend/internal/auth/token_test.go: TestSweepRefreshTokens_TakesARowStoredBehindIt.
• Spec: docs/725_REFRESH_REVOCATION_RACE.md. PR fix(auth): sweep refresh revocations until a pass finds nothing (#725) #890.

Verified

# against master's token.go: the new tests FAIL (expected 0, actual 1, for family / all user tokens / user tokens in tenant)
$ go test ./internal/auth/ -run TestRefresh -count=50 -race
ok  	github.com/opendefender/openrisk/internal/auth	49.553s
$ DATABASE_URL=postgres://…throwaway pg16… go test ./internal/auth/ -run Postgres -count=20 -race
ok  	github.com/opendefender/openrisk/internal/auth	20.246s
$ go test ./...
78 packages ok; internal/handler and cmd/server do not build on master (saml2_handler.go, #886), untouched here

Criteria 1 ✅ (the original test no longer exists; its successor and the new Postgres test pass, 50× and 20× with -race) · 2 ✅ · 3 ✅ (the refresh path diff is comments only)

Next — tech-lead review of PR #890, since it touches auth. After #886 lands, rerun go test ./... on the merge.

Blocked on — nothing