From 0fa897fcfcfc404c3b8bb3d25a61328cb43949f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 20 Feb 2020 18:52:23 +0100 Subject: [PATCH] Limit enumeration in principal search MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- apps/dav/lib/Connector/Sabre/Principal.php | 29 ++++- .../unit/Connector/Sabre/PrincipalTest.php | 122 ++++++++++++++++++ 2 files changed, 150 insertions(+), 1 deletion(-) diff --git a/apps/dav/lib/Connector/Sabre/Principal.php b/apps/dav/lib/Connector/Sabre/Principal.php index 449275e982..41b85f162c 100644 --- a/apps/dav/lib/Connector/Sabre/Principal.php +++ b/apps/dav/lib/Connector/Sabre/Principal.php @@ -245,7 +245,8 @@ class Principal implements BackendInterface { return []; } - $allowEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes'; + $allowEnumeration = $this->shareManager->allowEnumeration(); + $limitEnumeration = $this->shareManager->limitEnumerationToGroups(); // If sharing is restricted to group members only, // return only members that have groups in common @@ -259,6 +260,14 @@ class Principal implements BackendInterface { $restrictGroups = $this->groupManager->getUserGroupIds($user); } + $currentUserGroups = []; + if ($limitEnumeration) { + $currentUser = $this->userSession->getUser(); + if ($currentUser) { + $currentUserGroups = $this->groupManager->getUserGroupIds($currentUser); + } + } + foreach ($searchProperties as $prop => $value) { switch ($prop) { case '{http://sabredav.org/ns}email-address': @@ -270,6 +279,15 @@ class Principal implements BackendInterface { }); } + if ($limitEnumeration) { + $users = \array_filter($users, function (IUser $user) use ($currentUserGroups, $value) { + return !empty(array_intersect( + $this->groupManager->getUserGroupIds($user), + $currentUserGroups + )) || $user->getEMailAddress() === $value; + }); + } + $results[] = array_reduce($users, function(array $carry, IUser $user) use ($restrictGroups) { // is sharing restricted to groups only? if ($restrictGroups !== false) { @@ -293,6 +311,15 @@ class Principal implements BackendInterface { }); } + if ($limitEnumeration) { + $users = \array_filter($users, function (IUser $user) use ($currentUserGroups, $value) { + return !empty(array_intersect( + $this->groupManager->getUserGroupIds($user), + $currentUserGroups + )) || $user->getDisplayName() === $value; + }); + } + $results[] = array_reduce($users, function(array $carry, IUser $user) use ($restrictGroups) { // is sharing restricted to groups only? if ($restrictGroups !== false) { diff --git a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php index 5198b03185..6228ff275f 100644 --- a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php @@ -624,6 +624,128 @@ class PrincipalTest extends TestCase { ['{http://sabredav.org/ns}email-address' => 'user2@foo.bar'])); } + public function testSearchPrincipalWithEnumerationLimitedDisplayname() { + $this->shareManager->expects($this->at(0)) + ->method('shareAPIEnabled') + ->will($this->returnValue(true)); + + $this->shareManager->expects($this->at(1)) + ->method('allowEnumeration') + ->willReturn(true); + + $this->shareManager->expects($this->at(2)) + ->method('limitEnumerationToGroups') + ->willReturn(true); + + $this->shareManager->expects($this->once()) + ->method('shareWithGroupMembersOnly') + ->will($this->returnValue(false)); + + $user2 = $this->createMock(IUser::class); + $user2->method('getUID')->will($this->returnValue('user2')); + $user2->method('getDisplayName')->will($this->returnValue('User 2')); + $user2->method('getEMailAddress')->will($this->returnValue('user2@foo.bar')); + $user3 = $this->createMock(IUser::class); + $user3->method('getUID')->will($this->returnValue('user3')); + $user3->method('getDisplayName')->will($this->returnValue('User 22')); + $user3->method('getEMailAddress')->will($this->returnValue('user2@foo.bar123')); + $user4 = $this->createMock(IUser::class); + $user4->method('getUID')->will($this->returnValue('user4')); + $user4->method('getDisplayName')->will($this->returnValue('User 222')); + $user4->method('getEMailAddress')->will($this->returnValue('user2@foo.bar456')); + + + $this->userSession->expects($this->at(0)) + ->method('getUser') + ->willReturn($user2); + + $this->groupManager->expects($this->at(0)) + ->method('getUserGroupIds') + ->willReturn(['group1']); + $this->groupManager->expects($this->at(1)) + ->method('getUserGroupIds') + ->willReturn(['group1']); + $this->groupManager->expects($this->at(2)) + ->method('getUserGroupIds') + ->willReturn(['group1']); + $this->groupManager->expects($this->at(3)) + ->method('getUserGroupIds') + ->willReturn(['group2']); + + $this->userManager->expects($this->at(0)) + ->method('searchDisplayName') + ->with('User') + ->willReturn([$user2, $user3, $user4]); + + + $this->assertEquals([ + 'principals/users/user2', + 'principals/users/user3', + ], $this->connector->searchPrincipals('principals/users', + ['{DAV:}displayname' => 'User'])); + } + + public function testSearchPrincipalWithEnumerationLimitedMail() { + $this->shareManager->expects($this->at(0)) + ->method('shareAPIEnabled') + ->will($this->returnValue(true)); + + $this->shareManager->expects($this->at(1)) + ->method('allowEnumeration') + ->willReturn(true); + + $this->shareManager->expects($this->at(2)) + ->method('limitEnumerationToGroups') + ->willReturn(true); + + $this->shareManager->expects($this->once()) + ->method('shareWithGroupMembersOnly') + ->will($this->returnValue(false)); + + $user2 = $this->createMock(IUser::class); + $user2->method('getUID')->will($this->returnValue('user2')); + $user2->method('getDisplayName')->will($this->returnValue('User 2')); + $user2->method('getEMailAddress')->will($this->returnValue('user2@foo.bar')); + $user3 = $this->createMock(IUser::class); + $user3->method('getUID')->will($this->returnValue('user3')); + $user3->method('getDisplayName')->will($this->returnValue('User 22')); + $user3->method('getEMailAddress')->will($this->returnValue('user2@foo.bar123')); + $user4 = $this->createMock(IUser::class); + $user4->method('getUID')->will($this->returnValue('user4')); + $user4->method('getDisplayName')->will($this->returnValue('User 222')); + $user4->method('getEMailAddress')->will($this->returnValue('user2@foo.bar456')); + + + $this->userSession->expects($this->at(0)) + ->method('getUser') + ->willReturn($user2); + + $this->groupManager->expects($this->at(0)) + ->method('getUserGroupIds') + ->willReturn(['group1']); + $this->groupManager->expects($this->at(1)) + ->method('getUserGroupIds') + ->willReturn(['group1']); + $this->groupManager->expects($this->at(2)) + ->method('getUserGroupIds') + ->willReturn(['group1']); + $this->groupManager->expects($this->at(3)) + ->method('getUserGroupIds') + ->willReturn(['group2']); + + $this->userManager->expects($this->at(0)) + ->method('getByEmail') + ->with('user') + ->willReturn([$user2, $user3, $user4]); + + + $this->assertEquals([ + 'principals/users/user2', + 'principals/users/user3' + ], $this->connector->searchPrincipals('principals/users', + ['{http://sabredav.org/ns}email-address' => 'user'])); + } + public function testFindByUriSharingApiDisabled() { $this->shareManager->expects($this->once()) ->method('shareApiEnabled')