regression: add chat.sendMessage rate limiter

apps/meteor/server/api/ApiClass.ts

@@ -18,7 +18,7 @@ import type { RateLimiterOptionsToCheck } from 'meteor/rate-limit';
 import { RateLimiter } from 'meteor/rate-limit';
 import _ from 'underscore';
 
-import { checkPermissions, parseDeprecation } from './api.helpers';
+import { canBypassRateLimit, checkPermissions, parseDeprecation } from './api.helpers';
 import type {
 	FailureResult,
 	ForbiddenResult,
@@ -27,6 +27,7 @@ import type {
 	NotFoundResult,
 	Operations,
 	Options,
+	RateLimiterOptions,
 	SuccessResult,
 	TypedThis,
 	TypedAction,
@@ -45,7 +46,6 @@ import type { APIActionContext } from './router';
 import { RocketChatAPIRouter } from './router';
 import { isObject } from '../../lib/utils/isObject';
 import { checkCodeForUser } from '../lib/2fa/code';
-import { hasPermissionAsync } from '../lib/authorization/hasPermission';
 import { getNestedProp } from '../lib/getNestedProp';
 import { notifyOnUserChangeAsync } from '../lib/notifyListener';
 import { shouldBreakInVersion } from '../lib/shouldBreakInVersion';
@@ -127,11 +127,6 @@ interface IAPIDefaultFieldsToExclude {
 	inviteToken: number;
 }
 
-export type RateLimiterOptions = {
-	numRequestsAllowed?: number;
-	intervalTimeInMS?: number;
-};
-
 export const defaultRateLimiterOptions: RateLimiterOptions = {
 	numRequestsAllowed: settings.get<number>('API_Enable_Rate_Limiter_Limit_Calls_Default'),
 	intervalTimeInMS: settings.get<number>('API_Enable_Rate_Limiter_Limit_Time_Default'),
@@ -420,7 +415,7 @@ export class APIClass<TBasePath extends string = '', TOperations extends Record<
 			rateLimiterDictionary.hasOwnProperty(route) &&
 			settings.get<boolean>('API_Enable_Rate_Limiter') === true &&
 			(process.env.NODE_ENV !== 'development' || settings.get<boolean>('API_Enable_Rate_Limiter_Dev') === true) &&
-			!(userId && (await hasPermissionAsync(userId, 'api-bypass-rate-limit')))
+			!(userId && (await canBypassRateLimit(userId, rateLimiterDictionary[route].options.bypassPermissions)))
 		);
 	}
 

apps/meteor/server/api/api.helpers.ts

@@ -39,6 +39,10 @@ const isPermissionsPayload = (permissionsPayload: PermissionsRequiredKey): permi
 	);
 };
 
+export async function canBypassRateLimit(userId: IUser['_id'], bypassPermissions: string[] = []): Promise<boolean> {
+	return hasAtLeastOnePermissionAsync(userId, ['api-bypass-rate-limit', ...bypassPermissions]);
+}
+
 export async function checkPermissionsForInvocation(
 	userId: IUser['_id'],
 	permissionsPayload: PermissionsPayload,

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<T> = {
 	[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<number>('API_Enable_Rate_Limiter_Limit_Calls_Default'),

apps/meteor/server/api/definition.ts

@@ -99,6 +99,12 @@ export type NonEnterpriseTwoFactorOptions = {
 	twoFactorOptions: ITwoFactorOptions;
 };
 
+export type RateLimiterOptions = {
+	numRequestsAllowed?: number;
+	intervalTimeInMS?: number;
+	bypassPermissions?: string[];
+};
+
 export type Options = SharedOptions<'GET' | 'POST' | 'PUT' | 'DELETE' | 'PATCH'>;
 
 export type SharedOptions<TMethod extends string> = (
@@ -112,12 +118,7 @@ export type SharedOptions<TMethod extends string> = (
 			authRequired?: boolean;
 			userWithoutUsername?: boolean;
 			forceTwoFactorAuthenticationForNonEnterprise?: boolean;
-			rateLimiterOptions?:
-				| {
-						numRequestsAllowed?: number;
-						intervalTimeInMS?: number;
-				  }
-				| boolean;
+			rateLimiterOptions?: RateLimiterOptions | boolean;
 			queryOperations?: string[];
 			queryFields?: string[];
 	  }
@@ -132,12 +133,7 @@ export type SharedOptions<TMethod extends string> = (
 			userWithoutUsername?: boolean;
 			twoFactorRequired: true;
 			twoFactorOptions?: ITwoFactorOptions;
-			rateLimiterOptions?:
-				| {
-						numRequestsAllowed?: number;
-						intervalTimeInMS?: number;
-				  }
-				| boolean;
+			rateLimiterOptions?: RateLimiterOptions | boolean;
 
 			queryOperations?: string[];
 			queryFields?: string[];

apps/meteor/server/api/v1/chat.ts

@@ -895,6 +895,7 @@ const chatEndpoints = API.v1
 		'chat.sendMessage',
 		{
 			authRequired: true,
+			rateLimiterOptions: { numRequestsAllowed: 5, intervalTimeInMS: 1000, bypassPermissions: ['send-many-messages'] },
 			body: isChatSendMessageProps,
 			response: {
 				200: ajv.compile<{ message: IMessage }>({

apps/meteor/tests/unit/server/api/canBypassRateLimit.spec.ts

@@ -0,0 +1,40 @@
+import { expect } from 'chai';
+import { describe, it } from 'mocha';
+import mock from 'proxyquire';
+
+const userPermissions: Record<string, string[]> = {
+	bot: ['api-bypass-rate-limit', 'send-many-messages'],
+	legacyBot: ['send-many-messages'],
+	regular: [],
+};
+
+const mocks = {
+	'../lib/authorization/hasPermission': {
+		hasAtLeastOnePermissionAsync: async (userId: string, permissions: string[]): Promise<boolean> =>
+			permissions.some((permission) => userPermissions[userId].includes(permission)),
+	},
+	'../lib/deprecationWarningLogger': {
+		apiDeprecationLogger: { endpoint: () => undefined },
+	},
+};
+
+const { canBypassRateLimit } = mock.noCallThru().load('../../../../server/api/api.helpers', mocks);
+
+describe('canBypassRateLimit', () => {
+	it('should let a user holding api-bypass-rate-limit through on any route', async () => {
+		expect(await canBypassRateLimit('bot')).to.be.true;
+	});
+
+	it('should not let a regular user through', async () => {
+		expect(await canBypassRateLimit('regular')).to.be.false;
+		expect(await canBypassRateLimit('regular', ['send-many-messages'])).to.be.false;
+	});
+
+	it('should let a user holding a route specific permission through', async () => {
+		expect(await canBypassRateLimit('legacyBot', ['send-many-messages'])).to.be.true;
+	});
+
+	it('should not let a route specific permission apply to routes that do not declare it', async () => {
+		expect(await canBypassRateLimit('legacyBot')).to.be.false;
+	});
+});