diff --git a/.changeset/shaky-hotels-wash.md b/.changeset/shaky-hotels-wash.md new file mode 100644 index 0000000000000..efa0b33024d25 --- /dev/null +++ b/.changeset/shaky-hotels-wash.md @@ -0,0 +1,5 @@ +--- +'@rocket.chat/meteor': minor +--- + +Adds the `per` option to `rateLimiterOptions`, so a REST endpoint can rate limit per user instead of per IP address, and applies it to `chat.sendMessage` — users on a shared address no longer compete for a single message allowance. diff --git a/apps/meteor/definition/externals/meteor/rate-limit.d.ts b/apps/meteor/definition/externals/meteor/rate-limit.d.ts index 9d01911952efd..6c8655bd39d41 100644 --- a/apps/meteor/definition/externals/meteor/rate-limit.d.ts +++ b/apps/meteor/definition/externals/meteor/rate-limit.d.ts @@ -2,8 +2,15 @@ declare module 'meteor/rate-limit' { type RateLimiterOptionsToCheck = { IPAddr: string; route: string; + userId?: string; }; + type RateLimiterMatcher = (input: string) => unknown; + + type RateLimiterRule = { + route: string; + } & ({ IPAddr: RateLimiterMatcher } | { userId: RateLimiterMatcher }); + type RateLimiterCheckResult = { allowed: boolean; timeToReset: number; @@ -15,11 +22,6 @@ declare module 'meteor/rate-limit' { public increment(input: RateLimiterOptionsToCheck); - public addRule( - rule: { IPAddr: (input: any) => any; route: string }, - numRequestsAllowed: number, - intervalTime: number, - callback?: () => void, - ): void; + public addRule(rule: RateLimiterRule, numRequestsAllowed: number, intervalTime: number, callback?: () => void): void; } } diff --git a/apps/meteor/ee/server/api/mcp/index.spec.ts b/apps/meteor/ee/server/api/mcp/index.spec.ts index ab2e01fd404af..3ed2ace9fe7cd 100644 --- a/apps/meteor/ee/server/api/mcp/index.spec.ts +++ b/apps/meteor/ee/server/api/mcp/index.spec.ts @@ -29,7 +29,8 @@ jest.mock('../../../../server/api', () => ({ API: { v1: { registerRateLimiterForRoute: jest.fn(), - enforceRateLimitForRoute: jest.fn(), + resolveRateLimiter: jest.fn(), + canBypassRateLimit: jest.fn(), router: { getHonoRouter: jest.fn(() => ({ use: jest.fn(), post: jest.fn(), get: jest.fn() })), }, @@ -67,71 +68,77 @@ describe('MCP HTTP route', () => { beforeEach(() => { jest.mocked(settings.get).mockReturnValue(true); jest.mocked(handleRpcMessage).mockReset(); - jest.mocked(API.v1.enforceRateLimitForRoute).mockReset().mockResolvedValue(undefined); + jest.mocked(API.v1.resolveRateLimiter).mockReset().mockReturnValue(undefined); + jest.mocked(API.v1.canBypassRateLimit).mockReset().mockResolvedValue(false); jest .mocked(Users.findPersonalAccessTokenByHashedTokenAndUserId) .mockReset() .mockResolvedValue({ _id: 'user-id' } as never); }); - it('applies the API rate limiter before handling MCP requests', async () => { + const callMiddleware = async (next: jest.Mock) => { const middleware = mockRouter.use.mock.calls[0]?.[2]; - expect(middleware).toBeDefined(); if (!middleware) { throw new Error('MCP rate-limit middleware was not registered'); } - const request = new Request('http://localhost/api/v1/mcp', { - method: 'POST', - headers: { 'x-user-id': 'user-id' }, - }); - const response = new Response(); - const next = jest.fn().mockResolvedValue(undefined); - await middleware( + const request = new Request('http://localhost/api/v1/mcp', { method: 'POST', headers: { 'x-user-id': 'user-id' } }); + const res = new Response(); + + return middleware( { req: { method: 'POST', raw: request, header: (name: string) => request.headers.get(name) ?? undefined }, - res: response, - get: () => '192.0.2.1', + res, + get: (key: string) => (key === 'user' ? { _id: 'user-id' } : '192.0.2.1'), + json: (body: unknown, status: number) => new Response(JSON.stringify(body), { status, headers: res.headers }), }, next, ); + }; + + const limiterAllowing = (numInvocationsLeft: number) => ({ + key: 'mcp', + options: { numRequestsAllowed: 60, intervalTimeInMS: 60_000 }, + rateLimiter: { + increment: jest.fn(), + check: jest.fn().mockResolvedValue({ allowed: numInvocationsLeft > 0, numInvocationsLeft, timeToReset: 30_000 }), + }, + }); + + it('sits between authentication and the permission gate', () => { + const [path, auth, rateLimit, permissions] = mockRouter.use.mock.calls[0] ?? []; + + expect(path).toBe('/mcp'); + expect(auth).toBe(jest.mocked(authenticationMiddlewareForHono).mock.results[0]?.value); + expect(rateLimit).toEqual(expect.any(Function)); + expect(permissions).toBe(jest.mocked(permissionsMiddleware).mock.results[0]?.value); + }); + + it('lets MCP requests through while the caller is within the allowance', async () => { + jest.mocked(API.v1.resolveRateLimiter).mockReturnValue(limiterAllowing(59) as never); + const next = jest.fn().mockResolvedValue(undefined); + + await callMiddleware(next); - expect(API.v1.enforceRateLimitForRoute).toHaveBeenCalledWith({ - route: 'mcp', - method: 'post', - request, - response, - requestIp: '192.0.2.1', - userId: 'user-id', - }); expect(next).toHaveBeenCalledTimes(1); }); it('returns a JSON-RPC error with rate-limit headers when the limit is exceeded', async () => { - const middleware = mockRouter.use.mock.calls[0]?.[2]; - expect(middleware).toBeDefined(); - if (!middleware) { - throw new Error('MCP rate-limit middleware was not registered'); - } + jest.mocked(API.v1.resolveRateLimiter).mockReturnValue(limiterAllowing(0) as never); + const next = jest.fn(); - jest.mocked(API.v1.enforceRateLimitForRoute).mockImplementationOnce(async ({ response }: { response: Response }) => { - response.headers.set('X-RateLimit-Remaining', '0'); - throw Object.assign(new Error('Please slow down'), { error: 'error-too-many-requests', reason: 'Please slow down' }); - }); - const request = new Request('http://localhost/api/v1/mcp', { method: 'POST', headers: { 'x-user-id': 'user-id' } }); - const result = await middleware( - { - req: { method: 'POST', raw: request, header: (name: string) => request.headers.get(name) ?? undefined }, - res: new Response(), - get: () => '192.0.2.1', - }, - jest.fn(), - ); + const result = await callMiddleware(next); + expect(next).not.toHaveBeenCalled(); expect(result).toBeInstanceOf(Response); expect(result?.status).toBe(429); expect(result?.headers.get('X-RateLimit-Remaining')).toBe('0'); - await expect(result?.json()).resolves.toMatchObject({ error: { message: 'Please slow down' } }); + await expect(result?.json()).resolves.toEqual({ + jsonrpc: '2.0', + id: null, + // The wait hint has to survive: MCP clients use it to schedule the retry. + error: { code: -32000, message: expect.stringContaining('You must wait 30 seconds') }, + }); }); it('registers the endpoint with authentication, permission, and license gates', () => { diff --git a/apps/meteor/ee/server/api/mcp/index.ts b/apps/meteor/ee/server/api/mcp/index.ts index 0fe2c2543354e..12958c4e9b410 100644 --- a/apps/meteor/ee/server/api/mcp/index.ts +++ b/apps/meteor/ee/server/api/mcp/index.ts @@ -2,7 +2,6 @@ import { AI_LICENSE_MODULE } from '@rocket.chat/ai-search'; import { License } from '@rocket.chat/license'; import { Logger } from '@rocket.chat/logger'; import { Users } from '@rocket.chat/models'; -import type { MiddlewareHandler } from 'hono'; import type { StatusCode } from 'hono/utils/http-status'; import { Accounts } from 'meteor/accounts-base'; @@ -13,6 +12,7 @@ import { API } from '../../../../server/api'; import type { TypedOptions } from '../../../../server/api/definition'; import { authenticationMiddlewareForHono } from '../../../../server/api/v1/middlewares/authenticationHono'; import { permissionsMiddleware } from '../../../../server/api/v1/middlewares/permissions'; +import { rateLimiterMiddleware } from '../../../../server/api/v1/middlewares/rateLimiter'; import { settings } from '../../../../server/settings/cached'; import { license } from '../v1/middlewares/license'; @@ -183,37 +183,15 @@ const sendResponse = (response: McpHttpResponse): Response => { return new Response(response.body === undefined ? null : JSON.stringify(response.body), { status: response.statusCode, headers }); }; -const isRateLimitError = (error: unknown): error is { error: 'error-too-many-requests'; reason?: string } => - typeof error === 'object' && error !== null && 'error' in error && error.error === 'error-too-many-requests'; - -const rateLimitMiddleware: MiddlewareHandler = async (c, next) => { - try { - await API.v1.enforceRateLimitForRoute({ - route: MCP_ROUTE, - method: c.req.method.toLowerCase(), - request: c.req.raw, - response: c.res, - requestIp: c.get('remoteAddress'), - userId: c.req.header('x-user-id'), - }); - } catch (error) { - if (!isRateLimitError(error)) { - throw error; - } - - return sendResponse({ - statusCode: 429, - body: { jsonrpc: '2.0', id: null, error: { code: -32000, message: error.reason ?? 'Too many requests' } }, - headers: Object.fromEntries([...c.res.headers].filter(([name]) => name.toLowerCase().startsWith('x-ratelimit-'))), - }); - } - - const rateLimitHeaders = [...c.res.headers].filter(([name]) => name.toLowerCase().startsWith('x-ratelimit-')); - await next(); - for (const [name, value] of rateLimitHeaders) { - c.res.headers.set(name, value); - } -}; +const rateLimitMiddleware = rateLimiterMiddleware({ + settings, + resolve: (c) => API.v1.resolveRateLimiter(c, MCP_ROUTE, c.req.method.toLowerCase()), + canBypass: (userId) => API.v1.canBypassRateLimit(userId), + reject: (reason) => ({ + statusCode: 429, + body: { jsonrpc: '2.0', id: null, error: { code: -32000, message: reason } }, + }), +}); const router = API.v1.router.getHonoRouter(); API.v1.registerRateLimiterForRoute({ route: MCP_ROUTE, rateLimiterOptions: MCP_RATE_LIMIT_OPTIONS, methods: ['post'] }); diff --git a/apps/meteor/server/api/ApiClass.ts b/apps/meteor/server/api/ApiClass.ts index 453fd94f3568c..87b348f929fed 100644 --- a/apps/meteor/server/api/ApiClass.ts +++ b/apps/meteor/server/api/ApiClass.ts @@ -13,7 +13,6 @@ import { DDP } from 'meteor/ddp'; // eslint-disable-next-line import-x/no-duplicates import { DDPCommon } from 'meteor/ddp-common'; import { Meteor } from 'meteor/meteor'; -import type { RateLimiterOptionsToCheck } from 'meteor/rate-limit'; // eslint-disable-next-line import-x/no-duplicates import { RateLimiter } from 'meteor/rate-limit'; import _ from 'underscore'; @@ -38,10 +37,12 @@ import type { GenericRouteExecutionContext, TooManyRequestsResult, SuccessStatusCodes, + RateLimiterOptions, } from './definition'; import { getUserInfo } from './lib/getUserInfo'; import { parseJsonQuery } from './lib/parseJsonQuery'; -import type { APIActionContext } from './router'; +import { buildRateLimiterRule } from './rateLimiterKey'; +import type { APIActionContext, HonoContext } from './router'; import { RocketChatAPIRouter } from './router'; import { isObject } from '../../lib/utils/isObject'; import { checkCodeForUser } from '../lib/2fa/code'; @@ -51,6 +52,7 @@ import { notifyOnUserChangeAsync } from '../lib/notifyListener'; import { shouldBreakInVersion } from '../lib/shouldBreakInVersion'; import { authenticationMiddlewareForHono } from './v1/middlewares/authenticationHono'; import { permissionsMiddleware } from './v1/middlewares/permissions'; +import { rateLimiterMiddleware, type ResolvedRateLimiter } from './v1/middlewares/rateLimiter'; import { license } from '../../ee/server/api/v1/middlewares/license'; import { getDefaultUserFields } from '../lib/utils/functions/getDefaultUserFields'; import { settings } from '../settings'; @@ -127,11 +129,6 @@ interface IAPIDefaultFieldsToExclude { inviteToken: number; } -export type RateLimiterOptions = { - numRequestsAllowed?: number; - intervalTimeInMS?: number; -}; - export const defaultRateLimiterOptions: RateLimiterOptions = { numRequestsAllowed: settings.get('API_Enable_Rate_Limiter_Limit_Calls_Default'), intervalTimeInMS: settings.get('API_Enable_Rate_Limiter_Limit_Time_Default'), @@ -415,45 +412,15 @@ export class APIClass { - return ( - rateLimiterDictionary.hasOwnProperty(route) && - settings.get('API_Enable_Rate_Limiter') === true && - (process.env.NODE_ENV !== 'development' || settings.get('API_Enable_Rate_Limiter_Dev') === true) && - !(userId && (await hasPermissionAsync(userId, 'api-bypass-rate-limit'))) - ); - } - - protected async enforceRateLimit( - objectForRateLimitMatch: RateLimiterOptionsToCheck, - _: any, - response: Response, - userId?: string, - ): Promise { - if (!(await this.shouldVerifyRateLimit(objectForRateLimitMatch.route, userId))) { - return; - } + public resolveRateLimiter(_c: HonoContext, route: string, method: string): ResolvedRateLimiter | undefined { + const key = this.getFullRouteName(route, method); + const entry = rateLimiterDictionary[key]; - rateLimiterDictionary[objectForRateLimitMatch.route].rateLimiter.increment(objectForRateLimitMatch); - const attemptResult = await rateLimiterDictionary[objectForRateLimitMatch.route].rateLimiter.check(objectForRateLimitMatch); - const timeToResetAttempsInSeconds = Math.ceil(attemptResult.timeToReset / 1000); - response.headers.set( - 'X-RateLimit-Limit', - String(rateLimiterDictionary[objectForRateLimitMatch.route].options.numRequestsAllowed ?? ''), - ); - response.headers.set('X-RateLimit-Remaining', String(attemptResult.numInvocationsLeft)); - response.headers.set('X-RateLimit-Reset', String(new Date().getTime() + attemptResult.timeToReset)); + return entry && { key, ...entry }; + } - if (!attemptResult.allowed) { - throw new Meteor.Error( - 'error-too-many-requests', - `Error, too many requests. Please slow down. You must wait ${timeToResetAttempsInSeconds} seconds before trying this endpoint again.`, - { - timeToReset: attemptResult.timeToReset, - seconds: timeToResetAttempsInSeconds, - }, - ); - } + public async canBypassRateLimit(userId: string): Promise { + return hasPermissionAsync(userId, 'api-bypass-rate-limit'); } public registerRateLimiterForRoute({ @@ -472,24 +439,6 @@ export class APIClass { - return this.enforceRateLimit({ IPAddr: requestIp, route: this.getFullRouteName(route, method) }, request, response, userId); - } - public reloadRoutesToRefreshRateLimiter(): void { this._routes.forEach((route) => { if (this.shouldAddRateLimitToRoute(route.options)) { @@ -530,12 +479,8 @@ export class APIClass input, - route, - }; rateLimiterDictionary[route].rateLimiter.addRule( - rateLimitRule, + buildRateLimiterRule(route, rateLimiterOptions.per), rateLimiterOptions.numRequestsAllowed as number, rateLimiterOptions.intervalTimeInMS as number, ); @@ -879,11 +824,6 @@ export class APIClass { switch (e.error) { - case 'error-too-many-requests': - return api.tooManyRequests(typeof e === 'string' ? e : e.message); case 'unauthorized': case 'error-unauthorized': if (applyBreakingChanges) { @@ -965,6 +901,11 @@ export class APIClass api.resolveRateLimiter(c, route, method.toLowerCase()), + canBypass: (userId) => api.canBypassRateLimit(userId), + }), permissionsMiddleware(_options as TypedOptions), license(_options as TypedOptions, License), (operations[method as keyof Operations] as Record).action, diff --git a/apps/meteor/server/api/api.ts b/apps/meteor/server/api/api.ts index 56772f2159f76..400a2fba5b6ec 100644 --- a/apps/meteor/server/api/api.ts +++ b/apps/meteor/server/api/api.ts @@ -5,6 +5,7 @@ import type express from 'express'; import { WebApp } from 'meteor/webapp'; import { APIClass } from './ApiClass'; +import type { RateLimiterOptions } from './definition'; import { type APIActionHandler, RocketChatAPIRouter } from './router'; import { metrics } from '../lib/metrics'; import { settings } from '../settings'; @@ -21,10 +22,7 @@ export type Prettify = { [K in keyof T]: T[K]; } & unknown; -export type RateLimiterOptions = { - numRequestsAllowed?: number; - intervalTimeInMS?: number; -}; +export type { RateLimiterOptions } from './definition'; export const defaultRateLimiterOptions: RateLimiterOptions = { numRequestsAllowed: settings.get('API_Enable_Rate_Limiter_Limit_Calls_Default'), diff --git a/apps/meteor/server/api/definition.ts b/apps/meteor/server/api/definition.ts index 3f27770edb0b7..fc3c8f9c960c9 100644 --- a/apps/meteor/server/api/definition.ts +++ b/apps/meteor/server/api/definition.ts @@ -8,6 +8,14 @@ import type { ValidateFunction } from 'ajv'; import type { ITwoFactorOptions } from '../lib/2fa/code'; import type { DeprecationLoggerNextPlannedVersion } from '../lib/deprecationWarningLogger'; +export type RateLimiterSubject = 'ip' | 'user'; + +export type RateLimiterOptions = { + numRequestsAllowed?: number; + intervalTimeInMS?: number; + per?: RateLimiterSubject; +}; + export type SuccessStatusCodes = Exclude, Range<200>>; export type RedirectStatusCodes = Exclude, Range<300>>; @@ -112,12 +120,7 @@ export type SharedOptions = ( authRequired?: boolean; userWithoutUsername?: boolean; forceTwoFactorAuthenticationForNonEnterprise?: boolean; - rateLimiterOptions?: - | { - numRequestsAllowed?: number; - intervalTimeInMS?: number; - } - | boolean; + rateLimiterOptions?: RateLimiterOptions | boolean; queryOperations?: string[]; queryFields?: string[]; } @@ -132,12 +135,7 @@ export type SharedOptions = ( userWithoutUsername?: boolean; twoFactorRequired: true; twoFactorOptions?: ITwoFactorOptions; - rateLimiterOptions?: - | { - numRequestsAllowed?: number; - intervalTimeInMS?: number; - } - | boolean; + rateLimiterOptions?: RateLimiterOptions | boolean; queryOperations?: string[]; queryFields?: string[]; diff --git a/apps/meteor/server/api/rateLimiterKey.spec.ts b/apps/meteor/server/api/rateLimiterKey.spec.ts new file mode 100644 index 0000000000000..148206366af5e --- /dev/null +++ b/apps/meteor/server/api/rateLimiterKey.spec.ts @@ -0,0 +1,54 @@ +import { buildRateLimiterInput, buildRateLimiterRule } from './rateLimiterKey'; + +const ROUTE = '/v1/chat.sendMessagepost'; + +const bucketOf = (rule: Record) => Object.keys(rule).sort(); + +describe('buildRateLimiterRule', () => { + it('should bucket by address by default and when per is explicitly ip', () => { + expect(bucketOf(buildRateLimiterRule(ROUTE))).toEqual(['IPAddr', 'route']); + expect(bucketOf(buildRateLimiterRule(ROUTE, 'ip'))).toEqual(['IPAddr', 'route']); + }); + + it("should bucket by user when per is 'user'", () => { + expect(bucketOf(buildRateLimiterRule(ROUTE, 'user'))).toEqual(['route', 'userId']); + }); + + it('should carry the route so each endpoint counts separately', () => { + expect(buildRateLimiterRule(ROUTE, 'user')).toMatchObject({ route: ROUTE }); + expect(buildRateLimiterRule(ROUTE, 'ip')).toMatchObject({ route: ROUTE }); + }); + + it('should match a subject by returning it, so the package treats the rule as applicable', () => { + const matchUser = (buildRateLimiterRule(ROUTE, 'user') as { userId: (input: string) => unknown }).userId; + const matchAddress = (buildRateLimiterRule(ROUTE, 'ip') as { IPAddr: (input: string) => unknown }).IPAddr; + + expect(matchUser('alice')).toBe('alice'); + expect(matchAddress('1.2.3.4')).toBe('1.2.3.4'); + }); +}); + +describe('buildRateLimiterInput', () => { + it('should carry both subjects so either rule shape matches it', () => { + expect(buildRateLimiterInput({ route: ROUTE, IPAddr: '1.2.3.4', userId: 'alice' })).toEqual({ + IPAddr: '1.2.3.4', + userId: 'alice', + route: ROUTE, + }); + }); + + it('should fall back to the address as the user subject when unauthenticated, never leaving it falsy', () => { + const expected = { IPAddr: '1.2.3.4', userId: 'ip:1.2.3.4', route: ROUTE }; + + expect(buildRateLimiterInput({ route: ROUTE, IPAddr: '1.2.3.4' })).toEqual(expected); + expect(buildRateLimiterInput({ route: ROUTE, IPAddr: '1.2.3.4', userId: '' })).toEqual(expected); + }); + + it('should keep users on a shared address apart', () => { + const alice = buildRateLimiterInput({ route: ROUTE, IPAddr: '1.2.3.4', userId: 'alice' }); + const bob = buildRateLimiterInput({ route: ROUTE, IPAddr: '1.2.3.4', userId: 'bob' }); + + expect(alice.userId).not.toBe(bob.userId); + expect(alice.IPAddr).toBe(bob.IPAddr); + }); +}); diff --git a/apps/meteor/server/api/rateLimiterKey.ts b/apps/meteor/server/api/rateLimiterKey.ts new file mode 100644 index 0000000000000..99f20469dc925 --- /dev/null +++ b/apps/meteor/server/api/rateLimiterKey.ts @@ -0,0 +1,20 @@ +import type { RateLimiterOptionsToCheck, RateLimiterRule } from 'meteor/rate-limit'; + +import type { RateLimiterSubject } from './definition'; + +export const buildRateLimiterRule = (route: string, per: RateLimiterSubject = 'ip'): RateLimiterRule => + per === 'user' ? { userId: (input: string) => input, route } : { IPAddr: (input: string) => input, route }; + +export const buildRateLimiterInput = ({ + route, + IPAddr, + userId, +}: { + route: string; + IPAddr: string; + userId?: string; +}): RateLimiterOptionsToCheck => ({ + IPAddr, + route, + userId: userId || `ip:${IPAddr}`, +}); diff --git a/apps/meteor/server/api/v1/chat.ts b/apps/meteor/server/api/v1/chat.ts index 6c7cbaa97cb1d..820e088125805 100644 --- a/apps/meteor/server/api/v1/chat.ts +++ b/apps/meteor/server/api/v1/chat.ts @@ -895,6 +895,7 @@ const chatEndpoints = API.v1 'chat.sendMessage', { authRequired: true, + rateLimiterOptions: { numRequestsAllowed: 5, intervalTimeInMS: 1000, per: 'user' }, body: isChatSendMessageProps, response: { 200: ajv.compile<{ message: IMessage }>({ diff --git a/apps/meteor/server/api/v1/middlewares/rateLimiter.spec.ts b/apps/meteor/server/api/v1/middlewares/rateLimiter.spec.ts new file mode 100644 index 0000000000000..30196ac1eb47f --- /dev/null +++ b/apps/meteor/server/api/v1/middlewares/rateLimiter.spec.ts @@ -0,0 +1,177 @@ +import { Router } from '@rocket.chat/http-router'; +import Ajv from 'ajv'; +import express from 'express'; +import request from 'supertest'; + +import { rateLimiterMiddleware, type ResolvedRateLimiter } from './rateLimiter'; +import { remoteAddressMiddleware } from './remoteAddressMiddleware'; +import { CachedSettings } from '../../../settings/CachedSettings'; + +const ajv = new Ajv(); + +const makeRateLimiter = (numRequestsAllowed: number, intervalTimeInMS: number) => { + const counters = new Map(); + let lastReset = Date.now(); + + const keyOf = (input: { route: string; IPAddr: string; userId?: string }) => `route${input.route}userId${input.userId ?? ''}`; + + return { + increment(input: any) { + if (Date.now() - lastReset > intervalTimeInMS) { + counters.clear(); + lastReset = Date.now(); + } + counters.set(keyOf(input), (counters.get(keyOf(input)) ?? 0) + 1); + }, + async check(input: any) { + const used = counters.get(keyOf(input)) ?? 0; + return { + allowed: used <= numRequestsAllowed, + numInvocationsLeft: Math.max(0, numRequestsAllowed - used), + timeToReset: intervalTimeInMS - (Date.now() - lastReset), + }; + }, + }; +}; + +const buildApp = ({ + numRequestsAllowed, + settings, + canBypass = async () => false, +}: { + numRequestsAllowed?: number; + settings: CachedSettings; + canBypass?: (userId: string) => Promise; +}) => { + const resolved: ResolvedRateLimiter | undefined = numRequestsAllowed + ? { + key: '/v1/testget', + rateLimiter: makeRateLimiter(numRequestsAllowed, 60_000) as any, + options: { numRequestsAllowed, intervalTimeInMS: 60_000 }, + } + : undefined; + + const api = new Router('/api') + .use(remoteAddressMiddleware) + .use(async (c: any, next: any) => { + const userId = c.req.header('x-user-id'); + c.set('user', userId ? { _id: userId } : null); + return next(); + }) + .use(rateLimiterMiddleware({ settings, resolve: () => resolved, canBypass })) + .get('/test', { response: { 200: ajv.compile({ type: 'object', properties: { success: { type: 'boolean' } } }) } }, async () => ({ + statusCode: 200 as const, + body: { success: true }, + })); + + const app = express(); + app.use(api.router); + return app; +}; + +const enabledSettings = () => { + const settings = new CachedSettings(); + settings.set({ _id: 'API_Enable_Rate_Limiter', value: true } as any); + settings.set({ _id: 'API_Enable_Rate_Limiter_Dev', value: true } as any); + return settings; +}; + +describe('Rate limiter middleware', () => { + it('should let requests through until the allowance is spent, then reject with 429', async () => { + const app = buildApp({ numRequestsAllowed: 2, settings: enabledSettings() }); + + const first = await request(app).get('/api/test').set('x-user-id', 'alice'); + const second = await request(app).get('/api/test').set('x-user-id', 'alice'); + const third = await request(app).get('/api/test').set('x-user-id', 'alice'); + + expect([first.status, second.status, third.status]).toEqual([200, 200, 429]); + expect(third.body).toEqual({ + success: false, + error: expect.stringContaining('too many requests'), + }); + expect(third.body.error).toContain('[error-too-many-requests]'); + }); + + it('should keep a separate allowance per user', async () => { + const app = buildApp({ numRequestsAllowed: 2, settings: enabledSettings() }); + + await request(app).get('/api/test').set('x-user-id', 'alice'); + await request(app).get('/api/test').set('x-user-id', 'alice'); + const aliceBlocked = await request(app).get('/api/test').set('x-user-id', 'alice'); + const bob = await request(app).get('/api/test').set('x-user-id', 'bob'); + + expect(aliceBlocked.status).toBe(429); + expect(bob.status).toBe(200); + }); + + it('should report the allowance on the response headers', async () => { + const app = buildApp({ numRequestsAllowed: 2, settings: enabledSettings() }); + + const res = await request(app).get('/api/test').set('x-user-id', 'alice'); + + expect(res.headers['x-ratelimit-limit']).toBe('2'); + expect(res.headers['x-ratelimit-remaining']).toBe('1'); + expect(Number(res.headers['x-ratelimit-reset'])).toBeGreaterThan(Date.now()); + }); + + it('should keep the headers on the 429 response', async () => { + const app = buildApp({ numRequestsAllowed: 1, settings: enabledSettings() }); + + await request(app).get('/api/test').set('x-user-id', 'alice'); + const blocked = await request(app).get('/api/test').set('x-user-id', 'alice'); + + expect(blocked.status).toBe(429); + expect(blocked.headers['x-ratelimit-limit']).toBe('1'); + expect(blocked.headers['x-ratelimit-remaining']).toBe('0'); + }); + + it('should not limit a user allowed to bypass', async () => { + const app = buildApp({ numRequestsAllowed: 1, settings: enabledSettings(), canBypass: async (userId) => userId === 'admin' }); + + const statuses = []; + for (let i = 0; i < 5; i++) { + statuses.push((await request(app).get('/api/test').set('x-user-id', 'admin')).status); + } + + expect(statuses).toEqual([200, 200, 200, 200, 200]); + }); + + it('should not limit when the route has no rule registered', async () => { + const app = buildApp({ settings: enabledSettings() }); + + const res = await request(app).get('/api/test').set('x-user-id', 'alice'); + + expect(res.status).toBe(200); + expect(res.headers['x-ratelimit-limit']).toBeUndefined(); + }); + + it('should not limit in development while the dev setting is off', async () => { + const settings = new CachedSettings(); + settings.set({ _id: 'API_Enable_Rate_Limiter', value: true } as any); + settings.set({ _id: 'API_Enable_Rate_Limiter_Dev', value: false } as any); + const previous = process.env.NODE_ENV; + process.env.NODE_ENV = 'development'; + + try { + const app = buildApp({ numRequestsAllowed: 1, settings }); + + await request(app).get('/api/test').set('x-user-id', 'alice'); + const second = await request(app).get('/api/test').set('x-user-id', 'alice'); + + expect(second.status).toBe(200); + } finally { + process.env.NODE_ENV = previous; + } + }); + + it('should not limit while the rate limiter setting is off', async () => { + const settings = new CachedSettings(); + settings.set({ _id: 'API_Enable_Rate_Limiter', value: false } as any); + const app = buildApp({ numRequestsAllowed: 1, settings }); + + await request(app).get('/api/test').set('x-user-id', 'alice'); + const second = await request(app).get('/api/test').set('x-user-id', 'alice'); + + expect(second.status).toBe(200); + }); +}); diff --git a/apps/meteor/server/api/v1/middlewares/rateLimiter.ts b/apps/meteor/server/api/v1/middlewares/rateLimiter.ts new file mode 100644 index 0000000000000..47af2ecfe4bd6 --- /dev/null +++ b/apps/meteor/server/api/v1/middlewares/rateLimiter.ts @@ -0,0 +1,69 @@ +import type { MiddlewareHandler } from 'hono'; +import type { ContentfulStatusCode } from 'hono/utils/http-status'; +import type { RateLimiter } from 'meteor/rate-limit'; + +import type { CachedSettings } from '../../../settings/CachedSettings'; +import type { RateLimiterOptions } from '../../definition'; +import { buildRateLimiterInput } from '../../rateLimiterKey'; +import type { HonoContext } from '../../router'; + +export type ResolvedRateLimiter = { key: string; rateLimiter: RateLimiter; options: RateLimiterOptions }; + +const reasonFor = (timeToReset: number) => + `Error, too many requests. Please slow down. You must wait ${Math.ceil(timeToReset / 1000)} seconds before trying this endpoint again.`; + +const defaultReject = (reason: string) => ({ + statusCode: 429 as const, + body: { success: false, error: `${reason} [error-too-many-requests]` }, +}); + +export const rateLimiterMiddleware = + ({ + settings, + resolve, + canBypass, + reject = defaultReject, + }: { + settings: CachedSettings; + resolve: (c: HonoContext) => ResolvedRateLimiter | undefined; + canBypass: (userId: string) => Promise; + reject?: (reason: string) => { statusCode: ContentfulStatusCode; body: unknown }; + }): MiddlewareHandler => + async (c: HonoContext, next) => { + const limiter = resolve(c); + + if (!limiter) { + return next(); + } + + if (settings.get('API_Enable_Rate_Limiter') !== true) { + return next(); + } + + if (process.env.NODE_ENV === 'development' && settings.get('API_Enable_Rate_Limiter_Dev') !== true) { + return next(); + } + + const userId = c.get('user')?._id; + + if (userId && (await canBypass(userId))) { + return next(); + } + + const input = buildRateLimiterInput({ route: limiter.key, IPAddr: c.get('remoteAddress'), userId }); + + limiter.rateLimiter.increment(input); + const attempt = await limiter.rateLimiter.check(input); + + c.res.headers.set('X-RateLimit-Limit', String(limiter.options.numRequestsAllowed ?? '')); + c.res.headers.set('X-RateLimit-Remaining', String(attempt.numInvocationsLeft)); + c.res.headers.set('X-RateLimit-Reset', String(Date.now() + attempt.timeToReset)); + + if (attempt.allowed) { + return next(); + } + + const { statusCode, body } = reject(reasonFor(attempt.timeToReset)); + + return c.json(body, statusCode); + }; diff --git a/apps/meteor/server/api/webhooks.ts b/apps/meteor/server/api/webhooks.ts index 700f2be365164..52170c42c7fa7 100644 --- a/apps/meteor/server/api/webhooks.ts +++ b/apps/meteor/server/api/webhooks.ts @@ -4,7 +4,6 @@ import { Random } from '@rocket.chat/random'; import { isIntegrationsHooksAddSchema, isIntegrationsHooksRemoveSchema } from '@rocket.chat/rest-typings'; import type express from 'express'; import { Meteor } from 'meteor/meteor'; -import type { RateLimiterOptionsToCheck } from 'meteor/rate-limit'; import { WebApp } from 'meteor/webapp'; import _ from 'underscore'; @@ -12,7 +11,7 @@ import { APIClass } from './ApiClass'; import type { RateLimiterOptions } from './api'; import { API, defaultRateLimiterOptions } from './api'; import type { FailureResult, GenericRouteExecutionContext, SuccessResult, UnavailableResult } from './definition'; -import type { APIActionContext } from './router'; +import type { APIActionContext, HonoContext } from './router'; import { isPlainObject } from '../../lib/utils/isPlainObject'; import { hasPermissionAsync } from '../lib/authorization/hasPermission'; import { IsolatedVMScriptEngine } from '../lib/integrations/lib/isolated-vm/isolated-vm'; @@ -20,6 +19,7 @@ import { metrics } from '../lib/metrics'; import { settings } from '../settings'; import { loggerMiddleware } from './v1/middlewares/logger'; import { metricsMiddleware } from './v1/middlewares/metrics'; +import type { ResolvedRateLimiter } from './v1/middlewares/rateLimiter'; import { tracerSpanMiddleware } from './v1/middlewares/tracer'; import { incomingLogger, integrationLogger } from '../lib/integrations/logger'; import type { WebhookResponseItem } from '../lib/messages/processWebhookMessage'; @@ -395,22 +395,11 @@ class WebHookAPI extends APIClass<'/hooks'> { ); } - override async shouldVerifyRateLimit(): Promise { - return ( - settings.get('API_Enable_Rate_Limiter') === true && - (process.env.NODE_ENV !== 'development' || settings.get('API_Enable_Rate_Limiter_Dev') === true) - ); - } - - override async enforceRateLimit( - objectForRateLimitMatch: RateLimiterOptionsToCheck, - request: Request, - response: Response, - userId: string, - ): Promise { - const { method, url } = request; + override resolveRateLimiter(c: HonoContext, _route: string, _method: string): ResolvedRateLimiter | undefined { + const { method, url } = c.req.raw; const route = url.replace(`/${this.apiPath}`, ''); const nameRoute = this.getFullRouteName(route, method.toLowerCase()); + if (!this.getRateLimiter(nameRoute)) { this.addRateLimiterRuleForRoutes({ routes: [route], @@ -422,10 +411,13 @@ class WebHookAPI extends APIClass<'/hooks'> { }); } - const integrationForRateLimitMatch = objectForRateLimitMatch; - integrationForRateLimitMatch.route = nameRoute; + const entry = this.getRateLimiter(nameRoute); + + return entry && { key: nameRoute, ...entry }; + } - await super.enforceRateLimit(integrationForRateLimitMatch, request, response, userId); + override async canBypassRateLimit(): Promise { + return false; } }