From 79631ce62460a03143f76b82810a1516d52fca1d Mon Sep 17 00:00:00 2001 From: Bobby Wibowo Date: Tue, 12 Jul 2022 08:48:09 +0700 Subject: [PATCH] feat: RateLimiter custom middleware class this adds new production dependency rate-limiter-flexible this deprecates old rateLimits option in config to use the new rate limiters, the new option is named rateLimiters and rateLimitersWhitelist please consult config.sample.js rate limiters will also be now processed before any other middlewares, as only makes sense --- config.sample.js | 78 ++++++++++++-------------- controllers/middlewares/rateLimiter.js | 56 ++++++++++++++++++ lolisafe.js | 30 +++++----- package.json | 1 + yarn.lock | 5 ++ 5 files changed, 113 insertions(+), 57 deletions(-) create mode 100644 controllers/middlewares/rateLimiter.js diff --git a/config.sample.js b/config.sample.js index 7df42d9..7a6bd77 100644 --- a/config.sample.js +++ b/config.sample.js @@ -206,42 +206,20 @@ module.exports = { trustProxy: true, /* - Rate limits. - Please be aware that these apply to all users, including site owners. - https://github.com/nfriedly/express-rate-limit/tree/v6.3.0#configuration + Rate limiters. + https://github.com/animir/node-rate-limiter-flexible/wiki/Memory */ - rateLimits: [ - { - // 10 requests in 1 second - routes: [ - '/api/' - ], - config: { - windowMs: 1000, - max: 10, - legacyHeaders: true, - standardHeaders: true, - message: { - success: false, - description: 'Rate limit reached, please try again in a while.' - } - } - }, + rateLimiters: [ { // 2 requests in 5 seconds routes: [ + // If multiple routes, they will share the same points pool '/api/login', '/api/register' ], - config: { - windowMs: 5 * 1000, - max: 2, - legacyHeaders: true, - standardHeaders: true, - message: { - success: false, - description: 'Rate limit reached, please try again in 5 seconds.' - } + options: { + points: 2, + duration: 5 } }, { @@ -249,11 +227,9 @@ module.exports = { routes: [ '/api/album/zip' ], - config: { - windowMs: 30 * 1000, - max: 6, - legacyHeaders: true, - standardHeaders: true + options: { + points: 6, + duration: 30 } }, { @@ -261,19 +237,35 @@ module.exports = { routes: [ '/api/tokens/change' ], - config: { - windowMs: 60 * 1000, - max: 1, - legacyHeaders: true, - standardHeaders: true, - message: { - success: false, - description: 'Rate limit reached, please try again in 60 seconds.' - } + options: { + points: 1, + duration: 60 + } + }, + /* + Routes, whose scope would have encompassed other routes that have their own rate limit pools, + must only be set after said routes, otherwise their rate limit pools will never trigger. + i.e. since /api/ encompass all other /api/* routes, it must be set last + */ + { + // 10 requests in 1 second + routes: [ + '/api/' + ], + options: { + points: 10, + duration: 1 } } ], + /* + Whitelisted IP addresses for rate limiters. + */ + rateLimitersWhitelist: [ + '127.0.0.1' + ], + /* Uploads config. */ diff --git a/controllers/middlewares/rateLimiter.js b/controllers/middlewares/rateLimiter.js new file mode 100644 index 0000000..ad288b5 --- /dev/null +++ b/controllers/middlewares/rateLimiter.js @@ -0,0 +1,56 @@ +const { RateLimiterMemory } = require('rate-limiter-flexible') +const ClientError = require('./../utils/ClientError') + +class RateLimiter { + #requestKey + #whitelistedKeys + rateLimiterMemory + + constructor (requestKey, options = {}, whitelistedKeys) { + if (typeof options.points !== 'number' || typeof options.duration !== 'number') { + throw new Error('Points and Duration must be set with numbers in options') + } + + if (whitelistedKeys && typeof whitelistedKeys instanceof Set) { + throw new TypeError('Whitelisted keys must be a Set') + } + + this.#requestKey = requestKey + this.#whitelistedKeys = new Set(whitelistedKeys) + + this.rateLimiterMemory = new RateLimiterMemory(options) + } + + async #middleware (req, res, next) { + if (res.locals.rateLimit) return + + // If unset, assume points pool is shared to all visitors of each route + const key = this.#requestKey ? req[this.#requestKey] : req.path + + if (this.#whitelistedKeys.has(key)) { + // Set the Response local variable for earlier bypass in any subsequent RateLimit middlewares + res.locals.rateLimit = 'BYPASS' + return + } + + // Always consume only 1 point + await this.rateLimiterMemory.consume(key, 1) + .then(result => { + res.locals.rateLimit = result + res.set('Retry-After', String(result.msBeforeNext / 1000)) + res.set('X-RateLimit-Limit', String(this.rateLimiterMemory._points)) + res.set('X-RateLimit-Remaining', String(result.remainingPoints)) + res.set('X-RateLimit-Reset', String(new Date(Date.now() + result.msBeforeNext))) + }) + .catch(reject => { + // Re-throw with ClientError + throw new ClientError('Rate limit reached, please try again in a while.', { statusCode: 429 }) + }) + } + + get middleware () { + return this.#middleware.bind(this) + } +} + +module.exports = RateLimiter diff --git a/lolisafe.js b/lolisafe.js index d0e3aa8..ef7384c 100644 --- a/lolisafe.js +++ b/lolisafe.js @@ -14,7 +14,6 @@ const helmet = require('helmet') const HyperExpress = require('hyper-express') const LiveDirectory = require('live-directory') const NodeClam = require('clamscan') -// const rateLimit = require('express-rate-limit') // FIXME: Find alternative const { accessSync, constants } = require('fs') // Check required config files @@ -46,6 +45,7 @@ const utils = require('./controllers/utilsController') // Custom middlewares const NunjucksRenderer = require('./controllers/middlewares/nunjucksRenderer') +const RateLimiter = require('./controllers/middlewares/rateLimiter') // const ServeStatic = require('./controllers/middlewares/serveStatic') // TODO // Routes @@ -57,6 +57,21 @@ const player = require('./routes/player') const isDevMode = process.env.NODE_ENV === 'development' +// Rate limiters +if (Array.isArray(config.rateLimiters)) { + let whitelistedKeys + if (Array.isArray(config.rateLimitersWhitelist)) { + whitelistedKeys = new Set(config.rateLimitersWhitelist) + } + for (const rateLimit of config.rateLimiters) { + // Init RateLimiter using Request.ip as key + const rateLimiterInstance = new RateLimiter('ip', rateLimit.options, whitelistedKeys) + for (const route of rateLimit.routes) { + safe.use(route, rateLimiterInstance.middleware) + } + } +} + // Helmet security headers if (config.helmet instanceof Object) { // If an empty object, simply do not use Helmet @@ -111,19 +126,6 @@ const initLiveDirectory = (options = {}) => { return new LiveDirectory(options) } -// Configure rate limits (disabled during development) -// FIXME: express-rate-limit does not work with hyper-express, find alternative -/* -if (!isDevMode && Array.isArray(config.rateLimits) && config.rateLimits.length) { - for (const _rateLimit of config.rateLimits) { - const limiter = rateLimit(_rateLimit.config) - for (const route of _rateLimit.routes) { - safe.use(route, limiter) - } - } -} -*/ - const cdnPages = [...config.pages] // Defaults to no-op diff --git a/package.json b/package.json index bbf49da..3d6b03c 100644 --- a/package.json +++ b/package.json @@ -51,6 +51,7 @@ "node-fetch": "~2.6.7", "nunjucks": "~3.2.3", "randomstring": "~1.2.2", + "rate-limiter-flexible": "^2.3.7", "search-query-parser": "~1.6.0", "sharp": "~0.30.7", "systeminformation": "5.11.23" diff --git a/yarn.lock b/yarn.lock index bd573df..069ef66 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5373,6 +5373,11 @@ range-parser@^1.2.1: resolved "https://registry.yarnpkg.com/range-parser/-/range-parser-1.2.1.tgz#3cf37023d199e1c24d1a55b84800c2f3e6468031" integrity sha512-Hrgsx+orqoygnmhFbKaHE6c296J+HTAQXoxEF6gNupROmmGJRoyzfG3ccAveqCBrwr/2yxQ5BVd/GTl5agOwSg== +rate-limiter-flexible@^2.3.7: + version "2.3.7" + resolved "https://registry.yarnpkg.com/rate-limiter-flexible/-/rate-limiter-flexible-2.3.7.tgz#c23e1f818a1575f1de1fd173437f4072125e1615" + integrity sha512-dmc+J/IffVBvHlqq5/XClsdLdkOdQV/tjrz00cwneHUbEDYVrf4aUDAyR4Jybcf2+Vpn4NwoVrnnAyt/D0ciWw== + rc@^1.2.7, rc@^1.2.8: version "1.2.8" resolved "https://registry.yarnpkg.com/rc/-/rc-1.2.8.tgz#cd924bf5200a075b83c188cd6b9e211b7fc0d3ed"