diff --git a/src/server/auth/loginGuard.ts b/src/server/auth/loginGuard.ts index 392757f647..e7c61532b2 100644 --- a/src/server/auth/loginGuard.ts +++ b/src/server/auth/loginGuard.ts @@ -27,6 +27,21 @@ interface AttemptState { const attempts: Map = new Map(); +// Above this many tracked IPs, opportunistically drop entries whose window has elapsed and +// that are not currently locked. Without this the map only ever grew (entries were deleted +// only on a *successful* login), so every distinct IP that ever failed a login leaked a +// permanent entry — unbounded under distributed brute-force. Expired/unlocked entries are +// already treated as "allowed", so removing them never changes a guard decision. +const PRUNE_THRESHOLD = 256; + +function pruneExpiredAttempts(now: number): void { + for (const [key, state] of attempts) { + const windowElapsed = now - state.firstAttemptAt > WINDOW_MS; + const notLocked = !state.lockedUntil || state.lockedUntil <= now; + if (windowElapsed && notLocked) attempts.delete(key); + } +} + export interface GuardDecision { allowed: boolean; retryAfterSeconds?: number; @@ -65,6 +80,10 @@ export function recordLoginFailure( if (!options.enabled) return { allowed: true }; const key = clientKey(rawIp); const now = nowMs(); + + // Keep the map from growing without bound as distinct IPs fail logins over time. + if (attempts.size > PRUNE_THRESHOLD) pruneExpiredAttempts(now); + const existing = attempts.get(key); if (!existing || now - existing.firstAttemptAt > WINDOW_MS) { @@ -99,6 +118,11 @@ export function resetLoginGuardForTests(): void { attempts.clear(); } +/** Test-only: current number of tracked IP entries. */ +export function getLoginGuardSizeForTests(): number { + return attempts.size; +} + export const LOGIN_GUARD_TUNABLES = Object.freeze({ WINDOW_MS, LOCKOUT_MS, diff --git a/tests/unit/auth/loginGuard.test.ts b/tests/unit/auth/loginGuard.test.ts index 1dfb82ece1..f03dcdf746 100644 --- a/tests/unit/auth/loginGuard.test.ts +++ b/tests/unit/auth/loginGuard.test.ts @@ -1,10 +1,12 @@ import { describe, it, beforeEach } from "node:test"; import assert from "node:assert/strict"; +import { mock } from "node:test"; import { checkLoginGuard, clearLoginAttempts, recordLoginFailure, resetLoginGuardForTests, + getLoginGuardSizeForTests, LOGIN_GUARD_TUNABLES, } from "../../../src/server/auth/loginGuard"; @@ -62,4 +64,49 @@ describe("loginGuard", () => { } assert.equal(checkLoginGuard(undefined, { enabled: true }).allowed, false); }); + + it("prunes expired, unlocked entries so the attempts map does not grow without bound", () => { + mock.timers.enable({ apis: ["Date"] }); + try { + // Many distinct IPs each fail once (single, unlocked attempts). + for (let i = 0; i < 300; i++) { + recordLoginFailure(`9.${Math.floor(i / 256)}.${i % 256}.1`, { enabled: true }); + } + assert.ok(getLoginGuardSizeForTests() > 256, "entries should accumulate before pruning"); + + // Advance past the sliding window so all those entries are expired + unlocked. + mock.timers.tick(LOGIN_GUARD_TUNABLES.WINDOW_MS + 1000); + + // The next failure (map size > threshold) triggers the opportunistic prune. + recordLoginFailure("1.1.1.1", { enabled: true }); + + // Only the fresh entry should remain; the stale ones were reaped. + assert.equal(getLoginGuardSizeForTests(), 1); + } finally { + mock.timers.reset(); + } + }); + + it("never prunes a still-locked entry", () => { + mock.timers.enable({ apis: ["Date"] }); + try { + const lockedIp = "5.5.5.5"; + for (let i = 0; i < LOGIN_GUARD_TUNABLES.FAILURE_THRESHOLD; i++) { + recordLoginFailure(lockedIp, { enabled: true }); + } + assert.equal(checkLoginGuard(lockedIp, { enabled: true }).allowed, false); + + // Fill past the prune threshold with unlocked entries, then trigger a prune + // while still inside the lockout window. + for (let i = 0; i < 300; i++) { + recordLoginFailure(`8.${Math.floor(i / 256)}.${i % 256}.1`, { enabled: true }); + } + recordLoginFailure("2.2.2.2", { enabled: true }); + + // The locked IP must still be locked (not reaped). + assert.equal(checkLoginGuard(lockedIp, { enabled: true }).allowed, false); + } finally { + mock.timers.reset(); + } + }); });