diff --git a/packages/@n8n/api-types/src/dto/auth/__tests__/login-request.dto.test.ts b/packages/@n8n/api-types/src/dto/auth/__tests__/login-request.dto.test.ts index 8c9e888007d..008d93a8234 100644 --- a/packages/@n8n/api-types/src/dto/auth/__tests__/login-request.dto.test.ts +++ b/packages/@n8n/api-types/src/dto/auth/__tests__/login-request.dto.test.ts @@ -73,6 +73,14 @@ describe('LoginRequestDto', () => { }, expectedErrorPath: ['password'], }, + { + name: 'emailOrLdapLoginId exceeds max length', + request: { + emailOrLdapLoginId: 'a'.repeat(256), + password: 'securePassword123', + }, + expectedErrorPath: ['emailOrLdapLoginId'], + }, ])('should fail validation for $name', ({ request, expectedErrorPath }) => { const result = LoginRequestDto.safeParse(request); expect(result.success).toBe(false); diff --git a/packages/@n8n/api-types/src/dto/auth/login-request.dto.ts b/packages/@n8n/api-types/src/dto/auth/login-request.dto.ts index d1f6771b9c3..b84d728bd97 100644 --- a/packages/@n8n/api-types/src/dto/auth/login-request.dto.ts +++ b/packages/@n8n/api-types/src/dto/auth/login-request.dto.ts @@ -7,7 +7,7 @@ export class LoginRequestDto extends Z.class({ * is not enforced here. The controller determines whether this is an * email and validates when LDAP is disabled */ - emailOrLdapLoginId: z.string().trim(), + emailOrLdapLoginId: z.string().trim().max(255), password: z.string().min(1), mfaCode: z.string().optional(), mfaRecoveryCode: z.string().optional(), diff --git a/packages/@n8n/api-types/src/dto/password-reset/__tests__/forgot-password-request.dto.test.ts b/packages/@n8n/api-types/src/dto/password-reset/__tests__/forgot-password-request.dto.test.ts index 891d52fdad0..05be0803f3e 100644 --- a/packages/@n8n/api-types/src/dto/password-reset/__tests__/forgot-password-request.dto.test.ts +++ b/packages/@n8n/api-types/src/dto/password-reset/__tests__/forgot-password-request.dto.test.ts @@ -34,6 +34,11 @@ describe('ForgotPasswordRequestDto', () => { request: { email: '' }, expectedErrorPath: ['email'], }, + { + name: 'email exceeds max length', + request: { email: 'a'.repeat(244) + '@example.com' }, + expectedErrorPath: ['email'], + }, ])('should fail validation for $name', ({ request, expectedErrorPath }) => { const result = ForgotPasswordRequestDto.safeParse(request); diff --git a/packages/@n8n/api-types/src/dto/password-reset/forgot-password-request.dto.ts b/packages/@n8n/api-types/src/dto/password-reset/forgot-password-request.dto.ts index f6ab3cfac5b..8e42ea0a570 100644 --- a/packages/@n8n/api-types/src/dto/password-reset/forgot-password-request.dto.ts +++ b/packages/@n8n/api-types/src/dto/password-reset/forgot-password-request.dto.ts @@ -2,5 +2,5 @@ import { z } from 'zod'; import { Z } from 'zod-class'; export class ForgotPasswordRequestDto extends Z.class({ - email: z.string().email(), + email: z.string().email().max(255), }) {} diff --git a/packages/@n8n/decorators/package.json b/packages/@n8n/decorators/package.json index 5481ea9dca3..5f92dd18bd7 100644 --- a/packages/@n8n/decorators/package.json +++ b/packages/@n8n/decorators/package.json @@ -24,7 +24,8 @@ "devDependencies": { "@n8n/typescript-config": "workspace:*", "@types/express": "catalog:", - "@types/lodash": "catalog:" + "@types/lodash": "catalog:", + "zod": "catalog:" }, "dependencies": { "@n8n/constants": "workspace:*", diff --git a/packages/@n8n/decorators/src/controller/__tests__/route.test.ts b/packages/@n8n/decorators/src/controller/__tests__/route.test.ts index b5b7b5241f2..47f19145459 100644 --- a/packages/@n8n/decorators/src/controller/__tests__/route.test.ts +++ b/packages/@n8n/decorators/src/controller/__tests__/route.test.ts @@ -1,6 +1,7 @@ import { Container } from '@n8n/di'; import { ControllerRegistryMetadata } from '../controller-registry-metadata'; +import { createBodyKeyedRateLimiter } from '../rate-limit'; import { Get, Post, Put, Patch, Delete } from '../route'; import type { Controller } from '../types'; @@ -50,7 +51,11 @@ describe('Route Decorators', () => { usesTemplates: true, skipAuth: true, ipRateLimit: { limit: 10, windowMs: 60000 }, - keyedRateLimit: { limit: 10, windowMs: 60000, source: 'body', field: 'email' }, + keyedRateLimit: createBodyKeyedRateLimiter<{ email: string }>({ + limit: 10, + windowMs: 60000, + field: 'email', + }), }) testMethod() {} } @@ -64,7 +69,7 @@ describe('Route Decorators', () => { expect(routeMetadata.usesTemplates).toBe(true); expect(routeMetadata.skipAuth).toBe(true); expect(routeMetadata.ipRateLimit).toEqual({ limit: 10, windowMs: 60000 }); - expect(routeMetadata.keyedRateLimit).toEqual({ + expect(routeMetadata.keyedRateLimit).toMatchObject({ limit: 10, windowMs: 60000, source: 'body', diff --git a/packages/@n8n/decorators/src/controller/index.ts b/packages/@n8n/decorators/src/controller/index.ts index 97a8ffb33c4..bd4670baeb8 100644 --- a/packages/@n8n/decorators/src/controller/index.ts +++ b/packages/@n8n/decorators/src/controller/index.ts @@ -11,7 +11,13 @@ export type { Controller, CorsOptions, Method, - RateLimiterLimits, - KeyedRateLimiterConfig, StaticRouterMetadata, } from './types'; +export { + type RateLimiterLimits, + type BodyKeyedRateLimiterConfig, + type UserKeyedRateLimiterConfig, + type KeyedRateLimiterConfig, + createBodyKeyedRateLimiter, + createUserKeyedRateLimiter, +} from './rate-limit'; diff --git a/packages/@n8n/decorators/src/controller/rate-limit.ts b/packages/@n8n/decorators/src/controller/rate-limit.ts new file mode 100644 index 00000000000..dd45c7e927a --- /dev/null +++ b/packages/@n8n/decorators/src/controller/rate-limit.ts @@ -0,0 +1,76 @@ +export interface RateLimiterLimits { + /** + * The maximum number of requests to allow during the `window` before rate limiting the client. + * @default 5 + */ + limit?: number; + /** + * How long we should remember the requests. + * @default 300_000 (5 minutes) + */ + windowMs?: number; +} + +/** + * Configuration for extracting a key from the request body. + */ +export interface BodyKeyedRateLimiterConfig extends RateLimiterLimits { + /** How to extract key from request */ + source: 'body'; + /** The field name in the request body to use as the key */ + field: string; +} + +/** + * Configuration for extracting a key from the authenticated user. + */ +export interface UserKeyedRateLimiterConfig extends RateLimiterLimits { + /** How to extract key from request */ + source: 'user'; +} + +export type KeyedRateLimiterConfig = BodyKeyedRateLimiterConfig | UserKeyedRateLimiterConfig; + +/** + * Create a body keyed rate limiter configuration. This ends up creating + * a rate limiter that is keyed by the value of the specified field in the + * request body. + * + * @example + * createBodyKeyedRateLimiter({ + * field: 'email', + * limit: 10, + * windowMs: 60000, + * }); + */ +export const createBodyKeyedRateLimiter = ({ + limit, + windowMs, + field, +}: RateLimiterLimits & { + field: keyof T & string; +}): BodyKeyedRateLimiterConfig => ({ + source: 'body', + limit, + windowMs, + field, +}); + +/** + * Create a user keyed rate limiter configuration. This ends up creating + * a rate limiter that is keyed by the authenticated user's ID. + * + * @example + * createUserKeyedRateLimiter({ + * limit: 10, + * windowMs: 60000, + * }); + */ +export const createUserKeyedRateLimiter = ({ + limit, + windowMs, +}: RateLimiterLimits): UserKeyedRateLimiterConfig => ({ + source: 'user', + limit, + windowMs, +}); diff --git a/packages/@n8n/decorators/src/controller/route.ts b/packages/@n8n/decorators/src/controller/route.ts index 5fab341ed87..958731cce1e 100644 --- a/packages/@n8n/decorators/src/controller/route.ts +++ b/packages/@n8n/decorators/src/controller/route.ts @@ -2,13 +2,8 @@ import { Container } from '@n8n/di'; import type { RequestHandler } from 'express'; import { ControllerRegistryMetadata } from './controller-registry-metadata'; -import type { - Controller, - CorsOptions, - Method, - RateLimiterLimits, - KeyedRateLimiterConfig, -} from './types'; +import type { KeyedRateLimiterConfig, RateLimiterLimits } from './rate-limit'; +import type { Controller, CorsOptions, Method } from './types'; interface RouteOptions { middlewares?: RequestHandler[]; diff --git a/packages/@n8n/decorators/src/controller/types.ts b/packages/@n8n/decorators/src/controller/types.ts index 075a64b5a5f..697720c71fa 100644 --- a/packages/@n8n/decorators/src/controller/types.ts +++ b/packages/@n8n/decorators/src/controller/types.ts @@ -3,6 +3,8 @@ import type { Constructable } from '@n8n/di'; import type { Scope } from '@n8n/permissions'; import type { RequestHandler, Router } from 'express'; +import type { KeyedRateLimiterConfig, RateLimiterLimits } from './rate-limit'; + export type Method = 'get' | 'post' | 'put' | 'patch' | 'delete' | 'head' | 'options'; export type Arg = { type: 'body' | 'query' } | { type: 'param'; key: string }; @@ -15,43 +17,6 @@ export interface CorsOptions { maxAge?: number; } -export interface RateLimiterLimits { - /** - * The maximum number of requests to allow during the `window` before rate limiting the client. - * @default 5 - */ - limit?: number; - /** - * How long we should remember the requests. - * @default 300_000 (5 minutes) - */ - windowMs?: number; -} - -/** - * Configuration for extracting a key from the request body. - * @example - * { source: 'body', field: 'email' } - */ -export interface BodyKeyedRateLimiterConfig extends RateLimiterLimits { - /** How to extract key from request */ - source: 'body'; - /** The field name in the request body to use as the key */ - field: string; -} - -/** - * Configuration for extracting a key from the authenticated user. - * @example - * { source: 'user' } - */ -export interface UserKeyedRateLimiterConfig extends RateLimiterLimits { - /** How to extract key from request */ - source: 'user'; -} - -export type KeyedRateLimiterConfig = BodyKeyedRateLimiterConfig | UserKeyedRateLimiterConfig; - export type HandlerName = string; export interface AccessScope { diff --git a/packages/cli/src/__tests__/controller.registry.test.ts b/packages/cli/src/__tests__/controller.registry.test.ts index d7c1341156c..cf0fc17b31e 100644 --- a/packages/cli/src/__tests__/controller.registry.test.ts +++ b/packages/cli/src/__tests__/controller.registry.test.ts @@ -15,11 +15,15 @@ import { Licensed, RestController, RootLevelController, + createBodyKeyedRateLimiter, + createUserKeyedRateLimiter, } from '@n8n/decorators'; import { Container } from '@n8n/di'; import express, { json } from 'express'; import { mock } from 'jest-mock-extended'; import { agent as testAgent } from 'supertest'; +import { z } from 'zod'; +import { Z } from 'zod-class'; import type { AuthService } from '@/auth/auth.service'; import { ControllerRegistry } from '@/controller.registry'; @@ -48,7 +52,7 @@ describe('ControllerRegistry', () => { globalConfig, metadata, lastActiveAtService, - new RateLimitService(mock()), + new RateLimitService(), ).activate(app); agent = testAgent(app); }); @@ -88,19 +92,22 @@ describe('ControllerRegistry', () => { }); describe('Body-based keyed rate limiting', () => { + class TestBodyDto extends Z.class({ + email: z.string().max(20), + }) {} + @RestController('/test') // @ts-expect-error tsc complains about unused class class TestController { @Post('/body-keyed', { skipAuth: true, - keyedRateLimit: { + keyedRateLimit: createBodyKeyedRateLimiter({ limit: 3, windowMs: 60_000, - source: 'body', field: 'email', - }, + }), }) - bodyKeyed(@Body _body: { email: string }) { + bodyKeyed(@Body _body: TestBodyDto) { return { ok: true }; } } @@ -114,7 +121,7 @@ describe('ControllerRegistry', () => { 'should not rate limit when keyed value is %s', async (identifier) => { for (let i = 0; i < 5; i++) { - await agent.post('/rest/test/body-keyed').send({ email: identifier }).expect(200); + await agent.post('/rest/test/body-keyed').send({ email: identifier }).expect(400); } }, ); @@ -130,6 +137,112 @@ describe('ControllerRegistry', () => { await agent.post('/rest/test/body-keyed').send({ email: 'other@example.com' }).expect(200); }); + + it('should not rate limit when the value does not match the schema', async () => { + const tooLongEmail = 'a'.repeat(21); + + for (let i = 0; i < 4; i++) { + await agent.post('/rest/test/body-keyed').send({ email: tooLongEmail }).expect(400); + } + }); + + it('should not rate limit when no body is sent', async () => { + for (let i = 0; i < 5; i++) { + await agent.post('/rest/test/body-keyed').expect(400); + } + }); + + it('should not rate limit when the field is missing from body', async () => { + for (let i = 0; i < 5; i++) { + await agent.post('/rest/test/body-keyed').send({}).expect(400); + } + + for (let i = 0; i < 5; i++) { + await agent.post('/rest/test/body-keyed').send({ other: 'value' }).expect(400); + } + }); + + it('should not rate limit when numeric value fails string schema validation', async () => { + for (let i = 0; i < 5; i++) { + await agent.post('/rest/test/body-keyed').send({ email: 123 }).expect(400); + } + }); + + it('should rate limit empty string if it passes schema validation', async () => { + const emptyEmail = ''; + + for (let i = 0; i < 3; i++) { + await agent.post('/rest/test/body-keyed').send({ email: emptyEmail }).expect(200); + } + + await agent.post('/rest/test/body-keyed').send({ email: emptyEmail }).expect(429); + }); + + it('should maintain separate rate limit counters for different identifiers', async () => { + const email1 = 'user1@example.com'; + const email2 = 'user2@example.com'; + const email3 = 'user3@example.com'; + + // Each email should have its own counter + await agent.post('/rest/test/body-keyed').send({ email: email1 }).expect(200); // email1: 1 + await agent.post('/rest/test/body-keyed').send({ email: email2 }).expect(200); // email2: 1 + await agent.post('/rest/test/body-keyed').send({ email: email1 }).expect(200); // email1: 2 + await agent.post('/rest/test/body-keyed').send({ email: email3 }).expect(200); // email3: 1 + await agent.post('/rest/test/body-keyed').send({ email: email1 }).expect(200); // email1: 3 + + // email1 should be rate limited now + await agent.post('/rest/test/body-keyed').send({ email: email1 }).expect(429); + + // But email2 and email3 should still work + await agent.post('/rest/test/body-keyed').send({ email: email2 }).expect(200); // email2: 2 + await agent.post('/rest/test/body-keyed').send({ email: email3 }).expect(200); // email3: 2 + }); + }); + + describe('Body-based keyed rate limiting with numeric field', () => { + class TestNumericDto extends Z.class({ + userId: z.number(), + }) {} + + @RestController('/test') + // @ts-expect-error tsc complains about unused class + class TestController { + @Post('/numeric-keyed', { + skipAuth: true, + keyedRateLimit: createBodyKeyedRateLimiter({ + limit: 3, + windowMs: 60_000, + field: 'userId', + }), + }) + numericKeyed(@Body _body: TestNumericDto) { + return { ok: true }; + } + } + + beforeAll(() => { + authMiddleware.mockImplementation(async (_req, _res, next) => next()); + lastActiveAtService.middleware.mockImplementation(async (_req, _res, next) => next()); + }); + + it('should apply keyed rate limiting based on numeric field', async () => { + const userId = 12345; + + for (let i = 0; i < 3; i++) { + await agent.post('/rest/test/numeric-keyed').send({ userId }).expect(200); + } + + await agent.post('/rest/test/numeric-keyed').send({ userId }).expect(429); + + // Different numeric ID should work + await agent.post('/rest/test/numeric-keyed').send({ userId: 67890 }).expect(200); + }); + + it('should not rate limit when string value fails numeric schema validation', async () => { + for (let i = 0; i < 5; i++) { + await agent.post('/rest/test/numeric-keyed').send({ userId: 'not-a-number' }).expect(400); + } + }); }); describe('User-based keyed rate limiting', () => { @@ -137,11 +250,10 @@ describe('ControllerRegistry', () => { // @ts-expect-error tsc complains about unused class class TestController { @Post('/user-keyed', { - keyedRateLimit: { + keyedRateLimit: createUserKeyedRateLimiter({ limit: 3, windowMs: 60_000, - source: 'user', - }, + }), }) bodyKeyed(@Body _body: { email: string }) { return { ok: true }; diff --git a/packages/cli/src/controller.registry.ts b/packages/cli/src/controller.registry.ts index 8398fe63b06..09d6e1f5cc4 100644 --- a/packages/cli/src/controller.registry.ts +++ b/packages/cli/src/controller.registry.ts @@ -110,7 +110,10 @@ export class ControllerRegistry { return await controller[handlerName](...args); }; - const middlewares = this.buildMiddlewares(route, controllerMiddlewares); + const bodyArgIdx = route.args.findIndex((arg) => arg?.type === 'body'); + const bodyArgType = bodyArgIdx !== -1 ? (argTypes[bodyArgIdx] as ZodClass) : undefined; + + const middlewares = this.buildMiddlewares(route, controllerMiddlewares, bodyArgType); const finalHandler = route.usesTemplates ? async (req: Request, res: Response) => { await handler(req, res); @@ -137,6 +140,7 @@ export class ControllerRegistry { middlewares?: RequestHandler[]; }, controllerMiddlewares: RequestHandler[], + bodyDtoClass?: ZodClass, ): RequestHandler[] { const middlewares: RequestHandler[] = []; @@ -147,7 +151,17 @@ export class ControllerRegistry { // LAYER 2a: Keyed rate limiting with body source (BEFORE auth) if (inProduction && route.keyedRateLimit?.source === 'body') { - middlewares.push(this.rateLimitService.createKeyedRateLimitMiddleware(route.keyedRateLimit)); + assert( + bodyDtoClass, + 'Body argument type (@Body decorator) is required for body-based rate limiting', + ); + + middlewares.push( + this.rateLimitService.createBodyKeyedRateLimitMiddleware( + bodyDtoClass, + route.keyedRateLimit, + ), + ); } if (!route.skipAuth) { @@ -170,7 +184,7 @@ export class ControllerRegistry { // Separate ifs intentionally to prevent configuration errors in development if (inProduction) { middlewares.push( - this.rateLimitService.createKeyedRateLimitMiddleware(route.keyedRateLimit), + this.rateLimitService.createUserKeyedRateLimitMiddleware(route.keyedRateLimit), ); } } diff --git a/packages/cli/src/controllers/auth.controller.ts b/packages/cli/src/controllers/auth.controller.ts index a474a31fb9c..e9bd33bdc75 100644 --- a/packages/cli/src/controllers/auth.controller.ts +++ b/packages/cli/src/controllers/auth.controller.ts @@ -2,7 +2,14 @@ import { LoginRequestDto, ResolveSignupTokenQueryDto } from '@n8n/api-types'; import { Logger } from '@n8n/backend-common'; import type { User, PublicUser } from '@n8n/db'; import { UserRepository, AuthenticatedRequest, GLOBAL_OWNER_ROLE } from '@n8n/db'; -import { Body, Get, Post, Query, RestController } from '@n8n/decorators'; +import { + Body, + createBodyKeyedRateLimiter, + Get, + Post, + Query, + RestController, +} from '@n8n/decorators'; import { Container } from '@n8n/di'; import { isEmail } from 'class-validator'; import { Response } from 'express'; @@ -50,12 +57,11 @@ export class AuthController { limit: 1000, windowMs: 5 * Time.minutes.toMilliseconds, }, - keyedRateLimit: { + keyedRateLimit: createBodyKeyedRateLimiter({ limit: 5, windowMs: 1 * Time.minutes.toMilliseconds, - source: 'body', - field: 'emailOrLdapLoginId' satisfies keyof LoginRequestDto, - }, + field: 'emailOrLdapLoginId', + }), }) async login( req: AuthlessRequest, diff --git a/packages/cli/src/controllers/invitation.controller.ts b/packages/cli/src/controllers/invitation.controller.ts index 16d58a09835..e346fcae48c 100644 --- a/packages/cli/src/controllers/invitation.controller.ts +++ b/packages/cli/src/controllers/invitation.controller.ts @@ -2,7 +2,14 @@ import { AcceptInvitationRequestDto, InviteUsersRequestDto } from '@n8n/api-type import { Logger } from '@n8n/backend-common'; import type { User } from '@n8n/db'; import { UserRepository, AuthenticatedRequest } from '@n8n/db'; -import { Post, GlobalScope, RestController, Body, Param } from '@n8n/decorators'; +import { + Post, + GlobalScope, + RestController, + Body, + Param, + createBodyKeyedRateLimiter, +} from '@n8n/decorators'; import { Response } from 'express'; import { AuthService } from '@/auth/auth.service'; @@ -206,12 +213,11 @@ export class InvitationController { // Two layered rate limit to ensure multiple users can accept an invitation from // the same IP address but aggressive per inviteeId limit. ipRateLimit: { limit: 100, windowMs: 5 * Time.minutes.toMilliseconds }, - keyedRateLimit: { + keyedRateLimit: createBodyKeyedRateLimiter({ limit: 10, windowMs: 1 * Time.minutes.toMilliseconds, - source: 'body', - field: 'inviterId' satisfies keyof AcceptInvitationRequestDto, - }, + field: 'inviterId', + }), }) async acceptInvitation( req: AuthlessRequest, diff --git a/packages/cli/src/controllers/me.controller.ts b/packages/cli/src/controllers/me.controller.ts index 125d41a9bb3..07a14ce83ea 100644 --- a/packages/cli/src/controllers/me.controller.ts +++ b/packages/cli/src/controllers/me.controller.ts @@ -7,7 +7,7 @@ import { import { Logger } from '@n8n/backend-common'; import type { User, PublicUser } from '@n8n/db'; import { UserRepository, AuthenticatedRequest } from '@n8n/db'; -import { Body, Patch, Post, RestController } from '@n8n/decorators'; +import { Body, createUserKeyedRateLimiter, Patch, Post, RestController } from '@n8n/decorators'; import { plainToInstance } from 'class-transformer'; import { Response } from 'express'; @@ -166,9 +166,7 @@ export class MeController { * Update the logged-in user's password. */ @Patch('/password', { - keyedRateLimit: { - source: 'user', - }, + keyedRateLimit: createUserKeyedRateLimiter({}), }) async updatePassword( req: AuthenticatedRequest, diff --git a/packages/cli/src/controllers/mfa.controller.ts b/packages/cli/src/controllers/mfa.controller.ts index baea1039e29..92952f43d3b 100644 --- a/packages/cli/src/controllers/mfa.controller.ts +++ b/packages/cli/src/controllers/mfa.controller.ts @@ -1,5 +1,11 @@ import { AuthenticatedRequest, UserRepository } from '@n8n/db'; -import { Get, GlobalScope, Post, RestController } from '@n8n/decorators'; +import { + createUserKeyedRateLimiter, + Get, + GlobalScope, + Post, + RestController, +} from '@n8n/decorators'; import { Response } from 'express'; import { AuthService } from '@/auth/auth.service'; @@ -86,9 +92,7 @@ export class MFAController { @Post('/enable', { allowSkipMFA: true, - keyedRateLimit: { - source: 'user', - }, + keyedRateLimit: createUserKeyedRateLimiter({}), }) async activateMFA(req: MFA.Activate, res: Response) { const { mfaCode = null } = req.body; @@ -129,9 +133,7 @@ export class MFAController { @Post('/disable', { ipRateLimit: true, - keyedRateLimit: { - source: 'user', - }, + keyedRateLimit: createUserKeyedRateLimiter({}), }) async disableMFA(req: MFA.Disable, res: Response) { const { id: userId } = req.user; @@ -175,9 +177,7 @@ export class MFAController { @Post('/verify', { allowSkipMFA: true, - keyedRateLimit: { - source: 'user', - }, + keyedRateLimit: createUserKeyedRateLimiter({}), }) async verifyMFA(req: MFA.Verify) { const { id } = req.user; diff --git a/packages/cli/src/controllers/password-reset.controller.ts b/packages/cli/src/controllers/password-reset.controller.ts index 2b269823776..585d3c8f7cc 100644 --- a/packages/cli/src/controllers/password-reset.controller.ts +++ b/packages/cli/src/controllers/password-reset.controller.ts @@ -5,7 +5,14 @@ import { } from '@n8n/api-types'; import { Logger } from '@n8n/backend-common'; import { GLOBAL_OWNER_ROLE, UserRepository } from '@n8n/db'; -import { Body, Get, Post, Query, RestController } from '@n8n/decorators'; +import { + Body, + createBodyKeyedRateLimiter, + Get, + Post, + Query, + RestController, +} from '@n8n/decorators'; import { hasGlobalScope } from '@n8n/permissions'; import { Response } from 'express'; @@ -51,11 +58,10 @@ export class PasswordResetController { @Post('/forgot-password', { skipAuth: true, ipRateLimit: { limit: 20, windowMs: 5 * Time.minutes.toMilliseconds }, - keyedRateLimit: { + keyedRateLimit: createBodyKeyedRateLimiter({ limit: 3, - source: 'body', - field: 'email' satisfies keyof ForgotPasswordRequestDto, - }, + field: 'email', + }), }) async forgotPassword( _req: AuthlessRequest, diff --git a/packages/cli/src/services/rate-limit.service.ts b/packages/cli/src/services/rate-limit.service.ts index 3c230967c18..4fa8cc34a86 100644 --- a/packages/cli/src/services/rate-limit.service.ts +++ b/packages/cli/src/services/rate-limit.service.ts @@ -1,11 +1,13 @@ -import { Service } from '@n8n/di'; -import type { RateLimiterLimits, KeyedRateLimiterConfig } from '@n8n/decorators'; +import { Time } from '@n8n/constants'; import type { AuthenticatedRequest } from '@n8n/db'; +import type { RateLimiterLimits, UserKeyedRateLimiterConfig } from '@n8n/decorators'; +import { BodyKeyedRateLimiterConfig } from '@n8n/decorators'; +import { Service } from '@n8n/di'; import type { Request, RequestHandler } from 'express'; import { rateLimit as expressRateLimit } from 'express-rate-limit'; import assert from 'node:assert'; -import { ErrorReporter } from 'n8n-core'; -import { Time } from '@n8n/constants'; +import type { ZodTypeAny } from 'zod'; +import type { ZodClass } from 'zod-class'; const defaultLimits: Required = { limit: 5, @@ -22,8 +24,6 @@ const defaultLimits: Required = { */ @Service() export class RateLimitService { - constructor(private readonly errorReporter: ErrorReporter) {} - /** * Creates Layer 1: IP-based rate limit middleware * Always runs BEFORE authentication. @@ -42,53 +42,57 @@ export class RateLimitService { * Creates Layer 2: Keyed rate limit middleware * Position (before/after auth) depends on identifier source */ - createKeyedRateLimitMiddleware(config: KeyedRateLimiterConfig): RequestHandler { + createBodyKeyedRateLimitMiddleware( + bodyDtoClass: ZodClass, + config: BodyKeyedRateLimiterConfig, + ): RequestHandler { + const fieldName = config.field; + const bodyFieldSchema = bodyDtoClass.shape[fieldName]; + assert(bodyFieldSchema, `Missing field ${fieldName} in DTO schema`); + return expressRateLimit({ limit: config.limit ?? defaultLimits.limit, windowMs: config.windowMs ?? defaultLimits.windowMs, - keyGenerator: (req: Request) => this.extractReqIdentifier(req, config), + keyGenerator: (req: Request) => + this.extractBodyIdentifier(req.body, fieldName, bodyFieldSchema), skip: (req: Request) => { - const identifier = this.extractReqIdentifier(req, config); + const identifier = this.extractBodyIdentifier(req.body, fieldName, bodyFieldSchema); return identifier.startsWith('skip:'); }, - message: { message: 'Too many requests' }, }); } - private extractReqIdentifier(req: Request, config: KeyedRateLimiterConfig): string { - const { source } = config; + createUserKeyedRateLimitMiddleware(config: UserKeyedRateLimiterConfig): RequestHandler { + return expressRateLimit({ + limit: config.limit ?? defaultLimits.limit, + windowMs: config.windowMs ?? defaultLimits.windowMs, + keyGenerator: (req: AuthenticatedRequest) => this.extractUserIdentifier(req), + skip: (req: AuthenticatedRequest) => { + const identifier = this.extractUserIdentifier(req); + return identifier.startsWith('skip:'); + }, + }); + } - if (source === 'body') { - const body = req.body; - if (!body) { - return 'skip:no-body'; - } - - const value = body[config.field]; - if (typeof value !== 'string' && typeof value !== 'number') { - return 'skip:no-identifier'; - } - - return `body:${value}`; + private extractBodyIdentifier(body: unknown, fieldName: string, fieldSchema: ZodTypeAny): string { + if (!body || typeof body !== 'object') { + return 'skip:empty-body'; } - if (source === 'user') { - const authReq = req as AuthenticatedRequest; - if (!authReq.user) { - this.errorReporter.error(new Error('Missing user for user-based rate limited endpoint'), { - extra: { - request: { - method: req.method, - url: req.url, - }, - }, - }); - return 'skip:not-authenticated'; - } - - return `user:${authReq.user.id}`; + const value = (body as Record)[fieldName]; + if (typeof value !== 'string' && typeof value !== 'number') { + return 'skip:unsupported-type'; } - assert.fail(`Unknown source for keyed rate limiting: ${JSON.stringify(config)}`); + const parseResult = fieldSchema.safeParse(value); + if (!parseResult.success) { + return 'skip:validation-failed'; + } + + return `body:${value}`; + } + + private extractUserIdentifier(req: AuthenticatedRequest): string { + return `user:${req.user.id}`; } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index aa0233eed87..131920ed92f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -923,6 +923,9 @@ importers: '@types/lodash': specifier: 'catalog:' version: 4.17.17 + zod: + specifier: 3.25.67 + version: 3.25.67 packages/@n8n/di: dependencies: @@ -21684,7 +21687,7 @@ snapshots: '@currents/commit-info': 1.0.1-beta.0 async-retry: 1.3.3 axios: 1.12.0(debug@4.4.3) - axios-retry: 4.5.0(axios@1.12.0(debug@4.4.3)) + axios-retry: 4.5.0(axios@1.12.0) c12: 1.11.2(magicast@0.3.5) chalk: 4.1.2 commander: 12.1.0 @@ -27207,11 +27210,6 @@ snapshots: axe-core@4.7.2: {} - axios-retry@4.5.0(axios@1.12.0(debug@4.4.3)): - dependencies: - axios: 1.12.0(debug@4.4.3) - is-retry-allowed: 2.2.0 - axios-retry@4.5.0(axios@1.12.0): dependencies: axios: 1.12.0