mirror of
https://github.com/n8n-io/n8n.git
synced 2024-11-10 06:34:05 -08:00
feat(core): Rate-limit login endpoint to mitigate brute force password guessing attacks (#9028)
This commit is contained in:
parent
4668db20fb
commit
a6446fe057
|
@ -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');
|
||||
|
|
|
@ -1,5 +1,4 @@
|
|||
import { Response } from 'express';
|
||||
import { rateLimit } from 'express-rate-limit';
|
||||
import validator from 'validator';
|
||||
|
||||
import { AuthService } from '@/auth/auth.service';
|
||||
|
@ -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';
|
||||
|
@ -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(
|
||||
|
@ -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(
|
||||
|
|
|
@ -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 =
|
||||
|
@ -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);
|
||||
};
|
||||
|
|
|
@ -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';
|
||||
|
@ -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) => {
|
||||
|
@ -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)] : []),
|
||||
|
|
|
@ -19,6 +19,7 @@ export interface RouteMetadata {
|
|||
middlewares: RequestHandler[];
|
||||
usesTemplates: boolean;
|
||||
skipAuth: boolean;
|
||||
rateLimit: boolean;
|
||||
}
|
||||
|
||||
export type Controller = Record<
|
||||
|
|
40
packages/cli/test/unit/decorators/registerController.test.ts
Normal file
40
packages/cli/test/unit/decorators/registerController.test.ts
Normal file
|
@ -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…
Reference in a new issue