From 03f3db2588fc914569f8e2c2200032d6e17694eb Mon Sep 17 00:00:00 2001
From: Tim Kinnane <tim@nestedcode.com>
Date: Thu, 11 Aug 2016 03:27:57 +1000
Subject: [PATCH] Prevent last admin removal (#3971)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* More admin checks
# Conflicts:
# packages/rocketchat-lib/server/methods/insertOrUpdateUser.coffee
* missing server side validation
# Conflicts:
# packages/rocketchat-ui-flextab/flex-tab/tabs/userEdit.coffee
* Remove last admin check for removeUserFromRoom
* Move and fix last admin check for role removal
Check was on change admin status method, but that didn’t catch direct removal of user from role admin view.
* Fix last admin check for insert/update/delete user
Was checking if only one admin but not if the updated user was admin, preventing all updates.
* Allow translation of last admin error
* Fix assigning admin permission bypass
---
.../server/methods/addUserToRole.coffee | 3 +++
.../server/methods/removeUserFromRole.coffee | 9 ++++++++-
packages/rocketchat-lib/i18n/en.i18n.json | 2 ++
.../server/methods/insertOrUpdateUser.coffee | 10 +++++++++-
server/methods/deleteUser.coffee | 6 ++++++
5 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/packages/rocketchat-authorization/server/methods/addUserToRole.coffee b/packages/rocketchat-authorization/server/methods/addUserToRole.coffee
index bbc03adfd21..771789746a4 100644
--- a/packages/rocketchat-authorization/server/methods/addUserToRole.coffee
+++ b/packages/rocketchat-authorization/server/methods/addUserToRole.coffee
@@ -6,6 +6,9 @@ Meteor.methods
if not roleName or not _.isString(roleName) or not username or not _.isString(username)
throw new Meteor.Error 'error-invalid-arguments', 'Invalid arguments', { method: 'authorization:addUserToRole' }
+ if roleName is 'admin' and not RocketChat.authz.hasPermission Meteor.userId(), 'assign-admin-role'
+ throw new Meteor.Error 'error-action-not-allowed', 'Assigning admin is not allowed', { method: 'insertOrUpdateUser', action: 'Assign_admin' }
+
user = RocketChat.models.Users.findOneByUsername username, { fields: { _id: 1 } }
if not user?._id?
diff --git a/packages/rocketchat-authorization/server/methods/removeUserFromRole.coffee b/packages/rocketchat-authorization/server/methods/removeUserFromRole.coffee
index 4fe7f6f52fa..841754fab84 100644
--- a/packages/rocketchat-authorization/server/methods/removeUserFromRole.coffee
+++ b/packages/rocketchat-authorization/server/methods/removeUserFromRole.coffee
@@ -6,11 +6,18 @@ Meteor.methods
if not roleName or not _.isString(roleName) or not username or not _.isString(username)
throw new Meteor.Error 'error-invalid-arguments', 'Invalid arguments', { method: 'authorization:removeUserFromRole' }
- user = Meteor.users.findOne { username: username }, { fields: { _id: 1 } }
+ user = Meteor.users.findOne { username: username }, { fields: { _id: 1, roles: 1 } }
if not user?._id?
throw new Meteor.Error 'error-invalid-user', 'Invalid user', { method: 'authorization:removeUserFromRole' }
+ # prevent removing last user from admin role
+ if roleName is 'admin'
+ adminCount = Meteor.users.find({ roles: { $in: ['admin'] } }).count()
+ userIsAdmin = user.roles.indexOf('admin') > -1
+ if adminCount is 1 and userIsAdmin
+ throw new Meteor.Error 'error-action-not-allowed', 'Leaving the app without admins is not allowed', { method: 'removeUserFromRole', action: 'Remove_last_admin' }
+
remove = RocketChat.models.Roles.removeUserRoles user._id, roleName, scope
if RocketChat.settings.get('UI_DisplayRoles')
diff --git a/packages/rocketchat-lib/i18n/en.i18n.json b/packages/rocketchat-lib/i18n/en.i18n.json
index 56b8f701828..312156f8f12 100644
--- a/packages/rocketchat-lib/i18n/en.i18n.json
+++ b/packages/rocketchat-lib/i18n/en.i18n.json
@@ -168,6 +168,7 @@
"are_typing" : "are typing",
"Are_you_sure" : "Are you sure?",
"Are_you_sure_you_want_to_delete_your_account" : "Are you sure you want to delete your account?",
+ "Assign_admin" : "Assigning admin",
"at" : "at",
"Auth_Token" : "Auth Token",
"Author" : "Author",
@@ -918,6 +919,7 @@
"Remove_as_owner" : "Remove as owner",
"Remove_custom_oauth" : "Remove custom oauth",
"Remove_from_room" : "Remove from room",
+ "Remove_last_admin" : "Removing last admin",
"Remove_someone_from_room" : "Remove someone from the room",
"Removed" : "Removed",
"Report_Abuse" : "Report Abuse",
diff --git a/packages/rocketchat-lib/server/methods/insertOrUpdateUser.coffee b/packages/rocketchat-lib/server/methods/insertOrUpdateUser.coffee
index 4895fc2f658..201c6add92a 100644
--- a/packages/rocketchat-lib/server/methods/insertOrUpdateUser.coffee
+++ b/packages/rocketchat-lib/server/methods/insertOrUpdateUser.coffee
@@ -14,6 +14,9 @@ Meteor.methods
if not userData._id and canAddUser isnt true
throw new Meteor.Error 'error-action-not-allowed', 'Adding user is not allowd', { method: 'insertOrUpdateUser', action: 'Adding_user' }
+ if userData.role is 'admin' and not RocketChat.authz.hasPermission Meteor.userId(), 'assign-admin-role'
+ throw new Meteor.Error 'error-action-not-allowed', 'Assigning admin is not allowed', { method: 'insertOrUpdateUser', action: 'Assign_admin' }
+
unless s.trim(userData.name)
throw new Meteor.Error 'error-the-field-is-required', 'The field Name is required', { method: 'insertOrUpdateUser', field: 'Name' }
@@ -69,7 +72,6 @@ Meteor.methods
header = RocketChat.placeholders.replace(RocketChat.settings.get('Email_Header') || "")
footer = RocketChat.placeholders.replace(RocketChat.settings.get('Email_Footer') || "")
-
if RocketChat.settings.get('Accounts_UserAddedEmail_Customized')
subject = RocketChat.settings.get('Accounts_UserAddedEmailSubject')
html = RocketChat.settings.get('Accounts_UserAddedEmail')
@@ -95,10 +97,16 @@ Meteor.methods
return _id
else
+ # prevent removing admin role of last admin
+ adminCount = Meteor.users.find({ roles: { $in: ['admin'] } }).count()
+ if adminCount is 1 and userData.role isnt 'admin'
+ throw new Meteor.Error 'error-action-not-allowed', 'Leaving the app without admins is not allowed', { method: 'insertOrUpdateUser', action: 'Remove_last_admin' }
+
#update user
updateUser = {
$set: {
name: userData.name,
+ roles: [ userData.role ],
requirePasswordChange: userData.requirePasswordChange
}
}
diff --git a/server/methods/deleteUser.coffee b/server/methods/deleteUser.coffee
index ec5c304df49..7344baecf15 100644
--- a/server/methods/deleteUser.coffee
+++ b/server/methods/deleteUser.coffee
@@ -12,6 +12,12 @@ Meteor.methods
unless user?
throw new Meteor.Error 'error-invalid-user', "Invalid user", { method: 'deleteUser' }
+ # prevent deleting last admin
+ adminCount = Meteor.users.find({ roles: { $in: ['admin'] } }).count()
+ userIsAdmin = user.roles.indexOf('admin') > -1
+ if adminCount is 1 and userIsAdmin
+ throw new Meteor.Error 'error-action-not-allowed', 'Leaving the app without admins is not allowed', { method: 'deleteUser', action: 'Remove_last_admin' }
+
RocketChat.deleteUser(userId)
return true
--
GitLab