From c871ebe304b64a63f1274fa6ffccd2171ab013aa Mon Sep 17 00:00:00 2001 From: pierre-lehnen-rc <55164754+pierre-lehnen-rc@users.noreply.github.com> Date: Mon, 27 Apr 2020 16:03:35 -0300 Subject: [PATCH] Complement Guest role restrictions for Enterprise (#17393) Co-authored-by: Diego Sampaio --- app/authorization/client/hasPermission.js | 15 +++++ app/authorization/client/index.js | 2 +- .../client/lib/AuthorizationUtils.ts | 10 ---- .../client/views/permissions.html | 4 +- app/authorization/client/views/permissions.js | 7 +-- app/authorization/lib/AuthorizationUtils.ts | 56 +++++++++++++++++++ app/authorization/{lib.js => lib/index.js} | 2 + .../server/functions/hasPermission.js | 5 ++ app/authorization/server/index.js | 2 +- .../server/lib/AuthorizationUtils.ts | 10 ---- .../server/methods/addPermissionToRole.js | 7 +-- .../methods/removeRoleFromPermission.js | 8 --- app/models/client/models/Users.js | 8 ++- app/ui-flextab/client/tabs/userEdit.js | 21 ++++++- .../client/AuthorizationUtils.ts | 12 ---- ee/app/authorization/client/index.ts | 16 +++++- .../authorization/lib/addRoleRestrictions.js | 6 ++ ee/app/authorization/lib/guestPermissions.js | 6 ++ .../server/AuthorizationUtils.ts | 12 ---- ee/app/authorization/server/index.ts | 2 +- .../server/resetEnterprisePermissions.js | 6 ++ ee/app/license/client/index.ts | 13 ----- ee/app/license/server/license.ts | 9 +++ packages/rocketchat-i18n/i18n/en.i18n.json | 3 + server/publications/settings/index.js | 2 +- 25 files changed, 161 insertions(+), 83 deletions(-) delete mode 100644 app/authorization/client/lib/AuthorizationUtils.ts create mode 100644 app/authorization/lib/AuthorizationUtils.ts rename app/authorization/{lib.js => lib/index.js} (76%) delete mode 100644 app/authorization/server/lib/AuthorizationUtils.ts delete mode 100644 ee/app/authorization/client/AuthorizationUtils.ts create mode 100644 ee/app/authorization/lib/addRoleRestrictions.js create mode 100644 ee/app/authorization/lib/guestPermissions.js delete mode 100644 ee/app/authorization/server/AuthorizationUtils.ts create mode 100644 ee/app/authorization/server/resetEnterprisePermissions.js diff --git a/app/authorization/client/hasPermission.js b/app/authorization/client/hasPermission.js index 994c363428d..a48549979c7 100644 --- a/app/authorization/client/hasPermission.js +++ b/app/authorization/client/hasPermission.js @@ -3,11 +3,19 @@ import { Template } from 'meteor/templating'; import { ChatPermissions } from './lib/ChatPermissions'; import * as Models from '../../models'; +import { AuthorizationUtils } from '../lib/AuthorizationUtils'; function atLeastOne(permissions = [], scope, userId) { userId = userId || Meteor.userId(); + const user = Models.Users.findOneById(userId, { fields: { roles: 1 } }); return permissions.some((permissionId) => { + if (user && user.roles) { + if (AuthorizationUtils.isPermissionRestrictedForRoleList(permissionId, user.roles)) { + return false; + } + } + const permission = ChatPermissions.findOne(permissionId, { fields: { roles: 1 } }); const roles = (permission && permission.roles) || []; @@ -23,8 +31,15 @@ function atLeastOne(permissions = [], scope, userId) { function all(permissions = [], scope, userId) { userId = userId || Meteor.userId(); + const user = Models.Users.findOneById(userId, { fields: { roles: 1 } }); return permissions.every((permissionId) => { + if (user && user.roles) { + if (AuthorizationUtils.isPermissionRestrictedForRoleList(permissionId, user.roles)) { + return false; + } + } + const permission = ChatPermissions.findOne(permissionId, { fields: { roles: 1 } }); const roles = (permission && permission.roles) || []; diff --git a/app/authorization/client/index.js b/app/authorization/client/index.js index 0bb84757238..ca382bacc82 100644 --- a/app/authorization/client/index.js +++ b/app/authorization/client/index.js @@ -1,6 +1,6 @@ import { hasAllPermission, hasAtLeastOnePermission, hasPermission, userHasAllPermission } from './hasPermission'; import { hasRole } from './hasRole'; -import { AuthorizationUtils } from './lib/AuthorizationUtils'; +import { AuthorizationUtils } from '../lib/AuthorizationUtils'; import './usersNameChanged'; import './requiresPermission.html'; import './route'; diff --git a/app/authorization/client/lib/AuthorizationUtils.ts b/app/authorization/client/lib/AuthorizationUtils.ts deleted file mode 100644 index 2b75c6bad1a..00000000000 --- a/app/authorization/client/lib/AuthorizationUtils.ts +++ /dev/null @@ -1,10 +0,0 @@ -import { Meteor } from 'meteor/meteor'; - -export const AuthorizationUtils = class { - static isRoleReadOnly(roleId: string): boolean { - if (!roleId) { - throw new Meteor.Error('invalid-param'); - } - return false; - } -}; diff --git a/app/authorization/client/views/permissions.html b/app/authorization/client/views/permissions.html index 68f799e664e..d332e471849 100644 --- a/app/authorization/client/views/permissions.html +++ b/app/authorization/client/views/permissions.html @@ -27,7 +27,9 @@ {{permissionName permission}}
[ID: {{permission._id}}] {{#each role in allRoles}} - + {{#if isRolePermissionEnabled role permission}} + + {{/if}} {{else}} diff --git a/app/authorization/client/views/permissions.js b/app/authorization/client/views/permissions.js index de2332bca14..291eaac4bcd 100644 --- a/app/authorization/client/views/permissions.js +++ b/app/authorization/client/views/permissions.js @@ -10,8 +10,7 @@ import { ChatPermissions } from '../lib/ChatPermissions'; import { hasAllPermission } from '../hasPermission'; import { t } from '../../../utils/client'; import { SideNav } from '../../../ui-utils/client/lib/SideNav'; -import { CONSTANTS } from '../../lib'; -import { AuthorizationUtils } from '../lib/AuthorizationUtils'; +import { CONSTANTS, AuthorizationUtils } from '../../lib'; import { hasAtLeastOnePermission } from '..'; @@ -181,8 +180,8 @@ Template.permissionsTable.helpers({ return t(`${ permission._id }_description`); }, - disabled(role) { - return AuthorizationUtils.isRoleReadOnly(role._id); + isRolePermissionEnabled(role, permission) { + return !AuthorizationUtils.isPermissionRestrictedForRole(permission._id, role._id); }, }); diff --git a/app/authorization/lib/AuthorizationUtils.ts b/app/authorization/lib/AuthorizationUtils.ts new file mode 100644 index 00000000000..daa19828305 --- /dev/null +++ b/app/authorization/lib/AuthorizationUtils.ts @@ -0,0 +1,56 @@ +import { Meteor } from 'meteor/meteor'; + +const restrictedRolePermissions = new Map(); + +export const AuthorizationUtils = class { + static addRolePermissionWhiteList(roleId: string, list: [string]): void { + if (!roleId) { + throw new Meteor.Error('invalid-param'); + } + + if (!list) { + throw new Meteor.Error('invalid-param'); + } + + if (!restrictedRolePermissions.has(roleId)) { + restrictedRolePermissions.set(roleId, new Set()); + } + + const rules = restrictedRolePermissions.get(roleId); + + for (const permissionId of list) { + rules.add(permissionId); + } + } + + static isPermissionRestrictedForRole(permissionId: string, roleId: string): boolean { + if (!roleId || !permissionId) { + throw new Meteor.Error('invalid-param'); + } + + if (!restrictedRolePermissions.has(roleId)) { + return false; + } + + const rules = restrictedRolePermissions.get(roleId); + if (!rules || !rules.size) { + return false; + } + + return !rules.has(permissionId); + } + + static isPermissionRestrictedForRoleList(permissionId: string, roleList: [string]): boolean { + if (!roleList || !permissionId) { + throw new Meteor.Error('invalid-param'); + } + + for (const roleId of roleList) { + if (this.isPermissionRestrictedForRole(permissionId, roleId)) { + return true; + } + } + + return false; + } +}; diff --git a/app/authorization/lib.js b/app/authorization/lib/index.js similarity index 76% rename from app/authorization/lib.js rename to app/authorization/lib/index.js index e36c89a3a1c..fd4f5f0cb28 100644 --- a/app/authorization/lib.js +++ b/app/authorization/lib/index.js @@ -6,3 +6,5 @@ export const getSettingPermissionId = function(settingId) { export const CONSTANTS = { SETTINGS_LEVEL: 'settings', }; + +export { AuthorizationUtils } from './AuthorizationUtils'; diff --git a/app/authorization/server/functions/hasPermission.js b/app/authorization/server/functions/hasPermission.js index 6f5382496a2..0512c6a07e2 100644 --- a/app/authorization/server/functions/hasPermission.js +++ b/app/authorization/server/functions/hasPermission.js @@ -1,8 +1,13 @@ import mem from 'mem'; import { Permissions, Users, Subscriptions } from '../../../models/server/raw'; +import { AuthorizationUtils } from '../../lib/AuthorizationUtils'; const rolesHasPermission = mem(async (permission, roles) => { + if (AuthorizationUtils.isPermissionRestrictedForRoleList(permission, roles)) { + return false; + } + const result = await Permissions.findOne({ _id: permission, roles: { $in: roles } }, { projection: { _id: 1 } }); return !!result; }, { diff --git a/app/authorization/server/index.js b/app/authorization/server/index.js index b9383c427f0..0ebc74f0ce7 100644 --- a/app/authorization/server/index.js +++ b/app/authorization/server/index.js @@ -14,7 +14,7 @@ import { } from './functions/hasPermission'; import { hasRole } from './functions/hasRole'; import { removeUserFromRoles } from './functions/removeUserFromRoles'; -import { AuthorizationUtils } from './lib/AuthorizationUtils'; +import { AuthorizationUtils } from '../lib/AuthorizationUtils'; import './methods/addPermissionToRole'; import './methods/addUserToRole'; import './methods/deleteRole'; diff --git a/app/authorization/server/lib/AuthorizationUtils.ts b/app/authorization/server/lib/AuthorizationUtils.ts deleted file mode 100644 index 2b75c6bad1a..00000000000 --- a/app/authorization/server/lib/AuthorizationUtils.ts +++ /dev/null @@ -1,10 +0,0 @@ -import { Meteor } from 'meteor/meteor'; - -export const AuthorizationUtils = class { - static isRoleReadOnly(roleId: string): boolean { - if (!roleId) { - throw new Meteor.Error('invalid-param'); - } - return false; - } -}; diff --git a/app/authorization/server/methods/addPermissionToRole.js b/app/authorization/server/methods/addPermissionToRole.js index 5a95e5ddbfc..5ca74ed3dbc 100644 --- a/app/authorization/server/methods/addPermissionToRole.js +++ b/app/authorization/server/methods/addPermissionToRole.js @@ -2,13 +2,12 @@ import { Meteor } from 'meteor/meteor'; import { Permissions } from '../../../models/server'; import { hasPermission } from '../functions/hasPermission'; -import { AuthorizationUtils } from '../lib/AuthorizationUtils'; -import { CONSTANTS } from '../../lib'; +import { CONSTANTS, AuthorizationUtils } from '../../lib'; Meteor.methods({ 'authorization:addPermissionToRole'(permissionId, role) { - if (AuthorizationUtils.isRoleReadOnly(role)) { - throw new Meteor.Error('error-action-not-allowed', 'Role is readonly', { + if (AuthorizationUtils.isPermissionRestrictedForRole(permissionId, role)) { + throw new Meteor.Error('error-action-not-allowed', 'Permission is restricted', { method: 'authorization:addPermissionToRole', action: 'Adding_permission', }); diff --git a/app/authorization/server/methods/removeRoleFromPermission.js b/app/authorization/server/methods/removeRoleFromPermission.js index d0b7508f686..e0aa20ed34d 100644 --- a/app/authorization/server/methods/removeRoleFromPermission.js +++ b/app/authorization/server/methods/removeRoleFromPermission.js @@ -2,18 +2,10 @@ import { Meteor } from 'meteor/meteor'; import { Permissions } from '../../../models/server'; import { hasPermission } from '../functions/hasPermission'; -import { AuthorizationUtils } from '../lib/AuthorizationUtils'; import { CONSTANTS } from '../../lib'; Meteor.methods({ 'authorization:removeRoleFromPermission'(permissionId, role) { - if (AuthorizationUtils.isRoleReadOnly(role)) { - throw new Meteor.Error('error-action-not-allowed', 'Role is readonly', { - method: 'authorization:removeRoleFromPermission', - action: 'Removing_permission', - }); - } - const uid = Meteor.userId(); const permission = Permissions.findOneById(permissionId); diff --git a/app/models/client/models/Users.js b/app/models/client/models/Users.js index a2559c60dd7..51e71fedf9e 100644 --- a/app/models/client/models/Users.js +++ b/app/models/client/models/Users.js @@ -2,12 +2,16 @@ import { Meteor } from 'meteor/meteor'; import { Mongo } from 'meteor/mongo'; export const Users = { - isUserInRole(userId, roleName) { + findOneById(userId, options = {}) { const query = { _id: userId, }; - const user = this.findOne(query, { fields: { roles: 1 } }); + return this.findOne(query, options); + }, + + isUserInRole(userId, roleName) { + const user = this.findOneById(userId, { fields: { roles: 1 } }); return user && Array.isArray(user.roles) && user.roles.includes(roleName); }, diff --git a/app/ui-flextab/client/tabs/userEdit.js b/app/ui-flextab/client/tabs/userEdit.js index 6ccae14800f..ec37c0d816a 100644 --- a/app/ui-flextab/client/tabs/userEdit.js +++ b/app/ui-flextab/client/tabs/userEdit.js @@ -10,8 +10,9 @@ import { t, handleError } from '../../../utils'; import { Roles } from '../../../models'; import { Notifications } from '../../../notifications'; import { hasAtLeastOnePermission } from '../../../authorization'; -import { settings } from '../../../settings'; -import { callbacks } from '../../../callbacks'; +import { settings } from '../../../settings/client'; +import { callbacks } from '../../../callbacks/client'; +import { modal } from '../../../ui-utils/client'; Template.userEdit.helpers({ @@ -290,6 +291,22 @@ Template.userEdit.onCreated(function() { Meteor.call('insertOrUpdateUser', userData, (error) => { if (error) { + if (error.error === 'error-max-guests-number-reached') { + const message = TAPi18n.__('You_reached_the_maximum_number_of_guest_users_allowed_by_your_license.'); + const url = 'https://go.rocket.chat/i/guest-limit-exceeded'; + const email = 'sales@rocket.chat'; + const linkText = TAPi18n.__('Click_here_for_more_details_or_contact_sales_for_a_new_license', { url, email }); + + modal.open({ + type: 'error', + title: TAPi18n.__('Maximum_number_of_guests_reached'), + text: `${ message } ${ linkText }`, + html: true, + }); + + return true; + } + return handleError(error); } toastr.success(userData._id ? t('User_updated_successfully') : t('User_added_successfully')); diff --git a/ee/app/authorization/client/AuthorizationUtils.ts b/ee/app/authorization/client/AuthorizationUtils.ts deleted file mode 100644 index b12c3eb38c8..00000000000 --- a/ee/app/authorization/client/AuthorizationUtils.ts +++ /dev/null @@ -1,12 +0,0 @@ -import { AuthorizationUtils } from '../../../../app/authorization/client/lib/AuthorizationUtils'; -import { isEnterprise } from '../../license/client'; - -const { isRoleReadOnly: oldIsRoleReadOnly } = AuthorizationUtils; - -AuthorizationUtils.isRoleReadOnly = function(roleId: string): boolean { - if (isEnterprise() && roleId === 'guest') { - return true; - } - - return oldIsRoleReadOnly(roleId); -}; diff --git a/ee/app/authorization/client/index.ts b/ee/app/authorization/client/index.ts index fb2cb4f33c7..c4f3ed7a07c 100644 --- a/ee/app/authorization/client/index.ts +++ b/ee/app/authorization/client/index.ts @@ -1 +1,15 @@ -import './AuthorizationUtils'; +import { Meteor } from 'meteor/meteor'; + +import { addRoleRestrictions } from '../lib/addRoleRestrictions'; + +Meteor.startup(() => { + Meteor.call('license:isEnterprise', (err, result) => { + if (err) { + throw err; + } + + if (result) { + addRoleRestrictions(); + } + }); +}); diff --git a/ee/app/authorization/lib/addRoleRestrictions.js b/ee/app/authorization/lib/addRoleRestrictions.js new file mode 100644 index 00000000000..21416e9cb75 --- /dev/null +++ b/ee/app/authorization/lib/addRoleRestrictions.js @@ -0,0 +1,6 @@ +import { AuthorizationUtils } from '../../../../app/authorization/lib/AuthorizationUtils'; +import { guestPermissions } from './guestPermissions'; + +export const addRoleRestrictions = function() { + AuthorizationUtils.addRolePermissionWhiteList('guest', guestPermissions); +}; diff --git a/ee/app/authorization/lib/guestPermissions.js b/ee/app/authorization/lib/guestPermissions.js new file mode 100644 index 00000000000..43af8cd1ce2 --- /dev/null +++ b/ee/app/authorization/lib/guestPermissions.js @@ -0,0 +1,6 @@ +export const guestPermissions = [ + 'view-d-room', + 'view-joined-room', + 'view-p-room', + 'start-discussion', +]; diff --git a/ee/app/authorization/server/AuthorizationUtils.ts b/ee/app/authorization/server/AuthorizationUtils.ts deleted file mode 100644 index 6b6e423bea4..00000000000 --- a/ee/app/authorization/server/AuthorizationUtils.ts +++ /dev/null @@ -1,12 +0,0 @@ -import { AuthorizationUtils } from '../../../../app/authorization/server/lib/AuthorizationUtils'; -import { isEnterprise } from '../../license/server'; - -const { isRoleReadOnly: oldIsRoleReadOnly } = AuthorizationUtils; - -AuthorizationUtils.isRoleReadOnly = function(roleId: string): boolean { - if (isEnterprise() && roleId === 'guest') { - return true; - } - - return oldIsRoleReadOnly(roleId); -}; diff --git a/ee/app/authorization/server/index.ts b/ee/app/authorization/server/index.ts index fb2cb4f33c7..cd7f8dbb831 100644 --- a/ee/app/authorization/server/index.ts +++ b/ee/app/authorization/server/index.ts @@ -1 +1 @@ -import './AuthorizationUtils'; +import '../lib/addRoleRestrictions'; diff --git a/ee/app/authorization/server/resetEnterprisePermissions.js b/ee/app/authorization/server/resetEnterprisePermissions.js new file mode 100644 index 00000000000..cebc88ba83b --- /dev/null +++ b/ee/app/authorization/server/resetEnterprisePermissions.js @@ -0,0 +1,6 @@ +import { Permissions } from '../../../../app/models/server'; +import { guestPermissions } from '../lib/guestPermissions'; + +export const resetEnterprisePermissions = function() { + Permissions.update({ _id: { $nin: guestPermissions } }, { $pull: { roles: 'guest' } }, { multi: true }); +}; diff --git a/ee/app/license/client/index.ts b/ee/app/license/client/index.ts index 9949edfd073..c243ad6c93b 100644 --- a/ee/app/license/client/index.ts +++ b/ee/app/license/client/index.ts @@ -13,15 +13,6 @@ const allModules = new Promise>((resolve, reject) => { }); }); -let isEnterpriseServer = false; -CachedCollectionManager.onLogin(async () => { - try { - isEnterpriseServer = await callMethod('license:isEnterprise'); - } catch (e) { - console.error('Error checking if server is Enterprise', e); - } -}); - export async function hasLicense(feature: string): Promise { try { const features = await allModules; @@ -30,7 +21,3 @@ export async function hasLicense(feature: string): Promise { return false; } } - -export function isEnterprise(): boolean { - return isEnterpriseServer; -} diff --git a/ee/app/license/server/license.ts b/ee/app/license/server/license.ts index 2c31687d141..71ff33fbb70 100644 --- a/ee/app/license/server/license.ts +++ b/ee/app/license/server/license.ts @@ -1,6 +1,8 @@ import EventEmitter from 'events'; import { Users } from '../../../../app/models/server'; +import { resetEnterprisePermissions } from '../../authorization/server/resetEnterprisePermissions'; +import { addRoleRestrictions } from '../../authorization/lib/addRoleRestrictions'; import decrypt from './decrypt'; import { getBundleModules, isBundle } from './bundles'; @@ -21,6 +23,7 @@ export interface IValidLicense { } let maxGuestUsers = 0; +let addedRoleRestrictions = false; class LicenseClass { private url: string|null = null; @@ -76,6 +79,12 @@ class LicenseClass { }); this.validate(); + + if (!addedRoleRestrictions && this.hasAnyValidLicense()) { + addRoleRestrictions(); + resetEnterprisePermissions(); + addedRoleRestrictions = true; + } } hasModule(module: string): boolean { diff --git a/packages/rocketchat-i18n/i18n/en.i18n.json b/packages/rocketchat-i18n/i18n/en.i18n.json index 8b73227026b..24b939428ce 100644 --- a/packages/rocketchat-i18n/i18n/en.i18n.json +++ b/packages/rocketchat-i18n/i18n/en.i18n.json @@ -675,6 +675,7 @@ "Clear_filters": "Clear filters", "clear_history": "Clear History", "Click_here": "Click here", + "Click_here_for_more_details_or_contact_sales_for_a_new_license": "Click here for more details or contact __email__ for a new license.", "Click_here_for_more_info": "Click here for more info", "Click_here_to_enter_your_encryption_password": "Click here to enter your encryption password", "Click_here_to_view_and_copy_your_password": "Click here to view and copy your password.", @@ -2279,6 +2280,7 @@ "Max_number_incoming_livechats_displayed_description": "(Optional) Max number of items displayed in the incoming Omnichannel queue.", "Max_number_of_uses": "Max number of uses", "Maximum": "Maximum", + "Maximum_number_of_guests_reached": "Maximum number of guests reached", "Media": "Media", "Medium": "Medium", "Members": "Members", @@ -3738,6 +3740,7 @@ "You_need_to_type_in_your_username_in_order_to_do_this": "You need to type in your username in order to do this!", "You_need_to_verifiy_your_email_address_to_get_notications": "You need to verify your email address to get notifications", "You_need_to_write_something": "You need to write something!", + "You_reached_the_maximum_number_of_guest_users_allowed_by_your_license": "You reached the maximum number of guest users allowed by your license.", "You_should_inform_one_url_at_least": "You should define at least one URL.", "You_should_name_it_to_easily_manage_your_integrations": "You should name it to easily manage your integrations.", "You_will_not_be_able_to_recover": "You will not be able to recover this message!", diff --git a/server/publications/settings/index.js b/server/publications/settings/index.js index 005123a8b25..81651a50bcb 100644 --- a/server/publications/settings/index.js +++ b/server/publications/settings/index.js @@ -3,7 +3,7 @@ import { Meteor } from 'meteor/meteor'; import { Settings } from '../../../app/models/server'; import { Notifications } from '../../../app/notifications/server'; import { hasPermission, hasAtLeastOnePermission } from '../../../app/authorization/server'; -import { getSettingPermissionId } from '../../../app/authorization/lib.js'; +import { getSettingPermissionId } from '../../../app/authorization/lib'; Meteor.methods({ 'public-settings/get'(updatedAt) {