Merge pull request #3614 from nextcloud/discover-federatedsharing-endpoints

Discover federatedsharing endpoints

apps/dav/lib/CardDAV/SyncService.php

@@ -75,6 +75,7 @@ class SyncService {
 	/**
 	 * @param string $url
 	 * @param string $userName
+	 * @param string $addressBookUrl
 	 * @param string $sharedSecret
 	 * @param string $syncToken
 	 * @param int $targetBookId
@@ -83,14 +84,14 @@ class SyncService {
 	 * @return string
 	 * @throws \Exception
 	 */
-	public function syncRemoteAddressBook($url, $userName, $sharedSecret, $syncToken, $targetBookId, $targetPrincipal, $targetProperties) {
+	public function syncRemoteAddressBook($url, $userName, $addressBookUrl, $sharedSecret, $syncToken, $targetBookId, $targetPrincipal, $targetProperties) {
 		// 1. create addressbook
 		$book = $this->ensureSystemAddressBookExists($targetPrincipal, $targetBookId, $targetProperties);
 		$addressBookId = $book['id'];
 
 		// 2. query changes
 		try {
-			$response = $this->requestSyncReport($url, $userName, $sharedSecret, $syncToken);
+			$response = $this->requestSyncReport($url, $userName, $addressBookUrl, $sharedSecret, $syncToken);
 		} catch (ClientHttpException $ex) {
 			if ($ex->getCode() === Http::STATUS_UNAUTHORIZED) {
 				// remote server revoked access to the address book, remove it
@@ -105,7 +106,7 @@ class SyncService {
 		foreach ($response['response'] as $resource => $status) {
 			$cardUri = basename($resource);
 			if (isset($status[200])) {
-				$vCard = $this->download($url, $sharedSecret, $resource);
+				$vCard = $this->download($url, $userName, $sharedSecret, $resource);
 				$existingCard = $this->backend->getCard($addressBookId, $cardUri);
 				if ($existingCard === false) {
 					$this->backend->createCard($addressBookId, $cardUri, $vCard['body']);
@@ -162,6 +163,7 @@ class SyncService {
 	/**
 	 * @param string $url
 	 * @param string $userName
+	 * @param string $addressBookUrl
 	 * @param string $sharedSecret
 	 * @return Client
 	 */
@@ -185,31 +187,32 @@ class SyncService {
 	/**
 	 * @param string $url
 	 * @param string $userName
+	 * @param string $addressBookUrl
 	 * @param string $sharedSecret
 	 * @param string $syncToken
 	 * @return array
 	 */
-	protected function requestSyncReport($url, $userName, $sharedSecret, $syncToken) {
-		$client = $this->getClient($url, $userName, $sharedSecret);
+	 protected function requestSyncReport($url, $userName, $addressBookUrl, $sharedSecret, $syncToken) {
+		 $client = $this->getClient($url, $userName, $sharedSecret);
 
-		$addressBookUrl = "remote.php/dav/addressbooks/system/system/system";
-		$body = $this->buildSyncCollectionRequestBody($syncToken);
+		 $body = $this->buildSyncCollectionRequestBody($syncToken);
 
-		$response = $client->request('REPORT', $addressBookUrl, $body, [
-			'Content-Type' => 'application/xml'
-		]);
+		 $response = $client->request('REPORT', $addressBookUrl, $body, [
+			 'Content-Type' => 'application/xml'
+		 ]);
 
-		return $this->parseMultiStatus($response['body']);
-	}
+		 return $this->parseMultiStatus($response['body']);
+	 }
 
 	/**
 	 * @param string $url
+	 * @param string $userName
 	 * @param string $sharedSecret
 	 * @param string $resourcePath
 	 * @return array
 	 */
-	protected function download($url, $sharedSecret, $resourcePath) {
-		$client = $this->getClient($url, 'system', $sharedSecret);
+	protected function download($url, $userName, $sharedSecret, $resourcePath) {
+		$client = $this->getClient($url, $userName, $sharedSecret);
 		return $client->request('GET', $resourcePath);
 	}
 

apps/dav/tests/unit/CardDAV/SyncServiceTest.php

@@ -37,7 +37,7 @@ class SyncServiceTest extends TestCase {
 		$backend = $this->getBackendMock(0, 0, 0);
 
 		$ss = $this->getSyncServiceMock($backend, []);
-		$return = $ss->syncRemoteAddressBook('', 'system', '1234567890', null, '1', 'principals/system/system', []);
+		$return = $ss->syncRemoteAddressBook('', 'system', 'system', '1234567890', null, '1', 'principals/system/system', []);
 		$this->assertEquals('sync-token-1', $return);
 	}
 
@@ -46,7 +46,7 @@ class SyncServiceTest extends TestCase {
 		$backend->method('getCard')->willReturn(false);
 
 		$ss = $this->getSyncServiceMock($backend, ['0' => [200 => '']]);
-		$return = $ss->syncRemoteAddressBook('', 'system', '1234567890', null, '1', 'principals/system/system', []);
+		$return = $ss->syncRemoteAddressBook('', 'system', 'system', '1234567890', null, '1', 'principals/system/system', []);
 		$this->assertEquals('sync-token-1', $return);
 	}
 
@@ -55,7 +55,7 @@ class SyncServiceTest extends TestCase {
 		$backend->method('getCard')->willReturn(true);
 
 		$ss = $this->getSyncServiceMock($backend, ['0' => [200 => '']]);
-		$return = $ss->syncRemoteAddressBook('', 'system', '1234567890', null, '1', 'principals/system/system', []);
+		$return = $ss->syncRemoteAddressBook('', 'system', 'system', '1234567890', null, '1', 'principals/system/system', []);
 		$this->assertEquals('sync-token-1', $return);
 	}
 
@@ -63,7 +63,7 @@ class SyncServiceTest extends TestCase {
 		$backend = $this->getBackendMock(0, 0, 1);
 
 		$ss = $this->getSyncServiceMock($backend, ['0' => [404 => '']]);
-		$return = $ss->syncRemoteAddressBook('', 'system', '1234567890', null, '1', 'principals/system/system', []);
+		$return = $ss->syncRemoteAddressBook('', 'system', 'system', '1234567890', null, '1', 'principals/system/system', []);
 		$this->assertEquals('sync-token-1', $return);
 	}
 

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->query(\OCP\OCS\IDiscoveryService::class),
 				\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->query(\OCP\OCS\IDiscoveryService::class),
 			\OC::$server->getJobList()
 		);
 		$tokenHandler = new \OCA\FederatedFileSharing\TokenHandler(

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->query(\OCP\OCS\IDiscoveryService::class),
 				\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;
 		}

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->query(\OCP\OCS\IDiscoveryService::class),
 			\OC::$server->getUserSession()->getUser()->getUID()
 		);
 

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->query(\OCP\OCS\IDiscoveryService::class),
+					$shareWith
+				);
 
 			try {
 				$externalManager->addShare($remote, $token, '', $name, $owner, false, $shareWith, $remoteId);

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,

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->query(\OCP\OCS\IDiscoveryService::class),
 			$toDelete
 		);
 

apps/federatedfilesharing/tests/DiscoveryManagerTest.php

-<?php
-/**
- * @copyright Copyright (c) 2016, ownCloud, Inc.
- *
- * @author Björn Schießle <bjoern@schiessle.org>
- * @author Lukas Reschke <lukas@statuscode.ch>
- * @author Vincent Petry <pvince81@owncloud.com>
- *
- * @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 <http://www.gnu.org/licenses/>
- *
- */
-
-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'));
-	}
-}

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();

apps/federation/appinfo/routes.php

@@ -45,12 +45,12 @@ $application->registerRoutes(
 		'ocs' => [
 			// old endpoints, only used by Nextcloud and ownCloud
 			[
-				'name' => 'OCSAuthAPI#getSharedSecret',
+				'name' => 'OCSAuthAPI#getSharedSecretLegacy',
 				'url' => '/api/v1/shared-secret',
 				'verb' => 'GET',
 			],
 			[
-				'name' => 'OCSAuthAPI#requestSharedSecret',
+				'name' => 'OCSAuthAPI#requestSharedSecretLegacy',
 				'url' => '/api/v1/request-shared-secret',
 				'verb' => 'POST',
 			],

apps/federation/lib/AppInfo/Application.php

@@ -135,7 +135,8 @@ class Application extends \OCP\AppFramework\App {
 	public function getSyncService() {
 		$syncService = \OC::$server->query('CardDAVSyncService');
 		$dbHandler = $this->getContainer()->query('DbHandler');
-		return new SyncFederationAddressBooks($dbHandler, $syncService);
+		$discoveryService = \OC::$server->query(\OCP\OCS\IDiscoveryService::class);
+		return new SyncFederationAddressBooks($dbHandler, $syncService, $discoveryService);
 	}
 
 }

apps/federation/lib/BackgroundJob/GetSharedSecret.php

@@ -37,6 +37,7 @@ use OCP\Http\Client\IClient;
 use OCP\Http\Client\IResponse;
 use OCP\ILogger;
 use OCP\IURLGenerator;
+use OCP\OCS\IDiscoveryService;
 
 /**
  * Class GetSharedSecret
@@ -62,13 +63,18 @@ class GetSharedSecret extends Job{
 	/** @var DbHandler */
 	private $dbHandler;
 
+	/** @var IDiscoveryService  */
+	private $ocsDiscoveryService;
+
 	/** @var ILogger */
 	private $logger;
 
 	/** @var bool */
 	protected $retainJob = false;
 
-	private $endPoint = '/ocs/v2.php/apps/federation/api/v1/shared-secret?format=json';
+	private $format = '?format=json';
+
+	private $defaultEndPoint = '/ocs/v2.php/apps/federation/api/v1/shared-secret';
 
 	/**
 	 * RequestSharedSecret constructor.
@@ -79,6 +85,7 @@ class GetSharedSecret extends Job{
 	 * @param TrustedServers $trustedServers
 	 * @param ILogger $logger
 	 * @param DbHandler $dbHandler
+	 * @param IDiscoveryService $ocsDiscoveryService
 	 */
 	public function __construct(
 		IClient $httpClient = null,
@@ -86,13 +93,15 @@ class GetSharedSecret extends Job{
 		IJobList $jobList = null,
 		TrustedServers $trustedServers = null,
 		ILogger $logger = null,
-		DbHandler $dbHandler = null
+		DbHandler $dbHandler = null,
+		IDiscoveryService $ocsDiscoveryService = null
 	) {
 		$this->logger = $logger ? $logger : \OC::$server->getLogger();
 		$this->httpClient = $httpClient ? $httpClient : \OC::$server->getHTTPClientService()->newClient();
 		$this->jobList = $jobList ? $jobList : \OC::$server->getJobList();
 		$this->urlGenerator = $urlGenerator ? $urlGenerator : \OC::$server->getURLGenerator();
 		$this->dbHandler = $dbHandler ? $dbHandler : new DbHandler(\OC::$server->getDatabaseConnection(), \OC::$server->getL10N('federation'));
+		$this->ocsDiscoveryService = $ocsDiscoveryService ? $ocsDiscoveryService : \OC::$server->query(\OCP\OCS\IDiscoveryService::class);
 		if ($trustedServers) {
 			$this->trustedServers = $trustedServers;
 		} else {
@@ -142,10 +151,16 @@ class GetSharedSecret extends Job{
 		$source = rtrim($source, '/');
 		$token = $argument['token'];
 
+		$endPoints = $this->ocsDiscoveryService->discover($target, 'FEDERATED_SHARING');
+		$endPoint = isset($endPoints['shared-secret']) ? $endPoints['shared-secret'] : $this->defaultEndPoint;
+
+		// make sure that we have a well formated url
+		$url = rtrim($target, '/') . '/' . trim($endPoint, '/') . $this->format;
+
 		$result = null;
 		try {
 			$result = $this->httpClient->get(
-				$target . $this->endPoint,
+				$url,
 				[
 					'query' =>
 						[

apps/federation/lib/BackgroundJob/RequestSharedSecret.php

@@ -37,6 +37,7 @@ use OCP\BackgroundJob\IJobList;
 use OCP\Http\Client\IClient;
 use OCP\ILogger;
 use OCP\IURLGenerator;
+use OCP\OCS\IDiscoveryService;
 
 /**
  * Class RequestSharedSecret
@@ -62,7 +63,8 @@ class RequestSharedSecret extends Job {
 	/** @var TrustedServers */
 	private $trustedServers;
 
-	private $endPoint = '/ocs/v2.php/apps/federation/api/v1/request-shared-secret?format=json';
+	/** @var IDiscoveryService  */
+	private $ocsDiscoveryService;
 
 	/** @var ILogger */
 	private $logger;
@@ -70,6 +72,10 @@ class RequestSharedSecret extends Job {
 	/** @var bool */
 	protected $retainJob = false;
 
+	private $format = '?format=json';
+
+	private $defaultEndPoint = '/ocs/v2.php/apps/federation/api/v1/request-shared-secret';
+
 	/**
 	 * RequestSharedSecret constructor.
 	 *
@@ -78,19 +84,22 @@ class RequestSharedSecret extends Job {
 	 * @param IJobList $jobList
 	 * @param TrustedServers $trustedServers
 	 * @param DbHandler $dbHandler
+	 * @param IDiscoveryService $ocsDiscoveryService
 	 */
 	public function __construct(
 		IClient $httpClient = null,
 		IURLGenerator $urlGenerator = null,
 		IJobList $jobList = null,
 		TrustedServers $trustedServers = null,
-		DbHandler $dbHandler = null
+		DbHandler $dbHandler = null,
+		IDiscoveryService $ocsDiscoveryService = null
 	) {
 		$this->httpClient = $httpClient ? $httpClient : \OC::$server->getHTTPClientService()->newClient();
 		$this->jobList = $jobList ? $jobList : \OC::$server->getJobList();
 		$this->urlGenerator = $urlGenerator ? $urlGenerator : \OC::$server->getURLGenerator();
 		$this->dbHandler = $dbHandler ? $dbHandler : new DbHandler(\OC::$server->getDatabaseConnection(), \OC::$server->getL10N('federation'));
 		$this->logger = \OC::$server->getLogger();
+		$this->ocsDiscoveryService = $ocsDiscoveryService ? $ocsDiscoveryService : \OC::$server->query(\OCP\OCS\IDiscoveryService::class);
 		if ($trustedServers) {
 			$this->trustedServers = $trustedServers;
 		} else {
@@ -142,9 +151,15 @@ class RequestSharedSecret extends Job {
 		$source = rtrim($source, '/');
 		$token = $argument['token'];
 
+		$endPoints = $this->ocsDiscoveryService->discover($target, 'FEDERATED_SHARING');
+		$endPoint = isset($endPoints['shared-secret']) ? $endPoints['shared-secret'] : $this->defaultEndPoint;
+
+		// make sure that we have a well formated url
+		$url = rtrim($target, '/') . '/' . trim($endPoint, '/') . $this->format;
+
 		try {
 			$result = $this->httpClient->post(
-				$target . $this->endPoint,
+				$url,
 				[
 					'body' => [
 						'url' => $source,

apps/federation/lib/Controller/OCSAuthAPIController.php

@@ -90,6 +90,37 @@ class OCSAuthAPIController extends OCSController{
 		$this->logger = $logger;
 	}
 
+	/**
+	 * @NoCSRFRequired
+	 * @PublicPage
+	 *
+	 * request received to ask remote server for a shared secret, for legacy end-points
+	 *
+	 * @param string $url
+	 * @param string $token
+	 * @return Http\DataResponse
+	 * @throws OCSForbiddenException
+	 */
+	public function requestSharedSecretLegacy($url, $token) {
+		return $this->requestSharedSecret($url, $token);
+	}
+
+
+	/**
+	 * @NoCSRFRequired
+	 * @PublicPage
+	 *
+	 * create shared secret and return it, for legacy end-points
+	 *
+	 * @param string $url
+	 * @param string $token
+	 * @return Http\DataResponse
+	 * @throws OCSForbiddenException
+	 */
+	public function getSharedSecretLegacy($url, $token) {
+		return $this->getSharedSecret($url, $token);
+	}
+
 	/**
 	 * @NoCSRFRequired
 	 * @PublicPage

apps/federation/lib/SyncFederationAddressBooks.php

@@ -23,12 +23,10 @@
  */
 namespace OCA\Federation;
 
+use OC\OCS\DiscoveryService;
 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;
-use Symfony\Component\Console\Output\OutputInterface;
+use OCP\OCS\IDiscoveryService;
 
 class SyncFederationAddressBooks {
 
@@ -38,13 +36,21 @@ class SyncFederationAddressBooks {
 	/** @var SyncService */
 	private $syncService;
 
+	/** @var  DiscoveryService */
+	private $ocsDiscoveryService;
+
 	/**
 	 * @param DbHandler $dbHandler
 	 * @param SyncService $syncService
+	 * @param IDiscoveryService $ocsDiscoveryService
 	 */
-	function __construct(DbHandler $dbHandler, SyncService $syncService) {
+	public function __construct(DbHandler $dbHandler,
+								SyncService $syncService,
+								IDiscoveryService $ocsDiscoveryService
+	) {
 		$this->syncService = $syncService;
 		$this->dbHandler = $dbHandler;
+		$this->ocsDiscoveryService = $ocsDiscoveryService;
 	}
 
 	/**
@@ -59,6 +65,10 @@ class SyncFederationAddressBooks {
 			$sharedSecret = $trustedServer['shared_secret'];
 			$syncToken = $trustedServer['sync_token'];
 
+			$endPoints = $this->ocsDiscoveryService->discover($url, 'FEDERATED_SHARING');
+			$cardDavUser = isset($endPoints['carddav-user']) ? $endPoints['carddav-user'] : 'system';
+			$addressBookUrl = isset($endPoints['system-address-book']) ? trim($endPoints['system-address-book'], '/') : 'remote.php/dav/addressbooks/system/system/system';
+
 			if (is_null($sharedSecret)) {
 				continue;
 			}
@@ -68,7 +78,7 @@ class SyncFederationAddressBooks {
 					'{DAV:}displayname' => $url
 			];
 			try {
-				$newToken = $this->syncService->syncRemoteAddressBook($url, 'system', $sharedSecret, $syncToken, $targetBookId, $targetPrincipal, $targetBookProperties);
+				$newToken = $this->syncService->syncRemoteAddressBook($url, $cardDavUser, $addressBookUrl, $sharedSecret, $syncToken, $targetBookId, $targetPrincipal, $targetBookProperties);
 				if ($newToken !== $syncToken) {
 					$this->dbHandler->setServerStatus($url, TrustedServers::STATUS_OK, $newToken);
 				}

apps/federation/tests/BackgroundJob/GetSharedSecretTest.php

@@ -36,6 +36,7 @@ use OCP\Http\Client\IClient;
 use OCP\Http\Client\IResponse;
 use OCP\ILogger;
 use OCP\IURLGenerator;
+use OCP\OCS\IDiscoveryService;
 
 /**
  * Class GetSharedSecretTest
@@ -67,6 +68,9 @@ class GetSharedSecretTest extends TestCase {
 	/** @var \PHPUnit_Framework_MockObject_MockObject | IResponse */
 	private $response;
 
+	/** @var  \PHPUnit_Framework_MockObject_MockObject | IDiscoveryService */
+	private $discoverService;
+
 	/** @var GetSharedSecret */
 	private $getSharedSecret;
 
@@ -82,6 +86,9 @@ class GetSharedSecretTest extends TestCase {
 			->disableOriginalConstructor()->getMock();
 		$this->logger = $this->getMockBuilder(ILogger::class)->getMock();
 		$this->response = $this->getMockBuilder(IResponse::class)->getMock();
+		$this->discoverService = $this->getMockBuilder(IDiscoveryService::class)->getMock();
+
+		$this->discoverService->expects($this->any())->method('discover')->willReturn([]);
 
 		$this->getSharedSecret = new GetSharedSecret(
 			$this->httpClient,
@@ -89,7 +96,8 @@ class GetSharedSecretTest extends TestCase {
 			$this->jobList,
 			$this->trustedServers,
 			$this->logger,
-			$this->dbHandler
+			$this->dbHandler,
+			$this->discoverService
 		);
 	}
 
@@ -109,7 +117,8 @@ class GetSharedSecretTest extends TestCase {
 					$this->jobList,
 					$this->trustedServers,
 					$this->logger,
-					$this->dbHandler
+					$this->dbHandler,
+					$this->discoverService
 				]
 			)->setMethods(['parentExecute'])->getMock();
 		$this->invokePrivate($getSharedSecret, 'argument', [['url' => 'url']]);

apps/federation/tests/BackgroundJob/RequestSharedSecretTest.php

@@ -33,6 +33,7 @@ use OCP\BackgroundJob\IJobList;
 use OCP\Http\Client\IClient;
 use OCP\Http\Client\IResponse;
 use OCP\IURLGenerator;
+use OCP\OCS\IDiscoveryService;
 use Test\TestCase;
 
 class RequestSharedSecretTest extends TestCase {
@@ -55,6 +56,9 @@ class RequestSharedSecretTest extends TestCase {
 	/** @var \PHPUnit_Framework_MockObject_MockObject | IResponse */
 	private $response;
 
+	/** @var  \PHPUnit_Framework_MockObject_MockObject | IDiscoveryService */
+	private $discoveryService;
+
 	/** @var  RequestSharedSecret */
 	private $requestSharedSecret;
 
@@ -69,13 +73,17 @@ class RequestSharedSecretTest extends TestCase {
 		$this->dbHandler = $this->getMockBuilder(DbHandler::class)
 			->disableOriginalConstructor()->getMock();
 		$this->response = $this->getMockBuilder(IResponse::class)->getMock();
+		$this->discoveryService = $this->getMockBuilder(IDiscoveryService::class)->getMock();
+
+		$this->discoveryService->expects($this->any())->method('discover')->willReturn([]);
 
 		$this->requestSharedSecret = new RequestSharedSecret(
 			$this->httpClient,
 			$this->urlGenerator,
 			$this->jobList,
 			$this->trustedServers,
-			$this->dbHandler
+			$this->dbHandler,
+			$this->discoveryService
 		);
 	}
 
@@ -94,7 +102,8 @@ class RequestSharedSecretTest extends TestCase {
 					$this->urlGenerator,
 					$this->jobList,
 					$this->trustedServers,
-					$this->dbHandler
+					$this->dbHandler,
+					$this->discoveryService
 				]
 			)->setMethods(['parentExecute'])->getMock();
 		$this->invokePrivate($requestSharedSecret, 'argument', [['url' => 'url']]);

apps/federation/tests/SyncFederationAddressbooksTest.php

@@ -24,6 +24,7 @@
  */
 namespace OCA\Federation\Tests;
 
+use OC\OCS\DiscoveryService;
 use OCA\Federation\DbHandler;
 use OCA\Federation\SyncFederationAddressBooks;
 
@@ -32,7 +33,18 @@ class SyncFederationAddressbooksTest extends \Test\TestCase {
 	/** @var array */
 	private $callBacks = [];
 
-	function testSync() {
+	/** @var  \PHPUnit_Framework_MockObject_MockObject | DiscoveryService */
+	private $discoveryService;
+
+	public function setUp() {
+		parent::setUp();
+
+		$this->discoveryService = $this->getMockBuilder(DiscoveryService::class)
+			->disableOriginalConstructor()->getMock();
+		$this->discoveryService->expects($this->any())->method('discover')->willReturn([]);
+	}
+
+	public function testSync() {
 		/** @var DbHandler | \PHPUnit_Framework_MockObject_MockObject $dbHandler */
 		$dbHandler = $this->getMockBuilder('OCA\Federation\DbHandler')->
 			disableOriginalConstructor()->
@@ -55,14 +67,14 @@ class SyncFederationAddressbooksTest extends \Test\TestCase {
 			->willReturn(1);
 
 		/** @var \OCA\DAV\CardDAV\SyncService $syncService */
-		$s = new SyncFederationAddressBooks($dbHandler, $syncService);
+		$s = new SyncFederationAddressBooks($dbHandler, $syncService, $this->discoveryService);
 		$s->syncThemAll(function($url, $ex) {
 			$this->callBacks[] = [$url, $ex];
 		});
 		$this->assertEquals(1, count($this->callBacks));
 	}
 
-	function testException() {
+	public function testException() {
 		/** @var DbHandler | \PHPUnit_Framework_MockObject_MockObject $dbHandler */
 		$dbHandler = $this->getMockBuilder('OCA\Federation\DbHandler')->
 		disableOriginalConstructor()->
@@ -83,7 +95,7 @@ class SyncFederationAddressbooksTest extends \Test\TestCase {
 			->willThrowException(new \Exception('something did not work out'));
 
 		/** @var \OCA\DAV\CardDAV\SyncService $syncService */
-		$s = new SyncFederationAddressBooks($dbHandler, $syncService);
+		$s = new SyncFederationAddressBooks($dbHandler, $syncService, $this->discoveryService);
 		$s->syncThemAll(function($url, $ex) {
 			$this->callBacks[] = [$url, $ex];
 		});

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->query(\OCP\OCS\IDiscoveryService::class),
 				$uid
 			);
 		});