Merge pull request #51537 from nextcloud/backport/51380/stable29

[stable29] fix(cardav): only show users from enabled addressBooks in contacts menu
This commit is contained in:
Hamza 2025-03-21 16:09:52 +01:00 committed by GitHub
commit c896c5f32c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 167 additions and 22 deletions

View file

@ -285,12 +285,11 @@ class Application extends App implements IBootstrap {
$cm->setupContactsProvider($contactsManager, $userID, $urlGenerator);
}
private function setupSystemContactsProvider(IContactsManager $contactsManager,
IAppContainer $container): void {
private function setupSystemContactsProvider(IContactsManager $contactsManager, IAppContainer $container): void {
/** @var ContactsManager $cm */
$cm = $container->query(ContactsManager::class);
$urlGenerator = $container->getServer()->getURLGenerator();
$cm->setupSystemContactsProvider($contactsManager, $urlGenerator);
$cm->setupSystemContactsProvider($contactsManager, null, $urlGenerator);
}
public function registerCalendarManager(ICalendarManager $calendarManager,

View file

@ -32,15 +32,16 @@
*/
namespace OCA\DAV\CardDAV;
use OCA\DAV\Db\PropertyMapper;
use OCP\Constants;
use OCP\IAddressBook;
use OCP\IAddressBookEnabled;
use OCP\IURLGenerator;
use Sabre\VObject\Component\VCard;
use Sabre\VObject\Property;
use Sabre\VObject\Reader;
use Sabre\VObject\UUIDUtil;
class AddressBookImpl implements IAddressBook {
class AddressBookImpl implements IAddressBookEnabled {
/** @var CardDavBackend */
private $backend;
@ -54,6 +55,12 @@ class AddressBookImpl implements IAddressBook {
/** @var IURLGenerator */
private $urlGenerator;
/** @var PropertyMapper */
private $propertyMapper;
/** @var string|null */
private $userId;
/**
* AddressBookImpl constructor.
*
@ -61,16 +68,22 @@ class AddressBookImpl implements IAddressBook {
* @param array $addressBookInfo
* @param CardDavBackend $backend
* @param IUrlGenerator $urlGenerator
* @param PropertyMapper $propertyMapper
* @param string|null $userId
*/
public function __construct(
AddressBook $addressBook,
array $addressBookInfo,
CardDavBackend $backend,
IURLGenerator $urlGenerator) {
IURLGenerator $urlGenerator,
PropertyMapper $propertyMapper,
?string $userId) {
$this->addressBook = $addressBook;
$this->addressBookInfo = $addressBookInfo;
$this->backend = $backend;
$this->urlGenerator = $urlGenerator;
$this->propertyMapper = $propertyMapper;
$this->userId = $userId;
}
/**
@ -348,4 +361,25 @@ class AddressBookImpl implements IAddressBook {
$this->addressBookInfo['{DAV:}displayname'] === $this->urlGenerator->getBaseUrl()
);
}
public function isEnabled(): bool {
if (!$this->userId) {
return true;
}
if ($this->isSystemAddressBook()) {
$user = $this->userId ;
$uri = 'z-server-generated--system';
} else {
$user = str_replace('principals/users/', '', $this->addressBookInfo['principaluri']);
$uri = $this->addressBookInfo['uri'];
}
$path = 'addressbooks/users/' . $user . '/' . $uri;
$properties = $this->propertyMapper->findPropertyByPathAndName($user, $path, '{http://owncloud.org/ns}enabled');
if (count($properties) > 0) {
return (bool)$properties[0]->getPropertyvalue();
}
return true;
}
}

View file

@ -25,6 +25,7 @@
*/
namespace OCA\DAV\CardDAV;
use OCA\DAV\Db\PropertyMapper;
use OCP\Contacts\IManager;
use OCP\IL10N;
use OCP\IURLGenerator;
@ -36,15 +37,19 @@ class ContactsManager {
/** @var IL10N */
private $l10n;
/** @var PropertyMapper */
private $propertyMapper;
/**
* ContactsManager constructor.
*
* @param CardDavBackend $backend
* @param IL10N $l10n
*/
public function __construct(CardDavBackend $backend, IL10N $l10n) {
public function __construct(CardDavBackend $backend, IL10N $l10n, PropertyMapper $propertyMapper) {
$this->backend = $backend;
$this->l10n = $l10n;
$this->propertyMapper = $propertyMapper;
}
/**
@ -54,25 +59,27 @@ class ContactsManager {
*/
public function setupContactsProvider(IManager $cm, $userId, IURLGenerator $urlGenerator) {
$addressBooks = $this->backend->getAddressBooksForUser("principals/users/$userId");
$this->register($cm, $addressBooks, $urlGenerator);
$this->setupSystemContactsProvider($cm, $urlGenerator);
$this->register($cm, $addressBooks, $urlGenerator, $userId);
$this->setupSystemContactsProvider($cm, $userId, $urlGenerator);
}
/**
* @param IManager $cm
* @param ?string $userId
* @param IURLGenerator $urlGenerator
*/
public function setupSystemContactsProvider(IManager $cm, IURLGenerator $urlGenerator) {
public function setupSystemContactsProvider(IManager $cm, ?string $userId, IURLGenerator $urlGenerator) {
$addressBooks = $this->backend->getAddressBooksForUser("principals/system/system");
$this->register($cm, $addressBooks, $urlGenerator);
$this->register($cm, $addressBooks, $urlGenerator, $userId);
}
/**
* @param IManager $cm
* @param $addressBooks
* @param IURLGenerator $urlGenerator
* @param ?string $userId
*/
private function register(IManager $cm, $addressBooks, $urlGenerator) {
private function register(IManager $cm, $addressBooks, $urlGenerator, ?string $userId) {
foreach ($addressBooks as $addressBookInfo) {
$addressBook = new \OCA\DAV\CardDAV\AddressBook($this->backend, $addressBookInfo, $this->l10n);
$cm->registerAddressBook(
@ -80,7 +87,9 @@ class ContactsManager {
$addressBook,
$addressBookInfo,
$this->backend,
$urlGenerator
$urlGenerator,
$this->propertyMapper,
$userId,
)
);
}

View file

@ -33,6 +33,7 @@ namespace OCA\DAV\Tests\unit\CardDAV;
use OCA\DAV\CardDAV\AddressBook;
use OCA\DAV\CardDAV\AddressBookImpl;
use OCA\DAV\CardDAV\CardDavBackend;
use OCA\DAV\Db\PropertyMapper;
use OCP\IURLGenerator;
use Sabre\VObject\Component\VCard;
use Sabre\VObject\Property\Text;
@ -58,6 +59,9 @@ class AddressBookImplTest extends TestCase {
/** @var VCard | \PHPUnit\Framework\MockObject\MockObject */
private $vCard;
/** @var PropertyMapper | \PHPUnit\Framework\MockObject\MockObject */
private $propertyMapper;
protected function setUp(): void {
parent::setUp();
@ -73,12 +77,15 @@ class AddressBookImplTest extends TestCase {
->disableOriginalConstructor()->getMock();
$this->vCard = $this->createMock(VCard::class);
$this->urlGenerator = $this->createMock(IURLGenerator::class);
$this->propertyMapper = $this->createMock(PropertyMapper::class);
$this->addressBookImpl = new AddressBookImpl(
$this->addressBook,
$this->addressBookInfo,
$this->backend,
$this->urlGenerator
$this->urlGenerator,
$this->propertyMapper,
null
);
}
@ -101,6 +108,8 @@ class AddressBookImplTest extends TestCase {
$this->addressBookInfo,
$this->backend,
$this->urlGenerator,
$this->propertyMapper,
null
]
)
->setMethods(['vCard2Array', 'readCard'])
@ -147,6 +156,8 @@ class AddressBookImplTest extends TestCase {
$this->addressBookInfo,
$this->backend,
$this->urlGenerator,
$this->propertyMapper,
null
]
)
->setMethods(['vCard2Array', 'createUid', 'createEmptyVCard'])
@ -197,6 +208,8 @@ class AddressBookImplTest extends TestCase {
$this->addressBookInfo,
$this->backend,
$this->urlGenerator,
$this->propertyMapper,
null
]
)
->setMethods(['vCard2Array', 'createUid', 'createEmptyVCard', 'readCard'])
@ -234,6 +247,8 @@ class AddressBookImplTest extends TestCase {
$this->addressBookInfo,
$this->backend,
$this->urlGenerator,
$this->propertyMapper,
null
]
)
->setMethods(['vCard2Array', 'createUid', 'createEmptyVCard', 'readCard'])
@ -315,6 +330,8 @@ class AddressBookImplTest extends TestCase {
$this->addressBookInfo,
$this->backend,
$this->urlGenerator,
$this->propertyMapper,
null
]
)
->setMethods(['getUid'])
@ -511,7 +528,9 @@ class AddressBookImplTest extends TestCase {
$this->addressBook,
$addressBookInfo,
$this->backend,
$this->urlGenerator
$this->urlGenerator,
$this->propertyMapper,
null
);
$this->assertTrue($addressBookImpl->isSystemAddressBook());
@ -530,7 +549,9 @@ class AddressBookImplTest extends TestCase {
$this->addressBook,
$addressBookInfo,
$this->backend,
$this->urlGenerator
$this->urlGenerator,
$this->propertyMapper,
'user2'
);
$this->assertFalse($addressBookImpl->isSystemAddressBook());
@ -550,7 +571,9 @@ class AddressBookImplTest extends TestCase {
$this->addressBook,
$addressBookInfo,
$this->backend,
$this->urlGenerator
$this->urlGenerator,
$this->propertyMapper,
'user2'
);
$this->assertFalse($addressBookImpl->isSystemAddressBook());

View file

@ -27,6 +27,7 @@ namespace OCA\DAV\Tests\unit\CardDAV;
use OCA\DAV\CardDAV\CardDavBackend;
use OCA\DAV\CardDAV\ContactsManager;
use OCA\DAV\Db\PropertyMapper;
use OCP\Contacts\IManager;
use OCP\IL10N;
use OCP\IURLGenerator;
@ -43,9 +44,10 @@ class ContactsManagerTest extends TestCase {
$backEnd->method('getAddressBooksForUser')->willReturn([
['{DAV:}displayname' => 'Test address book', 'uri' => 'default'],
]);
$propertyMapper = $this->createMock(PropertyMapper::class);
$l = $this->createMock(IL10N::class);
$app = new ContactsManager($backEnd, $l);
$app = new ContactsManager($backEnd, $l, $propertyMapper);
$app->setupContactsProvider($cm, 'user01', $urlGenerator);
}
}

View file

@ -491,6 +491,7 @@ return array(
'OCP\\Http\\WellKnown\\IResponse' => $baseDir . '/lib/public/Http/WellKnown/IResponse.php',
'OCP\\Http\\WellKnown\\JrdResponse' => $baseDir . '/lib/public/Http/WellKnown/JrdResponse.php',
'OCP\\IAddressBook' => $baseDir . '/lib/public/IAddressBook.php',
'OCP\\IAddressBookEnabled' => $baseDir . '/lib/public/IAddressBookEnabled.php',
'OCP\\IAppConfig' => $baseDir . '/lib/public/IAppConfig.php',
'OCP\\IAvatar' => $baseDir . '/lib/public/IAvatar.php',
'OCP\\IAvatarManager' => $baseDir . '/lib/public/IAvatarManager.php',

View file

@ -524,6 +524,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'OCP\\Http\\WellKnown\\IResponse' => __DIR__ . '/../../..' . '/lib/public/Http/WellKnown/IResponse.php',
'OCP\\Http\\WellKnown\\JrdResponse' => __DIR__ . '/../../..' . '/lib/public/Http/WellKnown/JrdResponse.php',
'OCP\\IAddressBook' => __DIR__ . '/../../..' . '/lib/public/IAddressBook.php',
'OCP\\IAddressBookEnabled' => __DIR__ . '/../../..' . '/lib/public/IAddressBookEnabled.php',
'OCP\\IAppConfig' => __DIR__ . '/../../..' . '/lib/public/IAppConfig.php',
'OCP\\IAvatar' => __DIR__ . '/../../..' . '/lib/public/IAvatar.php',
'OCP\\IAvatarManager' => __DIR__ . '/../../..' . '/lib/public/IAvatarManager.php',

View file

@ -30,6 +30,7 @@ namespace OC;
use OCP\Constants;
use OCP\Contacts\IManager;
use OCP\IAddressBook;
use OCP\IAddressBookEnabled;
class ContactsManager implements IManager {
/**
@ -54,6 +55,9 @@ class ContactsManager implements IManager {
$this->loadAddressBooks();
$result = [];
foreach ($this->addressBooks as $addressBook) {
if ($addressBook instanceof IAddressBookEnabled && !$addressBook->isEnabled()) {
continue;
}
$searchOptions = $options;
$strictSearch = array_key_exists('strict_search', $options) && $options['strict_search'] === true;

View file

@ -0,0 +1,26 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors"
* SPDX-License-Identifier: AGPL-3.0-only
*/
// use OCP namespace for all classes that are considered public.
// This means that they should be used by apps instead of the internal Nextcloud classes
namespace OCP;
/**
* IAddressBook Interface extension for checking if the address book is enabled
*
* @since 32.0.0
*/
interface IAddressBookEnabled extends IAddressBook {
/**
* Check if the address book is enabled
* @return bool
* @since 32.0.0
*/
public function isEnabled(): bool;
}

View file

@ -63,10 +63,14 @@ class ContactsManagerTest extends \Test\TestCase {
*/
public function testSearch($search1, $search2, $expectedResult) {
/** @var \PHPUnit\Framework\MockObject\MockObject|IAddressBook $addressbook */
$addressbook1 = $this->getMockBuilder('\OCP\IAddressBook')
$addressbook1 = $this->getMockBuilder('\OCP\IAddressBookEnabled')
->disableOriginalConstructor()
->getMock();
$addressbook1->expects($this->once())
->method('isEnabled')
->willReturn(true);
$addressbook1->expects($this->once())
->method('search')
->willReturn($search1);
@ -75,10 +79,14 @@ class ContactsManagerTest extends \Test\TestCase {
->method('getKey')
->willReturn('simple:1');
$addressbook2 = $this->getMockBuilder('\OCP\IAddressBook')
$addressbook2 = $this->getMockBuilder('\OCP\IAddressBookEnabled')
->disableOriginalConstructor()
->getMock();
$addressbook2->expects($this->once())
->method('isEnabled')
->willReturn(true);
$addressbook2->expects($this->once())
->method('search')
->willReturn($search2);
@ -94,6 +102,44 @@ class ContactsManagerTest extends \Test\TestCase {
$this->assertEquals($expectedResult, $result);
}
/**
* @dataProvider searchProvider
*/
public function testSearchDisabledAb($search1): void {
/** @var \PHPUnit\Framework\MockObject\MockObject|IAddressBookEnabled $addressbook */
$addressbook1 = $this->getMockBuilder('\OCP\IAddressBookEnabled')
->disableOriginalConstructor()
->getMock();
$addressbook1->expects($this->once())
->method('isEnabled')
->willReturn(true);
$addressbook1->expects($this->once())
->method('search')
->willReturn($search1);
$addressbook1->expects($this->any())
->method('getKey')
->willReturn('simple:1');
$addressbook2 = $this->getMockBuilder('\OCP\IAddressBookEnabled')
->disableOriginalConstructor()
->getMock();
$addressbook2->expects($this->once())
->method('isEnabled')
->willReturn(false);
$addressbook2->expects($this->never())
->method('search');
$this->cm->registerAddressBook($addressbook1);
$this->cm->registerAddressBook($addressbook2);
$result = $this->cm->search('');
$this->assertEquals($search1, $result);
}
public function testDeleteHavePermission() {
/** @var \PHPUnit\Framework\MockObject\MockObject|IAddressBook $addressbook */
@ -243,8 +289,8 @@ class ContactsManagerTest extends \Test\TestCase {
public function testAddressBookEnumeration() {
// create mock for the addressbook
/** @var \PHPUnit\Framework\MockObject\MockObject|IAddressBook $addressbook */
$addressbook = $this->getMockBuilder('\OCP\IAddressBook')
/** @var \PHPUnit\Framework\MockObject\MockObject|IAddressBookEnabled $addressbook */
$addressbook = $this->getMockBuilder('\OCP\IAddressBookEnabled')
->disableOriginalConstructor()
->getMock();