Skip to content

Commit 37f1c86

Browse files
artongeartonge
committed
Add settings to ignore second display name in search
Signed-off-by: Louis Chemineau <louis@chmn.me>
1 parent ff0ccc8 commit 37f1c86

File tree

9 files changed

+154
-23
lines changed

9 files changed

+154
-23
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ public function getForm() {
9090
'restrictUserEnumerationToPhone' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no'),
9191
'restrictUserEnumerationFullMatch' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes'),
9292
'restrictUserEnumerationFullMatchUserId' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_userid', 'yes'),
93+
'restrictUserEnumerationFullMatchIgnoreSecondDisplayName' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_ignore_second_display_name', 'no'),
9394
'enforceLinkPassword' => Util::isPublicLinkPasswordRequired(false),
9495
'passwordExcludedGroups' => $excludedPasswordGroupsList,
9596
'passwordExcludedGroupsFeatureEnabled' => $this->config->getSystemValueBool('sharing.allow_disabled_password_enforcement_groups', false),

‎apps/settings/src/admin.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ window.addEventListener('DOMContentLoaded', () => {
157157

158158
$('#shareapi_restrict_user_enumeration_full_match').on('change', function() {
159159
$('#shareapi_restrict_user_enumeration_full_match_userid_setting').toggleClass('hidden', !this.checked)
160+
$('#shareapi_restrict_user_enumeration_full_match_ignore_second_display_name_setting').toggleClass('hidden', !this.checked)
160161
})
161162

162163
$('#allowLinks').change(function() {

‎apps/settings/templates/settings/admin/sharing.php‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -247,15 +247,24 @@
247247
} ?> />
248248
<label for="shareapi_restrict_user_enumeration_full_match"><?php p($l->t('Allow autocompletion when entering the full name or email address (ignoring missing phonebook match and being in the same group)'));?></label><br />
249249
</p>
250-
<p id="shareapi_restrict_user_enumeration_full_match_userid_setting" class="double-indent <?php if ($_['shareAPIEnabled'] === 'no' || $_['restrictUserEnumerationFullMatchUserId'] === 'no') {
250+
<p id="shareapi_restrict_user_enumeration_full_match_userid_setting" class="double-indent <?php if ($_['shareAPIEnabled'] === 'no' || $_['restrictUserEnumerationFullMatch'] === 'no') {
251251
p('hidden');
252252
}?>">
253253
<input type="checkbox" name="shareapi_restrict_user_enumeration_full_match_userid" value="1" id="shareapi_restrict_user_enumeration_full_match_userid" class="checkbox"
254-
<?php if ($_['shareeEnumerationFullMatchUserId'] === 'yes') {
254+
<?php if ($_['restrictUserEnumerationFullMatchUserId'] === 'yes') {
255255
print_unescaped('checked="checked"');
256256
} ?> />
257257
<label for="shareapi_restrict_user_enumeration_full_match_userid"><?php p($l->t('Match username when restricting to full match'));?></label><br />
258258
</p>
259+
<p id="shareapi_restrict_user_enumeration_full_match_ignore_second_display_name_setting" class="double-indent <?php if ($_['shareAPIEnabled'] === 'no' || $_['restrictUserEnumerationFullMatch'] === 'no') {
260+
p('hidden');
261+
}?>">
262+
<input type="checkbox" name="shareapi_restrict_user_enumeration_full_match_ignore_second_display_name" value="1" id="shareapi_restrict_user_enumeration_full_match_ignore_second_display_name" class="checkbox"
263+
<?php if ($_['restrictUserEnumerationFullMatchIgnoreSecondDisplayName'] === 'yes') {
264+
print_unescaped('checked="checked"');
265+
} ?> />
266+
<label for="shareapi_restrict_user_enumeration_full_match_ignore_second_display_name"><?php p($l->t('Ignore second display name in parentheses if any. Example: "First display name (second ignored display name)"'));?></label><br />
267+
</p>
259268

260269
<p>
261270
<input type="checkbox" id="publicShareDisclaimer" class="checkbox noJSAutoUpdate"

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ public function testGetFormWithoutExcludedGroups(): void {
8484
['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'],
8585
['core', 'shareapi_restrict_user_enumeration_full_match', 'yes', 'yes'],
8686
['core', 'shareapi_restrict_user_enumeration_full_match_userid', 'yes', 'yes'],
87+
['core', 'shareapi_restrict_user_enumeration_full_match_ignore_second_display_name', 'no', 'no'],
8788
['core', 'shareapi_enabled', 'yes', 'yes'],
8889
['core', 'shareapi_default_expire_date', 'no', 'no'],
8990
['core', 'shareapi_expire_after_n_days', '7', '7'],
@@ -118,6 +119,7 @@ public function testGetFormWithoutExcludedGroups(): void {
118119
'restrictUserEnumerationToPhone' => 'no',
119120
'restrictUserEnumerationFullMatch' => 'yes',
120121
'restrictUserEnumerationFullMatchUserId' => 'yes',
122+
'restrictUserEnumerationFullMatchIgnoreSecondDisplayName' => 'no',
121123
'enforceLinkPassword' => false,
122124
'onlyShareWithGroupMembers' => false,
123125
'shareAPIEnabled' => 'yes',
@@ -161,6 +163,7 @@ public function testGetFormWithExcludedGroups(): void {
161163
['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'],
162164
['core', 'shareapi_restrict_user_enumeration_full_match', 'yes', 'yes'],
163165
['core', 'shareapi_restrict_user_enumeration_full_match_userid', 'yes', 'yes'],
166+
['core', 'shareapi_restrict_user_enumeration_full_match_ignore_second_display_name', 'no', 'no'],
164167
['core', 'shareapi_enabled', 'yes', 'yes'],
165168
['core', 'shareapi_default_expire_date', 'no', 'no'],
166169
['core', 'shareapi_expire_after_n_days', '7', '7'],
@@ -195,6 +198,7 @@ public function testGetFormWithExcludedGroups(): void {
195198
'restrictUserEnumerationToPhone' => 'no',
196199
'restrictUserEnumerationFullMatch' => 'yes',
197200
'restrictUserEnumerationFullMatchUserId' => 'yes',
201+
'restrictUserEnumerationFullMatchIgnoreSecondDisplayName' => 'no',
198202
'enforceLinkPassword' => false,
199203
'onlyShareWithGroupMembers' => false,
200204
'shareAPIEnabled' => 'yes',

‎build/integration/features/bootstrap/CollaborationContext.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ protected function resetAppConfigs(): void {
123123
$this->deleteServerConfig('core', 'shareapi_restrict_user_enumeration_to_phone');
124124
$this->deleteServerConfig('core', 'shareapi_restrict_user_enumeration_full_match');
125125
$this->deleteServerConfig('core', 'shareapi_restrict_user_enumeration_full_match_userid');
126+
$this->deleteServerConfig('core', 'shareapi_restrict_user_enumeration_full_match_ignore_second_display_name');
126127
$this->deleteServerConfig('core', 'shareapi_only_share_with_group_members');
127128
}
128129

‎dist/settings-legacy-admin.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-legacy-admin.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: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ class UserPlugin implements ISearchPlugin {
5656
protected $shareeEnumerationFullMatch;
5757
/* @var bool */
5858
protected $shareeEnumerationFullMatchUserId;
59+
/* @var bool */
60+
protected $shareeEnumerationFullMatchIgnoreSecondDisplayName;
5961

6062
/** @var IConfig */
6163
private $config;
@@ -90,6 +92,7 @@ public function __construct(IConfig $config,
9092
$this->shareeEnumerationPhone = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes';
9193
$this->shareeEnumerationFullMatch = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes') === 'yes';
9294
$this->shareeEnumerationFullMatchUserId = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_userid', 'yes') === 'yes';
95+
$this->shareeEnumerationFullMatchIgnoreSecondDisplayName = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_ignore_second_display_name', 'no') === 'yes';
9396
}
9497

9598
public function search($search, $limit, $offset, ISearchResult $searchResult) {
@@ -181,6 +184,7 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) {
181184
$this->shareeEnumerationFullMatch &&
182185
$lowerSearch !== '' && (strtolower($uid) === $lowerSearch ||
183186
strtolower($userDisplayName) === $lowerSearch ||
187+
($this->shareeEnumerationFullMatchIgnoreSecondDisplayName && trim(strtolower(preg_replace('/ \(.*\)$/', '', $userDisplayName))) === $lowerSearch) ||
184188
strtolower($userEmail ?? '') === $lowerSearch)
185189
) {
186190
if (strtolower($uid) === $lowerSearch) {

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

Lines changed: 129 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -104,21 +104,12 @@ public function instantiatePlugin() {
104104
);
105105
}
106106

107-
public function mockConfig($shareWithGroupOnly, $shareeEnumeration, $shareeEnumerationLimitToGroup, $shareeEnumerationPhone = false) {
107+
public function mockConfig($mockedSettings) {
108108
$this->config->expects($this->any())
109109
->method('getAppValue')
110110
->willReturnCallback(
111-
function ($appName, $key, $default) use ($shareWithGroupOnly, $shareeEnumeration, $shareeEnumerationLimitToGroup, $shareeEnumerationPhone) {
112-
if ($appName === 'core' && $key === 'shareapi_only_share_with_group_members') {
113-
return $shareWithGroupOnly ? 'yes' : 'no';
114-
} elseif ($appName === 'core' && $key === 'shareapi_allow_share_dialog_user_enumeration') {
115-
return $shareeEnumeration ? 'yes' : 'no';
116-
} elseif ($appName === 'core' && $key === 'shareapi_restrict_user_enumeration_to_group') {
117-
return $shareeEnumerationLimitToGroup ? 'yes' : 'no';
118-
} elseif ($appName === 'core' && $key === 'shareapi_restrict_user_enumeration_to_phone') {
119-
return $shareeEnumerationPhone ? 'yes' : 'no';
120-
}
121-
return $default;
111+
function ($appName, $key, $default) use ($mockedSettings) {
112+
return $mockedSettings[$appName][$key] ?? $default;
122113
}
123114
);
124115
}
@@ -470,7 +461,13 @@ public function testSearch(
470461
array $users = [],
471462
$shareeEnumerationPhone = false
472463
) {
473-
$this->mockConfig($shareWithGroupOnly, $shareeEnumeration, false, $shareeEnumerationPhone);
464+
$this->mockConfig(["core" => [
465+
'shareapi_only_share_with_group_members' => $shareWithGroupOnly ? 'yes' : 'no',
466+
'shareapi_allow_share_dialog_user_enumeration' => $shareeEnumeration? 'yes' : 'no',
467+
'shareapi_restrict_user_enumeration_to_group' => false ? 'yes' : 'no',
468+
'shareapi_restrict_user_enumeration_to_phone' => $shareeEnumerationPhone ? 'yes' : 'no',
469+
]]);
470+
474471
$this->instantiatePlugin();
475472

476473
$this->session->expects($this->any())
@@ -586,6 +583,83 @@ public function dataSearchEnumeration() {
586583
['uid' => 'test2', 'groups' => ['groupB']],
587584
],
588585
['exact' => [], 'wide' => ['test1']],
586+
['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
587+
],
588+
[
589+
'test',
590+
['groupA'],
591+
[
592+
['uid' => 'test1', 'displayName' => 'Test user 1', 'groups' => ['groupA']],
593+
['uid' => 'test2', 'displayName' => 'Test user 2', 'groups' => ['groupA']],
594+
],
595+
['exact' => [], 'wide' => []],
596+
['core' => ['shareapi_allow_share_dialog_user_enumeration' => 'no']],
597+
],
598+
[
599+
'test1',
600+
['groupA'],
601+
[
602+
['uid' => 'test1', 'displayName' => 'Test user 1', 'groups' => ['groupA']],
603+
['uid' => 'test2', 'displayName' => 'Test user 2', 'groups' => ['groupA']],
604+
],
605+
['exact' => ['test1'], 'wide' => []],
606+
['core' => ['shareapi_allow_share_dialog_user_enumeration' => 'no']],
607+
],
608+
[
609+
'test1',
610+
['groupA'],
611+
[
612+
['uid' => 'test1', 'displayName' => 'Test user 1', 'groups' => ['groupA']],
613+
['uid' => 'test2', 'displayName' => 'Test user 2', 'groups' => ['groupA']],
614+
],
615+
['exact' => [], 'wide' => []],
616+
[
617+
'core' => [
618+
'shareapi_allow_share_dialog_user_enumeration' => 'no',
619+
'shareapi_restrict_user_enumeration_full_match_userid' => 'no',
620+
],
621+
]
622+
],
623+
[
624+
'Test user 1',
625+
['groupA'],
626+
[
627+
['uid' => 'test1', 'displayName' => 'Test user 1', 'groups' => ['groupA']],
628+
['uid' => 'test2', 'displayName' => 'Test user 2', 'groups' => ['groupA']],
629+
],
630+
['exact' => ['test1'], 'wide' => []],
631+
[
632+
'core' => [
633+
'shareapi_allow_share_dialog_user_enumeration' => 'no',
634+
'shareapi_restrict_user_enumeration_full_match_userid' => 'no',
635+
],
636+
]
637+
],
638+
[
639+
'Test user 1',
640+
['groupA'],
641+
[
642+
['uid' => 'test1', 'displayName' => 'Test user 1 (Second displayName for user 1)', 'groups' => ['groupA']],
643+
['uid' => 'test2', 'displayName' => 'Test user 2 (Second displayName for user 2)', 'groups' => ['groupA']],
644+
],
645+
['exact' => [], 'wide' => []],
646+
['core' => ['shareapi_allow_share_dialog_user_enumeration' => 'no'],
647+
]
648+
],
649+
[
650+
'Test user 1',
651+
['groupA'],
652+
[
653+
['uid' => 'test1', 'displayName' => 'Test user 1 (Second displayName for user 1)', 'groups' => ['groupA']],
654+
['uid' => 'test2', 'displayName' => 'Test user 2 (Second displayName for user 2)', 'groups' => ['groupA']],
655+
],
656+
['exact' => ['test1'], 'wide' => []],
657+
[
658+
'core' => [
659+
'shareapi_allow_share_dialog_user_enumeration' => 'no',
660+
'shareapi_restrict_user_enumeration_full_match_ignore_second_display_name' => 'yes',
661+
],
662+
]
589663
],
590664
[
591665
'test1',
@@ -595,6 +669,7 @@ public function dataSearchEnumeration() {
595669
['uid' => 'test2', 'groups' => ['groupB']],
596670
],
597671
['exact' => ['test1'], 'wide' => []],
672+
['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
598673
],
599674
[
600675
'test',
@@ -604,6 +679,7 @@ public function dataSearchEnumeration() {
604679
['uid' => 'test2', 'groups' => ['groupB', 'groupA']],
605680
],
606681
['exact' => [], 'wide' => ['test1', 'test2']],
682+
['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
607683
],
608684
[
609685
'test',
@@ -613,6 +689,7 @@ public function dataSearchEnumeration() {
613689
['uid' => 'test2', 'groups' => ['groupB', 'groupA']],
614690
],
615691
['exact' => [], 'wide' => ['test1', 'test2']],
692+
['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
616693
],
617694
[
618695
'test',
@@ -622,6 +699,7 @@ public function dataSearchEnumeration() {
622699
['uid' => 'test2', 'groups' => ['groupB', 'groupA']],
623700
],
624701
['exact' => [], 'wide' => ['test1', 'test2']],
702+
['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
625703
],
626704
[
627705
'test',
@@ -631,6 +709,7 @@ public function dataSearchEnumeration() {
631709
['uid' => 'test2', 'groups' => ['groupB', 'groupA']],
632710
],
633711
['exact' => [], 'wide' => []],
712+
['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
634713
],
635714
[
636715
'test',
@@ -640,6 +719,7 @@ public function dataSearchEnumeration() {
640719
['uid' => 'test2', 'groups' => []],
641720
],
642721
['exact' => [], 'wide' => []],
722+
['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
643723
],
644724
[
645725
'test',
@@ -649,26 +729,46 @@ public function dataSearchEnumeration() {
649729
['uid' => 'test2', 'groups' => []],
650730
],
651731
['exact' => [], 'wide' => []],
732+
['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
652733
],
653734
];
654735
}
655736

656737
/**
657738
* @dataProvider dataSearchEnumeration
658739
*/
659-
public function testSearchEnumerationLimit($search, $userGroups, $matchingUsers, $result) {
660-
$this->mockConfig(false, true, true);
740+
public function testSearchEnumerationLimit($search, $userGroups, $matchingUsers, $result, $mockedSettings) {
741+
$this->mockConfig($mockedSettings);
661742

662743
$userResults = [];
663744
foreach ($matchingUsers as $user) {
664745
$userResults[$user['uid']] = $user['uid'];
665746
}
666747

667-
$mappedResultExact = array_map(function ($user) {
668-
return ['label' => $user, 'value' => ['shareType' => 0, 'shareWith' => $user], 'icon' => 'icon-user', 'subline' => null, 'status' => [], 'shareWithDisplayNameUnique' => $user];
748+
$usersById = [];
749+
foreach ($matchingUsers as $user) {
750+
$usersById[$user['uid']] = $user;
751+
}
752+
753+
$mappedResultExact = array_map(function ($user) use ($usersById, $search) {
754+
return [
755+
'label' => $search === $user ? $user : $usersById[$user]['displayName'],
756+
'value' => ['shareType' => 0, 'shareWith' => $user],
757+
'icon' => 'icon-user',
758+
'subline' => null,
759+
'status' => [],
760+
'shareWithDisplayNameUnique' => $user,
761+
];
669762
}, $result['exact']);
670763
$mappedResultWide = array_map(function ($user) {
671-
return ['label' => $user, 'value' => ['shareType' => 0, 'shareWith' => $user], 'icon' => 'icon-user', 'subline' => null, 'status' => [], 'shareWithDisplayNameUnique' => $user];
764+
return [
765+
'label' => $user,
766+
'value' => ['shareType' => 0, 'shareWith' => $user],
767+
'icon' => 'icon-user',
768+
'subline' => null,
769+
'status' => [],
770+
'shareWithDisplayNameUnique' => $user,
771+
];
672772
}, $result['wide']);
673773

674774
$this->userManager
@@ -679,6 +779,17 @@ public function testSearchEnumerationLimit($search, $userGroups, $matchingUsers,
679779
}
680780
return null;
681781
});
782+
$this->userManager
783+
->method('searchDisplayName')
784+
->willReturnCallback(function ($search) use ($matchingUsers) {
785+
$users = array_filter(
786+
$matchingUsers,
787+
fn ($user) => str_contains(strtolower($user['displayName']), strtolower($search))
788+
);
789+
return array_map(
790+
fn ($user) => $this->getUserMock($user['uid'], $user['displayName']),
791+
$users);
792+
});
682793

683794
$this->groupManager->method('displayNamesInGroup')
684795
->willReturn($userResults);

0 commit comments

Comments
 (0)