refactor: Move add and remove role code out of Roles model (#34488)

apps/meteor/app/api/server/v1/roles.ts

@@ -5,6 +5,7 @@ import { isRoleAddUserToRoleProps, isRoleDeleteProps, isRoleRemoveUserFromRolePr
 import { check, Match } from 'meteor/check';
 import { Meteor } from 'meteor/meteor';
 
+import { removeUserFromRolesAsync } from '../../../../server/lib/roles/removeUserFromRoles';
 import { getUsersInRolePaginated } from '../../../authorization/server/functions/getUsersInRole';
 import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission';
 import { hasRoleAsync, hasAnyRoleAsync } from '../../../authorization/server/functions/hasRole';
@@ -221,7 +222,7 @@ API.v1.addRoute(
 				}
 			}
 
-			await Roles.removeUserRoles(user._id, [role._id], scope);
+			await removeUserFromRolesAsync(user._id, [role._id], scope);
 
 			if (settings.get('UI_DisplayRoles')) {
 				void api.broadcast('user.roleUpdate', {

apps/meteor/app/authorization/server/methods/addUserToRole.ts

@@ -4,6 +4,7 @@ import type { ServerMethods } from '@rocket.chat/ddp-client';
 import { Roles, Users } from '@rocket.chat/models';
 import { Meteor } from 'meteor/meteor';
 
+import { addUserRolesAsync } from '../../../../server/lib/roles/addUserRoles';
 import { methodDeprecationLogger } from '../../../lib/server/lib/deprecationWarningLogger';
 import { settings } from '../../../settings/server';
 import { hasPermissionAsync } from '../functions/hasPermission';
@@ -75,7 +76,7 @@ Meteor.methods<ServerMethods>({
 			});
 		}
 
-		const add = await Roles.addUserRoles(user._id, [role._id], scope);
+		const add = await addUserRolesAsync(user._id, [role._id], scope);
 
 		if (settings.get('UI_DisplayRoles')) {
 			void api.broadcast('user.roleUpdate', {

apps/meteor/app/authorization/server/methods/removeUserFromRole.ts

@@ -4,6 +4,7 @@ import type { ServerMethods } from '@rocket.chat/ddp-client';
 import { Roles, Users } from '@rocket.chat/models';
 import { Meteor } from 'meteor/meteor';
 
+import { removeUserFromRolesAsync } from '../../../../server/lib/roles/removeUserFromRoles';
 import { methodDeprecationLogger } from '../../../lib/server/lib/deprecationWarningLogger';
 import { settings } from '../../../settings/server';
 import { hasPermissionAsync } from '../functions/hasPermission';
@@ -79,7 +80,7 @@ Meteor.methods<ServerMethods>({
 			}
 		}
 
-		const remove = await Roles.removeUserRoles(user._id, [role._id], scope);
+		const remove = await removeUserFromRolesAsync(user._id, [role._id], scope);
 		const event = {
 			type: 'removed',
 			_id: role._id,

apps/meteor/app/integrations/server/methods/incoming/addIncomingIntegration.ts

@@ -1,12 +1,13 @@
 import type { INewIncomingIntegration, IIncomingIntegration } from '@rocket.chat/core-typings';
 import type { ServerMethods } from '@rocket.chat/ddp-client';
-import { Integrations, Roles, Subscriptions, Users, Rooms } from '@rocket.chat/models';
+import { Integrations, Subscriptions, Users, Rooms } from '@rocket.chat/models';
 import { Random } from '@rocket.chat/random';
 import { Babel } from 'meteor/babel-compiler';
 import { Match, check } from 'meteor/check';
 import { Meteor } from 'meteor/meteor';
 import _ from 'underscore';
 
+import { addUserRolesAsync } from '../../../../../server/lib/roles/addUserRoles';
 import { hasPermissionAsync, hasAllPermissionAsync } from '../../../../authorization/server/functions/hasPermission';
 import { notifyOnIntegrationChanged } from '../../../../lib/server/lib/notifyListener';
 import { validateScriptEngine, isScriptEngineFrozen } from '../../lib/validateScriptEngine';
@@ -154,7 +155,7 @@ export const addIncomingIntegration = async (userId: string, integration: INewIn
 		}
 	}
 
-	await Roles.addUserRoles(user._id, ['bot']);
+	await addUserRolesAsync(user._id, ['bot']);
 
 	const { insertedId } = await Integrations.insertOne(integrationData);
 

apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts

@@ -1,11 +1,12 @@
 import type { IIntegration, INewIncomingIntegration, IUpdateIncomingIntegration } from '@rocket.chat/core-typings';
 import type { ServerMethods } from '@rocket.chat/ddp-client';
-import { Integrations, Roles, Subscriptions, Users, Rooms } from '@rocket.chat/models';
+import { Integrations, Subscriptions, Users, Rooms } from '@rocket.chat/models';
 import { wrapExceptions } from '@rocket.chat/tools';
 import { Babel } from 'meteor/babel-compiler';
 import { Meteor } from 'meteor/meteor';
 import _ from 'underscore';
 
+import { addUserRolesAsync } from '../../../../../server/lib/roles/addUserRoles';
 import { hasAllPermissionAsync, hasPermissionAsync } from '../../../../authorization/server/functions/hasPermission';
 import { notifyOnIntegrationChanged } from '../../../../lib/server/lib/notifyListener';
 import { isScriptEngineFrozen, validateScriptEngine } from '../../lib/validateScriptEngine';
@@ -164,7 +165,7 @@ Meteor.methods<ServerMethods>({
 			});
 		}
 
-		await Roles.addUserRoles(user._id, ['bot']);
+		await addUserRolesAsync(user._id, ['bot']);
 
 		const updatedIntegration = await Integrations.findOneAndUpdate(
 			{ _id: integrationId },

apps/meteor/app/lib/server/functions/relinquishRoomOwnerships.ts

@@ -1,6 +1,7 @@
-import { Messages, Roles, Rooms, Subscriptions, ReadReceipts } from '@rocket.chat/models';
+import { Messages, Rooms, Subscriptions, ReadReceipts } from '@rocket.chat/models';
 
 import type { SubscribedRoomsForUserWithDetails } from './getRoomsWithSingleOwner';
+import { addUserRolesAsync } from '../../../../server/lib/roles/addUserRoles';
 import { FileUpload } from '../../../file-upload/server';
 import { notifyOnSubscriptionChanged } from '../lib/notifyListener';
 
@@ -29,7 +30,7 @@ export const relinquishRoomOwnerships = async function (
 	const changeOwner = subscribedRooms.filter(({ shouldChangeOwner }) => shouldChangeOwner);
 
 	for await (const { newOwner, rid } of changeOwner) {
-		newOwner && (await Roles.addUserRoles(newOwner, ['owner'], rid));
+		newOwner && (await addUserRolesAsync(newOwner, ['owner'], rid));
 	}
 
 	const roomIdsToRemove: string[] = subscribedRooms.filter(({ shouldBeRemoved }) => shouldBeRemoved).map(({ rid }) => rid);

apps/meteor/server/lib/roles/addUserRoles.ts

@@ -1,18 +1,45 @@
 import { MeteorError } from '@rocket.chat/core-services';
 import type { IRole, IUser, IRoom } from '@rocket.chat/core-typings';
-import { Roles } from '@rocket.chat/models';
+import { Roles, Subscriptions, Users } from '@rocket.chat/models';
 
 import { validateRoleList } from './validateRoleList';
+import { notifyOnSubscriptionChangedByRoomIdAndUserId } from '../../../app/lib/server/lib/notifyListener';
 
-export const addUserRolesAsync = async (userId: IUser['_id'], roleIds: IRole['_id'][], scope?: IRoom['_id']): Promise<boolean> => {
-	if (!userId || !roleIds?.length) {
+export const addUserRolesAsync = async (userId: IUser['_id'], roles: IRole['_id'][], scope?: IRoom['_id']): Promise<boolean> => {
+	if (!userId || !roles?.length) {
 		return false;
 	}
 
-	if (!(await validateRoleList(roleIds))) {
+	if (!(await validateRoleList(roles))) {
 		throw new MeteorError('error-invalid-role', 'Invalid role');
 	}
 
-	await Roles.addUserRoles(userId, roleIds, scope);
+	if (process.env.NODE_ENV === 'development' && (scope === 'Users' || scope === 'Subscriptions')) {
+		throw new Error('Roles.addUserRoles method received a role scope instead of a scope value.');
+	}
+
+	if (!Array.isArray(roles)) {
+		roles = [roles];
+		process.env.NODE_ENV === 'development' && console.warn('[WARN] RolesRaw.addUserRoles: roles should be an array');
+	}
+
+	for await (const roleId of roles) {
+		const role = await Roles.findOneById<Pick<IRole, '_id' | 'scope'>>(roleId, { projection: { scope: 1 } });
+
+		if (!role) {
+			process.env.NODE_ENV === 'development' && console.warn(`[WARN] RolesRaw.addUserRoles: role: ${roleId} not found`);
+			continue;
+		}
+
+		if (role.scope === 'Subscriptions' && scope) {
+			const addRolesResponse = await Subscriptions.addRolesByUserId(userId, [role._id], scope);
+			if (addRolesResponse.modifiedCount) {
+				void notifyOnSubscriptionChangedByRoomIdAndUserId(scope, userId);
+			}
+		} else {
+			await Users.addRolesByUserId(userId, [role._id]);
+		}
+	}
+
 	return true;
 };

apps/meteor/server/lib/roles/removeUserFromRoles.ts

@@ -1,11 +1,12 @@
 import { MeteorError } from '@rocket.chat/core-services';
 import type { IRole, IUser, IRoom } from '@rocket.chat/core-typings';
-import { Users, Roles } from '@rocket.chat/models';
+import { Users, Subscriptions, Roles } from '@rocket.chat/models';
 
 import { validateRoleList } from './validateRoleList';
+import { notifyOnSubscriptionChangedByRoomIdAndUserId } from '../../../app/lib/server/lib/notifyListener';
 
-export const removeUserFromRolesAsync = async (userId: IUser['_id'], roleIds: IRole['_id'][], scope?: IRoom['_id']): Promise<boolean> => {
-	if (!userId || !roleIds) {
+export const removeUserFromRolesAsync = async (userId: IUser['_id'], roles: IRole['_id'][], scope?: IRoom['_id']): Promise<boolean> => {
+	if (!userId || !roles) {
 		return false;
 	}
 
@@ -14,10 +15,29 @@ export const removeUserFromRolesAsync = async (userId: IUser['_id'], roleIds: IR
 		throw new MeteorError('error-invalid-user', 'Invalid user');
 	}
 
-	if (!(await validateRoleList(roleIds))) {
+	if (!(await validateRoleList(roles))) {
 		throw new MeteorError('error-invalid-role', 'Invalid role');
 	}
 
-	await Roles.removeUserRoles(userId, roleIds, scope);
+	if (process.env.NODE_ENV === 'development' && (scope === 'Users' || scope === 'Subscriptions')) {
+		throw new Error('Roles.removeUserRoles method received a role scope instead of a scope value.');
+	}
+
+	for await (const roleId of roles) {
+		const role = await Roles.findOneById<Pick<IRole, '_id' | 'scope'>>(roleId, { projection: { scope: 1 } });
+		if (!role) {
+			continue;
+		}
+
+		if (role.scope === 'Subscriptions' && scope) {
+			const removeRolesResponse = await Subscriptions.removeRolesByUserId(userId, [roleId], scope);
+			if (removeRolesResponse.modifiedCount) {
+				void notifyOnSubscriptionChangedByRoomIdAndUserId(scope, userId);
+			}
+		} else {
+			await Users.removeRolesByUserId(userId, [roleId]);
+		}
+	}
+
 	return true;
 };

apps/meteor/server/methods/afterVerifyEmail.ts

@@ -1,8 +1,11 @@
 import type { IRole } from '@rocket.chat/core-typings';
 import type { ServerMethods } from '@rocket.chat/ddp-client';
-import { Roles, Users } from '@rocket.chat/models';
+import { Users } from '@rocket.chat/models';
 import { Meteor } from 'meteor/meteor';
 
+import { addUserRolesAsync } from '../lib/roles/addUserRoles';
+import { removeUserFromRolesAsync } from '../lib/roles/removeUserFromRoles';
+
 const rolesToChangeTo: Map<IRole['_id'], [IRole['_id']]> = new Map([['anonymous', ['user']]]);
 
 declare module '@rocket.chat/ddp-client' {
@@ -33,9 +36,9 @@ Meteor.methods<ServerMethods>({
 					rolesThatNeedChanges.map(async (role) => {
 						const rolesToAdd = rolesToChangeTo.get(role);
 						if (rolesToAdd) {
-							await Roles.addUserRoles(userId, rolesToAdd);
+							await addUserRolesAsync(userId, rolesToAdd);
 						}
-						await Roles.removeUserRoles(user._id, [role]);
+						await removeUserFromRolesAsync(user._id, [role]);
 					}),
 				);
 			}

apps/meteor/server/models/raw/Roles.ts

@@ -4,7 +4,6 @@ import { Subscriptions, Users } from '@rocket.chat/models';
 import type { Collection, FindCursor, Db, Filter, FindOptions, Document, CountDocumentsOptions } from 'mongodb';
 
 import { BaseRaw } from './BaseRaw';
-import { notifyOnSubscriptionChangedByRoomIdAndUserId } from '../../../app/lib/server/lib/notifyListener';
 
 export class RolesRaw extends BaseRaw<IRole> implements IRolesModel {
 	constructor(db: Db, trash?: Collection<RocketChatRecordDeleted<IRole>>) {
@@ -19,37 +18,6 @@ export class RolesRaw extends BaseRaw<IRole> implements IRolesModel {
 		return options ? this.find(query, options) : this.find(query);
 	}
 
-	async addUserRoles(userId: IUser['_id'], roles: IRole['_id'][], scope?: IRoom['_id']): Promise<boolean> {
-		if (process.env.NODE_ENV === 'development' && (scope === 'Users' || scope === 'Subscriptions')) {
-			throw new Error('Roles.addUserRoles method received a role scope instead of a scope value.');
-		}
-
-		if (!Array.isArray(roles)) {
-			roles = [roles];
-			process.env.NODE_ENV === 'development' && console.warn('[WARN] RolesRaw.addUserRoles: roles should be an array');
-		}
-
-		for await (const roleId of roles) {
-			const role = await this.findOneById<Pick<IRole, '_id' | 'scope'>>(roleId, { projection: { scope: 1 } });
-
-			if (!role) {
-				process.env.NODE_ENV === 'development' && console.warn(`[WARN] RolesRaw.addUserRoles: role: ${roleId} not found`);
-				continue;
-			}
-
-			if (role.scope === 'Subscriptions' && scope) {
-				// TODO remove dependency from other models - this logic should be inside a function/service
-				const addRolesResponse = await Subscriptions.addRolesByUserId(userId, [role._id], scope);
-				if (addRolesResponse.modifiedCount) {
-					void notifyOnSubscriptionChangedByRoomIdAndUserId(scope, userId);
-				}
-			} else {
-				await Users.addRolesByUserId(userId, [role._id]);
-			}
-		}
-		return true;
-	}
-
 	async isUserInRoles(userId: IUser['_id'], roles: IRole['_id'][], scope?: IRoom['_id']): Promise<boolean> {
 		if (process.env.NODE_ENV === 'development' && (scope === 'Users' || scope === 'Subscriptions')) {
 			throw new Error('Roles.isUserInRoles method received a role scope instead of a scope value.');
@@ -78,30 +46,6 @@ export class RolesRaw extends BaseRaw<IRole> implements IRolesModel {
 		return false;
 	}
 
-	async removeUserRoles(userId: IUser['_id'], roles: IRole['_id'][], scope?: IRoom['_id']): Promise<boolean> {
-		if (process.env.NODE_ENV === 'development' && (scope === 'Users' || scope === 'Subscriptions')) {
-			throw new Error('Roles.removeUserRoles method received a role scope instead of a scope value.');
-		}
-
-		for await (const roleId of roles) {
-			const role = await this.findOneById<Pick<IRole, '_id' | 'scope'>>(roleId, { projection: { scope: 1 } });
-
-			if (!role) {
-				continue;
-			}
-
-			if (role.scope === 'Subscriptions' && scope) {
-				const removeRolesResponse = await Subscriptions.removeRolesByUserId(userId, [roleId], scope);
-				if (removeRolesResponse.modifiedCount) {
-					void notifyOnSubscriptionChangedByRoomIdAndUserId(scope, userId);
-				}
-			} else {
-				await Users.removeRolesByUserId(userId, [roleId]);
-			}
-		}
-		return true;
-	}
-
 	async findOneByIdOrName(_idOrName: IRole['_id'] | IRole['name'], options?: undefined): Promise<IRole | null>;
 
 	async findOneByIdOrName(_idOrName: IRole['_id'] | IRole['name'], options: FindOptions<IRole>): Promise<IRole | null>;

packages/model-typings/src/models/IRolesModel.ts

@@ -5,9 +5,7 @@ import type { IBaseModel } from './IBaseModel';
 
 export interface IRolesModel extends IBaseModel<IRole> {
 	findByUpdatedDate(updatedAfterDate: Date, options?: FindOptions<IRole>): FindCursor<IRole>;
-	addUserRoles(userId: IUser['_id'], roles: IRole['_id'][], scope?: IRoom['_id']): Promise<boolean>;
 	isUserInRoles(userId: IUser['_id'], roles: IRole['_id'][], scope?: IRoom['_id']): Promise<boolean>;
-	removeUserRoles(userId: IUser['_id'], roles: IRole['_id'][], scope?: IRoom['_id']): Promise<boolean>;
 	findOneByIdOrName(_idOrName: IRole['_id'] | IRole['name'], options?: undefined): Promise<IRole | null>;
 
 	findOneByIdOrName(_idOrName: IRole['_id'] | IRole['name'], options: FindOptions<IRole>): Promise<IRole | null>;