Archive/screentinker
1
0
Fork 0
mirror of https://github.com/screentinker/screentinker.git synced 2026-06-22 05:54:02 -06:00
Code Issues Actions Packages Projects Releases Wiki Activity

fix(server): rate-limit per endpoint, not the stripped req.path (#100)

Browse source 
app.use('/api/auth/login', rateLimit(...)) etc. keyed on req.path, which Express strips to
'/' for mounted middleware - so /login, /register, /totp/verify shared ONE per-IP counter
(coupled limits; the new /totp/verify brute-force limit was not actually independent). Key
on originalUrl instead. Also adds the /api/auth/totp/verify 10/min limit (tightening #2).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
ScreenTinker 2026-06-13 20:36:10 -05:00 committed by screentinker
parent c02086e305
commit c38d8dc0e6
1 changed files with 10 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
server/server.js
View file

@ -271,7 +271,12 @@ app.use('/socket.io-client', express.static(
const rateLimits = new Map(); const rateLimits = new Map();
function rateLimit(windowMs, maxRequests) { function rateLimit(windowMs, maxRequests) {
return (req, res, next) => { return (req, res, next) => {
const key = getClientIp(req) + req.path; // #100: key on the FULL path, not req.path. These limiters are mounted via
// app.use('/api/auth/login', ...) etc., and Express strips the mount path, so
// req.path was '/' for ALL of them - i.e. /login, /register, /totp/verify shared
// ONE per-IP counter (coupled limits; the /totp/verify brute-force limit wasn't
// actually independent). originalUrl keeps each endpoint's limit separate.
const key = getClientIp(req) + (req.originalUrl || req.url || req.path).split('?')[0];
const now = Date.now(); const now = Date.now();
const windowStart = now - windowMs; const windowStart = now - windowMs;
let hits = rateLimits.get(key) || []; let hits = rateLimits.get(key) || [];
@ -292,6 +297,10 @@ function rateLimit(windowMs, maxRequests) {
// Auth routes (public, rate limited) // Auth routes (public, rate limited)
app.use('/api/auth/login', rateLimit(60000, 10)); // 10 attempts per minute app.use('/api/auth/login', rateLimit(60000, 10)); // 10 attempts per minute
app.use('/api/auth/register', rateLimit(60000, 5)); // 5 registrations per minute app.use('/api/auth/register', rateLimit(60000, 5)); // 5 registrations per minute
// #100 (tightening #2): the TOTP verify endpoint is the brute-force surface for a
// 6-digit code. Cap attempts/min here; the per-user lockout (lib/totp-lockout) sits
// on top in the handler.
app.use('/api/auth/totp/verify', rateLimit(60000, 10));
// Admin password-reset endpoint: even if an admin's session is compromised, // Admin password-reset endpoint: even if an admin's session is compromised,
// cap the blast radius to 20 resets/min/IP. Express matches the longest // cap the blast radius to 20 resets/min/IP. Express matches the longest
// path prefix first, so this fires before /api/auth catches the request. // path prefix first, so this fires before /api/auth catches the request.