From d2a4ee16284d9473aaff2232dddd01d77e54befa Mon Sep 17 00:00:00 2001 From: bradgroux Date: Mon, 7 Sep 2026 16:54:35 -0500 Subject: [PATCH] fix: separate auth context reads from login attempt limits --- .../__tests__/middleware/rate-limit.test.ts | 30 +++++++++++++++++++ server/src/middleware/rate-limit.ts | 10 +++---- server/src/routes/auth.ts | 1 + 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/server/src/__tests__/middleware/rate-limit.test.ts b/server/src/__tests__/middleware/rate-limit.test.ts index b4dc3bd8..9641e76e 100644 --- a/server/src/__tests__/middleware/rate-limit.test.ts +++ b/server/src/__tests__/middleware/rate-limit.test.ts @@ -11,6 +11,7 @@ import { rateLimit, apiRateLimit, authRateLimit, + authStatusRateLimit, writeRateLimit, readRateLimit, uploadRateLimit, @@ -47,6 +48,35 @@ function createUploadLimiter(limit = 2) { // ── Factory tests ────────────────────────────────────────────────────────────── describe('Rate Limit Middleware', () => { + it('keeps ordinary auth context reads separate from the login attempt budget', async () => { + const app = express(); + app.set('trust proxy', 1); + app.use('/auth', authRateLimit); + app.get('/auth/context', authStatusRateLimit, (_req, res) => res.json({ ok: true })); + app.post('/auth/login', (_req, res) => res.json({ ok: true })); + app.post('/auth/context', (_req, res) => res.json({ ok: true })); + const remote = '198.51.100.63'; + for (let i = 0; i < 12; i++) { + const response = await request(app).get('/auth/context').set('X-Forwarded-For', remote); + expect(response.status).toBe(200); + expectConfiguredLimit(response, 120); + } + for (let i = 0; i < 10; i++) { + expect((await request(app).post('/auth/login').set('X-Forwarded-For', remote)).status).toBe( + 200 + ); + } + expect((await request(app).post('/auth/login').set('X-Forwarded-For', remote)).status).toBe( + 429 + ); + expect((await request(app).post('/auth/context').set('X-Forwarded-For', remote)).status).toBe( + 429 + ); + expect((await request(app).get('/auth/context').set('X-Forwarded-For', remote)).status).toBe( + 200 + ); + }); + describe('rateLimit factory', () => { it('should create middleware with default options', () => { const limiter = rateLimit(); diff --git a/server/src/middleware/rate-limit.ts b/server/src/middleware/rate-limit.ts index 558ce20d..8cd0f2ad 100644 --- a/server/src/middleware/rate-limit.ts +++ b/server/src/middleware/rate-limit.ts @@ -51,8 +51,8 @@ function isLocalhost(req: Request): boolean { return ip === '127.0.0.1' || ip === '::1' || ip === '::ffff:127.0.0.1'; } -function isAuthStatusRequest(req: Request): boolean { - return req.method === 'GET' && req.path === '/status'; +function isAuthReadRequest(req: Request): boolean { + return req.method === 'GET' && (req.path === '/status' || req.path === '/context'); } // ── Factory ──────────────────────────────────────────────────────────────────── @@ -112,12 +112,12 @@ export const authRateLimit = rateLimit({ limit: 10, windowMs: 15 * 60_000, // 15 minutes message: 'Too many authentication attempts. Please try again later.', - skip: (req) => isLocalhost(req) || isAuthStatusRequest(req), + skip: (req) => isLocalhost(req) || isAuthReadRequest(req), }); /** - * Read-style limiter for the auth status polling endpoint. - * This endpoint is called on normal route loads and must not consume login/setup attempts. + * Read-style limiter for auth status and authenticated context reads. + * Normal route loads must not consume login/setup attempts. */ export const authStatusRateLimit = rateLimit({ limit: 120, diff --git a/server/src/routes/auth.ts b/server/src/routes/auth.ts index b3d802dc..d937bd83 100644 --- a/server/src/routes/auth.ts +++ b/server/src/routes/auth.ts @@ -238,6 +238,7 @@ router.get( */ router.get( '/context', + authStatusRateLimit, authenticate, asyncHandler(async (req: AuthenticatedRequest, res: Response) => { res.json({