Skip to content

Commit 950d312

Browse files
st3inybackportbot[bot]
st3iny
authored and
backportbot[bot]
committed
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> [skip ci]
1 parent 64a7670 commit 950d312

File tree

13 files changed

+323
-18
lines changed

13 files changed

+323
-18
lines changed

‎apps/dav/appinfo/v1/caldav.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
use OC\KnownUser\KnownUserService;
3030
use OCA\DAV\CalDAV\CalDavBackend;
3131
use OCA\DAV\CalDAV\CalendarRoot;
32+
use OCA\DAV\CalDAV\DefaultCalendarValidator;
3233
use OCA\DAV\CalDAV\Security\RateLimitingPlugin;
3334
use OCA\DAV\CalDAV\Validation\CalDavValidatePlugin;
3435
use OCA\DAV\Connector\LegacyDAVACL;
@@ -112,7 +113,7 @@
112113

113114
$server->addPlugin(new \Sabre\DAV\Sync\Plugin());
114115
$server->addPlugin(new \Sabre\CalDAV\ICSExportPlugin());
115-
$server->addPlugin(new \OCA\DAV\CalDAV\Schedule\Plugin(\OC::$server->getConfig(), \OC::$server->get(LoggerInterface::class)));
116+
$server->addPlugin(new \OCA\DAV\CalDAV\Schedule\Plugin(\OC::$server->getConfig(), \OC::$server->get(LoggerInterface::class), \OC::$server->get(DefaultCalendarValidator::class)));
116117

117118
if ($sendInvitations) {
118119
$server->addPlugin(\OC::$server->query(\OCA\DAV\CalDAV\Schedule\IMipPlugin::class));

‎apps/dav/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
'OCA\\DAV\\CalDAV\\CalendarObject' => $baseDir . '/../lib/CalDAV/CalendarObject.php',
5959
'OCA\\DAV\\CalDAV\\CalendarProvider' => $baseDir . '/../lib/CalDAV/CalendarProvider.php',
6060
'OCA\\DAV\\CalDAV\\CalendarRoot' => $baseDir . '/../lib/CalDAV/CalendarRoot.php',
61+
'OCA\\DAV\\CalDAV\\DefaultCalendarValidator' => $baseDir . '/../lib/CalDAV/DefaultCalendarValidator.php',
6162
'OCA\\DAV\\CalDAV\\EventComparisonService' => $baseDir . '/../lib/CalDAV/EventComparisonService.php',
6263
'OCA\\DAV\\CalDAV\\FreeBusy\\FreeBusyGenerator' => $baseDir . '/../lib/CalDAV/FreeBusy/FreeBusyGenerator.php',
6364
'OCA\\DAV\\CalDAV\\ICSExportPlugin\\ICSExportPlugin' => $baseDir . '/../lib/CalDAV/ICSExportPlugin/ICSExportPlugin.php',

‎apps/dav/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ class ComposerStaticInitDAV
7373
'OCA\\DAV\\CalDAV\\CalendarObject' => __DIR__ . '/..' . '/../lib/CalDAV/CalendarObject.php',
7474
'OCA\\DAV\\CalDAV\\CalendarProvider' => __DIR__ . '/..' . '/../lib/CalDAV/CalendarProvider.php',
7575
'OCA\\DAV\\CalDAV\\CalendarRoot' => __DIR__ . '/..' . '/../lib/CalDAV/CalendarRoot.php',
76+
'OCA\\DAV\\CalDAV\\DefaultCalendarValidator' => __DIR__ . '/..' . '/../lib/CalDAV/DefaultCalendarValidator.php',
7677
'OCA\\DAV\\CalDAV\\EventComparisonService' => __DIR__ . '/..' . '/../lib/CalDAV/EventComparisonService.php',
7778
'OCA\\DAV\\CalDAV\\FreeBusy\\FreeBusyGenerator' => __DIR__ . '/..' . '/../lib/CalDAV/FreeBusy/FreeBusyGenerator.php',
7879
'OCA\\DAV\\CalDAV\\ICSExportPlugin\\ICSExportPlugin' => __DIR__ . '/..' . '/../lib/CalDAV/ICSExportPlugin/ICSExportPlugin.php',

‎apps/dav/lib/CalDAV/DefaultCalendarValidator.php‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\DAV\CalDAV;
11+
12+
use Sabre\DAV\Exception as DavException;
13+
14+
class DefaultCalendarValidator {
15+
/**
16+
* Check if a given Calendar node is suitable to be used as the default calendar for scheduling.
17+
*
18+
* @throws DavException If the calendar is not suitable to be used as the default calendar
19+
*/
20+
public function validateScheduleDefaultCalendar(Calendar $calendar): void {
21+
// Sanity checks for a calendar that should handle invitations
22+
if ($calendar->isSubscription()
23+
|| !$calendar->canWrite()
24+
|| $calendar->isShared()
25+
|| $calendar->isDeleted()) {
26+
throw new DavException('Calendar is a subscription, not writable, shared or deleted');
27+
}
28+
29+
// Calendar must support VEVENTs
30+
$sCCS = '{urn:ietf:params:xml:ns:caldav}supported-calendar-component-set';
31+
$calendarProperties = $calendar->getProperties([$sCCS]);
32+
if (isset($calendarProperties[$sCCS])) {
33+
$supportedComponents = $calendarProperties[$sCCS]->getValue();
34+
} else {
35+
$supportedComponents = ['VJOURNAL', 'VTODO', 'VEVENT'];
36+
}
37+
if (!in_array('VEVENT', $supportedComponents, true)) {
38+
throw new DavException('Calendar does not support VEVENT components');
39+
}
40+
}
41+
}

‎apps/dav/lib/CalDAV/InvitationResponse/InvitationResponseServer.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
use OCA\DAV\AppInfo\PluginManager;
2929
use OCA\DAV\CalDAV\Auth\CustomPrincipalPlugin;
3030
use OCA\DAV\CalDAV\Auth\PublicPrincipalPlugin;
31+
use OCA\DAV\CalDAV\DefaultCalendarValidator;
3132
use OCA\DAV\Connector\Sabre\AnonymousOptionsPlugin;
3233
use OCA\DAV\Connector\Sabre\BlockLegacyClientPlugin;
3334
use OCA\DAV\Connector\Sabre\CachingTree;
@@ -89,7 +90,7 @@ public function __construct(bool $public = true) {
8990
// calendar plugins
9091
$this->server->addPlugin(new \OCA\DAV\CalDAV\Plugin());
9192
$this->server->addPlugin(new \Sabre\CalDAV\ICSExportPlugin());
92-
$this->server->addPlugin(new \OCA\DAV\CalDAV\Schedule\Plugin(\OC::$server->getConfig(), \OC::$server->get(LoggerInterface::class)));
93+
$this->server->addPlugin(new \OCA\DAV\CalDAV\Schedule\Plugin(\OC::$server->getConfig(), \OC::$server->get(LoggerInterface::class), \OC::$server->get(DefaultCalendarValidator::class)));
9394
$this->server->addPlugin(new \Sabre\CalDAV\Subscriptions\Plugin());
9495
$this->server->addPlugin(new \Sabre\CalDAV\Notifications\Plugin());
9596
//$this->server->addPlugin(new \OCA\DAV\DAV\Sharing\Plugin($authBackend, \OC::$server->getRequest()));

‎apps/dav/lib/CalDAV/Schedule/Plugin.php‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,13 @@
3333
use OCA\DAV\CalDAV\CalDavBackend;
3434
use OCA\DAV\CalDAV\Calendar;
3535
use OCA\DAV\CalDAV\CalendarHome;
36+
use OCA\DAV\CalDAV\DefaultCalendarValidator;
3637
use OCP\IConfig;
3738
use Psr\Log\LoggerInterface;
3839
use Sabre\CalDAV\ICalendar;
3940
use Sabre\CalDAV\ICalendarObject;
4041
use Sabre\CalDAV\Schedule\ISchedulingObject;
42+
use Sabre\DAV\Exception as DavException;
4143
use Sabre\DAV\INode;
4244
use Sabre\DAV\IProperties;
4345
use Sabre\DAV\PropFind;
@@ -75,13 +77,15 @@ class Plugin extends \Sabre\CalDAV\Schedule\Plugin {
7577
public const CALENDAR_USER_TYPE = '{' . self::NS_CALDAV . '}calendar-user-type';
7678
public const SCHEDULE_DEFAULT_CALENDAR_URL = '{' . Plugin::NS_CALDAV . '}schedule-default-calendar-URL';
7779
private LoggerInterface $logger;
80+
private DefaultCalendarValidator $defaultCalendarValidator;
7881

7982
/**
8083
* @param IConfig $config
8184
*/
82-
public function __construct(IConfig $config, LoggerInterface $logger) {
85+
public function __construct(IConfig $config, LoggerInterface $logger, DefaultCalendarValidator $defaultCalendarValidator) {
8386
$this->config = $config;
8487
$this->logger = $logger;
88+
$this->defaultCalendarValidator = $defaultCalendarValidator;
8589
}
8690

8791
/**
@@ -384,11 +388,20 @@ public function propFindDefaultCalendarUrl(PropFind $propFind, INode $node) {
384388
* - isn't a calendar subscription
385389
* - user can write to it (no virtual/3rd-party calendars)
386390
* - calendar isn't a share
391+
* - calendar supports VEVENTs
387392
*/
388393
foreach ($calendarHome->getChildren() as $node) {
389-
if ($node instanceof Calendar && !$node->isSubscription() && $node->canWrite() && !$node->isShared() && !$node->isDeleted()) {
390-
$userCalendars[] = $node;
394+
if (!($node instanceof Calendar)) {
395+
continue;
391396
}
397+
398+
try {
399+
$this->defaultCalendarValidator->validateScheduleDefaultCalendar($node);
400+
} catch (DavException $e) {
401+
continue;
402+
}
403+
404+
$userCalendars[] = $node;
392405
}
393406

394407
if (count($userCalendars) > 0) {

‎apps/dav/lib/Connector/Sabre/ServerFactory.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
namespace OCA\DAV\Connector\Sabre;
3333

3434
use OCA\DAV\AppInfo\PluginManager;
35+
use OCA\DAV\CalDAV\DefaultCalendarValidator;
3536
use OCA\DAV\DAV\ViewOnlyPlugin;
3637
use OCA\DAV\Files\BrowserErrorPagePlugin;
3738
use OCP\EventDispatcher\IEventDispatcher;
@@ -191,7 +192,8 @@ public function createServer(string $baseUri,
191192
$server,
192193
$objectTree,
193194
$this->databaseConnection,
194-
$this->userSession->getUser()
195+
$this->userSession->getUser(),
196+
\OC::$server->get(DefaultCalendarValidator::class),
195197
)
196198
)
197199
);

‎apps/dav/lib/DAV/CustomPropertiesBackend.php‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,13 @@
2727
namespace OCA\DAV\DAV;
2828

2929
use Exception;
30+
use OCA\DAV\CalDAV\Calendar;
31+
use OCA\DAV\CalDAV\DefaultCalendarValidator;
3032
use OCA\DAV\Connector\Sabre\Directory;
3133
use OCA\DAV\Connector\Sabre\FilesPlugin;
3234
use OCP\DB\QueryBuilder\IQueryBuilder;
3335
use OCP\IDBConnection;
3436
use OCP\IUser;
35-
use Sabre\CalDAV\ICalendar;
3637
use Sabre\DAV\Exception as DavException;
3738
use Sabre\DAV\PropertyStorage\Backend\BackendInterface;
3839
use Sabre\DAV\PropFind;
@@ -154,6 +155,7 @@ class CustomPropertiesBackend implements BackendInterface {
154155

155156
private Server $server;
156157
private XmlService $xmlService;
158+
private DefaultCalendarValidator $defaultCalendarValidator;
157159

158160
/**
159161
* @param Tree $tree node tree
@@ -165,6 +167,7 @@ public function __construct(
165167
Tree $tree,
166168
IDBConnection $connection,
167169
IUser $user,
170+
DefaultCalendarValidator $defaultCalendarValidator,
168171
) {
169172
$this->server = $server;
170173
$this->tree = $tree;
@@ -175,6 +178,7 @@ public function __construct(
175178
$this->xmlService->elementMap,
176179
self::COMPLEX_XML_ELEMENT_MAP,
177180
);
181+
$this->defaultCalendarValidator = $defaultCalendarValidator;
178182
}
179183

180184
/**
@@ -338,10 +342,11 @@ private function validateProperty(string $path, string $propName, mixed $propVal
338342

339343
// $path is the principal here as this prop is only set on principals
340344
$node = $this->tree->getNodeForPath($href);
341-
if (!($node instanceof ICalendar) || $node->getOwner() !== $path) {
345+
if (!($node instanceof Calendar) || $node->getOwner() !== $path) {
342346
throw new DavException('No such calendar');
343347
}
344348

349+
$this->defaultCalendarValidator->validateScheduleDefaultCalendar($node);
345350
break;
346351
}
347352
}

‎apps/dav/lib/Server.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,8 @@ public function __construct(IRequest $request, string $baseUri) {
286286
$this->server,
287287
$this->server->tree,
288288
\OC::$server->getDatabaseConnection(),
289-
\OC::$server->getUserSession()->getUser()
289+
\OC::$server->getUserSession()->getUser(),
290+
\OC::$server->get(DefaultCalendarValidator::class),
290291
)
291292
)
292293
);

‎apps/dav/tests/unit/CalDAV/DefaultCalendarValidatorTest.php‎

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\DAV\Tests\unit\CalDAV;
11+
12+
use OCA\DAV\CalDAV\Calendar;
13+
use OCA\DAV\CalDAV\DefaultCalendarValidator;
14+
use Sabre\CalDAV\Xml\Property\SupportedCalendarComponentSet;
15+
use Test\TestCase;
16+
17+
class DefaultCalendarValidatorTest extends TestCase {
18+
private DefaultCalendarValidator $validator;
19+
20+
protected function setUp(): void {
21+
parent::setUp();
22+
23+
$this->validator = new DefaultCalendarValidator();
24+
}
25+
26+
public function testValidateScheduleDefaultCalendar(): void {
27+
$node = $this->createMock(Calendar::class);
28+
$node->expects(self::once())
29+
->method('isSubscription')
30+
->willReturn(false);
31+
$node->expects(self::once())
32+
->method('canWrite')
33+
->willReturn(true);
34+
$node->expects(self::once())
35+
->method('isShared')
36+
->willReturn(false);
37+
$node->expects(self::once())
38+
->method('isDeleted')
39+
->willReturn(false);
40+
$node->expects(self::once())
41+
->method('getProperties')
42+
->willReturn([
43+
'{urn:ietf:params:xml:ns:caldav}supported-calendar-component-set' => new SupportedCalendarComponentSet(['VEVENT']),
44+
]);
45+
46+
$this->validator->validateScheduleDefaultCalendar($node);
47+
}
48+
49+
public function testValidateScheduleDefaultCalendarWithEmptyProperties(): void {
50+
$node = $this->createMock(Calendar::class);
51+
$node->expects(self::once())
52+
->method('isSubscription')
53+
->willReturn(false);
54+
$node->expects(self::once())
55+
->method('canWrite')
56+
->willReturn(true);
57+
$node->expects(self::once())
58+
->method('isShared')
59+
->willReturn(false);
60+
$node->expects(self::once())
61+
->method('isDeleted')
62+
->willReturn(false);
63+
$node->expects(self::once())
64+
->method('getProperties')
65+
->willReturn([]);
66+
67+
$this->validator->validateScheduleDefaultCalendar($node);
68+
}
69+
70+
public function testValidateScheduleDefaultCalendarWithSubscription(): void {
71+
$node = $this->createMock(Calendar::class);
72+
$node->expects(self::once())
73+
->method('isSubscription')
74+
->willReturn(true);
75+
$node->expects(self::never())
76+
->method('canWrite');
77+
$node->expects(self::never())
78+
->method('isShared');
79+
$node->expects(self::never())
80+
->method('isDeleted');
81+
$node->expects(self::never())
82+
->method('getProperties');
83+
84+
$this->expectException(\Sabre\DAV\Exception::class);
85+
$this->validator->validateScheduleDefaultCalendar($node);
86+
}
87+
88+
public function testValidateScheduleDefaultCalendarWithoutWrite(): void {
89+
$node = $this->createMock(Calendar::class);
90+
$node->expects(self::once())
91+
->method('isSubscription')
92+
->willReturn(false);
93+
$node->expects(self::once())
94+
->method('canWrite')
95+
->willReturn(false);
96+
$node->expects(self::never())
97+
->method('isShared');
98+
$node->expects(self::never())
99+
->method('isDeleted');
100+
$node->expects(self::never())
101+
->method('getProperties');
102+
103+
$this->expectException(\Sabre\DAV\Exception::class);
104+
$this->validator->validateScheduleDefaultCalendar($node);
105+
}
106+
107+
public function testValidateScheduleDefaultCalendarWithShared(): void {
108+
$node = $this->createMock(Calendar::class);
109+
$node->expects(self::once())
110+
->method('isSubscription')
111+
->willReturn(false);
112+
$node->expects(self::once())
113+
->method('canWrite')
114+
->willReturn(true);
115+
$node->expects(self::once())
116+
->method('isShared')
117+
->willReturn(true);
118+
$node->expects(self::never())
119+
->method('isDeleted');
120+
$node->expects(self::never())
121+
->method('getProperties');
122+
123+
$this->expectException(\Sabre\DAV\Exception::class);
124+
$this->validator->validateScheduleDefaultCalendar($node);
125+
}
126+
127+
public function testValidateScheduleDefaultCalendarWithDeleted(): void {
128+
$node = $this->createMock(Calendar::class);
129+
$node->expects(self::once())
130+
->method('isSubscription')
131+
->willReturn(false);
132+
$node->expects(self::once())
133+
->method('canWrite')
134+
->willReturn(true);
135+
$node->expects(self::once())
136+
->method('isShared')
137+
->willReturn(false);
138+
$node->expects(self::once())
139+
->method('isDeleted')
140+
->willReturn(true);
141+
$node->expects(self::never())
142+
->method('getProperties');
143+
144+
$this->expectException(\Sabre\DAV\Exception::class);
145+
$this->validator->validateScheduleDefaultCalendar($node);
146+
}
147+
148+
public function testValidateScheduleDefaultCalendarWithoutVeventSupport(): void {
149+
$node = $this->createMock(Calendar::class);
150+
$node->expects(self::once())
151+
->method('isSubscription')
152+
->willReturn(false);
153+
$node->expects(self::once())
154+
->method('canWrite')
155+
->willReturn(true);
156+
$node->expects(self::once())
157+
->method('isShared')
158+
->willReturn(false);
159+
$node->expects(self::once())
160+
->method('isDeleted')
161+
->willReturn(false);
162+
$node->expects(self::once())
163+
->method('getProperties')
164+
->willReturn([
165+
'{urn:ietf:params:xml:ns:caldav}supported-calendar-component-set' => new SupportedCalendarComponentSet(['VTODO']),
166+
]);
167+
168+
$this->expectException(\Sabre\DAV\Exception::class);
169+
$this->validator->validateScheduleDefaultCalendar($node);
170+
}
171+
}

0 commit comments

Comments
 (0)