Merge pull request #32322 from nextcloud/backport/stable22/share_sear…

…ch_tweaks [stable23] Add share search tweaks
nextcloud · Jun 23, 2022 · 6644b17 · 6644b17
2 parents a45bb06 + cb66d11
commit 6644b17
Show file tree

Hide file tree

Showing 10 changed files with 204 additions and 23 deletions.
diff --git a/apps/dav/lib/Connector/Sabre/Principal.php b/apps/dav/lib/Connector/Sabre/Principal.php
@@ -270,6 +270,8 @@ protected function searchUserPrincipals(array $searchProperties, $test = 'allof'
 		$limitEnumerationGroup = $this->shareManager->limitEnumerationToGroups();
 		$limitEnumerationPhone = $this->shareManager->limitEnumerationToPhone();
 		$allowEnumerationFullMatch = $this->shareManager->allowEnumerationFullMatch();
+		$ignoreSecondDisplayName = $this->shareManager->ignoreSecondDisplayName();
+		$matchEmail = $this->shareManager->matchEmail();
 
 		// If sharing is restricted to group members only,
 		// return only members that have groups in common
@@ -298,7 +300,7 @@ protected function searchUserPrincipals(array $searchProperties, $test = 'allof'
 			switch ($prop) {
 				case '{http://sabredav.org/ns}email-address':
 					if (!$allowEnumeration) {
-						if ($allowEnumerationFullMatch) {
+						if ($allowEnumerationFullMatch && $matchEmail) {
 							$users = $this->userManager->getByEmail($value);
 						} else {
 							$users = [];
@@ -349,8 +351,9 @@ protected function searchUserPrincipals(array $searchProperties, $test = 'allof'
 						if ($allowEnumerationFullMatch) {
 							$lowerSearch = strtolower($value);
 							$users = $this->userManager->searchDisplayName($value, $searchLimit);
-							$users = \array_filter($users, static function (IUser $user) use ($lowerSearch) {
-								return strtolower($user->getDisplayName()) === $lowerSearch;
+							$users = \array_filter($users, static function (IUser $user) use ($lowerSearch, $ignoreSecondDisplayName) {
+								$lowerDisplayName = strtolower($user->getDisplayName());
+								return $lowerDisplayName === $lowerSearch || ($ignoreSecondDisplayName && trim(preg_replace('/ \(.*\)$/', '', $lowerDisplayName)) === $lowerSearch);
 							});
 						} else {
 							$users = [];

diff --git a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php
@@ -662,6 +662,10 @@ public function testSearchPrincipalWithEnumerationDisabledEmail(): void {
 			->method('allowEnumerationFullMatch')
 			->willReturn(true);
 
+		$this->shareManager->expects($this->once())
+			->method('matchEmail')
+			->willReturn(true);
+
 		$user2 = $this->createMock(IUser::class);
 		$user2->method('getUID')->willReturn('user2');
 		$user2->method('getDisplayName')->willReturn('User 2');

diff --git a/apps/settings/lib/Settings/Admin/Sharing.php b/apps/settings/lib/Settings/Admin/Sharing.php
@@ -84,6 +84,9 @@ public function getForm() {
 			'restrictUserEnumerationToGroup' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no'),
 			'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'),
+			'restrictUserEnumerationFullMatchEmail' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_email', '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
@@ -82,6 +82,9 @@ public function testGetFormWithoutExcludedGroups(): void {
 				['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'],
 				['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_email', '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'],
@@ -115,6 +118,9 @@ public function testGetFormWithoutExcludedGroups(): void {
 				'restrictUserEnumerationToGroup' => 'no',
 				'restrictUserEnumerationToPhone' => 'no',
 				'restrictUserEnumerationFullMatch' => 'yes',
+				'restrictUserEnumerationFullMatchUserId' => 'yes',
+				'restrictUserEnumerationFullMatchEmail' => 'yes',
+				'restrictUserEnumerationFullMatchIgnoreSecondDisplayName' => 'no',
 				'enforceLinkPassword' => false,
 				'onlyShareWithGroupMembers' => false,
 				'shareAPIEnabled' => 'yes',
@@ -154,6 +160,9 @@ public function testGetFormWithExcludedGroups(): void {
 				['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'],
 				['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_email', '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'],
@@ -187,6 +196,9 @@ public function testGetFormWithExcludedGroups(): void {
 				'restrictUserEnumerationToGroup' => 'no',
 				'restrictUserEnumerationToPhone' => 'no',
 				'restrictUserEnumerationFullMatch' => 'yes',
+				'restrictUserEnumerationFullMatchUserId' => 'yes',
+				'restrictUserEnumerationFullMatchEmail' => '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
@@ -69,6 +69,9 @@ protected function resetAppConfigs(): void {
 		$this->deleteServerConfig('core', 'shareapi_restrict_user_enumeration_to_group');
 		$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_email');
+		$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/MailPlugin.php b/lib/private/Collaboration/Collaborators/MailPlugin.php
@@ -51,6 +51,8 @@ class MailPlugin implements ISearchPlugin {
 	protected $shareeEnumerationPhone;
 	/* @var bool */
 	protected $shareeEnumerationFullMatch;
+	/* @var bool */
+	protected $shareeEnumerationFullMatchEmail;
 
 	/** @var IManager */
 	private $contactsManager;
@@ -88,12 +90,17 @@ public function __construct(IManager $contactsManager,
 		$this->shareeEnumerationInGroupOnly = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes';
 		$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->shareeEnumerationFullMatchEmail = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_email', 'yes') === 'yes';
 	}
 
 	/**
 	 * {@inheritdoc}
 	 */
 	public function search($search, $limit, $offset, ISearchResult $searchResult) {
+		if ($this->shareeEnumerationFullMatch && !$this->shareeEnumerationFullMatchEmail) {
+			return false;
+		}
+
 		$currentUserId = $this->userSession->getUser()->getUID();
 
 		$result = $userResults = ['wide' => [], 'exact' => []];

diff --git a/lib/private/Collaboration/Collaborators/UserPlugin.php b/lib/private/Collaboration/Collaborators/UserPlugin.php
@@ -54,6 +54,12 @@ class UserPlugin implements ISearchPlugin {
 	protected $shareeEnumerationPhone;
 	/* @var bool */
 	protected $shareeEnumerationFullMatch;
+	/* @var bool */
+	protected $shareeEnumerationFullMatchUserId;
+	/* @var bool */
+	protected $shareeEnumerationFullMatchEmail;
+	/* @var bool */
+	protected $shareeEnumerationFullMatchIgnoreSecondDisplayName;
 
 	/** @var IConfig */
 	private $config;
@@ -87,6 +93,9 @@ public function __construct(IConfig $config,
 		$this->shareeEnumerationInGroupOnly = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes';
 		$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->shareeEnumerationFullMatchEmail = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_email', '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) {
@@ -178,7 +187,8 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) {
 				$this->shareeEnumerationFullMatch &&
 				$lowerSearch !== '' && (strtolower($uid) === $lowerSearch ||
 				strtolower($userDisplayName) === $lowerSearch ||
-				strtolower($userEmail) === $lowerSearch)
+				($this->shareeEnumerationFullMatchIgnoreSecondDisplayName && trim(strtolower(preg_replace('/ \(.*\)$/', '', $userDisplayName))) === $lowerSearch) ||
+				($this->shareeEnumerationFullMatchEmail && strtolower($userEmail) === $lowerSearch))
 			) {
 				if (strtolower($uid) === $lowerSearch) {
 					$foundUserById = true;
@@ -228,7 +238,7 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) {
 			}
 		}
 
-		if ($this->shareeEnumerationFullMatch && $offset === 0 && !$foundUserById) {
+		if ($this->shareeEnumerationFullMatch && $this->shareeEnumerationFullMatchUserId && $offset === 0 && !$foundUserById) {
 			// On page one we try if the search result has a direct hit on the
 			// user id and if so, we add that to the exact match list
 			$user = $this->userManager->get($search);

diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php
@@ -1915,6 +1915,14 @@ public function allowEnumerationFullMatch(): bool {
 		return $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes') === 'yes';
 	}
 
+	public function matchEmail(): bool {
+		return $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_email', 'yes') === 'yes';
+	}
+
+	public function ignoreSecondDisplayName(): bool {
+		return $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_ignore_second_display_name', 'no') === 'yes';
+	}
+
 	public function currentUserCanEnumerateTargetUser(?IUser $currentUser, IUser $targetUser): bool {
 		if ($this->allowEnumerationFullMatch()) {
 			return true;

diff --git a/lib/public/Share/IManager.php b/lib/public/Share/IManager.php
@@ -448,6 +448,22 @@ public function limitEnumerationToPhone(): bool;
 	 */
 	public function allowEnumerationFullMatch(): bool;
 
+	/**
+	 * Check if the search should match the email
+	 *
+	 * @return bool
+	 * @since 25.0.0
+	 */
+	public function matchEmail(): bool;
+
+	/**
+	 * Check if the search should ignore the second in parentheses display name if there is any
+	 *
+	 * @return bool
+	 * @since 25.0.0
+	 */
+	public function ignoreSecondDisplayName(): bool;
+
 	/**
 	 * Check if the current user can enumerate the target user
 	 *