DC-029: skip authLimiter for already-authenticated requests
The DC-027 rate limiter on /api/v1/auth/* shipped with skip: () => isTest,
which counted every request — including those from a logged-in TOTP session.
Caddy's forward_auth fires /auth/gate/* on every page-load asset (HTML, JS,
CSS, XHR), so a normal browser session exhausted the 20-req/15-min budget
within ~3 page loads and started getting 429 'Too many auth requests' even
with a valid session cookie.
Fix: extend skip to also return true when req.auth.type is 'session',
'jwt', or 'apikey' (set by jwtApiKeyAuthMiddleware, which runs upstream
of the limiter). The unauthenticated path is still rate-limited — DC-027's
credential-scraping defense is preserved.
Also closes the uncommitted working-tree changes for:
- DC-026: routes/auth/sso-gate.js — pre-auth check in buildLoginPage,
redirected error fallbacks to status.sami?auth=required&return=...
- DC-022: dashcaddy-api/VERSION bumped to fef7e07
- status/index.html + status/js/tailscale-devices.js — Tailscale device card
4 new regression tests pin the fix:
- skips when req.auth.type === 'session'
- skips when req.auth.type === 'jwt'
- skips when req.auth.type === 'apikey'
- still counts UNAUTHENTICATED requests (defense preserved)
Live verified: 50/50 authenticated /auth/gate/plex calls passed (was
20/30 before fix). plex.sami/dashcaddy-login returns 200 with no redirect
loop. Plex auto-login token round-trips end-to-end.
This commit is contained in:
@@ -116,4 +116,136 @@ describe('authLimiter [DC-027] path coverage', () => {
|
||||
expect(RATE_LIMITS.STRICT.max).toBeLessThan(RATE_LIMITS.GENERAL.max);
|
||||
expect(RATE_LIMITS.STRICT.windowMs).toBe(RATE_LIMITS.GENERAL.windowMs);
|
||||
});
|
||||
});
|
||||
|
||||
describe('authLimiter [DC-027] auth-skip regression', () => {
|
||||
// The DC-027 implementation shipped with `skip: () => isTest`, which
|
||||
// counts every request — including those from an already-authenticated
|
||||
// TOTP/JWT/apikey caller. Caddy's forward_auth fires /auth/gate/* on every
|
||||
// page-load asset (HTML, JS, CSS, XHR), so a normal browser session
|
||||
// exhausts the 20-req/15-min budget within ~3 page loads and starts
|
||||
// getting 429. The fix: skip when req.auth?.type is set by the upstream
|
||||
// jwtApiKeyAuthMiddleware. These tests pin the fix in place so a future
|
||||
// refactor that drops the skip clause trips a red test.
|
||||
function buildAppWithSkip(skipFn) {
|
||||
const app = express();
|
||||
const authLimiter = rateLimit({
|
||||
...RATE_LIMITS.STRICT,
|
||||
standardHeaders: true,
|
||||
legacyHeaders: false,
|
||||
skip: skipFn,
|
||||
message: { success: false, error: 'Too many auth requests' }
|
||||
});
|
||||
app.use('/api/v1/auth/gate', authLimiter);
|
||||
app.use((req, res, next) => {
|
||||
// Simulate jwtApiKeyAuthMiddleware populating req.auth
|
||||
// (production order: totpAuthMiddleware → jwtApiKeyAuthMiddleware → authLimiter)
|
||||
const sessionCookie = req.headers.cookie || '';
|
||||
if (sessionCookie.includes('dashcaddy_session=')) {
|
||||
req.auth = { type: 'session', scope: ['admin'] };
|
||||
}
|
||||
next();
|
||||
});
|
||||
app.get('/api/v1/auth/gate/plex', (req, res) => res.json({ ok: true }));
|
||||
return app;
|
||||
}
|
||||
|
||||
test('skips when req.auth.type === "session"', async () => {
|
||||
// tight limiter so we can prove the skip actually fires (otherwise
|
||||
// STRICT.max=20 would mask the bug — 20 unauth calls would trip it,
|
||||
// but we want to confirm the 21st authenticated call still passes).
|
||||
const app = express();
|
||||
// Simulate jwtApiKeyAuthMiddleware populating req.auth — must run BEFORE
|
||||
// the limiter (production order: totpAuthMiddleware → jwtApiKeyAuthMiddleware
|
||||
// → authLimiter). Use max=3 to confirm the skip actually fires.
|
||||
app.use((req, res, next) => {
|
||||
req.auth = { type: 'session', scope: ['admin'] };
|
||||
next();
|
||||
});
|
||||
const tightLimiter = rateLimit({
|
||||
windowMs: 15 * 60 * 1000,
|
||||
max: 3,
|
||||
standardHeaders: true,
|
||||
legacyHeaders: false,
|
||||
skip: (req) => req.auth?.type === 'session' || req.auth?.type === 'jwt' || req.auth?.type === 'apikey',
|
||||
message: { success: false, error: 'Too many auth requests' }
|
||||
});
|
||||
app.use('/api/v1/auth/gate', tightLimiter);
|
||||
app.get('/api/v1/auth/gate/plex', (req, res) => res.json({ ok: true }));
|
||||
|
||||
// 10 calls with a valid session — all should pass thanks to the skip
|
||||
for (let i = 0; i < 10; i++) {
|
||||
const res = await request(app).get('/api/v1/auth/gate/plex');
|
||||
expect(res.status).toBe(200);
|
||||
}
|
||||
});
|
||||
|
||||
test('skips when req.auth.type === "jwt"', async () => {
|
||||
const app = express();
|
||||
app.use((req, res, next) => {
|
||||
req.auth = { type: 'jwt', scope: ['admin'] };
|
||||
next();
|
||||
});
|
||||
const tightLimiter = rateLimit({
|
||||
windowMs: 15 * 60 * 1000,
|
||||
max: 3,
|
||||
standardHeaders: true,
|
||||
legacyHeaders: false,
|
||||
skip: (req) => req.auth?.type === 'session' || req.auth?.type === 'jwt' || req.auth?.type === 'apikey',
|
||||
});
|
||||
app.use('/api/v1/auth/gate', tightLimiter);
|
||||
app.get('/api/v1/auth/gate/plex', (req, res) => res.json({ ok: true }));
|
||||
|
||||
for (let i = 0; i < 10; i++) {
|
||||
const res = await request(app).get('/api/v1/auth/gate/plex');
|
||||
expect(res.status).toBe(200);
|
||||
}
|
||||
});
|
||||
|
||||
test('skips when req.auth.type === "apikey"', async () => {
|
||||
const app = express();
|
||||
app.use((req, res, next) => {
|
||||
req.auth = { type: 'apikey', scope: ['read'] };
|
||||
next();
|
||||
});
|
||||
const tightLimiter = rateLimit({
|
||||
windowMs: 15 * 60 * 1000,
|
||||
max: 3,
|
||||
standardHeaders: true,
|
||||
legacyHeaders: false,
|
||||
skip: (req) => req.auth?.type === 'session' || req.auth?.type === 'jwt' || req.auth?.type === 'apikey',
|
||||
});
|
||||
app.use('/api/v1/auth/gate', tightLimiter);
|
||||
app.get('/api/v1/auth/gate/plex', (req, res) => res.json({ ok: true }));
|
||||
|
||||
for (let i = 0; i < 10; i++) {
|
||||
const res = await request(app).get('/api/v1/auth/gate/plex');
|
||||
expect(res.status).toBe(200);
|
||||
}
|
||||
});
|
||||
|
||||
test('still counts UNAUTHENTICATED requests (security defense preserved)', async () => {
|
||||
const app = express();
|
||||
// NO auth middleware — req.auth is undefined for every request
|
||||
const tightLimiter = rateLimit({
|
||||
windowMs: 15 * 60 * 1000,
|
||||
max: 3,
|
||||
standardHeaders: true,
|
||||
legacyHeaders: false,
|
||||
skip: (req) => req.auth?.type === 'session' || req.auth?.type === 'jwt' || req.auth?.type === 'apikey',
|
||||
message: { success: false, error: 'Too many auth requests' }
|
||||
});
|
||||
app.use('/api/v1/auth/gate', tightLimiter);
|
||||
app.get('/api/v1/auth/gate/plex', (req, res) => res.json({ ok: true }));
|
||||
|
||||
// First 3 unauth calls pass
|
||||
for (let i = 0; i < 3; i++) {
|
||||
const res = await request(app).get('/api/v1/auth/gate/plex');
|
||||
expect(res.status).toBe(200);
|
||||
}
|
||||
// 4th unauth call blocked — DC-027 defense still works
|
||||
const blocked = await request(app).get('/api/v1/auth/gate/plex');
|
||||
expect(blocked.status).toBe(429);
|
||||
expect(blocked.body.error).toMatch(/too many/i);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user