Skip to content

Fix cross-tab race condition in silent SIWE refresh-token rotation #305

Description

@Lakes41

Difficulty: Expert
Type: bug (protocol/concurrency)

Background

Per docs/refresh-token-contract.md, refresh tokens are strictly one-time-use: "After a successful /v1/auth/siwe/refresh call the presented token is immediately invalidated," and reuse of an already-used token should trigger "total session revocation" server-side. The frontend guards against a single tab issuing two concurrent refresh calls via a per-provider isRefreshing ref in lib/wallet/providers.tsx (const isRefreshing = useRef(false), checked and set inside performSilentRefresh). Cross-tab coordination is handled separately, via a BroadcastChannel (channel name guildpass:auth) that propagates signed-in/refreshed/signed-out events to peer tabs after a local action succeeds.

Problem

isRefreshing is a useRef local to each SiweAuthProvider instance — i.e., per tab, not shared across tabs. The multi-tab renewal timer (msUntilRenewal() from lib/session.ts, used to schedule proactive refresh ~60s before expiry) is computed independently in every open tab from the same shared expiresAt value read out of sessionStorage/the initial hydration. This means two tabs can each independently decide it's time to refresh and both call performSilentRefresh() at nearly the same wall-clock moment — before either tab's BroadcastChannel('refreshed') message has been sent or received. Per the one-time-use contract, only the first of these two concurrent /v1/auth/siwe/refresh calls can succeed; the second is presenting a refresh token that a correctly-implemented backend will have just invalidated, causing a 401 in that tab. Per the contract's replay-detection note, a naively strict backend implementation could even interpret this as a theft signal and revoke all outstanding refresh tokens for that address — logging the user out everywhere, in every tab, despite no actual malicious activity having occurred. Even with a lenient backend, the "losing" tab's session incorrectly appears expired and forces the admin to re-sign with their wallet, a significant unnecessary UX and trust cost for something that should be entirely transparent.

Expected Outcome

Only one tab at a time ever attempts a silent refresh for a given session; other tabs either wait for that in-flight refresh's result (via BroadcastChannel) or skip their own scheduled refresh entirely once they observe a peer's refresh completing, eliminating the race described above without requiring any backend contract changes (per the doc, multi-tab coordination is explicitly frontend-only).

Suggested Implementation

  1. Introduce a cross-tab coordination primitive. Two reasonable approaches — pick and justify one in the PR description:
    • Option A — Web Locks API (navigator.locks.request('guildpass-siwe-refresh', ...), widely supported in evergreen browsers): wrap the refresh attempt in a named lock so only one tab's callback executes at a time across the whole browser; other tabs' lock requests queue and, once granted, can short-circuit if they detect (via updated sessionStorage/a fresh BroadcastChannel message received while queued) that a peer already completed the refresh.
    • Option B — BroadcastChannel-based leader coordination: before calling siweRefresh(), a tab broadcasts a refresh-intent message with a random tab ID and a short claim window; all tabs (including the sender) briefly wait, and only the tab with, say, the lexicographically lowest ID proceeds, with the others waiting for the resulting refreshed/signed-out broadcast instead of calling siweRefresh() themselves.
  2. Whichever approach is chosen, ensure it degrades gracefully in environments without BroadcastChannel/Web Locks (the code already handles the absent-BroadcastChannel case defensively — preserve that).
  3. Add a bounded timeout so a tab that "loses" the coordination but never observes a peer's success/failure broadcast (e.g., the leader tab crashed or was closed mid-refresh) eventually falls back to attempting its own refresh rather than hanging indefinitely.
  4. Add targeted tests. Since this is inherently a multi-instance/multi-tab scenario, use two SiweAuthProvider instances (or two hook instances backed by mocked BroadcastChannel/navigator.locks) in the same test process to simulate two tabs and assert exactly one siweRefresh() API call is made when both "tabs" hit their renewal timer within the same tick, and that the second tab ends up with the correctly rotated token afterward rather than a forced sign-out.
  5. Document the coordination mechanism in docs/refresh-token-contract.md's "Multi-tab behaviour" section.

Acceptance Criteria

  • A test simulating two tabs whose renewal timers fire within the same short window results in exactly one call to siweRefresh().
  • The "losing" tab ends up authenticated with the newly rotated token (via the coordination mechanism), not forced into a re-auth prompt.
  • A tab that never receives a peer's completion signal within a bounded timeout still eventually attempts its own refresh (no permanent hang).
  • Behavior in browsers/environments without BroadcastChannel degrades to today's per-tab-only behavior (no regression, no crash).
  • docs/refresh-token-contract.md is updated to describe the new coordination mechanism.
  • npm run typecheck, npm run lint, npm test pass; relevant Playwright/e2e SIWE tests pass.

Likely Affected Files/Directories

  • lib/wallet/providers.tsx
  • lib/session.ts
  • docs/refresh-token-contract.md
  • test/siwe-reauth.test.ts
  • test/e2e/siwe-flow.spec.ts

Metadata

Metadata

Assignees

Labels

GrantFox OSSGrantFox Open Source Sponsorship program tagMaybe RewardedIssue may qualify for a reward upon successful completion per campaign rulesOfficial Campaign | FWC26Official FWC26 campaign issue — eligible for campaign scoring and rewardsbugConfirmed defect or incorrect behavior that needs to be fixedhelp wantedExtra attention is neededpriority: highAutomatically createdwalletAutomatically created

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions