diff --git a/apps/dav/lib/CardDAV/SyncService.php b/apps/dav/lib/CardDAV/SyncService.php index e09cc36c8bbbe..365b5c16f8c5e 100644 --- a/apps/dav/lib/CardDAV/SyncService.php +++ b/apps/dav/lib/CardDAV/SyncService.php @@ -10,14 +10,14 @@ use OCP\AppFramework\Db\TTransactional; use OCP\AppFramework\Http; +use OCP\Http\Client\IClientService; use OCP\IDBConnection; use OCP\IUser; use OCP\IUserManager; +use Psr\Http\Client\ClientExceptionInterface; use Psr\Log\LoggerInterface; -use Sabre\DAV\Client; use Sabre\DAV\Xml\Response\MultiStatus; use Sabre\DAV\Xml\Service; -use Sabre\HTTP\ClientHttpException; use Sabre\VObject\Reader; use function is_null; @@ -32,18 +32,21 @@ class SyncService { private ?array $localSystemAddressBook = null; private Converter $converter; protected string $certPath; + private IClientService $clientService; public function __construct(CardDavBackend $backend, IUserManager $userManager, IDBConnection $dbConnection, LoggerInterface $logger, - Converter $converter) { + Converter $converter, + IClientService $clientService) { $this->backend = $backend; $this->userManager = $userManager; $this->logger = $logger; $this->converter = $converter; $this->certPath = ''; $this->dbConnection = $dbConnection; + $this->clientService = $clientService; } /** @@ -57,7 +60,7 @@ public function syncRemoteAddressBook(string $url, string $userName, string $add // 2. query changes try { $response = $this->requestSyncReport($url, $userName, $addressBookUrl, $sharedSecret, $syncToken); - } catch (ClientHttpException $ex) { + } catch (ClientExceptionInterface $ex) { if ($ex->getCode() === Http::STATUS_UNAUTHORIZED) { // remote server revoked access to the address book, remove it $this->backend->deleteAddressBook($addressBookId); @@ -77,9 +80,9 @@ public function syncRemoteAddressBook(string $url, string $userName, string $add $this->atomic(function () use ($addressBookId, $cardUri, $vCard) { $existingCard = $this->backend->getCard($addressBookId, $cardUri); if ($existingCard === false) { - $this->backend->createCard($addressBookId, $cardUri, $vCard['body']); + $this->backend->createCard($addressBookId, $cardUri, $vCard); } else { - $this->backend->updateCard($addressBookId, $cardUri, $vCard['body']); + $this->backend->updateCard($addressBookId, $cardUri, $vCard); } }, $this->dbConnection); } else { @@ -106,56 +109,47 @@ public function ensureSystemAddressBookExists(string $principal, string $uri, ar } /** - * Check if there is a valid certPath we should use + * @throws ClientExceptionInterface */ - protected function getCertPath(): string { - - // we already have a valid certPath - if ($this->certPath !== '') { - return $this->certPath; - } - - $certManager = \OC::$server->getCertificateManager(); - $certPath = $certManager->getAbsoluteBundlePath(); - if (file_exists($certPath)) { - $this->certPath = $certPath; - } + protected function requestSyncReport(string $url, string $userName, string $addressBookUrl, string $sharedSecret, ?string $syncToken): array { + $client = $this->clientService->newClient(); - return $this->certPath; - } + // the trailing slash is important for merging base_uri and uri + $url = rtrim($url, '/') . '/'; - protected function getClient(string $url, string $userName, string $sharedSecret): Client { - $settings = [ - 'baseUri' => $url . '/', - 'userName' => $userName, - 'password' => $sharedSecret, + $options = [ + 'auth' => [$userName, $sharedSecret], + 'base_uri' => $url, + 'body' => $this->buildSyncCollectionRequestBody($syncToken), + 'headers' => ['Content-Type' => 'application/xml'] ]; - $client = new Client($settings); - $certPath = $this->getCertPath(); - $client->setThrowExceptions(true); - if ($certPath !== '' && !str_starts_with($url, 'http://')) { - $client->addCurlSetting(CURLOPT_CAINFO, $this->certPath); - } + $response = $client->request( + 'REPORT', + $addressBookUrl, + $options + ); - return $client; + return $this->parseMultiStatus($response->getBody()); } - protected function requestSyncReport(string $url, string $userName, string $addressBookUrl, string $sharedSecret, ?string $syncToken): array { - $client = $this->getClient($url, $userName, $sharedSecret); + protected function download(string $url, string $userName, string $sharedSecret, string $resourcePath): string { + $client = $this->clientService->newClient(); - $body = $this->buildSyncCollectionRequestBody($syncToken); + // the trailing slash is important for merging base_uri and uri + $url = rtrim($url, '/') . '/'; - $response = $client->request('REPORT', $addressBookUrl, $body, [ - 'Content-Type' => 'application/xml' - ]); + $options = [ + 'auth' => [$userName, $sharedSecret], + 'base_uri' => $url, + ]; - return $this->parseMultiStatus($response['body']); - } + $response = $client->get( + $resourcePath, + $options + ); - protected function download(string $url, string $userName, string $sharedSecret, string $resourcePath): array { - $client = $this->getClient($url, $userName, $sharedSecret); - return $client->request('GET', $resourcePath); + return (string)$response->getBody(); } private function buildSyncCollectionRequestBody(?string $syncToken): string { diff --git a/apps/dav/tests/unit/CardDAV/SyncServiceTest.php b/apps/dav/tests/unit/CardDAV/SyncServiceTest.php index a09d5de6a44fe..8f7727cd3d0d7 100644 --- a/apps/dav/tests/unit/CardDAV/SyncServiceTest.php +++ b/apps/dav/tests/unit/CardDAV/SyncServiceTest.php @@ -7,49 +7,285 @@ */ namespace OCA\DAV\Tests\unit\CardDAV; +use GuzzleHttp\Exception\ClientException; +use GuzzleHttp\Psr7\Request as PsrRequest; +use GuzzleHttp\Psr7\Response as PsrResponse; +use OC\Http\Client\Response; use OCA\DAV\CardDAV\CardDavBackend; use OCA\DAV\CardDAV\Converter; use OCA\DAV\CardDAV\SyncService; +use OCP\Http\Client\IClient; +use OCP\Http\Client\IClientService; use OCP\IDBConnection; use OCP\IUser; use OCP\IUserManager; +use Psr\Http\Client\ClientExceptionInterface; use Psr\Log\LoggerInterface; +use Psr\Log\Test\TestLogger; use Sabre\VObject\Component\VCard; use Test\TestCase; class SyncServiceTest extends TestCase { + + protected CardDavBackend $backend; + protected IUserManager $userManager; + protected IDBConnection $dbConnection; + protected LoggerInterface $logger; + protected Converter $converter; + protected IClient $client; + protected SyncService $service; + public function setUp(): void { + $addressBook = [ + 'id' => 1, + 'uri' => 'system', + 'principaluri' => 'principals/system/system', + '{DAV:}displayname' => 'system', + // watch out, incomplete address book mock. + ]; + + $this->backend = $this->createMock(CardDavBackend::class); + $this->backend->method('getAddressBooksByUri') + ->with('principals/system/system', 1) + ->willReturn($addressBook); + + $this->userManager = $this->createMock(IUserManager::class); + $this->dbConnection = $this->createMock(IDBConnection::class); + $this->logger = new TestLogger(); + $this->converter = $this->createMock(Converter::class); + $this->client = $this->createMock(IClient::class); + + $clientService = $this->createMock(IClientService::class); + $clientService->method('newClient') + ->willReturn($this->client); + + $this->service = new SyncService( + $this->backend, + $this->userManager, + $this->dbConnection, + $this->logger, + $this->converter, + $clientService + ); + } + public function testEmptySync(): void { - $backend = $this->getBackendMock(0, 0, 0); + $this->backend->expects($this->exactly(0)) + ->method('createCard'); + $this->backend->expects($this->exactly(0)) + ->method('updateCard'); + $this->backend->expects($this->exactly(0)) + ->method('deleteCard'); + + $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 + )); - $ss = $this->getSyncServiceMock($backend, []); - $return = $ss->syncRemoteAddressBook('', 'system', 'system', '1234567890', null, '1', 'principals/system/system', []); - $this->assertEquals('sync-token-1', $return); + $this->client + ->method('request') + ->willReturn($requestResponse); + + $token = $this->service->syncRemoteAddressBook( + '', + 'system', + 'system', + '1234567890', + null, + '1', + 'principals/system/system', + [] + ); + + $this->assertEquals('http://sabre.io/ns/sync/1', $token); } public function testSyncWithNewElement(): void { - $backend = $this->getBackendMock(1, 0, 0); - $backend->method('getCard')->willReturn(false); + $this->backend->expects($this->exactly(1)) + ->method('createCard'); + $this->backend->expects($this->exactly(0)) + ->method('updateCard'); + $this->backend->expects($this->exactly(0)) + ->method('deleteCard'); + + $this->backend->method('getCard') + ->willReturn(false); - $ss = $this->getSyncServiceMock($backend, ['0' => [200 => '']]); - $return = $ss->syncRemoteAddressBook('', 'system', 'system', '1234567890', null, '1', 'principals/system/system', []); - $this->assertEquals('sync-token-1', $return); + + $body = ' + + + /remote.php/dav/addressbooks/system/system/system/Database:alice.vcf + + + text/vcard; charset=utf-8 + "2df155fa5c2a24cd7f750353fc63f037" + + HTTP/1.1 200 OK + + + http://sabre.io/ns/sync/2 +'; + + $reportResponse = new Response(new PsrResponse( + 207, + ['Content-Type' => 'application/xml; charset=utf-8', 'Content-Length' => strlen($body)], + $body + )); + + $this->client + ->method('request') + ->willReturn($reportResponse); + + $vCard = 'BEGIN:VCARD +VERSION:3.0 +PRODID:-//Sabre//Sabre VObject 4.5.4//EN +UID:alice +FN;X-NC-SCOPE=v2-federated:alice +N;X-NC-SCOPE=v2-federated:alice;;;; +X-SOCIALPROFILE;TYPE=NEXTCLOUD;X-NC-SCOPE=v2-published:https://server2.internal/index.php/u/alice +CLOUD:alice@server2.internal +END:VCARD'; + + $getResponse = new Response(new PsrResponse( + 200, + ['Content-Type' => 'text/vcard; charset=utf-8', 'Content-Length' => strlen($vCard)], + $vCard, + )); + + $this->client + ->method('get') + ->willReturn($getResponse); + + $token = $this->service->syncRemoteAddressBook( + '', + 'system', + 'system', + '1234567890', + null, + '1', + 'principals/system/system', + [] + ); + + $this->assertEquals('http://sabre.io/ns/sync/2', $token); } public function testSyncWithUpdatedElement(): void { - $backend = $this->getBackendMock(0, 1, 0); - $backend->method('getCard')->willReturn(true); + $this->backend->expects($this->exactly(0)) + ->method('createCard'); + $this->backend->expects($this->exactly(1)) + ->method('updateCard'); + $this->backend->expects($this->exactly(0)) + ->method('deleteCard'); + + $this->backend->method('getCard') + ->willReturn(true); + + + $body = ' + + + /remote.php/dav/addressbooks/system/system/system/Database:alice.vcf + + + text/vcard; charset=utf-8 + "2df155fa5c2a24cd7f750353fc63f037" + + HTTP/1.1 200 OK + + + http://sabre.io/ns/sync/3 +'; + + $reportResponse = new Response(new PsrResponse( + 207, + ['Content-Type' => 'application/xml; charset=utf-8', 'Content-Length' => strlen($body)], + $body + )); + + $this->client + ->method('request') + ->willReturn($reportResponse); - $ss = $this->getSyncServiceMock($backend, ['0' => [200 => '']]); - $return = $ss->syncRemoteAddressBook('', 'system', 'system', '1234567890', null, '1', 'principals/system/system', []); - $this->assertEquals('sync-token-1', $return); + $vCard = 'BEGIN:VCARD +VERSION:3.0 +PRODID:-//Sabre//Sabre VObject 4.5.4//EN +UID:alice +FN;X-NC-SCOPE=v2-federated:alice +N;X-NC-SCOPE=v2-federated:alice;;;; +X-SOCIALPROFILE;TYPE=NEXTCLOUD;X-NC-SCOPE=v2-published:https://server2.internal/index.php/u/alice +CLOUD:alice@server2.internal +END:VCARD'; + + $getResponse = new Response(new PsrResponse( + 200, + ['Content-Type' => 'text/vcard; charset=utf-8', 'Content-Length' => strlen($vCard)], + $vCard, + )); + + $this->client + ->method('get') + ->willReturn($getResponse); + + $token = $this->service->syncRemoteAddressBook( + '', + 'system', + 'system', + '1234567890', + null, + '1', + 'principals/system/system', + [] + ); + + $this->assertEquals('http://sabre.io/ns/sync/3', $token); } public function testSyncWithDeletedElement(): void { - $backend = $this->getBackendMock(0, 0, 1); + $this->backend->expects($this->exactly(0)) + ->method('createCard'); + $this->backend->expects($this->exactly(0)) + ->method('updateCard'); + $this->backend->expects($this->exactly(1)) + ->method('deleteCard'); + + $body = ' + + + /remote.php/dav/addressbooks/system/system/system/Database:alice.vcf + HTTP/1.1 404 Not Found + +http://sabre.io/ns/sync/4 +'; + + $reportResponse = new Response(new PsrResponse( + 207, + ['Content-Type' => 'application/xml; charset=utf-8', 'Content-Length' => strlen($body)], + $body + )); + + $this->client + ->method('request') + ->willReturn($reportResponse); - $ss = $this->getSyncServiceMock($backend, ['0' => [404 => '']]); - $return = $ss->syncRemoteAddressBook('', 'system', 'system', '1234567890', null, '1', 'principals/system/system', []); - $this->assertEquals('sync-token-1', $return); + $token = $this->service->syncRemoteAddressBook( + '', + 'system', + 'system', + '1234567890', + null, + '1', + 'principals/system/system', + [] + ); + + $this->assertEquals('http://sabre.io/ns/sync/4', $token); } public function testEnsureSystemAddressBookExists(): void { @@ -68,8 +304,9 @@ public function testEnsureSystemAddressBookExists(): void { $dbConnection = $this->createMock(IDBConnection::class); $logger = $this->getMockBuilder(LoggerInterface::class)->disableOriginalConstructor()->getMock(); $converter = $this->createMock(Converter::class); + $clientService = $this->createMock(IClientService::class); - $ss = new SyncService($backend, $userManager, $dbConnection, $logger, $converter); + $ss = new SyncService($backend, $userManager, $dbConnection, $logger, $converter, $clientService); $ss->ensureSystemAddressBookExists('principals/users/adam', 'contacts', []); } @@ -122,7 +359,9 @@ public function testUpdateAndDeleteUser($activated, $createCalls, $updateCalls, ->method('createCardFromUser') ->willReturn($this->createMock(VCard::class)); - $ss = new SyncService($backend, $userManager, $dbConnection, $logger, $converter); + $clientService = $this->createMock(IClientService::class); + + $ss = new SyncService($backend, $userManager, $dbConnection, $logger, $converter, $clientService); $ss->updateUser($user); $ss->updateUser($user); @@ -130,45 +369,61 @@ public function testUpdateAndDeleteUser($activated, $createCalls, $updateCalls, $ss->deleteUser($user); } - /** - * @param int $createCount - * @param int $updateCount - * @param int $deleteCount - * @return \PHPUnit\Framework\MockObject\MockObject|CardDavBackend - */ - private function getBackendMock($createCount, $updateCount, $deleteCount) { - $backend = $this->getMockBuilder(CardDavBackend::class) - ->disableOriginalConstructor() - ->getMock(); - $backend->expects($this->exactly($createCount))->method('createCard'); - $backend->expects($this->exactly($updateCount))->method('updateCard'); - $backend->expects($this->exactly($deleteCount))->method('deleteCard'); - return $backend; - } + public function testDeleteAddressbookWhenAccessRevoked(): void { + $this->expectException(ClientExceptionInterface::class); - /** - * @param $backend - * @param $response - * @return SyncService|\PHPUnit\Framework\MockObject\MockObject - */ - private function getSyncServiceMock($backend, $response) { - $userManager = $this->getMockBuilder(IUserManager::class)->disableOriginalConstructor()->getMock(); - $dbConnection = $this->createMock(IDBConnection::class); - $logger = $this->getMockBuilder(LoggerInterface::class)->disableOriginalConstructor()->getMock(); - $converter = $this->createMock(Converter::class); - /** @var SyncService | \PHPUnit\Framework\MockObject\MockObject $ss */ - $ss = $this->getMockBuilder(SyncService::class) - ->setMethods(['ensureSystemAddressBookExists', 'requestSyncReport', 'download', 'getCertPath']) - ->setConstructorArgs([$backend, $userManager, $dbConnection, $logger, $converter]) - ->getMock(); - $ss->method('requestSyncReport')->withAnyParameters()->willReturn(['response' => $response, 'token' => 'sync-token-1']); - $ss->method('ensureSystemAddressBookExists')->willReturn(['id' => 1]); - $ss->method('download')->willReturn([ - 'body' => '', - 'statusCode' => 200, - 'headers' => [] - ]); - $ss->method('getCertPath')->willReturn(''); - return $ss; + $this->backend->expects($this->exactly(0)) + ->method('createCard'); + $this->backend->expects($this->exactly(0)) + ->method('updateCard'); + $this->backend->expects($this->exactly(0)) + ->method('deleteCard'); + $this->backend->expects($this->exactly(1)) + ->method('deleteAddressBook'); + + $request = new PsrRequest( + 'REPORT', + 'https://server2.internal/remote.php/dav/addressbooks/system/system/system', + ['Content-Type' => 'application/xml'], + ); + + $body = ' + + Sabre\DAV\Exception\NotAuthenticated + No public access to this resource., Username or password was incorrect, No \'Authorization: Bearer\' header found. Either the client didn\'t send one, or the server is mis-configured, Username or password was incorrect +'; + + $response = new PsrResponse( + 401, + ['Content-Type' => 'application/xml; charset=utf-8', 'Content-Length' => strlen($body)], + $body + ); + + $message = 'Client error: `REPORT https://server2.internal/cloud/remote.php/dav/addressbooks/system/system/system` resulted in a `401 Unauthorized` response: + + + Sabre\DA (truncated...) +'; + + $reportException = new ClientException( + $message, + $request, + $response + ); + + $this->client + ->method('request') + ->willThrowException($reportException); + + $this->service->syncRemoteAddressBook( + '', + 'system', + 'system', + '1234567890', + null, + '1', + 'principals/system/system', + [] + ); } }