fix: separate auth context reads from login attempt limits

This commit is contained in:
bradgroux 2026-09-07 16:54:35 -05:00
parent 09e302c4c6
commit d2a4ee1628
3 changed files with 36 additions and 5 deletions

View file

@ -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();

View file

@ -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,

View file

@ -238,6 +238,7 @@ router.get(
*/
router.get(
'/context',
authStatusRateLimit,
authenticate,
asyncHandler(async (req: AuthenticatedRequest, res: Response) => {
res.json({