diff options
author | John Molakvoæ <skjnldsv@users.noreply.github.com> | 2021-12-09 10:18:19 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-12-09 10:18:19 +0100 |
commit | 64cd011f47d09adf7724fb384af9039bd3525ba6 (patch) | |
tree | b664e62d3b3e55f9bcb6023c9a07541aca9c1f23 | |
parent | 39f77c8870e7589a87ac0d9e318ee86f18f43b0b (diff) | |
parent | f1a6668cc25f7a51e17c1874d7f040d8a56f1a26 (diff) | |
download | nextcloud-server-64cd011f47d09adf7724fb384af9039bd3525ba6.tar.gz nextcloud-server-64cd011f47d09adf7724fb384af9039bd3525ba6.zip |
Merge pull request #30156 from nextcloud/bugfix/noid/only-wildcard-search-if-enumeration-is-allowed
-rw-r--r-- | apps/dav/lib/CardDAV/AddressBookImpl.php | 2 | ||||
-rw-r--r-- | apps/dav/lib/CardDAV/CardDavBackend.php | 8 | ||||
-rw-r--r-- | apps/federatedfilesharing/lib/Notifier.php | 7 | ||||
-rw-r--r-- | apps/files/lib/Activity/Provider.php | 7 | ||||
-rw-r--r-- | apps/files_sharing/lib/Activity/Providers/Base.php | 7 | ||||
-rw-r--r-- | apps/files_sharing/lib/Controller/ShareAPIController.php | 8 | ||||
-rw-r--r-- | apps/files_sharing/tests/Controller/ShareAPIControllerTest.php | 12 | ||||
-rw-r--r-- | apps/sharebymail/lib/Activity.php | 7 | ||||
-rw-r--r-- | lib/private/Collaboration/Collaborators/MailPlugin.php | 18 | ||||
-rw-r--r-- | lib/private/Collaboration/Collaborators/RemotePlugin.php | 7 | ||||
-rw-r--r-- | lib/private/Contacts/ContactsMenu/ContactsStore.php | 9 | ||||
-rw-r--r-- | lib/private/ContactsManager.php | 24 | ||||
-rw-r--r-- | lib/private/Federation/CloudIdManager.php | 7 | ||||
-rw-r--r-- | lib/private/Share/Share.php | 7 | ||||
-rw-r--r-- | lib/public/Contacts/IManager.php | 4 | ||||
-rw-r--r-- | lib/public/IAddressBook.php | 2 |
16 files changed, 114 insertions, 22 deletions
diff --git a/apps/dav/lib/CardDAV/AddressBookImpl.php b/apps/dav/lib/CardDAV/AddressBookImpl.php index 8b0d494fd01..3db20cb4220 100644 --- a/apps/dav/lib/CardDAV/AddressBookImpl.php +++ b/apps/dav/lib/CardDAV/AddressBookImpl.php @@ -107,6 +107,8 @@ class AddressBookImpl implements IAddressBook { * - 'escape_like_param' - If set to false wildcards _ and % are not escaped * - 'limit' - Set a numeric limit for the search results * - 'offset' - Set the offset for the limited search results + * - 'wildcard' - Whether the search should use wildcards + * @psalm-param array{types?: bool, escape_like_param?: bool, limit?: int, offset?: int, wildcard?: bool} $options * @return array an array of contacts which are arrays of key-value-pairs * example result: * [ diff --git a/apps/dav/lib/CardDAV/CardDavBackend.php b/apps/dav/lib/CardDAV/CardDavBackend.php index 13926ef12ce..3e360fb2e41 100644 --- a/apps/dav/lib/CardDAV/CardDavBackend.php +++ b/apps/dav/lib/CardDAV/CardDavBackend.php @@ -1024,6 +1024,8 @@ class CardDavBackend implements BackendInterface, SyncSupport { * - 'escape_like_param' - If set to false wildcards _ and % are not escaped, otherwise they are * - 'limit' - Set a numeric limit for the search results * - 'offset' - Set the offset for the limited search results + * - 'wildcard' - Whether the search should use wildcards + * @psalm-param array{escape_like_param?: bool, limit?: int, offset?: int, wildcard?: bool} $options * @return array an array of contacts which are arrays of key-value-pairs */ public function search($addressBookId, $pattern, $searchProperties, $options = []): array { @@ -1055,6 +1057,7 @@ class CardDavBackend implements BackendInterface, SyncSupport { * @param string $pattern * @param array $searchProperties * @param array $options + * @psalm-param array{types?: bool, escape_like_param?: bool, limit?: int, offset?: int, wildcard?: bool} $options * @return array */ private function searchByAddressBookIds(array $addressBookIds, @@ -1062,6 +1065,7 @@ class CardDavBackend implements BackendInterface, SyncSupport { array $searchProperties, array $options = []): array { $escapePattern = !\array_key_exists('escape_like_param', $options) || $options['escape_like_param'] !== false; + $useWildcards = !\array_key_exists('wildcard', $options) || $options['wildcard'] !== false; $query2 = $this->db->getQueryBuilder(); @@ -1103,7 +1107,9 @@ class CardDavBackend implements BackendInterface, SyncSupport { // No need for like when the pattern is empty if ('' !== $pattern) { - if (!$escapePattern) { + if (!$useWildcards) { + $query2->andWhere($query2->expr()->eq('cp.value', $query2->createNamedParameter($pattern))); + } elseif (!$escapePattern) { $query2->andWhere($query2->expr()->ilike('cp.value', $query2->createNamedParameter($pattern))); } else { $query2->andWhere($query2->expr()->ilike('cp.value', $query2->createNamedParameter('%' . $this->db->escapeLikeParameter($pattern) . '%'))); diff --git a/apps/federatedfilesharing/lib/Notifier.php b/apps/federatedfilesharing/lib/Notifier.php index fa0119010cf..5e57b77b64a 100644 --- a/apps/federatedfilesharing/lib/Notifier.php +++ b/apps/federatedfilesharing/lib/Notifier.php @@ -255,7 +255,12 @@ class Notifier implements INotifier { } } - $addressBookEntries = $this->contactsManager->search($federatedCloudId, ['CLOUD']); + $addressBookEntries = $this->contactsManager->search($federatedCloudId, ['CLOUD'], [ + 'limit' => 1, + 'enumeration' => false, + 'fullmatch' => false, + 'strict_search' => true, + ]); foreach ($addressBookEntries as $entry) { if (isset($entry['CLOUD'])) { foreach ($entry['CLOUD'] as $cloudID) { diff --git a/apps/files/lib/Activity/Provider.php b/apps/files/lib/Activity/Provider.php index 8f3a5a4f417..a7db85a0de1 100644 --- a/apps/files/lib/Activity/Provider.php +++ b/apps/files/lib/Activity/Provider.php @@ -560,7 +560,12 @@ class Provider implements IProvider { return $this->displayNames[$search]; } - $addressBookContacts = $this->contactsManager->search($search, ['CLOUD']); + $addressBookContacts = $this->contactsManager->search($search, ['CLOUD'], [ + 'limit' => 1, + 'enumeration' => false, + 'fullmatch' => false, + 'strict_search' => true, + ]); foreach ($addressBookContacts as $contact) { if (isset($contact['isLocalSystemBook'])) { continue; diff --git a/apps/files_sharing/lib/Activity/Providers/Base.php b/apps/files_sharing/lib/Activity/Providers/Base.php index 948d629c8fc..e61bdffadcd 100644 --- a/apps/files_sharing/lib/Activity/Providers/Base.php +++ b/apps/files_sharing/lib/Activity/Providers/Base.php @@ -203,7 +203,12 @@ abstract class Base implements IProvider { return $this->displayNames[$search]; } - $addressBookContacts = $this->contactsManager->search($search, ['CLOUD']); + $addressBookContacts = $this->contactsManager->search($search, ['CLOUD'], [ + 'limit' => 1, + 'enumeration' => false, + 'fullmatch' => false, + 'strict_search' => true, + ]); foreach ($addressBookContacts as $contact) { if (isset($contact['isLocalSystemBook'])) { continue; diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 96b105aae49..ff134f61e17 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -334,8 +334,12 @@ class ShareAPIController extends OCSController { * @return string */ private function getDisplayNameFromAddressBook(string $query, string $property): string { - // FIXME: If we inject the contacts manager it gets initialized bofore any address books are registered - $result = \OC::$server->getContactsManager()->search($query, [$property]); + // FIXME: If we inject the contacts manager it gets initialized before any address books are registered + $result = \OC::$server->getContactsManager()->search($query, [$property], [ + 'limit' => 1, + 'enumeration' => false, + 'strict_search' => true, + ]); foreach ($result as $r) { foreach ($r[$property] as $value) { if ($value === $query && $r['FN']) { diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index 86a7d479899..0a837400725 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -4417,7 +4417,11 @@ class ShareAPIControllerTest extends TestCase { $cm->method('search') ->willReturnMap([ - ['user@server.com', ['CLOUD'], [], + ['user@server.com', ['CLOUD'], [ + 'limit' => 1, + 'enumeration' => false, + 'strict_search' => true, + ], [ [ 'CLOUD' => [ @@ -4427,7 +4431,11 @@ class ShareAPIControllerTest extends TestCase { ], ], ], - ['user@server.com', ['EMAIL'], [], + ['user@server.com', ['EMAIL'], [ + 'limit' => 1, + 'enumeration' => false, + 'strict_search' => true, + ], [ [ 'EMAIL' => [ diff --git a/apps/sharebymail/lib/Activity.php b/apps/sharebymail/lib/Activity.php index ff012654834..3992f0f942b 100644 --- a/apps/sharebymail/lib/Activity.php +++ b/apps/sharebymail/lib/Activity.php @@ -362,7 +362,12 @@ class Activity implements IProvider { * @return string */ protected function getContactName($email) { - $addressBookContacts = $this->contactsManager->search($email, ['EMAIL']); + $addressBookContacts = $this->contactsManager->search($email, ['EMAIL'], [ + 'limit' => 1, + 'enumeration' => false, + 'fullmatch' => false, + 'strict_search' => true, + ]); foreach ($addressBookContacts as $contact) { if (isset($contact['isLocalSystemBook'])) { diff --git a/lib/private/Collaboration/Collaborators/MailPlugin.php b/lib/private/Collaboration/Collaborators/MailPlugin.php index 7245501a8bf..c0d0a55a1a1 100644 --- a/lib/private/Collaboration/Collaborators/MailPlugin.php +++ b/lib/private/Collaboration/Collaborators/MailPlugin.php @@ -86,12 +86,7 @@ class MailPlugin implements ISearchPlugin { } /** - * @param $search - * @param $limit - * @param $offset - * @param ISearchResult $searchResult - * @return bool - * @since 13.0.0 + * {@inheritdoc} */ public function search($search, $limit, $offset, ISearchResult $searchResult) { $currentUserId = $this->userSession->getUser()->getUID(); @@ -101,7 +96,16 @@ class MailPlugin implements ISearchPlugin { $emailType = new SearchResultType('emails'); // Search in contacts - $addressBookContacts = $this->contactsManager->search($search, ['EMAIL', 'FN'], ['limit' => $limit, 'offset' => $offset]); + $addressBookContacts = $this->contactsManager->search( + $search, + ['EMAIL', 'FN'], + [ + 'limit' => $limit, + 'offset' => $offset, + 'enumeration' => (bool) $this->shareeEnumeration, + 'fullmatch' => (bool) $this->shareeEnumerationFullMatch, + ] + ); $lowerSearch = strtolower($search); foreach ($addressBookContacts as $contact) { if (isset($contact['EMAIL'])) { diff --git a/lib/private/Collaboration/Collaborators/RemotePlugin.php b/lib/private/Collaboration/Collaborators/RemotePlugin.php index 4fe62523b66..7d7a013a38c 100644 --- a/lib/private/Collaboration/Collaborators/RemotePlugin.php +++ b/lib/private/Collaboration/Collaborators/RemotePlugin.php @@ -67,7 +67,12 @@ class RemotePlugin implements ISearchPlugin { $resultType = new SearchResultType('remotes'); // Search in contacts - $addressBookContacts = $this->contactsManager->search($search, ['CLOUD', 'FN'], ['limit' => $limit, 'offset' => $offset]); + $addressBookContacts = $this->contactsManager->search($search, ['CLOUD', 'FN'], [ + 'limit' => $limit, + 'offset' => $offset, + 'enumeration' => false, + 'fullmatch' => false, + ]); foreach ($addressBookContacts as $contact) { if (isset($contact['isLocalSystemBook'])) { continue; diff --git a/lib/private/Contacts/ContactsMenu/ContactsStore.php b/lib/private/Contacts/ContactsMenu/ContactsStore.php index cd1cc9b6169..a27c2ae455a 100644 --- a/lib/private/Contacts/ContactsMenu/ContactsStore.php +++ b/lib/private/Contacts/ContactsMenu/ContactsStore.php @@ -96,7 +96,10 @@ class ContactsStore implements IContactsStore { * @return IEntry[] */ public function getContacts(IUser $user, $filter, ?int $limit = null, ?int $offset = null) { - $options = []; + $options = [ + 'enumeration' => $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes', + 'fullmatch' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes') === 'yes', + ]; if ($limit !== null) { $options['limit'] = $limit; } @@ -270,7 +273,9 @@ class ContactsStore implements IContactsStore { return null; } - $contacts = $this->contactsManager->search($shareWith, $filter); + $contacts = $this->contactsManager->search($shareWith, $filter, [ + 'strict_search' => true, + ]); $match = null; foreach ($contacts as $contact) { diff --git a/lib/private/ContactsManager.php b/lib/private/ContactsManager.php index e702a439153..937fb94a09a 100644 --- a/lib/private/ContactsManager.php +++ b/lib/private/ContactsManager.php @@ -42,13 +42,35 @@ class ContactsManager implements IManager { * - 'escape_like_param' - If set to false wildcards _ and % are not escaped * - 'limit' - Set a numeric limit for the search results * - 'offset' - Set the offset for the limited search results + * - 'enumeration' - (since 23.0.0) Whether user enumeration on system address book is allowed + * - 'fullmatch' - (since 23.0.0) Whether matching on full detail in system address book is allowed + * - 'strict_search' - (since 23.0.0) Whether the search pattern is full string or partial search + * @psalm-param array{escape_like_param?: bool, limit?: int, offset?: int, enumeration?: bool, fullmatch?: bool, strict_search?: bool} $options * @return array an array of contacts which are arrays of key-value-pairs */ public function search($pattern, $searchProperties = [], $options = []) { $this->loadAddressBooks(); $result = []; foreach ($this->addressBooks as $addressBook) { - $r = $addressBook->search($pattern, $searchProperties, $options); + $searchOptions = $options; + $strictSearch = array_key_exists('strict_search', $options) && $options['strict_search'] === true; + + if ($addressBook->isSystemAddressBook()) { + $fullMatch = !\array_key_exists('fullmatch', $options) || $options['fullmatch'] !== false; + if (!$fullMatch) { + // Neither full match is allowed, so skip the system address book + continue; + } + if ($strictSearch) { + $searchOptions['wildcard'] = false; + } else { + $searchOptions['wildcard'] = !\array_key_exists('enumeration', $options) || $options['enumeration'] !== false; + } + } else { + $searchOptions['wildcard'] = !$strictSearch; + } + + $r = $addressBook->search($pattern, $searchProperties, $searchOptions); $contacts = []; foreach ($r as $c) { $c['addressbook-key'] = $addressBook->getKey(); diff --git a/lib/private/Federation/CloudIdManager.php b/lib/private/Federation/CloudIdManager.php index 24437456fd0..77bb9437ba2 100644 --- a/lib/private/Federation/CloudIdManager.php +++ b/lib/private/Federation/CloudIdManager.php @@ -90,7 +90,12 @@ class CloudIdManager implements ICloudIdManager { } protected function getDisplayNameFromContact(string $cloudId): ?string { - $addressBookEntries = $this->contactsManager->search($cloudId, ['CLOUD']); + $addressBookEntries = $this->contactsManager->search($cloudId, ['CLOUD'], [ + 'limit' => 1, + 'enumeration' => false, + 'fullmatch' => false, + 'strict_search' => true, + ]); foreach ($addressBookEntries as $entry) { if (isset($entry['CLOUD'])) { foreach ($entry['CLOUD'] as $cloudID) { diff --git a/lib/private/Share/Share.php b/lib/private/Share/Share.php index a857a850f9b..548c8a2c451 100644 --- a/lib/private/Share/Share.php +++ b/lib/private/Share/Share.php @@ -593,7 +593,12 @@ class Share extends Constants { $row['share_with_displayname'] = $shareWithUser === null ? $row['share_with'] : $shareWithUser->getDisplayName(); } elseif (isset($row['share_with']) && $row['share_with'] != '' && $row['share_type'] === IShare::TYPE_REMOTE) { - $addressBookEntries = \OC::$server->getContactsManager()->search($row['share_with'], ['CLOUD']); + $addressBookEntries = \OC::$server->getContactsManager()->search($row['share_with'], ['CLOUD'], [ + 'limit' => 1, + 'enumeration' => false, + 'fullmatch' => false, + 'strict_search' => true, + ]); foreach ($addressBookEntries as $entry) { foreach ($entry['CLOUD'] as $cloudID) { if ($cloudID === $row['share_with']) { diff --git a/lib/public/Contacts/IManager.php b/lib/public/Contacts/IManager.php index 8d24249e997..e9bdc01c060 100644 --- a/lib/public/Contacts/IManager.php +++ b/lib/public/Contacts/IManager.php @@ -93,6 +93,10 @@ interface IManager { * - 'escape_like_param' - If set to false wildcards _ and % are not escaped * - 'limit' - Set a numeric limit for the search results * - 'offset' - Set the offset for the limited search results + * - 'enumeration' - (since 23.0.0) Whether user enumeration on system address book is allowed + * - 'fullmatch' - (since 23.0.0) Whether matching on full detail in system addresss book is allowed + * - 'strict_search' - (since 23.0.0) Whether the search pattern is full string or partial search + * @psalm-param array{escape_like_param?: bool, limit?: int, offset?: int, enumeration?: bool, fullmatch?: bool, strict_search?: bool} $options * @return array an array of contacts which are arrays of key-value-pairs * @since 6.0.0 */ diff --git a/lib/public/IAddressBook.php b/lib/public/IAddressBook.php index b0196764be3..4bb632ae070 100644 --- a/lib/public/IAddressBook.php +++ b/lib/public/IAddressBook.php @@ -67,6 +67,8 @@ namespace OCP { * - 'escape_like_param' - If set to false wildcards _ and % are not escaped * - 'limit' - Set a numeric limit for the search results * - 'offset' - Set the offset for the limited search results + * - 'wildcard' - (since 23.0.0) Whether the search should use wildcards + * @psalm-param array{types?: bool, escape_like_param?: bool, limit?: int, offset?: int, wildcard?: bool} $options * @return array an array of contacts which are arrays of key-value-pairs * example result: * [ |