Skip to content

Commit 19b6668

Browse files
sampaiodiegogabriellsh
authored andcommitted
refactor: Move add and remove role code out of Roles model (#34488)
1 parent 6d241da commit 19b6668

File tree

11 files changed

+78
-80
lines changed

11 files changed

+78
-80
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { isRoleAddUserToRoleProps, isRoleDeleteProps, isRoleRemoveUserFromRolePr
55
import { check, Match } from 'meteor/check';
66
import { Meteor } from 'meteor/meteor';
77

8+
import { removeUserFromRolesAsync } from '../../../../server/lib/roles/removeUserFromRoles';
89
import { getUsersInRolePaginated } from '../../../authorization/server/functions/getUsersInRole';
910
import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission';
1011
import { hasRoleAsync, hasAnyRoleAsync } from '../../../authorization/server/functions/hasRole';
@@ -221,7 +222,7 @@ API.v1.addRoute(
221222
}
222223
}
223224

224-
await Roles.removeUserRoles(user._id, [role._id], scope);
225+
await removeUserFromRolesAsync(user._id, [role._id], scope);
225226

226227
if (settings.get('UI_DisplayRoles')) {
227228
void api.broadcast('user.roleUpdate', {

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type { ServerMethods } from '@rocket.chat/ddp-client';
44
import { Roles, Users } from '@rocket.chat/models';
55
import { Meteor } from 'meteor/meteor';
66

7+
import { addUserRolesAsync } from '../../../../server/lib/roles/addUserRoles';
78
import { methodDeprecationLogger } from '../../../lib/server/lib/deprecationWarningLogger';
89
import { settings } from '../../../settings/server';
910
import { hasPermissionAsync } from '../functions/hasPermission';
@@ -75,7 +76,7 @@ Meteor.methods<ServerMethods>({
7576
});
7677
}
7778

78-
const add = await Roles.addUserRoles(user._id, [role._id], scope);
79+
const add = await addUserRolesAsync(user._id, [role._id], scope);
7980

8081
if (settings.get('UI_DisplayRoles')) {
8182
void api.broadcast('user.roleUpdate', {

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type { ServerMethods } from '@rocket.chat/ddp-client';
44
import { Roles, Users } from '@rocket.chat/models';
55
import { Meteor } from 'meteor/meteor';
66

7+
import { removeUserFromRolesAsync } from '../../../../server/lib/roles/removeUserFromRoles';
78
import { methodDeprecationLogger } from '../../../lib/server/lib/deprecationWarningLogger';
89
import { settings } from '../../../settings/server';
910
import { hasPermissionAsync } from '../functions/hasPermission';
@@ -79,7 +80,7 @@ Meteor.methods<ServerMethods>({
7980
}
8081
}
8182

82-
const remove = await Roles.removeUserRoles(user._id, [role._id], scope);
83+
const remove = await removeUserFromRolesAsync(user._id, [role._id], scope);
8384
const event = {
8485
type: 'removed',
8586
_id: role._id,

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
import type { INewIncomingIntegration, IIncomingIntegration } from '@rocket.chat/core-typings';
22
import type { ServerMethods } from '@rocket.chat/ddp-client';
3-
import { Integrations, Roles, Subscriptions, Users, Rooms } from '@rocket.chat/models';
3+
import { Integrations, Subscriptions, Users, Rooms } from '@rocket.chat/models';
44
import { Random } from '@rocket.chat/random';
55
import { Babel } from 'meteor/babel-compiler';
66
import { Match, check } from 'meteor/check';
77
import { Meteor } from 'meteor/meteor';
88
import _ from 'underscore';
99

10+
import { addUserRolesAsync } from '../../../../../server/lib/roles/addUserRoles';
1011
import { hasPermissionAsync, hasAllPermissionAsync } from '../../../../authorization/server/functions/hasPermission';
1112
import { notifyOnIntegrationChanged } from '../../../../lib/server/lib/notifyListener';
1213
import { validateScriptEngine, isScriptEngineFrozen } from '../../lib/validateScriptEngine';
@@ -154,7 +155,7 @@ export const addIncomingIntegration = async (userId: string, integration: INewIn
154155
}
155156
}
156157

157-
await Roles.addUserRoles(user._id, ['bot']);
158+
await addUserRolesAsync(user._id, ['bot']);
158159

159160
const { insertedId } = await Integrations.insertOne(integrationData);
160161

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import type { IIntegration, INewIncomingIntegration, IUpdateIncomingIntegration } from '@rocket.chat/core-typings';
22
import type { ServerMethods } from '@rocket.chat/ddp-client';
3-
import { Integrations, Roles, Subscriptions, Users, Rooms } from '@rocket.chat/models';
3+
import { Integrations, Subscriptions, Users, Rooms } from '@rocket.chat/models';
44
import { wrapExceptions } from '@rocket.chat/tools';
55
import { Babel } from 'meteor/babel-compiler';
66
import { Meteor } from 'meteor/meteor';
77
import _ from 'underscore';
88

9+
import { addUserRolesAsync } from '../../../../../server/lib/roles/addUserRoles';
910
import { hasAllPermissionAsync, hasPermissionAsync } from '../../../../authorization/server/functions/hasPermission';
1011
import { notifyOnIntegrationChanged } from '../../../../lib/server/lib/notifyListener';
1112
import { isScriptEngineFrozen, validateScriptEngine } from '../../lib/validateScriptEngine';
@@ -164,7 +165,7 @@ Meteor.methods<ServerMethods>({
164165
});
165166
}
166167

167-
await Roles.addUserRoles(user._id, ['bot']);
168+
await addUserRolesAsync(user._id, ['bot']);
168169

169170
const updatedIntegration = await Integrations.findOneAndUpdate(
170171
{ _id: integrationId },

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
import { Messages, Roles, Rooms, Subscriptions, ReadReceipts } from '@rocket.chat/models';
1+
import { Messages, Rooms, Subscriptions, ReadReceipts } from '@rocket.chat/models';
22

33
import type { SubscribedRoomsForUserWithDetails } from './getRoomsWithSingleOwner';
4+
import { addUserRolesAsync } from '../../../../server/lib/roles/addUserRoles';
45
import { FileUpload } from '../../../file-upload/server';
56
import { notifyOnSubscriptionChanged } from '../lib/notifyListener';
67

@@ -29,7 +30,7 @@ export const relinquishRoomOwnerships = async function (
2930
const changeOwner = subscribedRooms.filter(({ shouldChangeOwner }) => shouldChangeOwner);
3031

3132
for await (const { newOwner, rid } of changeOwner) {
32-
newOwner && (await Roles.addUserRoles(newOwner, ['owner'], rid));
33+
newOwner && (await addUserRolesAsync(newOwner, ['owner'], rid));
3334
}
3435

3536
const roomIdsToRemove: string[] = subscribedRooms.filter(({ shouldBeRemoved }) => shouldBeRemoved).map(({ rid }) => rid);
Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,45 @@
11
import { MeteorError } from '@rocket.chat/core-services';
22
import type { IRole, IUser, IRoom } from '@rocket.chat/core-typings';
3-
import { Roles } from '@rocket.chat/models';
3+
import { Roles, Subscriptions, Users } from '@rocket.chat/models';
44

55
import { validateRoleList } from './validateRoleList';
6+
import { notifyOnSubscriptionChangedByRoomIdAndUserId } from '../../../app/lib/server/lib/notifyListener';
67

7-
export const addUserRolesAsync = async (userId: IUser['_id'], roleIds: IRole['_id'][], scope?: IRoom['_id']): Promise<boolean> => {
8-
if (!userId || !roleIds?.length) {
8+
export const addUserRolesAsync = async (userId: IUser['_id'], roles: IRole['_id'][], scope?: IRoom['_id']): Promise<boolean> => {
9+
if (!userId || !roles?.length) {
910
return false;
1011
}
1112

12-
if (!(await validateRoleList(roleIds))) {
13+
if (!(await validateRoleList(roles))) {
1314
throw new MeteorError('error-invalid-role', 'Invalid role');
1415
}
1516

16-
await Roles.addUserRoles(userId, roleIds, scope);
17+
if (process.env.NODE_ENV === 'development' && (scope === 'Users' || scope === 'Subscriptions')) {
18+
throw new Error('Roles.addUserRoles method received a role scope instead of a scope value.');
19+
}
20+
21+
if (!Array.isArray(roles)) {
22+
roles = [roles];
23+
process.env.NODE_ENV === 'development' && console.warn('[WARN] RolesRaw.addUserRoles: roles should be an array');
24+
}
25+
26+
for await (const roleId of roles) {
27+
const role = await Roles.findOneById<Pick<IRole, '_id' | 'scope'>>(roleId, { projection: { scope: 1 } });
28+
29+
if (!role) {
30+
process.env.NODE_ENV === 'development' && console.warn(`[WARN] RolesRaw.addUserRoles: role: ${roleId} not found`);
31+
continue;
32+
}
33+
34+
if (role.scope === 'Subscriptions' && scope) {
35+
const addRolesResponse = await Subscriptions.addRolesByUserId(userId, [role._id], scope);
36+
if (addRolesResponse.modifiedCount) {
37+
void notifyOnSubscriptionChangedByRoomIdAndUserId(scope, userId);
38+
}
39+
} else {
40+
await Users.addRolesByUserId(userId, [role._id]);
41+
}
42+
}
43+
1744
return true;
1845
};
Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import { MeteorError } from '@rocket.chat/core-services';
22
import type { IRole, IUser, IRoom } from '@rocket.chat/core-typings';
3-
import { Users, Roles } from '@rocket.chat/models';
3+
import { Users, Subscriptions, Roles } from '@rocket.chat/models';
44

55
import { validateRoleList } from './validateRoleList';
6+
import { notifyOnSubscriptionChangedByRoomIdAndUserId } from '../../../app/lib/server/lib/notifyListener';
67

7-
export const removeUserFromRolesAsync = async (userId: IUser['_id'], roleIds: IRole['_id'][], scope?: IRoom['_id']): Promise<boolean> => {
8-
if (!userId || !roleIds) {
8+
export const removeUserFromRolesAsync = async (userId: IUser['_id'], roles: IRole['_id'][], scope?: IRoom['_id']): Promise<boolean> => {
9+
if (!userId || !roles) {
910
return false;
1011
}
1112

@@ -14,10 +15,29 @@ export const removeUserFromRolesAsync = async (userId: IUser['_id'], roleIds: IR
1415
throw new MeteorError('error-invalid-user', 'Invalid user');
1516
}
1617

17-
if (!(await validateRoleList(roleIds))) {
18+
if (!(await validateRoleList(roles))) {
1819
throw new MeteorError('error-invalid-role', 'Invalid role');
1920
}
2021

21-
await Roles.removeUserRoles(userId, roleIds, scope);
22+
if (process.env.NODE_ENV === 'development' && (scope === 'Users' || scope === 'Subscriptions')) {
23+
throw new Error('Roles.removeUserRoles method received a role scope instead of a scope value.');
24+
}
25+
26+
for await (const roleId of roles) {
27+
const role = await Roles.findOneById<Pick<IRole, '_id' | 'scope'>>(roleId, { projection: { scope: 1 } });
28+
if (!role) {
29+
continue;
30+
}
31+
32+
if (role.scope === 'Subscriptions' && scope) {
33+
const removeRolesResponse = await Subscriptions.removeRolesByUserId(userId, [roleId], scope);
34+
if (removeRolesResponse.modifiedCount) {
35+
void notifyOnSubscriptionChangedByRoomIdAndUserId(scope, userId);
36+
}
37+
} else {
38+
await Users.removeRolesByUserId(userId, [roleId]);
39+
}
40+
}
41+
2242
return true;
2343
};

apps/meteor/server/methods/afterVerifyEmail.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
import type { IRole } from '@rocket.chat/core-typings';
22
import type { ServerMethods } from '@rocket.chat/ddp-client';
3-
import { Roles, Users } from '@rocket.chat/models';
3+
import { Users } from '@rocket.chat/models';
44
import { Meteor } from 'meteor/meteor';
55

6+
import { addUserRolesAsync } from '../lib/roles/addUserRoles';
7+
import { removeUserFromRolesAsync } from '../lib/roles/removeUserFromRoles';
8+
69
const rolesToChangeTo: Map<IRole['_id'], [IRole['_id']]> = new Map([['anonymous', ['user']]]);
710

811
declare module '@rocket.chat/ddp-client' {
@@ -33,9 +36,9 @@ Meteor.methods<ServerMethods>({
3336
rolesThatNeedChanges.map(async (role) => {
3437
const rolesToAdd = rolesToChangeTo.get(role);
3538
if (rolesToAdd) {
36-
await Roles.addUserRoles(userId, rolesToAdd);
39+
await addUserRolesAsync(userId, rolesToAdd);
3740
}
38-
await Roles.removeUserRoles(user._id, [role]);
41+
await removeUserFromRolesAsync(user._id, [role]);
3942
}),
4043
);
4144
}

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

Lines changed: 0 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import { Subscriptions, Users } from '@rocket.chat/models';
44
import type { Collection, FindCursor, Db, Filter, FindOptions, Document, CountDocumentsOptions } from 'mongodb';
55

66
import { BaseRaw } from './BaseRaw';
7-
import { notifyOnSubscriptionChangedByRoomIdAndUserId } from '../../../app/lib/server/lib/notifyListener';
87

98
export class RolesRaw extends BaseRaw<IRole> implements IRolesModel {
109
constructor(db: Db, trash?: Collection<RocketChatRecordDeleted<IRole>>) {
@@ -19,37 +18,6 @@ export class RolesRaw extends BaseRaw<IRole> implements IRolesModel {
1918
return options ? this.find(query, options) : this.find(query);
2019
}
2120

22-
async addUserRoles(userId: IUser['_id'], roles: IRole['_id'][], scope?: IRoom['_id']): Promise<boolean> {
23-
if (process.env.NODE_ENV === 'development' && (scope === 'Users' || scope === 'Subscriptions')) {
24-
throw new Error('Roles.addUserRoles method received a role scope instead of a scope value.');
25-
}
26-
27-
if (!Array.isArray(roles)) {
28-
roles = [roles];
29-
process.env.NODE_ENV === 'development' && console.warn('[WARN] RolesRaw.addUserRoles: roles should be an array');
30-
}
31-
32-
for await (const roleId of roles) {
33-
const role = await this.findOneById<Pick<IRole, '_id' | 'scope'>>(roleId, { projection: { scope: 1 } });
34-
35-
if (!role) {
36-
process.env.NODE_ENV === 'development' && console.warn(`[WARN] RolesRaw.addUserRoles: role: ${roleId} not found`);
37-
continue;
38-
}
39-
40-
if (role.scope === 'Subscriptions' && scope) {
41-
// TODO remove dependency from other models - this logic should be inside a function/service
42-
const addRolesResponse = await Subscriptions.addRolesByUserId(userId, [role._id], scope);
43-
if (addRolesResponse.modifiedCount) {
44-
void notifyOnSubscriptionChangedByRoomIdAndUserId(scope, userId);
45-
}
46-
} else {
47-
await Users.addRolesByUserId(userId, [role._id]);
48-
}
49-
}
50-
return true;
51-
}
52-
5321
async isUserInRoles(userId: IUser['_id'], roles: IRole['_id'][], scope?: IRoom['_id']): Promise<boolean> {
5422
if (process.env.NODE_ENV === 'development' && (scope === 'Users' || scope === 'Subscriptions')) {
5523
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 {
7846
return false;
7947
}
8048

81-
async removeUserRoles(userId: IUser['_id'], roles: IRole['_id'][], scope?: IRoom['_id']): Promise<boolean> {
82-
if (process.env.NODE_ENV === 'development' && (scope === 'Users' || scope === 'Subscriptions')) {
83-
throw new Error('Roles.removeUserRoles method received a role scope instead of a scope value.');
84-
}
85-
86-
for await (const roleId of roles) {
87-
const role = await this.findOneById<Pick<IRole, '_id' | 'scope'>>(roleId, { projection: { scope: 1 } });
88-
89-
if (!role) {
90-
continue;
91-
}
92-
93-
if (role.scope === 'Subscriptions' && scope) {
94-
const removeRolesResponse = await Subscriptions.removeRolesByUserId(userId, [roleId], scope);
95-
if (removeRolesResponse.modifiedCount) {
96-
void notifyOnSubscriptionChangedByRoomIdAndUserId(scope, userId);
97-
}
98-
} else {
99-
await Users.removeRolesByUserId(userId, [roleId]);
100-
}
101-
}
102-
return true;
103-
}
104-
10549
async findOneByIdOrName(_idOrName: IRole['_id'] | IRole['name'], options?: undefined): Promise<IRole | null>;
10650

10751
async findOneByIdOrName(_idOrName: IRole['_id'] | IRole['name'], options: FindOptions<IRole>): Promise<IRole | null>;

0 commit comments

Comments
 (0)