Merge pull request #5585 from nextcloud/contacts_menu_privacy

Enhance privacy of contactsmenu fixes #5107

1 files changed, 251 insertions, 11 deletions

diff --git a/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php b/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php
index 08da360388f..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
  *
@@ -26,31 +27,41 @@ namespace Tests\Contacts\ContactsMenu;
 
 use OC\Contacts\ContactsMenu\ContactsStore;
 use OCP\Contacts\IManager;
+use OCP\IConfig;
+use OCP\IGroupManager;
 use OCP\IUser;
+use OCP\IUserManager;
 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;
 
 	protected function setUp() {
 		parent::setUp();
 
 		$this->contactsManager = $this->createMock(IManager::class);
-
-		$this->contactsStore = new ContactsStore($this->contactsManager);
+		$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,
@@ -76,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',
@@ -102,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,
@@ -130,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,
@@ -157,7 +171,230 @@ class ContactsStoreTest extends TestCase {
 		$this->assertEquals('https://photo', $entries[1]->getAvatar());
 	}
 
+	public function testGetContactsWhenUserIsInExcludeGroups() {
+		$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(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(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')
+			->willReturn('user001');
+
+		$this->groupManager->expects($this->once())
+			->method('getUserGroupIds')
+			->with($this->equalTo($currentUser))
+			->willReturn(['group1', 'group2', 'group3']);
+
+
+		$this->contactsManager->expects($this->once())
+			->method('search')
+			->with($this->equalTo(''), $this->equalTo(['FN', 'EMAIL']))
+			->willReturn([
+				[
+					'UID' => 'user123',
+					'isLocalSystemBook' => true
+				],
+				[
+					'UID' => 'user12345',
+					'isLocalSystemBook' => true
+				],
+			]);
+
+
+		$entries = $this->contactsStore->getContacts($currentUser, '');
+
+		$this->assertCount(0, $entries);
+	}
+
+	public function testGetContactsOnlyIfInTheSameGroup() {
+		$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(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')
+			->willReturn('user001');
+
+		$this->groupManager->expects($this->at(0))
+			->method('getUserGroupIds')
+			->with($this->equalTo($currentUser))
+			->willReturn(['group1', 'group2', 'group3']);
+
+		$user1 = $this->createMock(IUser::class);
+		$this->userManager->expects($this->at(0))
+			->method('get')
+			->with('user1')
+			->willReturn($user1);
+		$this->groupManager->expects($this->at(1))
+			->method('getUserGroupIds')
+			->with($this->equalTo($user1))
+			->willReturn(['group1']);
+		$user2 = $this->createMock(IUser::class);
+		$this->userManager->expects($this->at(1))
+			->method('get')
+			->with('user2')
+			->willReturn($user2);
+		$this->groupManager->expects($this->at(2))
+			->method('getUserGroupIds')
+			->with($this->equalTo($user2))
+			->willReturn(['group2', 'group3']);
+		$user3 = $this->createMock(IUser::class);
+		$this->userManager->expects($this->at(2))
+			->method('get')
+			->with('user3')
+			->willReturn($user3);
+		$this->groupManager->expects($this->at(3))
+			->method('getUserGroupIds')
+			->with($this->equalTo($user3))
+			->willReturn(['group8', 'group9']);
+
+		$this->contactsManager->expects($this->once())
+			->method('search')
+			->with($this->equalTo(''), $this->equalTo(['FN', 'EMAIL']))
+			->willReturn([
+				[
+					'UID' => 'user1',
+					'isLocalSystemBook' => true
+				],
+				[
+					'UID' => 'user2',
+					'isLocalSystemBook' => true
+				],
+				[
+					'UID' => 'user3',
+					'isLocalSystemBook' => true
+				],
+				[
+					'UID' => 'contact',
+				],
+			]);
+
+		$entries = $this->contactsStore->getContacts($currentUser, '');
+
+		$this->assertCount(3, $entries);
+		$this->assertEquals('user1', $entries[0]->getProperty('UID'));
+		$this->assertEquals('user2', $entries[1]->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')
@@ -176,7 +413,7 @@ class ContactsStoreTest extends TestCase {
 					'isLocalSystemBook' => true
 				],
 			]);
-		$user->expects($this->once())
+		$user->expects($this->any())
 			->method('getUID')
 			->willReturn('user123');
 
@@ -188,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')
@@ -206,7 +444,7 @@ class ContactsStoreTest extends TestCase {
 					'isLocalSystemBook' => false
 				],
 			]);
-		$user->expects($this->once())
+		$user->expects($this->any())
 			->method('getUID')
 			->willReturn('user123');
 
@@ -218,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');
@@ -226,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')