From f1838ab81185c1e1fa577aab463582b05652d7fa Mon Sep 17 00:00:00 2001 From: David Dreschner Date: Wed, 5 Aug 2026 13:42:16 +0200 Subject: [PATCH] fix(CalDAV): Check for user status before serving public calendars Signed-off-by: David Dreschner --- apps/dav/lib/CalDAV/PublicCalendarRoot.php | 32 +++++++++++ apps/dav/lib/RootCollection.php | 4 +- .../unit/CalDAV/PublicCalendarRootTest.php | 55 +++++++++++++++++-- lib/private/Sharing/SharingManager.php | 2 +- 4 files changed, 86 insertions(+), 7 deletions(-) diff --git a/apps/dav/lib/CalDAV/PublicCalendarRoot.php b/apps/dav/lib/CalDAV/PublicCalendarRoot.php index aec3f7b622486..96d5ca8a46039 100644 --- a/apps/dav/lib/CalDAV/PublicCalendarRoot.php +++ b/apps/dav/lib/CalDAV/PublicCalendarRoot.php @@ -8,10 +8,14 @@ namespace OCA\DAV\CalDAV; +use OCP\IAppConfig; use OCP\IConfig; use OCP\IL10N; +use OCP\IUserManager; use Psr\Log\LoggerInterface; use Sabre\DAV\Collection; +use Sabre\DAV\Exception\NotFound; +use Sabre\Uri; class PublicCalendarRoot extends Collection { @@ -21,12 +25,15 @@ class PublicCalendarRoot extends Collection { * @param CalDavBackend $caldavBackend * @param IL10N $l10n * @param IConfig $config + * @param IAppConfig $appConfig */ public function __construct( protected CalDavBackend $caldavBackend, protected IL10N $l10n, protected IConfig $config, + protected IAppConfig $appConfig, private LoggerInterface $logger, + private IUserManager $userManager, ) { } @@ -44,6 +51,9 @@ public function getName() { #[\Override] public function getChild($name) { $calendar = $this->caldavBackend->getPublicCalendar($name); + if (!$this->validateVisibility((string)$calendar['principaluri'])) { + throw new NotFound('Node with name \'' . $name . '\' could not be found'); + } return new PublicCalendar($this->caldavBackend, $calendar, $this->l10n, $this->config, $this->logger); } @@ -54,4 +64,26 @@ public function getChild($name) { public function getChildren() { return []; } + + /** + * Checks if the public calendar should be visible or not, based on + * the configuration of the `hide_disabled_user_shares` setting within + * `files_sharing` and the status of the owning user (disabled or not). + */ + private function validateVisibility(string $principalUri): bool { + $hideCalendarsOfDisabledUsers = $this->appConfig->getValueBool( + 'files_sharing', 'hide_disabled_user_shares', true + ); + + if (!$hideCalendarsOfDisabledUsers) { + return true; + } + + [$prefix, $name] = Uri\split($principalUri); + if ($prefix !== 'principals/users') { + return true; + } + + return $this->userManager->get((string)$name)?->isEnabled() !== false; + } } diff --git a/apps/dav/lib/RootCollection.php b/apps/dav/lib/RootCollection.php index aee0836038f77..41e5163afa26b 100644 --- a/apps/dav/lib/RootCollection.php +++ b/apps/dav/lib/RootCollection.php @@ -37,6 +37,7 @@ use OCP\Comments\ICommentsManager; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\IRootFolder; +use OCP\IAppConfig; use OCP\ICacheFactory; use OCP\IConfig; use OCP\IDBConnection; @@ -63,6 +64,7 @@ public function __construct() { $db = Server::get(IDBConnection::class); $dispatcher = Server::get(IEventDispatcher::class); $config = Server::get(IConfig::class); + $appConfig = Server::get(IAppConfig::class); $proxyMapper = Server::get(ProxyMapper::class); $rootFolder = Server::get(IRootFolder::class); $federatedCalendarFactory = Server::get(FederatedCalendarFactory::class); @@ -125,7 +127,7 @@ public function __construct() { $roomCalendarRoot = new CalendarRoot($calendarRoomPrincipalBackend, $caldavBackend, 'principals/calendar-rooms', $logger, $l10n, $config, $federatedCalendarFactory); $roomCalendarRoot->disableListing = $disableListing; - $publicCalendarRoot = new PublicCalendarRoot($caldavBackend, $l10n, $config, $logger); + $publicCalendarRoot = new PublicCalendarRoot($caldavBackend, $l10n, $config, $appConfig, $logger, $userManager); $systemTagCollection = Server::get(SystemTagsByIdCollection::class); $systemTagRelationsCollection = new SystemTagsRelationsCollection( diff --git a/apps/dav/tests/unit/CalDAV/PublicCalendarRootTest.php b/apps/dav/tests/unit/CalDAV/PublicCalendarRootTest.php index 71d5b1233e3e1..a10b6d18b3dc7 100644 --- a/apps/dav/tests/unit/CalDAV/PublicCalendarRootTest.php +++ b/apps/dav/tests/unit/CalDAV/PublicCalendarRootTest.php @@ -15,16 +15,19 @@ use OCA\DAV\CalDAV\PublicCalendarRoot; use OCA\DAV\Connector\Sabre\Principal; use OCP\EventDispatcher\IEventDispatcher; +use OCP\IAppConfig; use OCP\ICacheFactory; use OCP\IConfig; use OCP\IDBConnection; use OCP\IGroupManager; use OCP\IL10N; +use OCP\IUser; use OCP\IUserManager; use OCP\Security\ISecureRandom; use OCP\Server; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; +use Sabre\DAV\Exception\NotFound; use Test\TestCase; /** @@ -36,6 +39,7 @@ #[\PHPUnit\Framework\Attributes\Group(name: 'DB')] class PublicCalendarRootTest extends TestCase { public const UNIT_TEST_USER = ''; + private const DISABLED_USER_PRINCIPAL = 'principals/users/disabled-caldav-unit-test'; private CalDavBackend $backend; private PublicCalendarRoot $publicCalendarRoot; private IL10N&MockObject $l10n; @@ -43,6 +47,7 @@ class PublicCalendarRootTest extends TestCase { protected IUserManager&MockObject $userManager; protected IGroupManager&MockObject $groupManager; protected IConfig&MockObject $config; + protected IAppConfig&MockObject $appConfig; private ISecureRandom $random; private LoggerInterface&MockObject $logger; protected ICacheFactory&MockObject $cacheFactory; @@ -87,9 +92,10 @@ protected function setUp(): void { ); $this->l10n = $this->createMock(IL10N::class); $this->config = $this->createMock(IConfig::class); + $this->appConfig = $this->createMock(IAppConfig::class); $this->publicCalendarRoot = new PublicCalendarRoot($this->backend, - $this->l10n, $this->config, $this->logger); + $this->l10n, $this->config, $this->appConfig, $this->logger, $this->userManager); } protected function tearDown(): void { @@ -106,7 +112,10 @@ protected function tearDown(): void { ->withAnyParameters() ->willReturn([]); - $books = $this->backend->getCalendarsForUser(self::UNIT_TEST_USER); + $books = array_merge( + $this->backend->getCalendarsForUser(self::UNIT_TEST_USER), + $this->backend->getCalendarsForUser(self::DISABLED_USER_PRINCIPAL), + ); foreach ($books as $book) { $this->backend->deleteCalendar($book['id'], true); } @@ -136,10 +145,32 @@ public function testGetChildren(): void { $this->assertSame([], $calendarResults); } - protected function createPublicCalendar(): Calendar { - $this->backend->createCalendar(self::UNIT_TEST_USER, 'Example', []); + public function testGetChildHidesCalendarOfDisabledUser(): void { + $calendar = $this->createPublicCalendar(self::DISABLED_USER_PRINCIPAL); + $publicUri = $calendar->getPublishStatus(); + + $this->mockDisabledOwner(); + $this->setHideDisabledUserShares(true); + + $this->expectException(NotFound::class); + $this->publicCalendarRoot->getChild($publicUri); + } + + public function testGetChildServesCalendarOfDisabledUserWhenHidingIsDisabled(): void { + $calendar = $this->createPublicCalendar(self::DISABLED_USER_PRINCIPAL); + $publicUri = $calendar->getPublishStatus(); + + $this->mockDisabledOwner(); + $this->setHideDisabledUserShares(false); + + $calendarResult = $this->publicCalendarRoot->getChild($publicUri); + $this->assertEquals($calendar, $calendarResult); + } + + protected function createPublicCalendar(string $principal = self::UNIT_TEST_USER): Calendar { + $this->backend->createCalendar($principal, 'Example', []); - $calendarInfo = $this->backend->getCalendarsForUser(self::UNIT_TEST_USER)[0]; + $calendarInfo = $this->backend->getCalendarsForUser($principal)[0]; $calendar = new PublicCalendar($this->backend, $calendarInfo, $this->l10n, $this->config, $this->logger); $publicUri = $calendar->setPublishStatus(true); @@ -148,4 +179,18 @@ protected function createPublicCalendar(): Calendar { return $calendar; } + + private function mockDisabledOwner(): void { + $disabledUser = $this->createMock(IUser::class); + $disabledUser->method('isEnabled') + ->willReturn(false); + $this->userManager->method('get') + ->willReturn($disabledUser); + } + + private function setHideDisabledUserShares(bool $hide): void { + $this->appConfig->method('getValueBool') + ->with('files_sharing', 'hide_disabled_user_shares', 'yes') + ->willReturn($hide); + } } diff --git a/lib/private/Sharing/SharingManager.php b/lib/private/Sharing/SharingManager.php index 76593a8f4bc0b..dc96ad31a85ff 100644 --- a/lib/private/Sharing/SharingManager.php +++ b/lib/private/Sharing/SharingManager.php @@ -698,7 +698,7 @@ private function validateInteraction(ShareAccessContext $accessContext, Share $s $action = new ShareAction(null, array_values(array_map(static fn (SharePermission $permission): string => $permission->class, $share->getEnabledPermissions()))); $usersToCheck = []; - if ($share->owner->instance === null && ($ownerUser = $this->userManager->get($share->owner->userId)) !== null) { + if ($share->owner->instance === null && ($ownerUser = $this->userManager->get($share->owner->userId)) instanceof IUser) { $usersToCheck[] = $ownerUser; }