From 87e47afed85521439c351ae30e9849f0a74a399d Mon Sep 17 00:00:00 2001
From: Bjoern Schiessle <schiessle@owncloud.com>
Date: Fri, 26 Feb 2016 17:51:20 +0100
Subject: [PATCH] remove synced remote address book if the remote server
revoked access to his system address book
---
apps/dav/appinfo/application.php | 3 ++-
apps/dav/lib/carddav/syncservice.php | 21 +++++++++++++++++--
.../tests/unit/carddav/syncservicetest.php | 9 +++++---
.../command/syncfederationaddressbooks.php | 1 +
.../lib/syncfederationaddressbooks.php | 4 ++++
apps/federation/lib/trustedservers.php | 2 ++
apps/federation/templates/settings-admin.php | 6 +++++-
7 files changed, 39 insertions(+), 7 deletions(-)
diff --git a/apps/dav/appinfo/application.php b/apps/dav/appinfo/application.php
index 7a201e1dd78..ea9e1ad8f7c 100644
--- a/apps/dav/appinfo/application.php
+++ b/apps/dav/appinfo/application.php
@@ -69,7 +69,8 @@ class Application extends App {
/** @var IAppContainer $c */
return new SyncService(
$c->query('CardDavBackend'),
- $c->getServer()->getUserManager()
+ $c->getServer()->getUserManager(),
+ $c->getServer()->getLogger()
);
});
diff --git a/apps/dav/lib/carddav/syncservice.php b/apps/dav/lib/carddav/syncservice.php
index 4b5907620e6..2e7397fc70b 100644
--- a/apps/dav/lib/carddav/syncservice.php
+++ b/apps/dav/lib/carddav/syncservice.php
@@ -21,11 +21,14 @@
namespace OCA\DAV\CardDAV;
+use OCP\AppFramework\Http;
+use OCP\ILogger;
use OCP\IUser;
use OCP\IUserManager;
use Sabre\DAV\Client;
use Sabre\DAV\Xml\Response\MultiStatus;
use Sabre\DAV\Xml\Service;
+use Sabre\HTTP\ClientHttpException;
use Sabre\VObject\Reader;
class SyncService {
@@ -36,12 +39,16 @@ class SyncService {
/** @var IUserManager */
private $userManager;
+ /** @var ILogger */
+ private $logger;
+
/** @var array */
private $localSystemAddressBook;
- public function __construct(CardDavBackend $backend, IUserManager $userManager) {
+ public function __construct(CardDavBackend $backend, IUserManager $userManager, ILogger $logger) {
$this->backend = $backend;
$this->userManager = $userManager;
+ $this->logger = $logger;
}
/**
@@ -53,6 +60,7 @@ class SyncService {
* @param string $targetPrincipal
* @param array $targetProperties
* @return string
+ * @throws \Exception
*/
public function syncRemoteAddressBook($url, $userName, $sharedSecret, $syncToken, $targetBookId, $targetPrincipal, $targetProperties) {
// 1. create addressbook
@@ -60,7 +68,16 @@ class SyncService {
$addressBookId = $book['id'];
// 2. query changes
- $response = $this->requestSyncReport($url, $userName, $sharedSecret, $syncToken);
+ try {
+ $response = $this->requestSyncReport($url, $userName, $sharedSecret, $syncToken);
+ } catch (ClientHttpException $ex) {
+ if ($ex->getCode() === Http::STATUS_UNAUTHORIZED) {
+ // remote server revoked access to the address book, remove it
+ $this->backend->deleteAddressBook($addressBookId);
+ $this->logger->info('Authorization failed, remove address book: ' . $url, ['app' => 'dav']);
+ throw $ex;
+ }
+ }
// 3. apply changes
// TODO: use multi-get for download
diff --git a/apps/dav/tests/unit/carddav/syncservicetest.php b/apps/dav/tests/unit/carddav/syncservicetest.php
index a6af98f7e8c..7652afdc225 100644
--- a/apps/dav/tests/unit/carddav/syncservicetest.php
+++ b/apps/dav/tests/unit/carddav/syncservicetest.php
@@ -68,13 +68,15 @@ class SyncServiceTest extends TestCase {
/** @var IUserManager $userManager */
$userManager = $this->getMockBuilder('OCP\IUserManager')->disableOriginalConstructor()->getMock();
- $ss = new SyncService($backend, $userManager);
+ $logger = $this->getMockBuilder('OCP\ILogger')->disableOriginalConstructor()->getMock();
+ $ss = new SyncService($backend, $userManager, $logger);
$book = $ss->ensureSystemAddressBookExists('principals/users/adam', 'contacts', []);
}
public function testUpdateAndDeleteUser() {
/** @var CardDavBackend | \PHPUnit_Framework_MockObject_MockObject $backend */
$backend = $this->getMockBuilder('OCA\DAV\CardDAV\CardDAVBackend')->disableOriginalConstructor()->getMock();
+ $logger = $this->getMockBuilder('OCP\ILogger')->disableOriginalConstructor()->getMock();
$backend->expects($this->once())->method('createCard');
$backend->expects($this->once())->method('updateCard');
@@ -92,7 +94,7 @@ class SyncServiceTest extends TestCase {
$user->method('getBackendClassName')->willReturn('unittest');
$user->method('getUID')->willReturn('test-user');
- $ss = new SyncService($backend, $userManager);
+ $ss = new SyncService($backend, $userManager, $logger);
$ss->updateUser($user);
$user->method('getDisplayName')->willReturn('A test user for unit testing');
@@ -123,8 +125,9 @@ class SyncServiceTest extends TestCase {
*/
private function getSyncServiceMock($backend, $response) {
$userManager = $this->getMockBuilder('OCP\IUserManager')->disableOriginalConstructor()->getMock();
+ $logger = $this->getMockBuilder('OCP\ILogger')->disableOriginalConstructor()->getMock();
/** @var SyncService | \PHPUnit_Framework_MockObject_MockObject $ss */
- $ss = $this->getMock('OCA\DAV\CardDAV\SyncService', ['ensureSystemAddressBookExists', 'requestSyncReport', 'download'], [$backend, $userManager]);
+ $ss = $this->getMock('OCA\DAV\CardDAV\SyncService', ['ensureSystemAddressBookExists', 'requestSyncReport', 'download'], [$backend, $userManager, $logger]);
$ss->method('requestSyncReport')->withAnyParameters()->willReturn(['response' => $response, 'token' => 'sync-token-1']);
$ss->method('ensureSystemAddressBookExists')->willReturn(['id' => 1]);
$ss->method('download')->willReturn([
diff --git a/apps/federation/command/syncfederationaddressbooks.php b/apps/federation/command/syncfederationaddressbooks.php
index 61703d9d4e4..72d12e59b22 100644
--- a/apps/federation/command/syncfederationaddressbooks.php
+++ b/apps/federation/command/syncfederationaddressbooks.php
@@ -40,6 +40,7 @@ class SyncFederationAddressBooks extends Command {
$this->syncService->syncThemAll(function($url, $ex) use ($progress, $output) {
if ($ex instanceof \Exception) {
$output->writeln("Error while syncing $url : " . $ex->getMessage());
+
} else {
$progress->advance();
}
diff --git a/apps/federation/lib/syncfederationaddressbooks.php b/apps/federation/lib/syncfederationaddressbooks.php
index 886f6505b20..f9cee9a7137 100644
--- a/apps/federation/lib/syncfederationaddressbooks.php
+++ b/apps/federation/lib/syncfederationaddressbooks.php
@@ -3,6 +3,7 @@
namespace OCA\Federation;
use OCA\DAV\CardDAV\SyncService;
+use OCP\AppFramework\Http;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Helper\ProgressBar;
use Symfony\Component\Console\Input\InputInterface;
@@ -51,6 +52,9 @@ class SyncFederationAddressBooks {
$this->dbHandler->setServerStatus($url, TrustedServers::STATUS_OK, $newToken);
}
} catch (\Exception $ex) {
+ if ($ex->getCode() === Http::STATUS_UNAUTHORIZED) {
+ $this->dbHandler->setServerStatus($url, TrustedServers::STATUS_ACCESS_REVOKED);
+ }
$callback($url, $ex);
}
}
diff --git a/apps/federation/lib/trustedservers.php b/apps/federation/lib/trustedservers.php
index fed6b0944a8..3d515f24a7c 100644
--- a/apps/federation/lib/trustedservers.php
+++ b/apps/federation/lib/trustedservers.php
@@ -41,6 +41,8 @@ class TrustedServers {
const STATUS_PENDING = 2;
/** something went wrong, misconfigured server, software bug,... user interaction needed */
const STATUS_FAILURE = 3;
+ /** remote server revoked access */
+ const STATUS_ACCESS_REVOKED = 4;
/** @var dbHandler */
private $dbHandler;
diff --git a/apps/federation/templates/settings-admin.php b/apps/federation/templates/settings-admin.php
index 854bb744179..77c552ee789 100644
--- a/apps/federation/templates/settings-admin.php
+++ b/apps/federation/templates/settings-admin.php
@@ -26,7 +26,11 @@ style('federation', 'settings-admin')
<li id="<?php p($trustedServer['id']); ?>" class="icon-delete">
<?php if((int)$trustedServer['status'] === TrustedServers::STATUS_OK) { ?>
<span class="status success"></span>
- <?php } elseif((int)$trustedServer['status'] === TrustedServers::STATUS_PENDING) { ?>
+ <?php
+ } elseif(
+ (int)$trustedServer['status'] === TrustedServers::STATUS_PENDING ||
+ (int)$trustedServer['status'] === TrustedServers::STATUS_ACCESS_REVOKED
+ ) { ?>
<span class="status indeterminate"></span>
<?php } else {?>
<span class="status error"></span>
--
GitLab