From e524631ce3188ecaa16f83874a632f3702376e65 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Wed, 24 May 2023 22:27:51 +0200 Subject: [PATCH] fix(carddav): Don't show system address book cards to guests Signed-off-by: Christoph Wurst --- apps/dav/lib/CardDAV/SystemAddressbook.php | 14 ++-- .../unit/CardDAV/SystemAddressBookTest.php | 78 +++++++++++++++++++ 2 files changed, 84 insertions(+), 8 deletions(-) diff --git a/apps/dav/lib/CardDAV/SystemAddressbook.php b/apps/dav/lib/CardDAV/SystemAddressbook.php index 48ec533353a..1cffde63474 100644 --- 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 325b1120e8b..97bb92ad9bc 100644 --- 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 = << $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') -- 2.39.5