Skip to content

Commit 27f4f2d

Browse files
susnuxsusnux
authored
Merge pull request #55328 from nextcloud/backport/55311/stable31
[stable31] fix: add missing sharing options to ui and add full-match results
2 parents d383f00 + 970122f commit 27f4f2d

File tree

9 files changed

+98
-55
lines changed

9 files changed

+98
-55
lines changed

‎apps/settings/lib/Settings/Admin/Sharing.php‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ public function getForm() {
3737
$excludedPasswordGroups = $this->config->getAppValue('core', 'shareapi_enforce_links_password_excluded_groups', '');
3838
$onlyShareWithGroupMembersExcludeGroupList = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members_exclude_group_list', '');
3939

40+
/** @var \OC\Share20\Manager */
41+
$share20Manager = $this->shareManager;
4042
$parameters = [
4143
// Built-In Sharing
4244
'enabled' => $this->getHumanBooleanConfig('core', 'shareapi_enabled', true),
@@ -48,10 +50,10 @@ public function getForm() {
4850
'allowShareDialogUserEnumeration' => $this->getHumanBooleanConfig('core', 'shareapi_allow_share_dialog_user_enumeration', true),
4951
'restrictUserEnumerationToGroup' => $this->getHumanBooleanConfig('core', 'shareapi_restrict_user_enumeration_to_group'),
5052
'restrictUserEnumerationToPhone' => $this->getHumanBooleanConfig('core', 'shareapi_restrict_user_enumeration_to_phone'),
51-
'restrictUserEnumerationFullMatch' => $this->getHumanBooleanConfig('core', 'shareapi_restrict_user_enumeration_full_match', true),
52-
'restrictUserEnumerationFullMatchUserId' => $this->getHumanBooleanConfig('core', 'shareapi_restrict_user_enumeration_full_match_userid', true),
53-
'restrictUserEnumerationFullMatchEmail' => $this->getHumanBooleanConfig('core', 'shareapi_restrict_user_enumeration_full_match_email', true),
54-
'restrictUserEnumerationFullMatchIgnoreSecondDN' => $this->getHumanBooleanConfig('core', 'shareapi_restrict_user_enumeration_full_match_ignore_second_dn'),
53+
'restrictUserEnumerationFullMatch' => $this->shareManager->allowEnumerationFullMatch(),
54+
'restrictUserEnumerationFullMatchUserId' => $share20Manager->matchUserId(),
55+
'restrictUserEnumerationFullMatchEmail' => $this->shareManager->matchEmail(),
56+
'restrictUserEnumerationFullMatchIgnoreSecondDN' => $this->shareManager->ignoreSecondDisplayName(),
5557
'enforceLinksPassword' => Util::isPublicLinkPasswordRequired(false),
5658
'enforceLinksPasswordExcludedGroups' => json_decode($excludedPasswordGroups) ?? [],
5759
'enforceLinksPasswordExcludedGroupsEnabled' => $this->config->getSystemValueBool('sharing.allow_disabled_password_enforcement_groups', false),

‎apps/settings/src/components/AdminSettingsSharingForm.vue‎

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
-->
55
<template>
66
<form class="sharing">
7-
<NcCheckboxRadioSwitch aria-controls="settings-sharing-api settings-sharing-api-settings settings-sharing-default-permissions settings-sharing-privary-related"
7+
<NcCheckboxRadioSwitch aria-controls="settings-sharing-api settings-sharing-api-settings settings-sharing-default-permissions settings-sharing-privacy-related"
88
type="switch"
99
:checked.sync="settings.enabled">
1010
{{ t('settings', 'Allow apps to use the Share API') }}
@@ -161,7 +161,7 @@
161161
</fieldset>
162162
</div>
163163

164-
<div v-show="settings.enabled" id="settings-sharing-privary-related" class="sharing__section">
164+
<div v-show="settings.enabled" id="settings-sharing-privacy-related" class="sharing__section">
165165
<h3>{{ t('settings', 'Privacy settings for sharing') }}</h3>
166166

167167
<NcCheckboxRadioSwitch type="switch"
@@ -170,33 +170,52 @@
170170
{{ t('settings', 'Allow account name autocompletion in share dialog and allow access to the system address book') }}
171171
</NcCheckboxRadioSwitch>
172172
<fieldset v-show="settings.allowShareDialogUserEnumeration" id="settings-sharing-privacy-user-enumeration" class="sharing__sub-section">
173+
<legend class="hidden-visually">
174+
{{ t('settings', 'Sharing autocompletion restrictions') }}
175+
</legend>
173176
<em>
174-
{{ t('settings', 'If autocompletion "same group" and "phone number integration" are enabled a match in either is enough to show the user.') }}
177+
{{ t('settings', 'If autocompletion restrictions for both "same group" and "phonebook integration" are enabled, a match in either is enough to show the user.') }}
175178
</em>
176179
<NcCheckboxRadioSwitch :checked.sync="settings.restrictUserEnumerationToGroup">
177180
{{ t('settings', 'Restrict account name autocompletion and system address book access to users within the same groups') }}
178181
</NcCheckboxRadioSwitch>
179182
<NcCheckboxRadioSwitch :checked.sync="settings.restrictUserEnumerationToPhone">
180-
{{ t('settings', 'Restrict account name autocompletion to users based on phone number integration') }}
183+
{{ t('settings', 'Restrict account name autocompletion to users based on their phonebook') }}
181184
</NcCheckboxRadioSwitch>
182185
</fieldset>
183186

184-
<NcCheckboxRadioSwitch type="switch" :checked.sync="settings.restrictUserEnumerationFullMatch">
185-
{{ t('settings', 'Allow autocompletion when entering the full name or email address (ignoring missing phonebook match and being in the same group)') }}
187+
<NcCheckboxRadioSwitch v-model="settings.restrictUserEnumerationFullMatch"
188+
type="switch"
189+
aria-controls="settings-sharing-privacy-autocomplete">
190+
{{ t('settings', 'Allow autocompletion to full match when entering the full name (ignoring restrictions like group membership or missing phonebook match)') }}
186191
</NcCheckboxRadioSwitch>
192+
<fieldset v-show="settings.restrictUserEnumerationFullMatch" id="settings-sharing-privacy-autocomplete" class="sharing__sub-section">
193+
<legend class="hidden-visually">
194+
{{ t('settings', 'Full match autocompletion restrictions') }}
195+
</legend>
196+
<NcCheckboxRadioSwitch :checked.sync="settings.restrictUserEnumerationFullMatchUserId">
197+
{{ t('settings', 'Also allow autocompletion on full match of the user id') }}
198+
</NcCheckboxRadioSwitch>
199+
<NcCheckboxRadioSwitch :checked.sync="settings.restrictUserEnumerationFullMatchEmail">
200+
{{ t('settings', 'Also allow autocompletion on full match of the user email') }}
201+
</NcCheckboxRadioSwitch>
202+
<NcCheckboxRadioSwitch :checked.sync="settings.restrictUserEnumerationFullMatchIgnoreSecondDN">
203+
{{ t('settings', 'Do not use second user displayname for full match') }}
204+
</NcCheckboxRadioSwitch>
205+
</fieldset>
187206

188207
<NcCheckboxRadioSwitch type="switch" :checked.sync="publicShareDisclaimerEnabled">
189208
{{ t('settings', 'Show disclaimer text on the public link upload page (only shown when the file list is hidden)') }}
190209
</NcCheckboxRadioSwitch>
191210
<div v-if="publicShareDisclaimerEnabled"
192-
aria-describedby="settings-sharing-privary-related-disclaimer-hint"
211+
aria-describedby="settings-sharing-privacy-related-disclaimer-hint"
193212
class="sharing__sub-section">
194213
<NcTextArea class="sharing__input"
195214
:label="t('settings', 'Disclaimer text')"
196-
aria-describedby="settings-sharing-privary-related-disclaimer-hint"
215+
aria-describedby="settings-sharing-privacy-related-disclaimer-hint"
197216
:value="settings.publicShareDisclaimerText"
198217
@update:value="onUpdateDisclaimer" />
199-
<em id="settings-sharing-privary-related-disclaimer-hint" class="sharing__input">
218+
<em id="settings-sharing-privacy-related-disclaimer-hint" class="sharing__input">
200219
{{ t('settings', 'This text will be shown on the public link upload page when the file list is hidden.') }}
201220
</em>
202221
</div>

‎apps/settings/tests/Settings/Admin/SharingTest.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
*/
66
namespace OCA\Settings\Tests\Settings\Admin;
77

8+
use OC\Share20\Manager;
89
use OCA\Settings\Settings\Admin\Sharing;
910
use OCP\App\IAppManager;
1011
use OCP\AppFramework\Http\TemplateResponse;
@@ -13,7 +14,6 @@
1314
use OCP\IConfig;
1415
use OCP\IL10N;
1516
use OCP\IURLGenerator;
16-
use OCP\Share\IManager;
1717
use PHPUnit\Framework\MockObject\MockObject;
1818
use Test\TestCase;
1919

@@ -24,7 +24,7 @@ class SharingTest extends TestCase {
2424
private $config;
2525
/** @var IL10N&MockObject */
2626
private $l10n;
27-
/** @var IManager|MockObject */
27+
/** @var Manager|MockObject */
2828
private $shareManager;
2929
/** @var IAppManager|MockObject */
3030
private $appManager;
@@ -38,8 +38,8 @@ protected function setUp(): void {
3838
$this->config = $this->getMockBuilder(IConfig::class)->getMock();
3939
$this->l10n = $this->getMockBuilder(IL10N::class)->getMock();
4040

41-
/** @var IManager|MockObject */
42-
$this->shareManager = $this->getMockBuilder(IManager::class)->getMock();
41+
/** @var Manager|MockObject */
42+
$this->shareManager = $this->createMock(Manager::class);
4343
/** @var IAppManager|MockObject */
4444
$this->appManager = $this->getMockBuilder(IAppManager::class)->getMock();
4545
/** @var IURLGenerator|MockObject */

‎dist/settings-vue-settings-admin-sharing.js‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dist/settings-vue-settings-admin-sharing.js.map‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 41 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,10 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
6262
$users = [];
6363
$hasMoreResults = false;
6464

65-
$currentUserId = $this->userSession->getUser()->getUID();
66-
$currentUserGroups = $this->groupManager->getUserGroupIds($this->userSession->getUser());
65+
/** @var IUser */
66+
$currentUser = $this->userSession->getUser();
67+
$currentUserId = $currentUser->getUID();
68+
$currentUserGroups = $this->groupManager->getUserGroupIds($currentUser);
6769

6870
// ShareWithGroupOnly filtering
6971
$currentUserGroups = array_diff($currentUserGroups, $this->shareWithGroupOnlyExcludeGroupsList);
@@ -75,7 +77,7 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
7577
foreach ($usersInGroup as $userId => $displayName) {
7678
$userId = (string)$userId;
7779
$user = $this->userManager->get($userId);
78-
if (!$user->isEnabled()) {
80+
if (!$user?->isEnabled()) {
7981
// Ignore disabled users
8082
continue;
8183
}
@@ -85,37 +87,43 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
8587
$hasMoreResults = true;
8688
}
8789
}
90+
}
8891

89-
if (!$this->shareWithGroupOnly && $this->shareeEnumerationPhone) {
90-
$usersTmp = $this->userManager->searchKnownUsersByDisplayName($currentUserId, $search, $limit, $offset);
91-
if (!empty($usersTmp)) {
92+
// not limited to group only sharing
93+
if (!$this->shareWithGroupOnly) {
94+
if (!$this->shareeEnumerationPhone && !$this->shareeEnumerationInGroupOnly) {
95+
// no restrictions, add everything
96+
$usersTmp = $this->userManager->searchDisplayName($search, $limit, $offset);
97+
foreach ($usersTmp as $user) {
98+
if ($user->isEnabled()) { // Don't keep deactivated users
99+
$users[$user->getUID()] = $user;
100+
}
101+
}
102+
} else {
103+
// make sure to add phonebook matches if configured
104+
if ($this->shareeEnumerationPhone) {
105+
$usersTmp = $this->userManager->searchKnownUsersByDisplayName($currentUserId, $search, $limit, $offset);
92106
foreach ($usersTmp as $user) {
93107
if ($user->isEnabled()) { // Don't keep deactivated users
94108
$users[$user->getUID()] = $user;
95109
}
96110
}
97-
98-
uasort($users, function ($a, $b) {
99-
/**
100-
* @var \OC\User\User $a
101-
* @var \OC\User\User $b
102-
*/
103-
return strcasecmp($a->getDisplayName(), $b->getDisplayName());
104-
});
105111
}
106-
}
107-
} else {
108-
// Search in all users
109-
if ($this->shareeEnumerationPhone) {
110-
$usersTmp = $this->userManager->searchKnownUsersByDisplayName($currentUserId, $search, $limit, $offset);
111-
} else {
112-
$usersTmp = $this->userManager->searchDisplayName($search, $limit, $offset);
113-
}
114-
foreach ($usersTmp as $user) {
115-
if ($user->isEnabled()) { // Don't keep deactivated users
116-
$users[$user->getUID()] = $user;
112+
113+
// additionally we need to add full matches
114+
if ($this->shareeEnumerationFullMatch) {
115+
$usersTmp = $this->userManager->searchDisplayName($search, $limit, $offset);
116+
foreach ($usersTmp as $user) {
117+
if ($user->isEnabled() && mb_strtolower($user->getDisplayName()) === mb_strtolower($search)) {
118+
$users[$user->getUID()] = $user;
119+
}
120+
}
117121
}
118122
}
123+
124+
uasort($users, function (IUser $a, IUser $b) {
125+
return strcasecmp($a->getDisplayName(), $b->getDisplayName());
126+
});
119127
}
120128

121129
$this->takeOutCurrentUser($users);
@@ -147,11 +155,14 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
147155

148156

149157
if (
150-
$this->shareeEnumerationFullMatch &&
151-
$lowerSearch !== '' && (strtolower($uid) === $lowerSearch ||
152-
strtolower($userDisplayName) === $lowerSearch ||
153-
($this->shareeEnumerationFullMatchIgnoreSecondDisplayName && trim(strtolower(preg_replace('/ \(.*\)$/', '', $userDisplayName))) === $lowerSearch) ||
154-
($this->shareeEnumerationFullMatchEmail && strtolower($userEmail ?? '') === $lowerSearch))
158+
$this->shareeEnumerationFullMatch
159+
&& $lowerSearch !== ''
160+
&& (
161+
strtolower($uid) === $lowerSearch
162+
|| strtolower($userDisplayName) === $lowerSearch
163+
|| ($this->shareeEnumerationFullMatchIgnoreSecondDisplayName && trim(strtolower(preg_replace('/ \(.*\)$/', '', $userDisplayName))) === $lowerSearch)
164+
|| ($this->shareeEnumerationFullMatchEmail && strtolower($userEmail ?? '') === $lowerSearch)
165+
)
155166
) {
156167
if (strtolower($uid) === $lowerSearch) {
157168
$foundUserById = true;

‎lib/private/Share20/Manager.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1932,6 +1932,10 @@ public function matchEmail(): bool {
19321932
return $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_email', 'yes') === 'yes';
19331933
}
19341934

1935+
public function matchUserId(): bool {
1936+
return $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_userid', 'yes') === 'yes';
1937+
}
1938+
19351939
public function ignoreSecondDisplayName(): bool {
19361940
return $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_ignore_second_dn', 'no') === 'yes';
19371941
}

‎lib/public/Share/IManager.php‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -441,23 +441,26 @@ public function limitEnumerationToGroups(): bool;
441441
public function limitEnumerationToPhone(): bool;
442442

443443
/**
444-
* Check if user enumeration is allowed to return on full match
444+
* Check if user enumeration is allowed to return also on full match
445+
* and ignore limitations to phonebook or groups.
445446
*
446447
* @return bool
447448
* @since 21.0.1
448449
*/
449450
public function allowEnumerationFullMatch(): bool;
450451

451452
/**
452-
* Check if the search should match the email
453+
* When `allowEnumerationFullMatch` is enabled and `matchEmail` is set,
454+
* then also return results for full email matches.
453455
*
454456
* @return bool
455457
* @since 25.0.0
456458
*/
457459
public function matchEmail(): bool;
458460

459461
/**
460-
* Check if the search should ignore the second in parentheses display name if there is any
462+
* When `allowEnumerationFullMatch` is enabled and `ignoreSecondDisplayName` is set,
463+
* then the search should ignore matches on the second displayname and only use the first.
461464
*
462465
* @return bool
463466
* @since 25.0.0

‎tests/lib/Collaboration/Collaborators/UserPluginTest.php‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,10 @@ public function testSearch(
456456
->method('getUser')
457457
->willReturn($this->user);
458458

459+
$this->userManager->expects($this->any())
460+
->method('searchDisplayName')
461+
->willReturn($userResponse);
462+
459463
if (!$shareWithGroupOnly) {
460464
if ($shareeEnumerationPhone) {
461465
$this->userManager->expects($this->once())
@@ -766,10 +770,10 @@ public function testSearchEnumerationLimit($search, $userGroups, $matchingUsers,
766770
->willReturnCallback(function ($search) use ($matchingUsers) {
767771
$users = array_filter(
768772
$matchingUsers,
769-
fn ($user) => str_contains(strtolower($user['displayName']), strtolower($search))
773+
fn ($user) => str_contains(strtolower($user['displayName'] ?? $user['uid']), strtolower($search))
770774
);
771775
return array_map(
772-
fn ($user) => $this->getUserMock($user['uid'], $user['displayName']),
776+
fn ($user) => $this->getUserMock($user['uid'], $user['displayName'] ?? $user['uid']),
773777
$users);
774778
});
775779

0 commit comments

Comments
 (0)