Add a setting to restrict returning a full match unless in phonebook or same group

Signed-off-by: Joas Schilling <coding@schilljs.com>

5 files changed, 89 insertions, 14 deletions

diff --git a/apps/dav/lib/Connector/Sabre/Principal.php b/apps/dav/lib/Connector/Sabre/Principal.php
index bb456c954e9..b74747b1163 100644
--- a/apps/dav/lib/Connector/Sabre/Principal.php
+++ b/apps/dav/lib/Connector/Sabre/Principal.php
@@ -263,6 +263,7 @@ class Principal implements BackendInterface {
 		$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 @@ class Principal implements BackendInterface {
 		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 @@ class Principal implements BackendInterface {
 					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
index 33c1ec1b587..c9e3d44bf88 100644
--- a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php
@@ -570,6 +570,10 @@ class PrincipalTest extends TestCase {
 			->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 @@ class PrincipalTest extends TestCase {
 			['{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 @@ class PrincipalTest extends TestCase {
 			->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 @@ class PrincipalTest extends TestCase {
 			['{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
index 19eed576cd7..6285ef399a8 100644
--- a/apps/settings/lib/Settings/Admin/Sharing.php
+++ b/apps/settings/lib/Settings/Admin/Sharing.php
@@ -74,6 +74,7 @@ class Sharing implements ISettings {
 			'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
index a72bf0bd590..d7c24943b24 100644
--- 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 (ignoring missing phonebook match and being in the same group)'));?></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
index 5d0794170a0..1f24ef13d4b 100644
--- a/apps/settings/tests/Settings/Admin/SharingTest.php
+++ b/apps/settings/tests/Settings/Admin/SharingTest.php
@@ -74,6 +74,7 @@ class SharingTest extends TestCase {
 				['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 @@ class SharingTest extends TestCase {
 				'allowShareDialogUserEnumeration' => 'yes',
 				'restrictUserEnumerationToGroup' => 'no',
 				'restrictUserEnumerationToPhone' => 'no',
+				'restrictUserEnumerationFullMatch' => 'yes',
 				'enforceLinkPassword' => false,
 				'onlyShareWithGroupMembers' => false,
 				'shareAPIEnabled' => 'yes',
@@ -132,6 +134,7 @@ class SharingTest extends TestCase {
 				['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 @@ class SharingTest extends TestCase {
 				'allowShareDialogUserEnumeration' => 'yes',
 				'restrictUserEnumerationToGroup' => 'no',
 				'restrictUserEnumerationToPhone' => 'no',
+				'restrictUserEnumerationFullMatch' => 'yes',
 				'enforceLinkPassword' => false,
 				'onlyShareWithGroupMembers' => false,
 				'shareAPIEnabled' => 'yes',