diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php
index cc1d63d2d347195795b29d36b43723f3a8d9fc7a..2e8a2ffe5ed2b34e27b45b7eabf5e7d32f16e8cf 100644
--- a/apps/provisioning_api/lib/Controller/UsersController.php
+++ b/apps/provisioning_api/lib/Controller/UsersController.php
@@ -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);
}
}
diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php
index e04ee86feae29d7c6f974ebb3772dceb7dc8a617..4d3da5fd33af77b4cd9e0e15d8f3169f506adb3f 100644
--- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php
+++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php
@@ -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() {
diff --git a/lib/private/SubAdmin.php b/lib/private/SubAdmin.php
index 9a2beed09fe5c300dcfba7ab81b1a2f51d2bb317..0650c7c6c01285330f3f55a7955c6ceeb178f706 100644
--- a/lib/private/SubAdmin.php
+++ b/lib/private/SubAdmin.php
@@ -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();
/*
diff --git a/settings/ajax/togglegroups.php b/settings/ajax/togglegroups.php
deleted file mode 100644
index b9958bef0c986ad13ed689961a034c42c527f85b..0000000000000000000000000000000000000000
--- a/settings/ajax/togglegroups.php
+++ /dev/null
@@ -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 )));
-}
diff --git a/settings/js/users/groups.js b/settings/js/users/groups.js
index cfe01c17530940609eedcd52463b553c102e1fdf..aac1609bce7a1c7b3263514c2ae3b03673c160f1 100644
--- a/settings/js/users/groups.js
+++ b/settings/js/users/groups.js
@@ -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();
diff --git a/settings/js/users/users.js b/settings/js/users/users.js
index a2ccc059f15aca46ab633fcf16408154451f129a..3cf7b5e810a183a2ed3ceb87440b1b211d0d8576 100644
--- a/settings/js/users/users.js
+++ b/settings/js/users/users.js
@@ -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
});
},