diff --git a/src/web/middleware/auth.ts b/src/web/middleware/auth.ts index 6b372ca5..ee60915f 100644 --- a/src/web/middleware/auth.ts +++ b/src/web/middleware/auth.ts @@ -8,7 +8,7 @@ * - CORS (localhost only) */ -import { FastifyInstance } from 'fastify'; +import type { FastifyInstance, FastifyReply } from 'fastify'; import { randomBytes, timingSafeEqual } from 'node:crypto'; import { StaleExpirationMap } from '../../utils/index.js'; import type { AuthSessionRecord } from '../ports/auth-port.js'; @@ -69,6 +69,13 @@ export function registerAuthMiddleware(app: FastifyInstance, https: boolean): Au const authSessions = state.authSessions; const authFailures = state.authFailures; + function sendAuthRateLimit(reply: FastifyReply, clientIp: string): void { + const remainingMs = authFailures.getRemainingTtl(clientIp) ?? AUTH_FAILURE_WINDOW_MS; + const retryAfterSeconds = Math.max(1, Math.ceil(remainingMs / 1000)); + reply.header('Retry-After', String(retryAfterSeconds)); + reply.code(429).send('Too Many Requests — try again later'); + } + app.addHook('onRequest', (req, reply, done) => { // Hook events come from local Claude Code hooks (curl from localhost) — no auth headers available. // Safe: validated by HookEventSchema, only triggers broadcasts. @@ -90,13 +97,6 @@ export function registerAuthMiddleware(app: FastifyInstance, https: boolean): Au const clientIp = req.ip; - // Rate limit: reject if too many failed attempts from this IP - const failures = authFailures.get(clientIp) ?? 0; - if (failures >= AUTH_FAILURE_MAX) { - reply.code(429).send('Too Many Requests — try again later'); - return; - } - // Check session cookie first (avoids re-sending credentials on every request) // Use get() instead of has() so refreshOnGet extends the TTL on active sessions const sessionToken = req.cookies[AUTH_COOKIE_NAME]; @@ -140,6 +140,13 @@ export function registerAuthMiddleware(app: FastifyInstance, https: boolean): Au return; } + // Rate limit only requests that failed to authenticate on this attempt. + const failures = authFailures.get(clientIp) ?? 0; + if (failures >= AUTH_FAILURE_MAX) { + sendAuthRateLimit(reply, clientIp); + return; + } + // Auth failed — track failure count authFailures.set(clientIp, failures + 1); diff --git a/test/auth-security.test.ts b/test/auth-security.test.ts index 70a0241c..b4387f22 100644 --- a/test/auth-security.test.ts +++ b/test/auth-security.test.ts @@ -10,20 +10,53 @@ * * Port: 3160 (auth tests), 3161 (loopback no-auth tests), 3162 (network override tests) */ -import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, beforeEach, afterEach, vi } from 'vitest'; import { WebServer } from '../src/web/server.js'; +import { TmuxManager } from '../src/tmux-manager.js'; import { SettingsUpdateSchema } from '../src/web/schemas.js'; const AUTH_PORT = 3160; const NOAUTH_PORT = 3161; const NETWORK_OVERRIDE_PORT = 3162; +const AUTH_RATE_LIMIT_PORT = 3220; const TEST_USER = 'admin'; const TEST_PASS = 'test-password-12345'; +vi.spyOn(TmuxManager, 'isTmuxAvailable').mockReturnValue(true); + function basicAuthHeader(user: string, pass: string): string { return 'Basic ' + Buffer.from(`${user}:${pass}`).toString('base64'); } +async function startAuthServer(port: number): Promise<{ server: WebServer; baseUrl: string }> { + process.env.CODEMAN_PASSWORD = TEST_PASS; + process.env.CODEMAN_USERNAME = TEST_USER; + const server = new WebServer(port, false, true); + await server.start(); + return { server, baseUrl: `http://localhost:${port}` }; +} + +async function getSessionCookie(baseUrl: string): Promise { + const res = await fetch(`${baseUrl}/api/status`, { + headers: { Authorization: basicAuthHeader(TEST_USER, TEST_PASS) }, + }); + expect(res.status).toBe(200); + const setCookie = res.headers.get('set-cookie'); + expect(setCookie).toBeTruthy(); + const cookieMatch = setCookie!.match(/codeman_session=([^;]+)/); + expect(cookieMatch).toBeTruthy(); + return `codeman_session=${cookieMatch![1]}`; +} + +async function exhaustAuthFailures(baseUrl: string, prefix: string): Promise { + for (let i = 0; i < 10; i++) { + const res = await fetch(`${baseUrl}/api/status`, { + headers: { Authorization: basicAuthHeader(TEST_USER, `${prefix}-${i}`) }, + }); + expect(res.status).toBe(401); + } +} + describe('Auth Security', () => { let server: WebServer; let baseUrl: string; @@ -155,29 +188,63 @@ describe('Auth Security', () => { }); describe('Rate Limiting', () => { - it('should block after too many failed attempts', async () => { - // Send 10 failed attempts - for (let i = 0; i < 10; i++) { - await fetch(`${baseUrl}/api/status`, { - headers: { Authorization: basicAuthHeader(TEST_USER, 'wrong-' + i) }, - }); - } + let rateServer: WebServer; + let rateBaseUrl: string; - // 11th attempt should be rate-limited - const res = await fetch(`${baseUrl}/api/status`, { - headers: { Authorization: basicAuthHeader(TEST_USER, 'wrong-again') }, - }); - expect(res.status).toBe(429); + beforeEach(async () => { + ({ server: rateServer, baseUrl: rateBaseUrl } = await startAuthServer(AUTH_RATE_LIMIT_PORT)); }); - it('should rate-limit even with correct credentials after lockout', async () => { - // After being rate-limited, even correct credentials should fail - const res = await fetch(`${baseUrl}/api/status`, { + afterEach(async () => { + await rateServer.stop(); + }); + + it('should rate-limit wrong credentials after too many failed attempts', async () => { + await exhaustAuthFailures(rateBaseUrl, 'cod21-wrong'); + + const res = await fetch(`${rateBaseUrl}/api/status`, { + headers: { Authorization: basicAuthHeader(TEST_USER, 'wrong-again') }, + }); + + expect(res.status).toBe(429); + expect(res.headers.get('retry-after')).toMatch(/^\d+$/); + }); + + it('should allow an existing valid session cookie during auth failure lockout', async () => { + const cookie = await getSessionCookie(rateBaseUrl); + await exhaustAuthFailures(rateBaseUrl, 'cod21-cookie'); + + const res = await fetch(`${rateBaseUrl}/api/status`, { + headers: { Cookie: cookie }, + }); + + expect(res.status).toBe(200); + }); + + it('should allow correct credentials to recover from auth failure lockout', async () => { + await exhaustAuthFailures(rateBaseUrl, 'cod21-recover'); + + const res = await fetch(`${rateBaseUrl}/api/status`, { headers: { Authorization: basicAuthHeader(TEST_USER, TEST_PASS) }, }); - // Rate limit is per-IP and the previous test used the same IP - // This test verifies rate limiting isn't bypassed by correct creds - expect(res.status).toBe(429); + + expect(res.status).toBe(200); + expect(res.headers.get('set-cookie')).toContain('codeman_session='); + }); + + it('should clear failed attempt count after correct credentials recover access', async () => { + await exhaustAuthFailures(rateBaseUrl, 'cod21-clear'); + + const recoveryRes = await fetch(`${rateBaseUrl}/api/status`, { + headers: { Authorization: basicAuthHeader(TEST_USER, TEST_PASS) }, + }); + expect(recoveryRes.status).toBe(200); + + const wrongAfterRecovery = await fetch(`${rateBaseUrl}/api/status`, { + headers: { Authorization: basicAuthHeader(TEST_USER, 'wrong-after-recovery') }, + }); + + expect(wrongAfterRecovery.status).toBe(401); }); });