From eccd7cf6548e1ea639aea48cbf7e52e95dc98c11 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 26 Feb 2016 17:02:13 +0100 Subject: [PATCH 01/14] reuse the url_hash instead of calculating a new hash for the address book --- apps/federation/appinfo/database.xml | 3 +-- apps/federation/lib/dbhandler.php | 6 +++--- apps/federation/lib/syncfederationaddressbooks.php | 2 +- apps/federation/tests/lib/dbhandlertest.php | 14 +++++++------- .../tests/lib/syncfederationaddressbookstest.php | 2 ++ 5 files changed, 14 insertions(+), 13 deletions(-) diff --git a/apps/federation/appinfo/database.xml b/apps/federation/appinfo/database.xml index 05b7fb12b4..61c3b8ac6d 100644 --- a/apps/federation/appinfo/database.xml +++ b/apps/federation/appinfo/database.xml @@ -27,8 +27,7 @@ text true - 32 - md5 hash of the url without the protocol + sha1 hash of the url without the protocol token diff --git a/apps/federation/lib/dbhandler.php b/apps/federation/lib/dbhandler.php index 3ea84baa3e..df36228c76 100644 --- a/apps/federation/lib/dbhandler.php +++ b/apps/federation/lib/dbhandler.php @@ -112,7 +112,7 @@ class DbHandler { */ 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 +252,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 6419fdddf8..886f6505b2 100644 --- a/apps/federation/lib/syncfederationaddressbooks.php +++ b/apps/federation/lib/syncfederationaddressbooks.php @@ -40,7 +40,7 @@ class SyncFederationAddressBooks { if (is_null($sharedSecret)) { continue; } - $targetBookId = sha1($url); + $targetBookId = $trustedServer['url_hash']; $targetPrincipal = "principals/system/system"; $targetBookProperties = [ '{DAV:}displayname' => $url diff --git a/apps/federation/tests/lib/dbhandlertest.php b/apps/federation/tests/lib/dbhandlertest.php index 6fe5d9ea8e..ee28da8e02 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')], ]; } @@ -233,10 +233,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 770896535f..9290bad8bd 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' ] From 7189c72c33a03d57473f7e3443193d07bece7c15 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 26 Feb 2016 16:59:53 +0100 Subject: [PATCH 02/14] remove remote address book if the admin removes the server from the trusted servers list --- apps/dav/appinfo/app.php | 12 +++++++++ apps/federation/appinfo/application.php | 13 +++++---- .../backgroundjob/getsharedsecret.php | 13 ++++----- .../backgroundjob/requestsharedsecret.php | 3 ++- apps/federation/lib/dbhandler.php | 22 +++++++++++++++ apps/federation/lib/trustedservers.php | 13 ++++++++- apps/federation/settings/settings-admin.php | 3 ++- apps/federation/tests/lib/dbhandlertest.php | 9 +++++++ .../tests/lib/trustedserverstest.php | 27 ++++++++++++++++--- 9 files changed, 97 insertions(+), 18 deletions(-) diff --git a/apps/dav/appinfo/app.php b/apps/dav/appinfo/app.php index d33545222b..a38bee9824 100644 --- a/apps/dav/appinfo/app.php +++ b/apps/dav/appinfo/app.php @@ -28,6 +28,18 @@ $app->registerHooks(); return $app->getSyncService(); }); +$eventDispatcher = \OC::$server->getEventDispatcher(); + +$eventDispatcher->addListener('OCP\Federation\TrustedServerEvent::remove', + function(\Symfony\Component\EventDispatcher\GenericEvent $event) use ($app) { + /** @var \OCA\DAV\CardDAV\CardDavBackend $cardDavBackend */ + $cardDavBackend = $app->getContainer()->query('CardDavBackend'); + $addressBookUri = $event->getSubject(); + $addressBook = $cardDavBackend->getAddressBooksByUri('principals/system/system', $addressBookUri); + $cardDavBackend->deleteAddressBook($addressBook['id']); + } +); + $cm = \OC::$server->getContactsManager(); $cm->register(function() use ($cm, $app) { $userId = \OC::$server->getUserSession()->getUser()->getUID(); diff --git a/apps/federation/appinfo/application.php b/apps/federation/appinfo/application.php index 0d033f4498..93897d211c 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/backgroundjob/getsharedsecret.php b/apps/federation/backgroundjob/getsharedsecret.php index ebc106ba94..f896076139 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 302711af27..79b55fe4ee 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/lib/dbhandler.php b/apps/federation/lib/dbhandler.php index df36228c76..8720560efc 100644 --- a/apps/federation/lib/dbhandler.php +++ b/apps/federation/lib/dbhandler.php @@ -105,6 +105,28 @@ class DbHandler { $query->execute(); } + /** + * 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 * diff --git a/apps/federation/lib/trustedservers.php b/apps/federation/lib/trustedservers.php index 340accfdbd..fed6b0944a 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 { @@ -58,6 +60,9 @@ class TrustedServers { /** @var IConfig */ private $config; + /** @var EventDispatcherInterface */ + private $dispatcher; + /** * @param DbHandler $dbHandler * @param IClientService $httpClientService @@ -65,6 +70,7 @@ class TrustedServers { * @param IJobList $jobList * @param ISecureRandom $secureRandom * @param IConfig $config + * @param EventDispatcherInterface $dispatcher */ public function __construct( DbHandler $dbHandler, @@ -72,7 +78,8 @@ class TrustedServers { ILogger $logger, IJobList $jobList, ISecureRandom $secureRandom, - IConfig $config + IConfig $config, + EventDispatcherInterface $dispatcher ) { $this->dbHandler = $dbHandler; $this->httpClientService = $httpClientService; @@ -80,6 +87,7 @@ class TrustedServers { $this->jobList = $jobList; $this->secureRandom = $secureRandom; $this->config = $config; + $this->dispatcher = $dispatcher; } /** @@ -154,7 +162,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); } /** diff --git a/apps/federation/settings/settings-admin.php b/apps/federation/settings/settings-admin.php index 8c6bfe6bbb..a41d815feb 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/tests/lib/dbhandlertest.php b/apps/federation/tests/lib/dbhandlertest.php index ee28da8e02..28f76dbb22 100644 --- a/apps/federation/tests/lib/dbhandlertest.php +++ b/apps/federation/tests/lib/dbhandlertest.php @@ -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'); diff --git a/apps/federation/tests/lib/trustedserverstest.php b/apps/federation/tests/lib/trustedserverstest.php index 130a0e3bb2..80f7843d81 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']) From 87e47afed85521439c351ae30e9849f0a74a399d Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 26 Feb 2016 17:51:20 +0100 Subject: [PATCH 03/14] 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 7a201e1dd7..ea9e1ad8f7 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 4b5907620e..2e7397fc70 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 a6af98f7e8..7652afdc22 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 61703d9d4e..72d12e59b2 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 886f6505b2..f9cee9a713 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 fed6b0944a..3d515f24a7 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 854bb74417..77c552ee78 100644 --- a/apps/federation/templates/settings-admin.php +++ b/apps/federation/templates/settings-admin.php @@ -26,7 +26,11 @@ style('federation', 'settings-admin')
  • - + From 9e9cb739879107ba0da0cf43f382d00b87ddecf4 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Mon, 29 Feb 2016 11:11:31 +0100 Subject: [PATCH 04/14] fix doc-block --- apps/federation/lib/trustedservers.php | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/federation/lib/trustedservers.php b/apps/federation/lib/trustedservers.php index 3d515f24a7..6f99a3c6a8 100644 --- a/apps/federation/lib/trustedservers.php +++ b/apps/federation/lib/trustedservers.php @@ -235,6 +235,7 @@ class TrustedServers { * * @param $status * @return bool + * @throws HintException */ protected function checkOwnCloudVersion($status) { $decoded = json_decode($status, true); From 078cc3b3f0ac34919b4b30cfa08799ce7743bc87 Mon Sep 17 00:00:00 2001 From: Robin McCorkell Date: Mon, 29 Feb 2016 21:13:27 +0000 Subject: [PATCH 05/14] Use readiness notification socket to be absolutely sure this works --- apps/files_external/tests/env/start-amazons3-ceph.sh | 8 +++++++- apps/files_external/tests/env/start-swift-ceph.sh | 8 +++++++- apps/files_external/tests/env/stop-amazons3-ceph.sh | 1 + apps/files_external/tests/env/stop-swift-ceph.sh | 1 + 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/apps/files_external/tests/env/start-amazons3-ceph.sh b/apps/files_external/tests/env/start-amazons3-ceph.sh index 20fa7e7bb2..d36980fdd1 100755 --- a/apps/files_external/tests/env/start-amazons3-ceph.sh +++ b/apps/files_external/tests/env/start-amazons3-ceph.sh @@ -31,6 +31,10 @@ if [ -z "$thisFolder" ]; then thisFolder="." fi; +# create readiness notification socket +notify_sock=$(readlink -f "$thisFolder"/dockerContainerCeph.$EXECUTOR_NUMBER.amazons3.sock) +mkfifo "$notify_sock" + user=test accesskey=aaabbbccc secretkey=cccbbbaaa @@ -39,6 +43,7 @@ port=80 container=`docker run -d \ -e RGW_CIVETWEB_PORT=$port \ + -v "$notify_sock":/run/notifyme.sock \ ${docker_image}` host=`docker inspect --format="{{.NetworkSettings.IPAddress}}" $container` @@ -50,7 +55,8 @@ echo "${docker_image} container: $container" echo $container >> $thisFolder/dockerContainerCeph.$EXECUTOR_NUMBER.amazons3 echo -n "Waiting for ceph initialization" -if ! "$thisFolder"/env/wait-for-connection ${host} ${port} 60; then +ready=$(timeout 60 cat "$notify_sock") +if [[ $ready != 'READY=1' ]]; then echo "[ERROR] Waited 60 seconds, no response" >&2 exit 1 fi diff --git a/apps/files_external/tests/env/start-swift-ceph.sh b/apps/files_external/tests/env/start-swift-ceph.sh index 123f6b6871..357512ae4d 100755 --- a/apps/files_external/tests/env/start-swift-ceph.sh +++ b/apps/files_external/tests/env/start-swift-ceph.sh @@ -31,6 +31,10 @@ if [ -z "$thisFolder" ]; then thisFolder="." fi; +# create readiness notification socket +notify_sock=$(readlink -f "$thisFolder"/dockerContainerCeph.$EXECUTOR_NUMBER.swift.sock) +mkfifo "$notify_sock" + port=5001 user=test @@ -48,6 +52,7 @@ container=`docker run -d \ -e KEYSTONE_SERVICE=${service} \ -e OSD_SIZE=300 \ --privileged \ + -v "$notify_sock":/run/notifyme.sock \ ${docker_image}` host=`docker inspect --format="{{.NetworkSettings.IPAddress}}" $container` @@ -59,7 +64,8 @@ echo "${docker_image} container: $container" echo $container >> $thisFolder/dockerContainerCeph.$EXECUTOR_NUMBER.swift echo -n "Waiting for ceph initialization" -if ! "$thisFolder"/env/wait-for-connection ${host} 80 600; then +ready=$(timeout 600 cat "$notify_sock") +if [[ $ready != 'READY=1' ]]; then echo "[ERROR] Waited 600 seconds, no response" >&2 docker logs $container exit 1 diff --git a/apps/files_external/tests/env/stop-amazons3-ceph.sh b/apps/files_external/tests/env/stop-amazons3-ceph.sh index 3f56a6f1e5..dcf30d8c51 100755 --- a/apps/files_external/tests/env/stop-amazons3-ceph.sh +++ b/apps/files_external/tests/env/stop-amazons3-ceph.sh @@ -33,4 +33,5 @@ done; # cleanup rm $thisFolder/config.amazons3.php rm $thisFolder/dockerContainerCeph.$EXECUTOR_NUMBER.amazons3 +rm $thisFolder/dockerContainerCeph.$EXECUTOR_NUMBER.amazons3.sock diff --git a/apps/files_external/tests/env/stop-swift-ceph.sh b/apps/files_external/tests/env/stop-swift-ceph.sh index b1cbe94b48..9f15fb05a7 100755 --- a/apps/files_external/tests/env/stop-swift-ceph.sh +++ b/apps/files_external/tests/env/stop-swift-ceph.sh @@ -35,4 +35,5 @@ done; # cleanup rm $thisFolder/config.swift.php rm $thisFolder/dockerContainerCeph.$EXECUTOR_NUMBER.swift +rm $thisFolder/dockerContainerCeph.$EXECUTOR_NUMBER.swift.sock From a66899c52830cf9a8ccab995e9ff4047088be8b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Tue, 1 Mar 2016 10:41:05 +0100 Subject: [PATCH 06/14] Handle null case --- apps/dav/appinfo/app.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/apps/dav/appinfo/app.php b/apps/dav/appinfo/app.php index a38bee9824..5202e3d38b 100644 --- a/apps/dav/appinfo/app.php +++ b/apps/dav/appinfo/app.php @@ -20,6 +20,7 @@ */ use OCA\Dav\AppInfo\Application; +use Symfony\Component\EventDispatcher\GenericEvent; $app = new Application(); $app->registerHooks(); @@ -31,12 +32,14 @@ $app->registerHooks(); $eventDispatcher = \OC::$server->getEventDispatcher(); $eventDispatcher->addListener('OCP\Federation\TrustedServerEvent::remove', - function(\Symfony\Component\EventDispatcher\GenericEvent $event) use ($app) { + function(GenericEvent $event) use ($app) { /** @var \OCA\DAV\CardDAV\CardDavBackend $cardDavBackend */ $cardDavBackend = $app->getContainer()->query('CardDavBackend'); $addressBookUri = $event->getSubject(); $addressBook = $cardDavBackend->getAddressBooksByUri('principals/system/system', $addressBookUri); - $cardDavBackend->deleteAddressBook($addressBook['id']); + if (!is_null($addressBook)) { + $cardDavBackend->deleteAddressBook($addressBook['id']); + } } ); From 4943a0326d5b49fba787f13842791b4161bd38e0 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Tue, 1 Mar 2016 10:45:48 +0100 Subject: [PATCH 07/14] Use sockets for startup notification in primary storage tests --- tests/objectstore/start-swift-ceph.sh | 26 ++++++++++++-------------- tests/objectstore/stop-swift-ceph.sh | 1 + 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/tests/objectstore/start-swift-ceph.sh b/tests/objectstore/start-swift-ceph.sh index 8863a1f9f1..46cf51ad6c 100755 --- a/tests/objectstore/start-swift-ceph.sh +++ b/tests/objectstore/start-swift-ceph.sh @@ -28,6 +28,10 @@ docker pull ${docker_image} # retrieve current folder to place the config in the parent folder thisFolder="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" +# create readiness notification socket +notify_sock=$(readlink -f "$thisFolder"/dockerContainerCeph.$EXECUTOR_NUMBER.swift.sock) +mkfifo "$notify_sock" + port=5034 user=test @@ -45,6 +49,7 @@ container=`docker run -d \ -e KEYSTONE_SERVICE=${service} \ -e OSD_SIZE=300 \ -v ${thisFolder}/entrypoint.sh:/entrypoint.sh \ + -v "$notify_sock":/run/notifyme.sock \ --privileged \ --entrypoint /entrypoint.sh ${docker_image}` @@ -57,20 +62,13 @@ echo "${docker_image} container: $container" echo $container >> $thisFolder/dockerContainerCeph.$EXECUTOR_NUMBER.swift echo -n "Waiting for ceph initialization" -starttime=$(date +%s) -# support for GNU netcat and BSD netcat -while ! (nc -c -w 1 ${host} 80 &/dev/null \ - || nc -w 1 ${host} 80 &/dev/null); do - sleep 1 - echo -n '.' - if (( $(date +%s) > starttime + 160 )); then - echo - echo "[ERROR] Waited 120 seconds, no response" >&2 - exit 1 - fi -done -echo -sleep 20 # the keystone server also needs some time to fully initialize +ready=$(timeout 600 cat "$notify_sock") +if [[ $ready != 'READY=1' ]]; then + echo "[ERROR] Waited 600 seconds, no response" >&2 + docker logs $container + exit 1 +fi +sleep 1 cat > $thisFolder/swift.config.php < Date: Tue, 1 Mar 2016 11:21:14 +0100 Subject: [PATCH 08/14] increase version number --- apps/federation/appinfo/info.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/federation/appinfo/info.xml b/apps/federation/appinfo/info.xml index 7786deef38..be591b5b69 100644 --- a/apps/federation/appinfo/info.xml +++ b/apps/federation/appinfo/info.xml @@ -5,7 +5,7 @@ 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. AGPL Bjoern Schiessle - 0.0.3 + 0.0.4 Federation other From 03d0fa012f967dd058d3926751dc147537760ce3 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Sun, 28 Feb 2016 11:53:56 +0100 Subject: [PATCH 09/14] Checksum intergration test * Upload file with checksum * Chunked upload with checksum * Copy file with checksum should also copy the checksum * Moving a file with checksum should also move the checksum * Uploading a file with checksum and overwriting it with a file without cheksum should remove the checksum --- build/integration/config/behat.yml | 2 + .../features/bootstrap/ChecksumsContext.php | 227 ++++++++++++++++++ build/integration/features/checksums.feature | 76 ++++++ 3 files changed, 305 insertions(+) create mode 100644 build/integration/features/bootstrap/ChecksumsContext.php create mode 100644 build/integration/features/checksums.feature diff --git a/build/integration/config/behat.yml b/build/integration/config/behat.yml index d0c4586d28..4b5b5b16ef 100644 --- a/build/integration/config/behat.yml +++ b/build/integration/config/behat.yml @@ -20,6 +20,8 @@ default: baseUrl: http://localhost:8080 - CalDavContext: baseUrl: http://localhost:8080 + - ChecksumsContext: + baseUrl: http://localhost:8080 federation: paths: - %paths.base%/../federation_features diff --git a/build/integration/features/bootstrap/ChecksumsContext.php b/build/integration/features/bootstrap/ChecksumsContext.php new file mode 100644 index 0000000000..a5d20ba965 --- /dev/null +++ b/build/integration/features/bootstrap/ChecksumsContext.php @@ -0,0 +1,227 @@ +baseUrl = $baseUrl; + + // in case of ci deployment we take the server url from the environment + $testServerUrl = getenv('TEST_SERVER_URL'); + if ($testServerUrl !== false) { + $this->baseUrl = substr($testServerUrl, 0, -5); + } + } + + /** @BeforeScenario */ + public function tearUpScenario() { + $this->client = new Client(); + } + + /** @AfterScenario */ + public function tearDownScenario() { + } + + + /** + * @param string $userName + * @return string + */ + private function getPasswordForUser($userName) { + if($userName === 'admin') { + return 'admin'; + } + return '123456'; + } + + /** + * @When user :user uploads file :source to :destination with checksum :checksum + */ + public function userUploadsFileToWithChecksum($user, $source, $destination, $checksum) + { + $file = \GuzzleHttp\Stream\Stream::factory(fopen($source, 'r')); + try { + $this->response = $this->client->put( + $this->baseUrl . '/remote.php/webdav' . $destination, + [ + 'auth' => [ + $user, + $this->getPasswordForUser($user) + ], + 'body' => $file, + 'headers' => [ + 'OC-Checksum' => $checksum + ] + ] + ); + } catch (\GuzzleHttp\Exception\ServerException $e) { + // 4xx and 5xx responses cause an exception + $this->response = $e->getResponse(); + } + } + + /** + * @Then The webdav response should have a status code :statusCode + */ + public function theWebdavResponseShouldHaveAStatusCode($statusCode) { + if((int)$statusCode !== $this->response->getStatusCode()) { + throw new \Exception("Expected $statusCode, got ".$this->response->getStatusCode()); + } + } + + /** + * @When user :user request the checksum of :path via propfind + */ + public function userRequestTheChecksumOfViaPropfind($user, $path) + { + $request = $this->client->createRequest( + 'PROPFIND', + $this->baseUrl . '/remote.php/webdav' . $path, + [ + 'body' => ' + + + + +', + 'auth' => [ + $user, + $this->getPasswordForUser($user), + ] + ] + ); + $this->response = $this->client->send($request); + } + + /** + * @Then The webdav checksum should match :checksum + */ + public function theWebdavChecksumShouldMatch($checksum) + { + $service = new Sabre\Xml\Service(); + $parsed = $service->parse($this->response->getBody()->getContents()); + + /* + * Fetch the checksum array + * Maybe we want to do this a bit cleaner ;) + */ + $checksums = $parsed[0]['value'][1]['value'][0]['value'][0]; + + if ($checksums['value'][0]['value'] !== $checksum) { + throw new \Exception("Expected $checksum, got ".$checksums['value'][0]['value']); + } + } + + /** + * @When user :user downloads the file :path + */ + public function userDownloadsTheFile($user, $path) + { + $this->response = $this->client->get( + $this->baseUrl . '/remote.php/webdav' . $path, + [ + 'auth' => [ + $user, + $this->getPasswordForUser($user), + ] + ] + ); + } + + /** + * @Then The header checksum should match :checksum + */ + public function theHeaderChecksumShouldMatch($checksum) + { + if ($this->response->getHeader('OC-Checksum') !== $checksum) { + throw new \Exception("Expected $checksum, got ".$this->response->getHeader('OC-Checksum')); + } + } + + /** + * @Given User :user copied file :source to :destination + */ + public function userCopiedFileTo($user, $source, $destination) + { + $request = $this->client->createRequest( + 'MOVE', + $this->baseUrl . '/remote.php/webdav' . $source, + [ + 'auth' => [ + $user, + $this->getPasswordForUser($user), + ], + 'headers' => [ + 'Destination' => $this->baseUrl . '/remote.php/webdav' . $destination, + ], + ] + ); + $this->response = $this->client->send($request); + } + + /** + * @Then The webdav checksum should be empty + */ + public function theWebdavChecksumShouldBeEmpty() + { + $service = new Sabre\Xml\Service(); + $parsed = $service->parse($this->response->getBody()->getContents()); + + /* + * Fetch the checksum array + * Maybe we want to do this a bit cleaner ;) + */ + $status = $parsed[0]['value'][1]['value'][1]['value']; + + if ($status !== 'HTTP/1.1 404 Not Found') { + throw new \Exception("Expected 'HTTP/1.1 404 Not Found', got ".$status); + } + } + + /** + * @Then The OC-Checksum header should not be there + */ + public function theOcChecksumHeaderShouldNotBeThere() + { + if ($this->response->hasHeader('OC-Checksum')) { + throw new \Exception("Expected no checksum header but got ".$this->response->getHeader('OC-Checksum')); + } + } + + /** + * @Given user :user uploads chunk file :num of :total with :data to :destination with checksum :checksum + */ + public function userUploadsChunkFileOfWithToWithChecksum($user, $num, $total, $data, $destination, $checksum) + { + $num -= 1; + $this->response = $this->client->put( + $this->baseUrl . '/remote.php/webdav' . $destination . '-chunking-42-'.$total.'-'.$num, + [ + 'auth' => [ + $user, + $this->getPasswordForUser($user) + ], + 'body' => $data, + 'headers' => [ + 'OC-Checksum' => $checksum, + 'OC-Chunked' => '1', + ] + ] + ); + + } +} diff --git a/build/integration/features/checksums.feature b/build/integration/features/checksums.feature new file mode 100644 index 0000000000..d391e93afe --- /dev/null +++ b/build/integration/features/checksums.feature @@ -0,0 +1,76 @@ +Feature: checksums + + Scenario: Uploading a file with checksum should work + Given user "user0" exists + When user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" with checksum "MD5:d70b40f177b14b470d1756a3c12b963a" + Then The webdav response should have a status code "201" + + Scenario: Uploading a file with checksum should return the checksum in the propfind + Given user "user0" exists + And user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" with checksum "MD5:d70b40f177b14b470d1756a3c12b963a" + When user "user0" request the checksum of "/myChecksumFile.txt" via propfind + Then The webdav checksum should match "MD5:d70b40f177b14b470d1756a3c12b963a" + + Scenario: Uploading a file with checksum should return the checksum in the download header + Given user "user0" exists + And user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" with checksum "MD5:d70b40f177b14b470d1756a3c12b963a" + When user "user0" downloads the file "/myChecksumFile.txt" + Then The header checksum should match "MD5:d70b40f177b14b470d1756a3c12b963a" + + Scenario: Moving a file with checksum should return the checksum in the propfind + Given user "user0" exists + And user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" with checksum "MD5:d70b40f177b14b470d1756a3c12b963a" + When User "user0" moved file "/myChecksumFile.txt" to "/myMovedChecksumFile.txt" + And user "user0" request the checksum of "/myMovedChecksumFile.txt" via propfind + Then The webdav checksum should match "MD5:d70b40f177b14b470d1756a3c12b963a" + + Scenario: Moving file with checksum should return the checksum in the download header + Given user "user0" exists + And user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" with checksum "MD5:d70b40f177b14b470d1756a3c12b963a" + When User "user0" moved file "/myChecksumFile.txt" to "/myMovedChecksumFile.txt" + And user "user0" downloads the file "/myMovedChecksumFile.txt" + Then The header checksum should match "MD5:d70b40f177b14b470d1756a3c12b963a" + + Scenario: Copying a file with checksum should return the checksum in the propfind + Given user "user0" exists + And user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" with checksum "MD5:d70b40f177b14b470d1756a3c12b963a" + When User "user0" copied file "/myChecksumFile.txt" to "/myChecksumFileCopy.txt" + And user "user0" request the checksum of "/myChecksumFileCopy.txt" via propfind + Then The webdav checksum should match "MD5:d70b40f177b14b470d1756a3c12b963a" + + Scenario: Copying file with checksum should return the checksum in the download header + Given user "user0" exists + And user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" with checksum "MD5:d70b40f177b14b470d1756a3c12b963a" + When User "user0" copied file "/myChecksumFile.txt" to "/myChecksumFileCopy.txt" + And user "user0" downloads the file "/myChecksumFileCopy.txt" + Then The header checksum should match "MD5:d70b40f177b14b470d1756a3c12b963a" + + Scenario: Overwriting a file with checksum should remove the checksum and not return it in the propfind + Given user "user0" exists + And user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" with checksum "MD5:d70b40f177b14b470d1756a3c12b963a" + When user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" + And user "user0" request the checksum of "/myChecksumFile.txt" via propfind + Then The webdav checksum should be empty + + Scenario: Overwriting a file with checksum should remove the checksum and not return it in the download header + Given user "user0" exists + And user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" with checksum "MD5:d70b40f177b14b470d1756a3c12b963a" + When user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" + And user "user0" downloads the file "/myChecksumFile.txt" + Then The OC-Checksum header should not be there + + Scenario: Uploading a chunked file with checksum should return the checksum in the propfind + Given user "user0" exists + And user "user0" uploads chunk file "1" of "3" with "AAAAA" to "/myChecksumFile.txt" with checksum "MD5:e892fdd61a74bc89cd05673cc2e22f88" + And user "user0" uploads chunk file "2" of "3" with "BBBBB" to "/myChecksumFile.txt" with checksum "MD5:e892fdd61a74bc89cd05673cc2e22f88" + And user "user0" uploads chunk file "3" of "3" with "CCCCC" to "/myChecksumFile.txt" with checksum "MD5:e892fdd61a74bc89cd05673cc2e22f88" + When user "user0" request the checksum of "/myChecksumFile.txt" via propfind + Then The webdav checksum should match "MD5:e892fdd61a74bc89cd05673cc2e22f88" + + Scenario: Uploading a chunked file with checksum should return the checksum in the download header + Given user "user0" exists + And user "user0" uploads chunk file "1" of "3" with "AAAAA" to "/myChecksumFile.txt" with checksum "MD5:e892fdd61a74bc89cd05673cc2e22f88" + And user "user0" uploads chunk file "2" of "3" with "BBBBB" to "/myChecksumFile.txt" with checksum "MD5:e892fdd61a74bc89cd05673cc2e22f88" + And user "user0" uploads chunk file "3" of "3" with "CCCCC" to "/myChecksumFile.txt" with checksum "MD5:e892fdd61a74bc89cd05673cc2e22f88" + When user "user0" downloads the file "/myChecksumFile.txt" + Then The header checksum should match "MD5:e892fdd61a74bc89cd05673cc2e22f88" From 3e88a5067f3ef6fd81068f85a00ee2a49a8f9b79 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Sun, 28 Feb 2016 19:41:46 +0100 Subject: [PATCH 10/14] Remove checksum on upload of non checksumed file When we overwrite a checksumed file with a file without a checksum we should remove the checksum from the server. This is done by setting the column to empty. --- apps/dav/lib/connector/sabre/file.php | 2 ++ apps/dav/lib/connector/sabre/filesplugin.php | 7 ++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/apps/dav/lib/connector/sabre/file.php b/apps/dav/lib/connector/sabre/file.php index 38a1ee5f4e..b7e2108db3 100644 --- a/apps/dav/lib/connector/sabre/file.php +++ b/apps/dav/lib/connector/sabre/file.php @@ -218,6 +218,8 @@ class File extends Node implements IFile { if (isset($request->server['HTTP_OC_CHECKSUM'])) { $checksum = trim($request->server['HTTP_OC_CHECKSUM']); $this->fileView->putFileInfo($this->path, ['checksum' => $checksum]); + } else if ($this->getChecksum() !== NULL && $this->getChecksum() !== '') { + $this->fileView->putFileInfo($this->path, ['checksum' => '']); } $this->refreshInfo(); diff --git a/apps/dav/lib/connector/sabre/filesplugin.php b/apps/dav/lib/connector/sabre/filesplugin.php index eb9116d219..4b05922adf 100644 --- a/apps/dav/lib/connector/sabre/filesplugin.php +++ b/apps/dav/lib/connector/sabre/filesplugin.php @@ -27,6 +27,7 @@ namespace OCA\DAV\Connector\Sabre; +use Sabre\DAV\Exception\NotFound; use Sabre\DAV\IFile; use \Sabre\DAV\PropFind; use \Sabre\DAV\PropPatch; @@ -197,7 +198,7 @@ class FilesPlugin extends \Sabre\DAV\ServerPlugin { //Add OC-Checksum header /** @var $node File */ $checksum = $node->getChecksum(); - if ($checksum !== null) { + if ($checksum !== null && $checksum !== '') { $response->addHeader('OC-Checksum', $checksum); } } @@ -252,6 +253,10 @@ class FilesPlugin extends \Sabre\DAV\ServerPlugin { $propFind->handle(self::CHECKSUMS_PROPERTYNAME, function() use ($node) { $checksum = $node->getChecksum(); + if ($checksum === NULL || $checksum === '') { + return null; + } + return new ChecksumList($checksum); }); From ec140fa2ec2aaf489bae728971ec18e58719a38a Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Sun, 28 Feb 2016 20:21:43 +0100 Subject: [PATCH 11/14] Checksums on chunked files We should also store checksums on chunked files. We do not checksum individual chunks but only the final file. --- apps/dav/lib/connector/sabre/file.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/apps/dav/lib/connector/sabre/file.php b/apps/dav/lib/connector/sabre/file.php index b7e2108db3..b71c4ccc62 100644 --- a/apps/dav/lib/connector/sabre/file.php +++ b/apps/dav/lib/connector/sabre/file.php @@ -457,6 +457,14 @@ class File extends Node implements IFile { $this->fileView->changeLock($targetPath, ILockingProvider::LOCK_SHARED); + if (isset($request->server['HTTP_OC_CHECKSUM'])) { + $checksum = trim($request->server['HTTP_OC_CHECKSUM']); + $this->fileView->putFileInfo($targetPath, ['checksum' => $checksum]); + } else if ($this->getChecksum() !== NULL && $this->getChecksum() !== '') { + $this->fileView->putFileInfo($this->path, ['checksum' => '']); + } + $this->refreshInfo(); + $this->emitPostHooks($exists, $targetPath); $info = $this->fileView->getFileInfo($targetPath); From ac392457f24aed34dc2c5635d9ffd64ec256b061 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 29 Feb 2016 10:29:48 +0100 Subject: [PATCH 12/14] Fix unit tests --- apps/dav/lib/connector/sabre/file.php | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/apps/dav/lib/connector/sabre/file.php b/apps/dav/lib/connector/sabre/file.php index b71c4ccc62..0bf7d9cf2b 100644 --- a/apps/dav/lib/connector/sabre/file.php +++ b/apps/dav/lib/connector/sabre/file.php @@ -215,6 +215,8 @@ class File extends Node implements IFile { } } + $this->refreshInfo(); + if (isset($request->server['HTTP_OC_CHECKSUM'])) { $checksum = trim($request->server['HTTP_OC_CHECKSUM']); $this->fileView->putFileInfo($this->path, ['checksum' => $checksum]); @@ -457,18 +459,18 @@ class File extends Node implements IFile { $this->fileView->changeLock($targetPath, ILockingProvider::LOCK_SHARED); - if (isset($request->server['HTTP_OC_CHECKSUM'])) { - $checksum = trim($request->server['HTTP_OC_CHECKSUM']); - $this->fileView->putFileInfo($targetPath, ['checksum' => $checksum]); - } else if ($this->getChecksum() !== NULL && $this->getChecksum() !== '') { - $this->fileView->putFileInfo($this->path, ['checksum' => '']); - } - $this->refreshInfo(); - $this->emitPostHooks($exists, $targetPath); $info = $this->fileView->getFileInfo($targetPath); + if (isset($request->server['HTTP_OC_CHECKSUM'])) { + $checksum = trim($request->server['HTTP_OC_CHECKSUM']); + $this->fileView->putFileInfo($targetPath, ['checksum' => $checksum]); + } else if ($info->getChecksum() !== NULL && $info->getChecksum() !== '') { + $this->fileView->putFileInfo($this->path, ['checksum' => '']); + } + + $this->fileView->unlockFile($targetPath, ILockingProvider::LOCK_SHARED); return $info->getEtag(); From 57babe032b1c3abcf4e4208bb19488151a934470 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 1 Mar 2016 11:24:10 +0100 Subject: [PATCH 13/14] Save some calls to refreshInfo during upload --- apps/dav/lib/connector/sabre/file.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/apps/dav/lib/connector/sabre/file.php b/apps/dav/lib/connector/sabre/file.php index 0bf7d9cf2b..9c8344bc5d 100644 --- a/apps/dav/lib/connector/sabre/file.php +++ b/apps/dav/lib/connector/sabre/file.php @@ -220,10 +220,11 @@ class File extends Node implements IFile { if (isset($request->server['HTTP_OC_CHECKSUM'])) { $checksum = trim($request->server['HTTP_OC_CHECKSUM']); $this->fileView->putFileInfo($this->path, ['checksum' => $checksum]); - } else if ($this->getChecksum() !== NULL && $this->getChecksum() !== '') { + $this->refreshInfo(); + } else if ($this->getChecksum() !== null && $this->getChecksum() !== '') { $this->fileView->putFileInfo($this->path, ['checksum' => '']); + $this->refreshInfo(); } - $this->refreshInfo(); } catch (StorageNotAvailableException $e) { throw new ServiceUnavailable("Failed to check file size: " . $e->getMessage()); @@ -461,16 +462,16 @@ class File extends Node implements IFile { $this->emitPostHooks($exists, $targetPath); + // FIXME: should call refreshInfo but can't because $this->path is not the of the final file $info = $this->fileView->getFileInfo($targetPath); if (isset($request->server['HTTP_OC_CHECKSUM'])) { $checksum = trim($request->server['HTTP_OC_CHECKSUM']); $this->fileView->putFileInfo($targetPath, ['checksum' => $checksum]); - } else if ($info->getChecksum() !== NULL && $info->getChecksum() !== '') { + } else if ($info->getChecksum() !== null && $info->getChecksum() !== '') { $this->fileView->putFileInfo($this->path, ['checksum' => '']); } - $this->fileView->unlockFile($targetPath, ILockingProvider::LOCK_SHARED); return $info->getEtag(); From 3682a4220cce8a0b47e3aadc393acc82e036ed99 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Tue, 1 Mar 2016 13:54:46 +0100 Subject: [PATCH 14/14] local entrypoint.sh isn't needed - all fixes are upstream --- tests/objectstore/entrypoint.sh | 274 -------------------------- tests/objectstore/start-swift-ceph.sh | 3 +- 2 files changed, 1 insertion(+), 276 deletions(-) delete mode 100755 tests/objectstore/entrypoint.sh diff --git a/tests/objectstore/entrypoint.sh b/tests/objectstore/entrypoint.sh deleted file mode 100755 index 6dd6ef23fb..0000000000 --- a/tests/objectstore/entrypoint.sh +++ /dev/null @@ -1,274 +0,0 @@ -#!/bin/bash -set -e - -: ${CLUSTER:=ceph} -: ${RGW_NAME:=$(hostname -s)} -: ${MON_NAME:=$(hostname -s)} -: ${RGW_CIVETWEB_PORT:=80} -: ${OSD_SIZE:=100} - -: ${KEYSTONE_ADMIN_TOKEN:=admin} -: ${KEYSTONE_ADMIN_PORT:=35357} -: ${KEYSTONE_PUBLIC_PORT:=5001} - -: ${KEYSTONE_SERVICE:=${CLUSTER}} -: ${KEYSTONE_ENDPOINT_REGION:=region} - -: ${KEYSTONE_ADMIN_USER:=admin} -: ${KEYSTONE_ADMIN_TENANT:=admin} -: ${KEYSTONE_ADMIN_PASS:=admin} - -ip_address=$(head -n1 /etc/hosts | cut -d" " -f1) -: ${MON_IP:=${ip_address}} -subnet=$(ip route | grep "src ${ip_address}" | cut -d" " -f1) -: ${CEPH_NETWORK:=${subnet}} - -####### -# MON # -####### - -if [ ! -n "$CEPH_NETWORK" ]; then - echo "ERROR- CEPH_NETWORK must be defined as the name of the network for the OSDs" - exit 1 -fi - -if [ ! -n "$MON_IP" ]; then - echo "ERROR- MON_IP must be defined as the IP address of the monitor" - exit 1 -fi - -# bootstrap MON -if [ ! -e /etc/ceph/ceph.conf ]; then - fsid=$(uuidgen) - cat </etc/ceph/${CLUSTER}.conf -[global] -fsid = $fsid -mon initial members = ${MON_NAME} -mon host = ${MON_IP} -auth cluster required = cephx -auth service required = cephx -auth client required = cephx -osd crush chooseleaf type = 0 -osd journal size = 100 -osd pool default pg num = 8 -osd pool default pgp num = 8 -osd pool default size = 1 -public network = ${CEPH_NETWORK} -cluster network = ${CEPH_NETWORK} -debug ms = 1 - -[mon] -debug mon = 20 -debug paxos = 20 -debug auth = 20 - -[osd] -debug osd = 20 -debug filestore = 20 -debug journal = 20 -debug monc = 20 - -[mds] -debug mds = 20 -debug mds balancer = 20 -debug mds log = 20 -debug mds migrator = 20 - -[client.radosgw.gateway] -rgw keystone url = http://${MON_IP}:${KEYSTONE_ADMIN_PORT} -rgw keystone admin token = ${KEYSTONE_ADMIN_TOKEN} -rgw keystone accepted roles = _member_ -ENDHERE - - # Generate administrator key - ceph-authtool /etc/ceph/${CLUSTER}.client.admin.keyring --create-keyring --gen-key -n client.admin --set-uid=0 --cap mon 'allow *' --cap osd 'allow *' --cap mds 'allow' - - # Generate the mon. key - ceph-authtool /etc/ceph/${CLUSTER}.mon.keyring --create-keyring --gen-key -n mon. --cap mon 'allow *' - - # Generate initial monitor map - monmaptool --create --add ${MON_NAME} ${MON_IP} --fsid ${fsid} /etc/ceph/monmap -fi - -# If we don't have a monitor keyring, this is a new monitor -if [ ! -e /var/lib/ceph/mon/${CLUSTER}-${MON_NAME}/keyring ]; then - - if [ ! -e /etc/ceph/${CLUSTER}.client.admin.keyring ]; then - echo "ERROR- /etc/ceph/${CLUSTER}.client.admin.keyring must exist; get it from your existing mon" - exit 2 - fi - - if [ ! -e /etc/ceph/${CLUSTER}.mon.keyring ]; then - echo "ERROR- /etc/ceph/${CLUSTER}.mon.keyring must exist. You can extract it from your current monitor by running 'ceph auth get mon. -o /tmp/${CLUSTER}.mon.keyring'" - exit 3 - fi - - if [ ! -e /etc/ceph/monmap ]; then - echo "ERROR- /etc/ceph/monmap must exist. You can extract it from your current monitor by running 'ceph mon getmap -o /tmp/monmap'" - exit 4 - fi - - # Import the client.admin keyring and the monitor keyring into a new, temporary one - ceph-authtool /tmp/${CLUSTER}.mon.keyring --create-keyring --import-keyring /etc/ceph/${CLUSTER}.client.admin.keyring - ceph-authtool /tmp/${CLUSTER}.mon.keyring --import-keyring /etc/ceph/${CLUSTER}.mon.keyring - - # Make the monitor directory - mkdir -p /var/lib/ceph/mon/${CLUSTER}-${MON_NAME} - - # Prepare the monitor daemon's directory with the map and keyring - ceph-mon --mkfs -i ${MON_NAME} --monmap /etc/ceph/monmap --keyring /tmp/${CLUSTER}.mon.keyring - - # Clean up the temporary key - rm /tmp/${CLUSTER}.mon.keyring -fi - -# start MON -ceph-mon -i ${MON_NAME} --public-addr ${MON_IP}:6789 - -# change replica size -ceph osd pool set rbd size 1 - - -####### -# OSD # -####### - -if [ ! -e /var/lib/ceph/osd/${CLUSTER}-0/keyring ]; then - # bootstrap OSD - mkdir -p /var/lib/ceph/osd/${CLUSTER}-0 - # skip btrfs HACK if btrfs is already in place - if [ "$(stat -f /var/lib/ceph/osd/${CLUSTER}-0 2>/dev/null | grep btrfs | wc -l)" == "0" ]; then - # HACK create btrfs loopback device - echo "creating osd storage image" - dd if=/dev/zero of=/tmp/osddata bs=1M count=${OSD_SIZE} - mkfs.btrfs /tmp/osddata - echo "mounting via loopback" - mount -o loop /tmp/osddata /var/lib/ceph/osd/${CLUSTER}-0 - echo "now mounted:" - mount - # end HACK - fi - echo "creating osd" - ceph osd create - echo "creating osd filesystem" - ceph-osd -i 0 --mkfs - echo "creating osd keyring" - ceph auth get-or-create osd.0 osd 'allow *' mon 'allow profile osd' -o /var/lib/ceph/osd/${CLUSTER}-0/keyring - echo "configuring osd crush" - ceph osd crush add 0 1 root=default host=$(hostname -s) - echo "adding osd keyring" - ceph-osd -i 0 -k /var/lib/ceph/osd/${CLUSTER}-0/keyring -fi - -# start OSD -echo "starting osd" -ceph-osd --cluster=${CLUSTER} -i 0 - -#sleep 10 - -####### -# MDS # -####### - -if [ ! -e /var/lib/ceph/mds/${CLUSTER}-0/keyring ]; then - # create ceph filesystem - echo "creating osd pool" - ceph osd pool create cephfs_data 8 - echo "creating osd pool metadata" - ceph osd pool create cephfs_metadata 8 - echo "creating cephfs" - ceph fs new cephfs cephfs_metadata cephfs_data - - # bootstrap MDS - mkdir -p /var/lib/ceph/mds/${CLUSTER}-0 - echo "creating mds auth" - ceph auth get-or-create mds.0 mds 'allow' osd 'allow *' mon 'allow profile mds' > /var/lib/ceph/mds/${CLUSTER}-0/keyring -fi - -# start MDS -echo "starting mds" -ceph-mds --cluster=${CLUSTER} -i 0 - -#sleep 10 - - -####### -# RGW # -####### - -if [ ! -e /var/lib/ceph/radosgw/${RGW_NAME}/keyring ]; then - # bootstrap RGW - mkdir -p /var/lib/ceph/radosgw/${RGW_NAME} - echo "creating rgw auth" - ceph auth get-or-create client.radosgw.gateway osd 'allow rwx' mon 'allow rw' -o /var/lib/ceph/radosgw/${RGW_NAME}/keyring -fi - -# start RGW -echo "starting rgw" -radosgw -c /etc/ceph/ceph.conf -n client.radosgw.gateway -k /var/lib/ceph/radosgw/${RGW_NAME}/keyring --rgw-socket-path="" --rgw-frontends="civetweb port=${RGW_CIVETWEB_PORT}" - - -####### -# API # -####### - -# start ceph-rest-api -echo "starting rest api" -ceph-rest-api -n client.admin & - -############ -# Keystone # -############ - -if [ ! -e /etc/keystone/${CLUSTER}.conf ]; then - cat < /etc/keystone/${CLUSTER}.conf -[DEFAULT] -admin_token=${KEYSTONE_ADMIN_TOKEN} -admin_port=${KEYSTONE_ADMIN_PORT} -public_port=${KEYSTONE_PUBLIC_PORT} - -[database] -connection = sqlite:////var/lib/keystone/keystone.db -ENDHERE - - # start Keystone - echo "starting keystone" - keystone-all --config-file /etc/keystone/${CLUSTER}.conf & - - # wait until up - while ! nc ${MON_IP} ${KEYSTONE_ADMIN_PORT}