diff --git a/backend/src/handlers/admin.rs b/backend/src/handlers/admin.rs index bcc424a..20dfc39 100644 --- a/backend/src/handlers/admin.rs +++ b/backend/src/handlers/admin.rs @@ -132,6 +132,11 @@ pub async fn patch_config( "feed_rate_enabled", "export_rate_enabled", "join_rate_enabled", + // These two per-area rate toggles are HONOURED by their handlers (auth/handlers.rs reads + // `admin_login_rate_enabled` and `recover_rate_enabled`, both defaulting true) but were + // missing from this allowlist — so the switch existed in code and could never be flipped. + "admin_login_rate_enabled", + "recover_rate_enabled", "quota_enabled", "storage_quota_enabled", "upload_count_quota_enabled", diff --git a/e2e/specs/07-adversarial/auth-tampering.spec.ts b/e2e/specs/07-adversarial/auth-tampering.spec.ts index 9403889..eab23ae 100644 --- a/e2e/specs/07-adversarial/auth-tampering.spec.ts +++ b/e2e/specs/07-adversarial/auth-tampering.spec.ts @@ -7,6 +7,7 @@ * tampered signature, expired sessions, wrong role. */ import { test, expect } from '../../fixtures/test'; +import { ADMIN_PASSWORD } from '../../fixtures/api-client'; const BASE = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101'; @@ -144,20 +145,96 @@ test.describe('Adversarial — PIN brute-force', () => { }); test.describe('Adversarial — admin password brute-force', () => { - test('repeated wrong passwords do NOT lock the admin (documented finding)', async () => { - // The admin login handler does not currently implement lockout. This test - // documents the behavior so any future change is intentional. - const attempts = await Promise.all( - Array.from({ length: 10 }, () => - fetch(`${BASE}/api/v1/admin/login`, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ password: 'wrong-' + Math.random() }), - }) - ) - ); - const statuses = attempts.map((r) => r.status); - expect(statuses.every((s) => s === 401)).toBe(true); - console.warn('[finding] /admin/login has no rate-limit or lockout — bcrypt cost is the only defense.'); + // These tests deliberately exhaust the admin-login limiter for the shared test IP. That creates a + // chicken-and-egg for the NEXT test: its truncate auto-fixture must itself call admin_login before + // it can reset the counter — so a still-full window would lock the fixture out with 429 before it + // could clear anything. Disable the toggle here via patchConfig (which authenticates with a JWT, + // NOT the rate-limited admin_login path), so the next login is clear; truncate then wipes the map. + // Runs even if the test body failed, so a failure can't poison the rest of the run. + test.afterEach(async ({ api, adminToken }) => { + await api.patchConfig(adminToken, { admin_login_rate_enabled: 'false' }); + }); + + const tryLogin = (password: string) => + fetch(`${BASE}/api/v1/admin/login`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ password }), + }); + + test('admin login is IP rate-limited: a burst of wrong passwords starts returning 429', async ({ + api, + adminToken, + }) => { + // `admin_login` HAS a 5/min/IP throttle (auth/handlers.rs), gated by the master + // `rate_limits_enabled` + `admin_login_rate_enabled`. Both default TRUE in production — but the + // e2e reseed turns the master switch OFF for every test, so this defence ran in ZERO tests. + // (The previous version of this test observed all-401 under that disabled switch and wrongly + // "documented" that no rate limit exists.) Turn it on and prove it. + // + // patchConfig uses the already-minted adminToken (a JWT), so enabling the limiter does not lock + // us out of configuring it. + await api.patchConfig(adminToken, { + rate_limits_enabled: 'true', + admin_login_rate_enabled: 'true', + }); + + // Sequential, not parallel: the counter increments deterministically so the 429 is not itself + // subject to the counter race the PIN test covers. + const statuses: number[] = []; + for (let i = 0; i < 10; i++) { + statuses.push((await tryLogin('wrong-' + i)).status); + if (statuses[i] === 429) break; + } + + // The password path actually ran (budget existed) before the limiter engaged... + expect(statuses[0], 'first attempt should be a normal wrong-password 401, not a spurious 429').toBe(401); + // ...and the limiter DID engage within the window. Delete the throttle and this is never true. + expect( + statuses.some((s) => s === 429), + 'a burst of wrong admin passwords from one IP must start being rate-limited (429)' + ).toBe(true); + // No wrong password ever authenticated. + expect(statuses.some((s) => s === 200)).toBe(false); + }); + + test('once throttled, even the CORRECT admin password is refused (it is an IP limit, not a password check)', async ({ + api, + adminToken, + }) => { + // This is the assertion that makes the test non-vacuous: it isolates the RATE LIMIT from the + // password logic. If the throttle were removed, the correct password would return 200 here. + await api.patchConfig(adminToken, { + rate_limits_enabled: 'true', + admin_login_rate_enabled: 'true', + }); + + // Exhaust the window with wrong passwords until throttled. + let throttled = false; + for (let i = 0; i < 10 && !throttled; i++) { + throttled = (await tryLogin('wrong-' + i)).status === 429; + } + expect(throttled, 'the IP should be throttled after a burst').toBe(true); + + // The right password, while throttled, must STILL be refused — the limiter is checked before + // the bcrypt verify, so a valid credential does not buy a way around a brute-force lockout. + expect((await tryLogin(ADMIN_PASSWORD)).status, 'a throttled IP is refused even with the correct password').toBe(429); + }); + + test('the throttle is gated: with the limiter disabled, a burst is NOT rate-limited', async ({ + api, + adminToken, + }) => { + // The mirror of the above — proves the toggle actually gates the behaviour (and documents that + // the e2e default really is "off", which is why every OTHER admin test can hammer login freely). + await api.patchConfig(adminToken, { + rate_limits_enabled: 'true', + admin_login_rate_enabled: 'false', + }); + + const statuses: number[] = []; + for (let i = 0; i < 8; i++) statuses.push((await tryLogin('wrong-' + i)).status); + + expect(statuses.every((s) => s === 401), 'with admin_login_rate_enabled=false no attempt should be 429').toBe(true); }); });