fix(security): rate-limit the password-change endpoints per IP too

POST /api/auth/admin/change-password and POST /api/customer/profile/password
both verify the current password before replacing it, which makes them a
credential check an attacker holding a hijacked session can drive at will:
the session's own JWT skips the general limiter as authenticated, and they
were not in the auth gate's table. Both join it. Only failures count, so
the one change a user legitimately makes costs nothing.
This commit is contained in:
Paul Nothaft
2026-09-02 09:43:11 +02:00
parent 4515632300
commit 5a0c9f53b0
2 changed files with 15 additions and 3 deletions
@@ -64,6 +64,9 @@ function mountRoutes(app) {
app.post('/api/setup/admin', (req, res) => fail(res)); app.post('/api/setup/admin', (req, res) => fail(res));
app.post('/api/customer/auth/login', (req, res) => fail(res)); app.post('/api/customer/auth/login', (req, res) => fail(res));
app.post('/api/customer/auth/password-reset', (req, res) => fail(res)); app.post('/api/customer/auth/password-reset', (req, res) => fail(res));
// Password changes verify the current password before replacing it.
app.post('/api/auth/admin/change-password', (req, res) => fail(res));
app.post('/api/customer/profile/password', (req, res) => fail(res));
// Benign endpoints living under the very same prefixes the old registrations // Benign endpoints living under the very same prefixes the old registrations
// covered. Every one of these is called more than five times per window by a // covered. Every one of these is called more than five times per window by a
@@ -72,8 +75,8 @@ function mountRoutes(app) {
app.post('/api/auth/password-strength', (req, res) => res.json({ score: 3 })); app.post('/api/auth/password-strength', (req, res) => res.json({ score: 3 }));
app.post('/api/auth/logout', (req, res) => res.json({ ok: true })); app.post('/api/auth/logout', (req, res) => res.json({ ok: true }));
app.post('/api/auth/gallery/logout', (req, res) => res.json({ ok: true })); app.post('/api/auth/gallery/logout', (req, res) => res.json({ ok: true }));
app.post('/api/auth/admin/change-password', (req, res) => res.json({ ok: true }));
app.get('/api/auth/admin/sso/callback', (req, res) => res.json({ ok: true })); app.get('/api/auth/admin/sso/callback', (req, res) => res.json({ ok: true }));
app.get('/api/customer/profile', (req, res) => res.json({ ok: true }));
app.get('/api/setup/status', (req, res) => res.json({ needsSetup: false })); app.get('/api/setup/status', (req, res) => res.json({ needsSetup: false }));
app.get('/api/customer/auth/session', (req, res) => res.json({ ok: true })); app.get('/api/customer/auth/session', (req, res) => res.json({ ok: true }));
app.get('/api/gallery/:slug/verify-token/:token', (req, res) => res.json({ valid: true })); app.get('/api/gallery/:slug/verify-token/:token', (req, res) => res.json({ valid: true }));
@@ -121,7 +124,9 @@ describe('authRateLimitGate — credential endpoints are limited', () => {
['/api/setup/verify-token'], ['/api/setup/verify-token'],
['/api/setup/admin'], ['/api/setup/admin'],
['/api/customer/auth/login'], ['/api/customer/auth/login'],
['/api/customer/auth/password-reset'] ['/api/customer/auth/password-reset'],
['/api/auth/admin/change-password'],
['/api/customer/profile/password']
])('429s %s once the budget is spent', async (endpoint) => { ])('429s %s once the budget is spent', async (endpoint) => {
const app = await buildApp(); const app = await buildApp();
for (let i = 0; i < 5; i++) { for (let i = 0; i < 5; i++) {
@@ -195,8 +200,8 @@ describe('authRateLimitGate — benign endpoints are never limited', () => {
['POST', '/api/auth/password-strength'], ['POST', '/api/auth/password-strength'],
['POST', '/api/auth/logout'], ['POST', '/api/auth/logout'],
['POST', '/api/auth/gallery/logout'], ['POST', '/api/auth/gallery/logout'],
['POST', '/api/auth/admin/change-password'],
['GET', '/api/auth/admin/sso/callback'], ['GET', '/api/auth/admin/sso/callback'],
['GET', '/api/customer/profile'],
['GET', '/api/setup/status'], ['GET', '/api/setup/status'],
['GET', '/api/customer/auth/session'], ['GET', '/api/customer/auth/session'],
['GET', '/api/gallery/some-slug/verify-token/abc'] ['GET', '/api/gallery/some-slug/verify-token/abc']
@@ -58,6 +58,13 @@ const CREDENTIAL_ENDPOINTS = [
// Customer portal password, and the reset that replaces it. // Customer portal password, and the reset that replaces it.
{ method: 'POST', path: /^\/api\/customer\/auth\/login\/?$/i }, { method: 'POST', path: /^\/api\/customer\/auth\/login\/?$/i },
{ method: 'POST', path: /^\/api\/customer\/auth\/password-reset\/?$/i }, { method: 'POST', path: /^\/api\/customer\/auth\/password-reset\/?$/i },
// Password changes verify the CURRENT password first, so they are a
// credential check too — one an attacker holding a hijacked session can
// drive, and one the general limiter never sees because the session's own
// JWT skips it as authenticated. Only failed attempts count here, so the
// one legitimate change a user makes costs nothing.
{ method: 'POST', path: /^\/api\/auth\/admin\/change-password\/?$/i },
{ method: 'POST', path: /^\/api\/customer\/profile\/password\/?$/i },
]; ];
/** /**