From 449011dae7ea0ec7eb8d6c78e89709982020e709 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 10 Mar 2017 15:37:21 +0100 Subject: [PATCH] remove discovery manager in favour of the OCSDiscoveryService Signed-off-by: Bjoern Schiessle --- .../lib/AppInfo/Application.php | 11 +- .../lib/BackgroundJob/RetryJob.php | 9 +- .../Controller/MountPublicLinkController.php | 6 +- .../Controller/RequestHandlerController.php | 20 +- .../lib/DiscoveryManager.php | 143 ------------ .../lib/Notifications.php | 14 +- .../RequestHandlerControllerTest.php | 6 +- .../tests/DiscoveryManagerTest.php | 217 ------------------ .../tests/NotificationsTest.php | 13 +- .../files_sharing/lib/AppInfo/Application.php | 6 +- apps/files_sharing/lib/External/Manager.php | 35 +-- apps/files_sharing/lib/External/Storage.php | 16 +- apps/files_sharing/lib/Hooks.php | 6 +- .../tests/External/ManagerTest.php | 15 +- lib/private/Share/Share.php | 8 +- lib/private/Share20/ProviderFactory.php | 6 +- lib/public/IServerContainer.php | 6 + tests/lib/Share/ShareTest.php | 12 +- 18 files changed, 81 insertions(+), 468 deletions(-) delete mode 100644 apps/federatedfilesharing/lib/DiscoveryManager.php delete mode 100644 apps/federatedfilesharing/tests/DiscoveryManagerTest.php diff --git a/apps/federatedfilesharing/lib/AppInfo/Application.php b/apps/federatedfilesharing/lib/AppInfo/Application.php index 3e97edeada..daeb049a31 100644 --- a/apps/federatedfilesharing/lib/AppInfo/Application.php +++ b/apps/federatedfilesharing/lib/AppInfo/Application.php @@ -51,10 +51,7 @@ class Application extends App { $notification = new Notifications( $addressHandler, $server->getHTTPClientService(), - new \OCA\FederatedFileSharing\DiscoveryManager( - $server->getMemCacheFactory(), - $server->getHTTPClientService() - ), + $server->getOCSDiscoveryService(), \OC::$server->getJobList() ); return new RequestHandlerController( @@ -99,14 +96,10 @@ class Application extends App { \OC::$server->getL10N('federatedfilesharing'), \OC::$server->getCloudIdManager() ); - $discoveryManager = new \OCA\FederatedFileSharing\DiscoveryManager( - \OC::$server->getMemCacheFactory(), - \OC::$server->getHTTPClientService() - ); $notifications = new \OCA\FederatedFileSharing\Notifications( $addressHandler, \OC::$server->getHTTPClientService(), - $discoveryManager, + \OC::$server->getOCSDiscoveryService(), \OC::$server->getJobList() ); $tokenHandler = new \OCA\FederatedFileSharing\TokenHandler( diff --git a/apps/federatedfilesharing/lib/BackgroundJob/RetryJob.php b/apps/federatedfilesharing/lib/BackgroundJob/RetryJob.php index 2356c569d8..8d81d56802 100644 --- a/apps/federatedfilesharing/lib/BackgroundJob/RetryJob.php +++ b/apps/federatedfilesharing/lib/BackgroundJob/RetryJob.php @@ -27,7 +27,6 @@ namespace OCA\FederatedFileSharing\BackgroundJob; use OC\BackgroundJob\Job; use OC\BackgroundJob\JobList; use OCA\FederatedFileSharing\AddressHandler; -use OCA\FederatedFileSharing\DiscoveryManager; use OCA\FederatedFileSharing\Notifications; use OCP\BackgroundJob\IJobList; use OCP\ILogger; @@ -68,14 +67,10 @@ class RetryJob extends Job { \OC::$server->getL10N('federatedfilesharing'), \OC::$server->getCloudIdManager() ); - $discoveryManager = new DiscoveryManager( - \OC::$server->getMemCacheFactory(), - \OC::$server->getHTTPClientService() - ); $this->notifications = new Notifications( $addressHandler, \OC::$server->getHTTPClientService(), - $discoveryManager, + \OC::$server->getOCSDiscoveryService(), \OC::$server->getJobList() ); } @@ -108,7 +103,7 @@ class RetryJob extends Job { $try = (int)$argument['try'] + 1; $result = $this->notifications->sendUpdateToRemote($remote, $remoteId, $token, $action, $data, $try); - + if ($result === true || $try > $this->maxTry) { $this->retainJob = false; } diff --git a/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php b/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php index dd2e88d2da..d20018f38a 100644 --- a/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php +++ b/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php @@ -248,17 +248,13 @@ class MountPublicLinkController extends Controller { if (Helper::isSameUserOnSameServer($owner, $remote, $currentUser, $currentServer)) { return new JSONResponse(['message' => $this->l->t('Not allowed to create a federated share with the owner.')], Http::STATUS_BAD_REQUEST); } - $discoveryManager = new DiscoveryManager( - \OC::$server->getMemCacheFactory(), - \OC::$server->getHTTPClientService() - ); $externalManager = new Manager( \OC::$server->getDatabaseConnection(), Filesystem::getMountManager(), Filesystem::getLoader(), \OC::$server->getHTTPClientService(), \OC::$server->getNotificationManager(), - $discoveryManager, + \OC::$server->getOCSDiscoveryService(), \OC::$server->getUserSession()->getUser()->getUID() ); diff --git a/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php b/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php index a41481afd2..fa21268022 100644 --- a/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php +++ b/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php @@ -152,19 +152,15 @@ class RequestHandlerController extends OCSController { \OC_Util::setupFS($shareWith); - $discoveryManager = new DiscoveryManager( - \OC::$server->getMemCacheFactory(), - \OC::$server->getHTTPClientService() - ); $externalManager = new \OCA\Files_Sharing\External\Manager( - \OC::$server->getDatabaseConnection(), - \OC\Files\Filesystem::getMountManager(), - \OC\Files\Filesystem::getLoader(), - \OC::$server->getHTTPClientService(), - \OC::$server->getNotificationManager(), - $discoveryManager, - $shareWith - ); + \OC::$server->getDatabaseConnection(), + \OC\Files\Filesystem::getMountManager(), + \OC\Files\Filesystem::getLoader(), + \OC::$server->getHTTPClientService(), + \OC::$server->getNotificationManager(), + \OC::$server->getOCSDiscoveryService(), + $shareWith + ); try { $externalManager->addShare($remote, $token, '', $name, $owner, false, $shareWith, $remoteId); diff --git a/apps/federatedfilesharing/lib/DiscoveryManager.php b/apps/federatedfilesharing/lib/DiscoveryManager.php deleted file mode 100644 index 8c8c72dbd6..0000000000 --- a/apps/federatedfilesharing/lib/DiscoveryManager.php +++ /dev/null @@ -1,143 +0,0 @@ - - * @author Joas Schilling - * @author Lukas Reschke - * @author Vincent Petry - * - * @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\FederatedFileSharing; - -use GuzzleHttp\Exception\ClientException; -use GuzzleHttp\Exception\ConnectException; -use OCP\Http\Client\IClient; -use OCP\Http\Client\IClientService; -use OCP\ICache; -use OCP\ICacheFactory; - -/** - * Class DiscoveryManager handles the discovery of endpoints used by Federated - * Cloud Sharing. - * - * @package OCA\FederatedFileSharing - */ -class DiscoveryManager { - /** @var ICache */ - private $cache; - /** @var IClient */ - private $client; - - /** - * @param ICacheFactory $cacheFactory - * @param IClientService $clientService - */ - public function __construct(ICacheFactory $cacheFactory, - IClientService $clientService) { - $this->cache = $cacheFactory->create('ocs-discovery'); - $this->client = $clientService->newClient(); - } - - /** - * Returns whether the specified URL includes only safe characters, if not - * returns false - * - * @param string $url - * @return bool - */ - private function isSafeUrl($url) { - return (bool)preg_match('/^[\/\.A-Za-z0-9]+$/', $url); - } - - /** - * Discover the actual data and do some naive caching to ensure that the data - * is not requested multiple times. - * - * If no valid discovery data is found the Nextcloud defaults are returned. - * - * @param string $remote - * @return array - */ - private function discover($remote) { - // Check if something is in the cache - if($cacheData = $this->cache->get($remote)) { - return json_decode($cacheData, true); - } - - // Default response body - $discoveredServices = [ - 'webdav' => '/public.php/webdav', - 'share' => '/ocs/v1.php/cloud/shares', - ]; - - // Read the data from the response body - try { - $response = $this->client->get($remote . '/ocs-provider/', [ - 'timeout' => 10, - 'connect_timeout' => 10, - ]); - if($response->getStatusCode() === 200) { - $decodedService = json_decode($response->getBody(), true); - if(is_array($decodedService)) { - $endpoints = [ - 'webdav', - 'share', - ]; - - foreach($endpoints as $endpoint) { - if(isset($decodedService['services']['FEDERATED_SHARING']['endpoints'][$endpoint])) { - $endpointUrl = (string)$decodedService['services']['FEDERATED_SHARING']['endpoints'][$endpoint]; - if($this->isSafeUrl($endpointUrl)) { - $discoveredServices[$endpoint] = $endpointUrl; - } - } - } - } - } - } catch (ClientException $e) { - // Don't throw any exception since exceptions are handled before - } catch (ConnectException $e) { - // Don't throw any exception since exceptions are handled before - } - - // Write into cache - $this->cache->set($remote, json_encode($discoveredServices)); - return $discoveredServices; - } - - /** - * Return the public WebDAV endpoint used by the specified remote - * - * @param string $host - * @return string - */ - public function getWebDavEndpoint($host) { - return $this->discover($host)['webdav']; - } - - /** - * Return the sharing endpoint used by the specified remote - * - * @param string $host - * @return string - */ - public function getShareEndpoint($host) { - return $this->discover($host)['share']; - } -} diff --git a/apps/federatedfilesharing/lib/Notifications.php b/apps/federatedfilesharing/lib/Notifications.php index 8110b4915d..5abac71198 100644 --- a/apps/federatedfilesharing/lib/Notifications.php +++ b/apps/federatedfilesharing/lib/Notifications.php @@ -30,6 +30,7 @@ namespace OCA\FederatedFileSharing; use OCP\AppFramework\Http; use OCP\BackgroundJob\IJobList; use OCP\Http\Client\IClientService; +use OCP\OCS\IDiscoveryService; class Notifications { const RESPONSE_FORMAT = 'json'; // default response format for ocs calls @@ -40,8 +41,8 @@ class Notifications { /** @var IClientService */ private $httpClientService; - /** @var DiscoveryManager */ - private $discoveryManager; + /** @var IDiscoveryService */ + private $discoveryService; /** @var IJobList */ private $jobList; @@ -49,18 +50,18 @@ class Notifications { /** * @param AddressHandler $addressHandler * @param IClientService $httpClientService - * @param DiscoveryManager $discoveryManager + * @param IDiscoveryService $discoveryService * @param IJobList $jobList */ public function __construct( AddressHandler $addressHandler, IClientService $httpClientService, - DiscoveryManager $discoveryManager, + IDiscoveryService $discoveryService, IJobList $jobList ) { $this->addressHandler = $addressHandler; $this->httpClientService = $httpClientService; - $this->discoveryManager = $discoveryManager; + $this->discoveryService = $discoveryService; $this->jobList = $jobList; } @@ -287,7 +288,8 @@ class Notifications { 'result' => '', ]; - $endpoint = $this->discoveryManager->getShareEndpoint($remoteDomain); + $federationEndpoints = $this->discoveryService->discover($remoteDomain, 'FEDERATED_SHARING'); + $endpoint = isset($federationEndpoints['share']) ? $federationEndpoints['share'] : '/ocs/v2.php/cloud/shares'; try { $response = $client->post($remoteDomain . $endpoint . $urlSuffix . '?format=' . self::RESPONSE_FORMAT, [ 'body' => $fields, diff --git a/apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php b/apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php index 233395dec9..c695fb140e 100644 --- a/apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php +++ b/apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php @@ -270,17 +270,13 @@ class RequestHandlerControllerTest extends TestCase { ->method('newClient') ->willReturn($client); - $discoveryManager = new DiscoveryManager( - \OC::$server->getMemCacheFactory(), - $httpClientService - ); $manager = new \OCA\Files_Sharing\External\Manager( \OC::$server->getDatabaseConnection(), Filesystem::getMountManager(), Filesystem::getLoader(), $httpClientService, \OC::$server->getNotificationManager(), - $discoveryManager, + \OC::$server->getOCSDiscoveryService(), $toDelete ); diff --git a/apps/federatedfilesharing/tests/DiscoveryManagerTest.php b/apps/federatedfilesharing/tests/DiscoveryManagerTest.php deleted file mode 100644 index 77e24aad54..0000000000 --- a/apps/federatedfilesharing/tests/DiscoveryManagerTest.php +++ /dev/null @@ -1,217 +0,0 @@ - - * @author Lukas Reschke - * @author Vincent Petry - * - * @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\FederatedFileSharing\Tests; - -use OCA\FederatedFileSharing\DiscoveryManager; -use OCP\Http\Client\IClient; -use OCP\Http\Client\IClientService; -use OCP\ICache; -use OCP\ICacheFactory; - -class DiscoveryManagerTest extends \Test\TestCase { - /** @var ICache */ - private $cache; - /** @var IClient */ - private $client; - /** @var DiscoveryManager */ - private $discoveryManager; - - public function setUp() { - parent::setUp(); - $this->cache = $this->getMockBuilder('\OCP\ICache') - ->getMock(); - /** @var ICacheFactory $cacheFactory */ - $cacheFactory = $this->getMockBuilder('\OCP\ICacheFactory') - ->disableOriginalConstructor()->getMock(); - $cacheFactory - ->expects($this->once()) - ->method('create') - ->with('ocs-discovery') - ->willReturn($this->cache); - - $this->client = $this->getMockBuilder('\OCP\Http\Client\IClient') - ->disableOriginalConstructor()->getMock(); - /** @var IClientService $clientService */ - $clientService = $this->getMockBuilder('\OCP\Http\Client\IClientService') - ->disableOriginalConstructor()->getMock(); - $clientService - ->expects($this->once()) - ->method('newClient') - ->willReturn($this->client); - - $this->discoveryManager = new DiscoveryManager( - $cacheFactory, - $clientService - ); - } - - public function testWithMalformedFormattedEndpointCached() { - $response = $this->getMockBuilder('\OCP\Http\Client\IResponse') - ->getMock(); - $response - ->expects($this->once()) - ->method('getStatusCode') - ->willReturn(200); - $response - ->expects($this->once()) - ->method('getBody') - ->willReturn('CertainlyNotJson'); - $this->client - ->expects($this->once()) - ->method('get') - ->with('https://myhost.com/ocs-provider/', [ - 'timeout' => 10, - 'connect_timeout' => 10, - ]) - ->willReturn($response); - $this->cache - ->expects($this->at(0)) - ->method('get') - ->with('https://myhost.com') - ->willReturn(null); - $this->cache - ->expects($this->at(1)) - ->method('set') - ->with('https://myhost.com', '{"webdav":"\/public.php\/webdav","share":"\/ocs\/v1.php\/cloud\/shares"}'); - $this->cache - ->expects($this->at(2)) - ->method('get') - ->with('https://myhost.com') - ->willReturn('{"webdav":"\/public.php\/webdav","share":"\/ocs\/v1.php\/cloud\/shares"}'); - - $this->assertSame('/public.php/webdav', $this->discoveryManager->getWebDavEndpoint('https://myhost.com')); - $this->assertSame('/ocs/v1.php/cloud/shares', $this->discoveryManager->getShareEndpoint('https://myhost.com')); - } - - public function testGetWebDavEndpointWithValidFormattedEndpointAndNotCached() { - $response = $this->getMockBuilder('\OCP\Http\Client\IResponse') - ->getMock(); - $response - ->expects($this->once()) - ->method('getStatusCode') - ->willReturn(200); - $response - ->expects($this->once()) - ->method('getBody') - ->willReturn('{"version":2,"services":{"PRIVATE_DATA":{"version":1,"endpoints":{"store":"\/ocs\/v2.php\/privatedata\/setattribute","read":"\/ocs\/v2.php\/privatedata\/getattribute","delete":"\/ocs\/v2.php\/privatedata\/deleteattribute"}},"SHARING":{"version":1,"endpoints":{"share":"\/ocs\/v2.php\/apps\/files_sharing\/api\/v1\/shares"}},"FEDERATED_SHARING":{"version":1,"endpoints":{"share":"\/ocs\/v2.php\/cloud\/shares","webdav":"\/public.php\/MyCustomEndpoint\/"}},"ACTIVITY":{"version":1,"endpoints":{"list":"\/ocs\/v2.php\/cloud\/activity"}},"PROVISIONING":{"version":1,"endpoints":{"user":"\/ocs\/v2.php\/cloud\/users","groups":"\/ocs\/v2.php\/cloud\/groups","apps":"\/ocs\/v2.php\/cloud\/apps"}}}}'); - $this->client - ->expects($this->once()) - ->method('get') - ->with('https://myhost.com/ocs-provider/', [ - 'timeout' => 10, - 'connect_timeout' => 10, - ]) - ->willReturn($response); - - $expectedResult = '/public.php/MyCustomEndpoint/'; - $this->assertSame($expectedResult, $this->discoveryManager->getWebDavEndpoint('https://myhost.com')); - } - - public function testGetWebDavEndpointWithValidFormattedEndpointWithoutDataAndNotCached() { - $response = $this->getMockBuilder('\OCP\Http\Client\IResponse') - ->getMock(); - $response - ->expects($this->once()) - ->method('getStatusCode') - ->willReturn(200); - $response - ->expects($this->once()) - ->method('getBody') - ->willReturn('{"version":2,"PRIVATE_DATA":{"version":1,"endpoints":{"store":"\/ocs\/v2.php\/privatedata\/setattribute","read":"\/ocs\/v2.php\/privatedata\/getattribute","delete":"\/ocs\/v2.php\/privatedata\/deleteattribute"}},"SHARING":{"version":1,"endpoints":{"share":"\/ocs\/v2.php\/apps\/files_sharing\/api\/v1\/shares"}},"FEDERATED_SHARING":{"version":1,"endpoints":{"share":"\/ocs\/v2.php\/cloud\/shares","webdav":"\/public.php\/MyCustomEndpoint\/"}},"ACTIVITY":{"version":1,"endpoints":{"list":"\/ocs\/v2.php\/cloud\/activity"}},"PROVISIONING":{"version":1,"endpoints":{"user":"\/ocs\/v2.php\/cloud\/users","groups":"\/ocs\/v2.php\/cloud\/groups","apps":"\/ocs\/v2.php\/cloud\/apps"}}}'); - $this->client - ->expects($this->once()) - ->method('get') - ->with('https://myhost.com/ocs-provider/', [ - 'timeout' => 10, - 'connect_timeout' => 10, - ]) - ->willReturn($response); - - $expectedResult = '/public.php/webdav'; - $this->assertSame($expectedResult, $this->discoveryManager->getWebDavEndpoint('https://myhost.com')); - } - - public function testGetShareEndpointWithValidFormattedEndpointAndNotCached() { - $response = $this->getMockBuilder('\OCP\Http\Client\IResponse') - ->getMock(); - $response - ->expects($this->once()) - ->method('getStatusCode') - ->willReturn(200); - $response - ->expects($this->once()) - ->method('getBody') - ->willReturn('{"version":2,"services":{"PRIVATE_DATA":{"version":1,"endpoints":{"store":"\/ocs\/v2.php\/privatedata\/setattribute","read":"\/ocs\/v2.php\/privatedata\/getattribute","delete":"\/ocs\/v2.php\/privatedata\/deleteattribute"}},"SHARING":{"version":1,"endpoints":{"share":"\/ocs\/v2.php\/apps\/files_sharing\/api\/v1\/shares"}},"FEDERATED_SHARING":{"version":1,"endpoints":{"share":"\/ocs\/v2.php\/cloud\/MyCustomShareEndpoint","webdav":"\/public.php\/MyCustomEndpoint\/"}},"ACTIVITY":{"version":1,"endpoints":{"list":"\/ocs\/v2.php\/cloud\/activity"}},"PROVISIONING":{"version":1,"endpoints":{"user":"\/ocs\/v2.php\/cloud\/users","groups":"\/ocs\/v2.php\/cloud\/groups","apps":"\/ocs\/v2.php\/cloud\/apps"}}}}'); - $this->client - ->expects($this->once()) - ->method('get') - ->with('https://myhost.com/ocs-provider/', [ - 'timeout' => 10, - 'connect_timeout' => 10, - ]) - ->willReturn($response); - - $expectedResult = '/ocs/v2.php/cloud/MyCustomShareEndpoint'; - $this->assertSame($expectedResult, $this->discoveryManager->getShareEndpoint('https://myhost.com')); - } - - public function testWithMaliciousEndpointCached() { - $response = $this->getMockBuilder('\OCP\Http\Client\IResponse') - ->getMock(); - $response - ->expects($this->once()) - ->method('getStatusCode') - ->willReturn(200); - $response - ->expects($this->once()) - ->method('getBody') - ->willReturn('{"version":2,"services":{"PRIVATE_DATA":{"version":1,"endpoints":{"store":"\/ocs\/v2.php\/privatedata\/setattribute","read":"\/ocs\/v2.php\/privatedata\/getattribute","delete":"\/ocs\/v2.php\/privatedata\/deleteattribute"}},"SHARING":{"version":1,"endpoints":{"share":"\/ocs\/v2.php\/apps\/files_sharing\/api\/v1\/shares"}},"FEDERATED_SHARING":{"version":1,"endpoints":{"share":"\/ocs\/v2.php\/cl@oud\/MyCustomShareEndpoint","webdav":"\/public.php\/MyC:ustomEndpoint\/"}},"ACTIVITY":{"version":1,"endpoints":{"list":"\/ocs\/v2.php\/cloud\/activity"}},"PROVISIONING":{"version":1,"endpoints":{"user":"\/ocs\/v2.php\/cloud\/users","groups":"\/ocs\/v2.php\/cloud\/groups","apps":"\/ocs\/v2.php\/cloud\/apps"}}}}'); - $this->client - ->expects($this->once()) - ->method('get') - ->with('https://myhost.com/ocs-provider/', [ - 'timeout' => 10, - 'connect_timeout' => 10, - ]) - ->willReturn($response); - $this->cache - ->expects($this->at(0)) - ->method('get') - ->with('https://myhost.com') - ->willReturn(null); - $this->cache - ->expects($this->at(1)) - ->method('set') - ->with('https://myhost.com', '{"webdav":"\/public.php\/webdav","share":"\/ocs\/v1.php\/cloud\/shares"}'); - $this->cache - ->expects($this->at(2)) - ->method('get') - ->with('https://myhost.com') - ->willReturn('{"webdav":"\/public.php\/webdav","share":"\/ocs\/v1.php\/cloud\/shares"}'); - - $this->assertSame('/public.php/webdav', $this->discoveryManager->getWebDavEndpoint('https://myhost.com')); - $this->assertSame('/ocs/v1.php/cloud/shares', $this->discoveryManager->getShareEndpoint('https://myhost.com')); - } -} diff --git a/apps/federatedfilesharing/tests/NotificationsTest.php b/apps/federatedfilesharing/tests/NotificationsTest.php index a5f5c6bc07..4f70d5f395 100644 --- a/apps/federatedfilesharing/tests/NotificationsTest.php +++ b/apps/federatedfilesharing/tests/NotificationsTest.php @@ -25,10 +25,10 @@ namespace OCA\FederatedFileSharing\Tests; use OCA\FederatedFileSharing\AddressHandler; -use OCA\FederatedFileSharing\DiscoveryManager; use OCA\FederatedFileSharing\Notifications; use OCP\BackgroundJob\IJobList; use OCP\Http\Client\IClientService; +use OCP\OCS\IDiscoveryService; class NotificationsTest extends \Test\TestCase { @@ -38,8 +38,8 @@ class NotificationsTest extends \Test\TestCase { /** @var IClientService | \PHPUnit_Framework_MockObject_MockObject*/ private $httpClientService; - /** @var DiscoveryManager | \PHPUnit_Framework_MockObject_MockObject */ - private $discoveryManager; + /** @var IDiscoveryService | \PHPUnit_Framework_MockObject_MockObject */ + private $discoveryService; /** @var IJobList | \PHPUnit_Framework_MockObject_MockObject */ private $jobList; @@ -48,8 +48,7 @@ class NotificationsTest extends \Test\TestCase { parent::setUp(); $this->jobList = $this->getMockBuilder('OCP\BackgroundJob\IJobList')->getMock(); - $this->discoveryManager = $this->getMockBuilder('OCA\FederatedFileSharing\DiscoveryManager') - ->disableOriginalConstructor()->getMock(); + $this->discoveryService = $this->getMockBuilder(IDiscoveryService::class)->getMock(); $this->httpClientService = $this->getMockBuilder('OCP\Http\Client\IClientService')->getMock(); $this->addressHandler = $this->getMockBuilder('OCA\FederatedFileSharing\AddressHandler') ->disableOriginalConstructor()->getMock(); @@ -67,7 +66,7 @@ class NotificationsTest extends \Test\TestCase { $instance = new Notifications( $this->addressHandler, $this->httpClientService, - $this->discoveryManager, + $this->discoveryService, $this->jobList ); } else { @@ -76,7 +75,7 @@ class NotificationsTest extends \Test\TestCase { [ $this->addressHandler, $this->httpClientService, - $this->discoveryManager, + $this->discoveryService, $this->jobList ] )->setMethods($mockedMethods)->getMock(); diff --git a/apps/files_sharing/lib/AppInfo/Application.php b/apps/files_sharing/lib/AppInfo/Application.php index f9540df3ff..d4e83ce5d6 100644 --- a/apps/files_sharing/lib/AppInfo/Application.php +++ b/apps/files_sharing/lib/AppInfo/Application.php @@ -92,17 +92,13 @@ class Application extends App { $container->registerService('ExternalManager', function (SimpleContainer $c) use ($server) { $user = $server->getUserSession()->getUser(); $uid = $user ? $user->getUID() : null; - $discoveryManager = new DiscoveryManager( - \OC::$server->getMemCacheFactory(), - \OC::$server->getHTTPClientService() - ); return new \OCA\Files_Sharing\External\Manager( $server->getDatabaseConnection(), \OC\Files\Filesystem::getMountManager(), \OC\Files\Filesystem::getLoader(), $server->getHTTPClientService(), $server->getNotificationManager(), - $discoveryManager, + $server->getOCSDiscoveryService(), $uid ); }); diff --git a/apps/files_sharing/lib/External/Manager.php b/apps/files_sharing/lib/External/Manager.php index 0a57324c32..2c34890738 100644 --- a/apps/files_sharing/lib/External/Manager.php +++ b/apps/files_sharing/lib/External/Manager.php @@ -32,8 +32,11 @@ namespace OCA\Files_Sharing\External; use OC\Files\Filesystem; use OCA\FederatedFileSharing\DiscoveryManager; use OCP\Files; +use OCP\Files\Storage\IStorageFactory; use OCP\Http\Client\IClientService; +use OCP\IDBConnection; use OCP\Notification\IManager; +use OCP\OCS\IDiscoveryService; class Manager { const STORAGE = '\OCA\Files_Sharing\External\Storage'; @@ -44,7 +47,7 @@ class Manager { private $uid; /** - * @var \OCP\IDBConnection + * @var IDBConnection */ private $connection; @@ -54,7 +57,7 @@ class Manager { private $mountManager; /** - * @var \OCP\Files\Storage\IStorageFactory + * @var IStorageFactory */ private $storageLoader; @@ -67,24 +70,27 @@ class Manager { * @var IManager */ private $notificationManager; - /** @var DiscoveryManager */ - private $discoveryManager; /** - * @param \OCP\IDBConnection $connection + * @var IDiscoveryService + */ + private $discoveryService; + + /** + * @param IDBConnection $connection * @param \OC\Files\Mount\Manager $mountManager - * @param \OCP\Files\Storage\IStorageFactory $storageLoader + * @param IStorageFactory $storageLoader * @param IClientService $clientService * @param IManager $notificationManager - * @param DiscoveryManager $discoveryManager + * @param IDiscoveryService $discoveryService * @param string $uid */ - public function __construct(\OCP\IDBConnection $connection, + public function __construct(IDBConnection $connection, \OC\Files\Mount\Manager $mountManager, - \OCP\Files\Storage\IStorageFactory $storageLoader, + IStorageFactory $storageLoader, IClientService $clientService, IManager $notificationManager, - DiscoveryManager $discoveryManager, + IDiscoveryService $discoveryService, $uid) { $this->connection = $connection; $this->mountManager = $mountManager; @@ -92,7 +98,7 @@ class Manager { $this->clientService = $clientService; $this->uid = $uid; $this->notificationManager = $notificationManager; - $this->discoveryManager = $discoveryManager; + $this->discoveryService = $discoveryService; } /** @@ -260,7 +266,10 @@ class Manager { */ private function sendFeedbackToRemote($remote, $token, $remoteId, $feedback) { - $url = rtrim($remote, '/') . $this->discoveryManager->getShareEndpoint($remote) . '/' . $remoteId . '/' . $feedback . '?format=' . \OCP\Share::RESPONSE_FORMAT; + $federationEndpoints = $this->discoveryService->discover($remote, 'FEDERATED_SHARING'); + $endpoint = isset($federationEndpoints['share']) ? $federationEndpoints['share'] : '/ocs/v2.php/cloud/shares'; + + $url = rtrim($remote, '/') . $endpoint . '/' . $remoteId . '/' . $feedback . '?format=' . \OCP\Share::RESPONSE_FORMAT; $fields = array('token' => $token); $client = $this->clientService->newClient(); @@ -376,7 +385,7 @@ class Manager { /** * remove re-shares from share table and mapping in the federated_reshares table - * + * * @param $mountPointId */ protected function removeReShares($mountPointId) { diff --git a/apps/files_sharing/lib/External/Storage.php b/apps/files_sharing/lib/External/Storage.php index 51d97388db..7e0f1fd0b7 100644 --- a/apps/files_sharing/lib/External/Storage.php +++ b/apps/files_sharing/lib/External/Storage.php @@ -51,8 +51,6 @@ class Storage extends DAV implements ISharedStorage { private $memcacheFactory; /** @var \OCP\Http\Client\IClientService */ private $httpClient; - /** @var \OCP\ICertificateManager */ - private $certificateManager; /** @var bool */ private $updateChecked = false; @@ -64,14 +62,11 @@ class Storage extends DAV implements ISharedStorage { public function __construct($options) { $this->memcacheFactory = \OC::$server->getMemCacheFactory(); $this->httpClient = $options['HttpClientService']; - $discoveryManager = new DiscoveryManager( - $this->memcacheFactory, - $this->httpClient - ); $this->manager = $options['manager']; - $this->certificateManager = $options['certificateManager']; $this->cloudId = $options['cloudId']; + $discoveryService = \OC::$server->getOCSDiscoveryService(); + list($protocol, $remote) = explode('://', $this->cloudId->getRemote()); if (strpos($remote, '/')) { list($host, $root) = explode('/', $remote, 2); @@ -80,9 +75,12 @@ class Storage extends DAV implements ISharedStorage { $root = ''; } $secure = $protocol === 'https'; - $root = rtrim($root, '/') . $discoveryManager->getWebDavEndpoint($this->cloudId->getRemote()); + $federatedSharingEndpoints = $discoveryService->discover($this->cloudId->getRemote(), 'FEDERATED_SHARING'); + $webDavEndpoint = isset($federatedSharingEndpoints['webdav']) ? $federatedSharingEndpoints['webdav'] : '/public.php/webdav'; + $root = rtrim($root, '/') . $webDavEndpoint; $this->mountPoint = $options['mountpoint']; $this->token = $options['token']; + parent::__construct(array( 'secure' => $secure, 'host' => $host, @@ -350,7 +348,7 @@ class Storage extends DAV implements ISharedStorage { } return ($this->getPermissions($path) & \OCP\Constants::PERMISSION_SHARE); } - + public function getPermissions($path) { $response = $this->propfind($path); if (isset($response['{http://open-collaboration-services.org/ns}share-permissions'])) { diff --git a/apps/files_sharing/lib/Hooks.php b/apps/files_sharing/lib/Hooks.php index 2029e97d08..8c5fc6a8d6 100644 --- a/apps/files_sharing/lib/Hooks.php +++ b/apps/files_sharing/lib/Hooks.php @@ -32,17 +32,13 @@ use OCA\FederatedFileSharing\DiscoveryManager; class Hooks { public static function deleteUser($params) { - $discoveryManager = new DiscoveryManager( - \OC::$server->getMemCacheFactory(), - \OC::$server->getHTTPClientService() - ); $manager = new External\Manager( \OC::$server->getDatabaseConnection(), \OC\Files\Filesystem::getMountManager(), \OC\Files\Filesystem::getLoader(), \OC::$server->getHTTPClientService(), \OC::$server->getNotificationManager(), - $discoveryManager, + \OC::$server->getOCSDiscoveryService(), $params['uid']); $manager->removeUserShares($params['uid']); diff --git a/apps/files_sharing/tests/External/ManagerTest.php b/apps/files_sharing/tests/External/ManagerTest.php index f2a55babcc..efe4065e38 100644 --- a/apps/files_sharing/tests/External/ManagerTest.php +++ b/apps/files_sharing/tests/External/ManagerTest.php @@ -70,17 +70,14 @@ class ManagerTest extends TestCase { $this->mountManager = new \OC\Files\Mount\Manager(); $this->clientService = $this->getMockBuilder('\OCP\Http\Client\IClientService') ->disableOriginalConstructor()->getMock(); - $discoveryManager = new DiscoveryManager( - \OC::$server->getMemCacheFactory(), - \OC::$server->getHTTPClientService() - ); + $this->manager = new Manager( \OC::$server->getDatabaseConnection(), $this->mountManager, new StorageFactory(), $this->clientService, \OC::$server->getNotificationManager(), - $discoveryManager, + \OC::$server->getOCSDiscoveryService(), $this->uid ); $this->testMountProvider = new MountProvider(\OC::$server->getDatabaseConnection(), function() { @@ -143,7 +140,7 @@ class ManagerTest extends TestCase { ->disableOriginalConstructor()->getMock(); $client->expects($this->once()) ->method('post') - ->with($this->stringStartsWith('http://localhost/ocs/v1.php/cloud/shares/' . $openShares[0]['remote_id']), $this->anything()) + ->with($this->stringStartsWith('http://localhost/ocs/v2.php/cloud/shares/' . $openShares[0]['remote_id']), $this->anything()) ->willReturn($response); // Accept the first share @@ -186,7 +183,7 @@ class ManagerTest extends TestCase { ->disableOriginalConstructor()->getMock(); $client->expects($this->once()) ->method('post') - ->with($this->stringStartsWith('http://localhost/ocs/v1.php/cloud/shares/' . $openShares[1]['remote_id'] . '/decline'), $this->anything()) + ->with($this->stringStartsWith('http://localhost/ocs/v2.php/cloud/shares/' . $openShares[1]['remote_id'] . '/decline'), $this->anything()) ->willReturn($response); // Decline the third share @@ -226,11 +223,11 @@ class ManagerTest extends TestCase { ->disableOriginalConstructor()->getMock(); $client1->expects($this->once()) ->method('post') - ->with($this->stringStartsWith('http://localhost/ocs/v1.php/cloud/shares/' . $openShares[0]['remote_id'] . '/decline'), $this->anything()) + ->with($this->stringStartsWith('http://localhost/ocs/v2.php/cloud/shares/' . $openShares[0]['remote_id'] . '/decline'), $this->anything()) ->willReturn($response); $client2->expects($this->once()) ->method('post') - ->with($this->stringStartsWith('http://localhost/ocs/v1.php/cloud/shares/' . $acceptedShares[0]['remote_id'] . '/decline'), $this->anything()) + ->with($this->stringStartsWith('http://localhost/ocs/v2.php/cloud/shares/' . $acceptedShares[0]['remote_id'] . '/decline'), $this->anything()) ->willReturn($response); $this->manager->removeUserShares($this->uid); diff --git a/lib/private/Share/Share.php b/lib/private/Share/Share.php index 924e2f6839..2b4a57b89a 100644 --- a/lib/private/Share/Share.php +++ b/lib/private/Share/Share.php @@ -2742,12 +2742,10 @@ class Share extends Constants { 'result' => '', ]; $try = 0; - $discoveryManager = new DiscoveryManager( - \OC::$server->getMemCacheFactory(), - \OC::$server->getHTTPClientService() - ); + $discoveryService = \OC::$server->getOCSDiscoveryService(); while ($result['success'] === false && $try < 2) { - $endpoint = $discoveryManager->getShareEndpoint($protocol . $remoteDomain); + $federationEndpoints = $discoveryService->discover($protocol . $remoteDomain, 'FEDERATED_SHARING'); + $endpoint = isset($federationEndpoints['share']) ? $federationEndpoints['share'] : '/ocs/v2.php/cloud/shares'; $result = \OC::$server->getHTTPHelper()->post($protocol . $remoteDomain . $endpoint . $urlSuffix . '?format=' . self::RESPONSE_FORMAT, $fields); $try++; $protocol = 'http://'; diff --git a/lib/private/Share20/ProviderFactory.php b/lib/private/Share20/ProviderFactory.php index b411f42b26..5eb34a483e 100644 --- a/lib/private/Share20/ProviderFactory.php +++ b/lib/private/Share20/ProviderFactory.php @@ -104,14 +104,10 @@ class ProviderFactory implements IProviderFactory { $l, $this->serverContainer->getCloudIdManager() ); - $discoveryManager = new DiscoveryManager( - $this->serverContainer->getMemCacheFactory(), - $this->serverContainer->getHTTPClientService() - ); $notifications = new Notifications( $addressHandler, $this->serverContainer->getHTTPClientService(), - $discoveryManager, + $this->serverContainer->getOCSDiscoveryService(), $this->serverContainer->getJobList() ); $tokenHandler = new TokenHandler( diff --git a/lib/public/IServerContainer.php b/lib/public/IServerContainer.php index 8c74c05d80..4d084aae79 100644 --- a/lib/public/IServerContainer.php +++ b/lib/public/IServerContainer.php @@ -531,4 +531,10 @@ interface IServerContainer extends IContainer { * @since 12.0.0 */ public function getCloudIdManager(); + + /** + * @return \OC\OCS\DiscoveryService + * @since 12.0.0 + */ + public function getOCSDiscoveryService(); } diff --git a/tests/lib/Share/ShareTest.php b/tests/lib/Share/ShareTest.php index 6b0ebd76ed..40e88d597a 100644 --- a/tests/lib/Share/ShareTest.php +++ b/tests/lib/Share/ShareTest.php @@ -1064,11 +1064,11 @@ class ShareTest extends \Test\TestCase { $httpHelperMock->expects($this->at(0)) ->method('post') - ->with($this->stringStartsWith('https://' . $urlHost . '/ocs/v1.php/cloud/shares'), $this->anything()) + ->with($this->stringStartsWith('https://' . $urlHost . '/ocs/v2.php/cloud/shares'), $this->anything()) ->willReturn(['success' => false, 'result' => 'Exception']); $httpHelperMock->expects($this->at(1)) ->method('post') - ->with($this->stringStartsWith('http://' . $urlHost . '/ocs/v1.php/cloud/shares'), $this->anything()) + ->with($this->stringStartsWith('http://' . $urlHost . '/ocs/v2.php/cloud/shares'), $this->anything()) ->willReturn(['success' => true, 'result' => json_encode(['ocs' => ['meta' => ['statuscode' => 100]]])]); \OCP\Share::shareItem('test', 'test.txt', \OCP\Share::SHARE_TYPE_REMOTE, $shareWith, \OCP\Constants::PERMISSION_READ); @@ -1077,11 +1077,11 @@ class ShareTest extends \Test\TestCase { $httpHelperMock->expects($this->at(0)) ->method('post') - ->with($this->stringStartsWith('https://' . $urlHost . '/ocs/v1.php/cloud/shares/' . $share['id'] . '/unshare'), $this->anything()) + ->with($this->stringStartsWith('https://' . $urlHost . '/ocs/v2.php/cloud/shares/' . $share['id'] . '/unshare'), $this->anything()) ->willReturn(['success' => false, 'result' => 'Exception']); $httpHelperMock->expects($this->at(1)) ->method('post') - ->with($this->stringStartsWith('http://' . $urlHost . '/ocs/v1.php/cloud/shares/' . $share['id'] . '/unshare'), $this->anything()) + ->with($this->stringStartsWith('http://' . $urlHost . '/ocs/v2.php/cloud/shares/' . $share['id'] . '/unshare'), $this->anything()) ->willReturn(['success' => true, 'result' => json_encode(['ocs' => ['meta' => ['statuscode' => 100]]])]); \OCP\Share::unshare('test', 'test.txt', \OCP\Share::SHARE_TYPE_REMOTE, $shareWith); @@ -1490,7 +1490,7 @@ class ShareTest extends \Test\TestCase { $httpHelperMock->expects($this->at(0)) ->method('post') - ->with($this->stringStartsWith('https://localhost/ocs/v1.php/cloud/shares'), $this->anything()) + ->with($this->stringStartsWith('https://localhost/ocs/v2.php/cloud/shares'), $this->anything()) ->willReturn(['success' => true, 'result' => json_encode(['ocs' => ['meta' => ['statuscode' => 100]]])]); \OCP\Share::shareItem('test', 'test.txt', \OCP\Share::SHARE_TYPE_REMOTE, 'foo@localhost', \OCP\Constants::PERMISSION_READ); @@ -1507,7 +1507,7 @@ class ShareTest extends \Test\TestCase { $httpHelperMock->expects($this->at(0)) ->method('post') - ->with($this->stringStartsWith('https://localhost/ocs/v1.php/cloud/shares/' . $share['id'] . '/unshare'), $this->anything()) + ->with($this->stringStartsWith('https://localhost/ocs/v2.php/cloud/shares/' . $share['id'] . '/unshare'), $this->anything()) ->willReturn(['success' => true, 'result' => json_encode(['ocs' => ['meta' => ['statuscode' => 100]]])]); \OCP\Share::unshare('test', 'test.txt', \OCP\Share::SHARE_TYPE_REMOTE, 'foo@localhost');