diff --git a/apps/provisioning_api/lib/users.php b/apps/provisioning_api/lib/users.php index 68c89e41f6f7..412c2c005d61 100644 --- a/apps/provisioning_api/lib/users.php +++ b/apps/provisioning_api/lib/users.php @@ -373,6 +373,27 @@ public function getUsersGroups($parameters) { } + /** + * Returns whether the given user can manage the given group + * + * @param IUser $user user to check access + * @param IGroup|null $group group to check or null + * + * @return true if the user can manage the group + */ + private function canUserManageGroup($user, $group) { + if ($this->groupManager->isAdmin($user->getUID())) { + return true; + } + + if ($group !== null) { + $subAdminManager = $this->groupManager->getSubAdmin(); + return $subAdminManager->isSubAdminofGroup($user, $group); + } + + return false; + } + /** * @param array $parameters * @return OC_OCS_Result @@ -384,22 +405,22 @@ public function addToGroup($parameters) { return new OC_OCS_Result(null, \OCP\API::RESPOND_UNAUTHORISED); } - // Check they're an admin - if(!$this->groupManager->isAdmin($user->getUID())) { - // This user doesn't have rights to add a user to this group - return new OC_OCS_Result(null, \OCP\API::RESPOND_UNAUTHORISED); - } - $groupId = !empty($_POST['groupid']) ? $_POST['groupid'] : null; if($groupId === null) { return new OC_OCS_Result(null, 101); } $group = $this->groupManager->get($groupId); - $targetUser = $this->userManager->get($parameters['userid']); - if($group === null) { + if ($group === null) { return new OC_OCS_Result(null, 102); } + + // Check they're an admin or subadmin of the group + if(!$this->canUserManageGroup($user, $group)) { + return new OC_OCS_Result(null, 104); + } + + $targetUser = $this->userManager->get($parameters['userid']); if($targetUser === null) { return new OC_OCS_Result(null, 103); } @@ -430,16 +451,14 @@ public function removeFromGroup($parameters) { return new OC_OCS_Result(null, 102); } + if(!$this->canUserManageGroup($loggedInUser, $group)) { + return new OC_OCS_Result(null, 104); + } + $targetUser = $this->userManager->get($parameters['userid']); if($targetUser === null) { return new OC_OCS_Result(null, 103); } - - // 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)) { - return new OC_OCS_Result(null, 104); - } // Check they aren't removing themselves from 'admin' or their 'subadmin; group if($parameters['userid'] === $loggedInUser->getUID()) { if($this->groupManager->isAdmin($loggedInUser->getUID())) { @@ -448,6 +467,7 @@ public function removeFromGroup($parameters) { } } else { // Not an admin, check they are not removing themself from their subadmin group + $subAdminManager = $this->groupManager->getSubAdmin(); $subAdminGroups = $subAdminManager->getSubAdminsGroups($loggedInUser); foreach ($subAdminGroups as $key => $group) { $subAdminGroups[$key] = $group->getGID(); diff --git a/apps/provisioning_api/tests/userstest.php b/apps/provisioning_api/tests/userstest.php index 020071bcfa16..01ec84df4a71 100644 --- a/apps/provisioning_api/tests/userstest.php +++ b/apps/provisioning_api/tests/userstest.php @@ -1631,10 +1631,6 @@ public function testAddToGroupWithTargetGroupNotExisting() { $_POST['groupid'] = 'GroupToAddTo'; $loggedInUser = $this->getMock('\OCP\IUser'); - $loggedInUser - ->expects($this->once()) - ->method('getUID') - ->will($this->returnValue('admin')); $this->userSession ->expects($this->once()) ->method('getUser') @@ -1644,44 +1640,59 @@ public function testAddToGroupWithTargetGroupNotExisting() { ->method('get') ->with('GroupToAddTo') ->will($this->returnValue(null)); - $this->groupManager - ->expects($this->once()) - ->method('isAdmin') - ->with('admin') - ->will($this->returnValue(true)); $expected = new \OC_OCS_Result(null, 102); $this->assertEquals($expected, $this->api->addToGroup(['userid' => 'TargetUser'])); } public function testAddToGroupWithNoGroupSpecified() { + $loggedInUser = $this->getMock('\OCP\IUser'); + $this->userSession + ->expects($this->once()) + ->method('getUser') + ->will($this->returnValue($loggedInUser)); + + $expected = new \OC_OCS_Result(null, 101); + $this->assertEquals($expected, $this->api->addToGroup(['userid' => 'TargetUser'])); + } + + public function testAddToGroupWithTargetUserNotExisting() { + $_POST['groupid'] = 'GroupToAddTo'; + $loggedInUser = $this->getMock('\OCP\IUser'); $loggedInUser ->expects($this->once()) ->method('getUID') ->will($this->returnValue('admin')); + $targetGroup = $this->getMock('\OCP\IGroup'); $this->userSession ->expects($this->once()) ->method('getUser') ->will($this->returnValue($loggedInUser)); + $this->groupManager + ->expects($this->once()) + ->method('get') + ->with('GroupToAddTo') + ->will($this->returnValue($targetGroup)); $this->groupManager ->expects($this->once()) ->method('isAdmin') ->with('admin') ->will($this->returnValue(true)); - $expected = new \OC_OCS_Result(null, 101); + $expected = new \OC_OCS_Result(null, 103); $this->assertEquals($expected, $this->api->addToGroup(['userid' => 'TargetUser'])); } - public function testAddToGroupWithTargetUserNotExisting() { + public function testAddToGroupWithoutPermission() { $_POST['groupid'] = 'GroupToAddTo'; $loggedInUser = $this->getMock('\OCP\IUser'); $loggedInUser ->expects($this->once()) ->method('getUID') - ->will($this->returnValue('admin')); + ->will($this->returnValue('unauthorizedUser')); + $targetUser = $this->getMock('\OCP\IUser'); $targetGroup = $this->getMock('\OCP\IGroup'); $this->userSession ->expects($this->once()) @@ -1692,38 +1703,157 @@ public function testAddToGroupWithTargetUserNotExisting() { ->method('get') ->with('GroupToAddTo') ->will($this->returnValue($targetGroup)); + $subAdminManager = $this->getMockBuilder('\OC\Subadmin') + ->disableOriginalConstructor()->getMock(); + $this->groupManager + ->expects($this->once()) + ->method('getSubAdmin') + ->will($this->returnValue($subAdminManager)); $this->groupManager ->expects($this->once()) ->method('isAdmin') - ->with('admin') - ->will($this->returnValue(true)); + ->with('unauthorizedUser') + ->will($this->returnValue(false)); - $expected = new \OC_OCS_Result(null, 103); + $expected = new \OC_OCS_Result(null, 104); $this->assertEquals($expected, $this->api->addToGroup(['userid' => 'TargetUser'])); } - public function testAddToGroupWithoutPermission() { - $_POST['groupid'] = 'GroupToAddTo'; + public function testAddToOutsideGroupAsSubAdminFromSubAdmin() { + $_POST['groupid'] = 'outsidegroup'; $loggedInUser = $this->getMock('\OCP\IUser'); $loggedInUser + ->expects($this->any()) + ->method('getUID') + ->will($this->returnValue('subadmin')); + $targetUser = $this->getMock('\OCP\IUser'); + $subadminGroup = $this->getMock('\OCP\IGroup'); + $subadminGroup + ->expects($this->any()) + ->method('getGID') + ->will($this->returnValue('subadmin')); + $targetGroup = $this->getMock('\OCP\IGroup'); + $targetGroup + ->expects($this->any()) + ->method('getGID') + ->will($this->returnValue('outsidegroup')); + $this->userSession + ->expects($this->once()) + ->method('getUser') + ->will($this->returnValue($loggedInUser)); + $this->groupManager + ->expects($this->any()) + ->method('get') + ->will($this->returnValueMap([ + ['subadmin', $subadminGroup], + ['outsidegroup', $targetGroup] + ])); + $subAdminManager = $this->getMockBuilder('\OC\Subadmin') + ->disableOriginalConstructor()->getMock(); + $subAdminManager + ->expects($this->once()) + ->method('isSubAdminofGroup') + ->with($loggedInUser, $targetGroup) + ->will($this->returnValue(false)); + $this->groupManager ->expects($this->once()) + ->method('getSubAdmin') + ->will($this->returnValue($subAdminManager)); + $this->groupManager + ->expects($this->any()) + ->method('isAdmin') + ->with('subadmin') + ->will($this->returnValue(false)); + + $expected = new \OC_OCS_Result(null, 104); + $this->assertEquals($expected, $this->api->addToGroup(['userid' => 'subadmin'])); + } + + public function testAddToGroupSuccessful() { + $_POST['groupid'] = 'admin'; + $loggedInUser = $this->getMock('\OCP\IUser'); + $loggedInUser + ->expects($this->any()) ->method('getUID') ->will($this->returnValue('admin')); + $targetUser = $this->getMock('\OCP\IUser'); + $targetGroup = $this->getMock('\OCP\IGroup'); $this->userSession ->expects($this->once()) ->method('getUser') ->will($this->returnValue($loggedInUser)); $this->groupManager ->expects($this->once()) + ->method('get') + ->with('admin') + ->will($this->returnValue($targetGroup)); + $this->userManager + ->expects($this->once()) + ->method('get') + ->with('AnotherUser') + ->will($this->returnValue($targetUser)); + $this->groupManager + ->expects($this->any()) ->method('isAdmin') ->with('admin') - ->will($this->returnValue(false)); + ->will($this->returnValue(true)); + $targetGroup + ->expects($this->once()) + ->method('addUser') + ->with($targetUser); - $expected = new \OC_OCS_Result(null, 997); - $this->assertEquals($expected, $this->api->addToGroup(['userid' => 'TargetUser'])); + $expected = new \OC_OCS_Result(null, 100); + $this->assertEquals($expected, $this->api->addToGroup(['userid' => 'AnotherUser'])); } + public function testAddToGroupSuccessfulAsSubadmin() { + $_POST['groupid'] = 'group1'; + $loggedInUser = $this->getMock('\OCP\IUser'); + $loggedInUser + ->expects($this->any()) + ->method('getUID') + ->will($this->returnValue('subadmin')); + $targetUser = $this->getMock('\OCP\IUser'); + $targetGroup = $this->getMock('\OCP\IGroup'); + $this->userSession + ->expects($this->once()) + ->method('getUser') + ->will($this->returnValue($loggedInUser)); + $this->groupManager + ->expects($this->once()) + ->method('get') + ->with('group1') + ->will($this->returnValue($targetGroup)); + $this->userManager + ->expects($this->once()) + ->method('get') + ->with('AnotherUser') + ->will($this->returnValue($targetUser)); + $this->groupManager + ->expects($this->any()) + ->method('isAdmin') + ->with('subadmin') + ->will($this->returnValue(false)); + $targetGroup + ->expects($this->once()) + ->method('addUser') + ->with($targetUser); + $subAdminManager = $this->getMockBuilder('\OC\Subadmin') + ->disableOriginalConstructor()->getMock(); + $subAdminManager + ->expects($this->once()) + ->method('isSubAdminOfGroup') + ->with($loggedInUser, $targetGroup) + ->will($this->returnValue(true)); + $this->groupManager + ->expects($this->once()) + ->method('getSubAdmin') + ->will($this->returnValue($subAdminManager)); + + $expected = new \OC_OCS_Result(null, 100); + $this->assertEquals($expected, $this->api->addToGroup(['userid' => 'AnotherUser'])); + } public function testRemoveFromGroupWithoutLogIn() { $this->userSession ->expects($this->once()) @@ -1762,6 +1892,10 @@ public function testRemoveFromGroupWithNotExistingTargetGroup() { public function testRemoveFromGroupWithNotExistingTargetUser() { $loggedInUser = $this->getMock('\OCP\IUser'); + $loggedInUser + ->expects($this->once()) + ->method('getUID') + ->will($this->returnValue('subadmin')); $targetGroup = $this->getMock('\OCP\IGroup'); $this->userSession ->expects($this->once()) @@ -1777,6 +1911,22 @@ public function testRemoveFromGroupWithNotExistingTargetUser() { ->method('get') ->with('TargetUser') ->will($this->returnValue(null)); + $this->groupManager + ->expects($this->once()) + ->method('isAdmin') + ->with('subadmin') + ->will($this->returnValue(false)); + $subAdminManager = $this->getMockBuilder('\OC\Subadmin') + ->disableOriginalConstructor()->getMock(); + $subAdminManager + ->expects($this->once()) + ->method('isSubAdminofGroup') + ->with($loggedInUser, $targetGroup) + ->will($this->returnValue(true)); + $this->groupManager + ->expects($this->once()) + ->method('getSubAdmin') + ->will($this->returnValue($subAdminManager)); $expected = new \OC_OCS_Result(null, 103); $this->assertEquals($expected, $this->api->removeFromGroup(['userid' => 'TargetUser', '_delete' => ['groupid' => 'TargetGroup']])); @@ -1799,11 +1949,6 @@ public function testRemoveFromGroupWithoutPermission() { ->method('get') ->with('TargetGroup') ->will($this->returnValue($targetGroup)); - $this->userManager - ->expects($this->once()) - ->method('get') - ->with('TargetUser') - ->will($this->returnValue($targetUser)); $subAdminManager = $this->getMockBuilder('\OC\Subadmin') ->disableOriginalConstructor()->getMock(); $this->groupManager @@ -1846,12 +1991,6 @@ public function testRemoveFromGroupAsAdminFromAdmin() { ->method('get') ->with('admin') ->will($this->returnValue($targetUser)); - $subAdminManager = $this->getMockBuilder('\OC\Subadmin') - ->disableOriginalConstructor()->getMock(); - $this->groupManager - ->expects($this->once()) - ->method('getSubAdmin') - ->will($this->returnValue($subAdminManager)); $this->groupManager ->expects($this->any()) ->method('isAdmin') @@ -1901,7 +2040,7 @@ public function testRemoveFromGroupAsSubAdminFromSubAdmin() { ->with($loggedInUser) ->will($this->returnValue([$targetGroup])); $this->groupManager - ->expects($this->once()) + ->expects($this->any()) ->method('getSubAdmin') ->will($this->returnValue($subAdminManager)); $this->groupManager @@ -1936,12 +2075,6 @@ public function testRemoveFromGroupSuccessful() { ->method('get') ->with('AnotherUser') ->will($this->returnValue($targetUser)); - $subAdminManager = $this->getMockBuilder('\OC\Subadmin') - ->disableOriginalConstructor()->getMock(); - $this->groupManager - ->expects($this->once()) - ->method('getSubAdmin') - ->will($this->returnValue($subAdminManager)); $this->groupManager ->expects($this->any()) ->method('isAdmin') @@ -1956,6 +2089,53 @@ public function testRemoveFromGroupSuccessful() { $this->assertEquals($expected, $this->api->removeFromGroup(['userid' => 'AnotherUser', '_delete' => ['groupid' => 'admin']])); } + public function testRemoveFromGroupSuccessfulAsSubadmin() { + $loggedInUser = $this->getMock('\OCP\IUser'); + $loggedInUser + ->expects($this->any()) + ->method('getUID') + ->will($this->returnValue('subadmin')); + $targetUser = $this->getMock('\OCP\IUser'); + $targetGroup = $this->getMock('\OCP\IGroup'); + $this->userSession + ->expects($this->once()) + ->method('getUser') + ->will($this->returnValue($loggedInUser)); + $this->groupManager + ->expects($this->once()) + ->method('get') + ->with('group1') + ->will($this->returnValue($targetGroup)); + $this->userManager + ->expects($this->once()) + ->method('get') + ->with('AnotherUser') + ->will($this->returnValue($targetUser)); + $this->groupManager + ->expects($this->any()) + ->method('isAdmin') + ->with('subadmin') + ->will($this->returnValue(false)); + $targetGroup + ->expects($this->once()) + ->method('removeUser') + ->with($targetUser); + $subAdminManager = $this->getMockBuilder('\OC\Subadmin') + ->disableOriginalConstructor()->getMock(); + $subAdminManager + ->expects($this->once()) + ->method('isSubAdminOfGroup') + ->with($loggedInUser, $targetGroup) + ->will($this->returnValue(true)); + $this->groupManager + ->expects($this->once()) + ->method('getSubAdmin') + ->will($this->returnValue($subAdminManager)); + + $expected = new \OC_OCS_Result(null, 100); + $this->assertEquals($expected, $this->api->removeFromGroup(['userid' => 'AnotherUser', '_delete' => ['groupid' => 'group1']])); + } + public function testAddSubAdminWithNotExistingTargetUser() { $this->userManager ->expects($this->once())