Merge pull request #21538 from nextcloud/backport/21452/stable19

[stable19] Fix autocomplete for LDAP with `shareapi_only_share_with_group_members` on
This commit is contained in:
Joas Schilling 2020-07-01 10:13:19 +02:00 committed by GitHub
commit 9e9e74736d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 71 additions and 49 deletions

View File

@ -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"

View File

@ -33,7 +33,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;
@ -73,20 +72,19 @@ 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 {
// Search in all users
$usersTmp = $this->userManager->searchDisplayName($search, $limit, $offset);
$currentUserGroups = $this->groupManager->getUserGroupIds($this->userSession->getUser());
foreach ($usersTmp as $user) {
if ($user->isEnabled()) { // Don't keep deactivated users
$users[$user->getUID()] = $user;
@ -155,10 +153,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);
}

View File

@ -275,7 +275,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, []],
],
[],
@ -284,6 +284,7 @@ class UserPluginTest extends TestCase {
],
true,
false,
[['test1', $this->getUserMock('test1', 'Test One')]],
],
[
'test',
@ -291,13 +292,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',
@ -306,12 +308,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',
]],
],
[],
@ -321,6 +323,10 @@ class UserPluginTest extends TestCase {
],
false,
false,
[
['test1', $this->getUserMock('test1', 'Test One')],
['test2', $this->getUserMock('test2', 'Test Two')],
],
],
[
'test',
@ -329,18 +335,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',
@ -349,10 +359,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',
]],
],
[
@ -363,6 +373,10 @@ class UserPluginTest extends TestCase {
],
false,
false,
[
['test', $this->getUserMock('test', 'Test One')],
['test2', $this->getUserMock('test2', 'Test Two')],
],
],
[
'test',
@ -371,10 +385,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',
]],
],
[
@ -383,6 +397,10 @@ class UserPluginTest extends TestCase {
[],
true,
false,
[
['test', $this->getUserMock('test', 'Test One')],
['test2', $this->getUserMock('test2', 'Test Two')],
],
],
];
}
@ -399,6 +417,7 @@ class UserPluginTest extends TestCase {
* @param array $expected
* @param bool $reachedEnd
* @param bool|IUser $singleUser
* @param array $users
*/
public function testSearch(
$searchTerm,
@ -409,7 +428,8 @@ class UserPluginTest extends TestCase {
array $exactExpected,
array $expected,
$reachedEnd,
$singleUser
$singleUser,
array $users = []
) {
$this->mockConfig($shareWithGroupOnly, $shareeEnumeration, false);
$this->instantiatePlugin();
@ -424,39 +444,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();