Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Dec 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion apps/meteor/app/api/server/v1/roles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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 },
Expand Down
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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);
Expand Down
37 changes: 32 additions & 5 deletions apps/meteor/server/lib/roles/addUserRoles.ts
Original file line number Diff line number Diff line change
@@ -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;
};
30 changes: 25 additions & 5 deletions apps/meteor/server/lib/roles/removeUserFromRoles.ts
Original file line number Diff line number Diff line change
@@ -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;
}

Expand All @@ -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;
};
9 changes: 6 additions & 3 deletions apps/meteor/server/methods/afterVerifyEmail.ts
Original file line number Diff line number Diff line change
@@ -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' {
Expand Down Expand Up @@ -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]);
}),
);
}
Expand Down
56 changes: 0 additions & 56 deletions apps/meteor/server/models/raw/Roles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>>) {
Expand All @@ -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.');
Expand Down Expand Up @@ -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>;
Expand Down
2 changes: 0 additions & 2 deletions packages/model-typings/src/models/IRolesModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down
Loading