Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable22] Only wildcard search if enumeration is allowed #30247

Merged
merged 7 commits into from
Dec 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions apps/dav/lib/CardDAV/AddressBookImpl.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ public function getDisplayName() {
* - '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:
* [
Expand Down
8 changes: 7 additions & 1 deletion apps/dav/lib/CardDAV/CardDavBackend.php
Original file line number Diff line number Diff line change
Expand Up @@ -1024,6 +1024,8 @@ public function updateShares(IShareable $shareable, $add, $remove) {
* - '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 {
Expand Down Expand Up @@ -1055,13 +1057,15 @@ public function searchPrincipalUri(string $principalUri,
* @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,
string $pattern,
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();

Expand Down Expand Up @@ -1103,7 +1107,9 @@ private function searchByAddressBookIds(array $addressBookIds,

// 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) . '%')));
Expand Down
7 changes: 6 additions & 1 deletion apps/federatedfilesharing/lib/Notifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,12 @@ protected function getDisplayNameFromContact($federatedCloudId) {
}
}

$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) {
Expand Down
7 changes: 6 additions & 1 deletion apps/files/lib/Activity/Provider.php
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,12 @@ protected function getDisplayNameFromAddressBook(string $search): string {
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;
Expand Down
7 changes: 6 additions & 1 deletion apps/files_sharing/lib/Activity/Providers/Base.php
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,12 @@ protected function getDisplayNameFromAddressBook(string $search): string {
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;
Expand Down
8 changes: 6 additions & 2 deletions apps/files_sharing/lib/Controller/ShareAPIController.php
Original file line number Diff line number Diff line change
Expand Up @@ -334,8 +334,12 @@ protected function formatShare(IShare $share, Node $recipientNode = null): array
* @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']) {
Expand Down
12 changes: 10 additions & 2 deletions apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4417,7 +4417,11 @@ public function testFormatShare(array $expects, \OCP\Share\IShare $share, array

$cm->method('search')
->willReturnMap([
['user@server.com', ['CLOUD'], [],
['user@server.com', ['CLOUD'], [
'limit' => 1,
'enumeration' => false,
'strict_search' => true,
],
[
[
'CLOUD' => [
Expand All @@ -4427,7 +4431,11 @@ public function testFormatShare(array $expects, \OCP\Share\IShare $share, array
],
],
],
['user@server.com', ['EMAIL'], [],
['user@server.com', ['EMAIL'], [
'limit' => 1,
'enumeration' => false,
'strict_search' => true,
],
[
[
'EMAIL' => [
Expand Down
7 changes: 6 additions & 1 deletion apps/sharebymail/lib/Activity.php
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,12 @@ protected function generateUserParameter($uid) {
* @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'])) {
Expand Down
18 changes: 11 additions & 7 deletions lib/private/Collaboration/Collaborators/MailPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,7 @@ public function __construct(IManager $contactsManager,
}

/**
* @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();
Expand All @@ -101,7 +96,16 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) {
$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'])) {
Expand Down
7 changes: 6 additions & 1 deletion lib/private/Collaboration/Collaborators/RemotePlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,12 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) {
$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;
Expand Down
9 changes: 7 additions & 2 deletions lib/private/Contacts/ContactsMenu/ContactsStore.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,10 @@ public function __construct(IManager $contactsManager,
* @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;
}
Expand Down Expand Up @@ -242,7 +245,9 @@ public function findOne(IUser $user, $shareType, $shareWith) {
}

$userId = $user->getUID();
$allContacts = $this->contactsManager->search($shareWith, $filter);
$allContacts = $this->contactsManager->search($shareWith, $filter, [
'strict_search' => true,
]);
$contacts = array_filter($allContacts, function ($contact) use ($userId) {
return $contact['UID'] !== $userId;
});
Expand Down
24 changes: 23 additions & 1 deletion lib/private/ContactsManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
7 changes: 6 additions & 1 deletion lib/private/Federation/CloudIdManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,12 @@ public function resolveCloudId(string $cloudId): ICloudId {
}

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) {
Expand Down
7 changes: 6 additions & 1 deletion lib/private/Share/Share.php
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,12 @@ public static function getItems($itemType, $item = null, $shareType = null, $sha
$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']) {
Expand Down
4 changes: 4 additions & 0 deletions lib/public/Contacts/IManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
2 changes: 2 additions & 0 deletions lib/public/IAddressBook.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ public function getDisplayName();
* - '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:
* [
Expand Down