summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--lib/private/Contacts/ContactsMenu/ContactsStore.php62
-rw-r--r--settings/templates/settings/admin/sharing.php2
-rw-r--r--tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php157
3 files changed, 174 insertions, 47 deletions
diff --git a/lib/private/Contacts/ContactsMenu/ContactsStore.php b/lib/private/Contacts/ContactsMenu/ContactsStore.php
index 87aff258aae..3eda58cacfb 100644
--- a/lib/private/Contacts/ContactsMenu/ContactsStore.php
+++ b/lib/private/Contacts/ContactsMenu/ContactsStore.php
@@ -1,9 +1,10 @@
<?php
-
/**
* @copyright 2017 Christoph Wurst <christoph@winzerhof-wurst.at>
+ * @copyright 2017 Lukas Reschke <lukas@statuscode.ch>
*
* @author 2017 Christoph Wurst <christoph@winzerhof-wurst.at>
+ * @author 2017 Lukas Reschke <lukas@statuscode.ch>
*
* @license GNU AGPL version 3 or any later version
*
@@ -53,7 +54,10 @@ class ContactsStore {
* @param IUserManager $userManager
* @param IGroupManager $groupManager
*/
- public function __construct(IManager $contactsManager, IConfig $config, IUserManager $userManager, IGroupManager $groupManager) {
+ public function __construct(IManager $contactsManager,
+ IConfig $config,
+ IUserManager $userManager,
+ IGroupManager $groupManager) {
$this->contactsManager = $contactsManager;
$this->config = $config;
$this->userManager = $userManager;
@@ -68,27 +72,39 @@ class ContactsStore {
public function getContacts(IUser $user, $filter) {
$allContacts = $this->contactsManager->search($filter ?: '', [
'FN',
+ 'EMAIL'
]);
$entries = array_map(function(array $contact) {
return $this->contactArrayToEntry($contact);
}, $allContacts);
- return $this->filterContacts($user, $entries);
+ return $this->filterContacts(
+ $user,
+ $entries,
+ $filter
+ );
}
/**
- * @brief filters the contacts. Applies 3 filters:
+ * Filters the contacts. Applies 3 filters:
* 1. filter the current user
- * 2. if the `shareapi_exclude_groups` config option is enabled and the
+ * 2. if the `shareapi_allow_share_dialog_user_enumeration` config option is
+ * enabled it will filter all local users
+ * 3. if the `shareapi_exclude_groups` config option is enabled and the
* current user is in an excluded group it will filter all local users.
- * 3. if the `shareapi_only_share_with_group_members` config option is
+ * 4. if the `shareapi_only_share_with_group_members` config option is
* enabled it will filter all users which doens't have a common group
* with the current user.
+ *
* @param IUser $self
* @param Entry[] $entries
+ * @param string $filter
* @return Entry[] the filtered contacts
*/
- private function filterContacts(IUser $self, array $entries) {
+ private function filterContacts(IUser $self,
+ array $entries,
+ $filter) {
+ $disallowEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') !== 'yes';
$excludedGroups = $this->config->getAppValue('core', 'shareapi_exclude_groups', 'no') === 'yes';
// whether to filter out local users
@@ -101,7 +117,7 @@ class ContactsStore {
if ($excludedGroups) {
$excludedGroups = $this->config->getAppValue('core', 'shareapi_exclude_groups_list', '');
$decodedExcludeGroups = json_decode($excludedGroups, true);
- $excludeGroupsList = !is_null($decodedExcludeGroups) ? $decodedExcludeGroups : [];
+ $excludeGroupsList = ($decodedExcludeGroups !== null) ? $decodedExcludeGroups : [];
if (count(array_intersect($excludeGroupsList, $selfGroups)) !== 0) {
// a group of the current user is excluded -> filter all local users
@@ -111,12 +127,32 @@ class ContactsStore {
$selfUID = $self->getUID();
- return array_filter($entries, function(IEntry $entry) use ($self, $skipLocal, $ownGroupsOnly, $selfGroups, $selfUID) {
-
+ return array_values(array_filter($entries, function(IEntry $entry) use ($self, $skipLocal, $ownGroupsOnly, $selfGroups, $selfUID, $disallowEnumeration, $filter) {
if ($skipLocal && $entry->getProperty('isLocalSystemBook') === true) {
return false;
}
+ // Prevent enumerating local users
+ if($disallowEnumeration && $entry->getProperty('isLocalSystemBook')) {
+ $filterUser = true;
+
+ $mailAddresses = $entry->getEMailAddresses();
+ foreach($mailAddresses as $mailAddress) {
+ if($mailAddress === $filter) {
+ $filterUser = false;
+ break;
+ }
+ }
+
+ if($entry->getProperty('UID') && $entry->getProperty('UID') === $filter) {
+ $filterUser = false;
+ }
+
+ if($filterUser) {
+ return false;
+ }
+ }
+
if ($ownGroupsOnly && $entry->getProperty('isLocalSystemBook') === true) {
$contactGroups = $this->groupManager->getUserGroupIds($this->userManager->get($entry->getProperty('UID')));
if (count(array_intersect($contactGroups, $selfGroups)) === 0) {
@@ -126,9 +162,7 @@ class ContactsStore {
}
return $entry->getProperty('UID') !== $selfUID;
- });
-
-
+ }));
}
/**
@@ -173,7 +207,7 @@ class ContactsStore {
}
if ($match) {
- $match = $this->filterContacts($user, [$this->contactArrayToEntry($match)]);
+ $match = $this->filterContacts($user, [$this->contactArrayToEntry($match)], $shareWith);
if (count($match) === 1) {
$match = $match[0];
} else {
diff --git a/settings/templates/settings/admin/sharing.php b/settings/templates/settings/admin/sharing.php
index 38071a4bee9..9c9e8c07809 100644
--- a/settings/templates/settings/admin/sharing.php
+++ b/settings/templates/settings/admin/sharing.php
@@ -96,7 +96,7 @@
<p class="<?php if ($_['shareAPIEnabled'] === 'no') p('hidden');?>">
<input type="checkbox" name="shareapi_allow_share_dialog_user_enumeration" value="1" id="shareapi_allow_share_dialog_user_enumeration" class="checkbox"
<?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 needs to be entered.'));?></label><br />
+ <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 />
</p>
<p>
<input type="checkbox" id="publicShareDisclaimer" class="checkbox noJSAutoUpdate"
diff --git a/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php b/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php
index 360f117ed7a..cf801396cb1 100644
--- a/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php
+++ b/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php
@@ -1,9 +1,10 @@
<?php
-
/**
* @copyright 2017 Christoph Wurst <christoph@winzerhof-wurst.at>
+ * @copyright 2017 Lukas Reschke <lukas@statuscode.ch>
*
* @author 2017 Christoph Wurst <christoph@winzerhof-wurst.at>
+ * @author 2017 Lukas Reschke <lukas@statuscode.ch>
*
* @license GNU AGPL version 3 or any later version
*
@@ -34,19 +35,14 @@ use PHPUnit_Framework_MockObject_MockObject;
use Test\TestCase;
class ContactsStoreTest extends TestCase {
-
/** @var ContactsStore */
private $contactsStore;
-
/** @var IManager|PHPUnit_Framework_MockObject_MockObject */
private $contactsManager;
-
/** @var IUserManager|PHPUnit_Framework_MockObject_MockObject */
private $userManager;
-
/** @var IGroupManager|PHPUnit_Framework_MockObject_MockObject */
private $groupManager;
-
/** @var IConfig|PHPUnit_Framework_MockObject_MockObject */
private $config;
@@ -54,21 +50,18 @@ class ContactsStoreTest extends TestCase {
parent::setUp();
$this->contactsManager = $this->createMock(IManager::class);
-
$this->userManager = $this->createMock(IUserManager::class);
-
$this->groupManager = $this->createMock(IGroupManager::class);
-
$this->config = $this->createMock(IConfig::class);
-
$this->contactsStore = new ContactsStore($this->contactsManager, $this->config, $this->userManager, $this->groupManager);
}
public function testGetContactsWithoutFilter() {
+ /** @var IUser|PHPUnit_Framework_MockObject_MockObject $user */
$user = $this->createMock(IUser::class);
$this->contactsManager->expects($this->once())
->method('search')
- ->with($this->equalTo(''), $this->equalTo(['FN']))
+ ->with($this->equalTo(''), $this->equalTo(['FN', 'EMAIL']))
->willReturn([
[
'UID' => 123,
@@ -94,10 +87,11 @@ class ContactsStoreTest extends TestCase {
}
public function testGetContactsHidesOwnEntry() {
+ /** @var IUser|PHPUnit_Framework_MockObject_MockObject $user */
$user = $this->createMock(IUser::class);
$this->contactsManager->expects($this->once())
->method('search')
- ->with($this->equalTo(''), $this->equalTo(['FN']))
+ ->with($this->equalTo(''), $this->equalTo(['FN', 'EMAIL']))
->willReturn([
[
'UID' => 'user123',
@@ -120,10 +114,11 @@ class ContactsStoreTest extends TestCase {
}
public function testGetContactsWithoutBinaryImage() {
+ /** @var IUser|PHPUnit_Framework_MockObject_MockObject $user */
$user = $this->createMock(IUser::class);
$this->contactsManager->expects($this->once())
->method('search')
- ->with($this->equalTo(''), $this->equalTo(['FN']))
+ ->with($this->equalTo(''), $this->equalTo(['FN', 'EMAIL']))
->willReturn([
[
'UID' => 123,
@@ -148,10 +143,11 @@ class ContactsStoreTest extends TestCase {
}
public function testGetContactsWithoutAvatarURI() {
+ /** @var IUser|PHPUnit_Framework_MockObject_MockObject $user */
$user = $this->createMock(IUser::class);
$this->contactsManager->expects($this->once())
->method('search')
- ->with($this->equalTo(''), $this->equalTo(['FN']))
+ ->with($this->equalTo(''), $this->equalTo(['FN', 'EMAIL']))
->willReturn([
[
'UID' => 123,
@@ -176,21 +172,26 @@ class ContactsStoreTest extends TestCase {
}
public function testGetContactsWhenUserIsInExcludeGroups() {
- $this->config->expects($this->at(0))
+ $this->config->expects($this->at(0))->method('getAppValue')
+ ->with($this->equalTo('core'), $this->equalTo('shareapi_allow_share_dialog_user_enumeration'), $this->equalTo('yes'))
+ ->willReturn('yes');
+
+ $this->config->expects($this->at(1))
->method('getAppValue')
->with($this->equalTo('core'), $this->equalTo('shareapi_exclude_groups'), $this->equalTo('no'))
->willReturn('yes');
- $this->config->expects($this->at(1))
+ $this->config->expects($this->at(2))
->method('getAppValue')
->with($this->equalTo('core'), $this->equalTo('shareapi_only_share_with_group_members'), $this->equalTo('no'))
->willReturn('yes');
- $this->config->expects($this->at(2))
+ $this->config->expects($this->at(3))
->method('getAppValue')
->with($this->equalTo('core'), $this->equalTo('shareapi_exclude_groups_list'), $this->equalTo(''))
->willReturn('["group1", "group5", "group6"]');
+ /** @var IUser|PHPUnit_Framework_MockObject_MockObject $currentUser */
$currentUser = $this->createMock(IUser::class);
$currentUser->expects($this->once())
->method('getUID')
@@ -199,12 +200,12 @@ class ContactsStoreTest extends TestCase {
$this->groupManager->expects($this->once())
->method('getUserGroupIds')
->with($this->equalTo($currentUser))
- ->willReturn(["group1", "group2", "group3"]);
+ ->willReturn(['group1', 'group2', 'group3']);
$this->contactsManager->expects($this->once())
->method('search')
- ->with($this->equalTo(''), $this->equalTo(['FN']))
+ ->with($this->equalTo(''), $this->equalTo(['FN', 'EMAIL']))
->willReturn([
[
'UID' => 'user123',
@@ -220,19 +221,23 @@ class ContactsStoreTest extends TestCase {
$entries = $this->contactsStore->getContacts($currentUser, '');
$this->assertCount(0, $entries);
-
}
public function testGetContactsOnlyIfInTheSameGroup() {
- $this->config->expects($this->at(0)) ->method('getAppValue')
+ $this->config->expects($this->at(0))->method('getAppValue')
+ ->with($this->equalTo('core'), $this->equalTo('shareapi_allow_share_dialog_user_enumeration'), $this->equalTo('yes'))
+ ->willReturn('yes');
+
+ $this->config->expects($this->at(1)) ->method('getAppValue')
->with($this->equalTo('core'), $this->equalTo('shareapi_exclude_groups'), $this->equalTo('no'))
->willReturn('no');
- $this->config->expects($this->at(1))
+ $this->config->expects($this->at(2))
->method('getAppValue')
->with($this->equalTo('core'), $this->equalTo('shareapi_only_share_with_group_members'), $this->equalTo('no'))
->willReturn('yes');
+ /** @var IUser|PHPUnit_Framework_MockObject_MockObject $currentUser */
$currentUser = $this->createMock(IUser::class);
$currentUser->expects($this->once())
->method('getUID')
@@ -241,8 +246,7 @@ class ContactsStoreTest extends TestCase {
$this->groupManager->expects($this->at(0))
->method('getUserGroupIds')
->with($this->equalTo($currentUser))
- ->willReturn(["group1", "group2", "group3"]);
-
+ ->willReturn(['group1', 'group2', 'group3']);
$user1 = $this->createMock(IUser::class);
$this->userManager->expects($this->at(0))
@@ -252,7 +256,7 @@ class ContactsStoreTest extends TestCase {
$this->groupManager->expects($this->at(1))
->method('getUserGroupIds')
->with($this->equalTo($user1))
- ->willReturn(["group1"]);
+ ->willReturn(['group1']);
$user2 = $this->createMock(IUser::class);
$this->userManager->expects($this->at(1))
->method('get')
@@ -261,7 +265,7 @@ class ContactsStoreTest extends TestCase {
$this->groupManager->expects($this->at(2))
->method('getUserGroupIds')
->with($this->equalTo($user2))
- ->willReturn(["group2", "group3"]);
+ ->willReturn(['group2', 'group3']);
$user3 = $this->createMock(IUser::class);
$this->userManager->expects($this->at(2))
->method('get')
@@ -270,11 +274,11 @@ class ContactsStoreTest extends TestCase {
$this->groupManager->expects($this->at(3))
->method('getUserGroupIds')
->with($this->equalTo($user3))
- ->willReturn(["group8", "group9"]);
+ ->willReturn(['group8', 'group9']);
$this->contactsManager->expects($this->once())
->method('search')
- ->with($this->equalTo(''), $this->equalTo(['FN']))
+ ->with($this->equalTo(''), $this->equalTo(['FN', 'EMAIL']))
->willReturn([
[
'UID' => 'user1',
@@ -298,13 +302,99 @@ class ContactsStoreTest extends TestCase {
$this->assertCount(3, $entries);
$this->assertEquals('user1', $entries[0]->getProperty('UID'));
$this->assertEquals('user2', $entries[1]->getProperty('UID'));
- $this->assertEquals('contact', $entries[3]->getProperty('UID'));
+ $this->assertEquals('contact', $entries[2]->getProperty('UID'));
+ }
+ public function testGetContactsWithFilter() {
+ $this->config->expects($this->at(0))->method('getAppValue')
+ ->with($this->equalTo('core'), $this->equalTo('shareapi_allow_share_dialog_user_enumeration'), $this->equalTo('yes'))
+ ->willReturn('no');
- }
+ /** @var IUser|PHPUnit_Framework_MockObject_MockObject $user */
+ $user = $this->createMock(IUser::class);
+ $this->contactsManager->expects($this->any())
+ ->method('search')
+ ->willReturn([
+ [
+ 'UID' => 'a567',
+ 'FN' => 'Darren Roner',
+ 'EMAIL' => [
+ 'darren@roner.au',
+ ],
+ 'isLocalSystemBook' => true,
+ ],
+ [
+ 'UID' => 'john',
+ 'FN' => 'John Doe',
+ 'EMAIL' => [
+ 'john@example.com',
+ ],
+ 'isLocalSystemBook' => true,
+ ],
+ [
+ 'FN' => 'Anne D',
+ 'EMAIL' => [
+ 'anne@example.com',
+ ],
+ 'isLocalSystemBook' => false,
+ ],
+ ]);
+ $user->expects($this->any())
+ ->method('getUID')
+ ->willReturn('user123');
+ // Complete match on UID should match
+ $entry = $this->contactsStore->getContacts($user, 'a567');
+ $this->assertSame(2, count($entry));
+ $this->assertEquals([
+ 'darren@roner.au'
+ ], $entry[0]->getEMailAddresses());
+
+ // Partial match on UID should not match
+ $entry = $this->contactsStore->getContacts($user, 'a56');
+ $this->assertSame(1, count($entry));
+ $this->assertEquals([
+ 'anne@example.com'
+ ], $entry[0]->getEMailAddresses());
+
+ // Complete match on email should match
+ $entry = $this->contactsStore->getContacts($user, 'john@example.com');
+ $this->assertSame(2, count($entry));
+ $this->assertEquals([
+ 'john@example.com'
+ ], $entry[0]->getEMailAddresses());
+ $this->assertEquals([
+ 'anne@example.com'
+ ], $entry[1]->getEMailAddresses());
+
+ // Partial match on email should not match
+ $entry = $this->contactsStore->getContacts($user, 'john@example.co');
+ $this->assertSame(1, count($entry));
+ $this->assertEquals([
+ 'anne@example.com'
+ ], $entry[0]->getEMailAddresses());
+
+ // Match on FN should not match
+ $entry = $this->contactsStore->getContacts($user, 'Darren Roner');
+ $this->assertSame(1, count($entry));
+ $this->assertEquals([
+ 'anne@example.com'
+ ], $entry[0]->getEMailAddresses());
+
+ // Don't filter users in local addressbook
+ $entry = $this->contactsStore->getContacts($user, 'Anne D');
+ $this->assertSame(1, count($entry));
+ $this->assertEquals([
+ 'anne@example.com'
+ ], $entry[0]->getEMailAddresses());
+ }
public function testFindOneUser() {
+ $this->config->expects($this->at(0))->method('getAppValue')
+ ->with($this->equalTo('core'), $this->equalTo('shareapi_allow_share_dialog_user_enumeration'), $this->equalTo('yes'))
+ ->willReturn('yes');
+
+ /** @var IUser|PHPUnit_Framework_MockObject_MockObject $user */
$user = $this->createMock(IUser::class);
$this->contactsManager->expects($this->once())
->method('search')
@@ -323,7 +413,7 @@ class ContactsStoreTest extends TestCase {
'isLocalSystemBook' => true
],
]);
- $user->expects($this->once())
+ $user->expects($this->any())
->method('getUID')
->willReturn('user123');
@@ -335,6 +425,7 @@ class ContactsStoreTest extends TestCase {
}
public function testFindOneEMail() {
+ /** @var IUser|PHPUnit_Framework_MockObject_MockObject $user */
$user = $this->createMock(IUser::class);
$this->contactsManager->expects($this->once())
->method('search')
@@ -353,7 +444,7 @@ class ContactsStoreTest extends TestCase {
'isLocalSystemBook' => false
],
]);
- $user->expects($this->once())
+ $user->expects($this->any())
->method('getUID')
->willReturn('user123');
@@ -365,6 +456,7 @@ class ContactsStoreTest extends TestCase {
}
public function testFindOneNotSupportedType() {
+ /** @var IUser|PHPUnit_Framework_MockObject_MockObject $user */
$user = $this->createMock(IUser::class);
$entry = $this->contactsStore->findOne($user, 42, 'darren@roner.au');
@@ -373,6 +465,7 @@ class ContactsStoreTest extends TestCase {
}
public function testFindOneNoMatches() {
+ /** @var IUser|PHPUnit_Framework_MockObject_MockObject $user */
$user = $this->createMock(IUser::class);
$this->contactsManager->expects($this->once())
->method('search')
generated by cgit v1.2.3 (git 2.39.1) at 2025-02-25 09:36:09 +0000