dont show remote and email options if we have an exact match for local user email

Signed-off-by: Robin Appelman <robin@icewind.nl>
This commit is contained in:
Robin Appelman 2019-06-21 15:38:19 +02:00
parent 547ba642c6
commit 01c147a4a5
No known key found for this signature in database
GPG Key ID: 42B69D8A64526EFB
3 changed files with 105 additions and 70 deletions

View File

@ -85,6 +85,13 @@ class Search implements ISearch {
$searchResult->unsetResult($emailType);
}
// if we have an exact local user match, there is no need to show the remote and email matches
$userType = new SearchResultType('users');
if($searchResult->hasExactIdMatch($userType)) {
$searchResult->unsetResult($remoteType);
$searchResult->unsetResult($emailType);
}
return [$searchResult->asArray(), (bool)$hasMoreResults];
}

View File

@ -32,6 +32,7 @@ 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;
@ -75,11 +76,11 @@ class UserPlugin implements ISearchPlugin {
$userGroups = [];
if ($this->shareWithGroupOnly) {
// Search in all the groups this user is part of
$userGroups = $this->groupManager->getUserGroupIds($this->userSession->getUser());
$userGroups = $this->groupManager->getUserGroups($this->userSession->getUser());
foreach ($userGroups as $userGroup) {
$usersTmp = $this->groupManager->displayNamesInGroup($userGroup, $search, $limit, $offset);
foreach ($usersTmp as $uid => $userDisplayName) {
$users[(string) $uid] = $userDisplayName;
$usersInGroup = $userGroup->searchDisplayName($search, $limit, $offset);
foreach ($usersInGroup as $user) {
$users[$user->getUID()] = $user;
}
}
} else {
@ -88,7 +89,7 @@ class UserPlugin implements ISearchPlugin {
$currentUserGroups = $this->groupManager->getUserGroupIds($this->userSession->getUser());
foreach ($usersTmp as $user) {
if ($user->isEnabled()) { // Don't keep deactivated users
$users[(string) $user->getUID()] = $user->getDisplayName();
$users[$user->getUID()] = $user;
$addToWideResults = false;
if ($this->shareeEnumeration && !$this->shareeEnumerationInGroupOnly) {
@ -123,9 +124,15 @@ class UserPlugin implements ISearchPlugin {
$foundUserById = false;
$lowerSearch = strtolower($search);
foreach ($users as $uid => $userDisplayName) {
foreach ($users as $uid => $user) {
$userDisplayName = $user->getDisplayName();
$userEmail = $user->getEMailAddress();
$uid = (string) $uid;
if (strtolower($uid) === $lowerSearch || strtolower($userDisplayName) === $lowerSearch) {
if (
strtolower($uid) === $lowerSearch ||
strtolower($userDisplayName) === $lowerSearch ||
strtolower($userEmail) === $lowerSearch
) {
if (strtolower($uid) === $lowerSearch) {
$foundUserById = true;
}
@ -156,7 +163,10 @@ class UserPlugin implements ISearchPlugin {
if ($this->shareWithGroupOnly) {
// Only add, if we have a common group
$commonGroups = array_intersect($userGroups, $this->groupManager->getUserGroupIds($user));
$userGroupIds = array_map(function(IGroup $group) {
return $group->getGID();
}, $userGroups);
$commonGroups = array_intersect($userGroupIds, $this->groupManager->getUserGroupIds($user));
$addUser = !empty($commonGroups);
}
@ -179,6 +189,9 @@ class UserPlugin implements ISearchPlugin {
$type = new SearchResultType('users');
$searchResult->addResultSet($type, $result['wide'], $result['exact']);
if (count($result['exact'])) {
$searchResult->markExactIdMatch($type);
}
return $hasMoreResults;
}

View File

@ -27,6 +27,7 @@ use OC\Collaboration\Collaborators\SearchResult;
use OC\Collaboration\Collaborators\UserPlugin;
use OCP\Collaboration\Collaborators\ISearchResult;
use OCP\IConfig;
use OCP\IGroup;
use OCP\IGroupManager;
use OCP\IUser;
use OCP\IUserManager;
@ -94,8 +95,7 @@ class UserPluginTest extends TestCase {
->method('getAppValue')
->willReturnCallback(
function ($appName, $key, $default)
use ($shareWithGroupOnly, $shareeEnumeration, $shareeEnumerationLimitToGroup)
{
use ($shareWithGroupOnly, $shareeEnumeration, $shareeEnumerationLimitToGroup) {
if ($appName === 'core' && $key === 'shareapi_only_share_with_group_members') {
return $shareWithGroupOnly ? 'yes' : 'no';
} else if ($appName === 'core' && $key === 'shareapi_allow_share_dialog_user_enumeration') {
@ -127,6 +127,16 @@ class UserPluginTest extends TestCase {
return $user;
}
public function getGroupMock($gid) {
$group = $this->createMock(IGroup::class);
$group->expects($this->any())
->method('getGID')
->willReturn($gid);
return $group;
}
public function dataGetUsers() {
return [
['test', false, true, [], [], [], [], true, false],
@ -137,33 +147,33 @@ class UserPluginTest extends TestCase {
'test', false, true, [], [],
[
['label' => 'Test', 'value' => ['shareType' => Share::SHARE_TYPE_USER, 'shareWith' => 'test']],
], [], true, $this->getUserMock('test', 'Test')
], [], true, $this->getUserMock('test', 'Test'),
],
[
'test', false, false, [], [],
[
['label' => 'Test', 'value' => ['shareType' => Share::SHARE_TYPE_USER, 'shareWith' => 'test']],
], [], true, $this->getUserMock('test', 'Test')
], [], true, $this->getUserMock('test', 'Test'),
],
[
'test', true, true, [], [],
[], [], true, $this->getUserMock('test', 'Test')
[], [], true, $this->getUserMock('test', 'Test'),
],
[
'test', true, false, [], [],
[], [], true, $this->getUserMock('test', 'Test')
[], [], true, $this->getUserMock('test', 'Test'),
],
[
'test', true, true, ['test-group'], [['test-group', 'test', 2, 0, []]],
[
['label' => 'Test', 'value' => ['shareType' => Share::SHARE_TYPE_USER, 'shareWith' => 'test']],
], [], true, $this->getUserMock('test', 'Test')
], [], true, $this->getUserMock('test', 'Test'),
],
[
'test', true, false, ['test-group'], [['test-group', 'test', 2, 0, []]],
[
['label' => 'Test', 'value' => ['shareType' => Share::SHARE_TYPE_USER, 'shareWith' => 'test']],
], [], true, $this->getUserMock('test', 'Test')
], [], true, $this->getUserMock('test', 'Test'),
],
[
'test',
@ -267,7 +277,7 @@ class UserPluginTest extends TestCase {
true,
['abc', 'xyz'],
[
['abc', 'test', 2, 0, ['test1' => 'Test One']],
['abc', 'test', 2, 0, [$this->getUserMock('test1', 'Test One')]],
['xyz', 'test', 2, 0, []],
],
[],
@ -283,7 +293,7 @@ class UserPluginTest extends TestCase {
false,
['abc', 'xyz'],
[
['abc', 'test', 2, 0, ['test1' => 'Test One']],
['abc', 'test', 2, 0, [$this->getUserMock('test1', 'Test One')]],
['xyz', 'test', 2, 0, []],
],
[],
@ -298,12 +308,12 @@ class UserPluginTest extends TestCase {
['abc', 'xyz'],
[
['abc', 'test', 2, 0, [
'test1' => 'Test One',
'test2' => 'Test Two',
$this->getUserMock('test1', 'Test One'),
$this->getUserMock('test2', 'Test Two'),
]],
['xyz', 'test', 2, 0, [
'test1' => 'Test One',
'test2' => 'Test Two',
$this->getUserMock('test1', 'Test One'),
$this->getUserMock('test2', 'Test Two'),
]],
],
[],
@ -321,12 +331,12 @@ class UserPluginTest extends TestCase {
['abc', 'xyz'],
[
['abc', 'test', 2, 0, [
'test1' => 'Test One',
'test2' => 'Test Two',
$this->getUserMock('test1', 'Test One'),
$this->getUserMock('test2', 'Test Two'),
]],
['xyz', 'test', 2, 0, [
'test1' => 'Test One',
'test2' => 'Test Two',
$this->getUserMock('test1', 'Test One'),
$this->getUserMock('test2', 'Test Two'),
]],
],
[],
@ -341,10 +351,10 @@ class UserPluginTest extends TestCase {
['abc', 'xyz'],
[
['abc', 'test', 2, 0, [
'test' => 'Test One',
$this->getUserMock('test', 'Test One'),
]],
['xyz', 'test', 2, 0, [
'test2' => 'Test Two',
$this->getUserMock('test2', 'Test Two'),
]],
],
[
@ -363,10 +373,10 @@ class UserPluginTest extends TestCase {
['abc', 'xyz'],
[
['abc', 'test', 2, 0, [
'test' => 'Test One',
$this->getUserMock('test', 'Test One'),
]],
['xyz', 'test', 2, 0, [
'test2' => 'Test Two',
$this->getUserMock('test2', 'Test Two'),
]],
],
[
@ -416,25 +426,30 @@ 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->expects($this->exactly(2))
->method('getUserGroupIds')
->withConsecutive(
[$this->user],
[$singleUser]
)
$this->groupManager->method('getUserGroups')
->with($this->user)
->willReturn($groups);
$this->groupManager->method('getUserGroupIds')
->with($singleUser)
->willReturn($groupResponse);
} else {
$this->groupManager->expects($this->once())
->method('getUserGroupIds')
->method('getUserGroups')
->with($this->user)
->willReturn($groupResponse);
->willReturn($groups);
}
$this->groupManager->expects($this->exactly(sizeof($groupResponse)))
->method('displayNamesInGroup')
->with($this->anything(), $searchTerm, $this->limit, $this->offset)
->willReturnMap($userResponse);
foreach ($userResponse as $groupDefinition) {
[$gid, $search, $limit, $offset, $users] = $groupDefinition;
$groups[$gid]->method('searchDisplayName')
->with($search, $limit, $offset)
->willReturn($users);
}
}
if ($singleUser !== false) {
@ -457,24 +472,24 @@ class UserPluginTest extends TestCase {
$inputUsers = [
'alice' => 'Alice',
'bob' => 'Bob',
'carol' => 'Carol'
'carol' => 'Carol',
];
return [
[
$inputUsers,
['alice', 'carol'],
'bob'
'bob',
],
[
$inputUsers,
['alice', 'bob', 'carol'],
'dave'
'dave',
],
[
$inputUsers,
['alice', 'bob', 'carol'],
null
]
null,
],
];
}
@ -507,54 +522,54 @@ class UserPluginTest extends TestCase {
['groupA'],
[
['uid' => 'test1', 'groups' => ['groupA']],
[ 'uid' => 'test2', 'groups' => ['groupB'] ]
['uid' => 'test2', 'groups' => ['groupB']],
],
['test1']
['test1'],
],
[
'test',
['groupA'],
[
['uid' => 'test1', 'groups' => ['groupA']],
[ 'uid' => 'test2', 'groups' => ['groupB', 'groupA'] ]
['uid' => 'test2', 'groups' => ['groupB', 'groupA']],
],
['test1', 'test2']
['test1', 'test2'],
],
[
'test',
['groupA'],
[
['uid' => 'test1', 'groups' => ['groupA', 'groupC']],
[ 'uid' => 'test2', 'groups' => ['groupB', 'groupA'] ]
['uid' => 'test2', 'groups' => ['groupB', 'groupA']],
],
['test1', 'test2']
['test1', 'test2'],
],
[
'test',
['groupC', 'groupB'],
[
['uid' => 'test1', 'groups' => ['groupA', 'groupC']],
[ 'uid' => 'test2', 'groups' => ['groupB', 'groupA'] ]
['uid' => 'test2', 'groups' => ['groupB', 'groupA']],
],
['test1', 'test2']
['test1', 'test2'],
],
[
'test',
[],
[
['uid' => 'test1', 'groups' => ['groupA']],
[ 'uid' => 'test2', 'groups' => ['groupB', 'groupA'] ]
['uid' => 'test2', 'groups' => ['groupB', 'groupA']],
],
[]
[],
],
[
'test',
['groupC', 'groupB'],
[
['uid' => 'test1', 'groups' => []],
[ 'uid' => 'test2', 'groups' => [] ]
['uid' => 'test2', 'groups' => []],
],
[]
[],
],
];
}