Skip to content

Commit c40fcba

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 <[email protected]>
1 parent 2c841b2 commit c40fcba

File tree

9 files changed

+589
-66
lines changed

9 files changed

+589
-66
lines changed

‎build/integration/collaboration_features/autocomplete.feature‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -107,28 +107,22 @@ 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
| leon@example.com | emails |
115113
| user@example.com | emails |
116114
Then get email autocomplete for "[email protected]"
117115
| id | source |
118-
| autocomplete | users |
119116
| autocomplete@example.com | emails |
120117
When parameter "shareapi_restrict_user_enumeration_full_match" of app "core" is set to "yes"
121118
Then get email autocomplete for "auto"
122119
| id | source |
123-
| autocomplete | users |
124120
Then get email autocomplete for "example"
125121
| id | source |
126-
| autocomplete | users |
127122
| leon@example.com | emails |
128123
| user@example.com | emails |
129124
Then get email autocomplete for "[email protected]"
130125
| id | source |
131-
| autocomplete | users |
132126

133127
Scenario: getting autocomplete emails from address book without enumeration
134128
Given As an "admin"
@@ -156,7 +150,6 @@ Feature: autocomplete
156150
| user@example.com | emails |
157151
Then get email autocomplete for "[email protected]"
158152
| id | source |
159-
| autocomplete | users |
160153

161154
Scenario: getting autocomplete with limited enumeration by group
162155
Given As an "admin"

‎build/integration/sharees_features/sharees.feature‎

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,7 @@ Feature: sharees
281281
And the HTTP status code should be "200"
282282
# UserPlugin provides two identical results (except for the field order, but
283283
# that is hidden by the check).
284+
# MailPlugin does not add a result if there is already one for that user.
284285
And "exact users" sharees returned are
285286
| Sharee2 | 0 | Sharee2 | sharee2@system.com |
286287
| Sharee2 | 0 | Sharee2 | sharee2@system.com |
@@ -301,6 +302,7 @@ Feature: sharees
301302
Then the OCS status code should be "100"
302303
And the HTTP status code should be "200"
303304
And "exact users" sharees returned is empty
305+
# MailPlugin does not add a result if there is already one for that user.
304306
And "users" sharees returned are
305307
| Sharee2 | 0 | Sharee2 | sharee2@system.com |
306308
And "exact groups" sharees returned is empty
@@ -320,7 +322,8 @@ Feature: sharees
320322
And the HTTP status code should be "200"
321323
# UserPlugin only searches in the system e-mail address, but not in
322324
# secondary addresses.
323-
And "exact users" sharees returned is empty
325+
And "exact users" sharees returned are
326+
| Sharee2 (sharee2@secondary.com) | 0 | Sharee2 | sharee2@secondary.com |
324327
And "users" sharees returned is empty
325328
And "exact groups" sharees returned is empty
326329
And "groups" sharees returned is empty
@@ -340,7 +343,11 @@ Feature: sharees
340343
And "exact users" sharees returned is empty
341344
# UserPlugin only searches in the system e-mail address, but not in
342345
# secondary addresses.
343-
And "users" sharees returned is empty
346+
# MailPlugin adds a result for every e-mail address of the contact unless
347+
# there is an exact match.
348+
And "users" sharees returned are
349+
| Sharee2 (sharee2@system.com) | 0 | Sharee2 | sharee2@system.com |
350+
| Sharee2 (sharee2@secondary.com) | 0 | Sharee2 | sharee2@secondary.com |
344351
And "exact groups" sharees returned is empty
345352
And "groups" sharees returned is empty
346353
And "exact remotes" sharees returned is empty
@@ -394,8 +401,7 @@ Feature: sharees
394401
| shareType | 4 |
395402
Then the OCS status code should be "100"
396403
And the HTTP status code should be "200"
397-
And "exact users" sharees returned are
398-
| Sharee2 (sharee2@system.com) | 0 | Sharee2 | sharee2@system.com |
404+
And "exact users" sharees returned is empty
399405
And "users" sharees returned is empty
400406
And "exact groups" sharees returned is empty
401407
And "groups" sharees returned is empty
@@ -413,11 +419,7 @@ Feature: sharees
413419
Then the OCS status code should be "100"
414420
And the HTTP status code should be "200"
415421
And "exact users" sharees returned is empty
416-
# MailPlugin adds a result for every e-mail address of the contact unless
417-
# there is an exact match.
418-
And "users" sharees returned are
419-
| Sharee2 (sharee2@system.com) | 0 | Sharee2 | sharee2@system.com |
420-
| Sharee2 (sharee2@secondary.com) | 0 | Sharee2 | sharee2@secondary.com |
422+
And "users" sharees returned is empty
421423
And "exact groups" sharees returned is empty
422424
And "groups" sharees returned is empty
423425
And "exact remotes" sharees returned is empty
@@ -434,8 +436,7 @@ Feature: sharees
434436
| shareType | 4 |
435437
Then the OCS status code should be "100"
436438
And the HTTP status code should be "200"
437-
And "exact users" sharees returned are
438-
| Sharee2 (sharee2@secondary.com) | 0 | Sharee2 | sharee2@secondary.com |
439+
And "exact users" sharees returned is empty
439440
And "users" sharees returned is empty
440441
And "exact groups" sharees returned is empty
441442
And "groups" sharees returned is empty
@@ -453,11 +454,7 @@ Feature: sharees
453454
Then the OCS status code should be "100"
454455
And the HTTP status code should be "200"
455456
And "exact users" sharees returned is empty
456-
# MailPlugin adds a result for every e-mail address of the contact unless
457-
# there is an exact match.
458-
And "users" sharees returned are
459-
| Sharee2 (sharee2@system.com) | 0 | Sharee2 | sharee2@system.com |
460-
| Sharee2 (sharee2@secondary.com) | 0 | Sharee2 | sharee2@secondary.com |
457+
And "users" sharees returned is empty
461458
And "exact groups" sharees returned is empty
462459
And "groups" sharees returned is empty
463460
And "exact remotes" sharees returned is empty
@@ -540,7 +537,8 @@ Feature: sharees
540537
| shareTypes | 0 4 |
541538
Then the OCS status code should be "100"
542539
And the HTTP status code should be "200"
543-
And "exact users" sharees returned is empty
540+
And "exact users" sharees returned are
541+
| Sharee2 (sharee2@secondary.com) | 0 | Sharee2 | sharee2@secondary.com |
544542
And "users" sharees returned is empty
545543
And "exact groups" sharees returned is empty
546544
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
@@ -1202,11 +1202,13 @@
12021202
'OC\\Collaboration\\AutoComplete\\Manager' => $baseDir . '/lib/private/Collaboration/AutoComplete/Manager.php',
12031203
'OC\\Collaboration\\Collaborators\\GroupPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/GroupPlugin.php',
12041204
'OC\\Collaboration\\Collaborators\\LookupPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/LookupPlugin.php',
1205+
'OC\\Collaboration\\Collaborators\\MailByMailPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/MailByMailPlugin.php',
12051206
'OC\\Collaboration\\Collaborators\\MailPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/MailPlugin.php',
12061207
'OC\\Collaboration\\Collaborators\\RemoteGroupPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/RemoteGroupPlugin.php',
12071208
'OC\\Collaboration\\Collaborators\\RemotePlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/RemotePlugin.php',
12081209
'OC\\Collaboration\\Collaborators\\Search' => $baseDir . '/lib/private/Collaboration/Collaborators/Search.php',
12091210
'OC\\Collaboration\\Collaborators\\SearchResult' => $baseDir . '/lib/private/Collaboration/Collaborators/SearchResult.php',
1211+
'OC\\Collaboration\\Collaborators\\UserByMailPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/UserByMailPlugin.php',
12101212
'OC\\Collaboration\\Collaborators\\UserPlugin' => $baseDir . '/lib/private/Collaboration/Collaborators/UserPlugin.php',
12111213
'OC\\Collaboration\\Reference\\File\\FileReferenceEventListener' => $baseDir . '/lib/private/Collaboration/Reference/File/FileReferenceEventListener.php',
12121214
'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
@@ -1243,11 +1243,13 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
12431243
'OC\\Collaboration\\AutoComplete\\Manager' => __DIR__ . '/../../..' . '/lib/private/Collaboration/AutoComplete/Manager.php',
12441244
'OC\\Collaboration\\Collaborators\\GroupPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/GroupPlugin.php',
12451245
'OC\\Collaboration\\Collaborators\\LookupPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/LookupPlugin.php',
1246+
'OC\\Collaboration\\Collaborators\\MailByMailPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/MailByMailPlugin.php',
12461247
'OC\\Collaboration\\Collaborators\\MailPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/MailPlugin.php',
12471248
'OC\\Collaboration\\Collaborators\\RemoteGroupPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/RemoteGroupPlugin.php',
12481249
'OC\\Collaboration\\Collaborators\\RemotePlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/RemotePlugin.php',
12491250
'OC\\Collaboration\\Collaborators\\Search' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/Search.php',
12501251
'OC\\Collaboration\\Collaborators\\SearchResult' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/SearchResult.php',
1252+
'OC\\Collaboration\\Collaborators\\UserByMailPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/UserByMailPlugin.php',
12511253
'OC\\Collaboration\\Collaborators\\UserPlugin' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Collaborators/UserPlugin.php',
12521254
'OC\\Collaboration\\Reference\\File\\FileReferenceEventListener' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Reference/File/FileReferenceEventListener.php',
12531255
'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\IEmailValidator;
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+
IEmailValidator $emailValidator,
31+
mixed $shareWithGroupOnlyExcludeGroupsList = [],
32+
) {
33+
parent::__construct(
34+
$contactsManager,
35+
$cloudIdManager,
36+
$config,
37+
$groupManager,
38+
$knownUserService,
39+
$userSession,
40+
$emailValidator,
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
@@ -41,7 +41,8 @@ public function __construct(
4141
private KnownUserService $knownUserService,
4242
private IUserSession $userSession,
4343
private IEmailValidator $emailValidator,
44-
private mixed $shareWithGroupOnlyExcludeGroupsList = [],
44+
private mixed $shareWithGroupOnlyExcludeGroupsList,
45+
private int $shareType,
4546
) {
4647
$this->shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
4748
$this->shareWithGroupOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes';
@@ -139,7 +140,7 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
139140
continue;
140141
}
141142

142-
if (!$this->isCurrentUser($cloud) && !$searchResult->hasResult($userType, $cloud->getUser())) {
143+
if ($this->shareType === IShare::TYPE_USER && !$this->isCurrentUser($cloud) && !$searchResult->hasResult($userType, $cloud->getUser())) {
143144
$singleResult = [[
144145
'label' => $displayName,
145146
'uuid' => $contact['UID'] ?? $emailAddress,
@@ -183,22 +184,28 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
183184
}
184185
}
185186
if ($addToWide && !$this->isCurrentUser($cloud) && !$searchResult->hasResult($userType, $cloud->getUser())) {
186-
$userResults['wide'][] = [
187-
'label' => $displayName,
188-
'uuid' => $contact['UID'] ?? $emailAddress,
189-
'name' => $contact['FN'] ?? $displayName,
190-
'value' => [
191-
'shareType' => IShare::TYPE_USER,
192-
'shareWith' => $cloud->getUser(),
193-
],
194-
'shareWithDisplayNameUnique' => !empty($emailAddress) ? $emailAddress : $cloud->getUser()
195-
];
187+
if ($this->shareType === IShare::TYPE_USER) {
188+
$userResults['wide'][] = [
189+
'label' => $displayName,
190+
'uuid' => $contact['UID'] ?? $emailAddress,
191+
'name' => $contact['FN'] ?? $displayName,
192+
'value' => [
193+
'shareType' => IShare::TYPE_USER,
194+
'shareWith' => $cloud->getUser(),
195+
],
196+
'shareWithDisplayNameUnique' => !empty($emailAddress) ? $emailAddress : $cloud->getUser()
197+
];
198+
}
196199
continue;
197200
}
198201
}
199202
continue;
200203
}
201204

205+
if ($this->shareType !== IShare::TYPE_EMAIL) {
206+
continue;
207+
}
208+
202209
if ($exactEmailMatch
203210
|| (isset($contact['FN']) && strtolower($contact['FN']) === $lowerSearch)) {
204211
if ($exactEmailMatch) {
@@ -239,7 +246,8 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
239246
$userResults['wide'] = array_slice($userResults['wide'], $offset, $limit);
240247
}
241248

242-
if (!$searchResult->hasExactIdMatch($emailType) && $this->emailValidator->isValid($search)) {
249+
if ($this->shareType === IShare::TYPE_EMAIL
250+
&& !$searchResult->hasExactIdMatch($emailType) && $this->emailValidator->isValid($search)) {
243251
$result['exact'][] = [
244252
'label' => $search,
245253
'uuid' => $search,
@@ -250,10 +258,12 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
250258
];
251259
}
252260

253-
if (!empty($userResults['wide'])) {
261+
if ($this->shareType === IShare::TYPE_USER && !empty($userResults['wide'])) {
254262
$searchResult->addResultSet($userType, $userResults['wide'], []);
255263
}
256-
$searchResult->addResultSet($emailType, $result['wide'], $result['exact']);
264+
if ($this->shareType === IShare::TYPE_EMAIL) {
265+
$searchResult->addResultSet($emailType, $result['wide'], $result['exact']);
266+
}
257267

258268
return !$reachedEnd;
259269
}

‎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\IEmailValidator;
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+
IEmailValidator $emailValidator,
31+
mixed $shareWithGroupOnlyExcludeGroupsList = [],
32+
) {
33+
parent::__construct(
34+
$contactsManager,
35+
$cloudIdManager,
36+
$config,
37+
$groupManager,
38+
$knownUserService,
39+
$userSession,
40+
$emailValidator,
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;
@@ -1112,8 +1113,9 @@ public function __construct($webRoot, \OC\Config $config) {
11121113

11131114
// register default plugins
11141115
$instance->registerPlugin(['shareType' => 'SHARE_TYPE_USER', 'class' => UserPlugin::class]);
1116+
$instance->registerPlugin(['shareType' => 'SHARE_TYPE_USER', 'class' => UserByMailPlugin::class]);
11151117
$instance->registerPlugin(['shareType' => 'SHARE_TYPE_GROUP', 'class' => GroupPlugin::class]);
1116-
$instance->registerPlugin(['shareType' => 'SHARE_TYPE_EMAIL', 'class' => MailPlugin::class]);
1118+
$instance->registerPlugin(['shareType' => 'SHARE_TYPE_EMAIL', 'class' => MailByMailPlugin::class]);
11171119
$instance->registerPlugin(['shareType' => 'SHARE_TYPE_REMOTE', 'class' => RemotePlugin::class]);
11181120
$instance->registerPlugin(['shareType' => 'SHARE_TYPE_REMOTE_GROUP', 'class' => RemoteGroupPlugin::class]);
11191121

0 commit comments

Comments
 (0)