Merge pull request #31932 from nextcloud/feat/ignore_second_dn

Add settings to ignore second display name in search
pull/31959/head
Vincent Petry 3 years ago committed by GitHub
commit cd95fce105
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 1
      apps/settings/lib/Settings/Admin/Sharing.php
  2. 1
      apps/settings/src/admin.js
  3. 13
      apps/settings/templates/settings/admin/sharing.php
  4. 4
      apps/settings/tests/Settings/Admin/SharingTest.php
  5. 1
      build/integration/features/bootstrap/CollaborationContext.php
  6. 4
      dist/settings-legacy-admin.js
  7. 2
      dist/settings-legacy-admin.js.map
  8. 4
      lib/private/Collaboration/Collaborators/UserPlugin.php
  9. 147
      tests/lib/Collaboration/Collaborators/UserPluginTest.php

@ -90,6 +90,7 @@ class Sharing implements IDelegatedSettings {
'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),

@ -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() {

@ -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"

@ -84,6 +84,7 @@ class SharingTest extends TestCase {
['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'],
@ -118,6 +119,7 @@ class SharingTest extends TestCase {
'restrictUserEnumerationToPhone' => 'no',
'restrictUserEnumerationFullMatch' => 'yes',
'restrictUserEnumerationFullMatchUserId' => 'yes',
'restrictUserEnumerationFullMatchIgnoreSecondDisplayName' => 'no',
'enforceLinkPassword' => false,
'onlyShareWithGroupMembers' => false,
'shareAPIEnabled' => 'yes',
@ -161,6 +163,7 @@ class SharingTest extends TestCase {
['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'],
@ -195,6 +198,7 @@ class SharingTest extends TestCase {
'restrictUserEnumerationToPhone' => 'no',
'restrictUserEnumerationFullMatch' => 'yes',
'restrictUserEnumerationFullMatchUserId' => 'yes',
'restrictUserEnumerationFullMatchIgnoreSecondDisplayName' => 'no',
'enforceLinkPassword' => false,
'onlyShareWithGroupMembers' => false,
'shareAPIEnabled' => 'yes',

@ -123,6 +123,7 @@ EOF;
$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');
}

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

@ -56,6 +56,8 @@ class UserPlugin implements ISearchPlugin {
protected $shareeEnumerationFullMatch;
/* @var bool */
protected $shareeEnumerationFullMatchUserId;
/* @var bool */
protected $shareeEnumerationFullMatchIgnoreSecondDisplayName;
/** @var IConfig */
private $config;
@ -90,6 +92,7 @@ class UserPlugin implements ISearchPlugin {
$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) {
@ -181,6 +184,7 @@ class UserPlugin implements ISearchPlugin {
$this->shareeEnumerationFullMatch &&
$lowerSearch !== '' && (strtolower($uid) === $lowerSearch ||
strtolower($userDisplayName) === $lowerSearch ||
($this->shareeEnumerationFullMatchIgnoreSecondDisplayName && trim(strtolower(preg_replace('/ \(.*\)$/', '', $userDisplayName))) === $lowerSearch) ||
strtolower($userEmail ?? '') === $lowerSearch)
) {
if (strtolower($uid) === $lowerSearch) {

@ -104,21 +104,12 @@ class UserPluginTest extends TestCase {
);
}
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;
}
);
}
@ -470,7 +461,13 @@ class UserPluginTest extends TestCase {
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())
@ -586,6 +583,83 @@ class UserPluginTest extends TestCase {
['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',
@ -595,6 +669,7 @@ class UserPluginTest extends TestCase {
['uid' => 'test2', 'groups' => ['groupB']],
],
['exact' => ['test1'], 'wide' => []],
['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
],
[
'test',
@ -604,6 +679,7 @@ class UserPluginTest extends TestCase {
['uid' => 'test2', 'groups' => ['groupB', 'groupA']],
],
['exact' => [], 'wide' => ['test1', 'test2']],
['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
],
[
'test',
@ -613,6 +689,7 @@ class UserPluginTest extends TestCase {
['uid' => 'test2', 'groups' => ['groupB', 'groupA']],
],
['exact' => [], 'wide' => ['test1', 'test2']],
['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
],
[
'test',
@ -622,6 +699,7 @@ class UserPluginTest extends TestCase {
['uid' => 'test2', 'groups' => ['groupB', 'groupA']],
],
['exact' => [], 'wide' => ['test1', 'test2']],
['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
],
[
'test',
@ -631,6 +709,7 @@ class UserPluginTest extends TestCase {
['uid' => 'test2', 'groups' => ['groupB', 'groupA']],
],
['exact' => [], 'wide' => []],
['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
],
[
'test',
@ -640,6 +719,7 @@ class UserPluginTest extends TestCase {
['uid' => 'test2', 'groups' => []],
],
['exact' => [], 'wide' => []],
['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
],
[
'test',
@ -649,6 +729,7 @@ class UserPluginTest extends TestCase {
['uid' => 'test2', 'groups' => []],
],
['exact' => [], 'wide' => []],
['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
],
];
}
@ -656,19 +737,38 @@ class UserPluginTest extends TestCase {
/**
* @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
@ -679,6 +779,17 @@ class UserPluginTest extends TestCase {
}
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);

Loading…
Cancel
Save