diff --git a/.github/agents/reviewer.agent.md b/.github/agents/reviewer.agent.md index 1f66d23..145933c 100644 --- a/.github/agents/reviewer.agent.md +++ b/.github/agents/reviewer.agent.md @@ -115,7 +115,7 @@ This reviewer covers the whole application. Sections below map to subsystems: ### A07 — Identification & Authentication Failures - [ ] Login rate limiter: 3 failures → 10-minute lockout per IP -- [ ] Max 5 concurrent JWT tokens per user enforced (oldest invalidated on 6th login) +- [ ] Max `auth.MaxRefreshFamilies` (20) concurrent JWT tokens per user enforced (oldest invalidated on overflow) - [ ] JWT tokens are invalidated on logout (server-side token tracker) - [ ] Refresh token family rotation: reuse of an old token revokes the entire family - [ ] Refresh cookie flags: `HttpOnly`, `Secure` in production, `SameSite=Lax` diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 4aaaa1b..fddcfc9 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -107,7 +107,7 @@ Detailed conventions live in the individual agent files. The non-negotiables for 12. Secrets at rest (downstream API keys, VAPID private key, webhook secrets) encrypted with AES-256-GCM using the `enc::` prefix; startup fails fast on missing/wrong encryption key 13. Refresh tokens stored as SHA-256 hashes with family rotation; reuse revokes the entire family; delivered in httpOnly/Secure/SameSite=Lax cookies 14. All outbound HTTP (downstream, webhooks, push) uses `safehttp.Client` — redirects disabled, timeouts enforced, response body capped. Private-address blocking is opt-in via `OPENSCANNER_BLOCK_INTERNAL_HTTP=1` (default allows LAN/loopback because OpenScanner is a self-hosted homelab tool) -15. Max 5 concurrent JWT tokens per user; 3-strike lockout (10 min) on login; hourly cleanup goroutine for expired refresh tokens +15. Max 20 concurrent JWT tokens per user (`auth.MaxRefreshFamilies`); 3-strike lockout (10 min) on login; hourly cleanup goroutine for expired refresh tokens ## Tooling Conventions diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a50734..37ff156 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -105,6 +105,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 the case where a sibling device login (phone, tablet, second tab) pushes a desktop's access JWT out of the per-user concurrent-token cap and leaves \ playback failing until the next scheduled refresh. +- Per-user concurrent JWT cap raised from 5 to 20 (`auth.MaxRefreshFamilies`). + With a 15-minute access TTL refreshing roughly four times per hour, the + old limit pushed a desktop's session off the active list within an hour + of normal multi-device use. 20 leaves headroom for desktop + phone + + tablet without inflating the deny list. +- WebSocket clients (listener and admin) now detach handlers and wait for + `open` before closing a still-`CONNECTING` socket during reconnect. + Suppresses the cosmetic "WebSocket is closed before the connection is + established" browser console warning that appeared after a token-expiry + reconnect. ## [1.1.2] — 2026-04-24 diff --git a/backend/internal/auth/auth.go b/backend/internal/auth/auth.go index df029dc..57ee415 100644 --- a/backend/internal/auth/auth.go +++ b/backend/internal/auth/auth.go @@ -40,8 +40,11 @@ const ( RefreshTokenExpiry = 30 * 24 * time.Hour // MaxRefreshFamilies is the maximum number of active refresh token families per user. - // Matches the "max 5 concurrent JWT tokens per user" rule in the security policy. - MaxRefreshFamilies = 5 + // With access TTL = 15 min and silent refresh ≈1 min before expiry, every active + // browser/tab consumes ~4 token slots per hour. 20 leaves comfortable headroom for + // a typical multi-device homelab user (desktop + phone + tablet) without bloating + // the deny list, and still bounds the impact of a stolen refresh family. + MaxRefreshFamilies = 20 ) // JWTSecret is the HS256 signing key. It is initialised lazily: @@ -275,10 +278,11 @@ type tokenEntry struct { ExpiresAt time.Time } -// NewTokenTracker creates a TokenTracker with a default max of 5 tokens per user. +// NewTokenTracker creates a TokenTracker with a default max of MaxRefreshFamilies +// active tokens per user. func NewTokenTracker() *TokenTracker { return &TokenTracker{ - MaxTokens: 5, + MaxTokens: MaxRefreshFamilies, userTokens: make(map[int64][]tokenEntry), denied: make(map[string]time.Time), } diff --git a/backend/internal/auth/auth_test.go b/backend/internal/auth/auth_test.go index 0775197..72d23b3 100644 --- a/backend/internal/auth/auth_test.go +++ b/backend/internal/auth/auth_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/base64" + "strconv" "strings" "testing" "time" @@ -118,30 +119,31 @@ func TestGenerateAndParseToken(t *testing.T) { }) } -func TestTokenTracker_MaxFiveTokens(t *testing.T) { +func TestTokenTracker_MaxConcurrentTokens(t *testing.T) { tt := auth.NewTokenTracker() expires := time.Now().Add(24 * time.Hour) + n := auth.MaxRefreshFamilies - // Issue 5 tokens for user 1 — all should be active. - jtis := make([]string, 6) - for i := 0; i < 5; i++ { - jtis[i] = "jti-" + time.Now().Format("150405.000") + "-" + string(rune('a'+i)) + // Issue n tokens for user 1 — all should be active. + jtis := make([]string, n+1) + for i := 0; i < n; i++ { + jtis[i] = "jti-" + time.Now().Format("150405.000000") + "-" + strconv.Itoa(i) tt.Track(1, jtis[i], expires) } - for i := 0; i < 5; i++ { + for i := 0; i < n; i++ { if tt.IsRevoked(jtis[i]) { - t.Errorf("token %d should not be revoked with only 5 active", i) + t.Errorf("token %d should not be revoked with only %d active", i, n) } } - // Issue a 6th token — the oldest (jtis[0]) should be revoked. - jtis[5] = "jti-sixth" - tt.Track(1, jtis[5], expires) + // Issue an (n+1)th token — the oldest (jtis[0]) should be revoked. + jtis[n] = "jti-overflow" + tt.Track(1, jtis[n], expires) if !tt.IsRevoked(jtis[0]) { - t.Error("oldest token (index 0) should be revoked after 6th login") + t.Errorf("oldest token (index 0) should be revoked after %d-th login", n+1) } - for i := 1; i <= 5; i++ { + for i := 1; i <= n; i++ { if tt.IsRevoked(jtis[i]) { t.Errorf("token %d should still be active", i) } diff --git a/frontend/src/services/ws/adminClient.ts b/frontend/src/services/ws/adminClient.ts index 8308dc3..e341421 100644 --- a/frontend/src/services/ws/adminClient.ts +++ b/frontend/src/services/ws/adminClient.ts @@ -118,11 +118,21 @@ class AdminWsClient { private doConnect(): void { if (this.ws) { - this.ws.onopen = null; - this.ws.onmessage = null; - this.ws.onclose = null; - this.ws.onerror = null; - this.ws.close(); + const old = this.ws; + old.onopen = null; + old.onmessage = null; + old.onclose = null; + old.onerror = null; + // If the previous socket hasn't finished its handshake yet, + // calling close() on it makes the browser log a noisy + // "WebSocket is closed before the connection is established" + // warning that's purely cosmetic. Detach handlers and let it + // settle on its own — once OPEN, close it cleanly. + if (old.readyState === WebSocket.CONNECTING) { + old.onopen = () => old.close(); + } else if (old.readyState === WebSocket.OPEN) { + old.close(); + } this.ws = null; } diff --git a/frontend/src/services/ws/client.ts b/frontend/src/services/ws/client.ts index fd8c17a..dd230e2 100644 --- a/frontend/src/services/ws/client.ts +++ b/frontend/src/services/ws/client.ts @@ -46,11 +46,16 @@ class WsClient { if (this.ws) { // Detach handlers before closing to avoid browser console noise // when the socket is still in CONNECTING state. - this.ws.onopen = null; - this.ws.onmessage = null; - this.ws.onclose = null; - this.ws.onerror = null; - this.ws.close(); + const old = this.ws; + old.onopen = null; + old.onmessage = null; + old.onclose = null; + old.onerror = null; + if (old.readyState === WebSocket.CONNECTING) { + old.onopen = () => old.close(); + } else if (old.readyState === WebSocket.OPEN) { + old.close(); + } this.ws = null; } this.recentCallIds = []; @@ -70,7 +75,16 @@ class WsClient { private doConnect(): void { if (this.ws) { - this.ws.close(); + const old = this.ws; + old.onopen = null; + old.onmessage = null; + old.onclose = null; + old.onerror = null; + if (old.readyState === WebSocket.CONNECTING) { + old.addEventListener("open", () => old.close(), { once: true }); + } else if (old.readyState === WebSocket.OPEN) { + old.close(); + } this.ws = null; }