From 4d44e7045ca24ace4b36eaf231fa15169d36015c Mon Sep 17 00:00:00 2001 From: Jennie Robinson Faber Date: Mon, 27 Apr 2026 19:18:34 +0100 Subject: [PATCH] refactor(rate-limit): delegate auth limiting to handlers, add dev bypass Main's middleware-level auth limiter (5 req / 5 min, IP-only) duplicated the handler-level limiter introduced earlier on this branch (5/hr IP + 3/hr per-email, blocks email enumeration across IPs). Drop the middleware version and let the handlers own it. Added ALLOW_DEV_TEST_ENDPOINTS bypass to the rateLimit utility so parallel E2E runs from 127.0.0.1 don't exhaust per-IP/email budgets, mirroring the existing middleware bypass. Trimmed the obsolete middleware auth test; handler-level coverage lives in tests/server/api/auth-{login,verify}.test.js. Switched IP-isolation test to the payment path so it still exercises the limiter. --- server/middleware/03.rate-limit.js | 12 +------ server/utils/rateLimit.js | 5 +++ tests/server/middleware/rate-limit.test.js | 40 ++++------------------ 3 files changed, 13 insertions(+), 44 deletions(-) diff --git a/server/middleware/03.rate-limit.js b/server/middleware/03.rate-limit.js index 5ee6103..9ad88f4 100644 --- a/server/middleware/03.rate-limit.js +++ b/server/middleware/03.rate-limit.js @@ -1,12 +1,5 @@ import { RateLimiterMemory } from 'rate-limiter-flexible' -// Strict rate limit for auth endpoints -const authLimiter = new RateLimiterMemory({ - points: 5, // 5 requests - duration: 300, // per 5 minutes - keyPrefix: 'rl_auth' -}) - // Moderate rate limit for payment endpoints const paymentLimiter = new RateLimiterMemory({ points: 10, @@ -35,7 +28,6 @@ function getClientIp(event) { || 'unknown' } -const AUTH_PATHS = new Set(['/api/auth/login', '/api/auth/verify']) const PAYMENT_PREFIXES = ['/api/helcim/'] const UPLOAD_PATHS = new Set(['/api/upload/image']) @@ -51,9 +43,7 @@ export default defineEventHandler(async (event) => { const ip = getClientIp(event) try { - if (AUTH_PATHS.has(path)) { - await authLimiter.consume(ip) - } else if (PAYMENT_PREFIXES.some(p => path.startsWith(p))) { + if (PAYMENT_PREFIXES.some(p => path.startsWith(p))) { await paymentLimiter.consume(ip) } else if (UPLOAD_PATHS.has(path)) { await uploadLimiter.consume(ip) diff --git a/server/utils/rateLimit.js b/server/utils/rateLimit.js index 5fb5dda..4057174 100644 --- a/server/utils/rateLimit.js +++ b/server/utils/rateLimit.js @@ -4,6 +4,11 @@ const buckets = new Map() export function rateLimit(key, { max, windowMs }) { + // Bypass in test/dev opt-in mode so parallel E2E runs from a single IP + // (127.0.0.1) don't exhaust per-IP/email budgets. Mirrors the gate used by + // /api/dev/* endpoints and server/middleware/03.rate-limit.js. + if (process.env.ALLOW_DEV_TEST_ENDPOINTS === 'true') return true + const now = Date.now() // Probabilistic sweep: ~1% of calls evict keys whose newest entry has fully diff --git a/tests/server/middleware/rate-limit.test.js b/tests/server/middleware/rate-limit.test.js index 2f81a62..fa165ae 100644 --- a/tests/server/middleware/rate-limit.test.js +++ b/tests/server/middleware/rate-limit.test.js @@ -18,35 +18,6 @@ describe('rate-limit middleware', () => { }) }) - describe('auth endpoint limiting (5 per 5 min)', () => { - it('allows 5 requests then blocks the 6th', async () => { - const ip = '10.0.1.1' - - // First 5 should succeed - for (let i = 0; i < 5; i++) { - const event = createMockEvent({ - method: 'POST', - path: '/api/auth/login', - remoteAddress: ip - }) - await expect(rateLimitMiddleware(event)).resolves.toBeUndefined() - } - - // 6th should be rate limited - const event = createMockEvent({ - method: 'POST', - path: '/api/auth/login', - remoteAddress: ip - }) - await expect(rateLimitMiddleware(event)).rejects.toMatchObject({ - statusCode: 429 - }) - - // Check Retry-After header was set - expect(event._testSetHeaders['retry-after']).toBeDefined() - }) - }) - describe('payment endpoint limiting (10 per min)', () => { it('allows 10 requests then blocks the 11th', async () => { const ip = '10.0.2.1' @@ -68,16 +39,19 @@ describe('rate-limit middleware', () => { await expect(rateLimitMiddleware(event)).rejects.toMatchObject({ statusCode: 429 }) + + // Check Retry-After header was set + expect(event._testSetHeaders['retry-after']).toBeDefined() }) }) describe('IP isolation', () => { it('different IPs have separate rate limit counters', async () => { - // Exhaust limit for IP A - for (let i = 0; i < 5; i++) { + // Exhaust payment limit for IP A + for (let i = 0; i < 10; i++) { const event = createMockEvent({ method: 'POST', - path: '/api/auth/login', + path: '/api/helcim/initialize-payment', remoteAddress: '10.0.3.1' }) await rateLimitMiddleware(event) @@ -86,7 +60,7 @@ describe('rate-limit middleware', () => { // IP B should still be able to make requests const event = createMockEvent({ method: 'POST', - path: '/api/auth/login', + path: '/api/helcim/initialize-payment', remoteAddress: '10.0.3.2' }) await expect(rateLimitMiddleware(event)).resolves.toBeUndefined()