diff --git a/lib/Controller/ContactIntegrationController.php b/lib/Controller/ContactIntegrationController.php index 87ab098339..d3cfebd800 100644 --- a/lib/Controller/ContactIntegrationController.php +++ b/lib/Controller/ContactIntegrationController.php @@ -32,13 +32,16 @@ class ContactIntegrationController extends Controller { private ContactIntegrationService $service; + private string $uid; public function __construct(string $appName, IRequest $request, - ContactIntegrationService $service) { + ContactIntegrationService $service, + string $userId) { parent::__construct($appName, $request); $this->service = $service; + $this->uid = $userId; } /** @@ -49,7 +52,7 @@ public function __construct(string $appName, */ #[TrapError] public function match(string $mail): JSONResponse { - return (new JSONResponse($this->service->findMatches($mail)))->cacheFor(60 * 60, false, true); + return (new JSONResponse($this->service->findMatches($this->uid, $mail)))->cacheFor(60 * 60, false, true); } /** @@ -88,7 +91,7 @@ public function newContact(?string $contactName = null, ?string $mail = null): J */ #[TrapError] public function autoComplete(string $term): JSONResponse { - $res = $this->service->autoComplete($term); + $res = $this->service->autoComplete($this->uid, $term); return (new JSONResponse($res))->cacheFor(60 * 60, false, true); } } diff --git a/lib/Service/ContactIntegration/ContactIntegrationService.php b/lib/Service/ContactIntegration/ContactIntegrationService.php index f60141cbcc..9324410d2b 100644 --- a/lib/Service/ContactIntegration/ContactIntegrationService.php +++ b/lib/Service/ContactIntegration/ContactIntegrationService.php @@ -33,8 +33,8 @@ public function __construct(ContactsIntegration $ci) { $this->contactsIntegration = $ci; } - public function findMatches(string $mail): array { - $matches = $this->contactsIntegration->getContactsWithMail($mail); + public function findMatches(string $uid, string $mail): array { + $matches = $this->contactsIntegration->getContactsWithMail($uid, $mail); return $matches; } @@ -46,7 +46,7 @@ public function newContact(string $name, string $mail): ?array { return $this->contactsIntegration->newContact($name, $mail); } - public function autoComplete(string $term): array { - return $this->contactsIntegration->getContactsWithName($term); + public function autoComplete(string $uid, string $term): array { + return $this->contactsIntegration->getContactsWithName($uid, $term); } } diff --git a/lib/Service/ContactsIntegration.php b/lib/Service/ContactsIntegration.php index 0c291db1e9..1c8f215b0c 100644 --- a/lib/Service/ContactsIntegration.php +++ b/lib/Service/ContactsIntegration.php @@ -28,6 +28,7 @@ use OCP\IConfig; use OCP\IGroupManager; use OCP\IUserManager; +use function is_array; class ContactsIntegration { /** @var IManager */ @@ -60,55 +61,9 @@ public function __construct(IManager $contactsManager, * @return array */ public function getMatchingRecipient(string $userId, string $term): array { - if (!$this->contactsManager->isEnabled()) { - return []; - } - - // If 'Allow username autocompletion in share dialog' is disabled in the admin sharing settings, then we must not - // auto-complete system users - $shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'no') === 'yes'; - $shareeEnumerationInGroupOnly = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; - $shareeEnumerationFullMatch = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes') === 'yes'; - $shareeEnumerationFullMatchUserId = $shareeEnumerationFullMatch && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_userid', 'yes') === 'yes'; - $shareeEnumerationFullMatchEmail = $shareeEnumerationFullMatch && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_email', 'yes') === 'yes'; - - $result = $this->contactsManager->search( - $term, - ['UID', 'FN', 'EMAIL'], - [ - 'enumeration' => $shareeEnumeration, - 'fullmatch' => $shareeEnumerationFullMatch, - 'limit' => 20, - ], - ); - if (empty($result)) { - return []; - } + $result = $this->search($userId, $term, ['UID', 'FN', 'EMAIL']); $receivers = []; - - if ($shareeEnumeration && $shareeEnumerationInGroupOnly) { - $user = $this->userManager->get($userId); - if ($user === null) { - return []; - } - $userGroups = $this->groupManager->getUserGroupIds($user); - } - foreach ($result as $r) { - $isSystemUser = isset($r['isLocalSystemBook']) && $r['isLocalSystemBook']; - $isInSameGroup = false; - if ($isSystemUser && $shareeEnumerationInGroupOnly) { - foreach ($userGroups as $userGroup) { - if ($this->groupManager->isInGroup($r['UID'], $userGroup)) { - $isInSameGroup = true; - break; - } - } - if (!$shareeEnumerationFullMatch && !$isInSameGroup) { - continue; - } - } - $id = $r['UID']; $fn = $r['FN'] ?? null; if (!isset($r['EMAIL'])) { @@ -125,23 +80,10 @@ public function getMatchingRecipient(string $userId, string $term): array { if ($e === '') { continue; } - $lowerTerm = strtolower($term); - - if ($isSystemUser && $shareeEnumerationInGroupOnly && !$isInSameGroup) { - // Check for full match. If full match is disabled, matching results already filtered out - if (!($lowerTerm !== '' && ( - ($shareeEnumerationFullMatch && !empty($fn) && $lowerTerm === strtolower($fn)) || - ($shareeEnumerationFullMatchUserId && $lowerTerm === strtolower($id)) || - ($shareeEnumerationFullMatchEmail && $lowerTerm === strtolower($e))))) { - // Not a full Match - continue; - } - } - $receivers[] = [ 'id' => $id, // Show full name if possible or fall back to email - 'label' => $fn, + 'label' => $fn ?? $e, 'email' => $e, 'photo' => $photo, 'source' => 'contacts', @@ -243,26 +185,105 @@ public function newContact(string $name, string $mailAddr, string $type = 'HOME' return $createdContact; } + private function search(string $userId, string $term, array $fields, ?bool $strictSearch = null): array { + if (!$this->contactsManager->isEnabled()) { + return []; + } + + // If 'Allow username autocompletion in share dialog' is disabled in the admin sharing settings, then we must not + // auto-complete system users + $shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes'; + $shareeEnumerationInGroupOnly = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; + $shareeEnumerationFullMatch = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes') === 'yes'; + $shareeEnumerationFullMatchDisplayName = $shareeEnumerationFullMatch && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_displayname', 'yes') === 'yes'; + $shareeEnumerationFullMatchUserId = $shareeEnumerationFullMatch && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_userid', 'yes') === 'yes'; + $shareeEnumerationFullMatchEmail = $shareeEnumerationFullMatch && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_email', 'yes') === 'yes'; + + $options = [ + 'enumeration' => $shareeEnumeration, + 'fullmatch' => $shareeEnumerationFullMatch, + 'limit' => 20, + ]; + if ($strictSearch !== null) { + $options['strict_search'] = $strictSearch; + } + + $result = $this->contactsManager->search( + $term, + $fields, + $options, + ); + + $userGroups = []; + if ($shareeEnumeration && $shareeEnumerationInGroupOnly) { + $user = $this->userManager->get($userId); + if ($user === null) { + return []; + } + $userGroups = $this->groupManager->getUserGroupIds($user); + } + + $filteredResults = []; + foreach ($result as $r) { + $isSystemUser = isset($r['isLocalSystemBook']) && $r['isLocalSystemBook']; + $isInSameGroup = false; + if ($isSystemUser && $shareeEnumerationInGroupOnly) { + foreach ($userGroups as $userGroup) { + if ($this->groupManager->isInGroup($r['UID'], $userGroup)) { + $isInSameGroup = true; + break; + } + } + if (!$shareeEnumerationFullMatch && !$isInSameGroup) { + continue; + } + } + + if ($isSystemUser && $shareeEnumerationInGroupOnly && !$isInSameGroup && $shareeEnumerationFullMatch) { + // Check for full match. If full match is disabled, non-matching results already filtered out above. + $id = $r['UID']; + $fn = $r['FN'] ?? null; + $lowerTerm = strtolower($term); + $isMatch = ($lowerTerm !== '' && ( + ($shareeEnumerationFullMatchDisplayName && !empty($fn) && $lowerTerm === strtolower($fn)) + || ($shareeEnumerationFullMatchUserId && $lowerTerm === strtolower($id)))) ; + if ($shareeEnumerationFullMatchEmail && !$isMatch) { + $email = $r['EMAIL'] ?? null; + if ($email === null) { + continue; + } + $emails = is_array($email) ? $email : [$email]; + foreach ($emails as $e) { + if ($lowerTerm === strtolower($e)) { + $isMatch = true; + break; + } + } + } + if (!$isMatch) { + continue; + } + } + + $filteredResults[] = $r; + } + return $filteredResults; + } + /** * @param string[] $fields */ - private function doSearch(string $term, array $fields, bool $strictSearch): array { - $allowSystemUsers = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'no') === 'yes'; - - $result = $this->contactsManager->search($term, $fields, [ - 'strict_search' => $strictSearch, - 'limit' => 20, - ]); + private function doSearch(string $userId, string $term, array $fields, bool $strictSearch) : array { + $result = $this->search($userId, $term, $fields, $strictSearch); $matches = []; foreach ($result as $r) { - if (!$allowSystemUsers && isset($r['isLocalSystemBook']) && $r['isLocalSystemBook']) { - continue; - } $id = $r['UID']; $fn = $r['FN']; + $email = $r['EMAIL'] ?? null; $matches[] = [ 'id' => $id, 'label' => $fn, + 'email' => $email, ]; } return $matches; @@ -270,18 +291,15 @@ private function doSearch(string $term, array $fields, bool $strictSearch): arra /** * Extracts all Contacts with the specified mail address - * - * @param string $mailAddr - * @return array */ - public function getContactsWithMail(string $mailAddr) { - return $this->doSearch($mailAddr, ['EMAIL'], true); + public function getContactsWithMail(string $userId, string $mailAddr): array { + return $this->doSearch($userId, $mailAddr, ['EMAIL'], true); } /** * Extracts all Contacts with the specified name */ - public function getContactsWithName(string $name): array { - return $this->doSearch($name, ['FN'], false); + public function getContactsWithName(string $userId, string $name): array { + return $this->doSearch($userId, $name, ['FN'], false); } } diff --git a/tests/Unit/Service/ContactsIntegrationTest.php b/tests/Unit/Service/ContactsIntegrationTest.php index 348285e3bb..b48f49d00f 100644 --- a/tests/Unit/Service/ContactsIntegrationTest.php +++ b/tests/Unit/Service/ContactsIntegrationTest.php @@ -356,22 +356,24 @@ public function testGetMatchingRecipientRestrictedToGroupFullMatchFalse() { $this->assertEquals($expected, $actual); } - public function common($term, $searchResult, $allowSystemUsers, $allowSystemUsersInGroupOnly, $shareeEnumerationFullMatch, $shareeEnumerationFullMatchUserId = true, $shareeEnumerationFullMatchEmail = true) { + public function common($term, $searchResult, $allowSystemUsers, $allowSystemUsersInGroupOnly, $shareeEnumerationFullMatch, $shareeEnumerationFullMatchDisplayName = true, $shareeEnumerationFullMatchUserId = true, $shareeEnumerationFullMatchEmail = true) { $this->config->expects(self::atLeast(3)) ->method('getAppValue') ->withConsecutive( - ['core', 'shareapi_allow_share_dialog_user_enumeration', 'no'], + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes'], ['core', 'shareapi_restrict_user_enumeration_to_group', 'no'], ['core', 'shareapi_restrict_user_enumeration_full_match', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_full_match_displayname', 'yes'], ['core', 'shareapi_restrict_user_enumeration_full_match_userid', 'yes'], ['core', 'shareapi_restrict_user_enumeration_full_match_email', 'yes'], ) ->willReturnOnConsecutiveCalls( - $allowSystemUsers ? "yes" : " no", - $allowSystemUsersInGroupOnly ? "yes" : " no", - $shareeEnumerationFullMatch ? "yes" : " no", - $shareeEnumerationFullMatchUserId ? "yes" : "no", - $shareeEnumerationFullMatchEmail ? "yes" : " no"); + $allowSystemUsers ? 'yes' : 'no', + $allowSystemUsersInGroupOnly ? 'yes' : 'no', + $shareeEnumerationFullMatch ? 'yes' : 'no', + $shareeEnumerationFullMatchDisplayName ? 'yes' : 'no', + $shareeEnumerationFullMatchUserId ? 'yes' : 'no', + $shareeEnumerationFullMatchEmail ? 'yes' : 'no'); $this->contactsManager->expects($this->once()) ->method('isEnabled') ->will($this->returnValue(true)); @@ -418,6 +420,178 @@ public function getPhotoDataProvider() { ]; } + public function testGetContactsWithName(): void { + $name = 'John'; + $searchResult = [ + [ + 'UID' => 'jd', + 'FN' => 'John Doe', + 'EMAIL' => 'john@doe.com', + ], + [ + 'UID' => 'js', + 'FN' => 'John Smith', + 'EMAIL' => ['john@smith.com', 'jsmith@example.com'], + ], + ]; + + $this->contactsManager->expects($this->once()) + ->method('isEnabled') + ->willReturn(true); + $this->config->expects(self::exactly(3)) + ->method('getAppValue') + ->withConsecutive( + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no'], + ['core', 'shareapi_restrict_user_enumeration_full_match', 'yes'], + ) + ->willReturnOnConsecutiveCalls('yes', 'no', 'no'); + $this->contactsManager->expects($this->once()) + ->method('search') + ->with($name, ['FN'], [ + 'enumeration' => true, + 'fullmatch' => false, + 'strict_search' => false, + 'limit' => 20, + ]) + ->willReturn($searchResult); + + $expected = [ + [ + 'id' => 'jd', + 'label' => 'John Doe', + 'email' => 'john@doe.com', + ], + [ + 'id' => 'js', + 'label' => 'John Smith', + 'email' => ['john@smith.com', 'jsmith@example.com'], + ], + ]; + + $actual = $this->contactsIntegration->getContactsWithName('currentUser', $name); + + $this->assertEquals($expected, $actual); + } + + public function testGetContactsWithNameFiltersSystemUsersWhenDisabled(): void { + $name = 'John'; + // When enumeration is disabled the contacts manager filters system users; + // only non-system contacts are returned. + $searchResult = [ + [ + 'UID' => 'js', + 'FN' => 'John Smith', + 'EMAIL' => 'john@smith.com', + ], + ]; + + $this->contactsManager->expects($this->once()) + ->method('isEnabled') + ->willReturn(true); + $this->config->expects(self::exactly(3)) + ->method('getAppValue') + ->withConsecutive( + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no'], + ['core', 'shareapi_restrict_user_enumeration_full_match', 'yes'], + ) + ->willReturnOnConsecutiveCalls('no', 'no', 'no'); + $this->contactsManager->expects($this->once()) + ->method('search') + ->with($name, ['FN'], [ + 'enumeration' => false, + 'fullmatch' => false, + 'strict_search' => false, + 'limit' => 20, + ]) + ->willReturn($searchResult); + + $expected = [ + [ + 'id' => 'js', + 'label' => 'John Smith', + 'email' => 'john@smith.com', + ], + ]; + + $actual = $this->contactsIntegration->getContactsWithName('currentUser', $name); + + $this->assertEquals($expected, $actual); + } + + public function testGetContactsWithNameNoResults(): void { + $name = 'Nonexistent'; + + $this->contactsManager->expects($this->once()) + ->method('isEnabled') + ->willReturn(true); + $this->config->expects(self::exactly(3)) + ->method('getAppValue') + ->withConsecutive( + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no'], + ['core', 'shareapi_restrict_user_enumeration_full_match', 'yes'], + ) + ->willReturnOnConsecutiveCalls('yes', 'no', 'no'); + $this->contactsManager->expects($this->once()) + ->method('search') + ->with($name, ['FN'], [ + 'enumeration' => true, + 'fullmatch' => false, + 'strict_search' => false, + 'limit' => 20, + ]) + ->willReturn([]); + + $actual = $this->contactsIntegration->getContactsWithName('currentUser', $name); + + $this->assertEquals([], $actual); + } + + public function testGetContactsWithNameNoEmail(): void { + $name = 'John'; + $searchResult = [ + [ + 'UID' => 'jd', + 'FN' => 'John Doe', + ], + ]; + + $this->contactsManager->expects($this->once()) + ->method('isEnabled') + ->willReturn(true); + $this->config->expects(self::exactly(3)) + ->method('getAppValue') + ->withConsecutive( + ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes'], + ['core', 'shareapi_restrict_user_enumeration_to_group', 'no'], + ['core', 'shareapi_restrict_user_enumeration_full_match', 'yes'], + ) + ->willReturnOnConsecutiveCalls('yes', 'no', 'no'); + $this->contactsManager->expects($this->once()) + ->method('search') + ->with($name, ['FN'], [ + 'enumeration' => true, + 'fullmatch' => false, + 'strict_search' => false, + 'limit' => 20, + ]) + ->willReturn($searchResult); + + $expected = [ + [ + 'id' => 'jd', + 'label' => 'John Doe', + 'email' => null, + ], + ]; + + $actual = $this->contactsIntegration->getContactsWithName('currentUser', $name); + + $this->assertEquals($expected, $actual); + } + /** * @dataProvider getPhotoDataProvider */