fix(caldav): stricter default calendar checks

Reject calendars that
- are subscriptions
- are not writable
- are shared with a user
- are deleted
- don't support VEVENTs

Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>

4 files changed, 249 insertions, 9 deletions

diff --git a/apps/dav/tests/unit/CalDAV/DefaultCalendarValidatorTest.php b/apps/dav/tests/unit/CalDAV/DefaultCalendarValidatorTest.php
new file mode 100644
index 00000000000..194009827da
--- /dev/null
+++ b/apps/dav/tests/unit/CalDAV/DefaultCalendarValidatorTest.php
@@ -0,0 +1,171 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\DAV\Tests\unit\CalDAV;
+
+use OCA\DAV\CalDAV\Calendar;
+use OCA\DAV\CalDAV\DefaultCalendarValidator;
+use Sabre\CalDAV\Xml\Property\SupportedCalendarComponentSet;
+use Test\TestCase;
+
+class DefaultCalendarValidatorTest extends TestCase {
+	private DefaultCalendarValidator $validator;
+
+	protected function setUp(): void {
+		parent::setUp();
+
+		$this->validator = new DefaultCalendarValidator();
+	}
+
+	public function testValidateScheduleDefaultCalendar(): void {
+		$node = $this->createMock(Calendar::class);
+		$node->expects(self::once())
+			->method('isSubscription')
+			->willReturn(false);
+		$node->expects(self::once())
+			->method('canWrite')
+			->willReturn(true);
+		$node->expects(self::once())
+			->method('isShared')
+			->willReturn(false);
+		$node->expects(self::once())
+			->method('isDeleted')
+			->willReturn(false);
+		$node->expects(self::once())
+			->method('getProperties')
+			->willReturn([
+				'{urn:ietf:params:xml:ns:caldav}supported-calendar-component-set' => new SupportedCalendarComponentSet(['VEVENT']),
+			]);
+
+		$this->validator->validateScheduleDefaultCalendar($node);
+	}
+
+	public function testValidateScheduleDefaultCalendarWithEmptyProperties(): void {
+		$node = $this->createMock(Calendar::class);
+		$node->expects(self::once())
+			->method('isSubscription')
+			->willReturn(false);
+		$node->expects(self::once())
+			->method('canWrite')
+			->willReturn(true);
+		$node->expects(self::once())
+			->method('isShared')
+			->willReturn(false);
+		$node->expects(self::once())
+			->method('isDeleted')
+			->willReturn(false);
+		$node->expects(self::once())
+			->method('getProperties')
+			->willReturn([]);
+
+		$this->validator->validateScheduleDefaultCalendar($node);
+	}
+
+	public function testValidateScheduleDefaultCalendarWithSubscription(): void {
+		$node = $this->createMock(Calendar::class);
+		$node->expects(self::once())
+			->method('isSubscription')
+			->willReturn(true);
+		$node->expects(self::never())
+			->method('canWrite');
+		$node->expects(self::never())
+			->method('isShared');
+		$node->expects(self::never())
+			->method('isDeleted');
+		$node->expects(self::never())
+			->method('getProperties');
+
+		$this->expectException(\Sabre\DAV\Exception::class);
+		$this->validator->validateScheduleDefaultCalendar($node);
+	}
+
+	public function testValidateScheduleDefaultCalendarWithoutWrite(): void {
+		$node = $this->createMock(Calendar::class);
+		$node->expects(self::once())
+			->method('isSubscription')
+			->willReturn(false);
+		$node->expects(self::once())
+			->method('canWrite')
+			->willReturn(false);
+		$node->expects(self::never())
+			->method('isShared');
+		$node->expects(self::never())
+			->method('isDeleted');
+		$node->expects(self::never())
+			->method('getProperties');
+
+		$this->expectException(\Sabre\DAV\Exception::class);
+		$this->validator->validateScheduleDefaultCalendar($node);
+	}
+
+	public function testValidateScheduleDefaultCalendarWithShared(): void {
+		$node = $this->createMock(Calendar::class);
+		$node->expects(self::once())
+			->method('isSubscription')
+			->willReturn(false);
+		$node->expects(self::once())
+			->method('canWrite')
+			->willReturn(true);
+		$node->expects(self::once())
+			->method('isShared')
+			->willReturn(true);
+		$node->expects(self::never())
+			->method('isDeleted');
+		$node->expects(self::never())
+			->method('getProperties');
+
+		$this->expectException(\Sabre\DAV\Exception::class);
+		$this->validator->validateScheduleDefaultCalendar($node);
+	}
+
+	public function testValidateScheduleDefaultCalendarWithDeleted(): void {
+		$node = $this->createMock(Calendar::class);
+		$node->expects(self::once())
+			->method('isSubscription')
+			->willReturn(false);
+		$node->expects(self::once())
+			->method('canWrite')
+			->willReturn(true);
+		$node->expects(self::once())
+			->method('isShared')
+			->willReturn(false);
+		$node->expects(self::once())
+			->method('isDeleted')
+			->willReturn(true);
+		$node->expects(self::never())
+			->method('getProperties');
+
+		$this->expectException(\Sabre\DAV\Exception::class);
+		$this->validator->validateScheduleDefaultCalendar($node);
+	}
+
+	public function testValidateScheduleDefaultCalendarWithoutVeventSupport(): void {
+		$node = $this->createMock(Calendar::class);
+		$node->expects(self::once())
+			->method('isSubscription')
+			->willReturn(false);
+		$node->expects(self::once())
+			->method('canWrite')
+			->willReturn(true);
+		$node->expects(self::once())
+			->method('isShared')
+			->willReturn(false);
+		$node->expects(self::once())
+			->method('isDeleted')
+			->willReturn(false);
+		$node->expects(self::once())
+			->method('getProperties')
+			->willReturn([
+				'{urn:ietf:params:xml:ns:caldav}supported-calendar-component-set' => new SupportedCalendarComponentSet(['VTODO']),
+			]);
+
+		$this->expectException(\Sabre\DAV\Exception::class);
+		$this->validator->validateScheduleDefaultCalendar($node);
+	}
+}
diff --git a/apps/dav/tests/unit/CalDAV/Schedule/PluginTest.php b/apps/dav/tests/unit/CalDAV/Schedule/PluginTest.php
index c727079a367..18788ecc395 100644
--- a/apps/dav/tests/unit/CalDAV/Schedule/PluginTest.php
+++ b/apps/dav/tests/unit/CalDAV/Schedule/PluginTest.php
@@ -8,6 +8,7 @@ namespace OCA\DAV\Tests\unit\CalDAV\Schedule;
 use OCA\DAV\CalDAV\CalDavBackend;
 use OCA\DAV\CalDAV\Calendar;
 use OCA\DAV\CalDAV\CalendarHome;
+use OCA\DAV\CalDAV\DefaultCalendarValidator;
 use OCA\DAV\CalDAV\Plugin as CalDAVPlugin;
 use OCA\DAV\CalDAV\Schedule\Plugin;
 use OCA\DAV\CalDAV\Trashbin\Plugin as TrashbinPlugin;
@@ -39,6 +40,9 @@ class PluginTest extends TestCase {
 	/** @var MockObject|LoggerInterface */
 	private $logger;
 
+	/** @property MockObject|DefaultCalendarValidator */
+	private $defaultCalendarValidator;
+
 	protected function setUp(): void {
 		parent::setUp();
 
@@ -53,13 +57,14 @@ class PluginTest extends TestCase {
 		$this->server->xml = new Service();
 
 		$this->logger = $this->createMock(LoggerInterface::class);
+		$this->defaultCalendarValidator = $this->createMock(DefaultCalendarValidator::class);
 
-		$this->plugin = new Plugin($this->config, $this->logger);
+		$this->plugin = new Plugin($this->config, $this->logger, $this->defaultCalendarValidator);
 		$this->plugin->initialize($this->server);
 	}
 
 	public function testInitialize(): void {
-		$plugin = new Plugin($this->config, $this->logger);
+		$plugin = new Plugin($this->config, $this->logger, $this->defaultCalendarValidator);
 
 		$this->server->expects($this->exactly(10))
 			->method('on')
@@ -376,6 +381,13 @@ class PluginTest extends TestCase {
 			->with($calendarHome .'/' . $calendarUri, [], 1)
 			->willReturn($properties);
 
+		$this->defaultCalendarValidator->method('validateScheduleDefaultCalendar')
+			->willReturnCallback(function (Calendar $node) {
+				if ($node->isDeleted()) {
+					throw new \Sabre\DAV\Exception('Deleted calendar');
+				}
+			});
+
 		$this->plugin->propFindDefaultCalendarUrl($propFind, $node);
 
 		if (!$propertiesForPath) {
diff --git a/apps/dav/tests/unit/Connector/Sabre/CustomPropertiesBackendTest.php b/apps/dav/tests/unit/Connector/Sabre/CustomPropertiesBackendTest.php
index 4d6e1f13d5f..7da38afdf3a 100644
--- a/apps/dav/tests/unit/Connector/Sabre/CustomPropertiesBackendTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/CustomPropertiesBackendTest.php
@@ -7,9 +7,11 @@
  */
 namespace OCA\DAV\Tests\unit\Connector\Sabre;
 
+use OCA\DAV\CalDAV\DefaultCalendarValidator;
 use OCA\DAV\Connector\Sabre\Directory;
 use OCA\DAV\Connector\Sabre\File;
 use OCP\IUser;
+use PHPUnit\Framework\MockObject\MockObject;
 use Sabre\DAV\Tree;
 
 /**
@@ -41,6 +43,9 @@ class CustomPropertiesBackendTest extends \Test\TestCase {
 	 */
 	private $user;
 
+	/** @property MockObject|DefaultCalendarValidator */
+	private $defaultCalendarValidator;
+
 	protected function setUp(): void {
 		parent::setUp();
 		$this->server = new \Sabre\DAV\Server();
@@ -57,11 +62,14 @@ class CustomPropertiesBackendTest extends \Test\TestCase {
 			->method('getUID')
 			->willReturn($userId);
 
+		$this->defaultCalendarValidator = $this->createMock(DefaultCalendarValidator::class);
+
 		$this->plugin = new \OCA\DAV\DAV\CustomPropertiesBackend(
 			$this->server,
 			$this->tree,
 			\OC::$server->getDatabaseConnection(),
-			$this->user
+			$this->user,
+			$this->defaultCalendarValidator,
 		);
 	}
 
diff --git a/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php b/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php
index cb6447511e4..3c527e24363 100644
--- a/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php
+++ b/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php
@@ -5,11 +5,12 @@
  */
 namespace OCA\DAV\Tests\DAV;
 
+use OCA\DAV\CalDAV\Calendar;
+use OCA\DAV\CalDAV\DefaultCalendarValidator;
 use OCA\DAV\DAV\CustomPropertiesBackend;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\IDBConnection;
 use OCP\IUser;
-use Sabre\CalDAV\ICalendar;
 use Sabre\DAV\Exception\NotFound;
 use Sabre\DAV\PropFind;
 use Sabre\DAV\PropPatch;
@@ -41,6 +42,9 @@ class CustomPropertiesBackendTest extends TestCase {
 	/** @var CustomPropertiesBackend | \PHPUnit\Framework\MockObject\MockObject */
 	private $backend;
 
+	/** @property DefaultCalendarValidator | \PHPUnit\Framework\MockObject\MockObject */
+	private $defaultCalendarValidator;
+
 	protected function setUp(): void {
 		parent::setUp();
 
@@ -53,12 +57,14 @@ class CustomPropertiesBackendTest extends TestCase {
 			->with()
 			->willReturn('dummy_user_42');
 		$this->dbConnection = \OC::$server->getDatabaseConnection();
+		$this->defaultCalendarValidator = $this->createMock(DefaultCalendarValidator::class);
 
 		$this->backend = new CustomPropertiesBackend(
 			$this->server,
 			$this->tree,
 			$this->dbConnection,
 			$this->user,
+			$this->defaultCalendarValidator,
 		);
 	}
 
@@ -131,6 +137,7 @@ class CustomPropertiesBackendTest extends TestCase {
 			$this->tree,
 			$db,
 			$this->user,
+			$this->defaultCalendarValidator,
 		);
 
 		$propFind = $this->createMock(PropFind::class);
@@ -195,7 +202,7 @@ class CustomPropertiesBackendTest extends TestCase {
 	public function testPropFindPrincipalCall(): void {
 		$this->tree->method('getNodeForPath')
 			->willReturnCallback(function ($uri) {
-				$node = $this->createMock(ICalendar::class);
+				$node = $this->createMock(Calendar::class);
 				$node->method('getOwner')
 					->willReturn('principals/users/dummy_user_42');
 				return $node;
@@ -242,21 +249,21 @@ class CustomPropertiesBackendTest extends TestCase {
 		return [
 			[ // Exists
 				'dummy_user_42',
-				['calendars/dummy_user_42/foo/' => ICalendar::class],
+				['calendars/dummy_user_42/foo/' => Calendar::class],
 				['{urn:ietf:params:xml:ns:caldav}schedule-default-calendar-URL' => new Href('calendars/dummy_user_42/foo/')],
 				['{urn:ietf:params:xml:ns:caldav}schedule-default-calendar-URL'],
 				['{urn:ietf:params:xml:ns:caldav}schedule-default-calendar-URL' => new Href('calendars/dummy_user_42/foo/')],
 			],
 			[ // Doesn't exist
 				'dummy_user_42',
-				['calendars/dummy_user_42/foo/' => ICalendar::class],
+				['calendars/dummy_user_42/foo/' => Calendar::class],
 				['{urn:ietf:params:xml:ns:caldav}schedule-default-calendar-URL' => new Href('calendars/dummy_user_42/bar/')],
 				['{urn:ietf:params:xml:ns:caldav}schedule-default-calendar-URL'],
 				[],
 			],
 			[ // No privilege
 				'dummy_user_42',
-				['calendars/user2/baz/' => ICalendar::class],
+				['calendars/user2/baz/' => Calendar::class],
 				['{urn:ietf:params:xml:ns:caldav}schedule-default-calendar-URL' => new Href('calendars/user2/baz/')],
 				['{urn:ietf:params:xml:ns:caldav}schedule-default-calendar-URL'],
 				[],
@@ -349,7 +356,7 @@ class CustomPropertiesBackendTest extends TestCase {
 			});
 		$this->tree->method('getNodeForPath')
 			->willReturnCallback(function ($uri) {
-				$node = $this->createMock(ICalendar::class);
+				$node = $this->createMock(Calendar::class);
 				$node->method('getOwner')
 					->willReturn('principals/users/' . $this->user->getUID());
 				return $node;
@@ -377,6 +384,48 @@ class CustomPropertiesBackendTest extends TestCase {
 		];
 	}
 
+	public function testPropPatchWithUnsuitableCalendar(): void {
+		$path = 'principals/users/' . $this->user->getUID();
+
+		$node = $this->createMock(Calendar::class);
+		$node->expects(self::once())
+			->method('getOwner')
+			->willReturn($path);
+
+		$this->defaultCalendarValidator->expects(self::once())
+			->method('validateScheduleDefaultCalendar')
+			->with($node)
+			->willThrowException(new \Sabre\DAV\Exception("Invalid calendar"));
+
+		$this->server->method('calculateUri')
+			->willReturnCallback(function ($uri) {
+				if (str_starts_with($uri, self::BASE_URI)) {
+					return trim(substr($uri, strlen(self::BASE_URI)), '/');
+				}
+				return null;
+			});
+		$this->tree->expects(self::once())
+			->method('getNodeForPath')
+			->with('foo/bar/')
+			->willReturn($node);
+
+		$storedProps = $this->getProps($this->user->getUID(), $path);
+		$this->assertEquals([], $storedProps);
+
+		$propPatch = new PropPatch([
+			'{urn:ietf:params:xml:ns:caldav}schedule-default-calendar-URL' => new Href('foo/bar/'),
+		]);
+		$this->backend->propPatch($path, $propPatch);
+		try {
+			$propPatch->commit();
+		} catch (\Throwable $e) {
+			$this->assertInstanceOf(\Sabre\DAV\Exception::class, $e);
+		}
+
+		$storedProps = $this->getProps($this->user->getUID(), $path);
+		$this->assertEquals([], $storedProps);
+	}
+
 	/**
 	 * @dataProvider deleteProvider
 	 */