From 27c2e8b9384d10f808f5640eaa0d95d84a9980dd Mon Sep 17 00:00:00 2001 From: Miguel Serrano Date: Mon, 28 Oct 2024 12:37:54 +0100 Subject: [PATCH] Merge pull request #21327 from overleaf/msm-optional-subnet-rate-limiter [web] Add option to disable subnet rate limiting (+CE/SP Hotfix `5.2.1`) GitOrigin-RevId: 78d60c9638cede729dd93c3c2421f55b34c0dbfe --- server-ce/config/settings.js | 5 +++ server-ce/hotfix/5.2.1/Dockerfile | 5 +++ server-ce/hotfix/5.2.1/pr_21327.patch | 36 +++++++++++++++++++ .../web/app/src/infrastructure/RateLimiter.js | 2 +- services/web/config/settings.defaults.js | 2 ++ .../src/infrastructure/RateLimiterTests.js | 32 +++++++++++++++++ 6 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 server-ce/hotfix/5.2.1/Dockerfile create mode 100644 server-ce/hotfix/5.2.1/pr_21327.patch diff --git a/server-ce/config/settings.js b/server-ce/config/settings.js index 36224f0750..365b3314a2 100644 --- a/server-ce/config/settings.js +++ b/server-ce/config/settings.js @@ -212,6 +212,11 @@ const settings = { enabled: process.env.OVERLEAF_CSP_ENABLED !== 'false', }, + rateLimit: { + subnetRateLimiterDisabled: + process.env.SUBNET_RATE_LIMITER_DISABLED !== 'false', + }, + // These credentials are used for authenticating api requests // between services that may need to go over public channels httpAuthUsers, diff --git a/server-ce/hotfix/5.2.1/Dockerfile b/server-ce/hotfix/5.2.1/Dockerfile new file mode 100644 index 0000000000..63cc952d0d --- /dev/null +++ b/server-ce/hotfix/5.2.1/Dockerfile @@ -0,0 +1,5 @@ +FROM sharelatex/sharelatex:5.2.0 + +# Subnet rate limiter fix +COPY pr_21327.patch / +RUN cd / && patch -p0 < pr_21327.patch && rm pr_21327.patch diff --git a/server-ce/hotfix/5.2.1/pr_21327.patch b/server-ce/hotfix/5.2.1/pr_21327.patch new file mode 100644 index 0000000000..f2e49df985 --- /dev/null +++ b/server-ce/hotfix/5.2.1/pr_21327.patch @@ -0,0 +1,36 @@ +--- overleaf/services/web/app/src/infrastructure/RateLimiter.js ++++ overleaf/services/web/app/src/infrastructure/RateLimiter.js +@@ -39,7 +39,7 @@ class RateLimiter { + keyPrefix: `rate-limit:${name}`, + storeClient: rclient, + }) +- if (opts.subnetPoints) { ++ if (opts.subnetPoints && !Settings.rateLimit?.subnetRateLimiterDisabled) { + this._subnetRateLimiter = new RateLimiterFlexible.RateLimiterRedis({ + ...opts, + points: opts.subnetPoints, +--- overleaf/services/web/config/settings.defaults.js ++++ overleaf/services/web/config/settings.defaults.js +@@ -777,6 +777,8 @@ module.exports = { + reloadModuleViewsOnEachRequest: process.env.NODE_ENV === 'development', + + rateLimit: { ++ subnetRateLimiterDisabled: ++ process.env.SUBNET_RATE_LIMITER_DISABLED === 'true', + autoCompile: { + everyone: process.env.RATE_LIMIT_AUTO_COMPILE_EVERYONE || 100, + standard: process.env.RATE_LIMIT_AUTO_COMPILE_STANDARD || 25, +--- etc/overleaf/settings.js ++++ etc/overleaf/settings.js +@@ -212,6 +212,11 @@ const settings = { + enabled: process.env.OVERLEAF_CSP_ENABLED !== 'false', + }, + ++ rateLimit: { ++ subnetRateLimiterDisabled: ++ process.env.SUBNET_RATE_LIMITER_DISABLED !== 'false', ++ }, ++ + // These credentials are used for authenticating api requests + // between services that may need to go over public channels + httpAuthUsers, diff --git a/services/web/app/src/infrastructure/RateLimiter.js b/services/web/app/src/infrastructure/RateLimiter.js index f0f625a07e..6ce80b23a1 100644 --- a/services/web/app/src/infrastructure/RateLimiter.js +++ b/services/web/app/src/infrastructure/RateLimiter.js @@ -39,7 +39,7 @@ class RateLimiter { keyPrefix: `rate-limit:${name}`, storeClient: rclient, }) - if (opts.subnetPoints) { + if (opts.subnetPoints && !Settings.rateLimit?.subnetRateLimiterDisabled) { this._subnetRateLimiter = new RateLimiterFlexible.RateLimiterRedis({ ...opts, points: opts.subnetPoints, diff --git a/services/web/config/settings.defaults.js b/services/web/config/settings.defaults.js index 9bae2f566b..0144f18643 100644 --- a/services/web/config/settings.defaults.js +++ b/services/web/config/settings.defaults.js @@ -777,6 +777,8 @@ module.exports = { reloadModuleViewsOnEachRequest: process.env.NODE_ENV === 'development', rateLimit: { + subnetRateLimiterDisabled: + process.env.SUBNET_RATE_LIMITER_DISABLED === 'true', autoCompile: { everyone: process.env.RATE_LIMIT_AUTO_COMPILE_EVERYONE || 100, standard: process.env.RATE_LIMIT_AUTO_COMPILE_STANDARD || 25, diff --git a/services/web/test/unit/src/infrastructure/RateLimiterTests.js b/services/web/test/unit/src/infrastructure/RateLimiterTests.js index ca125c0455..4d0b2c2f98 100644 --- a/services/web/test/unit/src/infrastructure/RateLimiterTests.js +++ b/services/web/test/unit/src/infrastructure/RateLimiterTests.js @@ -19,11 +19,13 @@ describe('RateLimiter', function () { this.RateLimiterFlexible = { RateLimiterRedis: sinon.stub(), } + this.Settings = {} this.RateLimiter = SandboxedModule.require(modulePath, { requires: { './RedisWrapper': this.RedisWrapper, 'rate-limiter-flexible': this.RateLimiterFlexible, + '@overleaf/settings': this.Settings, }, }) }) @@ -60,4 +62,34 @@ describe('RateLimiter', function () { ).to.throw() }) }) + + describe('_subnetRateLimiter', function () { + it('should be defined by default', function () { + const rateLimiter = new this.RateLimiter.RateLimiter('some-limit', { + points: 20, + subnetPoints: 200, + duration: 60, + }) + expect(rateLimiter._subnetRateLimiter).not.to.be.undefined + }) + + it('should be undefined when subnet rate limiting is disabled', function () { + this.Settings.rateLimit = { subnetRateLimiterDisabled: true } + + const rateLimiter = new this.RateLimiter.RateLimiter('some-limit', { + points: 20, + subnetPoints: 200, + duration: 60, + }) + expect(rateLimiter._subnetRateLimiter).to.be.undefined + }) + + it('should be undefined when subnetPoints are not passed as an option', function () { + const rateLimiter = new this.RateLimiter.RateLimiter('some-limit', { + points: 20, + duration: 60, + }) + expect(rateLimiter._subnetRateLimiter).to.be.undefined + }) + }) })