From f7d1cf0585106c811da91aeba767a99a43f133a3 Mon Sep 17 00:00:00 2001
From: Joas Schilling
+
'yes', 'restrictUserEnumerationToGroup' => 'no', 'restrictUserEnumerationToPhone' => 'no', + 'restrictUserEnumerationFullMatch' => 'yes', 'enforceLinkPassword' => false, 'onlyShareWithGroupMembers' => false, 'shareAPIEnabled' => 'yes', @@ -132,6 +134,7 @@ public function testGetFormWithExcludedGroups() { ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'], ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'], ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'], + ['core', 'shareapi_restrict_user_enumeration_full_match', 'yes', 'yes'], ['core', 'shareapi_enabled', 'yes', 'yes'], ['core', 'shareapi_default_expire_date', 'no', 'no'], ['core', 'shareapi_expire_after_n_days', '7', '7'], @@ -156,6 +159,7 @@ public function testGetFormWithExcludedGroups() { 'allowShareDialogUserEnumeration' => 'yes', 'restrictUserEnumerationToGroup' => 'no', 'restrictUserEnumerationToPhone' => 'no', + 'restrictUserEnumerationFullMatch' => 'yes', 'enforceLinkPassword' => false, 'onlyShareWithGroupMembers' => false, 'shareAPIEnabled' => 'yes', diff --git a/build/integration/collaboration_features/autocomplete.feature b/build/integration/collaboration_features/autocomplete.feature index 0ca8ebbc10054..5e294709d7fa0 100644 --- a/build/integration/collaboration_features/autocomplete.feature +++ b/build/integration/collaboration_features/autocomplete.feature @@ -3,6 +3,7 @@ Feature: autocomplete Given using api version "2" And group "commongroup" exists And user "admin" belongs to group "commongroup" + And user "auto" exists And user "autocomplete" exists And user "autocomplete2" exists And user "autocomplete2" belongs to group "commongroup" @@ -20,9 +21,15 @@ Feature: autocomplete When parameter "shareapi_allow_share_dialog_user_enumeration" of app "core" is set to "no" Then get autocomplete for "auto" | id | source | + | auto | users | Then get autocomplete for "autocomplete" | id | source | | autocomplete | users | + When parameter "shareapi_restrict_user_enumeration_full_match" of app "core" is set to "no" + Then get autocomplete for "auto" + | id | source | + Then get autocomplete for "autocomplete" + | id | source | Scenario: getting autocomplete with limited enumeration by group @@ -30,6 +37,7 @@ Feature: autocomplete When parameter "shareapi_restrict_user_enumeration_to_group" of app "core" is set to "yes" Then get autocomplete for "auto" | id | source | + | auto | users | | autocomplete2 | users | Then get autocomplete for "autocomplete" | id | source | @@ -38,6 +46,13 @@ Feature: autocomplete Then get autocomplete for "autocomplete2" | id | source | | autocomplete2 | users | + When parameter "shareapi_restrict_user_enumeration_full_match" of app "core" is set to "no" + Then get autocomplete for "autocomplete" + | id | source | + | autocomplete2 | users | + Then get autocomplete for "autocomplete2" + | id | source | + | autocomplete2 | users | Scenario: getting autocomplete with limited enumeration by phone @@ -45,6 +60,7 @@ Feature: autocomplete When parameter "shareapi_restrict_user_enumeration_to_phone" of app "core" is set to "yes" Then get autocomplete for "auto" | id | source | + | auto | users | # autocomplete stores their phone number Given As an "autocomplete" @@ -57,10 +73,17 @@ Feature: autocomplete Given As an "admin" Then get autocomplete for "auto" | id | source | + | auto | users | # admin populates they have the phone number When search users by phone for region "DE" with | random-string1 | 0711 / 252 428-90 | + Then get autocomplete for "auto" + | id | source | + | auto | users | + | autocomplete | users | + + When parameter "shareapi_restrict_user_enumeration_full_match" of app "core" is set to "no" Then get autocomplete for "auto" | id | source | | autocomplete | users | @@ -83,6 +106,13 @@ Feature: autocomplete When search users by phone for region "DE" with | random-string1 | 0711 / 252 428-90 | + Then get autocomplete for "auto" + | id | source | + | auto | users | + | autocomplete | users | + | autocomplete2 | users | + + When parameter "shareapi_restrict_user_enumeration_full_match" of app "core" is set to "no" Then get autocomplete for "auto" | id | source | | autocomplete | users | @@ -108,6 +138,7 @@ Feature: autocomplete Then get autocomplete for "auto" | id | source | + | auto | users | | autocomplete | users | | autocomplete2 | users | When parameter "shareapi_only_share_with_group_members" of app "core" is set to "yes" @@ -121,6 +152,7 @@ Feature: autocomplete When parameter "shareapi_restrict_user_enumeration_to_phone" of app "core" is set to "yes" Then get autocomplete for "auto" | id | source | + | auto | users | # autocomplete stores their phone number Given As an "autocomplete" @@ -133,12 +165,14 @@ Feature: autocomplete Given As an "admin" Then get autocomplete for "auto" | id | source | + | auto | users | # admin populates they have the phone number When search users by phone for region "DE" with | random-string1 | 0711 / 252 428-90 | Then get autocomplete for "auto" | id | source | + | auto | users | | autocomplete | users | # autocomplete changes their phone number @@ -152,12 +186,14 @@ Feature: autocomplete Given As an "admin" Then get autocomplete for "auto" | id | source | + | auto | users | # admin populates they have the new phone number When search users by phone for region "DE" with | random-string1 | 0711 / 252 428-91 | Then get autocomplete for "auto" | id | source | + | auto | users | | autocomplete | users | diff --git a/build/integration/features/bootstrap/CollaborationContext.php b/build/integration/features/bootstrap/CollaborationContext.php index 8207267bf4d6e..cdba167e6775a 100644 --- a/build/integration/features/bootstrap/CollaborationContext.php +++ b/build/integration/features/bootstrap/CollaborationContext.php @@ -66,6 +66,7 @@ protected function resetAppConfigs(): void { $this->deleteServerConfig('core', 'shareapi_allow_share_dialog_user_enumeration'); $this->deleteServerConfig('core', 'shareapi_restrict_user_enumeration_to_group'); $this->deleteServerConfig('core', 'shareapi_restrict_user_enumeration_to_phone'); + $this->deleteServerConfig('core', 'shareapi_restrict_user_enumeration_full_match'); $this->deleteServerConfig('core', 'shareapi_only_share_with_group_members'); } } diff --git a/lib/private/Collaboration/Collaborators/MailPlugin.php b/lib/private/Collaboration/Collaborators/MailPlugin.php index 7da8cede6aa6d..240e16374d54c 100644 --- a/lib/private/Collaboration/Collaborators/MailPlugin.php +++ b/lib/private/Collaboration/Collaborators/MailPlugin.php @@ -49,6 +49,8 @@ class MailPlugin implements ISearchPlugin { protected $shareeEnumerationInGroupOnly; /* @var bool */ protected $shareeEnumerationPhone; + /* @var bool */ + protected $shareeEnumerationFullMatch; /** @var IManager */ private $contactsManager; @@ -81,6 +83,7 @@ public function __construct(IManager $contactsManager, $this->shareWithGroupOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes'; $this->shareeEnumerationInGroupOnly = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; $this->shareeEnumerationPhone = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; + $this->shareeEnumerationFullMatch = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes') === 'yes'; } /** @@ -137,7 +140,7 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) { continue; } } - if ($exactEmailMatch) { + if ($exactEmailMatch && $this->shareeEnumerationFullMatch) { try { $cloud = $this->cloudIdManager->resolveCloudId($contact['CLOUD'][0]); } catch (\InvalidArgumentException $e) { diff --git a/lib/private/Collaboration/Collaborators/UserPlugin.php b/lib/private/Collaboration/Collaborators/UserPlugin.php index 5114ccd8eb54a..06a8c6f0efd47 100644 --- a/lib/private/Collaboration/Collaborators/UserPlugin.php +++ b/lib/private/Collaboration/Collaborators/UserPlugin.php @@ -53,6 +53,8 @@ class UserPlugin implements ISearchPlugin { protected $shareeEnumerationInGroupOnly; /* @var bool */ protected $shareeEnumerationPhone; + /* @var bool */ + protected $shareeEnumerationFullMatch; /** @var IConfig */ private $config; @@ -85,6 +87,7 @@ public function __construct(IConfig $config, $this->shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes'; $this->shareeEnumerationInGroupOnly = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; $this->shareeEnumerationPhone = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; + $this->shareeEnumerationFullMatch = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes') === 'yes'; } public function search($search, $limit, $offset, ISearchResult $searchResult) { @@ -150,6 +153,7 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) { if ( + $this->shareeEnumerationFullMatch && $lowerSearch !== '' && (strtolower($uid) === $lowerSearch || strtolower($userDisplayName) === $lowerSearch || strtolower($userEmail) === $lowerSearch) @@ -202,7 +206,7 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) { } } - if ($offset === 0 && !$foundUserById) { + if ($this->shareeEnumerationFullMatch && $offset === 0 && !$foundUserById) { // On page one we try if the search result has a direct hit on the // user id and if so, we add that to the exact match list $user = $this->userManager->get($search); diff --git a/lib/private/Contacts/ContactsMenu/ContactsStore.php b/lib/private/Contacts/ContactsMenu/ContactsStore.php index 852765506c063..e0e0bf832b330 100644 --- a/lib/private/Contacts/ContactsMenu/ContactsStore.php +++ b/lib/private/Contacts/ContactsMenu/ContactsStore.php @@ -124,6 +124,7 @@ private function filterContacts(IUser $self, $disallowEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') !== 'yes'; $restrictEnumerationGroup = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; $restrictEnumerationPhone = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; + $allowEnumerationFullMatch = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes') === 'yes'; $excludedGroups = $this->config->getAppValue('core', 'shareapi_exclude_groups', 'no') === 'yes'; // whether to filter out local users @@ -146,7 +147,7 @@ private function filterContacts(IUser $self, $selfUID = $self->getUID(); - return array_values(array_filter($entries, function (IEntry $entry) use ($skipLocal, $ownGroupsOnly, $selfGroups, $selfUID, $disallowEnumeration, $restrictEnumerationGroup, $restrictEnumerationPhone, $filter) { + return array_values(array_filter($entries, function (IEntry $entry) use ($skipLocal, $ownGroupsOnly, $selfGroups, $selfUID, $disallowEnumeration, $restrictEnumerationGroup, $restrictEnumerationPhone, $allowEnumerationFullMatch, $filter) { if ($entry->getProperty('UID') === $selfUID) { return false; } @@ -160,6 +161,10 @@ private function filterContacts(IUser $self, // Prevent enumerating local users if ($disallowEnumeration) { + if (!$allowEnumerationFullMatch) { + return false; + } + $filterUser = true; $mailAddresses = $entry->getEMailAddresses(); diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 6e072740884ad..ce1ec1d60f667 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -1834,6 +1834,10 @@ public function limitEnumerationToPhone(): bool { $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; } + public function allowEnumerationFullMatch(): bool { + return $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes') === 'yes'; + } + /** * Copied from \OC_Util::isSharingDisabledForUser * diff --git a/lib/public/Share/IManager.php b/lib/public/Share/IManager.php index 0c8732b4b15ec..606e64299181c 100644 --- a/lib/public/Share/IManager.php +++ b/lib/public/Share/IManager.php @@ -392,6 +392,14 @@ public function limitEnumerationToGroups(): bool; */ public function limitEnumerationToPhone(): bool; + /** + * Check if user enumeration is allowed to return on full match + * + * @return bool + * @since 21.0.1 + */ + public function allowEnumerationFullMatch(): bool; + /** * Check if sharing is disabled for the given user * diff --git a/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php b/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php index ad83178096e9f..ad201d86a2ad0 100644 --- a/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php +++ b/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php @@ -683,9 +683,12 @@ public function testGetContactsOnlyEnumerateIfPhoneBookOrSameGroupInOwnGroupsOnl } public function testGetContactsWithFilter() { - $this->config->expects($this->at(0))->method('getAppValue') - ->with($this->equalTo('core'), $this->equalTo('shareapi_allow_share_dialog_user_enumeration'), $this->equalTo('yes')) - ->willReturn('no'); + $this->config + ->method('getAppValue') + ->willReturnMap([ + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'no'], + ['core', 'shareapi_restrict_user_enumeration_full_match', 'yes', 'yes'], + ]); /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $user */ $user = $this->createMock(IUser::class); @@ -766,6 +769,90 @@ public function testGetContactsWithFilter() { ], $entry[0]->getEMailAddresses()); } + public function testGetContactsWithFilterWithoutFullMatch() { + $this->config + ->method('getAppValue') + ->willReturnMap([ + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'no'], + ['core', 'shareapi_restrict_user_enumeration_full_match', 'yes', 'no'], + ]); + + /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $user */ + $user = $this->createMock(IUser::class); + $this->contactsManager->expects($this->any()) + ->method('search') + ->willReturn([ + [ + 'UID' => 'a567', + 'FN' => 'Darren Roner', + 'EMAIL' => [ + 'darren@roner.au', + ], + 'isLocalSystemBook' => true, + ], + [ + 'UID' => 'john', + 'FN' => 'John Doe', + 'EMAIL' => [ + 'john@example.com', + ], + 'isLocalSystemBook' => true, + ], + [ + 'FN' => 'Anne D', + 'EMAIL' => [ + 'anne@example.com', + ], + 'isLocalSystemBook' => false, + ], + ]); + $user->expects($this->any()) + ->method('getUID') + ->willReturn('user123'); + + // Complete match on UID should not match + $entry = $this->contactsStore->getContacts($user, 'a567'); + $this->assertSame(1, count($entry)); + $this->assertEquals([ + 'anne@example.com' + ], $entry[0]->getEMailAddresses()); + + // Partial match on UID should not match + $entry = $this->contactsStore->getContacts($user, 'a56'); + $this->assertSame(1, count($entry)); + $this->assertEquals([ + 'anne@example.com' + ], $entry[0]->getEMailAddresses()); + + // Complete match on email should not match + $entry = $this->contactsStore->getContacts($user, 'john@example.com'); + $this->assertSame(1, count($entry)); + $this->assertEquals([ + 'anne@example.com' + ], $entry[0]->getEMailAddresses()); + + // Partial match on email should not match + $entry = $this->contactsStore->getContacts($user, 'john@example.co'); + $this->assertSame(1, count($entry)); + $this->assertEquals([ + 'anne@example.com' + ], $entry[0]->getEMailAddresses()); + + // Match on FN should not match + $entry = $this->contactsStore->getContacts($user, 'Darren Roner'); + $this->assertSame(1, count($entry)); + $this->assertEquals([ + 'anne@example.com' + ], $entry[0]->getEMailAddresses()); + + // Don't filter users in local addressbook + $entry = $this->contactsStore->getContacts($user, 'Anne D'); + $this->assertSame(1, count($entry)); + $this->assertEquals([ + 'anne@example.com' + ], $entry[0]->getEMailAddresses()); + } + public function testFindOneUser() { $this->config->expects($this->at(0))->method('getAppValue') ->with($this->equalTo('core'), $this->equalTo('shareapi_allow_share_dialog_user_enumeration'), $this->equalTo('yes'))