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

fix: review findings

Browse Source 
Signed-off-by: Heikki Hellgren <heikki.hellgren@op.fi>
This commit is contained in:
Heikki Hellgren
2024-11-26 14:33:30 +02:00
committed by Hellgren Heikki
parent 279c15cb5d
commit d6bd7a540d
9 changed files with 83 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/famous-terms-rescue.md
+2 -1
View File
@@ -4,11 +4,12 @@
Added new rate limit middleware to allow rate limiting requests to the backend
If you are using the `configure` callback of the root HTTP router service and do NOT call `applyDefaults()` inside it, please see [the relevant changes](https://github.com/backstage/backstage/pull/26725/files#diff-86ad1b6a694dd250823aee39d410428dd837c9d9a04ca8c33bd1081fbe3f22af) that were made, to see if you want to apply them as well to your custom configuration.
Rate limiting can be turned on by adding the following configuration to `app-config.yaml`:
```yaml
backend:
rateLimit:
window: 60000
window: 6000ms
incomingRequestLimit: 100
```
app-config.yaml
+6
View File
@@ -36,6 +36,12 @@ backend:
# keys:
# - secret: ${BACKEND_SECRET}
# Used for testing rate limiting locally
# rateLimit:
# windowMs: 60000
# incomingRequestLimit: 1
# ipAllowList: []
auth:
# TODO: once plugins have been migrated we can remove this, but right now it
# is require for the backend-next to work in this repo
packages/backend-defaults/config.d.ts
Vendored
+4 -4
View File
@@ -791,10 +791,10 @@ export interface Config {
};
/**
* Rate limiting options
* Rate limiting options. Defining this as `true` will enable rate limiting with default values.
*/
rateLimit?:
| false
| true
| {
store?:
| {
@@ -806,9 +806,9 @@ export interface Config {
};
/**
* Time frame in milliseconds or as human duration for which requests are checked/remembered.
* Defaults to 6000ms.
* Defaults to one minute.
*/
window?: number | HumanDuration;
window?: string | HumanDuration;
/**
* The maximum number of connections to allow during the `window` before rate limiting the client.
* Defaults to 5.
packages/backend-defaults/package.json
+1 -1
View File
@@ -168,7 +168,7 @@
"cron": "^3.0.0",
"express": "^4.17.1",
"express-promise-router": "^4.1.0",
"express-rate-limit": "^7.4.0",
"express-rate-limit": "^7.5.0",
"fs-extra": "^11.2.0",
"git-url-parse": "^15.0.0",
"helmet": "^6.0.0",
packages/backend-defaults/src/entrypoints/rootHttpRouter/http/MiddlewareFactory.ts
+35 -27
View File
@@ -44,8 +44,8 @@ import {
} from '@backstage/errors';
import { applyInternalErrorFilter } from './applyInternalErrorFilter';
import { rateLimit } from 'express-rate-limit';
import { Config } from '@backstage/config';
import { durationToMilliseconds, HumanDuration } from '@backstage/types';
import { readDurationFromConfig } from '@backstage/config';
import { durationToMilliseconds } from '@backstage/types';
import { RateLimitStoreFactory } from './RateLimitStoreFactory';
type LogMeta = {
@@ -242,44 +242,50 @@ export class MiddlewareFactory {
* @returns An Express request handler
*/
rateLimit(): RequestHandler {
const conf = this.#config.getOptional<boolean | Config>(
'backend.rateLimit',
);
if (!conf || typeof conf !== 'object') {
const enabled = this.#config.has('backend.rateLimit');
if (!enabled) {
return (_req: Request, _res: Response, next: NextFunction) => {
next();
};
}
const rateLimitOptions = conf as Config;
const window = rateLimitOptions.getOptional<number | HumanDuration>(
'window',
);
let windowMs: number | undefined;
if (window !== undefined) {
if (typeof window === 'number') {
windowMs = window;
} else if (typeof window === 'object' && !Array.isArray(window)) {
windowMs = durationToMilliseconds(window);
} else {
throw new Error(
`Invalid configuration backend.rateLimit.window: ${window}, expected milliseconds number or HumanDuration object`,
);
}
const useDefaults = this.#config.getOptional('backend.rateLimit') === true;
const rateLimitOptions = useDefaults
? undefined
: this.#config.getOptionalConfig('backend.rateLimit');
let windowMs: number = 60000;
if (rateLimitOptions && rateLimitOptions.has('window')) {
const windowDuration = readDurationFromConfig(rateLimitOptions, {
key: 'window',
});
windowMs = durationToMilliseconds(windowDuration);
}
const ipAllowList = rateLimitOptions.getOptionalStringArray(
const ipAllowList = rateLimitOptions?.getOptionalStringArray(
'ipAllowList',
) ?? ['127.0.0.1', '0:0:0:0:0:0:0:1', '::1'];
return rateLimit({
windowMs,
limit: rateLimitOptions.getOptionalNumber('incomingRequestLimit'),
skipSuccessfulRequests: rateLimitOptions.getOptionalBoolean(
limit: rateLimitOptions?.getOptionalNumber('incomingRequestLimit'),
skipSuccessfulRequests: rateLimitOptions?.getOptionalBoolean(
'skipSuccessfulRequests',
),
message: {
error: {
name: 'Error',
message: `Too many requests, please try again later`,
},
response: {
statusCode: 429,
},
},
statusCode: 429,
skipFailedRequests:
rateLimitOptions.getOptionalBoolean('skipFailedRequests'),
passOnStoreError: rateLimitOptions.getOptionalBoolean('passOnStoreError'),
rateLimitOptions?.getOptionalBoolean('skipFailedRequests'),
passOnStoreError:
rateLimitOptions?.getOptionalBoolean('passOnStoreError'),
keyGenerator(req, _res): string {
if (!req.ip) {
return req.socket.remoteAddress!;
@@ -298,7 +304,9 @@ export class MiddlewareFactory {
validate: {
trustProxy: false,
},
store: new RateLimitStoreFactory(this.#config).create(),
store: useDefaults
? undefined
: RateLimitStoreFactory.create(this.#config),
});
}
packages/backend-defaults/src/entrypoints/rootHttpRouter/http/RateLimitStoreFactory.test.ts
+20 -15
View File
@@ -13,13 +13,25 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { mockServices, TestCaches } from '@backstage/backend-test-utils';
import { mockServices } from '@backstage/backend-test-utils';
import { RateLimitStoreFactory } from './RateLimitStoreFactory';
import { RedisStore } from 'rate-limit-redis';
jest.mock('@keyv/redis', () => {
const Actual = jest.requireActual('@keyv/redis');
return {
...Actual,
__esModule: true,
default: jest.fn(() => {
return {
getClient: jest.fn(() => ({
sendCommand: jest.fn().mockReturnValue('mock'),
})),
};
}),
};
});
describe('CacheRateLimitStoreFactory', () => {
const caches = TestCaches.create();
afterEach(jest.clearAllMocks);
it('should return undefined store with auto configuration if redis is not available', () => {
@@ -32,31 +44,24 @@ describe('CacheRateLimitStoreFactory', () => {
},
},
});
const factory = new RateLimitStoreFactory(config);
const store = factory.create();
const store = RateLimitStoreFactory.create(config);
expect(store).toBeUndefined();
});
it('should return redis store if configured explicitly', async () => {
if (!caches.supports('REDIS_7')) {
return;
}
const conf = await caches.init('REDIS_7');
const config = mockServices.rootConfig({
data: {
backend: {
rateLimit: {
store: {
client: 'redis',
connection: conf.connection,
connection: 'redis://localhost:6379',
},
},
},
},
});
const factory = new RateLimitStoreFactory(config);
const store = factory.create();
expect(store).toBeInstanceOf(RedisStore);
const store = RateLimitStoreFactory.create(config);
expect(store).not.toBeUndefined();
});
});
packages/backend-defaults/src/entrypoints/rootHttpRouter/http/RateLimitStoreFactory.ts
+8 -8
View File
@@ -15,7 +15,6 @@
*/
import { Config } from '@backstage/config';
import type { Store } from 'express-rate-limit';
import KeyvRedis from '@keyv/redis';
import { RedisStore } from 'rate-limit-redis';
/**
@@ -24,10 +23,8 @@ import { RedisStore } from 'rate-limit-redis';
* @internal
*/
export class RateLimitStoreFactory {
constructor(private readonly config: Config) {}
create(): Store | undefined {
const store = this.config.getOptionalConfig('backend.rateLimit.store');
static create(config: Config): Store | undefined {
const store = config.getOptionalConfig('backend.rateLimit.store');
if (!store) {
return undefined;
}
@@ -40,12 +37,15 @@ export class RateLimitStoreFactory {
}
}
redis(storeConfig: Config): Store {
private static redis(storeConfig: Config): Store {
const connectionString = storeConfig.getString('connection');
const KeyvRedis = require('@keyv/redis').default;
const keyv = new KeyvRedis(connectionString);
return new RedisStore({
// Keyv uses ioredis under the hood
sendCommand: (...args: string[]) => keyv.redis.call(...args),
sendCommand: async (...args: string[]) => {
const client = await keyv.getClient();
return client.sendCommand(args);
},
});
}
}
packages/backend-defaults/src/entrypoints/rootHttpRouter/rootHttpRouterServiceFactory.ts
+1 -1
View File
@@ -117,11 +117,11 @@ const rootHttpRouterServiceFactoryWithOptions = (
if (trustProxy !== undefined) {
app.set('trust proxy', trustProxy);
}
app.use(middleware.rateLimit());
app.use(middleware.helmet());
app.use(middleware.cors());
app.use(middleware.compression());
app.use(middleware.logging());
app.use(middleware.rateLimit());
app.use(healthRouter);
app.use(routes);
app.use(middleware.notFound());
yarn.lock
+6 -6
View File
@@ -3632,7 +3632,7 @@ __metadata:
cron: "npm:^3.0.0"
express: "npm:^4.17.1"
express-promise-router: "npm:^4.1.0"
express-rate-limit: "npm:^7.4.0"
express-rate-limit: "npm:^7.5.0"
fs-extra: "npm:^11.2.0"
git-url-parse: "npm:^15.0.0"
helmet: "npm:^6.0.0"
@@ -30045,12 +30045,12 @@ __metadata:
languageName: node
linkType: hard
"express-rate-limit@npm:^7.4.0":
version: 7.4.0
resolution: "express-rate-limit@npm:7.4.0"
"express-rate-limit@npm:^7.5.0":
version: 7.5.0
resolution: "express-rate-limit@npm:7.5.0"
peerDependencies:
express: 4 || 5 || ^5.0.0-beta.1
checksum: 10/33178c652bb1472aad2022194b5cd7963bd3e74d3eaf5e49eb1491a968fdce54551cc76b097ac10d3a1646d62cec2e6f2405ccef5ef5b60152a0c4a148749a4d
express: ^4.11 || 5 || ^5.0.0-beta.1
checksum: 10/eff34c83bf586789933a332a339b66649e2cca95c8e977d193aa8bead577d3182ac9f0e9c26f39389287539b8038890ff023f910b54ebb506a26a2ce135b92ca
languageName: node
linkType: hard