respect shareapi_allow_share_dialog_user_enumeration in Principal bac…

…kend for Sabre/DAV Signed-off-by: Georg Ehrke <developer@georgehrke.com>
nextcloud · Mar 3, 2020 · ccfd570 · ccfd570
1 parent 4ef42c2
commit ccfd570
Show file tree

Hide file tree

Showing 4 changed files with 101 additions and 2 deletions.
diff --git a/apps/dav/lib/Connector/Sabre/Principal.php b/apps/dav/lib/Connector/Sabre/Principal.php
@@ -82,6 +82,7 @@ class Principal implements BackendInterface {
 	 * @param IGroupManager $groupManager
 	 * @param IShareManager $shareManager
 	 * @param IUserSession $userSession
+	 * @param IAppManager $appManager
 	 * @param IConfig $config
 	 * @param string $principalPrefix
 	 */
@@ -239,6 +240,8 @@ protected function searchUserPrincipals(array $searchProperties, $test = 'allof'
 			return [];
 		}
 
+		$allowEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
+
 		// If sharing is restricted to group members only,
 		// return only members that have groups in common
 		$restrictGroups = false;
@@ -256,6 +259,12 @@ protected function searchUserPrincipals(array $searchProperties, $test = 'allof'
 				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;
+						});
+					}
+
 					$results[] = array_reduce($users, function(array $carry, IUser $user) use ($restrictGroups) {
 						// is sharing restricted to groups only?
 						if ($restrictGroups !== false) {
@@ -273,6 +282,12 @@ protected function searchUserPrincipals(array $searchProperties, $test = 'allof'
 				case '{DAV:}displayname':
 					$users = $this->userManager->searchDisplayName($value);
 
+					if (!$allowEnumeration) {
+						$users = \array_filter($users, static function(IUser $user) use ($value) {
+							return $user->getDisplayName() === $value;
+						});
+					}
+
 					$results[] = array_reduce($users, function(array $carry, IUser $user) use ($restrictGroups) {
 						// is sharing restricted to groups only?
 						if ($restrictGroups !== false) {

diff --git a/apps/dav/lib/RootCollection.php b/apps/dav/lib/RootCollection.php
@@ -61,7 +61,7 @@ public function __construct() {
 			$config,
 			\OC::$server->getAppManager()
 		);
-		$groupPrincipalBackend = new GroupPrincipalBackend($groupManager, $userSession, $shareManager, $l10n);
+		$groupPrincipalBackend = new GroupPrincipalBackend($groupManager, $userSession, $shareManager);
 		$calendarResourcePrincipalBackend = new ResourcePrincipalBackend($db, $userSession, $groupManager, $logger);
 		$calendarRoomPrincipalBackend = new RoomPrincipalBackend($db, $userSession, $groupManager, $logger);
 		// as soon as debug mode is enabled we allow listing of principals

diff --git a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php
@@ -395,7 +395,7 @@ public function testMultipleUIDDenied() {
 		// create a card
 		$uri0 = $this->getUniqueID('card');
 		$this->backend->createCard($bookId, $uri0, $this->vcardTest0);
-		
+
 		// create another card with same uid
 		$uri1 = $this->getUniqueID('card');
 		$this->expectException(BadRequest::class);

diff --git a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php
@@ -308,6 +308,11 @@ public function testSearchPrincipals($sharingEnabled, $groupsOnly, $test, $resul
 			->will($this->returnValue($sharingEnabled));
 
 		if ($sharingEnabled) {
+			$this->config->expects($this->once())
+				->method('getAppValue')
+				->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes')
+				->willReturn('yes');
+
 			$this->shareManager->expects($this->once())
 				->method('shareWithGroupMembersOnly')
 				->will($this->returnValue($groupsOnly));
@@ -324,6 +329,8 @@ public function testSearchPrincipals($sharingEnabled, $groupsOnly, $test, $resul
 					->will($this->returnValue(['group1', 'group2', 'group5']));
 			}
 		} else {
+			$this->config->expects($this->never())
+				->method('getAppValue');
 			$this->shareManager->expects($this->never())
 				->method('shareWithGroupMembersOnly');
 			$this->groupManager->expects($this->never())
@@ -396,6 +403,11 @@ public function testSearchPrincipalByCalendarUserAddressSet() {
 			->method('shareAPIEnabled')
 			->will($this->returnValue(true));
 
+		$this->config->expects($this->exactly(2))
+			->method('getAppValue')
+			->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes')
+			->willReturn('yes');
+
 		$this->shareManager->expects($this->exactly(2))
 			->method('shareWithGroupMembersOnly')
 			->will($this->returnValue(false));
@@ -417,6 +429,78 @@ public function testSearchPrincipalByCalendarUserAddressSet() {
 			['{urn:ietf:params:xml:ns:caldav}calendar-user-address-set' => 'user@example.com']));
 	}
 
+	public function testSearchPrincipalWithEnumerationDisabledDisplayname() {
+		$this->shareManager->expects($this->once())
+			->method('shareAPIEnabled')
+			->will($this->returnValue(true));
+
+		$this->config->expects($this->once())
+			->method('getAppValue')
+			->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes')
+			->willReturn('no');
+
+		$this->shareManager->expects($this->once())
+			->method('shareWithGroupMembersOnly')
+			->will($this->returnValue(false));
+
+		$user2 = $this->createMock(IUser::class);
+		$user2->method('getUID')->will($this->returnValue('user2'));
+		$user2->method('getDisplayName')->will($this->returnValue('User 2'));
+		$user2->method('getEMailAddress')->will($this->returnValue('user2@foo.bar'));
+		$user3 = $this->createMock(IUser::class);
+		$user3->method('getUID')->will($this->returnValue('user3'));
+		$user2->method('getDisplayName')->will($this->returnValue('User 22'));
+		$user2->method('getEMailAddress')->will($this->returnValue('user2@foo.bar123'));
+		$user4 = $this->createMock(IUser::class);
+		$user4->method('getUID')->will($this->returnValue('user4'));
+		$user2->method('getDisplayName')->will($this->returnValue('User 222'));
+		$user2->method('getEMailAddress')->will($this->returnValue('user2@foo.bar456'));
+
+		$this->userManager->expects($this->at(0))
+			->method('searchDisplayName')
+			->with('User 2')
+			->will($this->returnValue([$user2, $user3, $user4]));
+
+		$this->assertEquals(['principals/users/user2'], $this->connector->searchPrincipals('principals/users',
+			['{DAV:}displayname' => 'User 2']));
+	}
+
+	public function testSearchPrincipalWithEnumerationDisabledEmail() {
+		$this->shareManager->expects($this->once())
+			->method('shareAPIEnabled')
+			->will($this->returnValue(true));
+
+		$this->config->expects($this->once())
+			->method('getAppValue')
+			->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes')
+			->willReturn('no');
+
+		$this->shareManager->expects($this->once())
+			->method('shareWithGroupMembersOnly')
+			->will($this->returnValue(false));
+
+		$user2 = $this->createMock(IUser::class);
+		$user2->method('getUID')->will($this->returnValue('user2'));
+		$user2->method('getDisplayName')->will($this->returnValue('User 2'));
+		$user2->method('getEMailAddress')->will($this->returnValue('user2@foo.bar'));
+		$user3 = $this->createMock(IUser::class);
+		$user3->method('getUID')->will($this->returnValue('user3'));
+		$user2->method('getDisplayName')->will($this->returnValue('User 22'));
+		$user2->method('getEMailAddress')->will($this->returnValue('user2@foo.bar123'));
+		$user4 = $this->createMock(IUser::class);
+		$user4->method('getUID')->will($this->returnValue('user4'));
+		$user2->method('getDisplayName')->will($this->returnValue('User 222'));
+		$user2->method('getEMailAddress')->will($this->returnValue('user2@foo.bar456'));
+
+		$this->userManager->expects($this->at(0))
+			->method('getByEmail')
+			->with('user2@foo.bar')
+			->will($this->returnValue([$user2, $user3, $user4]));
+
+		$this->assertEquals(['principals/users/user2'], $this->connector->searchPrincipals('principals/users',
+			['{http://sabredav.org/ns}email-address' => 'user2@foo.bar']));
+	}
+
 	public function testFindByUriSharingApiDisabled() {
 		$this->shareManager->expects($this->once())
 			->method('shareApiEnabled')