chore: remove rate limiter for functions (#38354)

pull/38173/head^2
Guilherme Gazzo 3 months ago committed by GitHub
parent aa37226dad
commit c4eff65b38
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 17
      apps/meteor/app/api/server/v1/users.ts
  2. 4
      apps/meteor/app/crowd/server/crowd.ts
  3. 4
      apps/meteor/app/lib/server/functions/saveUserIdentity.ts
  4. 12
      apps/meteor/app/lib/server/functions/setEmail.ts
  5. 12
      apps/meteor/app/lib/server/functions/setRealName.ts
  6. 13
      apps/meteor/app/lib/server/functions/setStatusText.ts
  7. 84
      apps/meteor/app/lib/server/lib/RateLimiter.js
  8. 6
      apps/meteor/app/lib/server/methods/setRealName.ts
  9. 4
      apps/meteor/server/lib/cas/loginHandler.ts
  10. 6
      apps/meteor/tests/unit/app/lib/server/functions/setUsername.spec.ts
  11. 15
      apps/meteor/tests/unit/server/users/saveUserIdentity.spec.ts

@ -153,7 +153,14 @@ API.v1.addRoute(
API.v1.addRoute(
'users.updateOwnBasicInfo',
{ authRequired: true, validateParams: isUsersUpdateOwnBasicInfoParamsPOST },
{
authRequired: true,
validateParams: isUsersUpdateOwnBasicInfoParamsPOST,
rateLimiterOptions: {
numRequestsAllowed: 1,
intervalTimeInMS: 60000,
},
},
{
async post() {
const userData = {
@ -1374,7 +1381,13 @@ API.v1.addRoute(
API.v1.addRoute(
'users.setStatus',
{ authRequired: true },
{
authRequired: true,
rateLimiterOptions: {
numRequestsAllowed: 5,
intervalTimeInMS: 60000,
},
},
{
async post() {
check(

@ -9,7 +9,7 @@ import { Meteor } from 'meteor/meteor';
import { logger } from './logger';
import { crowdIntervalValuesToCronMap } from '../../../server/settings/crowd';
import { deleteUser } from '../../lib/server/functions/deleteUser';
import { _setRealName } from '../../lib/server/functions/setRealName';
import { setRealName } from '../../lib/server/functions/setRealName';
import { setUserActiveStatus } from '../../lib/server/functions/setUserActiveStatus';
import { notifyOnUserChange, notifyOnUserChangeById, notifyOnUserChangeAsync } from '../../lib/server/lib/notifyListener';
import { settings } from '../../settings/server';
@ -206,7 +206,7 @@ export class CROWD {
}
if (crowdUser.displayname) {
await _setRealName(id, crowdUser.displayname);
await setRealName(id, crowdUser.displayname);
}
await Users.updateOne(

@ -3,7 +3,7 @@ import type { Updater } from '@rocket.chat/models';
import { Messages, VideoConference, LivechatDepartmentAgents, Rooms, Subscriptions, Users, CallHistory } from '@rocket.chat/models';
import type { ClientSession } from 'mongodb';
import { _setRealName } from './setRealName';
import { setRealName } from './setRealName';
import { _setUsername } from './setUsername';
import { updateGroupDMsName } from './updateGroupDMsName';
import { validateName } from './validateName';
@ -65,7 +65,7 @@ export async function saveUserIdentity({
}
if (typeof rawName !== 'undefined' && nameChanged) {
if (!(await _setRealName(_id, name, user, updater, session))) {
if (!(await setRealName(_id, name, user, updater, session))) {
return false;
}
}

@ -6,10 +6,9 @@ import { Meteor } from 'meteor/meteor';
import type { ClientSession } from 'mongodb';
import { onceTransactionCommitedSuccessfully } from '../../../../server/database/utils';
import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission';
import * as Mailer from '../../../mailer/server/api';
import { settings } from '../../../settings/server';
import { RateLimiter, validateEmailDomain } from '../lib';
import { validateEmailDomain } from '../lib';
import { checkEmailAvailability } from './checkEmailAvailability';
import { sendConfirmationEmail } from '../../../../server/methods/sendConfirmationEmail';
@ -42,7 +41,7 @@ const _sendEmailChangeNotification = async function (to: string, newEmail: strin
}
};
const _setEmail = async function (
export const setEmail = async function (
userId: string,
email: string,
shouldSendVerificationEmail = true,
@ -105,10 +104,3 @@ const _setEmail = async function (
}
return result;
};
export const setEmail = RateLimiter.limitFunction(_setEmail, 1, 60000, {
async 0() {
const userId = Meteor.userId();
return !userId || !(await hasPermissionAsync(userId, 'edit-other-user-info'));
}, // Administrators have permission to change others emails, so don't limit those
});

@ -2,15 +2,12 @@ import { api } from '@rocket.chat/core-services';
import type { IUser } from '@rocket.chat/core-typings';
import type { Updater } from '@rocket.chat/models';
import { Users } from '@rocket.chat/models';
import { Meteor } from 'meteor/meteor';
import type { ClientSession } from 'mongodb';
import { onceTransactionCommitedSuccessfully } from '../../../../server/database/utils';
import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission';
import { settings } from '../../../settings/server';
import { RateLimiter } from '../lib';
export const _setRealName = async function (
export const setRealName = async function (
userId: string,
name: string,
fullUser?: IUser,
@ -65,10 +62,3 @@ export const _setRealName = async function (
return user;
};
export const setRealName = RateLimiter.limitFunction(_setRealName, 1, 60000, {
async 0() {
const userId = Meteor.userId();
return !userId || !(await hasPermissionAsync(userId, 'edit-other-user-info'));
}, // Administrators have permission to change others names, so don't limit those
});

@ -2,14 +2,11 @@ import { api } from '@rocket.chat/core-services';
import type { IUser } from '@rocket.chat/core-typings';
import type { Updater } from '@rocket.chat/models';
import { Users } from '@rocket.chat/models';
import { Meteor } from 'meteor/meteor';
import type { ClientSession } from 'mongodb';
import { onceTransactionCommitedSuccessfully } from '../../../../server/database/utils';
import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission';
import { RateLimiter } from '../lib';
async function _setStatusText(
export async function setStatusText(
userId: string,
statusText: string,
{
@ -59,11 +56,3 @@ async function _setStatusText(
return true;
}
export const setStatusText = RateLimiter.limitFunction(_setStatusText, 5, 60000, {
async 0() {
// Administrators have permission to change others status, so don't limit those
const userId = Meteor.userId();
return !userId || !(await hasPermissionAsync(userId, 'edit-other-user-info'));
},
});

@ -1,90 +1,6 @@
import { DDPRateLimiter } from 'meteor/ddp-rate-limiter';
import { Meteor } from 'meteor/meteor';
import { RateLimiter } from 'meteor/rate-limit';
export const RateLimiterClass = new (class {
limitFunction(fn, numRequests, timeInterval, matchers) {
if (process.env.TEST_MODE === 'true') {
return fn;
}
const rateLimiter = new (class extends RateLimiter {
async check(input) {
const reply = {
allowed: true,
timeToReset: 0,
numInvocationsLeft: Infinity,
};
const matchedRules = this._findAllMatchingRules(input);
for await (const rule of matchedRules) {
const ruleResult = await rule.apply(input);
let numInvocations = rule.counters[ruleResult.key];
if (ruleResult.timeToNextReset < 0) {
// Reset all the counters since the rule has reset
await rule.resetCounter();
ruleResult.timeSinceLastReset = new Date().getTime() - rule._lastResetTime;
ruleResult.timeToNextReset = rule.options.intervalTime;
numInvocations = 0;
}
if (numInvocations > rule.options.numRequestsAllowed) {
// Only update timeToReset if the new time would be longer than the
// previously set time. This is to ensure that if this input triggers
// multiple rules, we return the longest period of time until they can
// successfully make another call
if (reply.timeToReset < ruleResult.timeToNextReset) {
reply.timeToReset = ruleResult.timeToNextReset;
}
reply.allowed = false;
reply.numInvocationsLeft = 0;
reply.ruleId = rule.id;
await rule._executeCallback(reply, input);
} else {
// If this is an allowed attempt and we haven't failed on any of the
// other rules that match, update the reply field.
if (rule.options.numRequestsAllowed - numInvocations < reply.numInvocationsLeft && reply.allowed) {
reply.timeToReset = ruleResult.timeToNextReset;
reply.numInvocationsLeft = rule.options.numRequestsAllowed - numInvocations;
}
reply.ruleId = rule.id;
await rule._executeCallback(reply, input);
}
}
return reply;
}
})();
Object.entries(matchers).forEach(([key, matcher]) => {
matchers[key] = matcher;
});
rateLimiter.addRule(matchers, numRequests, timeInterval);
return async function (...args) {
const match = {};
Object.keys(matchers).forEach((key) => {
match[key] = args[key];
});
rateLimiter.increment(match);
const rateLimitResult = await rateLimiter.check(match);
if (rateLimitResult.allowed) {
return fn.apply(null, args);
}
throw new Meteor.Error(
'error-too-many-requests',
`Error, too many requests. Please slow down. You must wait ${Math.ceil(
rateLimitResult.timeToReset / 1000,
)} seconds before trying again.`,
{
timeToReset: rateLimitResult.timeToReset,
seconds: Math.ceil(rateLimitResult.timeToReset / 1000),
},
);
};
}
limitMethod(methodName, numRequests, timeInterval, matchers) {
if (process.env.TEST_MODE === 'true') {
return;

@ -16,8 +16,8 @@ declare module '@rocket.chat/ddp-client' {
Meteor.methods<ServerMethods>({
async setRealName(name) {
check(name, String);
if (!Meteor.userId()) {
const userId = Meteor.userId();
if (!userId) {
throw new Meteor.Error('error-invalid-user', 'Invalid user', { method: 'setRealName' });
}
@ -25,7 +25,7 @@ Meteor.methods<ServerMethods>({
throw new Meteor.Error('error-not-allowed', 'Not allowed', { method: 'setRealName' });
}
if (!(await setRealName(Meteor.userId(), name))) {
if (!(await setRealName(userId, name))) {
throw new Meteor.Error('error-could-not-change-name', 'Could not change name', {
method: 'setRealName',
});

@ -5,7 +5,7 @@ import { Accounts } from 'meteor/accounts-base';
import { createNewUser } from './createNewUser';
import { findExistingCASUser } from './findExistingCASUser';
import { logger } from './logger';
import { _setRealName } from '../../../app/lib/server/functions/setRealName';
import { setRealName } from '../../../app/lib/server/functions/setRealName';
import { settings } from '../../../app/settings/server';
export const loginHandlerCAS = async (options: any): Promise<undefined | Accounts.LoginMethodResult> => {
@ -90,7 +90,7 @@ export const loginHandlerCAS = async (options: any): Promise<undefined | Account
logger.debug('Syncing user attributes');
// Update name
if (internalAttributes.name) {
await _setRealName(user._id, internalAttributes.name);
await setRealName(user._id, internalAttributes.name);
}
// Update email

@ -37,9 +37,6 @@ describe('setUsername', () => {
setUserAvatar: sinon.stub(),
addUserToRoom: sinon.stub(),
notifyOnUserChange: sinon.stub(),
RateLimiter: {
limitFunction: sinon.stub(),
},
underscore: {
escape: sinon.stub(),
},
@ -56,7 +53,7 @@ describe('setUsername', () => {
'meteor/accounts-base': { Accounts: stubs.Accounts },
'underscore': stubs.underscore,
'../../../settings/server': { settings: stubs.settings },
'../lib': { notifyOnUserChange: stubs.notifyOnUserChange, RateLimiter: stubs.RateLimiter },
'../lib': { notifyOnUserChange: stubs.notifyOnUserChange },
'./addUserToRoom': { addUserToRoom: stubs.addUserToRoom },
'./checkUsernameAvailability': { checkUsernameAvailability: stubs.checkUsernameAvailability },
'./getAvatarSuggestionForUser': { getAvatarSuggestionForUser: stubs.getAvatarSuggestionForUser },
@ -92,7 +89,6 @@ describe('setUsername', () => {
stubs.setUserAvatar.reset();
stubs.addUserToRoom.reset();
stubs.notifyOnUserChange.reset();
stubs.RateLimiter.limitFunction.reset();
stubs.underscore.escape.reset();
stubs.SystemLogger.reset();
});

@ -45,13 +45,13 @@ const { saveUserIdentity } = proxyquire.noCallThru().load('../../../../app/lib/s
'../../../../app/file-upload/server': {
FileUpload: stubs.FileUpload,
},
'../../../../app/lib/server/functions/setRealName': {
_setRealName: stubs.setRealName,
},
'../../../../app/lib/server/functions/setUsername': {
_setUsername: stubs.setUsername,
_setUsernameWithSession: () => stubs.setUsername,
},
'../../../../app/lib/server/functions/setRealName': {
setRealName: stubs.setRealName,
},
'../../../../app/lib/server/functions/updateGroupDMsName': {
updateGroupDMsName: sinon.stub(),
},
@ -117,15 +117,6 @@ describe('Users - saveUserIdentity', () => {
expect(stubs.updateHistoryReferences.called).to.be.false;
});
it('should return false if _setName fails', async () => {
stubs.findOneUserById.returns({ name: 'oldName' });
stubs.setRealName.returns(false);
const result = await saveUserIdentity({ _id: 'valid_id', name: 'invalidName' });
expect(stubs.setRealName.calledWith('valid_id', 'invalidName', { name: 'oldName' })).to.be.true;
expect(result).to.be.false;
});
it('should update Subscriptions, VideoConference and Call History if name changes', async () => {
stubs.findOneUserById.returns({ name: 'oldName', username: 'oldUsername' });
stubs.setRealName.returns(true);

Loading…
Cancel
Save