diff --git a/app/authorization/server/functions/addUserRoles.ts b/app/authorization/server/functions/addUserRoles.ts index b5b7b7913f13..30347e8b50c9 100644 --- a/app/authorization/server/functions/addUserRoles.ts +++ b/app/authorization/server/functions/addUserRoles.ts @@ -2,36 +2,32 @@ import { Meteor } from 'meteor/meteor'; import _ from 'underscore'; import { getRoles } from './getRoles'; -import { Users } from '../../../models/server'; import { IRole, IUser } from '../../../../definition/IUser'; -import { Roles } from '../../../models/server/raw'; +import { Users, Roles } from '../../../models/server/raw'; -export const addUserRoles = (userId: IUser['_id'], roleNames: IRole['name'][], scope?: string): boolean => { +export const addUserRolesAsync = async (userId: IUser['_id'], roleNames: IRole['name'][], scope?: string): Promise => { if (!userId || !roleNames) { return false; } - const user = Users.db.findOneById(userId); + const user = await Users.findOneById(userId); if (!user) { throw new Meteor.Error('error-invalid-user', 'Invalid user', { function: 'RocketChat.authz.addUserRoles', }); } - if (!Array.isArray(roleNames)) { - // TODO: remove this check - roleNames = [roleNames]; - } - const existingRoleNames = _.pluck(getRoles(), '_id'); const invalidRoleNames = _.difference(roleNames, existingRoleNames); if (!_.isEmpty(invalidRoleNames)) { - for (const role of invalidRoleNames) { - Promise.await(Roles.createOrUpdate(role)); + for await (const role of invalidRoleNames) { + await Roles.createOrUpdate(role); } } - Promise.await(Roles.addUserRoles(userId, roleNames, scope)); - return true; + return Roles.addUserRoles(userId, roleNames, scope); }; + +export const addUserRoles = (userId: IUser['_id'], roleNames: IRole['name'][], scope?: string): boolean => + Promise.await(addUserRolesAsync(userId, roleNames, scope)); diff --git a/app/authorization/server/functions/removeUserFromRoles.js b/app/authorization/server/functions/removeUserFromRoles.js deleted file mode 100644 index a55d722bb891..000000000000 --- a/app/authorization/server/functions/removeUserFromRoles.js +++ /dev/null @@ -1,34 +0,0 @@ -import { Meteor } from 'meteor/meteor'; -import _ from 'underscore'; - -import { getRoles } from './getRoles'; -import { Users } from '../../../models/server'; -import { Roles } from '../../../models/server/raw'; - -export const removeUserFromRoles = (userId, roleNames, scope) => { - if (!userId || !roleNames) { - return false; - } - - const user = Users.findOneById(userId); - - if (!user) { - throw new Meteor.Error('error-invalid-user', 'Invalid user', { - function: 'RocketChat.authz.removeUserFromRoles', - }); - } - - roleNames = [].concat(roleNames); - const existingRoleNames = _.pluck(getRoles(), '_id'); - const invalidRoleNames = _.difference(roleNames, existingRoleNames); - - if (!_.isEmpty(invalidRoleNames)) { - throw new Meteor.Error('error-invalid-role', 'Invalid role', { - function: 'RocketChat.authz.removeUserFromRoles', - }); - } - - Promise.await(Roles.removeUserRoles(userId, roleNames, scope)); - - return true; -}; diff --git a/app/authorization/server/functions/removeUserFromRoles.ts b/app/authorization/server/functions/removeUserFromRoles.ts new file mode 100644 index 000000000000..315377a24fa1 --- /dev/null +++ b/app/authorization/server/functions/removeUserFromRoles.ts @@ -0,0 +1,41 @@ +import { Meteor } from 'meteor/meteor'; +import _ from 'underscore'; + +import { getRoles } from './getRoles'; +import { IRole, IUser } from '../../../../definition/IUser'; +import { Users, Roles } from '../../../models/server/raw'; +import { ensureArray } from '../../../../lib/utils/arrayUtils'; + +export const removeUserFromRolesAsync = async ( + userId: IUser['_id'], + roleNames: IRole['name'] | Array, + scope?: string, +): Promise => { + if (!userId || !roleNames) { + return false; + } + + const user = await Users.findOneById(userId); + + if (!user) { + throw new Meteor.Error('error-invalid-user', 'Invalid user', { + function: 'RocketChat.authz.removeUserFromRoles', + }); + } + + roleNames = ensureArray(roleNames); + + const existingRoleNames = _.pluck(getRoles(), '_id'); + const invalidRoleNames = _.difference(roleNames, existingRoleNames); + + if (!_.isEmpty(invalidRoleNames)) { + throw new Meteor.Error('error-invalid-role', 'Invalid role', { + function: 'RocketChat.authz.removeUserFromRoles', + }); + } + + return Roles.removeUserRoles(userId, roleNames, scope); +}; + +export const removeUserFromRoles = (userId: IUser['_id'], roleNames: IRole['name'] | Array, scope?: string): boolean => + Promise.await(removeUserFromRolesAsync(userId, roleNames, scope)); diff --git a/app/authorization/server/index.js b/app/authorization/server/index.js index eec7cb23101c..31560dd77a10 100644 --- a/app/authorization/server/index.js +++ b/app/authorization/server/index.js @@ -1,11 +1,11 @@ -import { addUserRoles } from './functions/addUserRoles'; +import { addUserRolesAsync, addUserRoles } from './functions/addUserRoles'; import { canAccessRoom, canAccessRoomId, roomAccessAttributes, roomAccessValidators } from './functions/canAccessRoom'; import { canSendMessage, validateRoomMessagePermissions } from './functions/canSendMessage'; import { getRoles } from './functions/getRoles'; import { getUsersInRole } from './functions/getUsersInRole'; import { hasAllPermission, hasAtLeastOnePermission, hasPermission } from './functions/hasPermission'; import { hasRole, subscriptionHasRole } from './functions/hasRole'; -import { removeUserFromRoles } from './functions/removeUserFromRoles'; +import { removeUserFromRoles, removeUserFromRolesAsync } from './functions/removeUserFromRoles'; import { AuthorizationUtils } from '../lib/AuthorizationUtils'; import './methods/addPermissionToRole'; import './methods/addUserToRole'; @@ -21,9 +21,11 @@ export { hasRole, subscriptionHasRole, removeUserFromRoles, + removeUserFromRolesAsync, canSendMessage, validateRoomMessagePermissions, roomAccessValidators, + addUserRolesAsync, addUserRoles, canAccessRoom, canAccessRoomId, diff --git a/app/meteor-accounts-saml/server/lib/SAML.ts b/app/meteor-accounts-saml/server/lib/SAML.ts index 63697c96f9d4..3e5c40de0553 100644 --- a/app/meteor-accounts-saml/server/lib/SAML.ts +++ b/app/meteor-accounts-saml/server/lib/SAML.ts @@ -19,6 +19,7 @@ import { ISAMLAction } from '../definition/ISAMLAction'; import { ISAMLUser } from '../definition/ISAMLUser'; import { SAMLUtils } from './Utils'; import { SystemLogger } from '../../../../server/lib/logger/system'; +import { ensureArray } from '../../../../lib/utils/arrayUtils'; const showErrorMessage = function (res: ServerResponse, err: string): void { res.writeHead(200, { @@ -127,7 +128,7 @@ export class SAML { if (!user) { // If we received any role from the mapping, use them - otherwise use the default role for creation. - const roles = userObject.roles?.length ? userObject.roles : SAMLUtils.ensureArray(defaultUserRole.split(',')); + const roles = userObject.roles?.length ? userObject.roles : ensureArray(defaultUserRole.split(',')); const newUser: Record = { name: userObject.fullName, diff --git a/app/meteor-accounts-saml/server/lib/Utils.ts b/app/meteor-accounts-saml/server/lib/Utils.ts index 7813b9da07f0..a07f5c5de238 100644 --- a/app/meteor-accounts-saml/server/lib/Utils.ts +++ b/app/meteor-accounts-saml/server/lib/Utils.ts @@ -9,6 +9,7 @@ import { ISAMLGlobalSettings } from '../definition/ISAMLGlobalSettings'; import { IUserDataMap, IAttributeMapping } from '../definition/IAttributeMapping'; import { StatusCode } from './constants'; import { Logger } from '../../../../server/lib/logger/Logger'; +import { ensureArray } from '../../../../lib/utils/arrayUtils'; let providerList: Array = []; let debug = false; @@ -327,7 +328,7 @@ export class SAMLUtils { const values: Record = { regex: '', }; - const fieldNames = this.ensureArray(mapping.fieldName); + const fieldNames = ensureArray(mapping.fieldName); let mainValue; for (const fieldName of fieldNames) { @@ -406,11 +407,6 @@ export class SAMLUtils { return name; } - public static ensureArray(param: T | Array): Array { - const emptyArray: Array = []; - return emptyArray.concat(param); - } - public static mapProfileToUserObject(profile: Record): ISAMLUser { const userDataMap = this.getUserDataMapping(); SAMLUtils.log('parsed userDataMap', userDataMap); @@ -448,7 +444,7 @@ export class SAMLUtils { idpSession: profile.sessionIndex, nameID: profile.nameID, }, - emailList: this.ensureArray(email), + emailList: ensureArray(email), fullName: name || profile.displayName || profile.username, eppn: profile.eppn, attributeList, diff --git a/ee/server/configuration/saml.ts b/ee/server/configuration/saml.ts index 3713795db8c2..6502bd246b41 100644 --- a/ee/server/configuration/saml.ts +++ b/ee/server/configuration/saml.ts @@ -4,6 +4,7 @@ import { SAMLUtils } from '../../../app/meteor-accounts-saml/server/lib/Utils'; import { settings } from '../../../app/settings/server'; import { addSettings } from '../settings/saml'; import { Users } from '../../../app/models/server'; +import { ensureArray } from '../../../lib/utils/arrayUtils'; onLicense('saml-enterprise', () => { SAMLUtils.events.on('mapUser', ({ profile, userObject }: { profile: Record; userObject: ISAMLUser }) => { @@ -20,7 +21,7 @@ onLicense('saml-enterprise', () => { value = value.split(','); } - userObject.roles = SAMLUtils.ensureArray(value); + userObject.roles = ensureArray(value); } }); diff --git a/ee/server/lib/ldap/Manager.ts b/ee/server/lib/ldap/Manager.ts index e47513a094af..c9f6c9779799 100644 --- a/ee/server/lib/ldap/Manager.ts +++ b/ee/server/lib/ldap/Manager.ts @@ -15,8 +15,8 @@ import { LDAPConnection } from '../../../../server/lib/ldap/Connection'; import { LDAPManager } from '../../../../server/lib/ldap/Manager'; import { logger, searchLogger, mapLogger } from '../../../../server/lib/ldap/Logger'; import { templateVarHandler } from '../../../../app/utils/lib/templateVarHandler'; -import { api } from '../../../../server/sdk/api'; import { addUserToRoom, removeUserFromRoom, createRoom } from '../../../../app/lib/server/functions'; +import { syncUserRoles } from '../syncUserRoles'; import { Team } from '../../../../server/sdk'; export class LDAPEEManager extends LDAPManager { @@ -178,22 +178,6 @@ export class LDAPEEManager extends LDAPManager { } } - private static broadcastRoleChange(type: string, _id: string, uid: string, username: string): void { - // #ToDo: would be better to broadcast this only once for all users and roles, or at least once by user. - if (!settings.get('UI_DisplayRoles')) { - return; - } - - api.broadcast('user.roleUpdate', { - type, - _id, - u: { - _id: uid, - username, - }, - }); - } - private static async syncUserRoles(ldap: LDAPConnection, user: IUser, dn: string): Promise { const { username } = user; if (!username) { @@ -201,13 +185,13 @@ export class LDAPEEManager extends LDAPManager { return; } - const syncUserRoles = settings.get('LDAP_Sync_User_Data_Roles') ?? false; + const shouldSyncUserRoles = settings.get('LDAP_Sync_User_Data_Roles') ?? false; const syncUserRolesAutoRemove = settings.get('LDAP_Sync_User_Data_Roles_AutoRemove') ?? false; const syncUserRolesFieldMap = (settings.get('LDAP_Sync_User_Data_RolesMap') ?? '').trim(); const syncUserRolesFilter = (settings.get('LDAP_Sync_User_Data_Roles_Filter') ?? '').trim(); const syncUserRolesBaseDN = (settings.get('LDAP_Sync_User_Data_Roles_BaseDN') ?? '').trim(); - if (!syncUserRoles || !syncUserRolesFieldMap) { + if (!shouldSyncUserRoles || !syncUserRolesFieldMap) { logger.debug('not syncing user roles'); return; } @@ -232,37 +216,28 @@ export class LDAPEEManager extends LDAPManager { } const ldapFields = Object.keys(fieldMap); + const roleList: Array = []; + const allowedRoles: Array = []; + for await (const ldapField of ldapFields) { if (!fieldMap[ldapField]) { continue; } const userField = fieldMap[ldapField]; - const [roleName] = userField.split(/\.(.+)/); - if (!_.find(roles, (el) => el.name === roleName)) { - logger.debug(`User Role doesn't exist: ${roleName}`); - continue; - } - - logger.debug(`User role exists for mapping ${ldapField} -> ${roleName}`); + allowedRoles.push(roleName); if (await this.isUserInGroup(ldap, syncUserRolesBaseDN, syncUserRolesFilter, { dn, username }, ldapField)) { - if (await Roles.addUserRoles(user._id, roleName)) { - this.broadcastRoleChange('added', roleName, user._id, username); - } - logger.debug(`Synced user group ${roleName} from LDAP for ${user.username}`); - continue; - } - - if (!syncUserRolesAutoRemove) { + roleList.push(roleName); continue; } - - if (await Roles.removeUserRoles(user._id, roleName)) { - this.broadcastRoleChange('removed', roleName, user._id, username); - } } + + await syncUserRoles(user._id, roleList, { + allowedRoles, + skipRemovingRoles: !syncUserRolesAutoRemove, + }); } private static createRoomForSync(channel: string): IRoom | undefined { diff --git a/ee/server/lib/oauth/Manager.ts b/ee/server/lib/oauth/Manager.ts index d1ba124d41c9..0688505b8878 100644 --- a/ee/server/lib/oauth/Manager.ts +++ b/ee/server/lib/oauth/Manager.ts @@ -1,8 +1,8 @@ -import { addUserRoles, removeUserFromRoles } from '../../../../app/authorization/server'; import { Rooms } from '../../../../app/models/server'; import { addUserToRoom, createRoom } from '../../../../app/lib/server/functions'; import { Logger } from '../../../../app/logger/server'; import { Roles } from '../../../../app/models/server/raw'; +import { syncUserRoles } from '../syncUserRoles'; export const logger = new Logger('OAuth'); @@ -49,15 +49,11 @@ export class OAuthEEManager { user.roles = []; } - const toRemove = user.roles.filter((val: any) => !rolesFromSSO.includes(val) && rolesToSync.includes(val)); - - // remove all roles that the user has, but sso doesnt - removeUserFromRoles(user._id, toRemove); - - const toAdd = rolesFromSSO.filter((val: any) => !user.roles.includes(val) && (!rolesToSync.length || rolesToSync.includes(val))); - - // add all roles that sso has, but the user doesnt - addUserRoles(user._id, toAdd); + Promise.await( + syncUserRoles(user._id, rolesFromSSO, { + allowedRoles: rolesToSync, + }), + ); } } diff --git a/ee/server/lib/syncUserRoles.ts b/ee/server/lib/syncUserRoles.ts new file mode 100644 index 000000000000..63d531b62a08 --- /dev/null +++ b/ee/server/lib/syncUserRoles.ts @@ -0,0 +1,85 @@ +import type { IUser } from '../../../definition/IUser'; +import type { IRole } from '../../../definition/IRole'; +import { settings } from '../../../app/settings/server'; +import { api } from '../../../server/sdk/api'; +import { addUserRolesAsync, removeUserFromRolesAsync } from '../../../app/authorization/server'; +import { Users } from '../../../app/models/server/raw'; +import { canAddNewUser } from '../../app/license/server/license'; + +type setUserRolesOptions = { + // If specified, the function will not add nor remove any role that is not on this list. + allowedRoles?: Array; + // If set to true, roles will only be added, not removed + skipRemovingRoles?: boolean; + // the scope value (eg: room id) to assign the roles to + scope?: string; +}; + +function filterRoleList( + roleList: Array, + rolesToFilterOut: Array, + rolesToFilterIn?: Array, +): Array { + const filteredRoles = roleList.filter((roleName) => !rolesToFilterOut.includes(roleName)); + + if (!rolesToFilterIn) { + return filteredRoles; + } + + return filteredRoles.filter((roleName) => rolesToFilterIn.includes(roleName)); +} + +function broadcastRoleChange(type: string, roleList: Array, user: IUser): void { + if (!settings.get('UI_DisplayRoles')) { + return; + } + + const { _id, username } = user; + + for (const roleName of roleList) { + api.broadcast('user.roleUpdate', { + type, + _id: roleName, + u: { + _id, + username, + }, + }); + } +} + +export async function syncUserRoles( + uid: IUser['_id'], + newRoleList: Array, + { allowedRoles, skipRemovingRoles, scope }: setUserRolesOptions, +): Promise { + const user = await Users.findOneById(uid); + if (!user) { + throw new Error('error-user-not-found'); + } + + const existingRoles = user.roles; + const rolesToAdd = filterRoleList(newRoleList, existingRoles, allowedRoles); + const rolesToRemove = filterRoleList(existingRoles, newRoleList, allowedRoles); + + if (!rolesToAdd.length || !rolesToRemove.length) { + return; + } + + const wasGuest = existingRoles.length === 1 && existingRoles[0] === 'guest'; + if (wasGuest && !canAddNewUser()) { + throw new Error('error-license-user-limit-reached'); + } + + if (await addUserRolesAsync(uid, rolesToAdd, scope)) { + broadcastRoleChange('added', rolesToAdd, user); + } + + if (skipRemovingRoles) { + return; + } + + if (await removeUserFromRolesAsync(uid, rolesToRemove, scope)) { + broadcastRoleChange('removed', rolesToRemove, user); + } +} diff --git a/lib/utils/arrayUtils.tests.ts b/lib/utils/arrayUtils.tests.ts new file mode 100644 index 000000000000..e5f003dcfbfd --- /dev/null +++ b/lib/utils/arrayUtils.tests.ts @@ -0,0 +1,19 @@ +import { expect } from 'chai'; + +import { ensureArray } from './arrayUtils'; + +describe('Array utils', () => { + it('should return an array with one item', () => { + const value = 'value'; + const result = ensureArray(value); + + expect(result).to.be.an('Array').with.lengthOf(1).and.eql([value]); + }); + + it('should return a new array with the same items', () => { + const value = ['value1', 'value2']; + const result = ensureArray(value); + + expect(result).to.be.an('Array').with.lengthOf(2).and.eql(value).and.not.equal(value); + }); +}); diff --git a/lib/utils/arrayUtils.ts b/lib/utils/arrayUtils.ts new file mode 100644 index 000000000000..17fb541508fb --- /dev/null +++ b/lib/utils/arrayUtils.ts @@ -0,0 +1,4 @@ +export function ensureArray(param: T | Array): Array { + const emptyArray: Array = []; + return emptyArray.concat(param); +} diff --git a/server/methods/removeUserFromRoom.ts b/server/methods/removeUserFromRoom.ts index 8de153c964ac..8c722d3b686c 100644 --- a/server/methods/removeUserFromRoom.ts +++ b/server/methods/removeUserFromRoom.ts @@ -1,7 +1,7 @@ import { Meteor } from 'meteor/meteor'; import { Match, check } from 'meteor/check'; -import { hasPermission, hasRole, getUsersInRole, removeUserFromRoles } from '../../app/authorization/server'; +import { hasPermission, hasRole, getUsersInRole, removeUserFromRolesAsync } from '../../app/authorization/server'; import { Users, Subscriptions, Rooms, Messages } from '../../app/models/server'; import { callbacks } from '../../lib/callbacks'; import { RoomMemberActions } from '../../definition/IRoomTypeConfig'; @@ -68,7 +68,7 @@ Meteor.methods({ Subscriptions.removeByRoomIdAndUserId(data.rid, removedUser._id); if (['c', 'p'].includes(room.t) === true) { - removeUserFromRoles(removedUser._id, ['moderator', 'owner'], data.rid); + await removeUserFromRolesAsync(removedUser._id, ['moderator', 'owner'], data.rid); } Messages.createUserRemovedWithRoomIdAndUser(data.rid, removedUser, { diff --git a/server/startup/initialData.js b/server/startup/initialData.js index d24c0019a718..2e0bb20024da 100644 --- a/server/startup/initialData.js +++ b/server/startup/initialData.js @@ -3,7 +3,7 @@ import { Accounts } from 'meteor/accounts-base'; import { RocketChatFile } from '../../app/file'; import { FileUpload } from '../../app/file-upload/server'; -import { addUserRoles, getUsersInRole } from '../../app/authorization/server'; +import { addUserRolesAsync, getUsersInRole } from '../../app/authorization/server'; import { Users, Rooms } from '../../app/models/server'; import { settings } from '../../app/settings/server'; import { checkUsernameAvailability, addUserToDefaultChannels } from '../../app/lib/server'; @@ -29,7 +29,7 @@ Meteor.startup(async function () { type: 'bot', }); - addUserRoles('rocket.cat', 'bot'); + await addUserRolesAsync('rocket.cat', 'bot'); const buffer = Buffer.from(Assets.getBinary('avatars/rocketcat.png')); @@ -113,7 +113,7 @@ Meteor.startup(async function () { Accounts.setPassword(id, process.env.ADMIN_PASS); - addUserRoles(id, 'admin'); + await addUserRolesAsync(id, 'admin'); } else { console.log('Users with admin role already exist; Ignoring environment variables ADMIN_PASS'.red); } @@ -139,7 +139,7 @@ Meteor.startup(async function () { const oldestUser = Users.getOldest({ _id: 1, username: 1, name: 1 }); if (oldestUser) { - addUserRoles(oldestUser._id, ['admin']); + await addUserRolesAsync(oldestUser._id, ['admin']); console.log(`No admins are found. Set ${oldestUser.username || oldestUser.name} as admin for being the oldest user`); } } @@ -190,7 +190,7 @@ Meteor.startup(async function () { Accounts.setPassword(adminUser._id, adminUser._id); - addUserRoles(adminUser._id, ['admin']); + await addUserRolesAsync(adminUser._id, ['admin']); if (settings.get('Show_Setup_Wizard') === 'pending') { Settings.updateValueById('Show_Setup_Wizard', 'in_progress');