From 870816466f2d1adaf956a83491c0645556b0d02b Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Fri, 27 Sep 2024 19:16:59 +0200 Subject: [PATCH] fix: make federation address book sync work with allow_local_remote_servers = false Client.preventLocalAddress expects an absolute URL, which means the base_uri option cannot be used. Signed-off-by: Daniel Kesselberg --- apps/dav/lib/CardDAV/SyncService.php | 46 ++++++++++++---- .../tests/unit/CardDAV/SyncServiceTest.php | 55 +++++++++++++++++++ 2 files changed, 91 insertions(+), 10 deletions(-) diff --git a/apps/dav/lib/CardDAV/SyncService.php b/apps/dav/lib/CardDAV/SyncService.php index 5f8752a9d492c..05a08d08286ba 100644 --- a/apps/dav/lib/CardDAV/SyncService.php +++ b/apps/dav/lib/CardDAV/SyncService.php @@ -108,25 +108,54 @@ public function ensureSystemAddressBookExists(string $principal, string $uri, ar }, $this->dbConnection); } + private function prepareUri(string $host, string $path): string { + /* + * The trailing slash is important for merging the uris together. + * + * $host is stored in oc_trusted_servers.url and usually without a trailing slash. + * + * Example for a report request + * + * $host = 'https://server.internal/cloud' + * $path = 'remote.php/dav/addressbooks/system/system/system' + * + * Without the trailing slash, the webroot is missing: + * https://server.internal/remote.php/dav/addressbooks/system/system/system + * + * Example for a download request + * + * $host = 'https://server.internal/cloud' + * $path = '/cloud/remote.php/dav/addressbooks/system/system/system/Database:alice.vcf' + * + * The response from the remote usually contains the webroot already and must be normalized to: + * https://server.internal/cloud/remote.php/dav/addressbooks/system/system/system/Database:alice.vcf + */ + $host = rtrim($host, '/') . '/'; + + $uri = \GuzzleHttp\Psr7\UriResolver::resolve( + \GuzzleHttp\Psr7\Utils::uriFor($host), + \GuzzleHttp\Psr7\Utils::uriFor($path) + ); + + return (string)$uri; + } + /** * @throws ClientExceptionInterface */ protected function requestSyncReport(string $url, string $userName, string $addressBookUrl, string $sharedSecret, ?string $syncToken): array { $client = $this->clientService->newClient(); - - // the trailing slash is important for merging base_uri and uri - $url = rtrim($url, '/') . '/'; + $uri = $this->prepareUri($url, $addressBookUrl); $options = [ 'auth' => [$userName, $sharedSecret], - 'base_uri' => $url, 'body' => $this->buildSyncCollectionRequestBody($syncToken), 'headers' => ['Content-Type' => 'application/xml'] ]; $response = $client->request( 'REPORT', - $addressBookUrl, + $uri, $options ); @@ -138,17 +167,14 @@ protected function requestSyncReport(string $url, string $userName, string $addr protected function download(string $url, string $userName, string $sharedSecret, string $resourcePath): string { $client = $this->clientService->newClient(); - - // the trailing slash is important for merging base_uri and uri - $url = rtrim($url, '/') . '/'; + $uri = $this->prepareUri($url, $resourcePath); $options = [ 'auth' => [$userName, $sharedSecret], - 'base_uri' => $url, ]; $response = $client->get( - $resourcePath, + $uri, $options ); diff --git a/apps/dav/tests/unit/CardDAV/SyncServiceTest.php b/apps/dav/tests/unit/CardDAV/SyncServiceTest.php index 570c55eab0540..db99f73306d26 100644 --- a/apps/dav/tests/unit/CardDAV/SyncServiceTest.php +++ b/apps/dav/tests/unit/CardDAV/SyncServiceTest.php @@ -426,4 +426,59 @@ public function testDeleteAddressbookWhenAccessRevoked(): void { [] ); } + + /** + * @dataProvider providerUseAbsoluteUriReport + */ + public function testUseAbsoluteUriReport(string $host, string $expected): void { + $body = ' + + http://sabre.io/ns/sync/1 +'; + + $requestResponse = new Response(new PsrResponse( + 207, + ['Content-Type' => 'application/xml; charset=utf-8', 'Content-Length' => strlen($body)], + $body + )); + + $this->client + ->method('request') + ->with( + 'REPORT', + $this->callback(function ($uri) use ($expected) { + $this->assertEquals($expected, $uri); + return true; + }), + $this->callback(function ($options) { + $this->assertIsArray($options); + return true; + }), + ) + ->willReturn($requestResponse); + + $this->service->syncRemoteAddressBook( + $host, + 'system', + 'remote.php/dav/addressbooks/system/system/system', + '1234567890', + null, + '1', + 'principals/system/system', + [] + ); + } + + public function providerUseAbsoluteUriReport(): array { + return [ + ['https://server.internal', 'https://server.internal/remote.php/dav/addressbooks/system/system/system'], + ['https://server.internal/', 'https://server.internal/remote.php/dav/addressbooks/system/system/system'], + ['https://server.internal/nextcloud', 'https://server.internal/nextcloud/remote.php/dav/addressbooks/system/system/system'], + ['https://server.internal/nextcloud/', 'https://server.internal/nextcloud/remote.php/dav/addressbooks/system/system/system'], + ['https://server.internal:8080', 'https://server.internal:8080/remote.php/dav/addressbooks/system/system/system'], + ['https://server.internal:8080/', 'https://server.internal:8080/remote.php/dav/addressbooks/system/system/system'], + ['https://server.internal:8080/nextcloud', 'https://server.internal:8080/nextcloud/remote.php/dav/addressbooks/system/system/system'], + ['https://server.internal:8080/nextcloud/', 'https://server.internal:8080/nextcloud/remote.php/dav/addressbooks/system/system/system'], + ]; + } }