diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 3bb2813ef2e0f..f68ad2a840e9b 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -1585,7 +1585,12 @@ public function addToGroup(string $userId, string $groupid = ''): DataResponse { $subAdminManager = $this->groupManager->getSubAdmin(); $isAdmin = $this->groupManager->isAdmin($loggedInUser->getUID()); $isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($loggedInUser->getUID()); - if (!$isAdmin && !($isDelegatedAdmin && $groupid !== 'admin') && !$subAdminManager->isSubAdminOfGroup($loggedInUser, $group)) { + if ( + !$isAdmin + && !($isDelegatedAdmin && $groupid !== 'admin') + // Group admin can add users to its groups only if it can see them + && !($subAdminManager->isSubAdminOfGroup($loggedInUser, $group) && $subAdminManager->isUserAccessible($loggedInUser, $targetUser)) + ) { throw new OCSException('', 104); } diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index af6bbb0ef1e61..ac796fa8f9787 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -3147,6 +3147,11 @@ public function testAddToGroupSuccessAsSubadmin(): void { ->with($loggedInUser, $targetGroup) ->willReturn(true); + $subAdminManager->expects($this->once()) + ->method('isUserAccessible') + ->with($loggedInUser, $targetUser) + ->willReturn(true); + $this->groupManager->expects($this->once()) ->method('getSubAdmin') ->willReturn($subAdminManager); @@ -3167,6 +3172,61 @@ public function testAddToGroupSuccessAsSubadmin(): void { $this->assertEquals(new DataResponse(), $this->api->addToGroup('TargetUser', 'GroupToAddTo')); } + public function testAddInvisibleUserToGroupFailAsSubadmin(): void { + $targetUser = $this->createMock(IUser::class); + $loggedInUser = $this->createMock(IUser::class); + $loggedInUser + ->expects($this->exactly(2)) + ->method('getUID') + ->willReturn('subadmin'); + + $targetGroup = $this->createMock(IGroup::class); + + $this->groupManager + ->expects($this->once()) + ->method('get') + ->with('GroupToAddTo') + ->willReturn($targetGroup); + + $subAdminManager = $this->createMock(SubAdmin::class); + $subAdminManager + ->expects($this->once()) + ->method('isSubAdminOfGroup') + ->with($loggedInUser, $targetGroup) + ->willReturn(true); + + $subAdminManager + ->expects($this->once()) + ->method('isUserAccessible') + ->with($loggedInUser, $targetUser) + ->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->expectException(OCSException::class); + $this->expectExceptionCode(104); + $this->api->addToGroup('TargetUser', 'GroupToAddTo'); + } + public function testAddToGroupSuccessAsAdmin(): void { $targetUser = $this->createMock(IUser::class); $loggedInUser = $this->createMock(IUser::class);