diff --git a/lib/private/Collaboration/Collaborators/MailByMailPlugin.php b/lib/private/Collaboration/Collaborators/MailByMailPlugin.php index 5c4156c6e1936..476108db8883b 100644 --- a/lib/private/Collaboration/Collaborators/MailByMailPlugin.php +++ b/lib/private/Collaboration/Collaborators/MailByMailPlugin.php @@ -13,6 +13,7 @@ use OCP\Federation\ICloudIdManager; use OCP\IConfig; use OCP\IGroupManager; +use OCP\IUserManager; use OCP\IUserSession; use OCP\Mail\IEmailValidator; use OCP\Share\IShare; @@ -30,6 +31,7 @@ public function __construct( KnownUserService $knownUserService, IUserSession $userSession, IEmailValidator $emailValidator, + IUserManager $userManager, mixed $shareWithGroupOnlyExcludeGroupsList = [], ) { parent::__construct( @@ -40,6 +42,7 @@ public function __construct( $knownUserService, $userSession, $emailValidator, + $userManager, $shareWithGroupOnlyExcludeGroupsList, IShare::TYPE_EMAIL, ); diff --git a/lib/private/Collaboration/Collaborators/MailPlugin.php b/lib/private/Collaboration/Collaborators/MailPlugin.php index 3149a4d14f964..1fe8fbf2a8e13 100644 --- a/lib/private/Collaboration/Collaborators/MailPlugin.php +++ b/lib/private/Collaboration/Collaborators/MailPlugin.php @@ -4,6 +4,7 @@ * SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later */ + namespace OC\Collaboration\Collaborators; use OC\KnownUser\KnownUserService; @@ -16,9 +17,11 @@ use OCP\IConfig; use OCP\IGroupManager; use OCP\IUser; +use OCP\IUserManager; use OCP\IUserSession; use OCP\Mail\IEmailValidator; use OCP\Share\IShare; +use RuntimeException; class MailPlugin implements ISearchPlugin { protected bool $shareWithGroupOnly; @@ -41,6 +44,7 @@ public function __construct( private KnownUserService $knownUserService, private IUserSession $userSession, private IEmailValidator $emailValidator, + private IUserManager $userManager, private mixed $shareWithGroupOnlyExcludeGroupsList, private int $shareType, ) { @@ -66,23 +70,29 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b } // Extract the email address from "Foo Bar " and then search with "foo.bar@example.tld" instead - $result = preg_match('/<([^@]+@.+)>$/', $search, $matches); - if ($result && filter_var($matches[1], FILTER_VALIDATE_EMAIL)) { - return $this->search($matches[1], $limit, $offset, $searchResult); + if (preg_match('/<([^@]+@.+)>$/', $search, $matches) && filter_var($matches[1], FILTER_VALIDATE_EMAIL)) { + $search = $matches[1]; } $currentUserId = $this->userSession->getUser()->getUID(); + $userGroups = $this->groupManager->getUserGroupIds($this->userSession->getUser()); - $result = $userResults = ['wide' => [], 'exact' => []]; - $userType = new SearchResultType('users'); - $emailType = new SearchResultType('emails'); + $hasMore = false; + $count = 0; + $results = ['wide' => [], 'exact' => []]; + $type = match ($this->shareType) { + IShare::TYPE_USER => new SearchResultType('users'), + IShare::TYPE_EMAIL => new SearchResultType('emails'), + default => throw new RuntimeException(), + }; // Search in contacts $addressBookContacts = $this->contactsManager->search( $search, ['EMAIL', 'FN'], [ - 'limit' => $limit, + // We request one more, so we can check if there are more results available + 'limit' => $limit + 1, 'offset' => $offset, 'enumeration' => $this->shareeEnumeration, 'fullmatch' => $this->shareeEnumerationFullMatch, @@ -90,59 +100,91 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b ); $lowerSearch = strtolower($search); foreach ($addressBookContacts as $contact) { - if (isset($contact['EMAIL'])) { - $emailAddresses = $contact['EMAIL']; - if (\is_string($emailAddresses)) { - $emailAddresses = [$emailAddresses]; + if (!isset($contact['EMAIL'])) { + continue; + } + + $emailAddresses = $contact['EMAIL']; + if (\is_string($emailAddresses)) { + $emailAddresses = [$emailAddresses]; + } + foreach ($emailAddresses as $emailAddress) { + $displayName = $emailAddress; + $emailAddressType = null; + if (\is_array($emailAddress)) { + $emailAddressData = $emailAddress; + $emailAddress = $emailAddressData['value']; + $emailAddressType = $emailAddressData['type']; + } + + if (!filter_var($emailAddress, FILTER_VALIDATE_EMAIL)) { + continue; } - foreach ($emailAddresses as $type => $emailAddress) { - $displayName = $emailAddress; - $emailAddressType = null; - if (\is_array($emailAddress)) { - $emailAddressData = $emailAddress; - $emailAddress = $emailAddressData['value']; - $emailAddressType = $emailAddressData['type']; + + if (isset($contact['FN'])) { + $displayName = $contact['FN'] . ' (' . $emailAddress . ')'; + } + $exactEmailMatch = strtolower($emailAddress) === $lowerSearch; + + if (isset($contact['isLocalSystemBook'])) { + $contactUser = $this->userManager->get($contact['UID']); + if ($contactUser === null) { + continue; } - if (!filter_var($emailAddress, FILTER_VALIDATE_EMAIL)) { + $contactGroups = $this->groupManager->getUserGroupIds($contactUser); + if ($this->shareWithGroupOnly && array_intersect($contactGroups, array_diff($userGroups, $this->shareWithGroupOnlyExcludeGroupsList)) === []) { continue; } - if (isset($contact['FN'])) { - $displayName = $contact['FN'] . ' (' . $emailAddress . ')'; + if ($exactEmailMatch && $this->shareeEnumerationFullMatch) { + try { + $cloud = $this->cloudIdManager->resolveCloudId($contact['CLOUD'][0] ?? ''); + } catch (\InvalidArgumentException $e) { + continue; + } + + if ($this->shareType === IShare::TYPE_USER && !$this->isCurrentUser($cloud) && !$searchResult->hasResult($type, $cloud->getUser())) { + $singleResult = [[ + 'label' => $displayName, + 'uuid' => $contact['UID'] ?? $emailAddress, + 'name' => $contact['FN'] ?? $displayName, + 'value' => [ + 'shareType' => IShare::TYPE_USER, + 'shareWith' => $cloud->getUser(), + ], + 'shareWithDisplayNameUnique' => !empty($emailAddress) ? $emailAddress : $cloud->getUser() + ]]; + $searchResult->addResultSet($type, [], $singleResult); + $searchResult->markExactIdMatch($type); + } + return false; } - $exactEmailMatch = strtolower($emailAddress) === $lowerSearch; - - if (isset($contact['isLocalSystemBook'])) { - if ($this->shareWithGroupOnly) { - /* - * Check if the user may share with the user associated with the e-mail of the just found contact - */ - $userGroups = $this->groupManager->getUserGroupIds($this->userSession->getUser()); - - // ShareWithGroupOnly filtering - $userGroups = array_diff($userGroups, $this->shareWithGroupOnlyExcludeGroupsList); - - $found = false; - foreach ($userGroups as $userGroup) { - if ($this->groupManager->isInGroup($contact['UID'], $userGroup)) { - $found = true; - break; - } - } - if (!$found) { + + if ($this->shareeEnumeration && $this->shareType === IShare::TYPE_USER) { + try { + if (!isset($contact['CLOUD'])) { continue; } + $cloud = $this->cloudIdManager->resolveCloudId($contact['CLOUD'][0] ?? ''); + } catch (\InvalidArgumentException $e) { + continue; } - if ($exactEmailMatch && $this->shareeEnumerationFullMatch) { - try { - $cloud = $this->cloudIdManager->resolveCloudId($contact['CLOUD'][0] ?? ''); - } catch (\InvalidArgumentException $e) { - continue; - } + $addToWide = !($this->shareeEnumerationInGroupOnly || $this->shareeEnumerationPhone); - if ($this->shareType === IShare::TYPE_USER && !$this->isCurrentUser($cloud) && !$searchResult->hasResult($userType, $cloud->getUser())) { - $singleResult = [[ + if (!$addToWide && $this->shareeEnumerationPhone && $this->knownUserService->isKnownToUser($currentUserId, $contact['UID'])) { + $addToWide = true; + } + + if (!$addToWide && $this->shareeEnumerationInGroupOnly) { + $addToWide = array_intersect($contactGroups, $userGroups) !== []; + } + + if ($addToWide && !$this->isCurrentUser($cloud) && !$searchResult->hasResult($type, $cloud->getUser())) { + if ($count++ >= $limit) { + $hasMore = true; + } else { + $results['wide'][] = [ 'label' => $displayName, 'uuid' => $contact['UID'] ?? $emailAddress, 'name' => $contact['FN'] ?? $displayName, @@ -151,68 +193,23 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b 'shareWith' => $cloud->getUser(), ], 'shareWithDisplayNameUnique' => !empty($emailAddress) ? $emailAddress : $cloud->getUser() - - ]]; - $searchResult->addResultSet($userType, [], $singleResult); - $searchResult->markExactIdMatch($emailType); + ]; } - return false; } - - if ($this->shareeEnumeration) { - try { - if (!isset($contact['CLOUD'])) { - continue; - } - $cloud = $this->cloudIdManager->resolveCloudId($contact['CLOUD'][0] ?? ''); - } catch (\InvalidArgumentException $e) { - continue; - } - - $addToWide = !($this->shareeEnumerationInGroupOnly || $this->shareeEnumerationPhone); - if (!$addToWide && $this->shareeEnumerationPhone && $this->knownUserService->isKnownToUser($currentUserId, $contact['UID'])) { - $addToWide = true; - } - - if (!$addToWide && $this->shareeEnumerationInGroupOnly) { - $addToWide = false; - $userGroups = $this->groupManager->getUserGroupIds($this->userSession->getUser()); - foreach ($userGroups as $userGroup) { - if ($this->groupManager->isInGroup($contact['UID'], $userGroup)) { - $addToWide = true; - break; - } - } - } - if ($addToWide && !$this->isCurrentUser($cloud) && !$searchResult->hasResult($userType, $cloud->getUser())) { - if ($this->shareType === IShare::TYPE_USER) { - $userResults['wide'][] = [ - 'label' => $displayName, - 'uuid' => $contact['UID'] ?? $emailAddress, - 'name' => $contact['FN'] ?? $displayName, - 'value' => [ - 'shareType' => IShare::TYPE_USER, - 'shareWith' => $cloud->getUser(), - ], - 'shareWithDisplayNameUnique' => !empty($emailAddress) ? $emailAddress : $cloud->getUser() - ]; - } - continue; - } - } - continue; } - if ($this->shareType !== IShare::TYPE_EMAIL) { - continue; - } + continue; + } - if ($exactEmailMatch - || (isset($contact['FN']) && strtolower($contact['FN']) === $lowerSearch)) { + if ($this->shareType === IShare::TYPE_EMAIL) { + if ($count++ >= $limit) { + $hasMore = true; + } elseif ($exactEmailMatch || (isset($contact['FN']) && strtolower($contact['FN']) === $lowerSearch)) { if ($exactEmailMatch) { - $searchResult->markExactIdMatch($emailType); + $searchResult->markExactIdMatch($type); } - $result['exact'][] = [ + + $results['exact'][] = [ 'label' => $displayName, 'uuid' => $contact['UID'] ?? $emailAddress, 'name' => $contact['FN'] ?? $displayName, @@ -223,7 +220,7 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b ], ]; } else { - $result['wide'][] = [ + $results['wide'][] = [ 'label' => $displayName, 'uuid' => $contact['UID'] ?? $emailAddress, 'name' => $contact['FN'] ?? $displayName, @@ -238,35 +235,25 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b } } - $reachedEnd = true; - if ($this->shareeEnumeration) { - $reachedEnd = (count($result['wide']) < $offset + $limit) - && (count($userResults['wide']) < $offset + $limit); - - $result['wide'] = array_slice($result['wide'], $offset, $limit); - $userResults['wide'] = array_slice($userResults['wide'], $offset, $limit); - } - if ($this->shareType === IShare::TYPE_EMAIL - && !$searchResult->hasExactIdMatch($emailType) && $this->emailValidator->isValid($search)) { - $result['exact'][] = [ - 'label' => $search, - 'uuid' => $search, - 'value' => [ - 'shareType' => IShare::TYPE_EMAIL, - 'shareWith' => $search, - ], - ]; + && !$searchResult->hasExactIdMatch($type) && $this->emailValidator->isValid($search)) { + if ($count++ >= $limit) { + $hasMore = true; + } else { + $results['exact'][] = [ + 'label' => $search, + 'uuid' => $search, + 'value' => [ + 'shareType' => IShare::TYPE_EMAIL, + 'shareWith' => $search, + ], + ]; + } } - if ($this->shareType === IShare::TYPE_USER && !empty($userResults['wide'])) { - $searchResult->addResultSet($userType, $userResults['wide'], []); - } - if ($this->shareType === IShare::TYPE_EMAIL) { - $searchResult->addResultSet($emailType, $result['wide'], $result['exact']); - } + $searchResult->addResultSet($type, $results['wide'], $results['exact']); - return !$reachedEnd; + return $hasMore; } public function isCurrentUser(ICloudId $cloud): bool { diff --git a/lib/private/Collaboration/Collaborators/UserByMailPlugin.php b/lib/private/Collaboration/Collaborators/UserByMailPlugin.php index 6c1367cdba841..50bbf7409fd70 100644 --- a/lib/private/Collaboration/Collaborators/UserByMailPlugin.php +++ b/lib/private/Collaboration/Collaborators/UserByMailPlugin.php @@ -13,6 +13,7 @@ use OCP\Federation\ICloudIdManager; use OCP\IConfig; use OCP\IGroupManager; +use OCP\IUserManager; use OCP\IUserSession; use OCP\Mail\IEmailValidator; use OCP\Share\IShare; @@ -30,6 +31,7 @@ public function __construct( KnownUserService $knownUserService, IUserSession $userSession, IEmailValidator $emailValidator, + IUserManager $userManager, mixed $shareWithGroupOnlyExcludeGroupsList = [], ) { parent::__construct( @@ -40,6 +42,7 @@ public function __construct( $knownUserService, $userSession, $emailValidator, + $userManager, $shareWithGroupOnlyExcludeGroupsList, IShare::TYPE_USER, ); diff --git a/tests/lib/Collaboration/Collaborators/MailPluginTest.php b/tests/lib/Collaboration/Collaborators/MailPluginTest.php index db2f6b970065a..a50fc251c25b6 100644 --- a/tests/lib/Collaboration/Collaborators/MailPluginTest.php +++ b/tests/lib/Collaboration/Collaborators/MailPluginTest.php @@ -39,6 +39,7 @@ class MailPluginTest extends TestCase { protected IGroupManager&MockObject $groupManager; protected KnownUserService&MockObject $knownUserService; protected IUserSession&MockObject $userSession; + protected IUserManager&MockObject $userManager; #[\Override] protected function setUp(): void { @@ -49,6 +50,17 @@ protected function setUp(): void { $this->groupManager = $this->createMock(IGroupManager::class); $this->knownUserService = $this->createMock(KnownUserService::class); $this->userSession = $this->createMock(IUserSession::class); + $this->userManager = $this->createMock(IUserManager::class); + $this->userManager + ->method('get') + ->willReturnCallback(function (string $uid): IUser { + $user = $this->createMock(IUser::class); + $user + ->method('getUID') + ->willReturn($uid); + + return $user; + }); $this->cloudIdManager = new CloudIdManager( $this->createMock(ICacheFactory::class), $this->createMock(IEventDispatcher::class), @@ -69,6 +81,7 @@ public function instantiatePlugin(int $shareType) { $this->knownUserService, $this->userSession, $this->getEmailValidatorWithStrictEmailCheck(), + $this->userManager, [], $shareType, ); @@ -575,7 +588,7 @@ function ($appName, $key, $default) use ($shareeEnumeration) { $moreResults = $this->plugin->search($searchTerm, 2, 0, $this->searchResult); $result = $this->searchResult->asArray(); - $this->assertSame($expectedExactIdMatch, $this->searchResult->hasExactIdMatch(new SearchResultType('emails'))); + $this->assertSame($expectedExactIdMatch, $this->searchResult->hasExactIdMatch(new SearchResultType('users'))); $this->assertEquals($expectedResult, $result); $this->assertSame($expectedMoreResults, $moreResults); } @@ -583,15 +596,15 @@ function ($appName, $key, $default) use ($shareeEnumeration) { public static function dataSearchUser(): array { return [ // data set 0 - ['test', [], true, ['exact' => []], false, false], + ['test', [], true, ['users' => [], 'exact' => ['users' => [],]], false, false], // data set 1 - ['test', [], false, ['exact' => []], false, false], + ['test', [], false, ['users' => [], 'exact' => ['users' => [],]], false, false], // data set 2 [ 'test@remote.com', [], true, - ['exact' => []], + ['users' => [], 'exact' => ['users' => [],]], false, false, ], @@ -600,7 +613,7 @@ public static function dataSearchUser(): array { 'test@remote.com', [], false, - ['exact' => []], + ['users' => [], 'exact' => ['users' => [],]], false, false, ], @@ -627,7 +640,7 @@ public static function dataSearchUser(): array { ], ], true, - ['exact' => []], + ['users' => [], 'exact' => ['users' => [],]], false, false, ], @@ -655,7 +668,7 @@ public static function dataSearchUser(): array { ], ], false, - ['exact' => []], + ['users' => [], 'exact' => ['users' => [],]], false, false, ], @@ -682,7 +695,7 @@ public static function dataSearchUser(): array { ], ], true, - ['exact' => []], + ['users' => [], 'exact' => ['users' => [],]], false, false, ], @@ -709,7 +722,7 @@ public static function dataSearchUser(): array { ], ], true, - ['exact' => []], + ['users' => [], 'exact' => ['users' => [],]], false, false, ], @@ -727,7 +740,7 @@ public static function dataSearchUser(): array { ] ], false, - ['users' => [], 'exact' => ['users' => [['uuid' => 'uid1', 'name' => 'User', 'label' => 'User (test@example.com)','value' => ['shareType' => IShare::TYPE_USER, 'shareWith' => 'test'], 'shareWithDisplayNameUnique' => 'test@example.com']]]], + ['users' => [], 'exact' => ['users' => [['uuid' => 'uid1', 'name' => 'User', 'label' => 'User (test@example.com)', 'value' => ['shareType' => IShare::TYPE_USER, 'shareWith' => 'test'], 'shareWithDisplayNameUnique' => 'test@example.com']]]], true, false, ], @@ -822,7 +835,7 @@ public static function dataSearchUser(): array { ], ], true, - ['exact' => []], + ['users' => [], 'exact' => ['users' => [],]], false, false, ], @@ -872,12 +885,6 @@ function ($appName, $key, $default) { return $userToGroupMapping[$user->getUID()]; }); - $this->groupManager->expects($this->any()) - ->method('isInGroup') - ->willReturnCallback(function ($userId, $group) use ($userToGroupMapping) { - return in_array($group, $userToGroupMapping[$userId]); - }); - $moreResults = $this->plugin->search($searchTerm, 2, 0, $this->searchResult); $result = $this->searchResult->asArray(); @@ -942,7 +949,7 @@ public static function dataSearchEmailGroupsOnly(): array { 'UID' => 'User', ] ], - ['emails' => [], 'exact' => ['emails' => [['label' => 'test@example.com', 'uuid' => 'test@example.com', 'value' => ['shareType' => IShare::TYPE_EMAIL,'shareWith' => 'test@example.com']]]]], + ['emails' => [], 'exact' => ['emails' => [['label' => 'test@example.com', 'uuid' => 'test@example.com', 'value' => ['shareType' => IShare::TYPE_EMAIL, 'shareWith' => 'test@example.com']]]]], false, false, [ @@ -996,12 +1003,6 @@ function ($appName, $key, $default) { return $userToGroupMapping[$user->getUID()]; }); - $this->groupManager->expects($this->any()) - ->method('isInGroup') - ->willReturnCallback(function ($userId, $group) use ($userToGroupMapping) { - return in_array($group, $userToGroupMapping[$userId]); - }); - $moreResults = $this->plugin->search($searchTerm, 2, 0, $this->searchResult); $result = $this->searchResult->asArray(); @@ -1024,7 +1025,7 @@ public static function dataSearchUserGroupsOnly(): array { 'UID' => 'User', ] ], - ['users' => [['label' => 'User (test@example.com)', 'uuid' => 'User', 'name' => 'User', 'value' => ['shareType' => IShare::TYPE_USER, 'shareWith' => 'test'],'shareWithDisplayNameUnique' => 'test@example.com',]], 'exact' => ['users' => []]], + ['users' => [['label' => 'User (test@example.com)', 'uuid' => 'User', 'name' => 'User', 'value' => ['shareType' => IShare::TYPE_USER, 'shareWith' => 'test'], 'shareWithDisplayNameUnique' => 'test@example.com',]], 'exact' => ['users' => []]], false, false, [ @@ -1044,7 +1045,7 @@ public static function dataSearchUserGroupsOnly(): array { 'UID' => 'User', ] ], - ['exact' => []], + ['users' => [], 'exact' => ['users' => [],]], false, false, [ @@ -1064,7 +1065,7 @@ public static function dataSearchUserGroupsOnly(): array { 'UID' => 'User', ] ], - ['exact' => []], + ['users' => [], 'exact' => ['users' => [],]], false, false, [