Skip to content

Commit 5bdf2f6

Browse files
danxuliudanxuliu
committed
fix: Fix user collaborators returned when searching for mail collaborators
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 <danxuliu@gmail.com>
1 parent f53acfa commit 5bdf2f6

File tree

9 files changed

+590
-67
lines changed

9 files changed

+590
-67
lines changed

‎build/integration/collaboration_features/autocomplete.feature‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -107,26 +107,20 @@ Feature: autocomplete
107107
When parameter "shareapi_restrict_user_enumeration_full_match" of app "core" is set to "no"
108108
Then get email autocomplete for "auto"
109109
| id | source |
110-
| autocomplete | users |
111110
Then get email autocomplete for "example"
112111
| id | source |
113-
| autocomplete | users |
114112
| user@example.com | emails |
115113
Then get email autocomplete for "autocomplete@example.com"
116114
| id | source |
117-
| autocomplete | users |
118115
| autocomplete@example.com | emails |
119116
When parameter "shareapi_restrict_user_enumeration_full_match" of app "core" is set to "yes"
120117
Then get email autocomplete for "auto"
121118
| id | source |
122-
| autocomplete | users |
123119
Then get email autocomplete for "example"
124120
| id | source |
125-
| autocomplete | users |
126121
| user@example.com | emails |
127122
Then get email autocomplete for "autocomplete@example.com"
128123
| id | source |
129-
| autocomplete | users |
130124

131125
Scenario: getting autocomplete emails from address book without enumeration
132126
Given As an "admin"
@@ -152,7 +146,6 @@ Feature: autocomplete
152146
| user@example.com | emails |
153147
Then get email autocomplete for "autocomplete@example.com"
154148
| id | source |
155-
| autocomplete | users |
156149

157150
Scenario: getting autocomplete with limited enumeration by group
158151
Given As an "admin"

‎build/integration/sharees_features/sharees.feature‎

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,7 @@ Feature: sharees
280280
And the HTTP status code should be "200"
281281
# UserPlugin provides two identical results (except for the field order, but
282282
# that is hidden by the check).
283+
# MailPlugin does not add a result if there is already one for that user.
283284
And "exact users" sharees returned are
284285
| Sharee2 | 0 | Sharee2 | sharee2@system.com |
285286
| Sharee2 | 0 | Sharee2 | sharee2@system.com |
@@ -300,6 +301,7 @@ Feature: sharees
300301
Then the OCS status code should be "100"
301302
And the HTTP status code should be "200"
302303
And "exact users" sharees returned is empty
304+
# MailPlugin does not add a result if there is already one for that user.
303305
And "users" sharees returned are
304306
| Sharee2 | 0 | Sharee2 | sharee2@system.com |
305307
And "exact groups" sharees returned is empty
@@ -319,7 +321,8 @@ Feature: sharees
319321
And the HTTP status code should be "200"
320322
# UserPlugin only searches in the system e-mail address, but not in
321323
# secondary addresses.
322-
And "exact users" sharees returned is empty
324+
And "exact users" sharees returned are
325+
| Sharee2 (sharee2@secondary.com) | 0 | Sharee2 | sharee2@secondary.com |
323326
And "users" sharees returned is empty
324327
And "exact groups" sharees returned is empty
325328
And "groups" sharees returned is empty
@@ -339,7 +342,11 @@ Feature: sharees
339342
And "exact users" sharees returned is empty
340343
# UserPlugin only searches in the system e-mail address, but not in
341344
# secondary addresses.
342-
And "users" sharees returned is empty
345+
# MailPlugin adds a result for every e-mail address of the contact unless
346+
# there is an exact match.
347+
And "users" sharees returned are
348+
| Sharee2 (sharee2@system.com) | 0 | Sharee2 | sharee2@system.com |
349+
| Sharee2 (sharee2@secondary.com) | 0 | Sharee2 | sharee2@secondary.com |
343350
And "exact groups" sharees returned is empty
344351
And "groups" sharees returned is empty
345352
And "exact remotes" sharees returned is empty
@@ -393,8 +400,7 @@ Feature: sharees
393400
| shareType | 4 |
394401
Then the OCS status code should be "100"
395402
And the HTTP status code should be "200"
396-
And "exact users" sharees returned are
397-
| Sharee2 (sharee2@system.com) | 0 | Sharee2 | sharee2@system.com |
403+
And "exact users" sharees returned is empty
398404
And "users" sharees returned is empty
399405
And "exact groups" sharees returned is empty
400406
And "groups" sharees returned is empty
@@ -412,11 +418,7 @@ Feature: sharees
412418
Then the OCS status code should be "100"
413419
And the HTTP status code should be "200"
414420
And "exact users" sharees returned is empty
415-
# MailPlugin adds a result for every e-mail address of the contact unless
416-
# there is an exact match.
417-
And "users" sharees returned are
418-
| Sharee2 (sharee2@system.com) | 0 | Sharee2 | sharee2@system.com |
419-
| Sharee2 (sharee2@secondary.com) | 0 | Sharee2 | sharee2@secondary.com |
421+
And "users" sharees returned is empty
420422
And "exact groups" sharees returned is empty
421423
And "groups" sharees returned is empty
422424
And "exact remotes" sharees returned is empty
@@ -433,8 +435,7 @@ Feature: sharees
433435
| shareType | 4 |
434436
Then the OCS status code should be "100"
435437
And the HTTP status code should be "200"
436-
And "exact users" sharees returned are
437-
| Sharee2 (sharee2@secondary.com) | 0 | Sharee2 | sharee2@secondary.com |
438+
And "exact users" sharees returned is empty
438439
And "users" sharees returned is empty
439440
And "exact groups" sharees returned is empty
440441
And "groups" sharees returned is empty
@@ -452,11 +453,7 @@ Feature: sharees
452453
Then the OCS status code should be "100"
453454
And the HTTP status code should be "200"
454455
And "exact users" sharees returned is empty
455-
# MailPlugin adds a result for every e-mail address of the contact unless
456-
# there is an exact match.
457-
And "users" sharees returned are
458-
| Sharee2 (sharee2@system.com) | 0 | Sharee2 | sharee2@system.com |
459-
| Sharee2 (sharee2@secondary.com) | 0 | Sharee2 | sharee2@secondary.com |
456+
And "users" sharees returned is empty
460457
And "exact groups" sharees returned is empty
461458
And "groups" sharees returned is empty
462459
And "exact remotes" sharees returned is empty
@@ -539,7 +536,8 @@ Feature: sharees
539536
| shareTypes | 0 4 |
540537
Then the OCS status code should be "100"
541538
And the HTTP status code should be "200"
542-
And "exact users" sharees returned is empty
539+
And "exact users" sharees returned are
540+
| Sharee2 (sharee2@secondary.com) | 0 | Sharee2 | sharee2@secondary.com |
543541
And "users" sharees returned is empty
544542
And "exact groups" sharees returned is empty
545543
And "groups" sharees returned is empty

‎lib/composer/composer/autoload_classmap.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1154,11 +1154,13 @@
11541154
'OC\\Collaboration\\AutoComplete\\Manager' => $baseDir . '/lib/private/Collaboration/AutoComplete/Manager.php',
11551155
'OC\\Collaboration\\Collaborators\\GroupPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/GroupPlugin.php',
11561156
'OC\\Collaboration\\Collaborators\\LookupPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/LookupPlugin.php',
1157+
'OC\\Collaboration\\Collaborators\\MailByMailPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/MailByMailPlugin.php',
11571158
'OC\\Collaboration\\Collaborators\\MailPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/MailPlugin.php',
11581159
'OC\\Collaboration\\Collaborators\\RemoteGroupPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/RemoteGroupPlugin.php',
11591160
'OC\\Collaboration\\Collaborators\\RemotePlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/RemotePlugin.php',
11601161
'OC\\Collaboration\\Collaborators\\Search' => $baseDir . '/lib/private/Collaboration/Collaborators/Search.php',
11611162
'OC\\Collaboration\\Collaborators\\SearchResult' => $baseDir . '/lib/private/Collaboration/Collaborators/SearchResult.php',
1163+
'OC\\Collaboration\\Collaborators\\UserByMailPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/UserByMailPlugin.php',
11621164
'OC\\Collaboration\\Collaborators\\UserPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/UserPlugin.php',
11631165
'OC\\Collaboration\\Reference\\File\\FileReferenceEventListener' => $baseDir . '/lib/private/Collaboration/Reference/File/FileReferenceEventListener.php',
11641166
'OC\\Collaboration\\Reference\\File\\FileReferenceProvider' => $baseDir . '/lib/private/Collaboration/Reference/File/FileReferenceProvider.php',

‎lib/composer/composer/autoload_static.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1203,11 +1203,13 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
12031203
'OC\\Collaboration\\AutoComplete\\Manager' => __DIR__ . '/../../..' . '/lib/private/Collaboration/AutoComplete/Manager.php',
12041204
'OC\\Collaboration\\Collaborators\\GroupPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/GroupPlugin.php',
12051205
'OC\\Collaboration\\Collaborators\\LookupPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/LookupPlugin.php',
1206+
'OC\\Collaboration\\Collaborators\\MailByMailPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/MailByMailPlugin.php',
12061207
'OC\\Collaboration\\Collaborators\\MailPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/MailPlugin.php',
12071208
'OC\\Collaboration\\Collaborators\\RemoteGroupPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/RemoteGroupPlugin.php',
12081209
'OC\\Collaboration\\Collaborators\\RemotePlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/RemotePlugin.php',
12091210
'OC\\Collaboration\\Collaborators\\Search' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/Search.php',
12101211
'OC\\Collaboration\\Collaborators\\SearchResult' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/SearchResult.php',
1212+
'OC\\Collaboration\\Collaborators\\UserByMailPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/UserByMailPlugin.php',
12111213
'OC\\Collaboration\\Collaborators\\UserPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/UserPlugin.php',
12121214
'OC\\Collaboration\\Reference\\File\\FileReferenceEventListener' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Reference/File/FileReferenceEventListener.php',
12131215
'OC\\Collaboration\\Reference\\File\\FileReferenceProvider' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Reference/File/FileReferenceProvider.php',

‎lib/private/Collaboration/Collaborators/MailByMailPlugin.php‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<?php
2+
3+
/**
4+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
5+
* SPDX-License-Identifier: AGPL-3.0-or-later
6+
*/
7+
namespace OC\Collaboration\Collaborators;
8+
9+
use OC\KnownUser\KnownUserService;
10+
use OCP\Contacts\IManager;
11+
use OCP\Federation\ICloudIdManager;
12+
use OCP\IConfig;
13+
use OCP\IGroupManager;
14+
use OCP\IUserSession;
15+
use OCP\Mail\IMailer;
16+
use OCP\Share\IShare;
17+
18+
/**
19+
* Dummy subclass to initialize a MailPlugin with a specific share type.
20+
*/
21+
class MailByMailPlugin extends MailPlugin {
22+
23+
public function __construct(
24+
IManager $contactsManager,
25+
ICloudIdManager $cloudIdManager,
26+
IConfig $config,
27+
IGroupManager $groupManager,
28+
KnownUserService $knownUserService,
29+
IUserSession $userSession,
30+
IMailer $mailer,
31+
mixed $shareWithGroupOnlyExcludeGroupsList = [],
32+
) {
33+
parent::__construct(
34+
$contactsManager,
35+
$cloudIdManager,
36+
$config,
37+
$groupManager,
38+
$knownUserService,
39+
$userSession,
40+
$mailer,
41+
$shareWithGroupOnlyExcludeGroupsList,
42+
IShare::TYPE_EMAIL,
43+
);
44+
}
45+
}

‎lib/private/Collaboration/Collaborators/MailPlugin.php‎

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@ public function __construct(
4040
private KnownUserService $knownUserService,
4141
private IUserSession $userSession,
4242
private IMailer $mailer,
43-
private mixed $shareWithGroupOnlyExcludeGroupsList = [],
43+
private mixed $shareWithGroupOnlyExcludeGroupsList,
44+
private int $shareType,
4445
) {
4546
$this->shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
4647
$this->shareWithGroupOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes';
@@ -138,7 +139,7 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
138139
continue;
139140
}
140141

141-
if (!$this->isCurrentUser($cloud) && !$searchResult->hasResult($userType, $cloud->getUser())) {
142+
if ($this->shareType === IShare::TYPE_USER && !$this->isCurrentUser($cloud) && !$searchResult->hasResult($userType, $cloud->getUser())) {
142143
$singleResult = [[
143144
'label' => $displayName,
144145
'uuid' => $contact['UID'] ?? $emailAddress,
@@ -179,22 +180,28 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
179180
}
180181
}
181182
if ($addToWide && !$this->isCurrentUser($cloud) && !$searchResult->hasResult($userType, $cloud->getUser())) {
182-
$userResults['wide'][] = [
183-
'label' => $displayName,
184-
'uuid' => $contact['UID'] ?? $emailAddress,
185-
'name' => $contact['FN'] ?? $displayName,
186-
'value' => [
187-
'shareType' => IShare::TYPE_USER,
188-
'shareWith' => $cloud->getUser(),
189-
],
190-
'shareWithDisplayNameUnique' => !empty($emailAddress) ? $emailAddress : $cloud->getUser()
191-
];
183+
if ($this->shareType === IShare::TYPE_USER) {
184+
$userResults['wide'][] = [
185+
'label' => $displayName,
186+
'uuid' => $contact['UID'] ?? $emailAddress,
187+
'name' => $contact['FN'] ?? $displayName,
188+
'value' => [
189+
'shareType' => IShare::TYPE_USER,
190+
'shareWith' => $cloud->getUser(),
191+
],
192+
'shareWithDisplayNameUnique' => !empty($emailAddress) ? $emailAddress : $cloud->getUser()
193+
];
194+
}
192195
continue;
193196
}
194197
}
195198
continue;
196199
}
197200

201+
if ($this->shareType !== IShare::TYPE_EMAIL) {
202+
continue;
203+
}
204+
198205
if ($exactEmailMatch
199206
|| (isset($contact['FN']) && strtolower($contact['FN']) === $lowerSearch)) {
200207
if ($exactEmailMatch) {
@@ -235,7 +242,8 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
235242
$userResults['wide'] = array_slice($userResults['wide'], $offset, $limit);
236243
}
237244

238-
if (!$searchResult->hasExactIdMatch($emailType) && $this->mailer->validateMailAddress($search)) {
245+
if ($this->shareType === IShare::TYPE_EMAIL
246+
&& !$searchResult->hasExactIdMatch($emailType) && $this->mailer->validateMailAddress($search)) {
239247
$result['exact'][] = [
240248
'label' => $search,
241249
'uuid' => $search,
@@ -246,10 +254,12 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
246254
];
247255
}
248256

249-
if (!empty($userResults['wide'])) {
257+
if ($this->shareType === IShare::TYPE_USER && !empty($userResults['wide'])) {
250258
$searchResult->addResultSet($userType, $userResults['wide'], []);
251259
}
252-
$searchResult->addResultSet($emailType, $result['wide'], $result['exact']);
260+
if ($this->shareType === IShare::TYPE_EMAIL) {
261+
$searchResult->addResultSet($emailType, $result['wide'], $result['exact']);
262+
}
253263

254264
return !$reachedEnd;
255265
}

‎lib/private/Collaboration/Collaborators/UserByMailPlugin.php‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<?php
2+
3+
/**
4+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
5+
* SPDX-License-Identifier: AGPL-3.0-or-later
6+
*/
7+
namespace OC\Collaboration\Collaborators;
8+
9+
use OC\KnownUser\KnownUserService;
10+
use OCP\Contacts\IManager;
11+
use OCP\Federation\ICloudIdManager;
12+
use OCP\IConfig;
13+
use OCP\IGroupManager;
14+
use OCP\IUserSession;
15+
use OCP\Mail\IMailer;
16+
use OCP\Share\IShare;
17+
18+
/**
19+
* Dummy subclass to initialize a MailPlugin with a specific share type.
20+
*/
21+
class UserByMailPlugin extends MailPlugin {
22+
23+
public function __construct(
24+
IManager $contactsManager,
25+
ICloudIdManager $cloudIdManager,
26+
IConfig $config,
27+
IGroupManager $groupManager,
28+
KnownUserService $knownUserService,
29+
IUserSession $userSession,
30+
IMailer $mailer,
31+
mixed $shareWithGroupOnlyExcludeGroupsList = [],
32+
) {
33+
parent::__construct(
34+
$contactsManager,
35+
$cloudIdManager,
36+
$config,
37+
$groupManager,
38+
$knownUserService,
39+
$userSession,
40+
$mailer,
41+
$shareWithGroupOnlyExcludeGroupsList,
42+
IShare::TYPE_USER,
43+
);
44+
}
45+
}

‎lib/private/Server.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,10 @@
2525
use OC\Avatar\AvatarManager;
2626
use OC\Blurhash\Listener\GenerateBlurhashMetadata;
2727
use OC\Collaboration\Collaborators\GroupPlugin;
28-
use OC\Collaboration\Collaborators\MailPlugin;
28+
use OC\Collaboration\Collaborators\MailByMailPlugin;
2929
use OC\Collaboration\Collaborators\RemoteGroupPlugin;
3030
use OC\Collaboration\Collaborators\RemotePlugin;
31+
use OC\Collaboration\Collaborators\UserByMailPlugin;
3132
use OC\Collaboration\Collaborators\UserPlugin;
3233
use OC\Collaboration\Reference\ReferenceManager;
3334
use OC\Command\CronBus;
@@ -1136,8 +1137,9 @@ public function __construct($webRoot, \OC\Config $config) {
11361137

11371138
// register default plugins
11381139
$instance->registerPlugin(['shareType' => 'SHARE_TYPE_USER', 'class' => UserPlugin::class]);
1140+
$instance->registerPlugin(['shareType' => 'SHARE_TYPE_USER', 'class' => UserByMailPlugin::class]);
11391141
$instance->registerPlugin(['shareType' => 'SHARE_TYPE_GROUP', 'class' => GroupPlugin::class]);
1140-
$instance->registerPlugin(['shareType' => 'SHARE_TYPE_EMAIL', 'class' => MailPlugin::class]);
1142+
$instance->registerPlugin(['shareType' => 'SHARE_TYPE_EMAIL', 'class' => MailByMailPlugin::class]);
11411143
$instance->registerPlugin(['shareType' => 'SHARE_TYPE_REMOTE', 'class' => RemotePlugin::class]);
11421144
$instance->registerPlugin(['shareType' => 'SHARE_TYPE_REMOTE_GROUP', 'class' => RemoteGroupPlugin::class]);
11431145

0 commit comments

Comments
 (0)