diff --git a/backend/lib/express/rate-limit.js b/backend/lib/express/rate-limit.js index baa34493d..a8754a826 100644 --- a/backend/lib/express/rate-limit.js +++ b/backend/lib/express/rate-limit.js @@ -4,24 +4,57 @@ import { isCI } from "../config.js"; * In-memory sliding window rate limiter for authentication endpoints */ -const ipRequests = new Map(); +// One store per limiter, so separate endpoints never share buckets +const stores = new Set(); // Clean up old entries every 2 minutes const cleanupTimer = setInterval(() => { const now = Date.now(); - for (const [ip, record] of ipRequests.entries()) { - if (now - record.startTime > record.windowMs * 2) { - ipRequests.delete(ip); + for (const store of stores) { + for (const [key, record] of store.entries()) { + if (now - record.startTime > record.windowMs * 2) { + store.delete(key); + } } } }, 120000); cleanupTimer.unref(); +/** + * The client IP as seen by the backend. X-Forwarded-For is only honoured when the socket is + * loopback, which is the admin nginx in this container; it sets the header to its own + * $remote_addr after realip processing, so the first entry is the resolved client. + * + * @param {Object} req + * @returns {String} + */ +export const getClientIp = (req) => { + const socketAddr = req.socket?.remoteAddress || "unknown"; + const isTrustedLocalProxy = socketAddr === "127.0.0.1" || socketAddr === "::1" || socketAddr === "::ffff:127.0.0.1"; + const forwardedFor = isTrustedLocalProxy ? req.headers["x-forwarded-for"]?.split(",")[0].trim() : null; + return forwardedFor || socketAddr; +}; + +/** + * @param {Object} [options] + * @param {Number} [options.windowMs] + * @param {Number} [options.max] + * @param {String} [options.message] + * @param {Boolean} [options.skipSuccessfulRequests] + * @param {Function} [options.keyGenerator] (req, clientIp) => String. Defaults to the client IP. + * Including the target account in the key stops every + * user being locked out when many clients share an IP + * (e.g. the admin UI behind another reverse proxy). + */ export default function createRateLimiter(options = {}) { const windowMs = options.windowMs || 60 * 1000; // 1 minute default const max = options.max || 10; // 10 requests default const message = options.message || "Too many attempts from this IP, please try again later."; const skipSuccessfulRequests = options.skipSuccessfulRequests !== false; + const keyGenerator = options.keyGenerator || ((_req, clientIp) => clientIp); + + const store = new Map(); + stores.add(store); return (req, res, next) => { // Bypass rate limiting in CI, test environments, or when explicitly disabled @@ -34,14 +67,10 @@ export default function createRateLimiter(options = {}) { return next(); } - const socketAddr = req.socket?.remoteAddress || "unknown"; - const isTrustedLocalProxy = - socketAddr === "127.0.0.1" || socketAddr === "::1" || socketAddr === "::ffff:127.0.0.1"; - const forwardedFor = isTrustedLocalProxy ? req.headers["x-forwarded-for"]?.split(",")[0].trim() : null; - const clientIp = forwardedFor || socketAddr; + const key = keyGenerator(req, getClientIp(req)); const now = Date.now(); - let record = ipRequests.get(clientIp); + let record = store.get(key); if (!record || now - record.startTime > windowMs) { record = { @@ -49,7 +78,7 @@ export default function createRateLimiter(options = {}) { startTime: now, windowMs, }; - ipRequests.set(clientIp, record); + store.set(key, record); if (skipSuccessfulRequests) { const currentRecord = record; diff --git a/backend/routes/tokens.js b/backend/routes/tokens.js index b5f25890c..012bb95ca 100644 --- a/backend/routes/tokens.js +++ b/backend/routes/tokens.js @@ -6,11 +6,40 @@ import apiValidator from "../lib/validator/api.js"; import { debug, express as logger } from "../logger.js"; import { getValidationSchema } from "../schema/index.js"; +// Keyed on IP + target account rather than IP alone: when the admin UI sits behind another +// proxy every client can resolve to the same IP, and an IP-only bucket would let anyone lock +// out every user with a handful of bad logins. Brute force against one account is still +// capped per IP. const tokenRateLimiter = createRateLimiter({ windowMs: 60 * 1000, max: 10, message: "Too many login attempts. Please try again in 1 minute.", skipSuccessfulRequests: true, + keyGenerator: (req, clientIp) => { + const identity = typeof req.body?.identity === "string" ? req.body.identity.trim().toLowerCase() : ""; + return `${clientIp}|${identity}`; + }, +}); + +// Keyed on the user the challenge belongs to, not the challenge token itself, so fetching +// fresh challenge tokens doesn't reset the code-guessing budget. The payload is read without +// verifying the signature; verify2FA does that. A forged token only affects buckets for the +// sender's own IP. +const challengeUserId = (token) => { + try { + const payload = JSON.parse(Buffer.from(String(token).split(".")[1], "base64url").toString()); + return String(payload?.attrs?.id ?? ""); + } catch { + return ""; + } +}; + +const twoFactorRateLimiter = createRateLimiter({ + windowMs: 60 * 1000, + max: 10, + message: "Too many login attempts. Please try again in 1 minute.", + skipSuccessfulRequests: true, + keyGenerator: (req, clientIp) => `${clientIp}|${challengeUserId(req.body?.challenge_token)}`, }); const router = express.Router({ @@ -72,7 +101,7 @@ router * * Verify 2FA code and get full token */ - .post(tokenRateLimiter, async (req, res, next) => { + .post(twoFactorRateLimiter, async (req, res, next) => { try { const { challenge_token, code } = await apiValidator(getValidationSchema("/tokens/2fa", "post"), req.body); const result = await internalToken.verify2FA(challenge_token, code); diff --git a/docker/rootfs/etc/nginx/nginx.conf b/docker/rootfs/etc/nginx/nginx.conf index 471d22536..2184049c4 100644 --- a/docker/rootfs/etc/nginx/nginx.conf +++ b/docker/rootfs/etc/nginx/nginx.conf @@ -77,6 +77,10 @@ http { # Real IP Determination + # Loopback: lets an NPM proxy host forwarding to 127.0.0.1 (e.g. the admin UI on :81) + # pass the real client IP through instead of every request appearing as 127.0.0.1 + set_real_ip_from 127.0.0.1; + set_real_ip_from ::1; # Local subnets: set_real_ip_from 10.0.0.0/8; set_real_ip_from 172.16.0.0/12; # Includes Docker subnet