Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): Rate-limit login endpoint to mitigate brute force password guessing attacks #9028

Merged
merged 5 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/cli/src/controllers/auth.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export class AuthController {
) {}

/** Log in a user */
@Post('/login', { skipAuth: true })
@Post('/login', { skipAuth: true, rateLimit: true })
async login(req: LoginRequest, res: Response): Promise<PublicUser | undefined> {
const { email, password, mfaToken, mfaRecoveryCode } = req.body;
if (!email) throw new ApplicationError('Email is required to log in');
Expand Down
14 changes: 2 additions & 12 deletions packages/cli/src/controllers/passwordReset.controller.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { Response } from 'express';
import { rateLimit } from 'express-rate-limit';
import validator from 'validator';

import { AuthService } from '@/auth/auth.service';
Expand All @@ -10,7 +9,7 @@ import { PasswordResetRequest } from '@/requests';
import { isSamlCurrentAuthenticationMethod } from '@/sso/ssoHelpers';
import { UserService } from '@/services/user.service';
import { License } from '@/License';
import { RESPONSE_ERROR_MESSAGES, inTest } from '@/constants';
import { RESPONSE_ERROR_MESSAGES } from '@/constants';
import { MfaService } from '@/Mfa/mfa.service';
import { Logger } from '@/Logger';
import { ExternalHooks } from '@/ExternalHooks';
Expand All @@ -23,12 +22,6 @@ import { NotFoundError } from '@/errors/response-errors/not-found.error';
import { UnprocessableRequestError } from '@/errors/response-errors/unprocessable.error';
import { UserRepository } from '@/databases/repositories/user.repository';

const throttle = rateLimit({
windowMs: 5 * 60 * 1000, // 5 minutes
limit: 5, // Limit each IP to 5 requests per `window` (here, per 5 minutes).
message: { message: 'Too many requests' },
});

@RestController()
export class PasswordResetController {
constructor(
Expand All @@ -48,10 +41,7 @@ export class PasswordResetController {
/**
* Send a password reset email.
*/
@Post('/forgot-password', {
middlewares: !inTest ? [throttle] : [],
skipAuth: true,
})
@Post('/forgot-password', { skipAuth: true, rateLimit: true })
async forgotPassword(req: PasswordResetRequest.Email) {
if (!this.mailer.isEmailSetUp) {
this.logger.debug(
Expand Down
3 changes: 3 additions & 0 deletions packages/cli/src/decorators/Route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ interface RouteOptions {
usesTemplates?: boolean;
/** When this flag is set to true, auth cookie isn't validated, and req.user will not be set */
skipAuth?: boolean;
/** When this flag is set to true, calls to this endpoint is rate limited to a max of 5 over a window of 5 minutes **/
rateLimit?: boolean;
}

const RouteFactory =
Expand All @@ -23,6 +25,7 @@ const RouteFactory =
handlerName: String(handlerName),
usesTemplates: options.usesTemplates ?? false,
skipAuth: options.skipAuth ?? false,
rateLimit: options.rateLimit ?? false,
});
Reflect.defineMetadata(CONTROLLER_ROUTES, routes, controllerClass);
};
Expand Down
19 changes: 18 additions & 1 deletion packages/cli/src/decorators/registerController.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { Container } from 'typedi';
import { Router } from 'express';
import type { Application, Request, Response, RequestHandler } from 'express';
import { rateLimit as expressRateLimit } from 'express-rate-limit';
import type { Scope } from '@n8n/permissions';
import { ApplicationError } from 'n8n-workflow';
import type { Class } from 'n8n-core';

import { AuthService } from '@/auth/auth.service';
import config from '@/config';
import { inE2ETests, inTest } from '@/constants';
import type { BooleanLicenseFeature } from '@/Interfaces';
import { License } from '@/License';
import type { AuthenticatedRequest } from '@/requests';
Expand All @@ -26,6 +28,12 @@ import type {
ScopeMetadata,
} from './types';

const throttle = expressRateLimit({
windowMs: 5 * 60 * 1000, // 5 minutes
limit: 5, // Limit each IP to 5 requests per `window` (here, per 5 minutes).
message: { message: 'Too many requests' },
});

export const createLicenseMiddleware =
(features: BooleanLicenseFeature[]): RequestHandler =>
(_req, res, next) => {
Expand Down Expand Up @@ -94,13 +102,22 @@ export const registerController = (app: Application, controllerClass: Class<obje
const authService = Container.get(AuthService);

routes.forEach(
({ method, path, middlewares: routeMiddlewares, handlerName, usesTemplates, skipAuth }) => {
({
method,
path,
middlewares: routeMiddlewares,
handlerName,
usesTemplates,
skipAuth,
rateLimit,
}) => {
const features = licenseFeatures?.[handlerName] ?? licenseFeatures?.['*'];
const scopes = requiredScopes?.[handlerName] ?? requiredScopes?.['*'];
const handler = async (req: Request, res: Response) =>
await controller[handlerName](req, res);
router[method](
path,
...(!inTest && !inE2ETests && rateLimit ? [throttle] : []),
// eslint-disable-next-line @typescript-eslint/unbound-method
...(skipAuth ? [] : [authService.authMiddleware]),
...(features ? [createLicenseMiddleware(features)] : []),
Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/decorators/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export interface RouteMetadata {
middlewares: RequestHandler[];
usesTemplates: boolean;
skipAuth: boolean;
rateLimit: boolean;
}

export type Controller = Record<
Expand Down
40 changes: 40 additions & 0 deletions packages/cli/test/unit/decorators/registerController.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
jest.mock('@/constants', () => ({
inE2ETests: false,
inTest: false,
}));

import express from 'express';
import { agent as testAgent } from 'supertest';

import { Get, RestController, registerController } from '@/decorators';
import { AuthService } from '@/auth/auth.service';
import { mockInstance } from '../../shared/mocking';

describe('registerController', () => {
@RestController('/test')
class TestController {
@Get('/unlimited', { skipAuth: true })
@Get('/rate-limited', { skipAuth: true, rateLimit: true })
endpoint() {
return { ok: true };
}
}

mockInstance(AuthService);
const app = express();
registerController(app, TestController);
const agent = testAgent(app);

it('should not rate-limit by default', async () => {
for (let i = 0; i < 6; i++) {
await agent.get('/rest/test/unlimited').expect(200);
}
});

it('should rate-limit when configured', async () => {
for (let i = 0; i < 5; i++) {
await agent.get('/rest/test/rate-limited').expect(200);
}
await agent.get('/rest/test/rate-limited').expect(429);
});
});
Loading