From 5a43b38bad639103ea086935b0b33bd70822b030 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Fri, 4 Feb 2022 14:27:13 +0100 Subject: [PATCH 1/4] Show only assignable groups to the subadmin --- settings/Controller/GroupsController.php | 28 +++++++++++++++++++----- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/settings/Controller/GroupsController.php b/settings/Controller/GroupsController.php index b72cac33fd1b..d80c37cb5fb8 100644 --- a/settings/Controller/GroupsController.php +++ b/settings/Controller/GroupsController.php @@ -170,13 +170,29 @@ public function getAssignableAndRemovableGroups() { $assignableGroups = []; $removableGroups = []; - foreach ($this->groupManager->getBackends() as $backend) { - $groups = $backend->getGroups(); - if ($backend->implementsActions($backend::ADD_TO_GROUP)) { - \array_push($assignableGroups, ...$groups); + $currentUser = $this->userSession->getUser(); + $subAdmin = $this->groupManager->getSubAdmin(); + + if ($this->groupManager->isAdmin($currentUser->getUID())) { + foreach ($this->groupManager->getBackends() as $backend) { + $groups = $backend->getGroups(); + if ($backend->implementsActions($backend::ADD_TO_GROUP)) { + \array_push($assignableGroups, ...$groups); + } + if ($backend->implementsActions($backend::REMOVE_FROM_GROUP)) { + \array_push($removableGroups, ...$groups); + } } - if ($backend->implementsActions($backend::REMOVE_FROM_GROUP)) { - \array_push($removableGroups, ...$groups); + } elseif ($subAdmin->isSubAdmin($currentUser)) { + $subAdminGroups = $subAdmin->getSubAdminsGroups($currentUser); + foreach ($subAdminGroups as $subAdminGroup) { + $backend = $subAdminGroup->getBackend(); + if ($backend->implementsActions($backend::ADD_TO_GROUP)) { + $assignableGroups[] = $subAdminGroup->getGID(); + } + if ($backend->implementsActions($backend::REMOVE_FROM_GROUP)) { + $removableGroups[] = $subAdminGroup->getGID(); + } } } From a1bafabaaffe9b75c33c7d4aaa5277b5672ada6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Fri, 4 Feb 2022 14:52:07 +0100 Subject: [PATCH 2/4] Add changelog entry --- changelog/unreleased/39752 | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 changelog/unreleased/39752 diff --git a/changelog/unreleased/39752 b/changelog/unreleased/39752 new file mode 100644 index 000000000000..56c5641f8d48 --- /dev/null +++ b/changelog/unreleased/39752 @@ -0,0 +1,7 @@ +Bugfix: Subadmin will be shown only his assignable groups in the users page + +Previously, the subadmin could see all groups even if he could only assign +users to a bunch of them. Now the subadmin will see the groups he can assign +to the user + +https://github.com/owncloud/core/pull/39752 From 3ffb2834c0b2d2c09b559cbf6e1dffb04a481be3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Mon, 7 Feb 2022 08:50:10 +0100 Subject: [PATCH 3/4] Suppress phan warning --- settings/Controller/GroupsController.php | 1 + 1 file changed, 1 insertion(+) diff --git a/settings/Controller/GroupsController.php b/settings/Controller/GroupsController.php index d80c37cb5fb8..23c0e1d09dc5 100644 --- a/settings/Controller/GroupsController.php +++ b/settings/Controller/GroupsController.php @@ -171,6 +171,7 @@ public function getAssignableAndRemovableGroups() { $removableGroups = []; $currentUser = $this->userSession->getUser(); + /* @phan-suppress-next-line PhanUndeclaredMethod */ $subAdmin = $this->groupManager->getSubAdmin(); if ($this->groupManager->isAdmin($currentUser->getUID())) { From 90a4cf15c51a4760bc6bf4f3d49da2bcd8d659f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Mon, 7 Feb 2022 11:07:03 +0100 Subject: [PATCH 4/4] Include subadmin test and adjust failing one --- .../Controller/GroupsControllerTest.php | 84 ++++++++++++++++++- 1 file changed, 83 insertions(+), 1 deletion(-) diff --git a/tests/Settings/Controller/GroupsControllerTest.php b/tests/Settings/Controller/GroupsControllerTest.php index 5b1e49f739cf..9a1973b004b6 100644 --- a/tests/Settings/Controller/GroupsControllerTest.php +++ b/tests/Settings/Controller/GroupsControllerTest.php @@ -11,8 +11,11 @@ namespace Tests\Settings\Controller; use OC\Group\MetaData; +use OC\SubAdmin; +use OC\User\User; use OC\Settings\Application; use OC\Settings\Controller\GroupsController; +use OCP\IGroup; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; @@ -31,7 +34,7 @@ protected function setUp(): void { $app = new Application(); $this->container = $app->getContainer(); $this->container['AppName'] = 'settings'; - $this->container['GroupManager'] = $this->getMockBuilder('\OCP\IGroupManager') + $this->container['GroupManager'] = $this->getMockBuilder('\OC\Group\Manager') ->disableOriginalConstructor()->getMock(); $this->container['UserSession'] = $this->getMockBuilder('\OC\User\Session') ->disableOriginalConstructor()->getMock(); @@ -351,6 +354,29 @@ public function testDestroyUnsuccessful() { } public function testGetAssignableAndRemovableGroups() { + $userid = 'MyAdminUser'; + + $user = $this->createMock(User::class); + $user->method('getUID') + ->will($this->returnValue($userid)); + $this->container['UserSession'] + ->method('getUser') + ->willReturn($user); + + $subadmin = $this->createMock(SubAdmin::class); + $subadmin->method('isSubAdmin') + ->with($this->equalTo($user)) + ->willReturn(false); + $this->container['GroupManager'] + ->expects($this->once()) + ->method('getSubAdmin') + ->willReturn($subadmin); + + $this->container['GroupManager'] + ->expects($this->once()) + ->method('isAdmin') + ->with($userid) + ->willReturn(true); $assignableGroups = ['assignableGroup']; $backend = $this->createMock('\OCP\GroupInterface'); $backend @@ -377,4 +403,60 @@ public function testGetAssignableAndRemovableGroups() { $response = $this->groupsController->getAssignableAndRemovableGroups(); $this->assertEquals($expectedResponse, $response); } + + public function testGetAssignableAndRemovableGroupsSubadmin() { + $userid = 'MySubAdminUser'; + + $user = $this->createMock(User::class); + $user->method('getUID') + ->will($this->returnValue($userid)); + $this->container['UserSession'] + ->method('getUser') + ->willReturn($user); + + $this->container['GroupManager'] + ->expects($this->once()) + ->method('isAdmin') + ->with($userid) + ->willReturn(false); + $backend = $this->createMock('\OCP\GroupInterface'); + $backend->method('implementsActions') + ->willReturn(true); + + $group1 = $this->createMock(IGroup::class); + $group1->method('getBackend') + ->willReturn($backend); + $group1->method('getGID') + ->willReturn('MyGroup1'); + $group2 = $this->createMock(IGroup::class); + $group2->method('getBackend') + ->willReturn($backend); + $group2->method('getGID') + ->willReturn('group2'); + + $subadmin = $this->createMock(SubAdmin::class); + $subadmin->method('isSubAdmin') + ->with($user) + ->willReturn(true); + $subadmin->method('getSubAdminsGroups') + ->with($user) + ->willReturn([$group1, $group2]); + + $this->container['GroupManager'] + ->expects($this->once()) + ->method('getSubAdmin') + ->willReturn($subadmin); + + $expectedResponse = new DataResponse( + [ + 'data' => [ + 'assignableGroups' => ['MyGroup1', 'group2'], + 'removableGroups' => ['MyGroup1', 'group2'], + ], + \OC\AppFramework\Http::STATUS_OK + ] + ); + $response = $this->groupsController->getAssignableAndRemovableGroups(); + $this->assertEquals($expectedResponse, $response); + } }