From 1482f6c225641f593527575d9da513fc8b2fe1d4 Mon Sep 17 00:00:00 2001 From: Kalista Payne Date: Wed, 6 May 2026 14:47:33 -0500 Subject: [PATCH] Improve rate limit handling (#15649) * Improve rate limit handling * fix(lint): blank lines --------- Co-authored-by: Phillip Thelen --- test/api/unit/middlewares/rateLimiter.test.js | 36 +++-- website/server/middlewares/appRoutes.js | 18 ++- website/server/middlewares/rateLimiter.js | 133 +++++++++--------- 3 files changed, 108 insertions(+), 79 deletions(-) diff --git a/test/api/unit/middlewares/rateLimiter.test.js b/test/api/unit/middlewares/rateLimiter.test.js index f7f418e5c5..74d8f26417 100644 --- a/test/api/unit/middlewares/rateLimiter.test.js +++ b/test/api/unit/middlewares/rateLimiter.test.js @@ -32,7 +32,8 @@ describe('rateLimiter middleware', () => { it('is disabled when the env var is not defined', () => { nconfGetStub.withArgs('RATE_LIMITER_ENABLED').returns(undefined); - const attachRateLimiter = requireAgain(pathToRateLimiter).default; + const setupRateLimiter = requireAgain(pathToRateLimiter).default; + const attachRateLimiter = setupRateLimiter(); attachRateLimiter(req, res, next); expect(next).to.have.been.calledOnce; @@ -43,7 +44,8 @@ describe('rateLimiter middleware', () => { it('is disabled when the env var is an not "true"', () => { nconfGetStub.withArgs('RATE_LIMITER_ENABLED').returns('false'); - const attachRateLimiter = requireAgain(pathToRateLimiter).default; + const setupRateLimiter = requireAgain(pathToRateLimiter).default; + const attachRateLimiter = setupRateLimiter(); attachRateLimiter(req, res, next); expect(next).to.have.been.calledOnce; @@ -55,7 +57,8 @@ describe('rateLimiter middleware', () => { it('does not throw when there are available points', async () => { nconfGetStub.withArgs('RATE_LIMITER_ENABLED').returns('true'); nconfGetStub.withArgs('RATE_LIMITER_IP_COST').returns(1); - const attachRateLimiter = requireAgain(pathToRateLimiter).default; + const setupRateLimiter = requireAgain(pathToRateLimiter).default; + const attachRateLimiter = setupRateLimiter(); await attachRateLimiter(req, res, next); expect(next).to.have.been.calledOnce; @@ -77,7 +80,8 @@ describe('rateLimiter middleware', () => { sandbox.stub(RateLimiterMemory.prototype, 'consume') .returns(Promise.reject(new Error('Unknown error.'))); - const attachRateLimiter = requireAgain(pathToRateLimiter).default; + const setupRateLimiter = requireAgain(pathToRateLimiter).default; + const attachRateLimiter = setupRateLimiter(); await attachRateLimiter(req, res, next); expect(next).to.have.been.calledOnce; @@ -92,7 +96,8 @@ describe('rateLimiter middleware', () => { it('does not throw when LIVELINESS_PROBE_KEY is correct', async () => { nconfGetStub.withArgs('RATE_LIMITER_ENABLED').returns('true'); nconfGetStub.withArgs('LIVELINESS_PROBE_KEY').returns('abc'); - const attachRateLimiter = requireAgain(pathToRateLimiter).default; + const setupRateLimiter = requireAgain(pathToRateLimiter).default; + const attachRateLimiter = setupRateLimiter(); req.query.liveliness = 'abc'; await attachRateLimiter(req, res, next); @@ -107,7 +112,8 @@ describe('rateLimiter middleware', () => { nconfGetStub.withArgs('RATE_LIMITER_ENABLED').returns('true'); nconfGetStub.withArgs('LIVELINESS_PROBE_KEY').returns('abc'); nconfGetStub.withArgs('RATE_LIMITER_IP_COST').returns(1); - const attachRateLimiter = requireAgain(pathToRateLimiter).default; + const setupRateLimiter = requireAgain(pathToRateLimiter).default; + const attachRateLimiter = setupRateLimiter(); req.query.liveliness = 'das'; await attachRateLimiter(req, res, next); @@ -124,7 +130,8 @@ describe('rateLimiter middleware', () => { nconfGetStub.withArgs('RATE_LIMITER_ENABLED').returns('true'); nconfGetStub.withArgs('LIVELINESS_PROBE_KEY').returns(undefined); nconfGetStub.withArgs('RATE_LIMITER_IP_COST').returns(1); - const attachRateLimiter = requireAgain(pathToRateLimiter).default; + const setupRateLimiter = requireAgain(pathToRateLimiter).default; + const attachRateLimiter = setupRateLimiter(); await attachRateLimiter(req, res, next); @@ -140,7 +147,8 @@ describe('rateLimiter middleware', () => { nconfGetStub.withArgs('RATE_LIMITER_ENABLED').returns('true'); nconfGetStub.withArgs('LIVELINESS_PROBE_KEY').returns(''); nconfGetStub.withArgs('RATE_LIMITER_IP_COST').returns(1); - const attachRateLimiter = requireAgain(pathToRateLimiter).default; + const setupRateLimiter = requireAgain(pathToRateLimiter).default; + const attachRateLimiter = setupRateLimiter(); req.query.liveliness = ''; await attachRateLimiter(req, res, next); @@ -156,7 +164,8 @@ describe('rateLimiter middleware', () => { it('throws when there are no available points remaining', async () => { nconfGetStub.withArgs('RATE_LIMITER_ENABLED').returns('true'); nconfGetStub.withArgs('RATE_LIMITER_IP_COST').returns(1); - const attachRateLimiter = requireAgain(pathToRateLimiter).default; + const setupRateLimiter = requireAgain(pathToRateLimiter).default; + const attachRateLimiter = setupRateLimiter(); // call for 31 times for (let i = 0; i < 31; i += 1) { @@ -180,7 +189,8 @@ describe('rateLimiter middleware', () => { it('uses the user id if supplied or the ip address', async () => { nconfGetStub.withArgs('RATE_LIMITER_ENABLED').returns('true'); nconfGetStub.withArgs('RATE_LIMITER_IP_COST').returns(1); - const attachRateLimiter = requireAgain(pathToRateLimiter).default; + const setupRateLimiter = requireAgain(pathToRateLimiter).default; + const attachRateLimiter = setupRateLimiter(); req.ip = 1; await attachRateLimiter(req, res, next); @@ -210,7 +220,8 @@ describe('rateLimiter middleware', () => { it('applies increased cost for registration calls with and without user id', async () => { nconfGetStub.withArgs('RATE_LIMITER_ENABLED').returns('true'); nconfGetStub.withArgs('RATE_LIMITER_REGISTRATION_COST').returns(3); - const attachRateLimiter = requireAgain(pathToRateLimiter).default; + const setupRateLimiter = requireAgain(pathToRateLimiter).default; + const attachRateLimiter = setupRateLimiter(); req.path = '/api/v4/user/auth/local/register'; req.ip = 1; @@ -241,7 +252,8 @@ describe('rateLimiter middleware', () => { it('applies increased cost for unauthenticated API calls', async () => { nconfGetStub.withArgs('RATE_LIMITER_ENABLED').returns('true'); nconfGetStub.withArgs('RATE_LIMITER_IP_COST').returns(10); - const attachRateLimiter = requireAgain(pathToRateLimiter).default; + const setupRateLimiter = requireAgain(pathToRateLimiter).default; + const attachRateLimiter = setupRateLimiter(); req.ip = 1; await attachRateLimiter(req, res, next); diff --git a/website/server/middlewares/appRoutes.js b/website/server/middlewares/appRoutes.js index eb390e0e77..f4530e8fe7 100644 --- a/website/server/middlewares/appRoutes.js +++ b/website/server/middlewares/appRoutes.js @@ -1,8 +1,9 @@ +import nconf from 'nconf'; import express from 'express'; import expressValidator from 'express-validator'; import path from 'path'; import setupBody from './setupBody'; -import rateLimiter from './rateLimiter'; +import setupRateLimiter from './rateLimiter'; import setupExpress from '../libs/setupExpress'; import * as routes from '../libs/routes'; @@ -10,6 +11,9 @@ const API_V3_CONTROLLERS_PATH = path.join(__dirname, '/../controllers/api-v3/'); const API_V4_CONTROLLERS_PATH = path.join(__dirname, '/../controllers/api-v4/'); const TOP_LEVEL_CONTROLLERS_PATH = path.join(__dirname, '/../controllers/top-level/'); +const RATE_LIMITER_V4_POINTS = nconf.get('RATE_LIMITER_V4_POINTS') || 100; +const RATE_LIMITER_V4_REGISTRATION_COST = nconf.get('RATE_LIMITER_V4_REGISTRATION_COST') || 10; + const app = express(); // re-set the view options because they are not inherited from the top level app @@ -25,7 +29,10 @@ app.use('/', topLevelRouter); const v3Router = express.Router(); // eslint-disable-line new-cap routes.walkControllers(v3Router, API_V3_CONTROLLERS_PATH); -app.use('/api/v3', rateLimiter, v3Router); +const v3RateLimiter = setupRateLimiter({ + keyPrefix: 'api-v3', +}); +app.use('/api/v3', v3RateLimiter, v3Router); // API v4 proxies API v3 routes by default. // It can also disable or override v3 routes @@ -49,6 +56,11 @@ const v4RouterOverrides = [ const v4Router = express.Router(); // eslint-disable-line new-cap routes.walkControllers(v4Router, API_V3_CONTROLLERS_PATH, v4RouterOverrides); routes.walkControllers(v4Router, API_V4_CONTROLLERS_PATH); -app.use('/api/v4', v4Router); +const v4RateLimiter = setupRateLimiter({ + keyPrefix: 'api-v4', + points: RATE_LIMITER_V4_POINTS, + registrationCost: RATE_LIMITER_V4_REGISTRATION_COST, +}); +app.use('/api/v4', v4RateLimiter, v4Router); export default app; diff --git a/website/server/middlewares/rateLimiter.js b/website/server/middlewares/rateLimiter.js index b47e9e9eec..f6d615d6ef 100644 --- a/website/server/middlewares/rateLimiter.js +++ b/website/server/middlewares/rateLimiter.js @@ -23,55 +23,39 @@ const REDIS_HOST = nconf.get('REDIS_HOST'); const REDIS_PASSWORD = nconf.get('REDIS_PASSWORD'); const REDIS_PORT = nconf.get('REDIS_PORT'); const LIVELINESS_PROBE_KEY = nconf.get('LIVELINESS_PROBE_KEY'); +const BASE_POINTS = nconf.get('RATE_LIMITER_BASE_POINTS') || 30; +const BASE_DURATION = nconf.get('RATE_LIMITER_BASE_DURATION') || 60; const REGISTRATION_COST = nconf.get('RATE_LIMITER_REGISTRATION_COST') || 5; const IP_RATE_LIMIT_COST = nconf.get('RATE_LIMITER_IP_COST') || 5; let redisClient; -let rateLimiter; -const rateLimiterOpts = { - keyPrefix: 'api-v3', - points: 30, // 30 requests - duration: 60, // per 1 minute by User ID or IP -}; +if (RATE_LIMITER_ENABLED && !IS_TEST) { + redisClient = redis.createClient({ + host: REDIS_HOST, + password: REDIS_PASSWORD, + port: REDIS_PORT, + enable_offline_queue: false, + }); -if (RATE_LIMITER_ENABLED) { - if (IS_TEST) { - rateLimiter = new RateLimiterMemory({ - ...rateLimiterOpts, - }); - } else { - redisClient = redis.createClient({ - host: REDIS_HOST, - password: REDIS_PASSWORD, - port: REDIS_PORT, - enable_offline_queue: false, - }); + redisClient.on('ready', () => { + SERVER_STATUS.REDIS = true; + }); - redisClient.on('ready', () => { - SERVER_STATUS.REDIS = true; - }); + redisClient.on('reconnecting', () => { + SERVER_STATUS.REDIS = false; + }); - redisClient.on('reconnecting', () => { - SERVER_STATUS.REDIS = false; - }); - - redisClient.on('error', error => { - logger.error(error, 'Redis Error'); - }); - - rateLimiter = new RateLimiterRedis({ - ...rateLimiterOpts, - storeClient: redisClient, - }); - } + redisClient.on('error', error => { + logger.error(error, 'Redis Error'); + }); } else { SERVER_STATUS.REDIS = true; } -function setResponseHeaders (res, rateLimiterRes) { +function setResponseHeaders (res, points, rateLimiterRes) { const headers = { - 'X-RateLimit-Limit': rateLimiterOpts.points, + 'X-RateLimit-Limit': points, 'X-RateLimit-Remaining': rateLimiterRes.remainingPoints, 'X-RateLimit-Reset': new Date(Date.now() + rateLimiterRes.msBeforeNext), }; @@ -83,34 +67,55 @@ function setResponseHeaders (res, rateLimiterRes) { res.set(headers); } -export default function rateLimiterMiddleware (req, res, next) { - if (!RATE_LIMITER_ENABLED) return next(); - if (LIVELINESS_PROBE_KEY && req.query.liveliness === LIVELINESS_PROBE_KEY) return next(); - - const userId = req.header('x-api-user'); - - let cost = 1; - if (req.path === '/api/v4/user/auth/local/register' || req.path === '/api/v3/user/auth/local/register') { - cost = REGISTRATION_COST; - } else if (!userId) { - cost = IP_RATE_LIMIT_COST; +export default function setupRateLimiter (options = {}) { + const rateLimiterOpts = { + keyPrefix: options.keyPrefix || 'api', + points: options.points || BASE_POINTS, // 30 requests + duration: options.duration || BASE_DURATION, // per 1 minute by User ID or IP + }; + let rateLimiter; + if (!RATE_LIMITER_ENABLED) { + return (req, res, next) => next(); } - - return rateLimiter.consume(userId || req.ip, cost) - .then(rateLimiterRes => { - setResponseHeaders(res, rateLimiterRes); - return next(); - }) - .catch(rateLimiterRes => { - if (rateLimiterRes instanceof RateLimiterRes) { - setResponseHeaders(res, rateLimiterRes); - return next(new TooManyRequests(apiError('clientRateLimited'))); - } - - // In case of an unhandled error we skip the middleware as it could mean - // , for example, that the connection to the redis database is not working. - // We do not want to block all requests in these cases. - logger.error(rateLimiterRes, 'Rate Limiter Error'); - return next(); + if (IS_TEST) { + rateLimiter = new RateLimiterMemory({ + ...rateLimiterOpts, }); + } else { + rateLimiter = new RateLimiterRedis({ + ...rateLimiterOpts, + storeClient: redisClient, + }); + } + return function rateLimiterMiddleware (req, res, next) { + if (!RATE_LIMITER_ENABLED) return next(); + if (LIVELINESS_PROBE_KEY && req.query.liveliness === LIVELINESS_PROBE_KEY) return next(); + + const userId = req.header('x-api-user'); + + let cost = 1; + if (req.path === '/api/v4/user/auth/local/register' || req.path === '/api/v3/user/auth/local/register') { + cost = options.registrationCost || REGISTRATION_COST; + } else if (!userId) { + cost = options.ipRateLimitCost || IP_RATE_LIMIT_COST; + } + + return rateLimiter.consume(userId || req.ip, cost) + .then(rateLimiterRes => { + setResponseHeaders(res, rateLimiterOpts.points, rateLimiterRes); + return next(); + }) + .catch(rateLimiterRes => { + if (rateLimiterRes instanceof RateLimiterRes) { + setResponseHeaders(res, rateLimiterOpts.points, rateLimiterRes); + return next(new TooManyRequests(apiError('clientRateLimited'))); + } + + // In case of an unhandled error we skip the middleware as it could mean + // , for example, that the connection to the redis database is not working. + // We do not want to block all requests in these cases. + logger.error(rateLimiterRes, 'Rate Limiter Error'); + return next(); + }); + }; }