fix(carddav): Don't show system address book cards to guests

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 48ec533353a94b77a8bd0c2c6014b28d6e536bdb..1cffde634747ca10f8b895923ece575d576e5bf1 100644 (file)
--- a/apps/dav/lib/CardDAV/SystemAddressbook.php
+++ b/apps/dav/lib/CardDAV/SystemAddressbook.php
@@ -92,7 +92,7 @@ class SystemAddressbook extends AddressBook {
                        // Should never happen because we don't allow anonymous access
                        return [];
                }
-               if (!$shareEnumeration || (!$shareEnumerationGroup && $shareEnumerationPhone)) {
+               if ($user->getBackendClassName() === 'Guests' || !$shareEnumeration || (!$shareEnumerationGroup && $shareEnumerationPhone)) {
                        $name = SyncService::getCardUri($user);
                        try {
                                return [parent::getChild($name)];
@@ -135,8 +135,8 @@ class SystemAddressbook extends AddressBook {
                $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();
+               $user = $this->userSession->getUser();
+               if (($user !== null && $user->getBackendClassName() === 'Guests') || !$shareEnumeration || (!$shareEnumerationGroup && $shareEnumerationPhone)) {
                        // No user or cards with no access
                        if ($user === null || !in_array(SyncService::getCardUri($user), $paths, true)) {
                                return [];
@@ -149,7 +149,6 @@ class SystemAddressbook extends AddressBook {
                        }
                }
                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 [];
@@ -196,19 +195,18 @@ class SystemAddressbook extends AddressBook {
         * @throws Forbidden
         */
        public function getChild($name): Card {
+               $user = $this->userSession->getUser();
                $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 (($user !== null && $user->getBackendClassName() === 'Guests') || !$shareEnumeration || (!$shareEnumerationGroup && $shareEnumerationPhone)) {
+                       $ownName = $user !== null ? SyncService::getCardUri($user) : 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();

diff --git a/apps/dav/tests/unit/CardDAV/SystemAddressBookTest.php b/apps/dav/tests/unit/CardDAV/SystemAddressBookTest.php
index 325b1120e8b27e6a71df84c259a82a4252ecefbe..97bb92ad9bc98c25778ca5a2bf79e3bf47fa17c5 100644 (file)
--- a/apps/dav/tests/unit/CardDAV/SystemAddressBookTest.php
+++ b/apps/dav/tests/unit/CardDAV/SystemAddressBookTest.php
@@ -90,6 +90,46 @@ class SystemAddressBookTest extends TestCase {
                );
        }
 
+       public function testGetChildrenAsGuest(): 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'],
+                       ]);
+               $user = $this->createMock(IUser::class);
+               $user->method('getUID')->willReturn('user');
+               $user->method('getBackendClassName')->willReturn('Guests');
+               $this->userSession->expects(self::once())
+                       ->method('getUser')
+                       ->willReturn($user);
+               $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, 'Guests:user.vcf')
+                       ->willReturn($originalCard);
+
+               $children = $this->addressBook->getChildren();
+
+               self::assertCount(1, $children);
+       }
+
        public function testGetFilteredChildForFederation(): void {
                $this->config->expects(self::exactly(3))
                        ->method('getAppValue')
@@ -172,6 +212,24 @@ VCF;
                $this->addressBook->getChild("LDAP:user.vcf");
        }
 
+       public function testGetChildAsGuest(): 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'],
+                       ]);
+               $user = $this->createMock(IUser::class);
+               $user->method('getBackendClassName')->willReturn('Guests');
+               $this->userSession->expects(self::once())
+                       ->method('getUser')
+                       ->willReturn($user);
+               $this->expectException(Forbidden::class);
+
+               $this->addressBook->getChild("LDAP:user.vcf");
+       }
+
        public function testGetChildWithGroupEnumerationRestriction(): void {
                $this->config->expects(self::exactly(3))
                        ->method('getAppValue')
@@ -322,6 +380,26 @@ VCF;
                self::assertCount(2, $cards);
        }
 
+       public function testGetMultipleChildrenAsGuest(): 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'],
+                       ]);
+               $user = $this->createMock(IUser::class);
+               $user->method('getUID')->willReturn('user');
+               $user->method('getBackendClassName')->willReturn('Guests');
+               $this->userSession->expects(self::once())
+                       ->method('getUser')
+                       ->willReturn($user);
+
+               $cards = $this->addressBook->getMultipleChildren(['Database:user1.vcf', 'LDAP:user2.vcf']);
+
+               self::assertEmpty($cards);
+       }
+
        public function testGetMultipleChildren(): void {
                $this->config
                        ->method('getAppValue')