From 16c26101d0392268ceeade2a144306c43bf1b658 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 8 Apr 2025 03:05:30 +0200 Subject: [PATCH] fix: Fix user collaborators returned when searching for mail collaborators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The MailPlugin collaborator returned results for both user and mail collaborators, but it was registered only for mail collaborators. While it might make sense to move the user results to the UserPlugin instead that change would be more complex and riskier, so for now the MailPlugin is now registered for both user and mail collaborators and the results are limited only to the registered type. As the plugins are registered only with their class and then resolved when needed using dependency injection it is not possible (as far as I know) to provide an explicit parameter in the constructor to differentiate whether the MailPlugin should return user or mail collaborators. To overcome this two subclasses are introduced, MailByMailPlugin and UserByMailPlugin, which just hardcode in their constructor the collaborator type that their parent MailPlugin must use, and those subclasses are the ones registered instead of the MailPlugin (which still contains all the logic). Signed-off-by: Daniel Calviño Sánchez --- .../autocomplete.feature | 7 - .../sharees_features/sharees.feature | 32 +- lib/composer/composer/autoload_classmap.php | 2 + lib/composer/composer/autoload_static.php | 2 + .../Collaborators/MailByMailPlugin.php | 45 ++ .../Collaborators/MailPlugin.php | 40 +- .../Collaborators/UserByMailPlugin.php | 45 ++ lib/private/Server.php | 6 +- .../Collaborators/MailPluginTest.php | 478 +++++++++++++++++- 9 files changed, 590 insertions(+), 67 deletions(-) create mode 100644 lib/private/Collaboration/Collaborators/MailByMailPlugin.php create mode 100644 lib/private/Collaboration/Collaborators/UserByMailPlugin.php diff --git a/build/integration/collaboration_features/autocomplete.feature b/build/integration/collaboration_features/autocomplete.feature index e9820f28be5..763b95bf170 100644 --- a/build/integration/collaboration_features/autocomplete.feature +++ b/build/integration/collaboration_features/autocomplete.feature @@ -107,28 +107,22 @@ Feature: autocomplete When parameter "shareapi_restrict_user_enumeration_full_match" of app "core" is set to "no" Then get email autocomplete for "auto" | id | source | - | autocomplete | users | Then get email autocomplete for "example" | id | source | - | autocomplete | users | | leon@example.com | emails | | user@example.com | emails | Then get email autocomplete for "autocomplete@example.com" | id | source | - | autocomplete | users | | autocomplete@example.com | emails | When parameter "shareapi_restrict_user_enumeration_full_match" of app "core" is set to "yes" Then get email autocomplete for "auto" | id | source | - | autocomplete | users | Then get email autocomplete for "example" | id | source | - | autocomplete | users | | leon@example.com | emails | | user@example.com | emails | Then get email autocomplete for "autocomplete@example.com" | id | source | - | autocomplete | users | Scenario: getting autocomplete emails from address book without enumeration Given As an "admin" @@ -156,7 +150,6 @@ Feature: autocomplete | user@example.com | emails | Then get email autocomplete for "autocomplete@example.com" | id | source | - | autocomplete | users | Scenario: getting autocomplete with limited enumeration by group Given As an "admin" diff --git a/build/integration/sharees_features/sharees.feature b/build/integration/sharees_features/sharees.feature index e9e7b7ab6fb..4ff6d70cc53 100644 --- a/build/integration/sharees_features/sharees.feature +++ b/build/integration/sharees_features/sharees.feature @@ -281,6 +281,7 @@ Feature: sharees And the HTTP status code should be "200" # UserPlugin provides two identical results (except for the field order, but # that is hidden by the check). + # MailPlugin does not add a result if there is already one for that user. And "exact users" sharees returned are | Sharee2 | 0 | Sharee2 | sharee2@system.com | | Sharee2 | 0 | Sharee2 | sharee2@system.com | @@ -301,6 +302,7 @@ Feature: sharees Then the OCS status code should be "100" And the HTTP status code should be "200" And "exact users" sharees returned is empty + # MailPlugin does not add a result if there is already one for that user. And "users" sharees returned are | Sharee2 | 0 | Sharee2 | sharee2@system.com | And "exact groups" sharees returned is empty @@ -320,7 +322,8 @@ Feature: sharees And the HTTP status code should be "200" # UserPlugin only searches in the system e-mail address, but not in # secondary addresses. - And "exact users" sharees returned is empty + And "exact users" sharees returned are + | Sharee2 (sharee2@secondary.com) | 0 | Sharee2 | sharee2@secondary.com | And "users" sharees returned is empty And "exact groups" sharees returned is empty And "groups" sharees returned is empty @@ -340,7 +343,11 @@ Feature: sharees And "exact users" sharees returned is empty # UserPlugin only searches in the system e-mail address, but not in # secondary addresses. - And "users" sharees returned is empty + # MailPlugin adds a result for every e-mail address of the contact unless + # there is an exact match. + And "users" sharees returned are + | Sharee2 (sharee2@system.com) | 0 | Sharee2 | sharee2@system.com | + | Sharee2 (sharee2@secondary.com) | 0 | Sharee2 | sharee2@secondary.com | And "exact groups" sharees returned is empty And "groups" sharees returned is empty And "exact remotes" sharees returned is empty @@ -394,8 +401,7 @@ Feature: sharees | shareType | 4 | Then the OCS status code should be "100" And the HTTP status code should be "200" - And "exact users" sharees returned are - | Sharee2 (sharee2@system.com) | 0 | Sharee2 | sharee2@system.com | + And "exact users" sharees returned is empty And "users" sharees returned is empty And "exact groups" sharees returned is empty And "groups" sharees returned is empty @@ -413,11 +419,7 @@ Feature: sharees Then the OCS status code should be "100" And the HTTP status code should be "200" And "exact users" sharees returned is empty - # MailPlugin adds a result for every e-mail address of the contact unless - # there is an exact match. - And "users" sharees returned are - | Sharee2 (sharee2@system.com) | 0 | Sharee2 | sharee2@system.com | - | Sharee2 (sharee2@secondary.com) | 0 | Sharee2 | sharee2@secondary.com | + And "users" sharees returned is empty And "exact groups" sharees returned is empty And "groups" sharees returned is empty And "exact remotes" sharees returned is empty @@ -434,8 +436,7 @@ Feature: sharees | shareType | 4 | Then the OCS status code should be "100" And the HTTP status code should be "200" - And "exact users" sharees returned are - | Sharee2 (sharee2@secondary.com) | 0 | Sharee2 | sharee2@secondary.com | + And "exact users" sharees returned is empty And "users" sharees returned is empty And "exact groups" sharees returned is empty And "groups" sharees returned is empty @@ -453,11 +454,7 @@ Feature: sharees Then the OCS status code should be "100" And the HTTP status code should be "200" And "exact users" sharees returned is empty - # MailPlugin adds a result for every e-mail address of the contact unless - # there is an exact match. - And "users" sharees returned are - | Sharee2 (sharee2@system.com) | 0 | Sharee2 | sharee2@system.com | - | Sharee2 (sharee2@secondary.com) | 0 | Sharee2 | sharee2@secondary.com | + And "users" sharees returned is empty And "exact groups" sharees returned is empty And "groups" sharees returned is empty And "exact remotes" sharees returned is empty @@ -540,7 +537,8 @@ Feature: sharees | shareTypes | 0 4 | Then the OCS status code should be "100" And the HTTP status code should be "200" - And "exact users" sharees returned is empty + And "exact users" sharees returned are + | Sharee2 (sharee2@secondary.com) | 0 | Sharee2 | sharee2@secondary.com | And "users" sharees returned is empty And "exact groups" sharees returned is empty And "groups" sharees returned is empty diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 1efc2b374f9..2b5c8fb939b 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1201,11 +1201,13 @@ return array( 'OC\\Collaboration\\AutoComplete\\Manager' => $baseDir . '/lib/private/Collaboration/AutoComplete/Manager.php', 'OC\\Collaboration\\Collaborators\\GroupPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/GroupPlugin.php', 'OC\\Collaboration\\Collaborators\\LookupPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/LookupPlugin.php', + 'OC\\Collaboration\\Collaborators\\MailByMailPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/MailByMailPlugin.php', 'OC\\Collaboration\\Collaborators\\MailPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/MailPlugin.php', 'OC\\Collaboration\\Collaborators\\RemoteGroupPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/RemoteGroupPlugin.php', 'OC\\Collaboration\\Collaborators\\RemotePlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/RemotePlugin.php', 'OC\\Collaboration\\Collaborators\\Search' => $baseDir . '/lib/private/Collaboration/Collaborators/Search.php', 'OC\\Collaboration\\Collaborators\\SearchResult' => $baseDir . '/lib/private/Collaboration/Collaborators/SearchResult.php', + 'OC\\Collaboration\\Collaborators\\UserByMailPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/UserByMailPlugin.php', 'OC\\Collaboration\\Collaborators\\UserPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/UserPlugin.php', 'OC\\Collaboration\\Reference\\File\\FileReferenceEventListener' => $baseDir . '/lib/private/Collaboration/Reference/File/FileReferenceEventListener.php', 'OC\\Collaboration\\Reference\\File\\FileReferenceProvider' => $baseDir . '/lib/private/Collaboration/Reference/File/FileReferenceProvider.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index bd3862b9e1c..8bb9e4f430f 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1242,11 +1242,13 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Collaboration\\AutoComplete\\Manager' => __DIR__ . '/../../..' . '/lib/private/Collaboration/AutoComplete/Manager.php', 'OC\\Collaboration\\Collaborators\\GroupPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/GroupPlugin.php', 'OC\\Collaboration\\Collaborators\\LookupPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/LookupPlugin.php', + 'OC\\Collaboration\\Collaborators\\MailByMailPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/MailByMailPlugin.php', 'OC\\Collaboration\\Collaborators\\MailPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/MailPlugin.php', 'OC\\Collaboration\\Collaborators\\RemoteGroupPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/RemoteGroupPlugin.php', 'OC\\Collaboration\\Collaborators\\RemotePlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/RemotePlugin.php', 'OC\\Collaboration\\Collaborators\\Search' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/Search.php', 'OC\\Collaboration\\Collaborators\\SearchResult' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/SearchResult.php', + 'OC\\Collaboration\\Collaborators\\UserByMailPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/UserByMailPlugin.php', 'OC\\Collaboration\\Collaborators\\UserPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/UserPlugin.php', 'OC\\Collaboration\\Reference\\File\\FileReferenceEventListener' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Reference/File/FileReferenceEventListener.php', 'OC\\Collaboration\\Reference\\File\\FileReferenceProvider' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Reference/File/FileReferenceProvider.php', diff --git a/lib/private/Collaboration/Collaborators/MailByMailPlugin.php b/lib/private/Collaboration/Collaborators/MailByMailPlugin.php new file mode 100644 index 00000000000..aaa81bfe9d5 --- /dev/null +++ b/lib/private/Collaboration/Collaborators/MailByMailPlugin.php @@ -0,0 +1,45 @@ +shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes'; $this->shareWithGroupOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes'; @@ -139,7 +140,7 @@ class MailPlugin implements ISearchPlugin { continue; } - if (!$this->isCurrentUser($cloud) && !$searchResult->hasResult($userType, $cloud->getUser())) { + if ($this->shareType === IShare::TYPE_USER && !$this->isCurrentUser($cloud) && !$searchResult->hasResult($userType, $cloud->getUser())) { $singleResult = [[ 'label' => $displayName, 'uuid' => $contact['UID'] ?? $emailAddress, @@ -180,22 +181,28 @@ class MailPlugin implements ISearchPlugin { } } if ($addToWide && !$this->isCurrentUser($cloud) && !$searchResult->hasResult($userType, $cloud->getUser())) { - $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() - ]; + 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; + } + if ($exactEmailMatch || (isset($contact['FN']) && strtolower($contact['FN']) === $lowerSearch)) { if ($exactEmailMatch) { @@ -236,7 +243,8 @@ class MailPlugin implements ISearchPlugin { $userResults['wide'] = array_slice($userResults['wide'], $offset, $limit); } - if (!$searchResult->hasExactIdMatch($emailType) && $this->mailer->validateMailAddress($search)) { + if ($this->shareType === IShare::TYPE_EMAIL + && !$searchResult->hasExactIdMatch($emailType) && $this->mailer->validateMailAddress($search)) { $result['exact'][] = [ 'label' => $search, 'uuid' => $search, @@ -247,10 +255,12 @@ class MailPlugin implements ISearchPlugin { ]; } - if (!empty($userResults['wide'])) { + if ($this->shareType === IShare::TYPE_USER && !empty($userResults['wide'])) { $searchResult->addResultSet($userType, $userResults['wide'], []); } - $searchResult->addResultSet($emailType, $result['wide'], $result['exact']); + if ($this->shareType === IShare::TYPE_EMAIL) { + $searchResult->addResultSet($emailType, $result['wide'], $result['exact']); + } return !$reachedEnd; } diff --git a/lib/private/Collaboration/Collaborators/UserByMailPlugin.php b/lib/private/Collaboration/Collaborators/UserByMailPlugin.php new file mode 100644 index 00000000000..21e96f25433 --- /dev/null +++ b/lib/private/Collaboration/Collaborators/UserByMailPlugin.php @@ -0,0 +1,45 @@ +registerPlugin(['shareType' => 'SHARE_TYPE_USER', 'class' => UserPlugin::class]); + $instance->registerPlugin(['shareType' => 'SHARE_TYPE_USER', 'class' => UserByMailPlugin::class]); $instance->registerPlugin(['shareType' => 'SHARE_TYPE_GROUP', 'class' => GroupPlugin::class]); - $instance->registerPlugin(['shareType' => 'SHARE_TYPE_EMAIL', 'class' => MailPlugin::class]); + $instance->registerPlugin(['shareType' => 'SHARE_TYPE_EMAIL', 'class' => MailByMailPlugin::class]); $instance->registerPlugin(['shareType' => 'SHARE_TYPE_REMOTE', 'class' => RemotePlugin::class]); $instance->registerPlugin(['shareType' => 'SHARE_TYPE_REMOTE_GROUP', 'class' => RemoteGroupPlugin::class]); diff --git a/tests/lib/Collaboration/Collaborators/MailPluginTest.php b/tests/lib/Collaboration/Collaborators/MailPluginTest.php index 6562e32880e..5215e1466a7 100644 --- a/tests/lib/Collaboration/Collaborators/MailPluginTest.php +++ b/tests/lib/Collaboration/Collaborators/MailPluginTest.php @@ -74,7 +74,7 @@ class MailPluginTest extends TestCase { $this->searchResult = new SearchResult(); } - public function instantiatePlugin() { + public function instantiatePlugin(int $shareType) { $this->plugin = new MailPlugin( $this->contactsManager, $this->cloudIdManager, @@ -82,7 +82,9 @@ class MailPluginTest extends TestCase { $this->groupManager, $this->knownUserService, $this->userSession, - $this->mailer + $this->mailer, + [], + $shareType, ); } @@ -96,8 +98,8 @@ class MailPluginTest extends TestCase { * @param bool $expectedMoreResults * @param bool $validEmail */ - #[\PHPUnit\Framework\Attributes\DataProvider('dataSearch')] - public function testSearch($searchTerm, $contacts, $shareeEnumeration, $expectedResult, $expectedExactIdMatch, $expectedMoreResults, $validEmail): void { + #[\PHPUnit\Framework\Attributes\DataProvider('dataSearchEmail')] + public function testSearchEmail($searchTerm, $contacts, $shareeEnumeration, $expectedResult, $expectedExactIdMatch, $expectedMoreResults, $validEmail): void { $this->config->expects($this->any()) ->method('getAppValue') ->willReturnCallback( @@ -109,7 +111,7 @@ class MailPluginTest extends TestCase { } ); - $this->instantiatePlugin(); + $this->instantiatePlugin(IShare::TYPE_EMAIL); $currentUser = $this->createMock(IUser::class); $currentUser->method('getUID') @@ -137,7 +139,7 @@ class MailPluginTest extends TestCase { $this->assertSame($expectedMoreResults, $moreResults); } - public static function dataSearch(): array { + public static function dataSearchEmail(): array { return [ // data set 0 ['test', [], true, ['emails' => [], 'exact' => ['emails' => []]], false, false, false], @@ -403,7 +405,7 @@ class MailPluginTest extends TestCase { false, ], // data set 13 - // Local user found by email + // Local user found by email => no result [ 'test@example.com', [ @@ -416,8 +418,8 @@ class MailPluginTest extends TestCase { ] ], false, - ['users' => [], 'exact' => ['users' => [['uuid' => 'uid1', 'name' => 'User', 'label' => 'User (test@example.com)','value' => ['shareType' => IShare::TYPE_USER, 'shareWith' => 'test'], 'shareWithDisplayNameUnique' => 'test@example.com']]]], - true, + ['exact' => []], + false, false, true, ], @@ -441,7 +443,7 @@ class MailPluginTest extends TestCase { true, ], // data set 15 - // Pagination and "more results" for user matches byyyyyyy emails + // Several local users found by email => no result nor pagination [ 'test@example', [ @@ -475,12 +477,9 @@ class MailPluginTest extends TestCase { ], ], true, - ['users' => [ - ['uuid' => 'uid1', 'name' => 'User1', 'label' => 'User1 (test@example.com)', 'value' => ['shareType' => IShare::TYPE_USER, 'shareWith' => 'test1'], 'shareWithDisplayNameUnique' => 'test@example.com'], - ['uuid' => 'uid2', 'name' => 'User2', 'label' => 'User2 (test@example.de)', 'value' => ['shareType' => IShare::TYPE_USER, 'shareWith' => 'test2'], 'shareWithDisplayNameUnique' => 'test@example.de'], - ], 'emails' => [], 'exact' => ['users' => [], 'emails' => []]], + ['emails' => [], 'exact' => ['emails' => []]], + false, false, - true, false, ], // data set 16 @@ -570,6 +569,302 @@ class MailPluginTest extends TestCase { ]; } + /** + * + * @param string $searchTerm + * @param array $contacts + * @param bool $shareeEnumeration + * @param array $expectedResult + * @param bool $expectedExactIdMatch + * @param bool $expectedMoreResults + */ + #[\PHPUnit\Framework\Attributes\DataProvider('dataSearchUser')] + public function testSearchUser($searchTerm, $contacts, $shareeEnumeration, $expectedResult, $expectedExactIdMatch, $expectedMoreResults): void { + $this->config->expects($this->any()) + ->method('getAppValue') + ->willReturnCallback( + function ($appName, $key, $default) use ($shareeEnumeration) { + if ($appName === 'core' && $key === 'shareapi_allow_share_dialog_user_enumeration') { + return $shareeEnumeration ? 'yes' : 'no'; + } + return $default; + } + ); + + $this->instantiatePlugin(IShare::TYPE_USER); + + $currentUser = $this->createMock(IUser::class); + $currentUser->method('getUID') + ->willReturn('current'); + $this->userSession->method('getUser') + ->willReturn($currentUser); + + $this->contactsManager->expects($this->any()) + ->method('search') + ->willReturnCallback(function ($search, $searchAttributes) use ($searchTerm, $contacts) { + if ($search === $searchTerm) { + return $contacts; + } + return []; + }); + + $moreResults = $this->plugin->search($searchTerm, 2, 0, $this->searchResult); + $result = $this->searchResult->asArray(); + + $this->assertSame($expectedExactIdMatch, $this->searchResult->hasExactIdMatch(new SearchResultType('emails'))); + $this->assertEquals($expectedResult, $result); + $this->assertSame($expectedMoreResults, $moreResults); + } + + public static function dataSearchUser(): array { + return [ + // data set 0 + ['test', [], true, ['exact' => []], false, false], + // data set 1 + ['test', [], false, ['exact' => []], false, false], + // data set 2 + [ + 'test@remote.com', + [], + true, + ['exact' => []], + false, + false, + ], + // data set 3 + [ + 'test@remote.com', + [], + false, + ['exact' => []], + false, + false, + ], + // data set 4 + [ + 'test', + [ + [ + 'UID' => 'uid3', + 'FN' => 'User3 @ Localhost', + ], + [ + 'UID' => 'uid2', + 'FN' => 'User2 @ Localhost', + 'EMAIL' => [ + ], + ], + [ + 'UID' => 'uid1', + 'FN' => 'User @ Localhost', + 'EMAIL' => [ + 'username@localhost', + ], + ], + ], + true, + ['exact' => []], + false, + false, + ], + // data set 5 + [ + 'test', + [ + [ + 'UID' => 'uid3', + 'FN' => 'User3 @ Localhost', + ], + [ + 'UID' => 'uid2', + 'FN' => 'User2 @ Localhost', + 'EMAIL' => [ + ], + ], + [ + 'isLocalSystemBook' => true, + 'UID' => 'uid1', + 'FN' => 'User @ Localhost', + 'EMAIL' => [ + 'username@localhost', + ], + ], + ], + false, + ['exact' => []], + false, + false, + ], + // data set 6 + [ + 'test@remote.com', + [ + [ + 'UID' => 'uid3', + 'FN' => 'User3 @ Localhost', + ], + [ + 'UID' => 'uid2', + 'FN' => 'User2 @ Localhost', + 'EMAIL' => [ + ], + ], + [ + 'UID' => 'uid1', + 'FN' => 'User @ Localhost', + 'EMAIL' => [ + 'username@localhost', + ], + ], + ], + true, + ['exact' => []], + false, + false, + ], + // data set 7 + [ + 'username@localhost', + [ + [ + 'UID' => 'uid3', + 'FN' => 'User3 @ Localhost', + ], + [ + 'UID' => 'uid2', + 'FN' => 'User2 @ Localhost', + 'EMAIL' => [ + ], + ], + [ + 'UID' => 'uid1', + 'FN' => 'User @ Localhost', + 'EMAIL' => [ + 'username@localhost', + ], + ], + ], + true, + ['exact' => []], + false, + false, + ], + // data set 8 + // Local user found by email + [ + 'test@example.com', + [ + [ + 'UID' => 'uid1', + 'FN' => 'User', + 'EMAIL' => ['test@example.com'], + 'CLOUD' => ['test@localhost'], + 'isLocalSystemBook' => true, + ] + ], + false, + ['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, + ], + // data set 9 + // Current local user found by email => no result + [ + 'test@example.com', + [ + [ + 'UID' => 'uid1', + 'FN' => 'User', + 'EMAIL' => ['test@example.com'], + 'CLOUD' => ['current@localhost'], + 'isLocalSystemBook' => true, + ] + ], + true, + ['exact' => []], + false, + false, + ], + // data set 10 + // Pagination and "more results" for user matches by emails + [ + 'test@example', + [ + [ + 'UID' => 'uid1', + 'FN' => 'User1', + 'EMAIL' => ['test@example.com'], + 'CLOUD' => ['test1@localhost'], + 'isLocalSystemBook' => true, + ], + [ + 'UID' => 'uid2', + 'FN' => 'User2', + 'EMAIL' => ['test@example.de'], + 'CLOUD' => ['test2@localhost'], + 'isLocalSystemBook' => true, + ], + [ + 'UID' => 'uid3', + 'FN' => 'User3', + 'EMAIL' => ['test@example.org'], + 'CLOUD' => ['test3@localhost'], + 'isLocalSystemBook' => true, + ], + [ + 'UID' => 'uid4', + 'FN' => 'User4', + 'EMAIL' => ['test@example.net'], + 'CLOUD' => ['test4@localhost'], + 'isLocalSystemBook' => true, + ], + ], + true, + ['users' => [ + ['uuid' => 'uid1', 'name' => 'User1', 'label' => 'User1 (test@example.com)', 'value' => ['shareType' => IShare::TYPE_USER, 'shareWith' => 'test1'], 'shareWithDisplayNameUnique' => 'test@example.com'], + ['uuid' => 'uid2', 'name' => 'User2', 'label' => 'User2 (test@example.de)', 'value' => ['shareType' => IShare::TYPE_USER, 'shareWith' => 'test2'], 'shareWithDisplayNameUnique' => 'test@example.de'], + ], 'exact' => ['users' => []]], + false, + true, + ], + // data set 11 + // Pagination and "more results" for normal emails + [ + 'test@example', + [ + [ + 'UID' => 'uid1', + 'FN' => 'User1', + 'EMAIL' => ['test@example.com'], + 'CLOUD' => ['test1@localhost'], + ], + [ + 'UID' => 'uid2', + 'FN' => 'User2', + 'EMAIL' => ['test@example.de'], + 'CLOUD' => ['test2@localhost'], + ], + [ + 'UID' => 'uid3', + 'FN' => 'User3', + 'EMAIL' => ['test@example.org'], + 'CLOUD' => ['test3@localhost'], + ], + [ + 'UID' => 'uid4', + 'FN' => 'User4', + 'EMAIL' => ['test@example.net'], + 'CLOUD' => ['test4@localhost'], + ], + ], + true, + ['exact' => []], + false, + false, + ], + ]; + } + /** * * @param string $searchTerm @@ -580,8 +875,8 @@ class MailPluginTest extends TestCase { * @param array $userToGroupMapping * @param bool $validEmail */ - #[\PHPUnit\Framework\Attributes\DataProvider('dataSearchGroupsOnly')] - public function testSearchGroupsOnly($searchTerm, $contacts, $expectedResult, $expectedExactIdMatch, $expectedMoreResults, $userToGroupMapping, $validEmail): void { + #[\PHPUnit\Framework\Attributes\DataProvider('dataSearchEmailGroupsOnly')] + public function testSearchEmailGroupsOnly($searchTerm, $contacts, $expectedResult, $expectedExactIdMatch, $expectedMoreResults, $userToGroupMapping, $validEmail): void { $this->config->expects($this->any()) ->method('getAppValue') ->willReturnCallback( @@ -595,7 +890,7 @@ class MailPluginTest extends TestCase { } ); - $this->instantiatePlugin(); + $this->instantiatePlugin(IShare::TYPE_EMAIL); /** @var IUser|\PHPUnit\Framework\MockObject\MockObject */ $currentUser = $this->createMock('\OCP\IUser'); @@ -640,7 +935,7 @@ class MailPluginTest extends TestCase { $this->assertSame($expectedMoreResults, $moreResults); } - public static function dataSearchGroupsOnly(): array { + public static function dataSearchEmailGroupsOnly(): array { return [ // The user `User` can share with the current user [ @@ -651,15 +946,15 @@ class MailPluginTest extends TestCase { 'EMAIL' => ['test@example.com'], 'CLOUD' => ['test@localhost'], 'isLocalSystemBook' => true, - 'UID' => 'User' + 'UID' => 'User', ] ], - ['users' => [['label' => 'User (test@example.com)', 'uuid' => 'User', 'name' => 'User', 'value' => ['shareType' => IShare::TYPE_USER, 'shareWith' => 'test'],'shareWithDisplayNameUnique' => 'test@example.com',]], 'emails' => [], 'exact' => ['emails' => [], 'users' => []]], + ['emails' => [], 'exact' => ['emails' => []]], false, false, [ 'currentUser' => ['group1'], - 'User' => ['group1'] + 'User' => ['group1'], ], false, ], @@ -672,7 +967,7 @@ class MailPluginTest extends TestCase { 'EMAIL' => ['test@example.com'], 'CLOUD' => ['test@localhost'], 'isLocalSystemBook' => true, - 'UID' => 'User' + 'UID' => 'User', ] ], ['emails' => [], 'exact' => ['emails' => []]], @@ -680,7 +975,7 @@ class MailPluginTest extends TestCase { false, [ 'currentUser' => ['group1'], - 'User' => ['group2'] + 'User' => ['group2'], ], false, ], @@ -693,7 +988,7 @@ class MailPluginTest extends TestCase { 'EMAIL' => ['test@example.com'], 'CLOUD' => ['test@localhost'], 'isLocalSystemBook' => true, - 'UID' => 'User' + 'UID' => 'User', ] ], ['emails' => [], 'exact' => ['emails' => [['label' => 'test@example.com', 'uuid' => 'test@example.com', 'value' => ['shareType' => IShare::TYPE_EMAIL,'shareWith' => 'test@example.com']]]]], @@ -701,10 +996,141 @@ class MailPluginTest extends TestCase { false, [ 'currentUser' => ['group1'], - 'User' => ['group2'] + 'User' => ['group2'], ], true, ] ]; } + + /** + * + * @param string $searchTerm + * @param array $contacts + * @param array $expectedResult + * @param bool $expectedExactIdMatch + * @param bool $expectedMoreResults + * @param array $userToGroupMapping + */ + #[\PHPUnit\Framework\Attributes\DataProvider('dataSearchUserGroupsOnly')] + public function testSearchUserGroupsOnly($searchTerm, $contacts, $expectedResult, $expectedExactIdMatch, $expectedMoreResults, $userToGroupMapping): void { + $this->config->expects($this->any()) + ->method('getAppValue') + ->willReturnCallback( + function ($appName, $key, $default) { + if ($appName === 'core' && $key === 'shareapi_allow_share_dialog_user_enumeration') { + return 'yes'; + } elseif ($appName === 'core' && $key === 'shareapi_only_share_with_group_members') { + return 'yes'; + } + return $default; + } + ); + + $this->instantiatePlugin(IShare::TYPE_USER); + + /** @var \OCP\IUser | \PHPUnit\Framework\MockObject\MockObject */ + $currentUser = $this->createMock('\OCP\IUser'); + + $currentUser->expects($this->any()) + ->method('getUID') + ->willReturn('currentUser'); + + $this->contactsManager->expects($this->any()) + ->method('search') + ->willReturnCallback(function ($search, $searchAttributes) use ($searchTerm, $contacts) { + if ($search === $searchTerm) { + return $contacts; + } + return []; + }); + + $this->userSession->expects($this->any()) + ->method('getUser') + ->willReturn($currentUser); + + $this->groupManager->expects($this->any()) + ->method('getUserGroupIds') + ->willReturnCallback(function (\OCP\IUser $user) use ($userToGroupMapping) { + 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(); + + $this->assertSame($expectedExactIdMatch, $this->searchResult->hasExactIdMatch(new SearchResultType('emails'))); + $this->assertEquals($expectedResult, $result); + $this->assertSame($expectedMoreResults, $moreResults); + } + + public static function dataSearchUserGroupsOnly(): array { + return [ + // The user `User` can share with the current user + [ + 'test', + [ + [ + 'FN' => 'User', + 'EMAIL' => ['test@example.com'], + 'CLOUD' => ['test@localhost'], + 'isLocalSystemBook' => true, + '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' => []]], + false, + false, + [ + 'currentUser' => ['group1'], + 'User' => ['group1'], + ], + ], + // The user `User` cannot share with the current user + [ + 'test', + [ + [ + 'FN' => 'User', + 'EMAIL' => ['test@example.com'], + 'CLOUD' => ['test@localhost'], + 'isLocalSystemBook' => true, + 'UID' => 'User', + ] + ], + ['exact' => []], + false, + false, + [ + 'currentUser' => ['group1'], + 'User' => ['group2'], + ], + ], + // The user `User` cannot share with the current user, but there is an exact match on the e-mail address -> share by e-mail + [ + 'test@example.com', + [ + [ + 'FN' => 'User', + 'EMAIL' => ['test@example.com'], + 'CLOUD' => ['test@localhost'], + 'isLocalSystemBook' => true, + 'UID' => 'User', + ] + ], + ['exact' => []], + false, + false, + [ + 'currentUser' => ['group1'], + 'User' => ['group2'], + ], + ] + ]; + } }