Mirrors
/
nextcloud-server
mirror of https://github.com/nextcloud/server
3
0
Fork 0
Code Releases Activity
Browse Source

Merge pull request #3141 from nextcloud/subadmin-check-on-removing-user-from-group 

Subadmin check on removing user from group
pull/3209/head
Joas Schilling 9 years ago
committed by GitHub
parent
16afaa783b 916cc57b0e
commit
5d486478d3
6 changed files with 288 additions and 147 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 48
      apps/provisioning_api/lib/Controller/UsersController.php
  2. 214
      apps/provisioning_api/tests/Controller/UsersControllerTest.php
  3. 2
      lib/private/SubAdmin.php
  4. 92
      settings/ajax/togglegroups.php
  5. 4
      settings/js/users/groups.js
  6. 75
      settings/js/users/users.js

48
apps/provisioning_api/lib/Controller/UsersController.php
View File

@ -33,10 +33,10 @@ use \OC_Helper;
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\OCS\OCSException;
use OCP\AppFramework\OCS\OCSForbiddenException;
use OCP\AppFramework\OCS\OCSNotFoundException;
use OCP\AppFramework\OCSController;
use OCP\Files\NotFoundException;
use OCP\IConfig;
use OCP\IGroup;
use OCP\IGroupManager;
use OCP\ILogger;
use OCP\IRequest;
@ -275,9 +275,9 @@ class UsersController extends OCSController {
break;
case 'quota':
$quota = $value;
if($quota !== 'none' and $quota !== 'default') {
if($quota !== 'none' && $quota !== 'default') {
if (is_numeric($quota)) {
$quota = floatval($quota);
$quota = (float) $quota;
} else {
$quota = \OCP\Util::computerFileSize($quota);
}
@ -421,6 +421,7 @@ class UsersController extends OCSController {
// Looking up someone else
if($subAdminManager->isUserAccessible($loggedInUser, $targetUser)) {
// Return the group that the method caller is subadmin of for the user in question
/** @var IGroup[] $getSubAdminsGroups */
$getSubAdminsGroups = $subAdminManager->getSubAdminsGroups($loggedInUser);
foreach ($getSubAdminsGroups as $key => $group) {
$getSubAdminsGroups[$key] = $group->getGID();
@ -440,6 +441,8 @@ class UsersController extends OCSController {
/**
* @PasswordConfirmationRequired
* @NoAdminRequired
*
* @param string $userId
* @param string $groupid
* @return DataResponse
@ -459,6 +462,13 @@ class UsersController extends OCSController {
throw new OCSException('', 103);
}
// If they're not an admin, check they are a subadmin of the group in question
$loggedInUser = $this->userSession->getUser();
$subAdminManager = $this->groupManager->getSubAdmin();
if (!$this->groupManager->isAdmin($loggedInUser->getUID()) && !$subAdminManager->isSubAdminOfGroup($loggedInUser, $group)) {
throw new OCSException('', 104);
}
// Add user to group
$group->addUser($targetUser);
return new DataResponse();
@ -492,25 +502,33 @@ class UsersController extends OCSController {
// If they're not an admin, check they are a subadmin of the group in question
$subAdminManager = $this->groupManager->getSubAdmin();
if(!$this->groupManager->isAdmin($loggedInUser->getUID()) && !$subAdminManager->isSubAdminofGroup($loggedInUser, $group)) {
if (!$this->groupManager->isAdmin($loggedInUser->getUID()) && !$subAdminManager->isSubAdminOfGroup($loggedInUser, $group)) {
throw new OCSException('', 104);
}
// Check they aren't removing themselves from 'admin' or their 'subadmin; group
if($userId === $loggedInUser->getUID()) {
if($this->groupManager->isAdmin($loggedInUser->getUID())) {
if($group->getGID() === 'admin') {
if ($userId === $loggedInUser->getUID()) {
if ($this->groupManager->isAdmin($loggedInUser->getUID())) {
if ($group->getGID() === 'admin') {
throw new OCSException('Cannot remove yourself from the admin group', 105);
}
} else {
// Not an admin, check they are not removing themself from their subadmin group
$subAdminGroups = $subAdminManager->getSubAdminsGroups($loggedInUser);
foreach ($subAdminGroups as $key => $group) {
$subAdminGroups[$key] = $group->getGID();
}
// Not an admin, so the user must be a subadmin of this group, but that is not allowed.
throw new OCSException('Cannot remove yourself from this group as you are a SubAdmin', 105);
}
if(in_array($group->getGID(), $subAdminGroups, true)) {
throw new OCSException('Cannot remove yourself from this group as you are a SubAdmin', 105);
}
} else if (!$this->groupManager->isAdmin($loggedInUser->getUID())) {
/** @var IGroup[] $subAdminGroups */
$subAdminGroups = $subAdminManager->getSubAdminsGroups($loggedInUser);
$subAdminGroups = array_map(function (IGroup $subAdminGroup) {
return $subAdminGroup->getGID();
}, $subAdminGroups);
$userGroups = $this->groupManager->getUserGroupIds($targetUser);
$userSubAdminGroups = array_intersect($subAdminGroups, $userGroups);
if (count($userSubAdminGroups) <= 1) {
// Subadmin must not be able to remove a user from all their subadmin groups.
throw new OCSException('Cannot remove user from this group as this is the only remaining group you are a SubAdmin of', 105);
}
}

214
apps/provisioning_api/tests/Controller/UsersControllerTest.php
View File

@ -30,6 +30,9 @@
namespace OCA\Provisioning_API\Tests\Controller;
use OCA\Provisioning_API\Controller\UsersController;
use OCP\AppFramework\Http\DataResponse;
use OCP\IGroup;
use OCP\IUser;
use OCP\IUserManager;
use OCP\IConfig;
use OCP\IUserSession;
@ -1598,11 +1601,10 @@ class UsersControllerTest extends OriginalTest {
* @expectedExceptionCode 102
*/
public function testAddToGroupWithTargetGroupNotExisting() {
$this->groupManager
->expects($this->once())
$this->groupManager->expects($this->once())
->method('get')
->with('GroupToAddTo')
->will($this->returnValue(null));
->willReturn(null);
$this->api->addToGroup('TargetUser', 'GroupToAddTo');
}
@ -1620,16 +1622,149 @@ class UsersControllerTest extends OriginalTest {
* @expectedExceptionCode 103
*/
public function testAddToGroupWithTargetUserNotExisting() {
$targetGroup = $this->getMockBuilder('\OCP\IGroup')->disableOriginalConstructor()->getMock();
$this->groupManager
->expects($this->once())
$targetGroup = $this->createMock(IGroup::class);
$this->groupManager->expects($this->once())
->method('get')
->with('GroupToAddTo')
->will($this->returnValue($targetGroup));
->willReturn($targetGroup);
$this->api->addToGroup('TargetUser', 'GroupToAddTo');
}
/**
* @expectedException \OCP\AppFramework\OCS\OCSException
* @expectedExceptionCode 104
*/
public function testAddToGroupNoSubadmin() {
$targetUser = $this->createMock(IUser::class);
$loggedInUser = $this->createMock(IUser::class);
$loggedInUser->expects($this->once())
->method('getUID')
->willReturn('subadmin');
$targetGroup = $this->createMock(IGroup::class);
$targetGroup->expects($this->never())
->method('addUser')
->with($targetUser);
$this->groupManager->expects($this->once())
->method('get')
->with('GroupToAddTo')
->willReturn($targetGroup);
$subAdminManager = $this->createMock(\OC\SubAdmin::class);
$subAdminManager->expects($this->once())
->method('isSubAdminOfGroup')
->with($loggedInUser, $targetGroup)
->willReturn(false);
$this->groupManager->expects($this->once())
->method('getSubAdmin')
->willReturn($subAdminManager);
$this->groupManager->expects($this->once())
->method('isAdmin')
->with('subadmin')
->willReturn(false);
$this->userManager->expects($this->once())
->method('get')
->with('TargetUser')
->willReturn($targetUser);
$this->userSession->expects($this->once())
->method('getUser')
->willReturn($loggedInUser);
$this->api->addToGroup('TargetUser', 'GroupToAddTo');
}
public function testAddToGroupSuccessAsSubadmin() {
$targetUser = $this->createMock(IUser::class);
$loggedInUser = $this->createMock(IUser::class);
$loggedInUser->expects($this->once())
->method('getUID')
->willReturn('subadmin');
$targetGroup = $this->createMock(IGroup::class);
$targetGroup->expects($this->once())
->method('addUser')
->with($targetUser);
$this->groupManager->expects($this->once())
->method('get')
->with('GroupToAddTo')
->willReturn($targetGroup);
$subAdminManager = $this->createMock(\OC\SubAdmin::class);
$subAdminManager->expects($this->once())
->method('isSubAdminOfGroup')
->with($loggedInUser, $targetGroup)
->willReturn(true);
$this->groupManager->expects($this->once())
->method('getSubAdmin')
->willReturn($subAdminManager);
$this->groupManager->expects($this->once())
->method('isAdmin')
->with('subadmin')
->willReturn(false);
$this->userManager->expects($this->once())
->method('get')
->with('TargetUser')
->willReturn($targetUser);
$this->userSession->expects($this->once())
->method('getUser')
->willReturn($loggedInUser);
$this->assertEquals(new DataResponse(), $this->api->addToGroup('TargetUser', 'GroupToAddTo'));
}
public function testAddToGroupSuccessAsAdmin() {
$targetUser = $this->createMock(IUser::class);
$loggedInUser = $this->createMock(IUser::class);
$loggedInUser->expects($this->once())
->method('getUID')
->willReturn('admin');
$targetGroup = $this->createMock(IGroup::class);
$targetGroup->expects($this->once())
->method('addUser')
->with($targetUser);
$this->groupManager->expects($this->once())
->method('get')
->with('GroupToAddTo')
->willReturn($targetGroup);
$subAdminManager = $this->createMock(\OC\SubAdmin::class);
$subAdminManager->expects($this->never())
->method('isSubAdminOfGroup');
$this->groupManager->expects($this->once())
->method('getSubAdmin')
->willReturn($subAdminManager);
$this->groupManager->expects($this->once())
->method('isAdmin')
->with('admin')
->willReturn(true);
$this->userManager->expects($this->once())
->method('get')
->with('TargetUser')
->willReturn($targetUser);
$this->userSession->expects($this->once())
->method('getUser')
->willReturn($loggedInUser);
$this->assertEquals(new DataResponse(), $this->api->addToGroup('TargetUser', 'GroupToAddTo'));
}
/**
* @expectedException \OCP\AppFramework\OCS\OCSException
* @expectedExceptionCode 101
@ -1813,22 +1948,79 @@ class UsersControllerTest extends OriginalTest {
->method('isSubAdminofGroup')
->with($loggedInUser, $targetGroup)
->will($this->returnValue(true));
$this->groupManager
->expects($this->once())
->method('getSubAdmin')
->will($this->returnValue($subAdminManager));
$this->groupManager
->expects($this->any())
->method('isAdmin')
->with('subadmin')
->will($this->returnValue(false));
$this->api->removeFromGroup('subadmin', 'subadmin');
}
/**
* @expectedException \OCP\AppFramework\OCS\OCSException
* @expectedExceptionCode 105
* @expectedExceptionMessage Cannot remove user from this group as this is the only remaining group you are a SubAdmin of
*/
public function testRemoveFromGroupAsSubAdminFromLastSubAdminGroup() {
$loggedInUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock();
$loggedInUser
->expects($this->any())
->method('getUID')
->will($this->returnValue('subadmin'));
$targetUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock();
$targetGroup = $this->getMockBuilder('\OCP\IGroup')->disableOriginalConstructor()->getMock();
$targetGroup
->expects($this->any())
->method('getGID')
->will($this->returnValue('subadmin'));
$this->userSession
->expects($this->once())
->method('getUser')
->will($this->returnValue($loggedInUser));
$this->groupManager
->expects($this->once())
->method('get')
->with('subadmin')
->will($this->returnValue($targetGroup));
$this->userManager
->expects($this->once())
->method('get')
->with('AnotherUser')
->will($this->returnValue($targetUser));
$subAdminManager = $this->getMockBuilder('OC\SubAdmin')
->disableOriginalConstructor()->getMock();
$subAdminManager
->expects($this->once())
->method('getSubAdminsGroups')
->with($loggedInUser)
->will($this->returnValue([$targetGroup]));
->method('isSubAdminofGroup')
->with($loggedInUser, $targetGroup)
->will($this->returnValue(true));
$this->groupManager
->expects($this->once())
->method('getSubAdmin')
->will($this->returnValue($subAdminManager));
$subAdminManager
->expects($this->once())
->method('getSubAdminsGroups')
->with($loggedInUser)
->will($this->returnValue([$targetGroup]));
$this->groupManager
->expects($this->any())
->method('isAdmin')
->with('subadmin')
->will($this->returnValue(false));
$this->groupManager
->expects($this->once())
->method('getUserGroupIds')
->with($targetUser)
->willReturn(['subadmin', 'other group']);
$this->api->removeFromGroup('subadmin', 'subadmin');
$this->api->removeFromGroup('AnotherUser', 'subadmin');
}
public function testRemoveFromGroupSuccessful() {

2
lib/private/SubAdmin.php
View File

@ -188,7 +188,7 @@ class SubAdmin extends PublicEmitter {
* @param IGroup $group
* @return bool
*/
public function isSubAdminofGroup(IUser $user, IGroup $group) {
public function isSubAdminOfGroup(IUser $user, IGroup $group) {
$qb = $this->dbConn->getQueryBuilder();
/*

92
settings/ajax/togglegroups.php
View File

@ -1,92 +0,0 @@
<?php
/**
* @copyright Copyright (c) 2016, ownCloud, Inc.
*
* @author Bart Visscher <bartv@thisnet.nl>
* @author Christopher Schäpers <kondou@ts.unde.re>
* @author Georg Ehrke <georg@owncloud.com>
* @author Jakob Sack <mail@jakobsack.de>
* @author Lukas Reschke <lukas@statuscode.ch>
* @author Robin Appelman <robin@icewind.nl>
* @author Thomas Müller <thomas.mueller@tmit.eu>
*
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/
OC_JSON::checkSubAdminUser();
OCP\JSON::callCheck();
$lastConfirm = (int) \OC::$server->getSession()->get('last-password-confirm');
if ($lastConfirm < (time() - 30 * 60 + 15)) { // allow 15 seconds delay
$l = \OC::$server->getL10N('core');
OC_JSON::error(array( 'data' => array( 'message' => $l->t('Password confirmation is required'))));
exit();
}
$success = true;
$username = (string)$_POST['username'];
$group = (string)$_POST['group'];
if($username === OC_User::getUser() && $group === "admin" && OC_User::isAdminUser($username)) {
$l = \OC::$server->getL10N('core');
OC_JSON::error(array( 'data' => array( 'message' => $l->t('Admins can\'t remove themself from the admin group'))));
exit();
}
$isUserAccessible = false;
$isGroupAccessible = false;
$currentUserObject = \OC::$server->getUserSession()->getUser();
$targetUserObject = \OC::$server->getUserManager()->get($username);
$targetGroupObject = \OC::$server->getGroupManager()->get($group);
if($targetUserObject !== null && $currentUserObject !== null && $targetGroupObject !== null) {
$isUserAccessible = \OC::$server->getGroupManager()->getSubAdmin()->isUserAccessible($currentUserObject, $targetUserObject);
$isGroupAccessible = \OC::$server->getGroupManager()->getSubAdmin()->isSubAdminofGroup($currentUserObject, $targetGroupObject);
}
if(!OC_User::isAdminUser(OC_User::getUser())
&& (!$isUserAccessible
|| !$isGroupAccessible)) {
$l = \OC::$server->getL10N('core');
OC_JSON::error(array( 'data' => array( 'message' => $l->t('Authentication error') )));
exit();
}
if(!OC_Group::groupExists($group)) {
OC_Group::createGroup($group);
}
$l = \OC::$server->getL10N('settings');
$error = $l->t("Unable to add user to group %s", $group);
$action = "add";
// Toggle group
if( OC_Group::inGroup( $username, $group )) {
$action = "remove";
$error = $l->t("Unable to remove user from group %s", $group);
$success = OC_Group::removeFromGroup( $username, $group );
$usersInGroup=OC_Group::usersInGroup($group);
}
else{
$success = OC_Group::addToGroup( $username, $group );
}
// Return Success story
if( $success ) {
OC_JSON::success(array("data" => array( "username" => $username, "action" => $action, "groupname" => $group )));
}
else{
OC_JSON::error(array("data" => array( "message" => $error )));
}

4
settings/js/users/groups.js
View File

@ -225,7 +225,9 @@ GroupList = {
toggleAddGroup: function (event) {
if (GroupList.isAddGroupButtonVisible()) {
event.stopPropagation();
if (event) {
event.stopPropagation();
}
$('#newgroup-form').show();
$('#newgroup-init').hide();
$('#newgroupname').focus();

75
settings/js/users/users.js
View File

@ -420,42 +420,63 @@ var UserList = {
var $element = $(element);
var checkHandler = null;
var addUserToGroup = null,
removeUserFromGroup = null;
if(user) { // Only if in a user row, and not the #newusergroups select
checkHandler = function (group) {
if (user === OC.currentUser && group === 'admin') {
var handleUserGroupMembership = function (group, add) {
if (user === OC.getCurrentUser().uid && group === 'admin') {
return false;
}
if (!OC.isUserAdmin() && checked.length === 1 && checked[0] === group) {
return false;
}
$.post(
OC.filePath('settings', 'ajax', 'togglegroups.php'),
{
username: user,
group: group
if (add && OC.isUserAdmin() && UserList.availableGroups.indexOf(group) === -1) {
GroupList.createGroup(group);
if (UserList.availableGroups.indexOf(group) === -1) {
UserList.availableGroups.push(group);
}
}
$.ajax({
url: OC.linkToOCS('cloud/users/' + user , 2) + 'groups',
data: {
groupid: group
},
function (response) {
if (response.status === 'success') {
GroupList.update();
var groupName = response.data.groupname;
if (UserList.availableGroups.indexOf(groupName) === -1 &&
response.data.action === 'add'
) {
UserList.availableGroups.push(groupName);
}
type: add ? 'POST' : 'DELETE',
beforeSend: function (request) {
request.setRequestHeader('Accept', 'application/json');
},
success: function() {
GroupList.update();
if (add && UserList.availableGroups.indexOf(group) === -1) {
UserList.availableGroups.push(group);
}
if (response.data.action === 'add') {
GroupList.incGroupCount(groupName);
} else {
GroupList.decGroupCount(groupName);
}
if (add) {
GroupList.incGroupCount(group);
} else {
GroupList.decGroupCount(group);
}
if (response.data.message) {
OC.Notification.show(response.data.message);
},
error: function() {
if (add) {
OC.Notification.show(t('settings', 'Unable to add user to group {group}', {
group: group
}));
} else {
OC.Notification.show(t('settings', 'Unable to remove user from group {group}', {
group: group
}));
}
}
);
});
};
addUserToGroup = function (group) {
return handleUserGroupMembership(group, true);
};
removeUserFromGroup = function (group) {
return handleUserGroupMembership(group, false);
};
}
var addGroup = function (select, group) {
@ -473,8 +494,8 @@ var UserList = {
createText: label,
selectedFirst: true,
checked: checked,
oncheck: checkHandler,
onuncheck: checkHandler,
oncheck: addUserToGroup,
onuncheck: removeUserFromGroup,
minWidth: 100
});
},

Write Preview
Loading…
Cancel
Save