Merge pull request #856 from nextcloud/fix/contacts-integration-system-usersv0.8.0-beta1

Fix contacts integration with disabled sharing enumeration option

2 files changed, 82 insertions, 10 deletions

diff --git a/lib/Service/ContactsIntegration.php b/lib/Service/ContactsIntegration.php
index 3bc8083cf..2fd1d1ac5 100644
--- a/lib/Service/ContactsIntegration.php
+++ b/lib/Service/ContactsIntegration.php
@@ -23,19 +23,22 @@
 namespace OCA\Mail\Service;
 
 use OCP\Contacts\IManager;
+use OCP\IConfig;
 
 class ContactsIntegration {
 
-	/**
-	 * @var IManager
-	 */
+	/** @var IManager */
 	private $contactsManager;
 
+	/** @var IConfig */
+	private $config;
+
 	/**
 	 * @param IManager $contactsManager
 	 */
-	public function __construct(IManager $contactsManager) {
+	public function __construct(IManager $contactsManager, IConfig $config) {
 		$this->contactsManager = $contactsManager;
+		$this->config = $config;
 	}
 
 	/**
@@ -49,9 +52,17 @@ class ContactsIntegration {
 			return [];
 		}
 
+		// If 'Allow username autocompletion in share dialog' is disabled in the admin sharing settings, then we must not
+		// auto-complete system users
+		$allowSystemUsers = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'no') === 'yes';
+
 		$result = $this->contactsManager->search($term, ['FN', 'EMAIL']);
 		$receivers = [];
 		foreach ($result as $r) {
+			if (!$allowSystemUsers && isset($r['isLocalSystemBook']) && $r['isLocalSystemBook']) {
+				continue;
+			}
+
 			$id = $r['UID'];
 			$fn = $r['FN'];
 			if (!isset($r['EMAIL'])) {
diff --git a/tests/Service/ContactsIntegrationTest.php b/tests/Service/ContactsIntegrationTest.php
index 736858491..bff62586c 100644
--- a/tests/Service/ContactsIntegrationTest.php
+++ b/tests/Service/ContactsIntegrationTest.php
@@ -23,19 +23,27 @@ namespace OCA\Mail\Tests\Service;
 
 use ChristophWurst\Nextcloud\Testing\TestCase;
 use OCA\Mail\Service\ContactsIntegration;
+use OCP\Contacts\IManager;
+use OCP\IConfig;
 
 class ContactsIntegrationTest extends TestCase {
 
+	/** @var IManager */
 	private $contactsManager;
+
+	/** @var IConfig */
+	private $config;
+
+	/** @var ContactsIntegration */
 	private $contactsIntegration;
 
 	protected function setUp() {
 		parent::setUp();
 
-		$this->contactsManager = $this->getMockBuilder('OCP\Contacts\IManager')
-			->disableOriginalConstructor()
-			->getMock();
-		$this->contactsIntegration = new ContactsIntegration($this->contactsManager);
+		$this->contactsManager = $this->createMock(IManager::class);
+		$this->config = $this->createMock(IConfig::class);
+		$this->contactsIntegration = new ContactsIntegration($this->contactsManager,
+			$this->config);
 	}
 
 	public function testDisabledContactsManager() {
@@ -75,7 +83,10 @@ class ContactsIntegrationTest extends TestCase {
 				'FN' => 'Johann Strauss II',
 			]
 		];
-
+		$this->config->expects($this->once())
+			->method('getAppValue')
+			->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'no')
+			->willReturn('yes');
 		$this->contactsManager->expects($this->once())
 			->method('isEnabled')
 			->will($this->returnValue(true));
@@ -83,7 +94,6 @@ class ContactsIntegrationTest extends TestCase {
 			->method('search')
 			->with($term, ['FN', 'EMAIL'])
 			->will($this->returnValue($searchResult));
-
 		$expected = [
 			[
 				'id' => 1,
@@ -104,6 +114,57 @@ class ContactsIntegrationTest extends TestCase {
 				'photo' => null,
 			],
 		];
+
+		$actual = $this->contactsIntegration->getMatchingRecipient($term);
+
+		$this->assertEquals($expected, $actual);
+	}
+
+	public function testGetMatchingRecipientNoSystemUsers() {
+		$term = 'jo'; // searching for: John Doe
+		$searchResult = [
+			[
+				// Simple match
+				'UID' => 1,
+				'FN' => 'Jonathan Frakes',
+				'EMAIL' => 'jonathan@frakes.com',
+			],
+			[
+				// Array of addresses
+				'UID' => 2,
+				'FN' => 'John Doe',
+				'EMAIL' => [
+					'john@doe.info',
+					'doe@john.info',
+				],
+				'isLocalSystemBook' => true,
+			],
+			[
+				// Johann Strauss II didn't have a email address ;-)
+				'UID' => 3,
+				'FN' => 'Johann Strauss II',
+			]
+		];
+		$this->config->expects($this->once())
+			->method('getAppValue')
+			->with('core', 'shareapi_allow_share_dialog_user_enumeration', 'no')
+			->willReturn('no');
+		$this->contactsManager->expects($this->once())
+			->method('isEnabled')
+			->will($this->returnValue(true));
+		$this->contactsManager->expects($this->once())
+			->method('search')
+			->with($term, ['FN', 'EMAIL'])
+			->will($this->returnValue($searchResult));
+		$expected = [
+			[
+				'id' => 1,
+				'label' => 'Jonathan Frakes (jonathan@frakes.com)',
+				'value' => '"Jonathan Frakes" <jonathan@frakes.com>',
+				'photo' => null,
+			],
+		];
+
 		$actual = $this->contactsIntegration->getMatchingRecipient($term);
 
 		$this->assertEquals($expected, $actual);