paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "[Auth] Create unauthorized requests rate limiting"

Browse Source 
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
This commit is contained in:
Fredrik Adelöw
2024-03-22 21:14:35 +01:00
parent f9dac1ee66
commit e848644f5a
9 changed files with 12 additions and 585 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/eleven-llamas-heal.md
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/backend-app-api': patch
---
Temporarily revert the rate limiting
packages/backend-app-api/config.d.ts
Vendored
-19
View File
@@ -14,8 +14,6 @@
* limitations under the License.
*/
import { HumanDuration } from '@backstage/types';
export interface Config {
backend?: {
auth?: {
@@ -35,23 +33,6 @@ export interface Config {
*/
dangerouslyDisableDefaultAuthPolicy?: boolean;
};
/** @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. */
packages/backend-app-api/package.json
-1
View File
@@ -64,7 +64,6 @@
"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",
packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.test.ts
+2 -190
View File
@@ -21,21 +21,18 @@ 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(options?: { cache?: CacheService; config?: JsonObject }) {
function setup() {
const barrier = createCredentialsBarrier({
httpAuth: mockServices.httpAuth({
defaultCredentials: mockCredentials.none(),
}),
config: mockServices.rootConfig({ data: options?.config }),
cache: options?.cache ?? mockServices.cache.mock(),
config: mockServices.rootConfig(),
});
const app = express();
@@ -176,188 +173,3 @@ 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: {
rateLimit: {
unauthorized: {
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);
}
jest.useRealTimers();
});
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: {
rateLimit: {
unauthorized: {
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,
`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,
`unauthorized_rate_limit_${randomIp}`,
{ resetTime, totalHits: 2 },
{ ttl: twoMinutesInMilliseconds },
);
expect(cacheMock.set).toHaveBeenNthCalledWith(
3,
`unauthorized_rate_limit_${randomIp}`,
{ resetTime, totalHits: 3 },
{ ttl: twoMinutesInMilliseconds },
);
expect(cacheMock.set).toHaveBeenNthCalledWith(
4,
`unauthorized_rate_limit_${randomIp}`,
{ resetTime, totalHits: 1 },
{ ttl: twoMinutesInMilliseconds },
);
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: {
rateLimit: {
unauthorized: {
max,
window: { minutes: 2 },
disabled: true,
},
},
},
},
});
barrier1.addAuthPolicy({ allow: 'unauthenticated', path: '/public' });
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: {
rateLimit: {
unauthorized: false,
},
},
},
});
barrier2.addAuthPolicy({ allow: 'unauthenticated', path: '/public' });
await request(app2).get('/public').send().expect(200);
jest.useRealTimers();
});
packages/backend-app-api/src/services/implementations/httpRouter/createCredentialsBarrier.ts
+3 -45
View File
@@ -15,17 +15,12 @@
*/
import {
CacheService,
HttpAuthService,
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';
import { RateLimitStore } from './rateLimitStore';
export function createPathPolicyPredicate(policyPath: string) {
if (policyPath === '/' || policyPath === '*') {
@@ -44,12 +39,11 @@ export function createPathPolicyPredicate(policyPath: string) {
export function createCredentialsBarrier(options: {
httpAuth: HttpAuthService;
config: RootConfigService;
cache: CacheService;
}): {
middleware: RequestHandler;
addAuthPolicy: (policy: HttpRouterServiceAuthPolicy) => void;
} {
const { httpAuth, config, cache } = options;
const { httpAuth, config } = options;
const disableDefaultAuthPolicy = config.getOptionalBoolean(
'backend.auth.dangerouslyDisableDefaultAuthPolicy',
@@ -65,49 +59,13 @@ export function createCredentialsBarrier(options: {
const unauthenticatedPredicates = new Array<(path: string) => boolean>();
const cookiePredicates = new Array<(path: string) => boolean>();
const rateLimitConfig = config.getOptional('backend.rateLimit.unauthorized');
const disabled =
rateLimitConfig === false ||
(typeof rateLimitConfig === 'object' &&
config?.getOptionalBoolean('backend.rateLimit.unauthorized.disabled') ===
true);
const duration =
typeof rateLimitConfig === 'object' &&
config?.has('backend.rateLimit.unauthorized.window')
? readDurationFromConfig(
config.getConfig('backend.rateLimit.unauthorized.window'),
)
: undefined;
const windowMs = duration ? durationToMilliseconds(duration) : 1 * 60 * 1000;
const max =
typeof rateLimitConfig === 'object' &&
config?.has('backend.rateLimit.unauthorized.max')
? config.getNumber('backend.rateLimit.unauthorized.max')
: 60;
// Default rate limit is 60 requests per 1 minute
const limiter = rateLimit({
windowMs,
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) => {
const middleware: RequestHandler = (req, _, next) => {
const allowsUnauthenticated = unauthenticatedPredicates.some(predicate =>
predicate(req.path),
);
if (allowsUnauthenticated) {
limiter(req, res, next);
next();
return;
}
packages/backend-app-api/src/services/implementations/httpRouter/httpRouterServiceFactory.ts
+2 -14
View File
@@ -39,32 +39,20 @@ 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,
cache,
plugin,
rootHttpRouter,
lifecycle,
}) {
async factory({ httpAuth, config, 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,
cache,
});
const credentialsBarrier = createCredentialsBarrier({ httpAuth, config });
router.use(createLifecycleMiddleware({ lifecycle }));
router.use(credentialsBarrier.middleware);
packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.test.ts
-187
View File
@@ -1,187 +0,0 @@
/*
* 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';
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({
prefix: 'rl_',
cache: cacheServiceMock,
});
});
afterEach(() => {
jest.useRealTimers();
});
it('should initialize with default options', () => {
expect(rateLimitStore.windowMs).toBe(1 * 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),
);
});
});
packages/backend-app-api/src/services/implementations/httpRouter/rateLimitStore.ts
-119
View File
@@ -1,119 +0,0 @@
/*
* 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: 60 requests per minute
*/
windowMs: number = 1 * 60 * 1000;
prefix: string;
#cache: CacheService;
private constructor(options: CacheStoreOptions) {
this.prefix = options.prefix ?? 'unauthorized_rate_limit_';
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));
}
}
yarn.lock
-10
View File
@@ -3247,7 +3247,6 @@ __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
@@ -27608,15 +27607,6 @@ __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"