Add a setting to restrict returning a full match unless in phonebook …

…or same group Signed-off-by: Joas Schilling <coding@schilljs.com>
nextcloud · Mar 10, 2021 · f7d1cf0 · f7d1cf0
1 parent 177ae33
commit f7d1cf0
Show file tree

Hide file tree

Showing 13 changed files with 243 additions and 20 deletions.
diff --git a/apps/dav/lib/Connector/Sabre/Principal.php b/apps/dav/lib/Connector/Sabre/Principal.php
@@ -263,6 +263,7 @@ protected function searchUserPrincipals(array $searchProperties, $test = 'allof'
 		$allowEnumeration = $this->shareManager->allowEnumeration();
 		$limitEnumerationGroup = $this->shareManager->limitEnumerationToGroups();
 		$limitEnumerationPhone = $this->shareManager->limitEnumerationToPhone();
+		$allowEnumerationFullMatch = $this->shareManager->allowEnumerationFullMatch();
 
 		// If sharing is restricted to group members only,
 		// return only members that have groups in common
@@ -290,15 +291,19 @@ protected function searchUserPrincipals(array $searchProperties, $test = 'allof'
 		foreach ($searchProperties as $prop => $value) {
 			switch ($prop) {
 				case '{http://sabredav.org/ns}email-address':
-					$users = $this->userManager->getByEmail($value);
-
 					if (!$allowEnumeration) {
-						$users = \array_filter($users, static function (IUser $user) use ($value) {
-							return $user->getEMailAddress() === $value;
-						});
+						if ($allowEnumerationFullMatch) {
+							$users = $this->userManager->getByEmail($value);
+							$users = \array_filter($users, static function (IUser $user) use ($value) {
+								return $user->getEMailAddress() === $value;
+							});
+						} else {
+							$users = [];
+						}
 					} else {
-						$users = \array_filter($users, function (IUser $user) use ($currentUser, $value, $limitEnumerationPhone, $limitEnumerationGroup, $currentUserGroups) {
-							if ($user->getEMailAddress() === $value) {
+						$users = $this->userManager->getByEmail($value);
+						$users = \array_filter($users, function (IUser $user) use ($currentUser, $value, $limitEnumerationPhone, $limitEnumerationGroup, $allowEnumerationFullMatch, $currentUserGroups) {
+							if ($allowEnumerationFullMatch && $user->getEMailAddress() === $value) {
 								return true;
 							}
 
@@ -336,15 +341,20 @@ protected function searchUserPrincipals(array $searchProperties, $test = 'allof'
 					break;
 
 				case '{DAV:}displayname':
-					$users = $this->userManager->searchDisplayName($value, $searchLimit);
 
 					if (!$allowEnumeration) {
-						$users = \array_filter($users, static function (IUser $user) use ($value) {
-							return $user->getDisplayName() === $value;
-						});
+						if ($allowEnumerationFullMatch) {
+							$users = $this->userManager->searchDisplayName($value, $searchLimit);
+							$users = \array_filter($users, static function (IUser $user) use ($value) {
+								return $user->getDisplayName() === $value;
+							});
+						} else {
+							$users = [];
+						}
 					} else {
-						$users = \array_filter($users, function (IUser $user) use ($currentUser, $value, $limitEnumerationPhone, $limitEnumerationGroup, $currentUserGroups) {
-							if ($user->getDisplayName() === $value) {
+						$users = $this->userManager->searchDisplayName($value, $searchLimit);
+						$users = \array_filter($users, function (IUser $user) use ($currentUser, $value, $limitEnumerationPhone, $limitEnumerationGroup, $allowEnumerationFullMatch, $currentUserGroups) {
+							if ($allowEnumerationFullMatch && $user->getDisplayName() === $value) {
 								return true;
 							}
 

diff --git a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php
@@ -570,6 +570,10 @@ public function testSearchPrincipalWithEnumerationDisabledDisplayname() {
 			->method('shareWithGroupMembersOnly')
 			->willReturn(false);
 
+		$this->shareManager->expects($this->once())
+			->method('allowEnumerationFullMatch')
+			->willReturn(true);
+
 		$user2 = $this->createMock(IUser::class);
 		$user2->method('getUID')->willReturn('user2');
 		$user2->method('getDisplayName')->willReturn('User 2');
@@ -592,6 +596,27 @@ public function testSearchPrincipalWithEnumerationDisabledDisplayname() {
 			['{DAV:}displayname' => 'User 2']));
 	}
 
+	public function testSearchPrincipalWithEnumerationDisabledDisplaynameOnFullMatch() {
+		$this->shareManager->expects($this->once())
+			->method('shareAPIEnabled')
+			->willReturn(true);
+
+		$this->shareManager->expects($this->once())
+			->method('allowEnumeration')
+			->willReturn(false);
+
+		$this->shareManager->expects($this->once())
+			->method('shareWithGroupMembersOnly')
+			->willReturn(false);
+
+		$this->shareManager->expects($this->once())
+			->method('allowEnumerationFullMatch')
+			->willReturn(false);
+
+		$this->assertEquals([], $this->connector->searchPrincipals('principals/users',
+			['{DAV:}displayname' => 'User 2']));
+	}
+
 	public function testSearchPrincipalWithEnumerationDisabledEmail() {
 		$this->shareManager->expects($this->once())
 			->method('shareAPIEnabled')
@@ -605,6 +630,10 @@ public function testSearchPrincipalWithEnumerationDisabledEmail() {
 			->method('shareWithGroupMembersOnly')
 			->willReturn(false);
 
+		$this->shareManager->expects($this->once())
+			->method('allowEnumerationFullMatch')
+			->willReturn(true);
+
 		$user2 = $this->createMock(IUser::class);
 		$user2->method('getUID')->willReturn('user2');
 		$user2->method('getDisplayName')->willReturn('User 2');
@@ -627,6 +656,28 @@ public function testSearchPrincipalWithEnumerationDisabledEmail() {
 			['{http://sabredav.org/ns}email-address' => 'user2@foo.bar']));
 	}
 
+	public function testSearchPrincipalWithEnumerationDisabledEmailOnFullMatch() {
+		$this->shareManager->expects($this->once())
+			->method('shareAPIEnabled')
+			->willReturn(true);
+
+		$this->shareManager->expects($this->once())
+			->method('allowEnumeration')
+			->willReturn(false);
+
+		$this->shareManager->expects($this->once())
+			->method('shareWithGroupMembersOnly')
+			->willReturn(false);
+
+		$this->shareManager->expects($this->once())
+			->method('allowEnumerationFullMatch')
+			->willReturn(false);
+
+
+		$this->assertEquals([], $this->connector->searchPrincipals('principals/users',
+			['{http://sabredav.org/ns}email-address' => 'user2@foo.bar']));
+	}
+
 	public function testSearchPrincipalWithEnumerationLimitedDisplayname() {
 		$this->shareManager->expects($this->at(0))
 			->method('shareAPIEnabled')

diff --git a/apps/settings/lib/Settings/Admin/Sharing.php b/apps/settings/lib/Settings/Admin/Sharing.php
@@ -74,6 +74,7 @@ public function getForm() {
 			'allowShareDialogUserEnumeration' => $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes'),
 			'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'),
 			'enforceLinkPassword' => Util::isPublicLinkPasswordRequired(),
 			'onlyShareWithGroupMembers' => $this->shareManager->shareWithGroupMembersOnly(),
 			'shareAPIEnabled' => $this->config->getAppValue('core', 'shareapi_enabled', 'yes'),

diff --git a/apps/settings/templates/settings/admin/sharing.php b/apps/settings/templates/settings/admin/sharing.php
@@ -163,7 +163,7 @@
 			<?php if ($_['allowShareDialogUserEnumeration'] === 'yes') {
 	print_unescaped('checked="checked"');
 } ?> />
-		<label for="shareapi_allow_share_dialog_user_enumeration"><?php p($l->t('Allow username autocompletion in share dialog (if this is disabled the full username or email address needs to be entered)'));?></label><br />
+		<label for="shareapi_allow_share_dialog_user_enumeration"><?php p($l->t('Allow username autocompletion in share dialog'));?></label><br />
 	</p>
 
 	<p id="shareapi_restrict_user_enumeration_to_group_setting" class="indent <?php if ($_['shareAPIEnabled'] === 'no' || $_['allowShareDialogUserEnumeration'] === 'no') {
@@ -190,6 +190,15 @@
 }?>">
 		<em><?php p($l->t('If autocompletion "same group" and "phonebook matches" are enabled a match in either is enough to show the user.'));?></em><br />
 	</p>
+	<p id="shareapi_restrict_user_enumeration_full_match_setting" class="indent <?php if ($_['shareAPIEnabled'] === 'no') {
+		p('hidden');
+	}?>">
+		<input type="checkbox" name="shareapi_restrict_user_enumeration_full_match" value="1" id="shareapi_restrict_user_enumeration_full_match" class="checkbox"
+				<?php if ($_['restrictUserEnumerationFullMatch'] === 'yes') {
+					print_unescaped('checked="checked"');
+				} ?> />
+		<label for="shareapi_restrict_user_enumeration_full_match"><?php p($l->t('Allow username autocompletion when entering the full name or email address'));?></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
@@ -74,6 +74,7 @@ public function testGetFormWithoutExcludedGroups() {
 				['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'],
 				['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_enabled', 'yes', 'yes'],
 				['core', 'shareapi_default_expire_date', 'no', 'no'],
 				['core', 'shareapi_expire_after_n_days', '7', '7'],
@@ -98,6 +99,7 @@ public function testGetFormWithoutExcludedGroups() {
 				'allowShareDialogUserEnumeration' => 'yes',
 				'restrictUserEnumerationToGroup' => 'no',
 				'restrictUserEnumerationToPhone' => 'no',
+				'restrictUserEnumerationFullMatch' => 'yes',
 				'enforceLinkPassword' => false,
 				'onlyShareWithGroupMembers' => false,
 				'shareAPIEnabled' => 'yes',
@@ -132,6 +134,7 @@ public function testGetFormWithExcludedGroups() {
 				['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'],
 				['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_enabled', 'yes', 'yes'],
 				['core', 'shareapi_default_expire_date', 'no', 'no'],
 				['core', 'shareapi_expire_after_n_days', '7', '7'],
@@ -156,6 +159,7 @@ public function testGetFormWithExcludedGroups() {
 				'allowShareDialogUserEnumeration' => 'yes',
 				'restrictUserEnumerationToGroup' => 'no',
 				'restrictUserEnumerationToPhone' => 'no',
+				'restrictUserEnumerationFullMatch' => 'yes',
 				'enforceLinkPassword' => false,
 				'onlyShareWithGroupMembers' => false,
 				'shareAPIEnabled' => 'yes',

diff --git a/build/integration/collaboration_features/autocomplete.feature b/build/integration/collaboration_features/autocomplete.feature
@@ -3,6 +3,7 @@ Feature: autocomplete
     Given using api version "2"
     And group "commongroup" exists
     And user "admin" belongs to group "commongroup"
+    And user "auto" exists
     And user "autocomplete" exists
     And user "autocomplete2" exists
     And user "autocomplete2" belongs to group "commongroup"
@@ -20,16 +21,23 @@ Feature: autocomplete
     When parameter "shareapi_allow_share_dialog_user_enumeration" of app "core" is set to "no"
     Then get autocomplete for "auto"
       | id | source |
+      | auto | users |
     Then get autocomplete for "autocomplete"
       | id | source |
       | autocomplete | users |
+    When parameter "shareapi_restrict_user_enumeration_full_match" of app "core" is set to "no"
+    Then get autocomplete for "auto"
+      | id | source |
+    Then get autocomplete for "autocomplete"
+      | id | source |
 
 
   Scenario: getting autocomplete with limited enumeration by group
     Given As an "admin"
     When parameter "shareapi_restrict_user_enumeration_to_group" of app "core" is set to "yes"
     Then get autocomplete for "auto"
       | id | source |
+      | auto | users |
       | autocomplete2 | users |
     Then get autocomplete for "autocomplete"
       | id | source |
@@ -38,13 +46,21 @@ Feature: autocomplete
     Then get autocomplete for "autocomplete2"
       | id | source |
       | autocomplete2 | users |
+    When parameter "shareapi_restrict_user_enumeration_full_match" of app "core" is set to "no"
+    Then get autocomplete for "autocomplete"
+      | id | source |
+      | autocomplete2 | users |
+    Then get autocomplete for "autocomplete2"
+      | id | source |
+      | autocomplete2 | users |
 
 
   Scenario: getting autocomplete with limited enumeration by phone
     Given As an "admin"
     When parameter "shareapi_restrict_user_enumeration_to_phone" of app "core" is set to "yes"
     Then get autocomplete for "auto"
       | id | source |
+      | auto | users |
 
     # autocomplete stores their phone number
     Given As an "autocomplete"
@@ -57,10 +73,17 @@ Feature: autocomplete
     Given As an "admin"
     Then get autocomplete for "auto"
       | id | source |
+      | auto | users |
 
     # admin populates they have the phone number
     When search users by phone for region "DE" with
       | random-string1 | 0711 / 252 428-90 |
+    Then get autocomplete for "auto"
+      | id | source |
+      | auto | users |
+      | autocomplete | users |
+
+    When parameter "shareapi_restrict_user_enumeration_full_match" of app "core" is set to "no"
     Then get autocomplete for "auto"
       | id | source |
       | autocomplete | users |
@@ -83,6 +106,13 @@ Feature: autocomplete
     When search users by phone for region "DE" with
       | random-string1 | 0711 / 252 428-90 |
 
+    Then get autocomplete for "auto"
+      | id | source |
+      | auto | users |
+      | autocomplete | users |
+      | autocomplete2 | users |
+
+    When parameter "shareapi_restrict_user_enumeration_full_match" of app "core" is set to "no"
     Then get autocomplete for "auto"
       | id | source |
       | autocomplete | users |
@@ -108,6 +138,7 @@ Feature: autocomplete
 
     Then get autocomplete for "auto"
       | id | source |
+      | auto | users |
       | autocomplete | users |
       | autocomplete2 | users |
     When parameter "shareapi_only_share_with_group_members" of app "core" is set to "yes"
@@ -121,6 +152,7 @@ Feature: autocomplete
     When parameter "shareapi_restrict_user_enumeration_to_phone" of app "core" is set to "yes"
     Then get autocomplete for "auto"
       | id | source |
+      | auto | users |
 
     # autocomplete stores their phone number
     Given As an "autocomplete"
@@ -133,12 +165,14 @@ Feature: autocomplete
     Given As an "admin"
     Then get autocomplete for "auto"
       | id | source |
+      | auto | users |
 
     # admin populates they have the phone number
     When search users by phone for region "DE" with
       | random-string1 | 0711 / 252 428-90 |
     Then get autocomplete for "auto"
       | id | source |
+      | auto | users |
       | autocomplete | users |
 
     # autocomplete changes their phone number
@@ -152,12 +186,14 @@ Feature: autocomplete
     Given As an "admin"
     Then get autocomplete for "auto"
       | id | source |
+      | auto | users |
 
     # admin populates they have the new phone number
     When search users by phone for region "DE" with
       | random-string1 | 0711 / 252 428-91 |
     Then get autocomplete for "auto"
       | id | source |
+      | auto | users |
       | autocomplete | users |
 
 

diff --git a/build/integration/features/bootstrap/CollaborationContext.php b/build/integration/features/bootstrap/CollaborationContext.php
@@ -66,6 +66,7 @@ protected function resetAppConfigs(): void {
 		$this->deleteServerConfig('core', 'shareapi_allow_share_dialog_user_enumeration');
 		$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_only_share_with_group_members');
 	}
 }
diff --git a/lib/private/Collaboration/Collaborators/MailPlugin.php b/lib/private/Collaboration/Collaborators/MailPlugin.php
@@ -49,6 +49,8 @@ class MailPlugin implements ISearchPlugin {
 	protected $shareeEnumerationInGroupOnly;
 	/* @var bool */
 	protected $shareeEnumerationPhone;
+	/* @var bool */
+	protected $shareeEnumerationFullMatch;
 
 	/** @var IManager */
 	private $contactsManager;
@@ -81,6 +83,7 @@ public function __construct(IManager $contactsManager,
 		$this->shareWithGroupOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes';
 		$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';
 	}
 
 	/**
@@ -137,7 +140,7 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) {
 								continue;
 							}
 						}
-						if ($exactEmailMatch) {
+						if ($exactEmailMatch && $this->shareeEnumerationFullMatch) {
 							try {
 								$cloud = $this->cloudIdManager->resolveCloudId($contact['CLOUD'][0]);
 							} catch (\InvalidArgumentException $e) {