From f21aceef455cd3ccc1951e58359aa94504f2dd4a Mon Sep 17 00:00:00 2001 From: Diego Sampaio Date: Wed, 16 Jun 2021 00:54:46 -0300 Subject: [PATCH] Chore: Move getUserRoles to service and add cache (#22345) --- app/lib/server/methods/getUserRoles.js | 26 ++------------ server/services/authorization/service.ts | 44 ++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 26 deletions(-) diff --git a/app/lib/server/methods/getUserRoles.js b/app/lib/server/methods/getUserRoles.js index b6b190a73b3..4f5595efa07 100644 --- a/app/lib/server/methods/getUserRoles.js +++ b/app/lib/server/methods/getUserRoles.js @@ -1,33 +1,13 @@ import { Meteor } from 'meteor/meteor'; -import _ from 'underscore'; -import { Roles, Users } from '../../../models'; +import { Authorization } from '../../../../server/sdk'; Meteor.methods({ - getUserRoles() { + async getUserRoles() { if (!Meteor.userId()) { throw new Meteor.Error('error-invalid-user', 'Invalid user', { method: 'getUserRoles' }); } - const options = { - sort: { - username: 1, - }, - fields: { - username: 1, - roles: 1, - }, - }; - - const roles = Roles.find({ scope: 'Users', description: { $exists: 1, $ne: '' } }).fetch(); - const roleIds = _.pluck(roles, '_id'); - - // Security issue: we should not send all user's roles to all clients, only the 'public' roles - // We must remove all roles that are not part of the query from the returned users - const users = Users.findUsersInRoles(roleIds, null, options).fetch(); - for (const user of users) { - user.roles = _.intersection(user.roles, roleIds); - } - return users; + return Authorization.getUsersFromPublicRoles(); }, }); diff --git a/server/services/authorization/service.ts b/server/services/authorization/service.ts index 0a8384db489..4ba342be071 100644 --- a/server/services/authorization/service.ts +++ b/server/services/authorization/service.ts @@ -11,6 +11,8 @@ import { SettingsRaw } from '../../../app/models/server/raw/Settings'; import { RoomsRaw } from '../../../app/models/server/raw/Rooms'; import { TeamMemberRaw } from '../../../app/models/server/raw/TeamMember'; import { TeamRaw } from '../../../app/models/server/raw/Team'; +import { RolesRaw } from '../../../app/models/server/raw/Roles'; +import { UsersRaw } from '../../../app/models/server/raw/Users'; import './canAccessRoomLivechat'; import './canAccessRoomTokenpass'; @@ -27,7 +29,9 @@ export class Authorization extends ServiceClass implements IAuthorization { private Permissions: Collection; - private Users: Collection; + private Users: UsersRaw; + + private Roles: RolesRaw; private getRolesCached = mem(this.getRoles.bind(this), { maxAge: 1000, cacheKey: JSON.stringify }) @@ -37,7 +41,9 @@ export class Authorization extends ServiceClass implements IAuthorization { super(); this.Permissions = db.collection('rocketchat_permissions'); - this.Users = db.collection('users'); + + this.Users = new UsersRaw(db.collection('users')); + this.Roles = new RolesRaw(db.collection('rocketchat_roles')); Subscriptions = new SubscriptionsRaw(db.collection('rocketchat_subscription')); Settings = new SettingsRaw(db.collection('rocketchat_settings')); @@ -83,6 +89,38 @@ export class Authorization extends ServiceClass implements IAuthorization { AuthorizationUtils.addRolePermissionWhiteList(role, permissions); } + async getUsersFromPublicRoles(): Promise[]> { + const roleIds = await this.getPublicRoles(); + + return this.getUserFromRoles(roleIds); + } + + private getPublicRoles = mem(async (): Promise => { + const roles = await this.Roles.find({ scope: 'Users', description: { $exists: 1, $ne: '' } }, { projection: { _id: 1 } }).toArray(); + + return roles.map(({ _id }) => _id); + }, { maxAge: 10000 }); + + private getUserFromRoles = mem(async (roleIds: string[]) => { + const options = { + sort: { + username: 1, + }, + fields: { + username: 1, + roles: 1, + }, + }; + + const users = await this.Users.findUsersInRoles(roleIds, null, options).toArray(); + + return users + .map((user) => ({ + ...user, + roles: user.roles.filter((roleId: string) => roleIds.includes(roleId)), + })); + }, { maxAge: 10000 }); + private async rolesHasPermission(permission: string, roles: string[]): Promise { if (AuthorizationUtils.isPermissionRestrictedForRoleList(permission, roles)) { return false; @@ -93,7 +131,7 @@ export class Authorization extends ServiceClass implements IAuthorization { } private async getRoles(uid: string, scope?: string): Promise { - const { roles: userRoles = [] } = await this.Users.findOne({ _id: uid }, { projection: { roles: 1 } }) || {}; + const { roles: userRoles = [] } = await this.Users.findOneById(uid, { projection: { roles: 1 } }) || {}; const { roles: subscriptionsRoles = [] } = (scope && await Subscriptions.findOne({ rid: scope, 'u._id': uid }, { projection: { roles: 1 } })) || {}; return [...userRoles, ...subscriptionsRoles].sort((a, b) => a.localeCompare(b)); }