Merge pull request #22686 from owncloud/cleanup_remote_address_books

remove remote address book if access was revoked

14 files changed, 113 insertions, 33 deletions

diff --git a/apps/federation/appinfo/application.php b/apps/federation/appinfo/application.php
index 0d033f44982..93897d211c3 100644
--- a/apps/federation/appinfo/application.php
+++ b/apps/federation/appinfo/application.php
@@ -75,13 +75,15 @@ class Application extends \OCP\AppFramework\App {
 		});
 
 		$container->registerService('TrustedServers', function(IAppContainer $c) {
+			$server = $c->getServer();
 			return new TrustedServers(
 				$c->query('DbHandler'),
-				\OC::$server->getHTTPClientService(),
-				\OC::$server->getLogger(),
-				\OC::$server->getJobList(),
-				\OC::$server->getSecureRandom(),
-				\OC::$server->getConfig()
+				$server->getHTTPClientService(),
+				$server->getLogger(),
+				$server->getJobList(),
+				$server->getSecureRandom(),
+				$server->getConfig(),
+				$server->getEventDispatcher()
 			);
 		});
 
@@ -94,6 +96,7 @@ class Application extends \OCP\AppFramework\App {
 				$c->query('TrustedServers')
 			);
 		});
+
 	}
 
 	private function registerMiddleware() {
diff --git a/apps/federation/appinfo/database.xml b/apps/federation/appinfo/database.xml
index 05b7fb12b49..61c3b8ac6d8 100644
--- a/apps/federation/appinfo/database.xml
+++ b/apps/federation/appinfo/database.xml
@@ -27,8 +27,7 @@
 				<type>text</type>
 				<default></default>
 				<notnull>true</notnull>
-				<length>32</length>
-				<comments>md5 hash of the url without the protocol</comments>
+				<comments>sha1 hash of the url without the protocol</comments>
 			</field>
 			<field>
 				<name>token</name>
diff --git a/apps/federation/appinfo/info.xml b/apps/federation/appinfo/info.xml
index 7786deef38e..be591b5b693 100644
--- a/apps/federation/appinfo/info.xml
+++ b/apps/federation/appinfo/info.xml
@@ -5,7 +5,7 @@
     <description>ownCloud Federation allows you to connect with other trusted ownClouds to exchange the user directory. For example this will be used to auto-complete external users for federated sharing.</description>
     <licence>AGPL</licence>
     <author>Bjoern Schiessle</author>
-    <version>0.0.3</version>
+    <version>0.0.4</version>
     <namespace>Federation</namespace>
     <category>other</category>
 	<dependencies>
diff --git a/apps/federation/backgroundjob/getsharedsecret.php b/apps/federation/backgroundjob/getsharedsecret.php
index ebc106ba94e..f896076139d 100644
--- a/apps/federation/backgroundjob/getsharedsecret.php
+++ b/apps/federation/backgroundjob/getsharedsecret.php
@@ -91,12 +91,13 @@ class GetSharedSecret extends QueuedJob{
 			$this->trustedServers = $trustedServers;
 		} else {
 			$this->trustedServers = new TrustedServers(
-					$this->dbHandler,
-					\OC::$server->getHTTPClientService(),
-					$this->logger,
-					$this->jobList,
-					\OC::$server->getSecureRandom(),
-					\OC::$server->getConfig()
+				$this->dbHandler,
+				\OC::$server->getHTTPClientService(),
+				$this->logger,
+				$this->jobList,
+				\OC::$server->getSecureRandom(),
+				\OC::$server->getConfig(),
+				\OC::$server->getEventDispatcher()
 			);
 		}
 	}
diff --git a/apps/federation/backgroundjob/requestsharedsecret.php b/apps/federation/backgroundjob/requestsharedsecret.php
index 302711af27f..79b55fe4ee4 100644
--- a/apps/federation/backgroundjob/requestsharedsecret.php
+++ b/apps/federation/backgroundjob/requestsharedsecret.php
@@ -95,7 +95,8 @@ class RequestSharedSecret extends QueuedJob {
 				$this->logger,
 				$this->jobList,
 				\OC::$server->getSecureRandom(),
-				\OC::$server->getConfig()
+				\OC::$server->getConfig(),
+				\OC::$server->getEventDispatcher()
 			);
 		}
 	}
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/dbhandler.php b/apps/federation/lib/dbhandler.php
index 3ea84baa3eb..8720560efc6 100644
--- a/apps/federation/lib/dbhandler.php
+++ b/apps/federation/lib/dbhandler.php
@@ -106,13 +106,35 @@ class DbHandler {
 	}
 
 	/**
+	 * get trusted server with given ID
+	 *
+	 * @param int $id
+	 * @return array
+	 * @throws \Exception
+	 */
+	public function getServerById($id) {
+		$query = $this->connection->getQueryBuilder();
+		$query->select('*')->from($this->dbTable)
+			->where($query->expr()->eq('id', $query->createParameter('id')))
+			->setParameter('id', $id);
+		$query->execute();
+		$result = $query->execute()->fetchAll();
+
+		if (empty($result)) {
+			throw new \Exception('No Server found with ID: ' . $id);
+		}
+
+		return $result[0];
+	}
+
+	/**
 	 * get all trusted servers
 	 *
 	 * @return array
 	 */
 	public function getAllServer() {
 		$query = $this->connection->getQueryBuilder();
-		$query->select(['url', 'id', 'status', 'shared_secret', 'sync_token'])->from($this->dbTable);
+		$query->select(['url', 'url_hash', 'id', 'status', 'shared_secret', 'sync_token'])->from($this->dbTable);
 		$result = $query->execute()->fetchAll();
 		return $result;
 	}
@@ -252,11 +274,11 @@ class DbHandler {
 	 */
 	protected function hash($url) {
 		$normalized = $this->normalizeUrl($url);
-		return md5($normalized);
+		return sha1($normalized);
 	}
 
 	/**
-	 * normalize URL, used to create the md5 hash
+	 * normalize URL, used to create the sha1 hash
 	 *
 	 * @param string $url
 	 * @return string
diff --git a/apps/federation/lib/syncfederationaddressbooks.php b/apps/federation/lib/syncfederationaddressbooks.php
index 6419fdddf8e..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;
@@ -40,7 +41,7 @@ class SyncFederationAddressBooks {
 			if (is_null($sharedSecret)) {
 				continue;
 			}
-			$targetBookId = sha1($url);
+			$targetBookId = $trustedServer['url_hash'];
 			$targetPrincipal = "principals/system/system";
 			$targetBookProperties = [
 					'{DAV:}displayname' => $url
@@ -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 340accfdbdf..6f99a3c6a8c 100644
--- a/apps/federation/lib/trustedservers.php
+++ b/apps/federation/lib/trustedservers.php
@@ -30,6 +30,8 @@ use OCP\Http\Client\IClientService;
 use OCP\IConfig;
 use OCP\ILogger;
 use OCP\Security\ISecureRandom;
+use Symfony\Component\EventDispatcher\EventDispatcherInterface;
+use Symfony\Component\EventDispatcher\GenericEvent;
 
 class TrustedServers {
 
@@ -39,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;
@@ -58,6 +62,9 @@ class TrustedServers {
 	/** @var IConfig */
 	private $config;
 
+	/** @var EventDispatcherInterface */
+	private $dispatcher;
+
 	/**
 	 * @param DbHandler $dbHandler
 	 * @param IClientService $httpClientService
@@ -65,6 +72,7 @@ class TrustedServers {
 	 * @param IJobList $jobList
 	 * @param ISecureRandom $secureRandom
 	 * @param IConfig $config
+	 * @param EventDispatcherInterface $dispatcher
 	 */
 	public function __construct(
 		DbHandler $dbHandler,
@@ -72,7 +80,8 @@ class TrustedServers {
 		ILogger $logger,
 		IJobList $jobList,
 		ISecureRandom $secureRandom,
-		IConfig $config
+		IConfig $config,
+		EventDispatcherInterface $dispatcher
 	) {
 		$this->dbHandler = $dbHandler;
 		$this->httpClientService = $httpClientService;
@@ -80,6 +89,7 @@ class TrustedServers {
 		$this->jobList = $jobList;
 		$this->secureRandom = $secureRandom;
 		$this->config = $config;
+		$this->dispatcher = $dispatcher;
 	}
 
 	/**
@@ -154,7 +164,10 @@ class TrustedServers {
 	 * @param int $id
 	 */
 	public function removeServer($id) {
+		$server = $this->dbHandler->getServerById($id);
 		$this->dbHandler->removeServer($id);
+		$event = new GenericEvent($server['url_hash']);
+		$this->dispatcher->dispatch('OCP\Federation\TrustedServerEvent::remove', $event);
 	}
 
 	/**
@@ -222,6 +235,7 @@ class TrustedServers {
 	 *
 	 * @param $status
 	 * @return bool
+	 * @throws HintException
 	 */
 	protected function checkOwnCloudVersion($status) {
 		$decoded = json_decode($status, true);
diff --git a/apps/federation/settings/settings-admin.php b/apps/federation/settings/settings-admin.php
index 8c6bfe6bbbb..a41d815feb8 100644
--- a/apps/federation/settings/settings-admin.php
+++ b/apps/federation/settings/settings-admin.php
@@ -34,7 +34,8 @@ $trustedServers = new \OCA\Federation\TrustedServers(
 	\OC::$server->getLogger(),
 	\OC::$server->getJobList(),
 	\OC::$server->getSecureRandom(),
-	\OC::$server->getConfig()
+	\OC::$server->getConfig(),
+	\OC::$server->getEventDispatcher()
 );
 
 $template->assign('trustedServers', $trustedServers->getServers());
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>
diff --git a/apps/federation/tests/lib/dbhandlertest.php b/apps/federation/tests/lib/dbhandlertest.php
index 6fe5d9ea8ef..28f76dbb22e 100644
--- a/apps/federation/tests/lib/dbhandlertest.php
+++ b/apps/federation/tests/lib/dbhandlertest.php
@@ -89,9 +89,9 @@ class DbHandlerTest extends TestCase {
 
 	public function dataTestAddServer() {
 		return [
-				['http://owncloud.org', 'http://owncloud.org', md5('owncloud.org')],
-				['https://owncloud.org', 'https://owncloud.org', md5('owncloud.org')],
-				['http://owncloud.org/', 'http://owncloud.org', md5('owncloud.org')],
+				['http://owncloud.org', 'http://owncloud.org', sha1('owncloud.org')],
+				['https://owncloud.org', 'https://owncloud.org', sha1('owncloud.org')],
+				['http://owncloud.org/', 'http://owncloud.org', sha1('owncloud.org')],
 		];
 	}
 
@@ -115,6 +115,15 @@ class DbHandlerTest extends TestCase {
 		$this->assertSame($id1, (int)$result[0]['id']);
 	}
 
+
+	public function testGetServerById() {
+		$this->dbHandler->addServer('server1');
+		$id = $this->dbHandler->addServer('server2');
+
+		$result = $this->dbHandler->getServerById($id);
+		$this->assertSame('server2', $result['url']);
+	}
+
 	public function testGetAll() {
 		$id1 = $this->dbHandler->addServer('server1');
 		$id2 = $this->dbHandler->addServer('server2');
@@ -233,10 +242,10 @@ class DbHandlerTest extends TestCase {
 
 	public function dataTestHash() {
 		return [
-			['server1', md5('server1')],
-			['http://server1', md5('server1')],
-			['https://server1', md5('server1')],
-			['http://server1/', md5('server1')],
+			['server1', sha1('server1')],
+			['http://server1', sha1('server1')],
+			['https://server1', sha1('server1')],
+			['http://server1/', sha1('server1')],
 		];
 	}
 
diff --git a/apps/federation/tests/lib/syncfederationaddressbookstest.php b/apps/federation/tests/lib/syncfederationaddressbookstest.php
index 770896535fa..9290bad8bd6 100644
--- a/apps/federation/tests/lib/syncfederationaddressbookstest.php
+++ b/apps/federation/tests/lib/syncfederationaddressbookstest.php
@@ -19,6 +19,7 @@ class SyncFederationAddressbooksTest extends \Test\TestCase {
 			willReturn([
 			[
 				'url' => 'https://cloud.drop.box',
+				'url_hash' => 'sha1',
 				'shared_secret' => 'iloveowncloud',
 				'sync_token' => '0'
 			]
@@ -47,6 +48,7 @@ class SyncFederationAddressbooksTest extends \Test\TestCase {
 		willReturn([
 			[
 				'url' => 'https://cloud.drop.box',
+				'url_hash' => 'sha1',
 				'shared_secret' => 'iloveowncloud',
 				'sync_token' => '0'
 			]
diff --git a/apps/federation/tests/lib/trustedserverstest.php b/apps/federation/tests/lib/trustedserverstest.php
index 130a0e3bb22..80f7843d818 100644
--- a/apps/federation/tests/lib/trustedserverstest.php
+++ b/apps/federation/tests/lib/trustedserverstest.php
@@ -23,7 +23,6 @@
 namespace OCA\Federation\Tests\lib;
 
 
-use OC\HintException;
 use OCA\Federation\DbHandler;
 use OCA\Federation\TrustedServers;
 use OCP\BackgroundJob\IJobList;
@@ -33,6 +32,7 @@ use OCP\Http\Client\IResponse;
 use OCP\IConfig;
 use OCP\ILogger;
 use OCP\Security\ISecureRandom;
+use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Test\TestCase;
 
 class TrustedServersTest extends TestCase {
@@ -64,11 +64,16 @@ class TrustedServersTest extends TestCase {
 	/** @var  \PHPUnit_Framework_MockObject_MockObject | IConfig */
 	private $config;
 
+	/** @var  \PHPUnit_Framework_MockObject_MockObject | EventDispatcherInterface */
+	private $dispatcher;
+
 	public function setUp() {
 		parent::setUp();
 
 		$this->dbHandler = $this->getMockBuilder('\OCA\Federation\DbHandler')
 			->disableOriginalConstructor()->getMock();
+		$this->dispatcher = $this->getMockBuilder('Symfony\Component\EventDispatcher\EventDispatcherInterface')
+			->disableOriginalConstructor()->getMock();
 		$this->httpClientService = $this->getMock('OCP\Http\Client\IClientService');
 		$this->httpClient = $this->getMock('OCP\Http\Client\IClient');
 		$this->response = $this->getMock('OCP\Http\Client\IResponse');
@@ -83,7 +88,8 @@ class TrustedServersTest extends TestCase {
 			$this->logger,
 			$this->jobList,
 			$this->secureRandom,
-			$this->config
+			$this->config,
+			$this->dispatcher
 		);
 
 	}
@@ -103,7 +109,8 @@ class TrustedServersTest extends TestCase {
 					$this->logger,
 					$this->jobList,
 					$this->secureRandom,
-					$this->config
+					$this->config,
+					$this->dispatcher
 				]
 			)
 			->setMethods(['normalizeUrl', 'updateProtocol'])
@@ -191,7 +198,18 @@ class TrustedServersTest extends TestCase {
 
 	public function testRemoveServer() {
 		$id = 42;
+		$server = ['url_hash' => 'url_hash'];
 		$this->dbHandler->expects($this->once())->method('removeServer')->with($id);
+		$this->dbHandler->expects($this->once())->method('getServerById')->with($id)
+			->willReturn($server);
+		$this->dispatcher->expects($this->once())->method('dispatch')
+			->willReturnCallback(
+				function($eventId, $event) {
+					$this->assertSame($eventId, 'OCP\Federation\TrustedServerEvent::remove');
+					$this->assertInstanceOf('Symfony\Component\EventDispatcher\GenericEvent', $event);
+					$this->assertSame('url_hash', $event->getSubject());
+				}
+			);
 		$this->trustedServers->removeServer($id);
 	}
 
@@ -247,7 +265,8 @@ class TrustedServersTest extends TestCase {
 					$this->logger,
 					$this->jobList,
 					$this->secureRandom,
-					$this->config
+					$this->config,
+					$this->dispatcher
 				]
 			)
 			->setMethods(['checkOwnCloudVersion'])