Merge pull request #21539 from nextcloud/backport/21452/stable18

[stable18] Fix autocomplete for LDAP with `shareapi_only_share_with_group_members` on

3 files changed, 71 insertions, 48 deletions

diff --git a/build/integration/sharees_features/sharees.feature b/build/integration/sharees_features/sharees.feature
index 5a6291d1e2f..1d770b96b72 100644
--- a/build/integration/sharees_features/sharees.feature
+++ b/build/integration/sharees_features/sharees.feature
@@ -72,6 +72,23 @@ Feature: sharees
     And "exact remotes" sharees returned is empty
     And "remotes" sharees returned is empty
 
+  Scenario: Search only with group members - allowed with exact match
+    Given As an "test"
+    And parameter "shareapi_only_share_with_group_members" of app "core" is set to "yes"
+    And user "Sharee1" belongs to group "ShareeGroup"
+    When getting sharees for
+      | search | Sharee1 |
+      | itemType | file |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And "exact users" sharees returned are
+      | Sharee1 | 0 | Sharee1 |
+    And "users" sharees returned is empty
+    And "exact groups" sharees returned is empty
+    And "groups" sharees returned is empty
+    And "exact remotes" sharees returned is empty
+    And "remotes" sharees returned is empty
+
   Scenario: Search only with group members - no group as non-member
     Given As an "Sharee1"
     And parameter "shareapi_only_share_with_group_members" of app "core" is set to "yes"
diff --git a/lib/private/Collaboration/Collaborators/UserPlugin.php b/lib/private/Collaboration/Collaborators/UserPlugin.php
index 5fb278fc5ff..c92f7bb0b6e 100644
--- a/lib/private/Collaboration/Collaborators/UserPlugin.php
+++ b/lib/private/Collaboration/Collaborators/UserPlugin.php
@@ -31,7 +31,6 @@ use OCP\Collaboration\Collaborators\ISearchPlugin;
 use OCP\Collaboration\Collaborators\ISearchResult;
 use OCP\Collaboration\Collaborators\SearchResultType;
 use OCP\IConfig;
-use OCP\IGroup;
 use OCP\IGroupManager;
 use OCP\IUser;
 use OCP\IUserManager;
@@ -68,14 +67,14 @@ class UserPlugin implements ISearchPlugin {
 		$users = [];
 		$hasMoreResults = false;
 
-		$userGroups = [];
+		$currentUserGroups = $this->groupManager->getUserGroupIds($this->userSession->getUser());
 		if ($this->shareWithGroupOnly) {
 			// Search in all the groups this user is part of
-			$userGroups = $this->groupManager->getUserGroups($this->userSession->getUser());
-			foreach ($userGroups as $userGroup) {
-				$usersInGroup = $userGroup->searchDisplayName($search, $limit, $offset);
-				foreach ($usersInGroup as $user) {
-					$users[$user->getUID()] = $user;
+			foreach ($currentUserGroups as $userGroupId) {
+				$usersInGroup = $this->groupManager->displayNamesInGroup($userGroupId, $search, $limit, $offset);
+				foreach ($usersInGroup as $userId => $displayName) {
+					$userId = (string) $userId;
+					$users[$userId] = $this->userManager->get($userId);
 				}
 			}
 		} else {
@@ -136,10 +135,7 @@ class UserPlugin implements ISearchPlugin {
 
 				if ($this->shareWithGroupOnly) {
 					// Only add, if we have a common group
-					$userGroupIds = array_map(function(IGroup $group) {
-						return $group->getGID();
-					}, $userGroups);
-					$commonGroups = array_intersect($userGroupIds, $this->groupManager->getUserGroupIds($user));
+					$commonGroups = array_intersect($currentUserGroups, $this->groupManager->getUserGroupIds($user));
 					$addUser = !empty($commonGroups);
 				}
 
diff --git a/tests/lib/Collaboration/Collaborators/UserPluginTest.php b/tests/lib/Collaboration/Collaborators/UserPluginTest.php
index 94ed0684699..c73277c946b 100644
--- a/tests/lib/Collaboration/Collaborators/UserPluginTest.php
+++ b/tests/lib/Collaboration/Collaborators/UserPluginTest.php
@@ -258,7 +258,7 @@ class UserPluginTest extends TestCase {
 				true,
 				['abc', 'xyz'],
 				[
-					['abc', 'test', 2, 0, [$this->getUserMock('test1', 'Test One')]],
+					['abc', 'test', 2, 0, ['test1' => 'Test One']],
 					['xyz', 'test', 2, 0, []],
 				],
 				[],
@@ -267,6 +267,7 @@ class UserPluginTest extends TestCase {
 				],
 				true,
 				false,
+				[['test1', $this->getUserMock('test1', 'Test One')]],
 			],
 			[
 				'test',
@@ -274,13 +275,14 @@ class UserPluginTest extends TestCase {
 				false,
 				['abc', 'xyz'],
 				[
-					['abc', 'test', 2, 0, [$this->getUserMock('test1', 'Test One')]],
+					['abc', 'test', 2, 0, ['test1' => 'Test One']],
 					['xyz', 'test', 2, 0, []],
 				],
 				[],
 				[],
 				true,
 				false,
+				[['test1', $this->getUserMock('test1', 'Test One')]],
 			],
 			[
 				'test',
@@ -289,12 +291,12 @@ class UserPluginTest extends TestCase {
 				['abc', 'xyz'],
 				[
 					['abc', 'test', 2, 0, [
-						$this->getUserMock('test1', 'Test One'),
-						$this->getUserMock('test2', 'Test Two'),
+						'test1' => 'Test One',
+						'test2' => 'Test Two',
 					]],
 					['xyz', 'test', 2, 0, [
-						$this->getUserMock('test1', 'Test One'),
-						$this->getUserMock('test2', 'Test Two'),
+						'test1' => 'Test One',
+						'test2' => 'Test Two',
 					]],
 				],
 				[],
@@ -304,6 +306,10 @@ class UserPluginTest extends TestCase {
 				],
 				false,
 				false,
+				[
+					['test1', $this->getUserMock('test1', 'Test One')],
+					['test2', $this->getUserMock('test2', 'Test Two')],
+				],
 			],
 			[
 				'test',
@@ -312,18 +318,22 @@ class UserPluginTest extends TestCase {
 				['abc', 'xyz'],
 				[
 					['abc', 'test', 2, 0, [
-						$this->getUserMock('test1', 'Test One'),
-						$this->getUserMock('test2', 'Test Two'),
+						'test1' => 'Test One',
+						'test2' => 'Test Two',
 					]],
 					['xyz', 'test', 2, 0, [
-						$this->getUserMock('test1', 'Test One'),
-						$this->getUserMock('test2', 'Test Two'),
+						'test1' => 'Test One',
+						'test2' => 'Test Two',
 					]],
 				],
 				[],
 				[],
 				true,
 				false,
+				[
+					['test1', $this->getUserMock('test1', 'Test One')],
+					['test2', $this->getUserMock('test2', 'Test Two')],
+				],
 			],
 			[
 				'test',
@@ -332,10 +342,10 @@ class UserPluginTest extends TestCase {
 				['abc', 'xyz'],
 				[
 					['abc', 'test', 2, 0, [
-						$this->getUserMock('test', 'Test One'),
+						'test' => 'Test One',
 					]],
 					['xyz', 'test', 2, 0, [
-						$this->getUserMock('test2', 'Test Two'),
+						'test2' => 'Test Two',
 					]],
 				],
 				[
@@ -346,6 +356,10 @@ class UserPluginTest extends TestCase {
 				],
 				false,
 				false,
+				[
+					['test', $this->getUserMock('test', 'Test One')],
+					['test2', $this->getUserMock('test2', 'Test Two')],
+				],
 			],
 			[
 				'test',
@@ -354,10 +368,10 @@ class UserPluginTest extends TestCase {
 				['abc', 'xyz'],
 				[
 					['abc', 'test', 2, 0, [
-						$this->getUserMock('test', 'Test One'),
+						'test' => 'Test One',
 					]],
 					['xyz', 'test', 2, 0, [
-						$this->getUserMock('test2', 'Test Two'),
+						'test2' => 'Test Two',
 					]],
 				],
 				[
@@ -366,6 +380,10 @@ class UserPluginTest extends TestCase {
 				[],
 				true,
 				false,
+				[
+					['test', $this->getUserMock('test', 'Test One')],
+					['test2', $this->getUserMock('test2', 'Test Two')],
+				],
 			],
 		];
 	}
@@ -382,6 +400,7 @@ class UserPluginTest extends TestCase {
 	 * @param array $expected
 	 * @param bool $reachedEnd
 	 * @param bool|IUser $singleUser
+	 * @param array $users
 	 */
 	public function testSearch(
 		$searchTerm,
@@ -392,7 +411,8 @@ class UserPluginTest extends TestCase {
 		array $exactExpected,
 		array $expected,
 		$reachedEnd,
-		$singleUser
+		$singleUser,
+		array $users = []
 	) {
 		$this->config->expects($this->any())
 			->method('getAppValue')
@@ -421,39 +441,29 @@ class UserPluginTest extends TestCase {
 				->with($searchTerm, $this->limit, $this->offset)
 				->willReturn($userResponse);
 		} else {
-			$groups = array_combine($groupResponse, array_map(function ($gid) {
-				return $this->getGroupMock($gid);
-			}, $groupResponse));
-			if ($singleUser !== false) {
-				$this->groupManager->method('getUserGroups')
-					->with($this->user)
-					->willReturn($groups);
+			$this->groupManager->method('getUserGroupIds')
+				->with($this->user)
+				->willReturn($groupResponse);
 
+			if ($singleUser !== false) {
 				$this->groupManager->method('getUserGroupIds')
 					->with($singleUser)
 					->willReturn($groupResponse);
-			} else {
-				$this->groupManager->expects($this->once())
-					->method('getUserGroups')
-					->with($this->user)
-					->willReturn($groups);
 			}
 
-			foreach ($userResponse as $groupDefinition) {
-				[$gid, $search, $limit, $offset, $users] = $groupDefinition;
-				$groups[$gid]->method('searchDisplayName')
-					->with($search, $limit, $offset)
-					->willReturn($users);
-			}
+			$this->groupManager->method('displayNamesInGroup')
+				->willReturnMap($userResponse);
 		}
 
 		if ($singleUser !== false) {
-			$this->userManager->expects($this->once())
-				->method('get')
-				->with($searchTerm)
-				->willReturn($singleUser);
+			$users[] = [$searchTerm, $singleUser];
 		}
 
+		if (!empty($users)) {
+			$this->userManager->expects($this->atLeastOnce())
+				->method('get')
+				->willReturnMap($users);
+		}
 
 		$moreResults = $this->plugin->search($searchTerm, $this->limit, $this->offset, $this->searchResult);
 		$result = $this->searchResult->asArray();