fix(carddav): Check enumeration settings for all SAB methods

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

diff --git a/apps/dav/lib/CardDAV/SystemAddressbook.php b/apps/dav/lib/CardDAV/SystemAddressbook.php
index c936ad2344ff746a24d8ca9eff9ad7a408caf1fe..48ec533353a94b77a8bd0c2c6014b28d6e536bdb 100644 (file)
--- a/apps/dav/lib/CardDAV/SystemAddressbook.php
+++ b/apps/dav/lib/CardDAV/SystemAddressbook.php
@@ -44,7 +44,9 @@ use Sabre\DAV\Exception\NotFound;
 use Sabre\VObject\Component\VCard;
 use Sabre\VObject\Reader;
 use function array_filter;
+use function array_intersect;
 use function array_unique;
+use function in_array;
 
 class SystemAddressbook extends AddressBook {
        public const URI_SHARED = 'z-server-generated--system';
@@ -90,7 +92,7 @@ class SystemAddressbook extends AddressBook {
                        // Should never happen because we don't allow anonymous access
                        return [];
                }
-               if (!$shareEnumeration || !$shareEnumerationGroup && $shareEnumerationPhone) {
+               if (!$shareEnumeration || (!$shareEnumerationGroup && $shareEnumerationPhone)) {
                        $name = SyncService::getCardUri($user);
                        try {
                                return [parent::getChild($name)];
@@ -130,6 +132,41 @@ class SystemAddressbook extends AddressBook {
         * @throws NotFound
         */
        public function getMultipleChildren($paths): 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';
+               if (!$shareEnumeration || (!$shareEnumerationGroup && $shareEnumerationPhone)) {
+                       $user = $this->userSession->getUser();
+                       // No user or cards with no access
+                       if ($user === null || !in_array(SyncService::getCardUri($user), $paths, true)) {
+                               return [];
+                       }
+                       // Only return the own card
+                       try {
+                               return [parent::getChild(SyncService::getCardUri($user))];
+                       } catch (NotFound $e) {
+                               return [];
+                       }
+               }
+               if ($shareEnumerationGroup) {
+                       $user = $this->userSession->getUser();
+                       if ($this->groupManager === null || $user === null) {
+                               // Group manager or user is not available, so we can't determine which data is safe
+                               return [];
+                       }
+                       $groups = $this->groupManager->getUserGroups($user);
+                       $allowedNames = [];
+                       foreach ($groups as $group) {
+                               $users = $group->getUsers();
+                               foreach ($users as $groupUser) {
+                                       if ($groupUser->getBackendClassName() === 'Guests') {
+                                               continue;
+                                       }
+                                       $allowedNames[] = SyncService::getCardUri($groupUser);
+                               }
+                       }
+                       return parent::getMultipleChildren(array_intersect($paths, $allowedNames));
+               }
                if (!$this->isFederation()) {
                        return parent::getMultipleChildren($paths);
                }
@@ -159,6 +196,37 @@ class SystemAddressbook extends AddressBook {
         * @throws Forbidden
         */
        public function getChild($name): Card {
+               $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';
+               if (!$shareEnumeration || (!$shareEnumerationGroup && $shareEnumerationPhone)) {
+                       $currentUser = $this->userSession->getUser();
+                       $ownName = $currentUser !== null ? SyncService::getCardUri($currentUser) : null;
+                       if ($ownName === $name) {
+                               return parent::getChild($name);
+                       }
+                       throw new Forbidden();
+               }
+               if ($shareEnumerationGroup) {
+                       $user = $this->userSession->getUser();
+                       if ($user === null || $this->groupManager === null) {
+                               // Group manager is not available, so we can't determine which data is safe
+                               throw new Forbidden();
+                       }
+                       $groups = $this->groupManager->getUserGroups($user);
+                       foreach ($groups as $group) {
+                               foreach ($group->getUsers() as $groupUser) {
+                                       if ($groupUser->getBackendClassName() === 'Guests') {
+                                               continue;
+                                       }
+                                       $otherName = SyncService::getCardUri($groupUser);
+                                       if ($otherName === $name) {
+                                               return parent::getChild($name);
+                                       }
+                               }
+                       }
+                       throw new Forbidden();
+               }
                if (!$this->isFederation()) {
                        return parent::getChild($name);
                }

diff --git a/apps/dav/tests/unit/CardDAV/SystemAddressBookTest.php b/apps/dav/tests/unit/CardDAV/SystemAddressBookTest.php
index a753a1c5a7342647880b4bcdb841f9e2a86bfd05..325b1120e8b27e6a71df84c259a82a4252ecefbe 100644 (file)
--- a/apps/dav/tests/unit/CardDAV/SystemAddressBookTest.php
+++ b/apps/dav/tests/unit/CardDAV/SystemAddressBookTest.php
@@ -26,21 +26,26 @@ declare(strict_types=1);
 namespace OCA\DAV\Tests\unit\CardDAV;
 
 use OC\AppFramework\Http\Request;
+use OCA\DAV\CardDAV\SyncService;
 use OCA\DAV\CardDAV\SystemAddressbook;
 use OCA\Federation\TrustedServers;
 use OCP\Accounts\IAccountManager;
 use OCP\IConfig;
+use OCP\IGroup;
+use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\IRequest;
+use OCP\IUser;
 use OCP\IUserSession;
 use PHPUnit\Framework\MockObject\MockObject;
 use Sabre\CardDAV\Backend\BackendInterface;
+use Sabre\DAV\Exception\Forbidden;
+use Sabre\DAV\Exception\NotFound;
 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;
@@ -49,6 +54,7 @@ class SystemAddressBookTest extends TestCase {
        private IRequest|MockObject $request;
        private array $server;
        private TrustedServers|MockObject $trustedServers;
+       private IGroupManager|MockObject $groupManager;
        private SystemAddressbook $addressBook;
 
        protected function setUp(): void {
@@ -70,6 +76,7 @@ class SystemAddressBookTest extends TestCase {
                ];
                $this->request->method('__get')->with('server')->willReturn($this->server);
                $this->trustedServers = $this->createMock(TrustedServers::class);
+               $this->groupManager = $this->createMock(IGroupManager::class);
 
                $this->addressBook = new SystemAddressbook(
                        $this->cardDavBackend,
@@ -79,11 +86,18 @@ class SystemAddressBookTest extends TestCase {
                        $this->userSession,
                        $this->request,
                        $this->trustedServers,
-                       null,
+                       $this->groupManager,
                );
        }
 
        public function testGetFilteredChildForFederation(): void {
+               $this->config->expects(self::exactly(3))
+                       ->method('getAppValue')
+                       ->willReturnMap([
+                               ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'],
+                               ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'],
+                               ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'],
+                       ]);
                $this->trustedServers->expects(self::once())
                        ->method('getServers')
                        ->willReturn([
@@ -125,4 +139,228 @@ VCF;
                }
        }
 
+       public function testGetChildNotFound(): void {
+               $this->config->expects(self::exactly(3))
+                       ->method('getAppValue')
+                       ->willReturnMap([
+                               ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'],
+                               ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'],
+                               ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'],
+                       ]);
+               $this->trustedServers->expects(self::once())
+                       ->method('getServers')
+                       ->willReturn([
+                               [
+                                       'shared_secret' => 'shared123',
+                               ],
+                       ]);
+               $this->expectException(NotFound::class);
+
+               $this->addressBook->getChild("LDAP:user.vcf");
+       }
+
+       public function testGetChildWithoutEnumeration(): void {
+               $this->config->expects(self::exactly(3))
+                       ->method('getAppValue')
+                       ->willReturnMap([
+                               ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'no'],
+                               ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'],
+                               ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'],
+                       ]);
+               $this->expectException(Forbidden::class);
+
+               $this->addressBook->getChild("LDAP:user.vcf");
+       }
+
+       public function testGetChildWithGroupEnumerationRestriction(): void {
+               $this->config->expects(self::exactly(3))
+                       ->method('getAppValue')
+                       ->willReturnMap([
+                               ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'],
+                               ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'yes'],
+                               ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'],
+                       ]);
+               $user = $this->createMock(IUser::class);
+               $user->method('getBackendClassName')->willReturn('LDAP');
+               $this->userSession->expects(self::once())
+                       ->method('getUser')
+                       ->willReturn($user);
+               $otherUser = $this->createMock(IUser::class);
+               $user->method('getBackendClassName')->willReturn('LDAP');
+               $otherUser->method('getUID')->willReturn('other');
+               $group = $this->createMock(IGroup::class);
+               $group->expects(self::once())
+                       ->method('getUsers')
+                       ->willReturn([$otherUser]);
+               $this->groupManager->expects(self::once())
+                       ->method('getUserGroups')
+                       ->with($user)
+                       ->willReturn([$group]);
+               $cardData = <<<VCF
+BEGIN:VCARD
+VERSION:3.0
+PRODID:-//Sabre//Sabre VObject 4.4.2//EN
+UID:admin
+FN;X-NC-SCOPE=v2-federated:other
+END:VCARD
+VCF;
+               $this->cardDavBackend->expects(self::once())
+                       ->method('getCard')
+                       ->with($this->addressBookInfo['id'], "{$otherUser->getBackendClassName()}:{$otherUser->getUID()}.vcf")
+                       ->willReturn([
+                               'id' => 123,
+                               'carddata' => $cardData,
+                       ]);
+
+               $this->addressBook->getChild("{$otherUser->getBackendClassName()}:{$otherUser->getUID()}.vcf");
+       }
+
+       public function testGetChildWithPhoneNumberEnumerationRestriction(): void {
+               $this->config->expects(self::exactly(3))
+                       ->method('getAppValue')
+                       ->willReturnMap([
+                               ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'],
+                               ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'],
+                               ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'yes'],
+                       ]);
+               $user = $this->createMock(IUser::class);
+               $user->method('getBackendClassName')->willReturn('LDAP');
+               $this->userSession->expects(self::once())
+                       ->method('getUser')
+                       ->willReturn($user);
+               $this->expectException(Forbidden::class);
+
+               $this->addressBook->getChild("LDAP:user.vcf");
+       }
+
+       public function testGetOwnChildWithPhoneNumberEnumerationRestriction(): void {
+               $this->config->expects(self::exactly(3))
+                       ->method('getAppValue')
+                       ->willReturnMap([
+                               ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'],
+                               ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'],
+                               ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'yes'],
+                       ]);
+               $user = $this->createMock(IUser::class);
+               $user->method('getBackendClassName')->willReturn('LDAP');
+               $user->method('getUID')->willReturn('user');
+               $this->userSession->expects(self::once())
+                       ->method('getUser')
+                       ->willReturn($user);
+               $cardData = <<<VCF
+BEGIN:VCARD
+VERSION:3.0
+PRODID:-//Sabre//Sabre VObject 4.4.2//EN
+UID:admin
+FN;X-NC-SCOPE=v2-federated:user
+END:VCARD
+VCF;
+               $this->cardDavBackend->expects(self::once())
+                       ->method('getCard')
+                       ->with($this->addressBookInfo['id'], 'LDAP:user.vcf')
+                       ->willReturn([
+                               'id' => 123,
+                               'carddata' => $cardData,
+                       ]);
+
+               $this->addressBook->getChild("LDAP:user.vcf");
+       }
+
+       public function testGetMultipleChildrenWithGroupEnumerationRestriction(): void {
+               $this->config
+                       ->method('getAppValue')
+                       ->willReturnMap([
+                               ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'],
+                               ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'yes'],
+                               ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'],
+                       ]);
+               $user = $this->createMock(IUser::class);
+               $user->method('getUID')->willReturn('user');
+               $user->method('getBackendClassName')->willReturn('LDAP');
+               $other1 = $this->createMock(IUser::class);
+               $other1->method('getUID')->willReturn('other1');
+               $other1->method('getBackendClassName')->willReturn('LDAP');
+               $other2 = $this->createMock(IUser::class);
+               $other2->method('getUID')->willReturn('other2');
+               $other2->method('getBackendClassName')->willReturn('LDAP');
+               $other3 = $this->createMock(IUser::class);
+               $other3->method('getUID')->willReturn('other3');
+               $other3->method('getBackendClassName')->willReturn('LDAP');
+               $this->userSession
+                       ->method('getUser')
+                       ->willReturn($user);
+               $group1 = $this->createMock(IGroup::class);
+               $group1
+                       ->method('getUsers')
+                       ->willReturn([$user, $other1]);
+               $group2 = $this->createMock(IGroup::class);
+               $group2
+                       ->method('getUsers')
+                       ->willReturn([$other1, $other2, $user]);
+               $this->groupManager
+                       ->method('getUserGroups')
+                       ->with($user)
+                       ->willReturn([$group1]);
+               $this->cardDavBackend->expects(self::once())
+                       ->method('getMultipleCards')
+                       ->with($this->addressBookInfo['id'], [
+                               SyncService::getCardUri($user),
+                               SyncService::getCardUri($other1),
+                       ])
+                       ->willReturn([
+                               [],
+                               [],
+                       ]);
+
+               $cards = $this->addressBook->getMultipleChildren([
+                       SyncService::getCardUri($user),
+                       SyncService::getCardUri($other1),
+                       // SyncService::getCardUri($other2), // Omitted to test that it's not returned as stray
+                       SyncService::getCardUri($other3), // No overlapping group with this one
+               ]);
+
+               self::assertCount(2, $cards);
+       }
+
+       public function testGetMultipleChildren(): void {
+               $this->config
+                       ->method('getAppValue')
+                       ->willReturnMap([
+                               ['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'],
+                               ['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'],
+                               ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'],
+                       ]);
+               $this->trustedServers
+                       ->method('getServers')
+                       ->willReturn([
+                               [
+                                       'shared_secret' => 'shared123',
+                               ],
+                       ]);
+               $cardData = <<<VCF
+BEGIN:VCARD
+VERSION:3.0
+PRODID:-//Sabre//Sabre VObject 4.4.2//EN
+UID:admin
+FN;X-NC-SCOPE=v2-federated:user
+END:VCARD
+VCF;
+               $this->cardDavBackend->expects(self::once())
+                       ->method('getMultipleCards')
+                       ->with($this->addressBookInfo['id'], ['Database:user1.vcf', 'LDAP:user2.vcf'])
+                       ->willReturn([
+                               [
+                                       'id' => 123,
+                                       'carddata' => $cardData,
+                               ],
+                               [
+                                       'id' => 321,
+                                       'carddata' => $cardData,
+                               ],
+                       ]);
+
+               $cards = $this->addressBook->getMultipleChildren(['Database:user1.vcf', 'LDAP:user2.vcf']);
+
+               self::assertCount(2, $cards);
+       }
 }