From ccfd570d33ea15dff9252a2083abf6359bb5a3bf Mon Sep 17 00:00:00 2001 From: Georg Ehrke Date: Tue, 26 Nov 2019 16:37:57 +0100 Subject: respect shareapi_allow_share_dialog_user_enumeration in Principal backend for Sabre/DAV Signed-off-by: Georg Ehrke --- apps/dav/lib/Connector/Sabre/Principal.php | 15 ++++ apps/dav/lib/RootCollection.php | 2 +- apps/dav/tests/unit/CardDAV/CardDavBackendTest.php | 2 +- .../tests/unit/Connector/Sabre/PrincipalTest.php | 84 ++++++++++++++++++++++ 4 files changed, 101 insertions(+), 2 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/Principal.php b/apps/dav/lib/Connector/Sabre/Principal.php index 812f9d54162..d77c3943340 100644 --- a/apps/dav/lib/Connector/Sabre/Principal.php +++ b/apps/dav/lib/Connector/Sabre/Principal.php @@ -82,6 +82,7 @@ class Principal implements BackendInterface { * @param IGroupManager $groupManager * @param IShareManager $shareManager * @param IUserSession $userSession + * @param IAppManager $appManager * @param IConfig $config * @param string $principalPrefix */ @@ -239,6 +240,8 @@ class Principal implements BackendInterface { return []; } + $allowEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes'; + // If sharing is restricted to group members only, // return only members that have groups in common $restrictGroups = false; @@ -256,6 +259,12 @@ class Principal implements BackendInterface { case '{http://sabredav.org/ns}email-address': $users = $this->userManager->getByEmail($value); + if (!$allowEnumeration) { + $users = \array_filter($users, static function(IUser $user) use ($value) { + return $user->getEMailAddress() === $value; + }); + } + $results[] = array_reduce($users, function(array $carry, IUser $user) use ($restrictGroups) { // is sharing restricted to groups only? if ($restrictGroups !== false) { @@ -273,6 +282,12 @@ class Principal implements BackendInterface { case '{DAV:}displayname': $users = $this->userManager->searchDisplayName($value); + if (!$allowEnumeration) { + $users = \array_filter($users, static function(IUser $user) use ($value) { + return $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/lib/RootCollection.php b/apps/dav/lib/RootCollection.php index 38c8b2f6b47..807ae1fcaa4 100644 --- a/apps/dav/lib/RootCollection.php +++ b/apps/dav/lib/RootCollection.php @@ -61,7 +61,7 @@ class RootCollection extends SimpleCollection { $config, \OC::$server->getAppManager() ); - $groupPrincipalBackend = new GroupPrincipalBackend($groupManager, $userSession, $shareManager, $l10n); + $groupPrincipalBackend = new GroupPrincipalBackend($groupManager, $userSession, $shareManager); $calendarResourcePrincipalBackend = new ResourcePrincipalBackend($db, $userSession, $groupManager, $logger); $calendarRoomPrincipalBackend = new RoomPrincipalBackend($db, $userSession, $groupManager, $logger); // as soon as debug mode is enabled we allow listing of principals diff --git a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php index 86c85a972e3..8954b6066f7 100644 --- a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php +++ b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php @@ -395,7 +395,7 @@ class CardDavBackendTest extends TestCase { // create a card $uri0 = $this->getUniqueID('card'); $this->backend->createCard($bookId, $uri0, $this->vcardTest0); - + // create another card with same uid $uri1 = $this->getUniqueID('card'); $this->expectException(BadRequest::class); diff --git a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php index 225189e7d01..5fce37d0521 100644 --- a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php @@ -308,6 +308,11 @@ class PrincipalTest extends TestCase { ->will($this->returnValue($sharingEnabled)); if ($sharingEnabled) { + $this->config->expects($this->once()) + ->method('getAppValue') + ->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') + ->willReturn('yes'); + $this->shareManager->expects($this->once()) ->method('shareWithGroupMembersOnly') ->will($this->returnValue($groupsOnly)); @@ -324,6 +329,8 @@ class PrincipalTest extends TestCase { ->will($this->returnValue(['group1', 'group2', 'group5'])); } } else { + $this->config->expects($this->never()) + ->method('getAppValue'); $this->shareManager->expects($this->never()) ->method('shareWithGroupMembersOnly'); $this->groupManager->expects($this->never()) @@ -396,6 +403,11 @@ class PrincipalTest extends TestCase { ->method('shareAPIEnabled') ->will($this->returnValue(true)); + $this->config->expects($this->exactly(2)) + ->method('getAppValue') + ->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') + ->willReturn('yes'); + $this->shareManager->expects($this->exactly(2)) ->method('shareWithGroupMembersOnly') ->will($this->returnValue(false)); @@ -417,6 +429,78 @@ class PrincipalTest extends TestCase { ['{urn:ietf:params:xml:ns:caldav}calendar-user-address-set' => 'user@example.com'])); } + public function testSearchPrincipalWithEnumerationDisabledDisplayname() { + $this->shareManager->expects($this->once()) + ->method('shareAPIEnabled') + ->will($this->returnValue(true)); + + $this->config->expects($this->once()) + ->method('getAppValue') + ->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') + ->willReturn('no'); + + $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')); + $user2->method('getDisplayName')->will($this->returnValue('User 22')); + $user2->method('getEMailAddress')->will($this->returnValue('user2@foo.bar123')); + $user4 = $this->createMock(IUser::class); + $user4->method('getUID')->will($this->returnValue('user4')); + $user2->method('getDisplayName')->will($this->returnValue('User 222')); + $user2->method('getEMailAddress')->will($this->returnValue('user2@foo.bar456')); + + $this->userManager->expects($this->at(0)) + ->method('searchDisplayName') + ->with('User 2') + ->will($this->returnValue([$user2, $user3, $user4])); + + $this->assertEquals(['principals/users/user2'], $this->connector->searchPrincipals('principals/users', + ['{DAV:}displayname' => 'User 2'])); + } + + public function testSearchPrincipalWithEnumerationDisabledEmail() { + $this->shareManager->expects($this->once()) + ->method('shareAPIEnabled') + ->will($this->returnValue(true)); + + $this->config->expects($this->once()) + ->method('getAppValue') + ->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') + ->willReturn('no'); + + $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')); + $user2->method('getDisplayName')->will($this->returnValue('User 22')); + $user2->method('getEMailAddress')->will($this->returnValue('user2@foo.bar123')); + $user4 = $this->createMock(IUser::class); + $user4->method('getUID')->will($this->returnValue('user4')); + $user2->method('getDisplayName')->will($this->returnValue('User 222')); + $user2->method('getEMailAddress')->will($this->returnValue('user2@foo.bar456')); + + $this->userManager->expects($this->at(0)) + ->method('getByEmail') + ->with('user2@foo.bar') + ->will($this->returnValue([$user2, $user3, $user4])); + + $this->assertEquals(['principals/users/user2'], $this->connector->searchPrincipals('principals/users', + ['{http://sabredav.org/ns}email-address' => 'user2@foo.bar'])); + } + public function testFindByUriSharingApiDisabled() { $this->shareManager->expects($this->once()) ->method('shareApiEnabled') -- cgit v1.2.3