fix(19): CR-04 scope login lockout to username, add TTL auto-expiry + admin-reset unlock
This commit is contained in:
@@ -35,6 +35,7 @@ import {
|
|||||||
CredentialValidationError,
|
CredentialValidationError,
|
||||||
} from '../broker/credentialSync.js';
|
} from '../broker/credentialSync.js';
|
||||||
import { hashPassword } from '../auth/localCredentials.js';
|
import { hashPassword } from '../auth/localCredentials.js';
|
||||||
|
import { resetLoginAttempts } from './localAuth.js';
|
||||||
import { COLOR_PALETTE } from '../auth/user.js';
|
import { COLOR_PALETTE } from '../auth/user.js';
|
||||||
// Side-effect import: brings in the ContextVariableMap augmentation for c.get('user')
|
// Side-effect import: brings in the ContextVariableMap augmentation for c.get('user')
|
||||||
import '../auth/devBypass.js';
|
import '../auth/devBypass.js';
|
||||||
@@ -220,9 +221,10 @@ adminRouter.post(
|
|||||||
const { newPassword } = c.req.valid('json');
|
const { newPassword } = c.req.valid('json');
|
||||||
// T-19-06: NEVER log newPassword or the request body
|
// T-19-06: NEVER log newPassword or the request body
|
||||||
|
|
||||||
// Verify the target user has a local_credentials row (404 if not)
|
// Verify the target user has a local_credentials row (404 if not).
|
||||||
|
// Also read the username so we can clear any login lockout for it (CR-04).
|
||||||
const [credRow] = await db
|
const [credRow] = await db
|
||||||
.select({ id: localCredentials.id })
|
.select({ id: localCredentials.id, username: localCredentials.username })
|
||||||
.from(localCredentials)
|
.from(localCredentials)
|
||||||
.where(eq(localCredentials.userId, targetId))
|
.where(eq(localCredentials.userId, targetId))
|
||||||
.limit(1);
|
.limit(1);
|
||||||
@@ -237,6 +239,11 @@ adminRouter.post(
|
|||||||
.set({ passwordHash: hashPassword(newPassword) })
|
.set({ passwordHash: hashPassword(newPassword) })
|
||||||
.where(eq(localCredentials.userId, targetId));
|
.where(eq(localCredentials.userId, targetId));
|
||||||
|
|
||||||
|
// CR-04: an admin password reset must immediately clear any rate-limit / lockout
|
||||||
|
// state for this username, so a locked-out member regains access at once rather than
|
||||||
|
// waiting for the TTL. The lockout is keyed on username (not member id).
|
||||||
|
resetLoginAttempts(credRow.username);
|
||||||
|
|
||||||
return c.json({ ok: true }, 200);
|
return c.json({ ok: true }, 200);
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
console.error(
|
console.error(
|
||||||
|
|||||||
@@ -14,8 +14,16 @@
|
|||||||
* - Dummy-hash timing defense: verifyPassword is always called, even for unknown usernames,
|
* - Dummy-hash timing defense: verifyPassword is always called, even for unknown usernames,
|
||||||
* to prevent timing-oracle username enumeration attacks (T-19-12 / RESEARCH Pitfall 2).
|
* to prevent timing-oracle username enumeration attacks (T-19-12 / RESEARCH Pitfall 2).
|
||||||
* - Same 401 body for wrong password AND unknown username — no field discrimination (T-19-12).
|
* - Same 401 body for wrong password AND unknown username — no field discrimination (T-19-12).
|
||||||
* - Per-IP in-memory rate-limiting: 5 failures → 429; 10 failures → 423 (T-19-11).
|
* - Per-USERNAME in-memory rate-limiting: 5 failures → 429; 10 failures → 423 (T-19-11).
|
||||||
* - Lockout (423) cleared only by admin password reset — no self-service unlock.
|
* - Lockout (423) auto-expires after LOCKOUT_TTL_MS (CR-04) so a single attacker cannot
|
||||||
|
* permanently deny login for the whole household, and no process restart is needed to
|
||||||
|
* recover. Admin password reset still clears it immediately (resetLoginAttempts).
|
||||||
|
*
|
||||||
|
* CR-04: the limiter is keyed on the submitted username, NOT the client IP. In this
|
||||||
|
* deployment all household traffic egresses the Pangolin tunnel with the same
|
||||||
|
* X-Forwarded-For first hop, so IP-keying made one bad actor (or one fat-fingered user)
|
||||||
|
* able to lock out every member, and X-Forwarded-For is attacker-spoofable. Username-keying
|
||||||
|
* scopes the lockout to the identity actually under attack and the TTL makes it self-healing.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import { Hono } from 'hono';
|
import { Hono } from 'hono';
|
||||||
@@ -52,22 +60,42 @@ const loginSchema = z.object({
|
|||||||
});
|
});
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
// Per-IP rate-limiting state (in-memory Map — household scale, no Redis needed).
|
// Per-USERNAME rate-limiting state (in-memory Map — household scale, no Redis needed).
|
||||||
//
|
//
|
||||||
// State shape per IP: { count, lockedUntil (epoch ms), lockedOut (bool) }
|
// State shape per username: { count, lockedUntil (epoch ms), lockedOut (bool), lockedAt (epoch ms) }
|
||||||
// count >= RATE_WINDOW_FAILURES AND Date.now() < lockedUntil → 429
|
// count >= RATE_WINDOW_FAILURES AND Date.now() < lockedUntil → 429
|
||||||
// count >= LOCKOUT_FAILURES → 423 (permanent until admin reset)
|
// count >= LOCKOUT_FAILURES → 423 (until LOCKOUT_TTL_MS elapses OR admin reset)
|
||||||
// Success → delete entry (clears counter)
|
// Success → delete entry (clears counter)
|
||||||
//
|
//
|
||||||
// RATE_WINDOW_FAILURES: 5 failures → 60s cooldown (429)
|
// RATE_WINDOW_FAILURES: 5 failures → 60s cooldown (429)
|
||||||
// LOCKOUT_FAILURES: 10 failures → account locked (423)
|
// LOCKOUT_FAILURES: 10 failures → account locked (423)
|
||||||
|
// LOCKOUT_TTL_MS: 15 min after which a 423 lockout auto-expires (CR-04)
|
||||||
|
//
|
||||||
|
// CR-04: keyed on the validated username (not the client IP) so a lockout is scoped to
|
||||||
|
// the identity under attack, and self-heals after LOCKOUT_TTL_MS without a restart.
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
export const loginAttempts = new Map<string, { count: number; lockedUntil: number; lockedOut: boolean }>();
|
export const loginAttempts = new Map<
|
||||||
|
string,
|
||||||
|
{ count: number; lockedUntil: number; lockedOut: boolean; lockedAt: number }
|
||||||
|
>();
|
||||||
|
|
||||||
const RATE_WINDOW_FAILURES = 5;
|
const RATE_WINDOW_FAILURES = 5;
|
||||||
const RATE_WINDOW_SECS = 60;
|
const RATE_WINDOW_SECS = 60;
|
||||||
const LOCKOUT_FAILURES = 10;
|
const LOCKOUT_FAILURES = 10;
|
||||||
|
const LOCKOUT_TTL_MS = 15 * 60 * 1000; // CR-04: 423 lockout auto-expires after 15 minutes
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Immediately clear any rate-limit / lockout state for a username (CR-04).
|
||||||
|
*
|
||||||
|
* Called by the admin password-reset path so a reset is an instant unlock and the
|
||||||
|
* lockout is not "unrecoverable without a process restart". Safe to call for an
|
||||||
|
* unknown username (no-op). Normalizes the username the same way the login schema
|
||||||
|
* does (trim) so the key matches what the limiter stored.
|
||||||
|
*/
|
||||||
|
export function resetLoginAttempts(username: string): void {
|
||||||
|
loginAttempts.delete(username.trim());
|
||||||
|
}
|
||||||
|
|
||||||
// Pre-computed dummy hash used to run verifyPassword on unknown-username paths
|
// Pre-computed dummy hash used to run verifyPassword on unknown-username paths
|
||||||
// (timing defense — prevents timing-oracle username enumeration, RESEARCH Pitfall 2).
|
// (timing defense — prevents timing-oracle username enumeration, RESEARCH Pitfall 2).
|
||||||
@@ -79,34 +107,42 @@ const DUMMY_HASH = hashPassword('dummy-constant-time-filler-xyzzy');
|
|||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
localAuthRouter.post('/local/login', zValidator('json', loginSchema, noEchoHook), async (c) => {
|
localAuthRouter.post('/local/login', zValidator('json', loginSchema, noEchoHook), async (c) => {
|
||||||
// Derive client IP from Pangolin-set X-Forwarded-For header; fall back to host header.
|
// CR-04: the rate-limit / lockout key is the validated, normalized username — NOT the
|
||||||
const ip = c.req.header('x-forwarded-for')?.split(',')[0]?.trim()
|
// client IP. zValidator has already run, so c.req.valid('json') is available here.
|
||||||
?? c.req.raw.headers.get('host')
|
const { username, password } = c.req.valid('json');
|
||||||
?? 'unknown';
|
const key = username; // loginSchema .trim()s the username; the map key matches resetLoginAttempts
|
||||||
|
|
||||||
const attempt = loginAttempts.get(ip);
|
const now = Date.now();
|
||||||
|
const attempt = loginAttempts.get(key);
|
||||||
|
|
||||||
// 423: account locked (>= LOCKOUT_FAILURES total failures, admin must reset)
|
// 423: account locked (>= LOCKOUT_FAILURES failures). CR-04: the lockout auto-expires
|
||||||
// Check lockedOut FIRST — lockout takes precedence over rate window.
|
// after LOCKOUT_TTL_MS so a single attacker cannot deny login indefinitely and no restart
|
||||||
|
// is required to recover. On expiry, drop the entry so the next attempt starts clean.
|
||||||
if (attempt?.lockedOut) {
|
if (attempt?.lockedOut) {
|
||||||
return c.json({ error: 'Account locked' }, 423);
|
if (now - attempt.lockedAt >= LOCKOUT_TTL_MS) {
|
||||||
|
loginAttempts.delete(key);
|
||||||
|
} else {
|
||||||
|
return c.json({ error: 'Account locked' }, 423);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Re-read after a possible expiry-delete above.
|
||||||
|
const live = loginAttempts.get(key);
|
||||||
|
|
||||||
// 429: rate window — >= RATE_WINDOW_FAILURES failures within the cooldown window.
|
// 429: rate window — >= RATE_WINDOW_FAILURES failures within the cooldown window.
|
||||||
// Increment the counter even on 429 so continued brute-force accumulates toward lockout.
|
// Increment the counter even on 429 so continued brute-force accumulates toward lockout.
|
||||||
if (attempt && attempt.count >= RATE_WINDOW_FAILURES && Date.now() < attempt.lockedUntil) {
|
if (live && live.count >= RATE_WINDOW_FAILURES && now < live.lockedUntil) {
|
||||||
attempt.count += 1;
|
live.count += 1;
|
||||||
attempt.lockedUntil = Date.now() + RATE_WINDOW_SECS * 1000;
|
live.lockedUntil = now + RATE_WINDOW_SECS * 1000;
|
||||||
attempt.lockedOut = attempt.count >= LOCKOUT_FAILURES;
|
live.lockedOut = live.count >= LOCKOUT_FAILURES;
|
||||||
loginAttempts.set(ip, attempt);
|
if (live.lockedOut) live.lockedAt = now;
|
||||||
if (attempt.lockedOut) {
|
loginAttempts.set(key, live);
|
||||||
|
if (live.lockedOut) {
|
||||||
return c.json({ error: 'Account locked' }, 423);
|
return c.json({ error: 'Account locked' }, 423);
|
||||||
}
|
}
|
||||||
return c.json({ error: 'Too many attempts' }, 429);
|
return c.json({ error: 'Too many attempts' }, 429);
|
||||||
}
|
}
|
||||||
|
|
||||||
const { username, password } = c.req.valid('json');
|
|
||||||
|
|
||||||
// Look up local_credentials by username
|
// Look up local_credentials by username
|
||||||
let cred: { userId: number; passwordHash: string } | undefined;
|
let cred: { userId: number; passwordHash: string } | undefined;
|
||||||
try {
|
try {
|
||||||
@@ -129,18 +165,19 @@ localAuthRouter.post('/local/login', zValidator('json', loginSchema, noEchoHook)
|
|||||||
: verifyPassword(DUMMY_HASH, password);
|
: verifyPassword(DUMMY_HASH, password);
|
||||||
|
|
||||||
if (!valid || !cred) {
|
if (!valid || !cred) {
|
||||||
// Increment failure counter
|
// Increment failure counter (keyed on username)
|
||||||
const cur = loginAttempts.get(ip) ?? { count: 0, lockedUntil: 0, lockedOut: false };
|
const cur = loginAttempts.get(key) ?? { count: 0, lockedUntil: 0, lockedOut: false, lockedAt: 0 };
|
||||||
cur.count += 1;
|
cur.count += 1;
|
||||||
cur.lockedUntil = Date.now() + RATE_WINDOW_SECS * 1000;
|
cur.lockedUntil = Date.now() + RATE_WINDOW_SECS * 1000;
|
||||||
cur.lockedOut = cur.count >= LOCKOUT_FAILURES;
|
cur.lockedOut = cur.count >= LOCKOUT_FAILURES;
|
||||||
loginAttempts.set(ip, cur);
|
if (cur.lockedOut && cur.lockedAt === 0) cur.lockedAt = Date.now();
|
||||||
|
loginAttempts.set(key, cur);
|
||||||
// Same 401 body for wrong password AND unknown username — no field discrimination (T-19-12)
|
// Same 401 body for wrong password AND unknown username — no field discrimination (T-19-12)
|
||||||
return c.json({ error: 'Invalid credentials' }, 401);
|
return c.json({ error: 'Invalid credentials' }, 401);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Success: clear failure counter, issue the local-session JWT cookie, return ok
|
// Success: clear failure counter, issue the local-session JWT cookie, return ok
|
||||||
loginAttempts.delete(ip);
|
loginAttempts.delete(key);
|
||||||
try {
|
try {
|
||||||
await issueLocalSessionCookie(c, cred.userId);
|
await issueLocalSessionCookie(c, cred.userId);
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
|
|||||||
@@ -5,8 +5,9 @@
|
|||||||
* Test 1: valid username+password → 200 { ok:true } + Set-Cookie for local-session
|
* Test 1: valid username+password → 200 { ok:true } + Set-Cookie for local-session
|
||||||
* Test 2: wrong password → 401 { error: 'Invalid credentials' }
|
* Test 2: wrong password → 401 { error: 'Invalid credentials' }
|
||||||
* Test 3: unknown username → 401 with SAME body as Test 2 (no enumeration / no field discrimination)
|
* Test 3: unknown username → 401 with SAME body as Test 2 (no enumeration / no field discrimination)
|
||||||
* Test 4: 5 consecutive failures from one IP → 6th returns 429
|
* Test 4: 5 consecutive failures for one username → 6th returns 429
|
||||||
* Test 5: 10 failures → 423 (lockedOut); a cleared map resets the counter
|
* Test 5: 10 failures → 423 (lockedOut); a cleared map resets the counter
|
||||||
|
* Test 5b (CR-04): a 423 lockout auto-expires after LOCKOUT_TTL_MS (no admin reset needed)
|
||||||
* Test 6: POST /api/auth/local/logout clears the local-session cookie (expired Set-Cookie)
|
* Test 6: POST /api/auth/local/logout clears the local-session cookie (expired Set-Cookie)
|
||||||
* Test 6b: GET /api/auth/local/logout (alias) also clears the local-session cookie
|
* Test 6b: GET /api/auth/local/logout (alias) also clears the local-session cookie
|
||||||
* Test 7 (no-echo): malformed body (missing password) → 400 { error: 'Invalid request' };
|
* Test 7 (no-echo): malformed body (missing password) → 400 { error: 'Invalid request' };
|
||||||
@@ -246,8 +247,9 @@ describe('POST /api/auth/local/login', () => {
|
|||||||
expect(body.error).toBe('Account locked');
|
expect(body.error).toBe('Account locked');
|
||||||
|
|
||||||
// Clear the map (simulates admin reset) → counter gone → next attempt is 401 again (not locked)
|
// Clear the map (simulates admin reset) → counter gone → next attempt is 401 again (not locked)
|
||||||
|
// CR-04: the limiter is keyed on the USERNAME ('alice'), not the IP.
|
||||||
const { loginAttempts } = await import('../../src/routes/localAuth.js');
|
const { loginAttempts } = await import('../../src/routes/localAuth.js');
|
||||||
loginAttempts.delete('10.0.0.2');
|
loginAttempts.delete('alice');
|
||||||
|
|
||||||
const resAfterReset = await app.fetch(
|
const resAfterReset = await app.fetch(
|
||||||
makeLoginRequest({ username: 'alice', password: 'wrong' }, '10.0.0.2')
|
makeLoginRequest({ username: 'alice', password: 'wrong' }, '10.0.0.2')
|
||||||
@@ -255,6 +257,36 @@ describe('POST /api/auth/local/login', () => {
|
|||||||
expect(resAfterReset.status).toBe(401);
|
expect(resAfterReset.status).toBe(401);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('Test 5b (CR-04): a 423 lockout auto-expires after the TTL — no admin reset needed', async () => {
|
||||||
|
mockCredRow = undefined;
|
||||||
|
|
||||||
|
const app = await getApp();
|
||||||
|
|
||||||
|
// 10 failures → lockout for username 'bob'
|
||||||
|
for (let i = 0; i < 10; i++) {
|
||||||
|
await app.fetch(makeLoginRequest({ username: 'bob', password: 'wrong' }, '10.0.0.3'));
|
||||||
|
}
|
||||||
|
|
||||||
|
// Confirm locked (423)
|
||||||
|
const resLocked = await app.fetch(
|
||||||
|
makeLoginRequest({ username: 'bob', password: 'wrong' }, '10.0.0.3'),
|
||||||
|
);
|
||||||
|
expect(resLocked.status).toBe(423);
|
||||||
|
|
||||||
|
// Simulate the TTL elapsing by back-dating lockedAt well past LOCKOUT_TTL_MS (15 min).
|
||||||
|
const { loginAttempts } = await import('../../src/routes/localAuth.js');
|
||||||
|
const entry = loginAttempts.get('bob');
|
||||||
|
expect(entry?.lockedOut).toBe(true);
|
||||||
|
if (entry) entry.lockedAt = Date.now() - 16 * 60 * 1000;
|
||||||
|
|
||||||
|
// Next attempt: the lockout has expired → handler drops the entry and processes the
|
||||||
|
// login normally, so a wrong password is a fresh 401 (not a 423). CR-04: self-healing.
|
||||||
|
const resAfterTtl = await app.fetch(
|
||||||
|
makeLoginRequest({ username: 'bob', password: 'wrong' }, '10.0.0.3'),
|
||||||
|
);
|
||||||
|
expect(resAfterTtl.status).toBe(401);
|
||||||
|
});
|
||||||
|
|
||||||
it('Test 7 (no-echo): malformed body (missing password) → 400 { error: "Invalid request" }; body has no echoed value or Zod received field', async () => {
|
it('Test 7 (no-echo): malformed body (missing password) → 400 { error: "Invalid request" }; body has no echoed value or Zod received field', async () => {
|
||||||
const app = await getApp();
|
const app = await getApp();
|
||||||
// Body with username but missing password (Zod will reject)
|
// Body with username but missing password (Zod will reject)
|
||||||
|
|||||||
@@ -332,7 +332,8 @@ export function LoginPage({ authMode }: LoginPageProps) {
|
|||||||
}}
|
}}
|
||||||
>
|
>
|
||||||
<AlertCircle size={16} aria-hidden="true" style={{ flexShrink: 0 }} />
|
<AlertCircle size={16} aria-hidden="true" style={{ flexShrink: 0 }} />
|
||||||
This account is temporarily locked. Contact your admin to reset access.
|
Too many failed attempts for this account. Try again in about 15 minutes, or
|
||||||
|
contact your admin to reset access.
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user