fix(CalDAV): Check for user status before serving public calendars

Signed-off-by: David Dreschner <david.dreschner@nextcloud.com>

Author: DerDreschner

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->getValueString(
+			'files_sharing', 'hide_disabled_user_shares', 'yes'
+		) === 'yes';
+
+		if (!$hideCalendarsOfDisabledUsers) {
+			return true;
+		}
+
+		[$prefix, $name] = Uri\split($principalUri);
+		if ($prefix !== 'principals/users') {
+			return true;
+		}
+
+		return $this->userManager->get((string)$name)?->isEnabled() !== false;
+	}
 }

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(

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,13 +39,15 @@
 #[\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;
 	private Principal&MockObject $principal;
 	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('getValueString')
+			->with('files_sharing', 'hide_disabled_user_shares', 'yes')
+			->willReturn($hide ? 'yes' : 'no');
+	}
 }

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;
 		}