Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add settings to ignore second display name in search #31932

Merged
merged 1 commit into from
Apr 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/settings/lib/Settings/Admin/Sharing.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ public function getForm() {
'restrictUserEnumerationToPhone' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no'),
'restrictUserEnumerationFullMatch' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes'),
'restrictUserEnumerationFullMatchUserId' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_userid', 'yes'),
'restrictUserEnumerationFullMatchIgnoreSecondDisplayName' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_ignore_second_display_name', 'no'),
'enforceLinkPassword' => Util::isPublicLinkPasswordRequired(false),
'passwordExcludedGroups' => $excludedPasswordGroupsList,
'passwordExcludedGroupsFeatureEnabled' => $this->config->getSystemValueBool('sharing.allow_disabled_password_enforcement_groups', false),
Expand Down
1 change: 1 addition & 0 deletions apps/settings/src/admin.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ window.addEventListener('DOMContentLoaded', () => {

$('#shareapi_restrict_user_enumeration_full_match').on('change', function() {
$('#shareapi_restrict_user_enumeration_full_match_userid_setting').toggleClass('hidden', !this.checked)
$('#shareapi_restrict_user_enumeration_full_match_ignore_second_display_name_setting').toggleClass('hidden', !this.checked)
})

$('#allowLinks').change(function() {
Expand Down
13 changes: 11 additions & 2 deletions apps/settings/templates/settings/admin/sharing.php
Original file line number Diff line number Diff line change
Expand Up @@ -247,15 +247,24 @@
} ?> />
<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 />
</p>
<p id="shareapi_restrict_user_enumeration_full_match_userid_setting" class="double-indent <?php if ($_['shareAPIEnabled'] === 'no' || $_['restrictUserEnumerationFullMatchUserId'] === 'no') {
<p id="shareapi_restrict_user_enumeration_full_match_userid_setting" class="double-indent <?php if ($_['shareAPIEnabled'] === 'no' || $_['restrictUserEnumerationFullMatch'] === 'no') {
p('hidden');
}?>">
<input type="checkbox" name="shareapi_restrict_user_enumeration_full_match_userid" value="1" id="shareapi_restrict_user_enumeration_full_match_userid" class="checkbox"
<?php if ($_['shareeEnumerationFullMatchUserId'] === 'yes') {
<?php if ($_['restrictUserEnumerationFullMatchUserId'] === 'yes') {
print_unescaped('checked="checked"');
} ?> />
<label for="shareapi_restrict_user_enumeration_full_match_userid"><?php p($l->t('Match username when restricting to full match'));?></label><br />
</p>
<p id="shareapi_restrict_user_enumeration_full_match_ignore_second_display_name_setting" class="double-indent <?php if ($_['shareAPIEnabled'] === 'no' || $_['restrictUserEnumerationFullMatch'] === 'no') {
p('hidden');
}?>">
<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"
<?php if ($_['restrictUserEnumerationFullMatchIgnoreSecondDisplayName'] === 'yes') {
print_unescaped('checked="checked"');
} ?> />
<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 />
</p>

<p>
<input type="checkbox" id="publicShareDisclaimer" class="checkbox noJSAutoUpdate"
Expand Down
4 changes: 4 additions & 0 deletions apps/settings/tests/Settings/Admin/SharingTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ public function testGetFormWithoutExcludedGroups(): void {
['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'],
['core', 'shareapi_restrict_user_enumeration_full_match', 'yes', 'yes'],
['core', 'shareapi_restrict_user_enumeration_full_match_userid', 'yes', 'yes'],
['core', 'shareapi_restrict_user_enumeration_full_match_ignore_second_display_name', 'no', 'no'],
['core', 'shareapi_enabled', 'yes', 'yes'],
['core', 'shareapi_default_expire_date', 'no', 'no'],
['core', 'shareapi_expire_after_n_days', '7', '7'],
Expand Down Expand Up @@ -118,6 +119,7 @@ public function testGetFormWithoutExcludedGroups(): void {
'restrictUserEnumerationToPhone' => 'no',
'restrictUserEnumerationFullMatch' => 'yes',
'restrictUserEnumerationFullMatchUserId' => 'yes',
'restrictUserEnumerationFullMatchIgnoreSecondDisplayName' => 'no',
'enforceLinkPassword' => false,
'onlyShareWithGroupMembers' => false,
'shareAPIEnabled' => 'yes',
Expand Down Expand Up @@ -161,6 +163,7 @@ public function testGetFormWithExcludedGroups(): void {
['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'],
['core', 'shareapi_restrict_user_enumeration_full_match', 'yes', 'yes'],
['core', 'shareapi_restrict_user_enumeration_full_match_userid', 'yes', 'yes'],
['core', 'shareapi_restrict_user_enumeration_full_match_ignore_second_display_name', 'no', 'no'],
['core', 'shareapi_enabled', 'yes', 'yes'],
['core', 'shareapi_default_expire_date', 'no', 'no'],
['core', 'shareapi_expire_after_n_days', '7', '7'],
Expand Down Expand Up @@ -195,6 +198,7 @@ public function testGetFormWithExcludedGroups(): void {
'restrictUserEnumerationToPhone' => 'no',
'restrictUserEnumerationFullMatch' => 'yes',
'restrictUserEnumerationFullMatchUserId' => 'yes',
'restrictUserEnumerationFullMatchIgnoreSecondDisplayName' => 'no',
'enforceLinkPassword' => false,
'onlyShareWithGroupMembers' => false,
'shareAPIEnabled' => 'yes',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ protected function resetAppConfigs(): void {
$this->deleteServerConfig('core', 'shareapi_restrict_user_enumeration_to_phone');
$this->deleteServerConfig('core', 'shareapi_restrict_user_enumeration_full_match');
$this->deleteServerConfig('core', 'shareapi_restrict_user_enumeration_full_match_userid');
$this->deleteServerConfig('core', 'shareapi_restrict_user_enumeration_full_match_ignore_second_display_name');
$this->deleteServerConfig('core', 'shareapi_only_share_with_group_members');
}

Expand Down
4 changes: 2 additions & 2 deletions dist/settings-legacy-admin.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/settings-legacy-admin.js.map

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions lib/private/Collaboration/Collaborators/UserPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ class UserPlugin implements ISearchPlugin {
protected $shareeEnumerationFullMatch;
/* @var bool */
protected $shareeEnumerationFullMatchUserId;
/* @var bool */
protected $shareeEnumerationFullMatchIgnoreSecondDisplayName;

/** @var IConfig */
private $config;
Expand Down Expand Up @@ -90,6 +92,7 @@ public function __construct(IConfig $config,
$this->shareeEnumerationPhone = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes';
$this->shareeEnumerationFullMatch = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes') === 'yes';
$this->shareeEnumerationFullMatchUserId = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_userid', 'yes') === 'yes';
$this->shareeEnumerationFullMatchIgnoreSecondDisplayName = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_ignore_second_display_name', 'no') === 'yes';
}

public function search($search, $limit, $offset, ISearchResult $searchResult) {
Expand Down Expand Up @@ -181,6 +184,7 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) {
$this->shareeEnumerationFullMatch &&
$lowerSearch !== '' && (strtolower($uid) === $lowerSearch ||
strtolower($userDisplayName) === $lowerSearch ||
($this->shareeEnumerationFullMatchIgnoreSecondDisplayName && trim(strtolower(preg_replace('/ \(.*\)$/', '', $userDisplayName))) === $lowerSearch) ||
strtolower($userEmail ?? '') === $lowerSearch)
) {
if (strtolower($uid) === $lowerSearch) {
Expand Down
147 changes: 129 additions & 18 deletions tests/lib/Collaboration/Collaborators/UserPluginTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -104,21 +104,12 @@ public function instantiatePlugin() {
);
}

public function mockConfig($shareWithGroupOnly, $shareeEnumeration, $shareeEnumerationLimitToGroup, $shareeEnumerationPhone = false) {
public function mockConfig($mockedSettings) {
$this->config->expects($this->any())
->method('getAppValue')
->willReturnCallback(
function ($appName, $key, $default) use ($shareWithGroupOnly, $shareeEnumeration, $shareeEnumerationLimitToGroup, $shareeEnumerationPhone) {
if ($appName === 'core' && $key === 'shareapi_only_share_with_group_members') {
return $shareWithGroupOnly ? 'yes' : 'no';
} elseif ($appName === 'core' && $key === 'shareapi_allow_share_dialog_user_enumeration') {
return $shareeEnumeration ? 'yes' : 'no';
} elseif ($appName === 'core' && $key === 'shareapi_restrict_user_enumeration_to_group') {
return $shareeEnumerationLimitToGroup ? 'yes' : 'no';
} elseif ($appName === 'core' && $key === 'shareapi_restrict_user_enumeration_to_phone') {
return $shareeEnumerationPhone ? 'yes' : 'no';
}
return $default;
function ($appName, $key, $default) use ($mockedSettings) {
return $mockedSettings[$appName][$key] ?? $default;
}
);
}
Expand Down Expand Up @@ -470,7 +461,13 @@ public function testSearch(
array $users = [],
$shareeEnumerationPhone = false
) {
$this->mockConfig($shareWithGroupOnly, $shareeEnumeration, false, $shareeEnumerationPhone);
$this->mockConfig(["core" => [
'shareapi_only_share_with_group_members' => $shareWithGroupOnly ? 'yes' : 'no',
'shareapi_allow_share_dialog_user_enumeration' => $shareeEnumeration? 'yes' : 'no',
'shareapi_restrict_user_enumeration_to_group' => false ? 'yes' : 'no',
'shareapi_restrict_user_enumeration_to_phone' => $shareeEnumerationPhone ? 'yes' : 'no',
]]);

$this->instantiatePlugin();

$this->session->expects($this->any())
Expand Down Expand Up @@ -586,6 +583,83 @@ public function dataSearchEnumeration() {
['uid' => 'test2', 'groups' => ['groupB']],
],
['exact' => [], 'wide' => ['test1']],
['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
],
[
'test',
['groupA'],
[
['uid' => 'test1', 'displayName' => 'Test user 1', 'groups' => ['groupA']],
['uid' => 'test2', 'displayName' => 'Test user 2', 'groups' => ['groupA']],
],
['exact' => [], 'wide' => []],
['core' => ['shareapi_allow_share_dialog_user_enumeration' => 'no']],
],
[
'test1',
['groupA'],
[
['uid' => 'test1', 'displayName' => 'Test user 1', 'groups' => ['groupA']],
['uid' => 'test2', 'displayName' => 'Test user 2', 'groups' => ['groupA']],
],
['exact' => ['test1'], 'wide' => []],
['core' => ['shareapi_allow_share_dialog_user_enumeration' => 'no']],
],
[
'test1',
['groupA'],
[
['uid' => 'test1', 'displayName' => 'Test user 1', 'groups' => ['groupA']],
['uid' => 'test2', 'displayName' => 'Test user 2', 'groups' => ['groupA']],
],
['exact' => [], 'wide' => []],
[
'core' => [
'shareapi_allow_share_dialog_user_enumeration' => 'no',
'shareapi_restrict_user_enumeration_full_match_userid' => 'no',
],
]
],
[
'Test user 1',
['groupA'],
[
['uid' => 'test1', 'displayName' => 'Test user 1', 'groups' => ['groupA']],
['uid' => 'test2', 'displayName' => 'Test user 2', 'groups' => ['groupA']],
],
['exact' => ['test1'], 'wide' => []],
[
'core' => [
'shareapi_allow_share_dialog_user_enumeration' => 'no',
'shareapi_restrict_user_enumeration_full_match_userid' => 'no',
],
]
],
[
'Test user 1',
['groupA'],
[
['uid' => 'test1', 'displayName' => 'Test user 1 (Second displayName for user 1)', 'groups' => ['groupA']],
['uid' => 'test2', 'displayName' => 'Test user 2 (Second displayName for user 2)', 'groups' => ['groupA']],
],
['exact' => [], 'wide' => []],
['core' => ['shareapi_allow_share_dialog_user_enumeration' => 'no'],
]
],
[
'Test user 1',
['groupA'],
[
['uid' => 'test1', 'displayName' => 'Test user 1 (Second displayName for user 1)', 'groups' => ['groupA']],
['uid' => 'test2', 'displayName' => 'Test user 2 (Second displayName for user 2)', 'groups' => ['groupA']],
],
['exact' => ['test1'], 'wide' => []],
[
'core' => [
'shareapi_allow_share_dialog_user_enumeration' => 'no',
'shareapi_restrict_user_enumeration_full_match_ignore_second_display_name' => 'yes',
],
]
],
[
'test1',
Expand All @@ -595,6 +669,7 @@ public function dataSearchEnumeration() {
['uid' => 'test2', 'groups' => ['groupB']],
],
['exact' => ['test1'], 'wide' => []],
['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
],
[
'test',
Expand All @@ -604,6 +679,7 @@ public function dataSearchEnumeration() {
['uid' => 'test2', 'groups' => ['groupB', 'groupA']],
],
['exact' => [], 'wide' => ['test1', 'test2']],
['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
],
[
'test',
Expand All @@ -613,6 +689,7 @@ public function dataSearchEnumeration() {
['uid' => 'test2', 'groups' => ['groupB', 'groupA']],
],
['exact' => [], 'wide' => ['test1', 'test2']],
['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
],
[
'test',
Expand All @@ -622,6 +699,7 @@ public function dataSearchEnumeration() {
['uid' => 'test2', 'groups' => ['groupB', 'groupA']],
],
['exact' => [], 'wide' => ['test1', 'test2']],
['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
],
[
'test',
Expand All @@ -631,6 +709,7 @@ public function dataSearchEnumeration() {
['uid' => 'test2', 'groups' => ['groupB', 'groupA']],
],
['exact' => [], 'wide' => []],
['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
],
[
'test',
Expand All @@ -640,6 +719,7 @@ public function dataSearchEnumeration() {
['uid' => 'test2', 'groups' => []],
],
['exact' => [], 'wide' => []],
['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
],
[
'test',
Expand All @@ -649,26 +729,46 @@ public function dataSearchEnumeration() {
['uid' => 'test2', 'groups' => []],
],
['exact' => [], 'wide' => []],
['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
],
];
}

/**
* @dataProvider dataSearchEnumeration
*/
public function testSearchEnumerationLimit($search, $userGroups, $matchingUsers, $result) {
$this->mockConfig(false, true, true);
public function testSearchEnumerationLimit($search, $userGroups, $matchingUsers, $result, $mockedSettings) {
$this->mockConfig($mockedSettings);

$userResults = [];
foreach ($matchingUsers as $user) {
$userResults[$user['uid']] = $user['uid'];
}

$mappedResultExact = array_map(function ($user) {
return ['label' => $user, 'value' => ['shareType' => 0, 'shareWith' => $user], 'icon' => 'icon-user', 'subline' => null, 'status' => [], 'shareWithDisplayNameUnique' => $user];
$usersById = [];
foreach ($matchingUsers as $user) {
$usersById[$user['uid']] = $user;
}

$mappedResultExact = array_map(function ($user) use ($usersById, $search) {
return [
'label' => $search === $user ? $user : $usersById[$user]['displayName'],
'value' => ['shareType' => 0, 'shareWith' => $user],
'icon' => 'icon-user',
'subline' => null,
'status' => [],
'shareWithDisplayNameUnique' => $user,
];
}, $result['exact']);
$mappedResultWide = array_map(function ($user) {
return ['label' => $user, 'value' => ['shareType' => 0, 'shareWith' => $user], 'icon' => 'icon-user', 'subline' => null, 'status' => [], 'shareWithDisplayNameUnique' => $user];
return [
'label' => $user,
'value' => ['shareType' => 0, 'shareWith' => $user],
'icon' => 'icon-user',
'subline' => null,
'status' => [],
'shareWithDisplayNameUnique' => $user,
];
}, $result['wide']);

$this->userManager
Expand All @@ -679,6 +779,17 @@ public function testSearchEnumerationLimit($search, $userGroups, $matchingUsers,
}
return null;
});
$this->userManager
->method('searchDisplayName')
->willReturnCallback(function ($search) use ($matchingUsers) {
$users = array_filter(
$matchingUsers,
fn ($user) => str_contains(strtolower($user['displayName']), strtolower($search))
);
return array_map(
fn ($user) => $this->getUserMock($user['uid'], $user['displayName']),
$users);
});

$this->groupManager->method('displayNamesInGroup')
->willReturn($userResults);
Expand Down