Skip to content

fix(auth): raise per-user JWT cap to 20; quiet WS reconnect race#31

Merged
revtex merged 1 commit intodevfrom
fix/concurrent-token-cap
Apr 25, 2026
Merged

fix(auth): raise per-user JWT cap to 20; quiet WS reconnect race#31
revtex merged 1 commit intodevfrom
fix/concurrent-token-cap

Conversation

@revtex
Copy link
Copy Markdown
Owner

@revtex revtex commented Apr 25, 2026

Symptoms

After yesterday's audio HTTP migration, two related console annoyances surfaced when using OpenScanner from a phone and a desktop simultaneously:

  1. GET /api/calls/:id/audio 401 (Unauthorized) on the desktop after a sibling-device login (mostly fixed in fix(audio): silently retry on 401 from /api/calls/:id/audio #30 with the silent retry, but the 401 itself was a symptom of a tighter root cause).
  2. WebSocket connection to 'wss://\u2026/api/admin/ws' failed: WebSocket is closed before the connection is established after the admin WS hits an XPR (token-expired) frame and reconnects.

Root cause

The server caps each user at 5 concurrent access JWTs (auth.MaxRefreshFamilies, TokenTracker.MaxTokens). With access TTL=15 min and a silent refresh ~1 min before expiry, every active browser/tab consumes ~4 token slots per hour. Desktop + phone alone burn through 5 slots in barely over an hour, at which point the desktop's JWT \u2014 and therefore its os_session cookie \u2014 ends up server-side-revoked. The audio 401 retry covered the symptom, but the cap itself was the wrong shape for a multi-device homelab tool.

The WS warning is a separate, smaller bug: when the admin client receives XPR it reconnects via doConnect(), which closes the existing socket immediately. If that existing socket is still in CONNECTING state, Chrome logs the warning. The reconnect itself succeeds, so it's purely cosmetic \u2014 but worth fixing now while the area is fresh.

Changes

  • backend/internal/auth/auth.go: MaxRefreshFamilies 5 \u2192 20. NewTokenTracker defaults to MaxRefreshFamilies instead of a hard-coded 5. Comment explains the rationale (15-min access TTL + multi-device homelab usage).
  • backend/internal/auth/auth_test.go: TestTokenTracker_MaxFiveTokens renamed and parameterised on auth.MaxRefreshFamilies so future cap tweaks don't require test edits.
  • frontend/src/services/ws/client.ts + adminClient.ts: doConnect and disconnect detach handlers on the previous socket, then either close it directly (if OPEN) or hook onopen to a self-close (if CONNECTING). Eliminates the noisy browser warning.
  • .github/copilot-instructions.md and .github/agents/reviewer.agent.md: bump the documented limit from 5 to 20 so future reviewers don't flag the new constant.
  • CHANGELOG.md: two bullets under [Unreleased] Fixed.

Verification

  • go build ./... clean.
  • go test ./internal/auth/... ./internal/handler/routes/... pass; refresh_test.go already derived from auth.MaxRefreshFamilies.
  • pnpm exec tsc --noEmit clean.
  • pnpm test --run \u2014 188 / 188 pass.

Self-review

  • The cap is still bounded; this is not removing a defence, just sizing it for the actual user model.
  • TTL on the deny list and the per-user slice cleanup are unchanged, so memory growth is still tracked.
  • WS reconnect change is purely a handler-detach reorder; the new socket and reconnect/backoff logic are untouched.
  • No public API, schema, or response-shape changes.

Follow-up

The audio 401-retry path (#30) is still the right belt-and-braces handler for unrelated revocations (admin force-logout, etc.) and stays in.

Two related fixes for the per-user concurrent JWT cap:

1. Raise auth.MaxRefreshFamilies (and TokenTracker.MaxTokens) from 5
   to 20. With a 15-min access TTL refreshing roughly four times an
   hour, the old limit pushed a desktop's session off the active list
   within an hour of normal multi-device use (desktop + phone). 20
   leaves headroom without inflating the deny list, and the TTL still
   bounds the impact of a stolen refresh family.

2. WebSocket clients (listener and admin) no longer call .close() on a
   socket still in CONNECTING state during a reconnect. Detach the
   handlers and route the close through onopen instead, suppressing
   the cosmetic 'WebSocket is closed before the connection is
   established' browser console warning seen after a token-expiry
   refresh.

Tests updated to derive from auth.MaxRefreshFamilies instead of
hard-coded 5. All Go and frontend tests pass.
@revtex revtex merged commit 17ba0bf into dev Apr 25, 2026
7 checks passed
@revtex revtex deleted the fix/concurrent-token-cap branch April 25, 2026 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant