From 502d1c61fd31691e518ebab06834088eed58143b Mon Sep 17 00:00:00 2001
From: Camila Belo <camilaibs@gmail.com>
Date: Thu, 14 Mar 2024 13:58:23 +0100
Subject: [PATCH 1/8] feat(backend-app-api): start implementing in memory rate
 limit

Signed-off-by: Camila Belo <camilaibs@gmail.com>
---
 packages/backend-app-api/config.d.ts          | 12 +++++++++
 packages/backend-app-api/package.json         |  1 +
 .../httpRouter/createCredentialsBarrier.ts    | 26 +++++++++++++++++--
 yarn.lock                                     | 10 +++++++
 4 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/packages/backend-app-api/config.d.ts b/packages/backend-app-api/config.d.ts
index 137615a152..2371623b82 100644
--- a/packages/backend-app-api/config.d.ts
+++ b/packages/backend-app-api/config.d.ts
@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+import { HumanDuration } from '@backstage/types';
+
 export interface Config {
   backend?: {
     auth?: {
@@ -32,6 +34,16 @@ export interface Config {
        * unless you configure credentials for service calls.
        */
       dangerouslyDisableDefaultAuthPolicy?: boolean;
+      rateLimit?: {
+        /**
+         * Limit each IP to max requests per window
+         */
+        max?: number;
+        /**
+         * The duration for which the rate limit is enforced
+         */
+        window?: HumanDuration;
+      };
     };
   };
 
diff --git a/packages/backend-app-api/package.json b/packages/backend-app-api/package.json
index 6f69c6551f..406ce80e4b 100644
--- a/packages/backend-app-api/package.json
+++ b/packages/backend-app-api/package.json
@@ -64,6 +64,7 @@
     "cors": "^2.8.5",
     "express": "^4.17.1",
     "express-promise-router": "^4.1.0",
+    "express-rate-limit": "^7.2.0",
     "fs-extra": "^11.2.0",
     "helmet": "^6.0.0",
     "jose": "^5.0.0",
diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts b/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts
index a69fa5a804..6e5e06c59e 100644
--- a/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts
+++ b/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts
@@ -19,8 +19,11 @@ import {
   HttpRouterServiceAuthPolicy,
   RootConfigService,
 } from '@backstage/backend-plugin-api';
+import { durationToMilliseconds } from '@backstage/types';
 import { RequestHandler } from 'express';
 import { pathToRegexp } from 'path-to-regexp';
+import { rateLimit } from 'express-rate-limit';
+import { readDurationFromConfig } from '@backstage/config';
 
 export function createPathPolicyPredicate(policyPath: string) {
   if (policyPath === '/' || policyPath === '*') {
@@ -59,13 +62,32 @@ export function createCredentialsBarrier(options: {
   const unauthenticatedPredicates = new Array<(path: string) => boolean>();
   const cookiePredicates = new Array<(path: string) => boolean>();
 
-  const middleware: RequestHandler = (req, _, next) => {
+  // Default rate limit is 100 requests per 15 minutes
+  const max = config?.has('backend.auth.rateLimit.max')
+    ? config.getNumber('backend.auth.rateLimit.max')
+    : 100;
+
+  const duration = config?.has('backend.auth.rateLimit.window')
+    ? readDurationFromConfig(config.getConfig('backend.auth.rateLimit.window'))
+    : undefined;
+
+  // Default rate limit window is 15 minutes
+  const windowMs = duration ? durationToMilliseconds(duration) : 15 * 60 * 1000;
+
+  const limiter = rateLimit({
+    windowMs,
+    max,
+    standardHeaders: true, // Return rate limit info in the `RateLimit-*` headers
+    legacyHeaders: false, // Disable the `X-RateLimit-*` headers
+  });
+
+  const middleware: RequestHandler = (req, res, next) => {
     const allowsUnauthenticated = unauthenticatedPredicates.some(predicate =>
       predicate(req.path),
     );
 
     if (allowsUnauthenticated) {
-      next();
+      limiter(req, res, next);
       return;
     }
 
diff --git a/yarn.lock b/yarn.lock
index a0d94a3027..081bbeb40c 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -3248,6 +3248,7 @@ __metadata:
     cors: ^2.8.5
     express: ^4.17.1
     express-promise-router: ^4.1.0
+    express-rate-limit: ^7.2.0
     fs-extra: ^11.2.0
     helmet: ^6.0.0
     http-errors: ^2.0.0
@@ -28044,6 +28045,15 @@ __metadata:
   languageName: node
   linkType: hard
 
+"express-rate-limit@npm:^7.2.0":
+  version: 7.2.0
+  resolution: "express-rate-limit@npm:7.2.0"
+  peerDependencies:
+    express: 4 || 5 || ^5.0.0-beta.1
+  checksum: 6adc3e06d430e91cf8ef8da23107466ffd9193b5680b03bc12702894cab0adcbb980634602b839b3a1444620f2379707098af3a4dcc38c909e89214abaf6c1bf
+  languageName: node
+  linkType: hard
+
 "express-session@npm:^1.17.1, express-session@npm:^1.17.3":
   version: 1.18.0
   resolution: "express-session@npm:1.18.0"

From cff90f1b5cd3e126a61c23562603b007cb89b384 Mon Sep 17 00:00:00 2001
From: Camila Belo <camilaibs@gmail.com>
Date: Thu, 14 Mar 2024 18:38:08 +0100
Subject: [PATCH 2/8] feat(backend-app-api): create rate limit cache store

Signed-off-by: Camila Belo <camilaibs@gmail.com>
---
 .../httpRouter/createCredentialsBarrier.ts    |   8 +-
 .../httpRouter/httpRouterServiceFactory.ts    |  16 +-
 .../httpRouter/rateLimitStore.test.ts         | 184 ++++++++++++++++++
 .../httpRouter/rateLimitStore.ts              | 119 +++++++++++
 4 files changed, 323 insertions(+), 4 deletions(-)
 create mode 100644 packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.test.ts
 create mode 100644 packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.ts

diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts b/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts
index 6e5e06c59e..72b93220ee 100644
--- a/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts
+++ b/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts
@@ -15,6 +15,7 @@
  */
 
 import {
+  CacheService,
   HttpAuthService,
   HttpRouterServiceAuthPolicy,
   RootConfigService,
@@ -24,6 +25,7 @@ import { RequestHandler } from 'express';
 import { pathToRegexp } from 'path-to-regexp';
 import { rateLimit } from 'express-rate-limit';
 import { readDurationFromConfig } from '@backstage/config';
+import { RateLimitStore } from './rateLimitStore';
 
 export function createPathPolicyPredicate(policyPath: string) {
   if (policyPath === '/' || policyPath === '*') {
@@ -42,11 +44,12 @@ export function createPathPolicyPredicate(policyPath: string) {
 export function createCredentialsBarrier(options: {
   httpAuth: HttpAuthService;
   config: RootConfigService;
+  cache: CacheService;
 }): {
   middleware: RequestHandler;
   addAuthPolicy: (policy: HttpRouterServiceAuthPolicy) => void;
 } {
-  const { httpAuth, config } = options;
+  const { httpAuth, config, cache } = options;
 
   const disableDefaultAuthPolicy = config.getOptionalBoolean(
     'backend.auth.dangerouslyDisableDefaultAuthPolicy',
@@ -78,7 +81,8 @@ export function createCredentialsBarrier(options: {
     windowMs,
     max,
     standardHeaders: true, // Return rate limit info in the `RateLimit-*` headers
-    legacyHeaders: false, // Disable the `X-RateLimit-*` headers
+    legacyHeaders: false, // Disable the `X-RateLimit-*` headers,
+    store: RateLimitStore.fromOptions({ cache }),
   });
 
   const middleware: RequestHandler = (req, res, next) => {
diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/httpRouterServiceFactory.ts b/packages/backend-app-api/src/services/implementations/httpRouter/httpRouterServiceFactory.ts
index e72d091b96..50f9e5c97a 100644
--- a/packages/backend-app-api/src/services/implementations/httpRouter/httpRouterServiceFactory.ts
+++ b/packages/backend-app-api/src/services/implementations/httpRouter/httpRouterServiceFactory.ts
@@ -39,20 +39,32 @@ export const httpRouterServiceFactory = createServiceFactory(
   (options?: HttpRouterFactoryOptions) => ({
     service: coreServices.httpRouter,
     deps: {
+      cache: coreServices.cache,
       plugin: coreServices.pluginMetadata,
       config: coreServices.rootConfig,
       lifecycle: coreServices.lifecycle,
       rootHttpRouter: coreServices.rootHttpRouter,
       httpAuth: coreServices.httpAuth,
     },
-    async factory({ httpAuth, config, plugin, rootHttpRouter, lifecycle }) {
+    async factory({
+      httpAuth,
+      config,
+      cache,
+      plugin,
+      rootHttpRouter,
+      lifecycle,
+    }) {
       const getPath = options?.getPath ?? (id => `/api/${id}`);
       const path = getPath(plugin.getId());
 
       const router = PromiseRouter();
       rootHttpRouter.use(path, router);
 
-      const credentialsBarrier = createCredentialsBarrier({ httpAuth, config });
+      const credentialsBarrier = createCredentialsBarrier({
+        httpAuth,
+        config,
+        cache,
+      });
 
       router.use(createLifecycleMiddleware({ lifecycle }));
       router.use(credentialsBarrier.middleware);
diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.test.ts b/packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.test.ts
new file mode 100644
index 0000000000..843cb83ed1
--- /dev/null
+++ b/packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.test.ts
@@ -0,0 +1,184 @@
+/*
+ * Copyright 2024 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { ServiceMock, mockServices } from '@backstage/backend-test-utils';
+import { RateLimitStore } from './rateLimitStore';
+import { CacheService } from '@backstage/backend-plugin-api';
+
+jest.setSystemTime(Date.parse('2024-03-15T08:39:11.869Z'));
+
+describe('RateLimitStore', () => {
+  let cacheServiceMock: ServiceMock<CacheService>;
+  let rateLimitStore: RateLimitStore;
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+    cacheServiceMock = mockServices.cache.mock();
+    rateLimitStore = RateLimitStore.fromOptions({
+      prefix: 'rl_',
+      cache: cacheServiceMock,
+    });
+  });
+
+  it('should initialize with default options', () => {
+    expect(rateLimitStore.windowMs).toBe(15 * 60 * 1000);
+  });
+
+  it('should initialize with custom options', () => {
+    rateLimitStore.init({ windowMs: 10 * 60 * 1000 });
+    expect(rateLimitStore.windowMs).toBe(10 * 60 * 1000);
+  });
+
+  it('should get rate limit info for existing key', async () => {
+    const key = 'existingKey';
+    const existingValue = {
+      totalHits: 5,
+      resetTime: new Date(),
+    };
+
+    cacheServiceMock.get.mockResolvedValue({
+      totalHits: existingValue.totalHits,
+      resetTime: existingValue.resetTime.getTime(),
+    });
+
+    const returnedValue = await rateLimitStore.get(key);
+
+    expect(returnedValue).toEqual(existingValue);
+    expect(cacheServiceMock.get).toHaveBeenCalledWith(
+      rateLimitStore.prefixKey(key),
+    );
+  });
+
+  it('should get default rate limit info for non-existing key', async () => {
+    const key = 'nonExistingKey';
+    const defaultValue = {
+      totalHits: 0,
+      resetTime: new Date(Date.now() + rateLimitStore.windowMs),
+    };
+
+    cacheServiceMock.get.mockResolvedValue(undefined);
+
+    const returnedValue = await rateLimitStore.get(key);
+
+    expect(returnedValue).toEqual(defaultValue);
+    expect(cacheServiceMock.get).toHaveBeenCalledWith(
+      rateLimitStore.prefixKey(key),
+    );
+  });
+
+  it('should increment rate limit for existing key', async () => {
+    const key = 'exisitingKey';
+    const existingValue = {
+      totalHits: 5,
+      resetTime: new Date(),
+    };
+
+    cacheServiceMock.get.mockResolvedValue({
+      totalHits: existingValue.totalHits,
+      resetTime: existingValue.resetTime.getTime(),
+    });
+
+    const incrementedValue = await rateLimitStore.increment(key);
+
+    expect(incrementedValue.totalHits).toBe(existingValue.totalHits + 1);
+    expect(incrementedValue.resetTime).toEqual(existingValue.resetTime);
+    expect(cacheServiceMock.set).toHaveBeenCalledWith(
+      rateLimitStore.prefixKey(key),
+      {
+        totalHits: existingValue.totalHits + 1,
+        resetTime: existingValue.resetTime.getTime(),
+      },
+      { ttl: rateLimitStore.windowMs },
+    );
+  });
+
+  it('should increment rate limit for non-existing key', async () => {
+    const key = 'nonExistingKey';
+
+    cacheServiceMock.get.mockResolvedValue(undefined);
+
+    const incrementedValue = await rateLimitStore.increment(key);
+
+    expect(incrementedValue.totalHits).toBe(1);
+    expect(incrementedValue.resetTime).toBeInstanceOf(Date);
+    expect(cacheServiceMock.set).toHaveBeenCalledWith(
+      rateLimitStore.prefixKey(key),
+      {
+        totalHits: incrementedValue.totalHits,
+        resetTime: incrementedValue.resetTime?.getTime(),
+      },
+      { ttl: rateLimitStore.windowMs },
+    );
+  });
+
+  it('should decrement rate limit for existing key', async () => {
+    const key = 'exisitingKey';
+    const existingValue = {
+      totalHits: 5,
+      resetTime: new Date(),
+    };
+
+    cacheServiceMock.get.mockResolvedValue({
+      totalHits: existingValue.totalHits,
+      resetTime: existingValue.resetTime.getTime(),
+    });
+
+    await rateLimitStore.decrement(key);
+
+    expect(cacheServiceMock.set).toHaveBeenCalledWith(
+      rateLimitStore.prefixKey(key),
+      {
+        totalHits: existingValue.totalHits - 1,
+        resetTime: existingValue.resetTime.getTime(),
+      },
+      { ttl: rateLimitStore.windowMs },
+    );
+  });
+
+  it('should not decrement rate limit below zero', async () => {
+    const key = 'testKey';
+    const existingValue = {
+      totalHits: 0,
+      resetTime: new Date(),
+    };
+
+    cacheServiceMock.get.mockResolvedValue({
+      totalHits: existingValue.totalHits,
+      resetTime: existingValue.resetTime.getTime(),
+    });
+
+    await rateLimitStore.decrement(key);
+
+    expect(cacheServiceMock.set).toHaveBeenCalledWith(
+      rateLimitStore.prefixKey(key),
+      {
+        totalHits: 0,
+        resetTime: existingValue.resetTime.getTime(),
+      },
+      { ttl: rateLimitStore.windowMs },
+    );
+  });
+
+  it('should reset rate limit for existing key', async () => {
+    const key = 'exitingKey';
+
+    await rateLimitStore.resetKey(key);
+
+    expect(cacheServiceMock.delete).toHaveBeenCalledWith(
+      rateLimitStore.prefixKey(key),
+    );
+  });
+});
diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.ts b/packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.ts
new file mode 100644
index 0000000000..c111fbc2d2
--- /dev/null
+++ b/packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.ts
@@ -0,0 +1,119 @@
+/*
+ * Copyright 2024 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { CacheService } from '@backstage/backend-plugin-api';
+import type {
+  Store,
+  Options,
+  IncrementResponse,
+  ClientRateLimitInfo,
+} from 'express-rate-limit';
+
+type CacheStoreOptions = {
+  /**
+   * Optional field to differentiate hit countswhen multiple rate-limits are in use
+   */
+  prefix?: string;
+  /**
+   * The cache service to use for storing the hit counts.
+   */
+  cache: CacheService;
+};
+
+type CacheStoreValue = {
+  totalHits: number;
+  resetTime: number;
+};
+
+/**
+ * A `Store` that stores the hit count for each client.
+ */
+export class RateLimitStore implements Store {
+  /**
+   * The duration of time before which all hit counts are reset (in milliseconds).
+   * default: 15 minutes
+   */
+  windowMs: number = 15 * 60 * 1000;
+  prefix: string;
+  #cache: CacheService;
+
+  private constructor(options: CacheStoreOptions) {
+    this.prefix = options.prefix ?? 'rl_';
+    this.#cache = options.cache;
+  }
+
+  static fromOptions(options: CacheStoreOptions): RateLimitStore {
+    return new RateLimitStore(options);
+  }
+
+  #getDefaultValue() {
+    return { totalHits: 0, resetTime: Date.now() + this.windowMs };
+  }
+
+  init(options: Partial<Options>): void {
+    if (options.windowMs) {
+      this.windowMs = options.windowMs;
+    }
+  }
+
+  prefixKey(key: string): string {
+    return `${this.prefix}${key}`;
+  }
+
+  async get(key: string): Promise<ClientRateLimitInfo | undefined> {
+    const value =
+      (await this.#cache.get<CacheStoreValue>(this.prefixKey(key))) ??
+      this.#getDefaultValue();
+    return {
+      totalHits: value.totalHits,
+      resetTime: new Date(value.resetTime),
+    };
+  }
+
+  async increment(key: string): Promise<IncrementResponse> {
+    const value =
+      (await this.#cache.get<CacheStoreValue>(this.prefixKey(key))) ??
+      this.#getDefaultValue();
+    const totalHits = value.totalHits + 1;
+    const resetTime = value.resetTime;
+    await this.#cache.set(
+      this.prefixKey(key),
+      { totalHits, resetTime },
+      { ttl: this.windowMs },
+    );
+    return {
+      totalHits,
+      resetTime: new Date(resetTime),
+    };
+  }
+
+  async decrement(key: string): Promise<void> {
+    const value =
+      (await this.#cache.get<CacheStoreValue>(this.prefixKey(key))) ??
+      this.#getDefaultValue();
+    const totalHits = value.totalHits > 0 ? value.totalHits - 1 : 0;
+    const resetTime = value.resetTime;
+    await this.#cache.set(
+      this.prefixKey(key),
+      { totalHits, resetTime },
+      { ttl: this.windowMs },
+    );
+  }
+
+  async resetKey(key: string): Promise<void> {
+    await this.#cache.delete(this.prefixKey(key));
+  }
+}

From 4b34c796cb92cd7a636a17aec3567ff9fce57991 Mon Sep 17 00:00:00 2001
From: Camila Belo <camilaibs@gmail.com>
Date: Fri, 15 Mar 2024 09:55:52 +0100
Subject: [PATCH 3/8] test(backend-app-api): update barrier credential tests

Signed-off-by: Camila Belo <camilaibs@gmail.com>
---
 .../createCredentialsBarrier.test.ts          | 133 +++++++++++++++++-
 1 file changed, 131 insertions(+), 2 deletions(-)

diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.test.ts b/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.test.ts
index a5246c91b2..e28179e1ad 100644
--- a/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.test.ts
+++ b/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.test.ts
@@ -21,18 +21,21 @@ import request from 'supertest';
 import { createCredentialsBarrier } from './createCredentialsBarrier';
 import { mockCredentials, mockServices } from '@backstage/backend-test-utils';
 import { MiddlewareFactory } from '../../../http';
+import { CacheService } from '@backstage/backend-plugin-api';
+import { JsonObject } from '@backstage/types';
 
 const errorMiddleware = MiddlewareFactory.create({
   config: mockServices.rootConfig(),
   logger: mockServices.rootLogger(),
 }).error();
 
-function setup() {
+function setup(options?: { cache?: CacheService; config?: JsonObject }) {
   const barrier = createCredentialsBarrier({
     httpAuth: mockServices.httpAuth({
       defaultCredentials: mockCredentials.none(),
     }),
-    config: mockServices.rootConfig(),
+    config: mockServices.rootConfig({ data: options?.config }),
+    cache: options?.cache ?? mockServices.cache.mock(),
   });
 
   const app = express();
@@ -173,3 +176,129 @@ describe('createCredentialsBarrier', () => {
     await request(app).get('/other').send().expect(200);
   });
 });
+
+it('do not limit authenticated requests', async () => {
+  const now = Date.now();
+  jest.useFakeTimers({ now });
+  const cacheMock = mockServices.cache.mock();
+  const twoMinutesInMilliseconds = 2 * 60 * 1000;
+  const resetTime = now + twoMinutesInMilliseconds;
+  cacheMock.get.mockResolvedValue({ totalHits: 2, resetTime });
+
+  const max = 2;
+  const configMock = {
+    backend: {
+      auth: {
+        rateLimit: {
+          max, // 2 requests per window
+          window: { minutes: 2 }, // rate limit window expiration time,
+        },
+      },
+    },
+  };
+  const { app, barrier } = setup({
+    cache: cacheMock,
+    config: configMock,
+  });
+
+  // exceed the rate limit for authenticated requests
+  for (let i = 0; i < max + 1; i += 1) {
+    await request(app)
+      .get('/')
+      .set('authorization', mockCredentials.user.header())
+      .send()
+      .expect(200);
+  }
+
+  barrier.addAuthPolicy({ allow: 'user-cookie', path: '/' });
+
+  for (let i = 0; i < max + 1; i += 1) {
+    await request(app)
+      .get('/')
+      .set('cookie', mockCredentials.limitedUser.cookie())
+      .send()
+      .expect(200);
+  }
+});
+
+it('limit the number of unauthenticated requests', async () => {
+  const now = Date.now();
+  jest.useFakeTimers({ now });
+  const cacheMock = mockServices.cache.mock();
+  const twoMinutesInMilliseconds = 2 * 60 * 1000;
+  const resetTime = now + twoMinutesInMilliseconds;
+  cacheMock.get
+    .mockResolvedValueOnce(undefined) // Request 1
+    .mockResolvedValueOnce({ totalHits: 1, resetTime }) // Request 2
+    .mockResolvedValueOnce({ totalHits: 2, resetTime }) // Request 3
+    .mockResolvedValueOnce(undefined); // Request 4
+
+  const configMock = {
+    backend: {
+      auth: {
+        rateLimit: {
+          max: 2, // 2 requests per window
+          window: { minutes: 2 }, // rate limit window expiration time,
+        },
+      },
+    },
+  };
+  const { app, barrier } = setup({
+    cache: cacheMock,
+    config: configMock,
+  });
+
+  barrier.addAuthPolicy({ allow: 'unauthenticated', path: '/public' });
+
+  const randomIp = '48.105.15.17';
+  // Enable trust proxy to get the real IP from the X-Forwarded-For header
+  app.enable('trust proxy');
+  await request(app)
+    .get('/public')
+    .set('X-Forwarded-For', randomIp)
+    .send()
+    .expect(200);
+  await request(app)
+    .get('/public')
+    .set('X-Forwarded-For', randomIp)
+    .send()
+    .expect(200);
+  await request(app)
+    .get('/public')
+    .set('X-Forwarded-For', randomIp)
+    .send()
+    .expect(429);
+  await request(app)
+    .get('/public')
+    .set('X-Forwarded-For', randomIp)
+    .send()
+    .expect(200);
+
+  expect(cacheMock.get).toHaveBeenCalledTimes(4);
+  expect(cacheMock.set).toHaveBeenNthCalledWith(
+    1,
+    `rl_${randomIp}`, // rl is the default prefix for the rate limit store
+    { resetTime, totalHits: 1 },
+    { ttl: twoMinutesInMilliseconds },
+  );
+  expect(cacheMock.set).toHaveBeenNthCalledWith(
+    2,
+    `rl_${randomIp}`,
+    { resetTime, totalHits: 2 },
+    { ttl: twoMinutesInMilliseconds },
+  );
+  expect(cacheMock.set).toHaveBeenNthCalledWith(
+    3,
+    `rl_${randomIp}`,
+    { resetTime, totalHits: 3 },
+    { ttl: twoMinutesInMilliseconds },
+  );
+  expect(cacheMock.set).toHaveBeenNthCalledWith(
+    4,
+    `rl_${randomIp}`,
+    { resetTime, totalHits: 1 },
+    { ttl: twoMinutesInMilliseconds },
+  );
+
+  jest.useRealTimers();
+});

From a5d341e47f1623d3ba51f4a8022c7e48987233dd Mon Sep 17 00:00:00 2001
From: Camila Belo <camilaibs@gmail.com>
Date: Fri, 15 Mar 2024 12:39:48 +0100
Subject: [PATCH 4/8] docs(backend-app-api): add package changeset file

Signed-off-by: Camila Belo <camilaibs@gmail.com>
---
 .changeset/new-spoons-stare.md | 5 +++++
 1 file changed, 5 insertions(+)
 create mode 100644 .changeset/new-spoons-stare.md

diff --git a/.changeset/new-spoons-stare.md b/.changeset/new-spoons-stare.md
new file mode 100644
index 0000000000..04f7ef303e
--- /dev/null
+++ b/.changeset/new-spoons-stare.md
@@ -0,0 +1,5 @@
+---
+'@backstage/backend-app-api': minor
+---
+
+Adds an initial rate-limiting implementation so that any incoming requests that have a 'none' principal are rate-limited automatically.

From d8c26d5e6df8a2b9dc2c585c746ed44f25d84c93 Mon Sep 17 00:00:00 2001
From: Camila Belo <camilaibs@gmail.com>
Date: Mon, 18 Mar 2024 18:09:16 +0100
Subject: [PATCH 5/8] refactor: apply review suggestions

Signed-off-by: Camila Belo <camilaibs@gmail.com>
---
 .../httpRouter/createCredentialsBarrier.ts                | 8 ++++----
 .../implementations/httpRouter/rateLimitStore.test.ts     | 2 +-
 .../services/implementations/httpRouter/rateLimitStore.ts | 4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts b/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts
index 72b93220ee..16e0203155 100644
--- a/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts
+++ b/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts
@@ -65,17 +65,17 @@ export function createCredentialsBarrier(options: {
   const unauthenticatedPredicates = new Array<(path: string) => boolean>();
   const cookiePredicates = new Array<(path: string) => boolean>();
 
-  // Default rate limit is 100 requests per 15 minutes
+  // Default rate limit is 60 requests per 1 minute
   const max = config?.has('backend.auth.rateLimit.max')
     ? config.getNumber('backend.auth.rateLimit.max')
-    : 100;
+    : 60;
 
   const duration = config?.has('backend.auth.rateLimit.window')
     ? readDurationFromConfig(config.getConfig('backend.auth.rateLimit.window'))
     : undefined;
 
-  // Default rate limit window is 15 minutes
-  const windowMs = duration ? durationToMilliseconds(duration) : 15 * 60 * 1000;
+  // Default rate limit window is 1 minute
+  const windowMs = duration ? durationToMilliseconds(duration) : 1 * 60 * 1000;
 
   const limiter = rateLimit({
     windowMs,
diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.test.ts b/packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.test.ts
index 843cb83ed1..44035fc62e 100644
--- a/packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.test.ts
+++ b/packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.test.ts
@@ -34,7 +34,7 @@ describe('RateLimitStore', () => {
   });
 
   it('should initialize with default options', () => {
-    expect(rateLimitStore.windowMs).toBe(15 * 60 * 1000);
+    expect(rateLimitStore.windowMs).toBe(1 * 60 * 1000);
   });
 
   it('should initialize with custom options', () => {
diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.ts b/packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.ts
index c111fbc2d2..423a1012a9 100644
--- a/packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.ts
+++ b/packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.ts
@@ -44,9 +44,9 @@ type CacheStoreValue = {
 export class RateLimitStore implements Store {
   /**
    * The duration of time before which all hit counts are reset (in milliseconds).
-   * default: 15 minutes
+   * default: 60 requests per minute
    */
-  windowMs: number = 15 * 60 * 1000;
+  windowMs: number = 1 * 60 * 1000;
   prefix: string;
   #cache: CacheService;
 

From 4e265180aafebf264aadc40df98c51aca2e5f2c7 Mon Sep 17 00:00:00 2001
From: Camila Belo <camilaibs@gmail.com>
Date: Tue, 19 Mar 2024 10:49:03 +0100
Subject: [PATCH 6/8] refactor: add optional disabled config

Signed-off-by: Camila Belo <camilaibs@gmail.com>
---
 packages/backend-app-api/config.d.ts          | 24 +++++---
 .../createCredentialsBarrier.test.ts          | 61 +++++++++++++++++++
 .../httpRouter/createCredentialsBarrier.ts    | 33 +++++++---
 3 files changed, 99 insertions(+), 19 deletions(-)

diff --git a/packages/backend-app-api/config.d.ts b/packages/backend-app-api/config.d.ts
index 2371623b82..3625e9323f 100644
--- a/packages/backend-app-api/config.d.ts
+++ b/packages/backend-app-api/config.d.ts
@@ -34,16 +34,20 @@ export interface Config {
        * unless you configure credentials for service calls.
        */
       dangerouslyDisableDefaultAuthPolicy?: boolean;
-      rateLimit?: {
-        /**
-         * Limit each IP to max requests per window
-         */
-        max?: number;
-        /**
-         * The duration for which the rate limit is enforced
-         */
-        window?: HumanDuration;
-      };
+      rateLimit?:
+        | false
+        | {
+            /**
+             * Limit each IP to max requests per window
+             */
+            max?: number;
+            /**
+             * The duration for which the rate limit is enforced
+             */
+            window?: HumanDuration;
+
+            disable?: true;
+          };
     };
   };
 
diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.test.ts b/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.test.ts
index e28179e1ad..94109ccfc0 100644
--- a/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.test.ts
+++ b/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.test.ts
@@ -219,6 +219,8 @@ it('do not limit authenticated requests', async () => {
       .send()
       .expect(200);
   }
+
+  jest.useRealTimers();
 });
 
 it('limit the number of unauthenticated requests', async () => {
@@ -302,3 +304,62 @@ it('limit the number of unauthenticated requests', async () => {
 
   jest.useRealTimers();
 });
+
+it('skip limiting requests when the rate limit is disabled', async () => {
+  const now = Date.now();
+  jest.useFakeTimers({ now });
+  const max = 2;
+  const cacheMock = mockServices.cache.mock();
+  const twoMinutesInMilliseconds = 2 * 60 * 1000;
+  const resetTime = now + twoMinutesInMilliseconds;
+
+  // Simulate that the totalHits exceeds the configured max value
+  cacheMock.get.mockResolvedValue({ totalHits: max + 1, resetTime });
+
+  const { app: app1, barrier: barrier1 } = setup({
+    cache: cacheMock,
+    config: {
+      backend: {
+        auth: {
+          rateLimit: {
+            max,
+            window: { minutes: 2 },
+            disabled: true,
+          },
+        },
+      },
+    },
+  });
+
+  barrier1.addAuthPolicy({ allow: 'unauthenticated', path: '/public' });
+
+  // exceed the rate limit for authenticated requests
+  for (let i = 0; i < max + 1; i += 1) {
+    await request(app1)
+      .get('/public')
+      .set('authorization', mockCredentials.user.header())
+      .send()
+      .expect(200);
+  }
+
+  // Simulate that the totalHits exceeds the default max value
+  cacheMock.get.mockResolvedValue({ totalHits: 61, resetTime });
+
+  const { app: app2, barrier: barrier2 } = setup({
+    cache: cacheMock,
+    config: {
+      backend: {
+        auth: {
+          rateLimit: false,
+        },
+      },
+    },
+  });
+
+  barrier2.addAuthPolicy({ allow: 'unauthenticated', path: '/public' });
+
+  // exceed the rate limit for authenticated requests
+  await request(app2).get('/public').send().expect(200);
+
+  jest.useRealTimers();
+});
diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts b/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts
index 16e0203155..aca7838804 100644
--- a/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts
+++ b/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts
@@ -65,24 +65,39 @@ export function createCredentialsBarrier(options: {
   const unauthenticatedPredicates = new Array<(path: string) => boolean>();
   const cookiePredicates = new Array<(path: string) => boolean>();
 
-  // Default rate limit is 60 requests per 1 minute
-  const max = config?.has('backend.auth.rateLimit.max')
-    ? config.getNumber('backend.auth.rateLimit.max')
-    : 60;
+  const rateLimitConfig = config.getOptional('backend.auth.rateLimit');
 
-  const duration = config?.has('backend.auth.rateLimit.window')
-    ? readDurationFromConfig(config.getConfig('backend.auth.rateLimit.window'))
-    : undefined;
+  const disabled =
+    rateLimitConfig === false ||
+    (typeof rateLimitConfig === 'object' &&
+      config?.getOptionalBoolean('backend.auth.rateLimit.disabled') === true);
+
+  const duration =
+    typeof rateLimitConfig === 'object' &&
+    config?.has('backend.auth.rateLimit.window')
+      ? readDurationFromConfig(
+          config.getConfig('backend.auth.rateLimit.window'),
+        )
+      : undefined;
 
-  // Default rate limit window is 1 minute
   const windowMs = duration ? durationToMilliseconds(duration) : 1 * 60 * 1000;
 
+  const max =
+    typeof rateLimitConfig === 'object' &&
+    config?.has('backend.auth.rateLimit.max')
+      ? config.getNumber('backend.auth.rateLimit.max')
+      : 60;
+
+  // Default rate limit is 60 requests per 1 minute
   const limiter = rateLimit({
     windowMs,
-    max,
+    limit: max,
     standardHeaders: true, // Return rate limit info in the `RateLimit-*` headers
     legacyHeaders: false, // Disable the `X-RateLimit-*` headers,
     store: RateLimitStore.fromOptions({ cache }),
+    skip() {
+      return disabled;
+    },
   });
 
   const middleware: RequestHandler = (req, res, next) => {

From e65c9eb8b717a66875b8143c4728a018159a44ab Mon Sep 17 00:00:00 2001
From: Camila Belo <camilaibs@gmail.com>
Date: Tue, 19 Mar 2024 11:40:41 +0100
Subject: [PATCH 7/8] refactor: change config place and visibility

Signed-off-by: Camila Belo <camilaibs@gmail.com>
---
 packages/backend-app-api/config.d.ts          | 31 ++++++++++---------
 .../createCredentialsBarrier.test.ts          | 26 +++++++---------
 .../httpRouter/createCredentialsBarrier.ts    | 13 ++++----
 .../httpRouter/rateLimitStore.test.ts         |  7 +++--
 .../httpRouter/rateLimitStore.ts              |  2 +-
 5 files changed, 42 insertions(+), 37 deletions(-)

diff --git a/packages/backend-app-api/config.d.ts b/packages/backend-app-api/config.d.ts
index 3625e9323f..cbf65849be 100644
--- a/packages/backend-app-api/config.d.ts
+++ b/packages/backend-app-api/config.d.ts
@@ -34,21 +34,24 @@ export interface Config {
        * unless you configure credentials for service calls.
        */
       dangerouslyDisableDefaultAuthPolicy?: boolean;
-      rateLimit?:
-        | false
-        | {
-            /**
-             * Limit each IP to max requests per window
-             */
-            max?: number;
-            /**
-             * The duration for which the rate limit is enforced
-             */
-            window?: HumanDuration;
-
-            disable?: true;
-          };
     };
+    /** @visibility frontend */
+    rateLimit?:
+      | false
+      | {
+          /**
+           * Limit each IP to max requests per window, defaults to 60 requests.
+           */
+          max?: number;
+          /**
+           * The duration for which the rate limit is enforced, defaults to 1 minute.
+           */
+          window?: HumanDuration;
+          /**
+           * Disable the rate limit verification.
+           */
+          disable?: true;
+        };
   };
 
   /** Discovery options. */
diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.test.ts b/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.test.ts
index 94109ccfc0..5beeaa3a8e 100644
--- a/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.test.ts
+++ b/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.test.ts
@@ -188,8 +188,8 @@ it('do not limit authenticated requests', async () => {
   const max = 2;
   const configMock = {
     backend: {
-      auth: {
-        rateLimit: {
+      rateLimit: {
+        unauthorized: {
           max, // 2 requests per window
           window: { minutes: 2 }, // rate limit window expiration time,
         },
@@ -237,8 +237,8 @@ it('limit the number of unauthenticated requests', async () => {
 
   const configMock = {
     backend: {
-      auth: {
-        rateLimit: {
+      rateLimit: {
+        unauthorized: {
           max: 2, // 2 requests per window
           window: { minutes: 2 }, // rate limit window expiration time,
         },
@@ -279,25 +279,25 @@ it('limit the number of unauthenticated requests', async () => {
   expect(cacheMock.get).toHaveBeenCalledTimes(4);
   expect(cacheMock.set).toHaveBeenNthCalledWith(
     1,
-    `rl_${randomIp}`, // rl is the default prefix for the rate limit store
+    `unauthorized_rate_limit_${randomIp}`, // unauthorized_rate_limit_ is the default prefix for the rate limit store
     { resetTime, totalHits: 1 },
     { ttl: twoMinutesInMilliseconds },
   );
   expect(cacheMock.set).toHaveBeenNthCalledWith(
     2,
-    `rl_${randomIp}`,
+    `unauthorized_rate_limit_${randomIp}`,
     { resetTime, totalHits: 2 },
     { ttl: twoMinutesInMilliseconds },
   );
   expect(cacheMock.set).toHaveBeenNthCalledWith(
     3,
-    `rl_${randomIp}`,
+    `unauthorized_rate_limit_${randomIp}`,
     { resetTime, totalHits: 3 },
     { ttl: twoMinutesInMilliseconds },
   );
   expect(cacheMock.set).toHaveBeenNthCalledWith(
     4,
-    `rl_${randomIp}`,
+    `unauthorized_rate_limit_${randomIp}`,
     { resetTime, totalHits: 1 },
     { ttl: twoMinutesInMilliseconds },
   );
@@ -320,8 +320,8 @@ it('skip limiting requests when the rate limit is disabled', async () => {
     cache: cacheMock,
     config: {
       backend: {
-        auth: {
-          rateLimit: {
+        rateLimit: {
+          unauthorized: {
             max,
             window: { minutes: 2 },
             disabled: true,
@@ -333,7 +333,6 @@ it('skip limiting requests when the rate limit is disabled', async () => {
 
   barrier1.addAuthPolicy({ allow: 'unauthenticated', path: '/public' });
 
-  // exceed the rate limit for authenticated requests
   for (let i = 0; i < max + 1; i += 1) {
     await request(app1)
       .get('/public')
@@ -349,8 +348,8 @@ it('skip limiting requests when the rate limit is disabled', async () => {
     cache: cacheMock,
     config: {
       backend: {
-        auth: {
-          rateLimit: false,
+        rateLimit: {
+          unauthorized: false,
         },
       },
     },
@@ -358,7 +357,6 @@ it('skip limiting requests when the rate limit is disabled', async () => {
 
   barrier2.addAuthPolicy({ allow: 'unauthenticated', path: '/public' });
 
-  // exceed the rate limit for authenticated requests
   await request(app2).get('/public').send().expect(200);
 
   jest.useRealTimers();
diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts b/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts
index aca7838804..b47b926576 100644
--- a/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts
+++ b/packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts
@@ -65,18 +65,19 @@ export function createCredentialsBarrier(options: {
   const unauthenticatedPredicates = new Array<(path: string) => boolean>();
   const cookiePredicates = new Array<(path: string) => boolean>();
 
-  const rateLimitConfig = config.getOptional('backend.auth.rateLimit');
+  const rateLimitConfig = config.getOptional('backend.rateLimit.unauthorized');
 
   const disabled =
     rateLimitConfig === false ||
     (typeof rateLimitConfig === 'object' &&
-      config?.getOptionalBoolean('backend.auth.rateLimit.disabled') === true);
+      config?.getOptionalBoolean('backend.rateLimit.unauthorized.disabled') ===
+        true);
 
   const duration =
     typeof rateLimitConfig === 'object' &&
-    config?.has('backend.auth.rateLimit.window')
+    config?.has('backend.rateLimit.unauthorized.window')
       ? readDurationFromConfig(
-          config.getConfig('backend.auth.rateLimit.window'),
+          config.getConfig('backend.rateLimit.unauthorized.window'),
         )
       : undefined;
 
@@ -84,8 +85,8 @@ export function createCredentialsBarrier(options: {
 
   const max =
     typeof rateLimitConfig === 'object' &&
-    config?.has('backend.auth.rateLimit.max')
-      ? config.getNumber('backend.auth.rateLimit.max')
+    config?.has('backend.rateLimit.unauthorized.max')
+      ? config.getNumber('backend.rateLimit.unauthorized.max')
       : 60;
 
   // Default rate limit is 60 requests per 1 minute
diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.test.ts b/packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.test.ts
index 44035fc62e..22ab7c816d 100644
--- a/packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.test.ts
+++ b/packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.test.ts
@@ -18,13 +18,12 @@ import { ServiceMock, mockServices } from '@backstage/backend-test-utils';
 import { RateLimitStore } from './rateLimitStore';
 import { CacheService } from '@backstage/backend-plugin-api';
 
-jest.setSystemTime(Date.parse('2024-03-15T08:39:11.869Z'));
-
 describe('RateLimitStore', () => {
   let cacheServiceMock: ServiceMock<CacheService>;
   let rateLimitStore: RateLimitStore;
 
   beforeEach(() => {
+    jest.useFakeTimers({ now: Date.parse('2024-03-15T08:39:11.869Z') });
     jest.clearAllMocks();
     cacheServiceMock = mockServices.cache.mock();
     rateLimitStore = RateLimitStore.fromOptions({
@@ -33,6 +32,10 @@ describe('RateLimitStore', () => {
     });
   });
 
+  afterEach(() => {
+    jest.useRealTimers();
+  });
+
   it('should initialize with default options', () => {
     expect(rateLimitStore.windowMs).toBe(1 * 60 * 1000);
   });
diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.ts b/packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.ts
index 423a1012a9..6286522a2a 100644
--- a/packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.ts
+++ b/packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.ts
@@ -51,7 +51,7 @@ export class RateLimitStore implements Store {
   #cache: CacheService;
 
   private constructor(options: CacheStoreOptions) {
-    this.prefix = options.prefix ?? 'rl_';
+    this.prefix = options.prefix ?? 'unauthorized_rate_limit_';
     this.#cache = options.cache;
   }
 

From 7c2eac7f0e3f5920ac23a465ebaee90d454ccc51 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Tue, 19 Mar 2024 13:48:06 +0100
Subject: [PATCH 8/8] Apply suggestions from code review

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
---
 .changeset/new-spoons-stare.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.changeset/new-spoons-stare.md b/.changeset/new-spoons-stare.md
index 04f7ef303e..49a21b1dcd 100644
--- a/.changeset/new-spoons-stare.md
+++ b/.changeset/new-spoons-stare.md
@@ -1,5 +1,5 @@
 ---
-'@backstage/backend-app-api': minor
+'@backstage/backend-app-api': patch
 ---
 
-Adds an initial rate-limiting implementation so that any incoming requests that have a 'none' principal are rate-limited automatically.
+Adds an initial rate-limiting implementation so that any incoming requests that have a `'none'` principal are rate-limited automatically.