refactor(rate-limit): delegate auth limiting to handlers, add dev bypass
Some checks failed
Test / playwright (push) Blocked by required conditions
Test / Notify on failure (push) Blocked by required conditions
Test / visual (push) Blocked by required conditions
Test / vitest (push) Has been cancelled

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.
This commit is contained in:
Jennie Robinson Faber 2026-04-27 19:18:34 +01:00
parent c1367ebd29
commit 4d44e7045c
3 changed files with 13 additions and 44 deletions

View file

@ -1,12 +1,5 @@
import { RateLimiterMemory } from 'rate-limiter-flexible' 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 // Moderate rate limit for payment endpoints
const paymentLimiter = new RateLimiterMemory({ const paymentLimiter = new RateLimiterMemory({
points: 10, points: 10,
@ -35,7 +28,6 @@ function getClientIp(event) {
|| 'unknown' || 'unknown'
} }
const AUTH_PATHS = new Set(['/api/auth/login', '/api/auth/verify'])
const PAYMENT_PREFIXES = ['/api/helcim/'] const PAYMENT_PREFIXES = ['/api/helcim/']
const UPLOAD_PATHS = new Set(['/api/upload/image']) const UPLOAD_PATHS = new Set(['/api/upload/image'])
@ -51,9 +43,7 @@ export default defineEventHandler(async (event) => {
const ip = getClientIp(event) const ip = getClientIp(event)
try { try {
if (AUTH_PATHS.has(path)) { if (PAYMENT_PREFIXES.some(p => path.startsWith(p))) {
await authLimiter.consume(ip)
} else if (PAYMENT_PREFIXES.some(p => path.startsWith(p))) {
await paymentLimiter.consume(ip) await paymentLimiter.consume(ip)
} else if (UPLOAD_PATHS.has(path)) { } else if (UPLOAD_PATHS.has(path)) {
await uploadLimiter.consume(ip) await uploadLimiter.consume(ip)

View file

@ -4,6 +4,11 @@
const buckets = new Map() const buckets = new Map()
export function rateLimit(key, { max, windowMs }) { 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() const now = Date.now()
// Probabilistic sweep: ~1% of calls evict keys whose newest entry has fully // Probabilistic sweep: ~1% of calls evict keys whose newest entry has fully

View file

@ -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)', () => { describe('payment endpoint limiting (10 per min)', () => {
it('allows 10 requests then blocks the 11th', async () => { it('allows 10 requests then blocks the 11th', async () => {
const ip = '10.0.2.1' const ip = '10.0.2.1'
@ -68,16 +39,19 @@ describe('rate-limit middleware', () => {
await expect(rateLimitMiddleware(event)).rejects.toMatchObject({ await expect(rateLimitMiddleware(event)).rejects.toMatchObject({
statusCode: 429 statusCode: 429
}) })
// Check Retry-After header was set
expect(event._testSetHeaders['retry-after']).toBeDefined()
}) })
}) })
describe('IP isolation', () => { describe('IP isolation', () => {
it('different IPs have separate rate limit counters', async () => { it('different IPs have separate rate limit counters', async () => {
// Exhaust limit for IP A // Exhaust payment limit for IP A
for (let i = 0; i < 5; i++) { for (let i = 0; i < 10; i++) {
const event = createMockEvent({ const event = createMockEvent({
method: 'POST', method: 'POST',
path: '/api/auth/login', path: '/api/helcim/initialize-payment',
remoteAddress: '10.0.3.1' remoteAddress: '10.0.3.1'
}) })
await rateLimitMiddleware(event) await rateLimitMiddleware(event)
@ -86,7 +60,7 @@ describe('rate-limit middleware', () => {
// IP B should still be able to make requests // IP B should still be able to make requests
const event = createMockEvent({ const event = createMockEvent({
method: 'POST', method: 'POST',
path: '/api/auth/login', path: '/api/helcim/initialize-payment',
remoteAddress: '10.0.3.2' remoteAddress: '10.0.3.2'
}) })
await expect(rateLimitMiddleware(event)).resolves.toBeUndefined() await expect(rateLimitMiddleware(event)).resolves.toBeUndefined()