From bf2873e4efa1ee5477c32b5388f7683a571ec7a2 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Wed, 23 Jan 2019 14:57:54 +0100 Subject: [PATCH] Allow to disable system addressbook sync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On some instances, users don't know each other and it doesn't makes sense to expose users as contacts (through Contacts Menu, Calendar Attendees,…) to everyone. Signed-off-by: Thomas Citharel Fix and add tests Signed-off-by: Thomas Citharel Change settings template according to #13796 Signed-off-by: Thomas Citharel Empty system addressbook instead of removing it This makes Federated Cloud Sharing not fail when synching trusted servers addressbooks (only synching an empty one, not bumping on an non-existent one). Signed-off-by: Thomas Citharel Fix tests Signed-off-by: Thomas Citharel --- apps/dav/appinfo/info.xml | 1 + .../composer/composer/autoload_classmap.php | 1 + .../dav/composer/composer/autoload_static.php | 1 + apps/dav/js/settings-admin-carddav.js | 28 ++++++++ apps/dav/lib/CardDAV/CardDavBackend.php | 23 ++++++ apps/dav/lib/CardDAV/ContactsManager.php | 12 +++- apps/dav/lib/CardDAV/SyncService.php | 10 ++- .../dav/lib/Command/SyncSystemAddressBook.php | 29 +++++--- apps/dav/lib/Settings/CardDAVSettings.php | 68 ++++++++++++++++++ apps/dav/templates/settings-admin-carddav.php | 55 ++++++++++++++ .../unit/CardDAV/ContactsManagerTest.php | 22 +++++- .../tests/unit/CardDAV/SyncServiceTest.php | 13 ++++ .../Command/SyncSystemAddressBookTest.php | 71 +++++++++++++++++++ .../unit/Settings/CardDAVSettingsTest.php | 58 +++++++++++++++ 14 files changed, 377 insertions(+), 15 deletions(-) create mode 100644 apps/dav/js/settings-admin-carddav.js create mode 100644 apps/dav/lib/Settings/CardDAVSettings.php create mode 100644 apps/dav/templates/settings-admin-carddav.php create mode 100644 apps/dav/tests/unit/Command/SyncSystemAddressBookTest.php create mode 100644 apps/dav/tests/unit/Settings/CardDAVSettingsTest.php diff --git a/apps/dav/appinfo/info.xml b/apps/dav/appinfo/info.xml index 6d801098c400d..6675dff35ec66 100644 --- a/apps/dav/appinfo/info.xml +++ b/apps/dav/appinfo/info.xml @@ -52,6 +52,7 @@ OCA\DAV\Settings\CalDAVSettings + OCA\DAV\Settings\CardDAVSettings diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index 61fd5c25917b1..ae7cace97683b 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -182,6 +182,7 @@ 'OCA\\DAV\\RootCollection' => $baseDir . '/../lib/RootCollection.php', 'OCA\\DAV\\Server' => $baseDir . '/../lib/Server.php', 'OCA\\DAV\\Settings\\CalDAVSettings' => $baseDir . '/../lib/Settings/CalDAVSettings.php', + 'OCA\\DAV\\Settings\\CardDAVSettings' => $baseDir . '/../lib/Settings/CardDAVSettings.php', 'OCA\\DAV\\SystemTag\\SystemTagMappingNode' => $baseDir . '/../lib/SystemTag/SystemTagMappingNode.php', 'OCA\\DAV\\SystemTag\\SystemTagNode' => $baseDir . '/../lib/SystemTag/SystemTagNode.php', 'OCA\\DAV\\SystemTag\\SystemTagPlugin' => $baseDir . '/../lib/SystemTag/SystemTagPlugin.php', diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php index 4898c78021cae..5d4eecf2070b3 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -197,6 +197,7 @@ class ComposerStaticInitDAV 'OCA\\DAV\\RootCollection' => __DIR__ . '/..' . '/../lib/RootCollection.php', 'OCA\\DAV\\Server' => __DIR__ . '/..' . '/../lib/Server.php', 'OCA\\DAV\\Settings\\CalDAVSettings' => __DIR__ . '/..' . '/../lib/Settings/CalDAVSettings.php', + 'OCA\\DAV\\Settings\\CardDAVSettings' => __DIR__ . '/..' . '/../lib/Settings/CardDAVSettings.php', 'OCA\\DAV\\SystemTag\\SystemTagMappingNode' => __DIR__ . '/..' . '/../lib/SystemTag/SystemTagMappingNode.php', 'OCA\\DAV\\SystemTag\\SystemTagNode' => __DIR__ . '/..' . '/../lib/SystemTag/SystemTagNode.php', 'OCA\\DAV\\SystemTag\\SystemTagPlugin' => __DIR__ . '/..' . '/../lib/SystemTag/SystemTagPlugin.php', diff --git a/apps/dav/js/settings-admin-carddav.js b/apps/dav/js/settings-admin-carddav.js new file mode 100644 index 0000000000000..9add6234b78ca --- /dev/null +++ b/apps/dav/js/settings-admin-carddav.js @@ -0,0 +1,28 @@ +/** + * @copyright 2019, Thomas Citharel + * + * @author Thomas Citharel + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ +'use strict'; + +$('#carddavSyncSystemAddressbook').change(function () { + var val = $(this)[0].checked; + + OCP.AppConfig.setValue('dav', 'syncSystemAddressbook', val ? 'yes' : 'no'); +}); diff --git a/apps/dav/lib/CardDAV/CardDavBackend.php b/apps/dav/lib/CardDAV/CardDavBackend.php index 699248f7b17c9..5106bd89e334e 100644 --- a/apps/dav/lib/CardDAV/CardDavBackend.php +++ b/apps/dav/lib/CardDAV/CardDavBackend.php @@ -470,7 +470,30 @@ function deleteAddressBook($addressBookId) { $query->delete($this->dbCardsPropertiesTable) ->where($query->expr()->eq('addressbookid', $query->createNamedParameter($addressBookId))) ->execute(); + } + + /** + * Delete all the content from an addressbook + * + * @param mixed $addressBookId + * @return void + */ + public function emptyAddressBook($addressBookId): void + { + $query = $this->db->getQueryBuilder(); + $query->delete('cards') + ->where($query->expr()->eq('addressbookid', $query->createParameter('addressbookid'))) + ->setParameter('addressbookid', $addressBookId) + ->execute(); + + $query->delete('addressbookchanges') + ->where($query->expr()->eq('addressbookid', $query->createParameter('addressbookid'))) + ->setParameter('addressbookid', $addressBookId) + ->execute(); + $query->delete($this->dbCardsPropertiesTable) + ->where($query->expr()->eq('addressbookid', $query->createNamedParameter($addressBookId))) + ->execute(); } /** diff --git a/apps/dav/lib/CardDAV/ContactsManager.php b/apps/dav/lib/CardDAV/ContactsManager.php index 67e3ef2c72c38..fede2f2a89dc5 100644 --- a/apps/dav/lib/CardDAV/ContactsManager.php +++ b/apps/dav/lib/CardDAV/ContactsManager.php @@ -26,6 +26,7 @@ namespace OCA\DAV\CardDAV; use OCP\Contacts\IManager; +use OCP\IConfig; use OCP\IL10N; use OCP\IURLGenerator; @@ -36,15 +37,20 @@ class ContactsManager { /** @var IL10N */ private $l10n; + /** @var IConfig */ + private $config; + /** * ContactsManager constructor. * * @param CardDavBackend $backend * @param IL10N $l10n + * @param IConfig $config */ - public function __construct(CardDavBackend $backend, IL10N $l10n) { + public function __construct(CardDavBackend $backend, IL10N $l10n, IConfig $config) { $this->backend = $backend; $this->l10n = $l10n; + $this->config = $config; } /** @@ -55,7 +61,9 @@ public function __construct(CardDavBackend $backend, IL10N $l10n) { public function setupContactsProvider(IManager $cm, $userId, IURLGenerator $urlGenerator) { $addressBooks = $this->backend->getAddressBooksForUser("principals/users/$userId"); $this->register($cm, $addressBooks, $urlGenerator); - $this->setupSystemContactsProvider($cm, $urlGenerator); + if ($this->config->getAppValue('dav', 'syncSystemAddressbook', 'yes') === 'yes') { + $this->setupSystemContactsProvider($cm, $urlGenerator); + } } /** diff --git a/apps/dav/lib/CardDAV/SyncService.php b/apps/dav/lib/CardDAV/SyncService.php index 6f6fa0ba379e3..fc45c2b8bde84 100644 --- a/apps/dav/lib/CardDAV/SyncService.php +++ b/apps/dav/lib/CardDAV/SyncService.php @@ -338,5 +338,13 @@ public function syncInstance(\Closure $progressCallback = null) { } } - + /** + * Delete the content of the system addressbook and the addressbook itself since it will be regenerated anyway + * @return void + */ + public function purgeSystemAddressBook(): void + { + $systemAddressBook = $this->getLocalSystemAddressBook(); + $this->backend->emptyAddressBook($systemAddressBook['id']); + } } diff --git a/apps/dav/lib/Command/SyncSystemAddressBook.php b/apps/dav/lib/Command/SyncSystemAddressBook.php index e91ab38593dd9..0eea9b284e485 100644 --- a/apps/dav/lib/Command/SyncSystemAddressBook.php +++ b/apps/dav/lib/Command/SyncSystemAddressBook.php @@ -22,6 +22,7 @@ namespace OCA\DAV\Command; use OCA\DAV\CardDAV\SyncService; +use OCP\IConfig; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\ProgressBar; use Symfony\Component\Console\Input\InputInterface; @@ -32,12 +33,17 @@ class SyncSystemAddressBook extends Command { /** @var SyncService */ private $syncService; + /** @var IConfig */ + private $config; + /** * @param SyncService $syncService + * @param IConfig $config */ - function __construct(SyncService $syncService) { + function __construct(SyncService $syncService, IConfig $config) { parent::__construct(); $this->syncService = $syncService; + $this->config = $config; } protected function configure() { @@ -51,14 +57,19 @@ protected function configure() { * @param OutputInterface $output */ protected function execute(InputInterface $input, OutputInterface $output) { - $output->writeln('Syncing users ...'); - $progress = new ProgressBar($output); - $progress->start(); - $this->syncService->syncInstance(function() use ($progress) { - $progress->advance(); - }); + if ($this->config->getAppValue('dav', 'syncSystemAddressbook', 'yes') === 'yes') { + $output->writeln('Syncing users ...'); + $progress = new ProgressBar($output); + $progress->start(); + $this->syncService->syncInstance(function() use ($progress) { + $progress->advance(); + }); - $progress->finish(); - $output->writeln(''); + $progress->finish(); + $output->writeln(''); + } else { + $this->syncService->purgeSystemAddressBook(); + $output->writeln('Syncing system addressbook is disabled. Addressbook has been removed'); + } } } diff --git a/apps/dav/lib/Settings/CardDAVSettings.php b/apps/dav/lib/Settings/CardDAVSettings.php new file mode 100644 index 0000000000000..df5cdbcd3ba90 --- /dev/null +++ b/apps/dav/lib/Settings/CardDAVSettings.php @@ -0,0 +1,68 @@ + + * + * @author Thomas Citharel + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\DAV\Settings; + +use OCP\AppFramework\Http\TemplateResponse; +use OCP\IConfig; +use OCP\Settings\ISettings; + +class CardDAVSettings implements ISettings { + + /** @var IConfig */ + private $config; + + /** + * CardDAVSettings constructor. + * + * @param IConfig $config + */ + public function __construct(IConfig $config) { + $this->config = $config; + } + + /** + * @return TemplateResponse + */ + public function getForm() { + $parameters = [ + 'sync_system_addressbook' => $this->config->getAppValue('dav', 'syncSystemAddressbook', 'yes'), + ]; + + return new TemplateResponse('dav', 'settings-admin-carddav', $parameters); + } + + /** + * @return string + */ + public function getSection() { + return 'groupware'; + } + + /** + * @return int + */ + public function getPriority() { + return 10; + } +} diff --git a/apps/dav/templates/settings-admin-carddav.php b/apps/dav/templates/settings-admin-carddav.php new file mode 100644 index 0000000000000..8638a0cbefda3 --- /dev/null +++ b/apps/dav/templates/settings-admin-carddav.php @@ -0,0 +1,55 @@ + + * + * @author Thomas Citharel + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +script('dav', [ + 'settings-admin-carddav' +]); + +/** @var \OCP\IL10N $l */ +/** @var array $_ */ +?> +
+

t('Contacts server')); ?>

+

+ ', + '', + '', + ], + $l->t('Also install the {contactsappstoreopen}Contacts app{linkclose}, or {contactsdocopen}connect your desktop & mobile for syncing ↗{linkclose}.') + )); ?> +

+

+ /> + +
+ t('Syncs the instance users as contacts in a global system addressbook')); ?> +

+
diff --git a/apps/dav/tests/unit/CardDAV/ContactsManagerTest.php b/apps/dav/tests/unit/CardDAV/ContactsManagerTest.php index 768b867234c7a..ddef430c248d2 100644 --- a/apps/dav/tests/unit/CardDAV/ContactsManagerTest.php +++ b/apps/dav/tests/unit/CardDAV/ContactsManagerTest.php @@ -28,15 +28,27 @@ use OCA\DAV\CardDAV\CardDavBackend; use OCA\DAV\CardDAV\ContactsManager; use OCP\Contacts\IManager; +use OCP\IConfig; use OCP\IL10N; use OCP\IURLGenerator; use Test\TestCase; class ContactsManagerTest extends TestCase { - public function test() { + + public function dataForContactsManagerTest() { + return [ + ['yes'], + ['no'] + ]; + } + + /** + * @dataProvider dataForContactsManagerTest + */ + public function test(string $syncSystemAddressBook) { /** @var IManager | \PHPUnit_Framework_MockObject_MockObject $cm */ $cm = $this->getMockBuilder(IManager::class)->disableOriginalConstructor()->getMock(); - $cm->expects($this->exactly(2))->method('registerAddressBook'); + $cm->expects($this->exactly($syncSystemAddressBook === 'yes' ? 2 : 1))->method('registerAddressBook'); $urlGenerator = $this->getMockBuilder(IURLGenerator::class)->disableOriginalConstructor()->getMock(); /** @var CardDavBackend | \PHPUnit_Framework_MockObject_MockObject $backEnd */ $backEnd = $this->getMockBuilder(CardDavBackend::class)->disableOriginalConstructor()->getMock(); @@ -45,7 +57,11 @@ public function test() { ]); $l = $this->createMock(IL10N::class); - $app = new ContactsManager($backEnd, $l); + $config = $this->createMock(IConfig::class); + + $config->method('getAppValue')->with('dav', 'syncSystemAddressbook', 'yes')->willReturn($syncSystemAddressBook); + + $app = new ContactsManager($backEnd, $l, $config); $app->setupContactsProvider($cm, 'user01', $urlGenerator); } } diff --git a/apps/dav/tests/unit/CardDAV/SyncServiceTest.php b/apps/dav/tests/unit/CardDAV/SyncServiceTest.php index 7d444571fac8c..2e7d310573e4c 100644 --- a/apps/dav/tests/unit/CardDAV/SyncServiceTest.php +++ b/apps/dav/tests/unit/CardDAV/SyncServiceTest.php @@ -171,6 +171,19 @@ public function testUpdateAndDeleteUser($activated, $createCalls, $updateCalls, $ss->deleteUser($user); } + public function testPurgeSystemAddressBook() + { + $backend = $this->getMockBuilder(CardDavBackend::class)->disableOriginalConstructor()->getMock(); + $logger = $this->getMockBuilder(ILogger::class)->disableOriginalConstructor()->getMock(); + $userManager = $this->getMockBuilder(IUserManager::class)->disableOriginalConstructor()->getMock(); + $accountManager = $this->getMockBuilder(AccountManager::class)->disableOriginalConstructor()->getMock(); + + $backend->expects($this->once())->method('emptyAddressBook'); + + $ss = new SyncService($backend, $userManager, $logger, $accountManager); + $ss->purgeSystemAddressBook(); + } + /** * @param int $createCount * @param int $updateCount diff --git a/apps/dav/tests/unit/Command/SyncSystemAddressBookTest.php b/apps/dav/tests/unit/Command/SyncSystemAddressBookTest.php new file mode 100644 index 0000000000000..7ca71c7623690 --- /dev/null +++ b/apps/dav/tests/unit/Command/SyncSystemAddressBookTest.php @@ -0,0 +1,71 @@ + + * + * @author Thomas Citharel + * + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ +namespace OCA\DAV\Tests\Command; + +use OCA\DAV\Command\SyncSystemAddressBook; +use OCA\DAV\CardDAV\SyncService; +use OCP\IConfig; +use InvalidArgumentException; +use Symfony\Component\Console\Tester\CommandTester; +use Test\TestCase; +/** + * Class SyncSystemAddressBookTest + * + * @package OCA\DAV\Tests\Command + */ +class SyncSystemAddressBookTest extends TestCase { + + /** @var SyncService */ + private $syncService; + + /** @var IConfig */ + private $config; + + /** @var SyncSystemAddressBook */ + private $command; + + protected function setUp() { + parent::setUp(); + $this->syncService = $this->createMock(SyncService::class); + $this->config = $this->createMock(IConfig::class); + $this->command = new SyncSystemAddressBook( + $this->syncService, + $this->config + ); + } + + public function testSyncEnabled() + { + $this->config->method('getAppValue')->with('dav', 'syncSystemAddressbook', 'yes')->willReturn('yes'); + $this->syncService->expects($this->once())->method('syncInstance'); + $commandTester = new CommandTester($this->command); + $commandTester->execute([]); + } + + public function testSyncDisabled() + { + $this->config->method('getAppValue')->with('dav', 'syncSystemAddressbook', 'yes')->willReturn('no'); + $this->syncService->expects($this->once())->method('purgeSystemAddressBook'); + $commandTester = new CommandTester($this->command); + $commandTester->execute([]); + } +} \ No newline at end of file diff --git a/apps/dav/tests/unit/Settings/CardDAVSettingsTest.php b/apps/dav/tests/unit/Settings/CardDAVSettingsTest.php new file mode 100644 index 0000000000000..580efd1cc27ee --- /dev/null +++ b/apps/dav/tests/unit/Settings/CardDAVSettingsTest.php @@ -0,0 +1,58 @@ + + * + * @author Thomas Citharel + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\DAV\Tests\Unit\DAV\Settings; + +use OCA\DAV\Settings\CardDAVSettings; +use OCP\IConfig; +use Test\TestCase; + +class CardDAVSettingsTest extends TestCase { + + /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ + private $config; + + /** @var CardDAVSettings */ + private $settings; + + public function setUp() { + parent::setUp(); + + $this->config = $this->createMock(IConfig::class); + $this->settings = new CardDAVSettings($this->config); + } + + public function testGetForm() { + $result = $this->settings->getForm(); + + $this->assertInstanceOf('OCP\AppFramework\Http\TemplateResponse', $result); + } + + public function testGetSection() { + $this->assertEquals('groupware', $this->settings->getSection()); + } + + public function testGetPriority() { + $this->assertEquals(10, $this->settings->getPriority()); + } +}