fix(auth): add brute-force rate limiting on POST /login (issue #94 bug 6)
This commit is contained in:
parent
bacdb9f49c
commit
6ee284e3f6
1 changed files with 86 additions and 0 deletions
|
|
@ -12,6 +12,78 @@ import pool from '../db/pool.js';
|
|||
|
||||
const router = express.Router();
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// BUG FIX #6: In-memory login rate limiter.
|
||||
//
|
||||
// Brute-force protection for POST /login. Tracks failed attempts per
|
||||
// (IP, username) pair; after MAX_ATTEMPTS failures within WINDOW_MS the
|
||||
// endpoint returns 429 for LOCKOUT_MS regardless of the password supplied.
|
||||
//
|
||||
// This is intentionally simple — no Redis dependency, no persistent state
|
||||
// across restarts. For a production deployment behind a load balancer, use
|
||||
// express-rate-limit with a Redis store or a dedicated WAF rule instead.
|
||||
// ---------------------------------------------------------------------------
|
||||
const MAX_ATTEMPTS = parseInt(process.env.LOGIN_MAX_ATTEMPTS || '10', 10);
|
||||
const WINDOW_MS = parseInt(process.env.LOGIN_WINDOW_MS || String(15 * 60 * 1000), 10); // 15 min
|
||||
const LOCKOUT_MS = parseInt(process.env.LOGIN_LOCKOUT_MS || String(15 * 60 * 1000), 10); // 15 min
|
||||
|
||||
// Map key → { attempts: number, lockedUntil: number | null, firstAttempt: number }
|
||||
const loginAttempts = new Map();
|
||||
|
||||
// Housekeeping: prune expired entries every 10 min so the Map doesn't grow
|
||||
// unboundedly on high-traffic or attack traffic.
|
||||
setInterval(() => {
|
||||
const now = Date.now();
|
||||
for (const [key, entry] of loginAttempts.entries()) {
|
||||
const expired = entry.lockedUntil
|
||||
? now > entry.lockedUntil
|
||||
: now - entry.firstAttempt > WINDOW_MS;
|
||||
if (expired) loginAttempts.delete(key);
|
||||
}
|
||||
}, 10 * 60 * 1000).unref();
|
||||
|
||||
function getAttemptKey(req, username) {
|
||||
// Use the real client IP (trust proxy headers set by nginx/load-balancer)
|
||||
const ip = req.ip || req.socket.remoteAddress || 'unknown';
|
||||
return `${ip}:${(username || '').trim().toLowerCase()}`;
|
||||
}
|
||||
|
||||
function checkRateLimit(req, username) {
|
||||
const key = getAttemptKey(req, username);
|
||||
const now = Date.now();
|
||||
const entry = loginAttempts.get(key);
|
||||
|
||||
if (entry) {
|
||||
// Still locked out?
|
||||
if (entry.lockedUntil && now < entry.lockedUntil) {
|
||||
const retryAfterSec = Math.ceil((entry.lockedUntil - now) / 1000);
|
||||
return { limited: true, retryAfterSec };
|
||||
}
|
||||
// Window expired — reset
|
||||
if (now - entry.firstAttempt > WINDOW_MS) {
|
||||
loginAttempts.delete(key);
|
||||
}
|
||||
}
|
||||
return { limited: false };
|
||||
}
|
||||
|
||||
function recordFailedAttempt(req, username) {
|
||||
const key = getAttemptKey(req, username);
|
||||
const now = Date.now();
|
||||
const entry = loginAttempts.get(key) || { attempts: 0, lockedUntil: null, firstAttempt: now };
|
||||
|
||||
// Don't update firstAttempt if there's an existing entry within the window
|
||||
entry.attempts += 1;
|
||||
if (entry.attempts >= MAX_ATTEMPTS) {
|
||||
entry.lockedUntil = now + LOCKOUT_MS;
|
||||
}
|
||||
loginAttempts.set(key, entry);
|
||||
}
|
||||
|
||||
function clearAttempts(req, username) {
|
||||
loginAttempts.delete(getAttemptKey(req, username));
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// POST /login
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
@ -23,6 +95,15 @@ router.post('/login', async (req, res, next) => {
|
|||
return res.status(400).json({ error: 'Username and password are required' });
|
||||
}
|
||||
|
||||
// BUG FIX #6: Check rate limit before hitting the DB or bcrypt.
|
||||
const rateCheck = checkRateLimit(req, username);
|
||||
if (rateCheck.limited) {
|
||||
res.set('Retry-After', String(rateCheck.retryAfterSec));
|
||||
return res.status(429).json({
|
||||
error: `Too many failed login attempts. Try again in ${rateCheck.retryAfterSec} seconds.`,
|
||||
});
|
||||
}
|
||||
|
||||
const result = await pool.query(
|
||||
'SELECT * FROM users WHERE username = $1',
|
||||
[username.trim().toLowerCase()]
|
||||
|
|
@ -31,6 +112,7 @@ router.post('/login', async (req, res, next) => {
|
|||
if (result.rows.length === 0) {
|
||||
// Timing-safe: still run compare on a dummy hash so response time is constant
|
||||
await bcrypt.compare(password, '$2b$12$invalidhashpadding000000000000000000000000000000000000');
|
||||
recordFailedAttempt(req, username);
|
||||
return res.status(401).json({ error: 'Invalid credentials' });
|
||||
}
|
||||
|
||||
|
|
@ -38,9 +120,13 @@ router.post('/login', async (req, res, next) => {
|
|||
const valid = await bcrypt.compare(password, user.password_hash);
|
||||
|
||||
if (!valid) {
|
||||
recordFailedAttempt(req, username);
|
||||
return res.status(401).json({ error: 'Invalid credentials' });
|
||||
}
|
||||
|
||||
// Successful login — clear any accumulated failed attempts
|
||||
clearAttempts(req, username);
|
||||
|
||||
// Regenerate session ID to prevent fixation attacks
|
||||
req.session.regenerate((err) => {
|
||||
if (err) return next(err);
|
||||
|
|
|
|||
Loading…
Reference in a new issue