feat(dav): store scopes for properties and filter locally scoped properties for federated address book sync

Signed-off-by: Anna Larch <anna@nextcloud.com>
pull/38048/head
Anna Larch 2023-05-09 19:17:39 +07:00 committed by Christoph Wurst
parent fc076271c7
commit bd80a1b2dd
No known key found for this signature in database
GPG Key ID: CC42AC2A7F0E56D8
10 changed files with 431 additions and 65 deletions

@ -5,7 +5,7 @@
<name>WebDAV</name>
<summary>WebDAV endpoint</summary>
<description>WebDAV endpoint</description>
<version>1.26.0</version>
<version>1.27.0</version>
<licence>agpl</licence>
<author>owncloud.org</author>
<namespace>DAV</namespace>

@ -293,6 +293,7 @@ return array(
'OCA\\DAV\\Migration\\Version1017Date20210216083742' => $baseDir . '/../lib/Migration/Version1017Date20210216083742.php',
'OCA\\DAV\\Migration\\Version1018Date20210312100735' => $baseDir . '/../lib/Migration/Version1018Date20210312100735.php',
'OCA\\DAV\\Migration\\Version1024Date20211221144219' => $baseDir . '/../lib/Migration/Version1024Date20211221144219.php',
'OCA\\DAV\\Migration\\Version1027Date20230504122946' => $baseDir . '/../lib/Migration/Version1027Date20230504122946.php',
'OCA\\DAV\\Profiler\\ProfilerPlugin' => $baseDir . '/../lib/Profiler/ProfilerPlugin.php',
'OCA\\DAV\\Provisioning\\Apple\\AppleProvisioningNode' => $baseDir . '/../lib/Provisioning/Apple/AppleProvisioningNode.php',
'OCA\\DAV\\Provisioning\\Apple\\AppleProvisioningPlugin' => $baseDir . '/../lib/Provisioning/Apple/AppleProvisioningPlugin.php',

@ -308,6 +308,7 @@ class ComposerStaticInitDAV
'OCA\\DAV\\Migration\\Version1017Date20210216083742' => __DIR__ . '/..' . '/../lib/Migration/Version1017Date20210216083742.php',
'OCA\\DAV\\Migration\\Version1018Date20210312100735' => __DIR__ . '/..' . '/../lib/Migration/Version1018Date20210312100735.php',
'OCA\\DAV\\Migration\\Version1024Date20211221144219' => __DIR__ . '/..' . '/../lib/Migration/Version1024Date20211221144219.php',
'OCA\\DAV\\Migration\\Version1027Date20230504122946' => __DIR__ . '/..' . '/../lib/Migration/Version1027Date20230504122946.php',
'OCA\\DAV\\Profiler\\ProfilerPlugin' => __DIR__ . '/..' . '/../lib/Profiler/ProfilerPlugin.php',
'OCA\\DAV\\Provisioning\\Apple\\AppleProvisioningNode' => __DIR__ . '/..' . '/../lib/Provisioning/Apple/AppleProvisioningNode.php',
'OCA\\DAV\\Provisioning\\Apple\\AppleProvisioningPlugin' => __DIR__ . '/..' . '/../lib/Provisioning/Apple/AppleProvisioningPlugin.php',

@ -29,15 +29,12 @@ namespace OCA\DAV\CardDAV;
use Exception;
use OCP\Accounts\IAccountManager;
use OCP\Accounts\PropertyDoesNotExistException;
use OCP\IImage;
use OCP\IUser;
use Sabre\VObject\Component\VCard;
use Sabre\VObject\Property\Text;
use function array_merge;
class Converter {
/** @var IAccountManager */
private $accountManager;
@ -46,13 +43,7 @@ class Converter {
}
public function createCardFromUser(IUser $user): ?VCard {
$account = $this->accountManager->getAccount($user);
$userProperties = $account->getProperties();
try {
$additionalEmailsCollection = $account->getPropertyCollection(IAccountManager::COLLECTION_EMAIL);
$userProperties = array_merge($userProperties, $additionalEmailsCollection->getProperties());
} catch (PropertyDoesNotExistException $e) {
}
$userProperties = $this->accountManager->getAccount($user)->getAllProperties();
$uid = $user->getUID();
$cloudId = $user->getCloudId();
@ -65,47 +56,49 @@ class Converter {
$publish = false;
foreach ($userProperties as $property) {
$shareWithTrustedServers =
$property->getScope() === IAccountManager::SCOPE_FEDERATED ||
$property->getScope() === IAccountManager::SCOPE_PUBLISHED;
$emptyValue = $property->getValue() === '';
if ($shareWithTrustedServers && !$emptyValue) {
$publish = true;
switch ($property->getName()) {
case IAccountManager::PROPERTY_DISPLAYNAME:
$vCard->add(new Text($vCard, 'FN', $property->getValue()));
$vCard->add(new Text($vCard, 'N', $this->splitFullName($property->getValue())));
break;
case IAccountManager::PROPERTY_AVATAR:
if ($image !== null) {
$vCard->add('PHOTO', $image->data(), ['ENCODING' => 'b', 'TYPE' => $image->mimeType()]);
}
break;
case IAccountManager::COLLECTION_EMAIL:
case IAccountManager::PROPERTY_EMAIL:
$vCard->add(new Text($vCard, 'EMAIL', $property->getValue(), ['TYPE' => 'OTHER']));
break;
case IAccountManager::PROPERTY_WEBSITE:
$vCard->add(new Text($vCard, 'URL', $property->getValue()));
break;
case IAccountManager::PROPERTY_PHONE:
$vCard->add(new Text($vCard, 'TEL', $property->getValue(), ['TYPE' => 'OTHER']));
break;
case IAccountManager::PROPERTY_ADDRESS:
$vCard->add(new Text($vCard, 'ADR', $property->getValue(), ['TYPE' => 'OTHER']));
break;
case IAccountManager::PROPERTY_TWITTER:
$vCard->add(new Text($vCard, 'X-SOCIALPROFILE', $property->getValue(), ['TYPE' => 'TWITTER']));
break;
case IAccountManager::PROPERTY_ORGANISATION:
$vCard->add(new Text($vCard, 'ORG', $property->getValue()));
break;
case IAccountManager::PROPERTY_ROLE:
$vCard->add(new Text($vCard, 'TITLE', $property->getValue()));
break;
}
if (empty($property->getValue())) {
continue;
}
$scope = $property->getScope();
// Do not write private data to the system address book at all
if ($scope === IAccountManager::SCOPE_PRIVATE || empty($scope)) {
continue;
}
$publish = true;
switch ($property->getName()) {
case IAccountManager::PROPERTY_DISPLAYNAME:
$vCard->add(new Text($vCard, 'FN', $property->getValue(), ['X-NC-SCOPE' => $scope]));
$vCard->add(new Text($vCard, 'N', $this->splitFullName($property->getValue()), ['X-NC-SCOPE' => $scope]));
break;
case IAccountManager::PROPERTY_AVATAR:
if ($image !== null) {
$vCard->add('PHOTO', $image->data(), ['ENCODING' => 'b', 'TYPE' => $image->mimeType(), ['X-NC-SCOPE' => $scope]]);
}
break;
case IAccountManager::COLLECTION_EMAIL:
case IAccountManager::PROPERTY_EMAIL:
$vCard->add(new Text($vCard, 'EMAIL', $property->getValue(), ['TYPE' => 'OTHER', 'X-NC-SCOPE' => $scope]));
break;
case IAccountManager::PROPERTY_WEBSITE:
$vCard->add(new Text($vCard, 'URL', $property->getValue(), ['X-NC-SCOPE' => $scope]));
break;
case IAccountManager::PROPERTY_PHONE:
$vCard->add(new Text($vCard, 'TEL', $property->getValue(), ['TYPE' => 'OTHER', 'X-NC-SCOPE' => $scope]));
break;
case IAccountManager::PROPERTY_ADDRESS:
$vCard->add(new Text($vCard, 'ADR', $property->getValue(), ['TYPE' => 'OTHER', 'X-NC-SCOPE' => $scope]));
break;
case IAccountManager::PROPERTY_TWITTER:
$vCard->add(new Text($vCard, 'X-SOCIALPROFILE', $property->getValue(), ['TYPE' => 'TWITTER', 'X-NC-SCOPE' => $scope]));
break;
case IAccountManager::PROPERTY_ORGANISATION:
$vCard->add(new Text($vCard, 'ORG', $property->getValue(), ['X-NC-SCOPE' => $scope]));
break;
case IAccountManager::PROPERTY_ROLE:
$vCard->add(new Text($vCard, 'TITLE', $property->getValue(), ['X-NC-SCOPE' => $scope]));
break;
}
}

@ -27,20 +27,34 @@ declare(strict_types=1);
*/
namespace OCA\DAV\CardDAV;
use OCA\DAV\Exception\UnsupportedLimitOnInitialSyncException;
use OCA\Federation\TrustedServers;
use OCP\Accounts\IAccountManager;
use OCP\IConfig;
use OCP\IL10N;
use OCP\IRequest;
use Sabre\CardDAV\Backend\SyncSupport;
use Sabre\CardDAV\Backend\BackendInterface;
use Sabre\CardDAV\Card;
use Sabre\DAV\Exception\Forbidden;
use Sabre\DAV\Exception\NotFound;
use Sabre\VObject\Component\VCard;
use Sabre\VObject\Reader;
class SystemAddressbook extends AddressBook {
/** @var IConfig */
private $config;
private ?TrustedServers $trustedServers;
private ?IRequest $request;
public function __construct(BackendInterface $carddavBackend, array $addressBookInfo, IL10N $l10n, IConfig $config) {
public function __construct(BackendInterface $carddavBackend, array $addressBookInfo, IL10N $l10n, IConfig $config, ?IRequest $request = null, ?TrustedServers $trustedServers = null) {
parent::__construct($carddavBackend, $addressBookInfo, $l10n);
$this->config = $config;
$this->request = $request;
$this->trustedServers = $trustedServers;
}
public function getChildren() {
public function getChildren(): array {
$shareEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
$shareEnumerationGroup = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes';
$shareEnumerationPhone = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes';
@ -50,4 +64,165 @@ class SystemAddressbook extends AddressBook {
return parent::getChildren();
}
/**
* @param array $paths
* @return Card[]
* @throws NotFound
*/
public function getMultipleChildren($paths): array {
if (!$this->isFederation()) {
return parent::getMultipleChildren($paths);
}
$objs = $this->carddavBackend->getMultipleCards($this->addressBookInfo['id'], $paths);
$children = [];
/** @var array $obj */
foreach ($objs as $obj) {
if (empty($obj)) {
continue;
}
$carddata = $this->extractCarddata($obj);
if (empty($carddata)) {
continue;
} else {
$obj['carddata'] = $carddata;
}
$children[] = new Card($this->carddavBackend, $this->addressBookInfo, $obj);
}
return $children;
}
/**
* @param string $name
* @return Card
* @throws NotFound
* @throws Forbidden
*/
public function getChild($name): Card {
if (!$this->isFederation()) {
return parent::getChild($name);
}
$obj = $this->carddavBackend->getCard($this->addressBookInfo['id'], $name);
if (!$obj) {
throw new NotFound('Card not found');
}
$carddata = $this->extractCarddata($obj);
if (empty($carddata)) {
throw new Forbidden();
} else {
$obj['carddata'] = $carddata;
}
return new Card($this->carddavBackend, $this->addressBookInfo, $obj);
}
/**
* @throws UnsupportedLimitOnInitialSyncException
*/
public function getChanges($syncToken, $syncLevel, $limit = null) {
if (!$syncToken && $limit) {
throw new UnsupportedLimitOnInitialSyncException();
}
if (!$this->carddavBackend instanceof SyncSupport) {
return null;
}
if (!$this->isFederation()) {
return parent::getChanges($syncToken, $syncLevel, $limit);
}
$changed = $this->carddavBackend->getChangesForAddressBook(
$this->addressBookInfo['id'],
$syncToken,
$syncLevel,
$limit
);
if (empty($changed)) {
return $changed;
}
$added = $modified = $deleted = [];
foreach ($changed['added'] as $uri) {
try {
$this->getChild($uri);
$added[] = $uri;
} catch (NotFound | Forbidden $e) {
$deleted[] = $uri;
}
}
foreach ($changed['modified'] as $uri) {
try {
$this->getChild($uri);
$modified[] = $uri;
} catch (NotFound | Forbidden $e) {
$deleted[] = $uri;
}
}
$changed['added'] = $added;
$changed['modified'] = $modified;
$changed['deleted'] = $deleted;
return $changed;
}
private function isFederation(): bool {
if ($this->trustedServers === null || $this->request === null) {
return false;
}
/** @psalm-suppress NoInterfaceProperties */
if ($this->request->server['PHP_AUTH_USER'] !== 'system') {
return false;
}
/** @psalm-suppress NoInterfaceProperties */
$sharedSecret = $this->request->server['PHP_AUTH_PW'];
if ($sharedSecret === null) {
return false;
}
$servers = $this->trustedServers->getServers();
$trusted = array_filter($servers, function ($trustedServer) use ($sharedSecret) {
return $trustedServer['shared_secret'] === $sharedSecret;
});
// Authentication is fine, but it's not for a federated share
if (empty($trusted)) {
return false;
}
return true;
}
/**
* If the validation doesn't work the card is "not found" so we
* return empty carddata even if the carddata might exist in the local backend.
* This can happen when a user sets the required properties
* FN, N to a local scope only but the request is from
* a federated share.
*
* @see https://github.com/nextcloud/server/issues/38042
*
* @param array $obj
* @return string|null
*/
private function extractCarddata(array $obj): ?string {
$obj['acl'] = $this->getChildACL();
$cardData = $obj['carddata'];
/** @var VCard $vCard */
$vCard = Reader::read($cardData);
foreach ($vCard->children() as $child) {
$scope = $child->offsetGet('X-NC-SCOPE');
if ($scope !== null && $scope->getValue() === IAccountManager::SCOPE_LOCAL) {
$vCard->remove($child);
}
}
$messages = $vCard->validate();
if (!empty($messages)) {
return null;
}
return $vCard->serialize();
}
}

@ -30,8 +30,13 @@ namespace OCA\DAV\CardDAV;
use OCA\DAV\AppInfo\PluginManager;
use OCA\DAV\CardDAV\Integration\IAddressBookProvider;
use OCA\DAV\CardDAV\Integration\ExternalAddressBook;
use OCA\Federation\TrustedServers;
use OCP\AppFramework\QueryException;
use OCP\IConfig;
use OCP\IL10N;
use OCP\IRequest;
use Psr\Container\ContainerExceptionInterface;
use Psr\Container\NotFoundExceptionInterface;
use Sabre\CardDAV\Backend;
use Sabre\DAV\Exception\MethodNotAllowed;
use Sabre\CardDAV\IAddressBook;
@ -73,7 +78,15 @@ class UserAddressBooks extends \Sabre\CardDAV\AddressBookHome {
/** @var IAddressBook[] $objects */
$objects = array_map(function (array $addressBook) {
if ($addressBook['principaluri'] === 'principals/system/system') {
return new SystemAddressbook($this->carddavBackend, $addressBook, $this->l10n, $this->config);
$trustedServers = null;
$request = null;
try {
$trustedServers = \OC::$server->get(TrustedServers::class);
$request = \OC::$server->get(IRequest::class);
} catch (NotFoundExceptionInterface | ContainerExceptionInterface $e) {
// nothing to do, the request / trusted servers don't exist
}
return new SystemAddressbook($this->carddavBackend, $addressBook, $this->l10n, $this->config, $request, $trustedServers);
}
return new AddressBook($this->carddavBackend, $addressBook, $this->l10n);

@ -0,0 +1,54 @@
<?php
declare(strict_types=1);
/**
* @copyright Copyright (c) 2023 Anna Larch <anna.larch@gmx.net>
*
* @author Anna Larch <anna.larch@gmx.net>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* 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
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
namespace OCA\DAV\Migration;
use Closure;
use OCA\DAV\CardDAV\SyncService;
use OCP\DB\ISchemaWrapper;
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;
use Psr\Container\ContainerExceptionInterface;
use Psr\Container\NotFoundExceptionInterface;
use Psr\Log\LoggerInterface;
class Version1027Date20230504122946 extends SimpleMigrationStep {
private SyncService $syncService;
private LoggerInterface $logger;
public function __construct(SyncService $syncService, LoggerInterface $logger) {
$this->syncService = $syncService;
$this->logger = $logger;
}
/**
* @param IOutput $output
* @param Closure(): ISchemaWrapper $schemaClosure
* @param array $options
*/
public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void {
$this->syncService->syncInstance();
}
}

@ -70,17 +70,15 @@ class ConverterTest extends TestCase {
public function getAccountManager(IUser $user) {
$account = $this->createMock(IAccount::class);
$account->expects($this->any())
->method('getProperties')
->method('getAllProperties')
->willReturnCallback(function () use ($user) {
return [
$this->getAccountPropertyMock(IAccountManager::PROPERTY_DISPLAYNAME, $user->getDisplayName(), IAccountManager::SCOPE_FEDERATED),
$this->getAccountPropertyMock(IAccountManager::PROPERTY_ADDRESS, '', IAccountManager::SCOPE_LOCAL),
$this->getAccountPropertyMock(IAccountManager::PROPERTY_WEBSITE, '', IAccountManager::SCOPE_LOCAL),
$this->getAccountPropertyMock(IAccountManager::PROPERTY_EMAIL, $user->getEMailAddress(), IAccountManager::SCOPE_FEDERATED),
$this->getAccountPropertyMock(IAccountManager::PROPERTY_AVATAR, $user->getAvatarImage(-1)->data(), IAccountManager::SCOPE_FEDERATED),
$this->getAccountPropertyMock(IAccountManager::PROPERTY_PHONE, '', IAccountManager::SCOPE_LOCAL),
$this->getAccountPropertyMock(IAccountManager::PROPERTY_TWITTER, '', IAccountManager::SCOPE_LOCAL),
];
yield $this->getAccountPropertyMock(IAccountManager::PROPERTY_DISPLAYNAME, $user->getDisplayName(), IAccountManager::SCOPE_FEDERATED);
yield $this->getAccountPropertyMock(IAccountManager::PROPERTY_ADDRESS, '', IAccountManager::SCOPE_LOCAL);
yield $this->getAccountPropertyMock(IAccountManager::PROPERTY_WEBSITE, '', IAccountManager::SCOPE_LOCAL);
yield $this->getAccountPropertyMock(IAccountManager::PROPERTY_EMAIL, $user->getEMailAddress(), IAccountManager::SCOPE_FEDERATED);
yield $this->getAccountPropertyMock(IAccountManager::PROPERTY_AVATAR, $user->getAvatarImage(-1)->data(), IAccountManager::SCOPE_FEDERATED);
yield $this->getAccountPropertyMock(IAccountManager::PROPERTY_PHONE, '', IAccountManager::SCOPE_LOCAL);
yield $this->getAccountPropertyMock(IAccountManager::PROPERTY_TWITTER, '', IAccountManager::SCOPE_LOCAL);
});
$accountManager = $this->getMockBuilder(IAccountManager::class)

@ -0,0 +1,123 @@
<?php
declare(strict_types=1);
/*
* @copyright 2023 Christoph Wurst <christoph@winzerhof-wurst.at>
*
* @author 2023 Christoph Wurst <christoph@winzerhof-wurst.at>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* 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
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
namespace OCA\DAV\Tests\unit\CardDAV;
use OC\AppFramework\Http\Request;
use OCA\DAV\CardDAV\SystemAddressbook;
use OCA\Federation\TrustedServers;
use OCP\Accounts\IAccountManager;
use OCP\IConfig;
use OCP\IL10N;
use OCP\IRequest;
use PHPUnit\Framework\MockObject\MockObject;
use Sabre\CardDAV\Backend\BackendInterface;
use Sabre\VObject\Component\VCard;
use Sabre\VObject\Reader;
use Test\TestCase;
class SystemAddressBookTest extends TestCase {
private MockObject|BackendInterface $cardDavBackend;
private array $addressBookInfo;
private IL10N|MockObject $l10n;
private IConfig|MockObject $config;
private IRequest|MockObject $request;
private array $server;
private TrustedServers|MockObject $trustedServers;
private SystemAddressbook $addressBook;
protected function setUp(): void {
parent::setUp();
$this->cardDavBackend = $this->createMock(BackendInterface::class);
$this->addressBookInfo = [
'id' => 123,
'{DAV:}displayname' => 'Accounts',
'principaluri' => 'principals/system/system',
];
$this->l10n = $this->createMock(IL10N::class);
$this->config = $this->createMock(IConfig::class);
$this->request = $this->createMock(Request::class);
$this->server = [
'PHP_AUTH_USER' => 'system',
'PHP_AUTH_PW' => 'shared123',
];
$this->request->method('__get')->with('server')->willReturn($this->server);
$this->trustedServers = $this->createMock(TrustedServers::class);
$this->addressBook = new SystemAddressbook(
$this->cardDavBackend,
$this->addressBookInfo,
$this->l10n,
$this->config,
$this->request,
$this->trustedServers,
);
}
public function testGetFilteredChildForFederation(): void {
$this->trustedServers->expects(self::once())
->method('getServers')
->willReturn([
[
'shared_secret' => 'shared123',
],
]);
$vcfWithScopes = <<<VCF
BEGIN:VCARD
VERSION:3.0
PRODID:-//Sabre//Sabre VObject 4.4.2//EN
UID:admin
FN;X-NC-SCOPE=v2-federated:admin
N;X-NC-SCOPE=v2-federated:admin;;;;
ADR;TYPE=OTHER;X-NC-SCOPE=v2-local:Testing test test test;;;;;;
EMAIL;TYPE=OTHER;X-NC-SCOPE=v2-federated:miau_lalala@gmx.net
TEL;TYPE=OTHER;X-NC-SCOPE=v2-local:+435454454544
CLOUD:admin@http://localhost
END:VCARD
VCF;
$originalCard = [
'carddata' => $vcfWithScopes,
];
$this->cardDavBackend->expects(self::once())
->method('getCard')
->with(123, 'user.vcf')
->willReturn($originalCard);
$card = $this->addressBook->getChild("user.vcf");
/** @var VCard $vCard */
$vCard = Reader::read($card->get());
foreach ($vCard->children() as $child) {
$scope = $child->offsetGet('X-NC-SCOPE');
if ($scope !== null) {
self::assertNotEquals(IAccountManager::SCOPE_PRIVATE, $scope->getValue());
self::assertNotEquals(IAccountManager::SCOPE_LOCAL, $scope->getValue());
}
}
}
}

@ -418,6 +418,14 @@
<code>string|null</code>
</ImplementedReturnTypeMismatch>
</file>
<file src="apps/dav/lib/CardDAV/SystemAddressbook.php">
<InvalidNullableReturnType>
<code>getChanges</code>
</InvalidNullableReturnType>
<NullableReturnStatement>
<code>null</code>
</NullableReturnStatement>
</file>
<file src="apps/dav/lib/CardDAV/UserAddressBooks.php">
<InvalidArgument>
<code><![CDATA[$this->principalUri]]></code>