From a4bf16e779323f4930a30ebe12b75ca24ef020d3 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 ddbf9a6aab7..39a6de00f1b 100644 --- a/build/integration/collaboration_features/autocomplete.feature +++ b/build/integration/collaboration_features/autocomplete.feature @@ -105,26 +105,20 @@ 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 | | 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 | | 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" @@ -150,7 +144,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 438be13becd..87192dc9cd3 100644 --- a/build/integration/sharees_features/sharees.feature +++ b/build/integration/sharees_features/sharees.feature @@ -278,6 +278,7 @@ Feature: sharees | shareType | 0 | Then the OCS status code should be "100" And the HTTP status code should be "200" + # 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 | And "users" sharees returned is empty @@ -297,6 +298,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 @@ -316,7 +318,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 @@ -336,7 +339,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 @@ -390,8 +397,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 @@ -409,11 +415,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 @@ -430,8 +432,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 @@ -449,11 +450,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 @@ -530,7 +527,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 3cff7834610..e355b07d006 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1154,11 +1154,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 4318a2a192d..cb5c4b74bbf 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1203,11 +1203,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'; @@ -138,7 +139,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, @@ -179,22 +180,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) { @@ -235,7 +242,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, @@ -246,10 +254,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 539a5d6fd85..d9fc3d15d0e 100644 --- a/tests/lib/Collaboration/Collaborators/MailPluginTest.php +++ b/tests/lib/Collaboration/Collaborators/MailPluginTest.php @@ -73,7 +73,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, @@ -81,12 +81,14 @@ class MailPluginTest extends TestCase { $this->groupManager, $this->knownUserService, $this->userSession, - $this->mailer + $this->mailer, + [], + $shareType, ); } /** - * @dataProvider dataSearch + * @dataProvider dataSearchEmail * * @param string $searchTerm * @param array $contacts @@ -96,7 +98,7 @@ class MailPluginTest extends TestCase { * @param bool $expectedMoreResults * @param bool $validEmail */ - public function testSearch($searchTerm, $contacts, $shareeEnumeration, $expectedResult, $expectedExactIdMatch, $expectedMoreResults, $validEmail): void { + public function testSearchEmail($searchTerm, $contacts, $shareeEnumeration, $expectedResult, $expectedExactIdMatch, $expectedMoreResults, $validEmail): void { $this->config->expects($this->any()) ->method('getAppValue') ->willReturnCallback( @@ -108,7 +110,7 @@ class MailPluginTest extends TestCase { } ); - $this->instantiatePlugin(); + $this->instantiatePlugin(IShare::TYPE_EMAIL); $currentUser = $this->createMock(IUser::class); $currentUser->method('getUID') @@ -136,7 +138,7 @@ class MailPluginTest extends TestCase { $this->assertSame($expectedMoreResults, $moreResults); } - public function dataSearch() { + public function dataSearchEmail() { return [ // data set 0 ['test', [], true, ['emails' => [], 'exact' => ['emails' => []]], false, false, false], @@ -402,7 +404,7 @@ class MailPluginTest extends TestCase { false, ], // data set 13 - // Local user found by email + // Local user found by email => no result [ 'test@example.com', [ @@ -415,8 +417,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, ], @@ -440,7 +442,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', [ @@ -474,12 +476,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,7 +569,303 @@ class MailPluginTest extends TestCase { } /** - * @dataProvider dataSearchGroupsOnly + * @dataProvider dataSearchUser + * + * @param string $searchTerm + * @param array $contacts + * @param bool $shareeEnumeration + * @param array $expectedResult + * @param bool $expectedExactIdMatch + * @param bool $expectedMoreResults + */ + 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 function dataSearchUser() { + 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, + ], + ]; + } + + /** + * @dataProvider dataSearchEmailGroupsOnly * * @param string $searchTerm * @param array $contacts @@ -580,7 +875,7 @@ class MailPluginTest extends TestCase { * @param array $userToGroupMapping * @param bool $validEmail */ - public function testSearchGroupsOnly($searchTerm, $contacts, $expectedResult, $expectedExactIdMatch, $expectedMoreResults, $userToGroupMapping, $validEmail): void { + public function testSearchEmailGroupsOnly($searchTerm, $contacts, $expectedResult, $expectedExactIdMatch, $expectedMoreResults, $userToGroupMapping, $validEmail): void { $this->config->expects($this->any()) ->method('getAppValue') ->willReturnCallback( @@ -594,7 +889,7 @@ class MailPluginTest extends TestCase { } ); - $this->instantiatePlugin(); + $this->instantiatePlugin(IShare::TYPE_EMAIL); /** @var \OCP\IUser | \PHPUnit\Framework\MockObject\MockObject */ $currentUser = $this->createMock('\OCP\IUser'); @@ -639,7 +934,7 @@ class MailPluginTest extends TestCase { $this->assertSame($expectedMoreResults, $moreResults); } - public function dataSearchGroupsOnly() { + public function dataSearchEmailGroupsOnly() { return [ // The user `User` can share with the current user [ @@ -650,15 +945,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, ], @@ -671,7 +966,7 @@ class MailPluginTest extends TestCase { 'EMAIL' => ['test@example.com'], 'CLOUD' => ['test@localhost'], 'isLocalSystemBook' => true, - 'UID' => 'User' + 'UID' => 'User', ] ], ['emails' => [], 'exact' => ['emails' => []]], @@ -679,7 +974,7 @@ class MailPluginTest extends TestCase { false, [ 'currentUser' => ['group1'], - 'User' => ['group2'] + 'User' => ['group2'], ], false, ], @@ -692,7 +987,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']]]]], @@ -700,10 +995,141 @@ class MailPluginTest extends TestCase { false, [ 'currentUser' => ['group1'], - 'User' => ['group2'] + 'User' => ['group2'], ], true, ] ]; } + + /** + * @dataProvider dataSearchUserGroupsOnly + * + * @param string $searchTerm + * @param array $contacts + * @param array $expectedResult + * @param bool $expectedExactIdMatch + * @param bool $expectedMoreResults + * @param array $userToGroupMapping + */ + 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 function dataSearchUserGroupsOnly() { + 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'], + ], + ] + ]; + } }