Merge pull request #31932 from nextcloud/feat/ignore_second_dn

Add settings to ignore second display name in search
nextcloud · Apr 13, 2022 · cd95fce · cd95fce
2 parents 02e5001 + 37f1c86
commit cd95fce
Show file tree

Hide file tree

Showing 9 changed files with 154 additions and 23 deletions.
diff --git a/apps/settings/lib/Settings/Admin/Sharing.php b/apps/settings/lib/Settings/Admin/Sharing.php
@@ -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),

diff --git a/apps/settings/src/admin.js b/apps/settings/src/admin.js
@@ -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() {

diff --git a/apps/settings/templates/settings/admin/sharing.php b/apps/settings/templates/settings/admin/sharing.php
@@ -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"

diff --git a/apps/settings/tests/Settings/Admin/SharingTest.php b/apps/settings/tests/Settings/Admin/SharingTest.php
@@ -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'],
@@ -118,6 +119,7 @@ public function testGetFormWithoutExcludedGroups(): void {
 				'restrictUserEnumerationToPhone' => 'no',
 				'restrictUserEnumerationFullMatch' => 'yes',
 				'restrictUserEnumerationFullMatchUserId' => 'yes',
+				'restrictUserEnumerationFullMatchIgnoreSecondDisplayName' => 'no',
 				'enforceLinkPassword' => false,
 				'onlyShareWithGroupMembers' => false,
 				'shareAPIEnabled' => 'yes',
@@ -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'],
@@ -195,6 +198,7 @@ public function testGetFormWithExcludedGroups(): void {
 				'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
@@ -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');
 	}
 

diff --git a/dist/settings-legacy-admin.js b/dist/settings-legacy-admin.js
diff --git a/dist/settings-legacy-admin.js.map b/dist/settings-legacy-admin.js.map
diff --git 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 @@ 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) {
@@ -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) {

diff --git a/tests/lib/Collaboration/Collaborators/UserPluginTest.php b/tests/lib/Collaboration/Collaborators/UserPluginTest.php
@@ -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;
 				}
 			);
 	}
@@ -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())
@@ -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',
@@ -595,6 +669,7 @@ public function dataSearchEnumeration() {
 					['uid' => 'test2', 'groups' => ['groupB']],
 				],
 				['exact' => ['test1'], 'wide' => []],
+				['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
 			],
 			[
 				'test',
@@ -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',
@@ -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',
@@ -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',
@@ -631,6 +709,7 @@ public function dataSearchEnumeration() {
 					['uid' => 'test2', 'groups' => ['groupB', 'groupA']],
 				],
 				['exact' => [], 'wide' => []],
+				['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
 			],
 			[
 				'test',
@@ -640,6 +719,7 @@ public function dataSearchEnumeration() {
 					['uid' => 'test2', 'groups' => []],
 				],
 				['exact' => [], 'wide' => []],
+				['core' => ['shareapi_restrict_user_enumeration_to_group' => 'yes']],
 			],
 			[
 				'test',
@@ -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
@@ -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);