From d37433b9589efec51603a1ee79f067bceb459759 Mon Sep 17 00:00:00 2001 From: Diego Sampaio Date: Mon, 23 Dec 2024 17:04:12 -0300 Subject: [PATCH] refactor: Move add and remove role code out of Roles model (#34488) --- apps/meteor/app/api/server/v1/roles.ts | 3 +- .../server/methods/addUserToRole.ts | 3 +- .../server/methods/removeUserFromRole.ts | 3 +- .../incoming/addIncomingIntegration.ts | 5 +- .../incoming/updateIncomingIntegration.ts | 5 +- .../functions/relinquishRoomOwnerships.ts | 5 +- apps/meteor/server/lib/roles/addUserRoles.ts | 37 ++++++++++-- .../server/lib/roles/removeUserFromRoles.ts | 30 ++++++++-- .../meteor/server/methods/afterVerifyEmail.ts | 9 ++- apps/meteor/server/models/raw/Roles.ts | 56 ------------------- .../model-typings/src/models/IRolesModel.ts | 2 - 11 files changed, 78 insertions(+), 80 deletions(-) diff --git a/apps/meteor/app/api/server/v1/roles.ts b/apps/meteor/app/api/server/v1/roles.ts index 20f6e38f3567..b52e20e12920 100644 --- a/apps/meteor/app/api/server/v1/roles.ts +++ b/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', { diff --git a/apps/meteor/app/authorization/server/methods/addUserToRole.ts b/apps/meteor/app/authorization/server/methods/addUserToRole.ts index 81582dd7e9fb..6f26c4a61461 100644 --- a/apps/meteor/app/authorization/server/methods/addUserToRole.ts +++ b/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({ }); } - 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', { diff --git a/apps/meteor/app/authorization/server/methods/removeUserFromRole.ts b/apps/meteor/app/authorization/server/methods/removeUserFromRole.ts index 56f0c2e307ab..77f8c54e2f49 100644 --- a/apps/meteor/app/authorization/server/methods/removeUserFromRole.ts +++ b/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({ } } - 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, diff --git a/apps/meteor/app/integrations/server/methods/incoming/addIncomingIntegration.ts b/apps/meteor/app/integrations/server/methods/incoming/addIncomingIntegration.ts index 8f1f1b5dd576..eaa3b98fd629 100644 --- a/apps/meteor/app/integrations/server/methods/incoming/addIncomingIntegration.ts +++ b/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); diff --git a/apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts b/apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts index b4ef5b9bf973..cdf0d4465309 100644 --- a/apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts +++ b/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({ }); } - await Roles.addUserRoles(user._id, ['bot']); + await addUserRolesAsync(user._id, ['bot']); const updatedIntegration = await Integrations.findOneAndUpdate( { _id: integrationId }, diff --git a/apps/meteor/app/lib/server/functions/relinquishRoomOwnerships.ts b/apps/meteor/app/lib/server/functions/relinquishRoomOwnerships.ts index 2d53b9b55c7c..a62f893319a9 100644 --- a/apps/meteor/app/lib/server/functions/relinquishRoomOwnerships.ts +++ b/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); diff --git a/apps/meteor/server/lib/roles/addUserRoles.ts b/apps/meteor/server/lib/roles/addUserRoles.ts index a064553f5cb4..1333f0af6237 100644 --- a/apps/meteor/server/lib/roles/addUserRoles.ts +++ b/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 => { - if (!userId || !roleIds?.length) { +export const addUserRolesAsync = async (userId: IUser['_id'], roles: IRole['_id'][], scope?: IRoom['_id']): Promise => { + 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>(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; }; diff --git a/apps/meteor/server/lib/roles/removeUserFromRoles.ts b/apps/meteor/server/lib/roles/removeUserFromRoles.ts index 2145fd9aee52..de12ec91e66a 100644 --- a/apps/meteor/server/lib/roles/removeUserFromRoles.ts +++ b/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 => { - if (!userId || !roleIds) { +export const removeUserFromRolesAsync = async (userId: IUser['_id'], roles: IRole['_id'][], scope?: IRoom['_id']): Promise => { + 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>(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; }; diff --git a/apps/meteor/server/methods/afterVerifyEmail.ts b/apps/meteor/server/methods/afterVerifyEmail.ts index 06bf389b0a03..9e1589c1895a 100644 --- a/apps/meteor/server/methods/afterVerifyEmail.ts +++ b/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 = new Map([['anonymous', ['user']]]); declare module '@rocket.chat/ddp-client' { @@ -33,9 +36,9 @@ Meteor.methods({ 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]); }), ); } diff --git a/apps/meteor/server/models/raw/Roles.ts b/apps/meteor/server/models/raw/Roles.ts index 36fc7674416f..fad0312bc52f 100644 --- a/apps/meteor/server/models/raw/Roles.ts +++ b/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 implements IRolesModel { constructor(db: Db, trash?: Collection>) { @@ -19,37 +18,6 @@ export class RolesRaw extends BaseRaw implements IRolesModel { return options ? this.find(query, options) : this.find(query); } - async addUserRoles(userId: IUser['_id'], roles: IRole['_id'][], scope?: IRoom['_id']): Promise { - 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>(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 { 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 implements IRolesModel { return false; } - async removeUserRoles(userId: IUser['_id'], roles: IRole['_id'][], scope?: IRoom['_id']): Promise { - 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>(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; async findOneByIdOrName(_idOrName: IRole['_id'] | IRole['name'], options: FindOptions): Promise; diff --git a/packages/model-typings/src/models/IRolesModel.ts b/packages/model-typings/src/models/IRolesModel.ts index 84d49ef6baee..dabff59952a3 100644 --- a/packages/model-typings/src/models/IRolesModel.ts +++ b/packages/model-typings/src/models/IRolesModel.ts @@ -5,9 +5,7 @@ import type { IBaseModel } from './IBaseModel'; export interface IRolesModel extends IBaseModel { findByUpdatedDate(updatedAfterDate: Date, options?: FindOptions): FindCursor; - addUserRoles(userId: IUser['_id'], roles: IRole['_id'][], scope?: IRoom['_id']): Promise; isUserInRoles(userId: IUser['_id'], roles: IRole['_id'][], scope?: IRoom['_id']): Promise; - removeUserRoles(userId: IUser['_id'], roles: IRole['_id'][], scope?: IRoom['_id']): Promise; findOneByIdOrName(_idOrName: IRole['_id'] | IRole['name'], options?: undefined): Promise; findOneByIdOrName(_idOrName: IRole['_id'] | IRole['name'], options: FindOptions): Promise;