From e297e1ce18287bd22a50cb843040aef66b4746fa Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Tue, 10 May 2022 13:05:08 +0200 Subject: [PATCH] Add settings to ignore second display name in search Signed-off-by: Louis Chemineau --- apps/settings/lib/Settings/Admin/Sharing.php | 1 + .../tests/Settings/Admin/SharingTest.php | 4 + .../bootstrap/CollaborationContext.php | 1 + .../Collaborators/UserPlugin.php | 4 + .../Collaborators/UserPluginTest.php | 151 +++++++++++++++--- 5 files changed, 143 insertions(+), 18 deletions(-) diff --git a/apps/settings/lib/Settings/Admin/Sharing.php b/apps/settings/lib/Settings/Admin/Sharing.php index c44fec94b6e..ac001212aa4 100644 --- a/apps/settings/lib/Settings/Admin/Sharing.php +++ b/apps/settings/lib/Settings/Admin/Sharing.php @@ -85,6 +85,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(), 'onlyShareWithGroupMembers' => $this->shareManager->shareWithGroupMembersOnly(), 'shareAPIEnabled' => $this->config->getAppValue('core', 'shareapi_enabled', 'yes'), diff --git a/apps/settings/tests/Settings/Admin/SharingTest.php b/apps/settings/tests/Settings/Admin/SharingTest.php index a353a1a653d..31c6ef9ed94 100644 --- a/apps/settings/tests/Settings/Admin/SharingTest.php +++ b/apps/settings/tests/Settings/Admin/SharingTest.php @@ -83,6 +83,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'], @@ -117,6 +118,7 @@ class SharingTest extends TestCase { 'restrictUserEnumerationToPhone' => 'no', 'restrictUserEnumerationFullMatch' => 'yes', 'restrictUserEnumerationFullMatchUserId' => 'yes', + 'restrictUserEnumerationFullMatchIgnoreSecondDisplayName' => 'no', 'enforceLinkPassword' => false, 'onlyShareWithGroupMembers' => false, 'shareAPIEnabled' => 'yes', @@ -157,6 +159,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'], @@ -191,6 +194,7 @@ class SharingTest extends TestCase { 'restrictUserEnumerationToPhone' => 'no', 'restrictUserEnumerationFullMatch' => 'yes', 'restrictUserEnumerationFullMatchUserId' => 'yes', + 'restrictUserEnumerationFullMatchIgnoreSecondDisplayName' => 'no', 'enforceLinkPassword' => false, 'onlyShareWithGroupMembers' => false, 'shareAPIEnabled' => 'yes', diff --git a/build/integration/features/bootstrap/CollaborationContext.php b/build/integration/features/bootstrap/CollaborationContext.php index a61105f090c..77fea0c3960 100644 --- a/build/integration/features/bootstrap/CollaborationContext.php +++ b/build/integration/features/bootstrap/CollaborationContext.php @@ -70,6 +70,7 @@ class CollaborationContext implements Context { $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'); } diff --git a/lib/private/Collaboration/Collaborators/UserPlugin.php b/lib/private/Collaboration/Collaborators/UserPlugin.php index 510f383249a..819ecfa50d8 100644 --- a/lib/private/Collaboration/Collaborators/UserPlugin.php +++ b/lib/private/Collaboration/Collaborators/UserPlugin.php @@ -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) { diff --git a/tests/lib/Collaboration/Collaborators/UserPluginTest.php b/tests/lib/Collaboration/Collaborators/UserPluginTest.php index acbcd42f04f..2b7a08fe4e1 100644 --- a/tests/lib/Collaboration/Collaborators/UserPluginTest.php +++ b/tests/lib/Collaboration/Collaborators/UserPluginTest.php @@ -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,21 @@ class UserPluginTest extends TestCase { } return null; }); + $this->userManager + ->method('searchDisplayName') + ->willReturnCallback(function ($search) use ($matchingUsers) { + $users = array_filter( + $matchingUsers, + function ($user) use ($search) { + return str_contains(strtolower($user['displayName']), strtolower($search)); + }); + return array_map( + function ($user) { + return $this->getUserMock($user['uid'], $user['displayName']); + }, + $users + ); + }); $this->groupManager->method('displayNamesInGroup') ->willReturn($userResults); -- 2.39.5