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>
nextcloud · Jun 4, 2024 · 9db8a35 · 9db8a35
1 parent c1661b6
commit 9db8a35
Show file tree

Hide file tree

Showing 2 changed files with 292 additions and 8 deletions.
diff --git a/apps/dav/lib/DAV/CustomPropertiesBackend.php b/apps/dav/lib/DAV/CustomPropertiesBackend.php
@@ -8,12 +8,12 @@
 namespace OCA\DAV\DAV;
 
 use Exception;
+use OCA\DAV\CalDAV\Calendar;
 use OCA\DAV\Connector\Sabre\Directory;
 use OCA\DAV\Connector\Sabre\FilesPlugin;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\IDBConnection;
 use OCP\IUser;
-use Sabre\CalDAV\ICalendar;
 use Sabre\DAV\Exception as DavException;
 use Sabre\DAV\PropertyStorage\Backend\BackendInterface;
 use Sabre\DAV\PropFind;
@@ -319,10 +319,29 @@ private function validateProperty(string $path, string $propName, mixed $propVal
 
 				// $path is the principal here as this prop is only set on principals
 				$node = $this->tree->getNodeForPath($href);
-				if (!($node instanceof ICalendar) || $node->getOwner() !== $path) {
+				if (!($node instanceof Calendar) || $node->getOwner() !== $path) {
 					throw new DavException('No such calendar');
 				}
 
+				// Sanity checks for a calendar that should handle invitations
+				if ($node->isSubscription()
+					|| !$node->canWrite()
+					|| $node->isShared()
+					|| $node->isDeleted()) {
+					throw new DavException('Calendar is a subscription, not writable, shared or deleted');
+				}
+
+				$sCCS = '{urn:ietf:params:xml:ns:caldav}supported-calendar-component-set';
+				$calendarProperties = $node->getProperties([$sCCS]);
+				if (isset($calendarProperties[$sCCS])) {
+					$supportedComponents = $calendarProperties[$sCCS]->getValue();
+				} else {
+					$supportedComponents = ['VJOURNAL', 'VTODO', 'VEVENT'];
+				}
+				if (!in_array('VEVENT', $supportedComponents, true)) {
+					throw new DavException('Calendar does not support VEVENT components');
+				}
+
 				break;
 		}
 	}

diff --git 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\DAV\CustomPropertiesBackend;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\IDBConnection;
 use OCP\IUser;
-use Sabre\CalDAV\ICalendar;
+use Sabre\CalDAV\Xml\Property\SupportedCalendarComponentSet;
 use Sabre\DAV\Exception\NotFound;
 use Sabre\DAV\PropFind;
 use Sabre\DAV\PropPatch;
@@ -195,9 +196,19 @@ public function testPropFindCalendarCall(): void {
 	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');
+				$node->method('isSubscription')
+					->willReturn(false);
+				$node->method('canWrite')
+					->willReturn(true);
+				$node->method('isShared')
+					->willReturn(false);
+				$node->method('isDeleted')
+					->willReturn(false);
+				$node->method('getProperties')
+					->willReturn([]);
 				return $node;
 			});
 
@@ -242,21 +253,21 @@ public function propFindPrincipalScheduleDefaultCalendarProviderUrlProvider(): a
 		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'],
 				[],
@@ -319,6 +330,18 @@ public function testPropFindPrincipalScheduleDefaultCalendarUrl(
 					$node = $this->createMock($nodes[$uri]);
 					$node->method('getOwner')
 						->willReturn("principals/users/$owner");
+					if ($nodes[$uri] === Calendar::class) {
+						$node->method('isSubscription')
+							->willReturn(false);
+						$node->method('canWrite')
+							->willReturn(true);
+						$node->method('isShared')
+							->willReturn(false);
+						$node->method('isDeleted')
+							->willReturn(false);
+						$node->method('getProperties')
+							->willReturn([]);
+					}
 					return $node;
 				}
 				throw new NotFound('Node not found');
@@ -349,9 +372,19 @@ public function testPropPatch(string $path, array $existing, array $props, array
 			});
 		$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());
+				$node->method('isSubscription')
+					->willReturn(false);
+				$node->method('canWrite')
+					->willReturn(true);
+				$node->method('isShared')
+					->willReturn(false);
+				$node->method('isDeleted')
+					->willReturn(false);
+				$node->method('getProperties')
+					->willReturn([]);
 				return $node;
 			});
 
@@ -377,6 +410,238 @@ public function propPatchProvider() {
 		];
 	}
 
+	public function testPropPatchWithSubscription(): void {
+		$path = 'principals/users/' . $this->user->getUID();
+
+		$node = $this->createMock(Calendar::class);
+		$node->expects(self::once())
+			->method('getOwner')
+			->willReturn($path);
+		$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->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);
+
+		$this->expectException(\Sabre\DAV\Exception::class);
+
+		$propPatch = new PropPatch([
+			'{urn:ietf:params:xml:ns:caldav}schedule-default-calendar-URL' => new Href('foo/bar/'),
+		]);
+		$this->backend->propPatch($path, $propPatch);
+		$propPatch->commit();
+	}
+
+	public function testPropPatchWithoutWrite(): void {
+		$path = 'principals/users/' . $this->user->getUID();
+
+		$node = $this->createMock(Calendar::class);
+		$node->expects(self::once())
+			->method('getOwner')
+			->willReturn($path);
+		$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->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);
+		$this->expectException(\Sabre\DAV\Exception::class);
+		$propPatch->commit();
+
+		$storedProps = $this->getProps($this->user->getUID(), $path);
+		$this->assertEquals([], $storedProps);
+	}
+
+	public function testPropPatchWithShared(): void {
+		$path = 'principals/users/' . $this->user->getUID();
+
+		$node = $this->createMock(Calendar::class);
+		$node->expects(self::once())
+			->method('getOwner')
+			->willReturn($path);
+		$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->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);
+		$this->expectException(\Sabre\DAV\Exception::class);
+		$propPatch->commit();
+
+		$storedProps = $this->getProps($this->user->getUID(), $path);
+		$this->assertEquals([], $storedProps);
+	}
+
+	public function testPropPatchWithDeleted(): void {
+		$path = 'principals/users/' . $this->user->getUID();
+
+		$node = $this->createMock(Calendar::class);
+		$node->expects(self::once())
+			->method('getOwner')
+			->willReturn($path);
+		$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->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);
+		$this->expectException(\Sabre\DAV\Exception::class);
+		$propPatch->commit();
+
+		$storedProps = $this->getProps($this->user->getUID(), $path);
+		$this->assertEquals([], $storedProps);
+	}
+
+	public function testPropPatchWithoutVeventSupport(): void {
+		$path = 'principals/users/' . $this->user->getUID();
+
+		$node = $this->createMock(Calendar::class);
+		$node->expects(self::once())
+			->method('getOwner')
+			->willReturn($path);
+		$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->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);
+		$this->expectException(\Sabre\DAV\Exception::class);
+		$propPatch->commit();
+
+		$storedProps = $this->getProps($this->user->getUID(), $path);
+		$this->assertEquals([], $storedProps);
+	}
+
 	/**
 	 * @dataProvider deleteProvider
 	 */