feat(server): allow ratelimiting to be explicitly disabled (#3757)

* feat(server): allow ratelimiting to be explicitly disabled
- allows rate limiter to be enabled or disabled explicitly
- example .env file for testing now explicitly disables it
- disables rate limiter in CI tests, except where explicitly testing the rate limiter

.circleci/config.yml

@@ -496,6 +496,7 @@ jobs:
       S3_REGION: '' # optional, defaults to 'us-east-1'
       ENCRYPTION_KEYS_PATH: 'test/assets/automate/encryptionKeys.json'
       FF_BILLING_INTEGRATION_ENABLED: 'true'
+      RATELIMITER_ENABLED: 'false'
     steps:
       - checkout
       - restore_cache:
@@ -578,6 +579,7 @@ jobs:
       S3_REGION: '' # optional, defaults to 'us-east-1'
       ENCRYPTION_KEYS_PATH: 'test/assets/automate/encryptionKeys.json'
       DISABLE_ALL_FFS: 'true'
+      RATELIMITER_ENABLED: 'false'
 
   test-server-multiregion:
     <<: *test-server-job
@@ -624,6 +626,7 @@ jobs:
       FF_WORKSPACES_MODULE_ENABLED: 'true'
       FF_WORKSPACES_MULTI_REGION_ENABLED: 'true'
       RUN_TESTS_IN_MULTIREGION_MODE: true
+      RATELIMITER_ENABLED: 'false'
 
   test-frontend-2:
     docker: &docker-node-browsers-image

packages/server/.env.test-example

@@ -6,4 +6,5 @@ PORT=0
 POSTGRES_URL=postgres://speckle:speckle@127.0.0.1/speckle2_test
 POSTGRES_USER=''
 MULTI_REGION_CONFIG_PATH="multiregion.test.json"
-#RUN_TESTS_IN_MULTIREGION_MODE=true
+#RUN_TESTS_IN_MULTIREGION_MODE=true
+RATELIMITER_ENABLED='false'

packages/server/modules/core/services/ratelimiter.ts

@@ -2,7 +2,7 @@ import express from 'express'
 import {
   getRedisUrl,
   getIntFromEnv,
-  isTestEnv
+  getBooleanFromEnv
 } from '@/modules/shared/helpers/envHelper'
 import {
   BurstyRateLimiter,
@@ -55,6 +55,10 @@ export type RateLimiterMapping = {
 
 export type RateLimitAction = keyof typeof LIMITS
 
+export const isRateLimiterEnabled = (): boolean => {
+  return getBooleanFromEnv('RATELIMITER_ENABLED', true)
+}
+
 export const LIMITS = <const>{
   ALL_REQUESTS: {
     regularOptions: {
@@ -307,7 +311,7 @@ export const createRateLimiterMiddleware = (
     res: express.Response,
     next: express.NextFunction
   ) => {
-    if (isTestEnv()) return next()
+    if (!isRateLimiterEnabled()) return next()
     const path = getRequestPath(req) || ''
     const action = getActionForPath(path, req.method)
     const source = getSourceFromRequest(req)

packages/server/modules/core/tests/ratelimiter.spec.ts

@@ -95,6 +95,8 @@ describe('Rate Limiting', () => {
     })
   })
 
+  //FIXME the tests in this describe block cannot be run in parallel
+  //      with other tests as it modifies process.env
   describe('rateLimiterMiddleware', () => {
     it('should set header with remaining points if not rate limited', async () => {
       const request = httpMocks.createRequest({
@@ -121,7 +123,7 @@ describe('Rate Limiting', () => {
 
       const SUT = createRateLimiterMiddleware(testMappings)
 
-      await temporarilyDisableTestEnv(async () => {
+      await temporarilyEnableRateLimiter(async () => {
         await SUT(request, response, next)
       })
 
@@ -145,7 +147,7 @@ describe('Rate Limiting', () => {
       const SUT = createRateLimiterMiddleware(createTestRateLimiterMappings())
       response = httpMocks.createResponse()
 
-      await temporarilyDisableTestEnv(async () => {
+      await temporarilyEnableRateLimiter(async () => {
         await SUT(request, response, next)
       })
 
@@ -156,11 +158,11 @@ describe('Rate Limiting', () => {
 })
 
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
-const temporarilyDisableTestEnv = async (callback: () => Promise<any>) => {
-  const oldNodeEnv = process.env.NODE_ENV
-  process.env.NODE_ENV = 'temporarily-disabled-test'
+const temporarilyEnableRateLimiter = async (callback: () => Promise<any>) => {
+  const oldRateLimiterEnabledFlag = process.env.RATELIMITER_ENABLED
+  process.env.RATELIMITER_ENABLED = 'true'
   await callback()
-  process.env.NODE_ENV = oldNodeEnv
+  process.env.RATELIMITER_ENABLED = oldRateLimiterEnabledFlag
 }
 
 // eslint-disable-next-line @typescript-eslint/no-explicit-any

packages/server/modules/shared/helpers/envHelper.ts

@@ -3,6 +3,31 @@ import { trimEnd } from 'lodash'
 import * as Environment from '@speckle/shared/dist/commonjs/environment/index.js'
 import { ensureError } from '@speckle/shared'
 
+export function getStringFromEnv(
+  envVarKey: string,
+  options?: Partial<{
+    /**
+     * If set to true, wont throw if the env var is not set
+     */
+    unsafe: boolean
+  }>
+): string {
+  const envVar = process.env[envVarKey]
+  if (!envVar) {
+    if (options?.unsafe) return ''
+    throw new MisconfiguredEnvironmentError(`${envVarKey} env var not configured`)
+  }
+  return envVar
+}
+
+export function getIntFromEnv(envVarKey: string, aDefault = '0'): number {
+  return parseInt(process.env[envVarKey] || aDefault)
+}
+
+export function getBooleanFromEnv(envVarKey: string, aDefault = false): boolean {
+  return ['1', 'true', true].includes(process.env[envVarKey] || aDefault.toString())
+}
+
 export function getSessionSecret() {
   if (!process.env.SESSION_SECRET) {
     throw new MisconfiguredEnvironmentError('SESSION_SECRET env var not configured')
@@ -45,31 +70,6 @@ export function getMaximumObjectSizeMB() {
   return getIntFromEnv('MAX_OBJECT_SIZE_MB', '100')
 }
 
-export function getIntFromEnv(envVarKey: string, aDefault = '0'): number {
-  return parseInt(process.env[envVarKey] || aDefault)
-}
-
-export function getBooleanFromEnv(envVarKey: string, aDefault = false): boolean {
-  return ['1', 'true', true].includes(process.env[envVarKey] || aDefault.toString())
-}
-
-export function getStringFromEnv(
-  envVarKey: string,
-  options?: Partial<{
-    /**
-     * If set to true, wont throw if the env var is not set
-     */
-    unsafe: boolean
-  }>
-): string {
-  const envVar = process.env[envVarKey]
-  if (!envVar) {
-    if (options?.unsafe) return ''
-    throw new MisconfiguredEnvironmentError(`${envVarKey} env var not configured`)
-  }
-  return envVar
-}
-
 /**
  * Whether the server is supposed to serve frontend 2.0
  */

utils/helm/speckle-server/templates/_helpers.tpl

@@ -964,6 +964,10 @@ Generate the environment variables for Speckle server and Speckle objects deploy
 {{- end }}
 
 # Rate Limiting
+
+- name: RATELIMITER_ENABLED
+  value: "{{ .Values.server.ratelimiting.enabled }}"
+
 {{- if .Values.server.ratelimiting.all_requests }}
 - name: RATELIMIT_ALL_REQUESTS
   value: "{{ .Values.server.ratelimiting.all_requests }}"

utils/helm/speckle-server/values.schema.json

@@ -1211,6 +1211,11 @@
         "ratelimiting": {
           "type": "object",
           "properties": {
+            "enabled": {
+              "type": "boolean",
+              "description": "If enabled, rate limiting will be applied to the Speckle server.",
+              "default": true
+            },
             "all_requests": {
               "type": "number",
               "description": "The maximum number of requests that can be made to the Speckle server in a moving one second window.",

utils/helm/speckle-server/values.yaml

@@ -746,6 +746,8 @@ server:
     ##
     memory: 3Gi
   ratelimiting:
+    ## @param server.ratelimiting.enabled If enabled, rate limiting will be applied to the Speckle server.
+    enabled: true
     ## @param server.ratelimiting.all_requests The maximum number of requests that can be made to the Speckle server in a moving one second window.
     all_requests: 500
     ## @param server.ratelimiting.burst_all_requests If the regular limit is exceeded, the limit is increased to the burst limit. This is the maximum number of requests that can be made to the Speckle server in a moving one minute window.