Skip to content

Commit ea73087

Browse files
kesselbkesselb
committed
fix(dav): restrict calendar and address book sharing to the owner
Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
1 parent 11a8826 commit ea73087

14 files changed

Lines changed: 195 additions & 85 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@
233233
'OCA\\DAV\\DAV\\Sharing\\IShareable' => $baseDir . '/../lib/DAV/Sharing/IShareable.php',
234234
'OCA\\DAV\\DAV\\Sharing\\Plugin' => $baseDir . '/../lib/DAV/Sharing/Plugin.php',
235235
'OCA\\DAV\\DAV\\Sharing\\SharingMapper' => $baseDir . '/../lib/DAV/Sharing/SharingMapper.php',
236+
'OCA\\DAV\\DAV\\Sharing\\SharingPrivilegeSetTrait' => $baseDir . '/../lib/DAV/Sharing/SharingPrivilegeSetTrait.php',
236237
'OCA\\DAV\\DAV\\Sharing\\SharingService' => $baseDir . '/../lib/DAV/Sharing/SharingService.php',
237238
'OCA\\DAV\\DAV\\Sharing\\Xml\\Invite' => $baseDir . '/../lib/DAV/Sharing/Xml/Invite.php',
238239
'OCA\\DAV\\DAV\\Sharing\\Xml\\ShareRequest' => $baseDir . '/../lib/DAV/Sharing/Xml/ShareRequest.php',

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,7 @@ class ComposerStaticInitDAV
248248
'OCA\\DAV\\DAV\\Sharing\\IShareable' => __DIR__ . '/..' . '/../lib/DAV/Sharing/IShareable.php',
249249
'OCA\\DAV\\DAV\\Sharing\\Plugin' => __DIR__ . '/..' . '/../lib/DAV/Sharing/Plugin.php',
250250
'OCA\\DAV\\DAV\\Sharing\\SharingMapper' => __DIR__ . '/..' . '/../lib/DAV/Sharing/SharingMapper.php',
251+
'OCA\\DAV\\DAV\\Sharing\\SharingPrivilegeSetTrait' => __DIR__ . '/..' . '/../lib/DAV/Sharing/SharingPrivilegeSetTrait.php',
251252
'OCA\\DAV\\DAV\\Sharing\\SharingService' => __DIR__ . '/..' . '/../lib/DAV/Sharing/SharingService.php',
252253
'OCA\\DAV\\DAV\\Sharing\\Xml\\Invite' => __DIR__ . '/..' . '/../lib/DAV/Sharing/Xml/Invite.php',
253254
'OCA\\DAV\\DAV\\Sharing\\Xml\\ShareRequest' => __DIR__ . '/..' . '/../lib/DAV/Sharing/Xml/ShareRequest.php',

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use DateTimeInterface;
1313
use OCA\DAV\CalDAV\Trashbin\Plugin as TrashbinPlugin;
1414
use OCA\DAV\DAV\Sharing\IShareable;
15+
use OCA\DAV\DAV\Sharing\SharingPrivilegeSetTrait;
1516
use OCA\DAV\Exception\UnsupportedLimitOnInitialSyncException;
1617
use OCP\DB\Exception;
1718
use OCP\IConfig;
@@ -31,6 +32,8 @@
3132
* @property CalDavBackend $caldavBackend
3233
*/
3334
class Calendar extends \Sabre\CalDAV\Calendar implements IRestorable, IShareable, IMoveTarget {
35+
use SharingPrivilegeSetTrait;
36+
3437
private IConfig $config;
3538
protected IL10N $l10n;
3639
private bool $useTrashbin = true;
@@ -135,6 +138,11 @@ public function getACL() {
135138
'principal' => $this->getOwner() . '/calendar-proxy-write',
136139
'protected' => true,
137140
];
141+
$acl[] = [
142+
'privilege' => '{DAV:}write-acl',
143+
'principal' => $this->getOwner(),
144+
'protected' => true,
145+
];
138146
} else {
139147
$acl[] = [
140148
'privilege' => '{DAV:}write-properties',

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ public function __construct(bool $public = true) {
7676
$this->server->addPlugin(new \Sabre\CalDAV\Notifications\Plugin());
7777
//$this->server->addPlugin(new \OCA\DAV\DAV\Sharing\Plugin($authBackend, \OC::$server->getRequest()));
7878
$this->server->addPlugin(new \OCA\DAV\CalDAV\Publishing\PublishPlugin(
79-
\OC::$server->getConfig(),
8079
\OC::$server->getURLGenerator()
8180
));
8281

‎apps/dav/lib/CalDAV/Publishing/PublishPlugin.php‎

Lines changed: 14 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
use OCA\DAV\CalDAV\Calendar;
1010
use OCA\DAV\CalDAV\Publishing\Xml\Publisher;
11-
use OCP\IConfig;
1211
use OCP\IURLGenerator;
1312
use Sabre\CalDAV\Xml\Property\AllowedSharingModes;
1413
use Sabre\DAV\Exception\NotFound;
@@ -29,13 +28,6 @@ class PublishPlugin extends ServerPlugin {
2928
*/
3029
protected $server;
3130

32-
/**
33-
* Config instance to get instance secret.
34-
*
35-
* @var IConfig
36-
*/
37-
protected $config;
38-
3931
/**
4032
* URL Generator for absolute URLs.
4133
*
@@ -46,11 +38,9 @@ class PublishPlugin extends ServerPlugin {
4638
/**
4739
* PublishPlugin constructor.
4840
*
49-
* @param IConfig $config
5041
* @param IURLGenerator $urlGenerator
5142
*/
52-
public function __construct(IConfig $config, IURLGenerator $urlGenerator) {
53-
$this->config = $config;
43+
public function __construct(IURLGenerator $urlGenerator) {
5444
$this->urlGenerator = $urlGenerator;
5545
}
5646

@@ -108,16 +98,10 @@ public function propFind(PropFind $propFind, INode $node) {
10898
}
10999
});
110100

111-
$propFind->handle('{'.self::NS_CALENDARSERVER.'}allowed-sharing-modes', function () use ($node) {
112-
$canShare = (!$node->isSubscription() && $node->canWrite());
113-
$canPublish = (!$node->isSubscription() && $node->canWrite());
101+
$propFind->handle('{' . self::NS_CALENDARSERVER . '}allowed-sharing-modes', function () use ($propFind, $node) {
102+
$canShare = !$node->isSubscription() && $this->canChangeSharing($propFind->getPath());
114103

115-
if ($this->config->getAppValue('dav', 'limitAddressBookAndCalendarSharingToOwner', 'no') === 'yes') {
116-
$canShare = $canShare && ($node->getOwner() === $node->getPrincipalURI());
117-
$canPublish = $canPublish && ($node->getOwner() === $node->getPrincipalURI());
118-
}
119-
120-
return new AllowedSharingModes($canShare, $canPublish);
104+
return new AllowedSharingModes($canShare, $canShare);
121105
});
122106
}
123107
}
@@ -169,20 +153,7 @@ public function httpPost(RequestInterface $request, ResponseInterface $response)
169153
}
170154
$this->server->transactionType = 'post-publish-calendar';
171155

172-
// Getting ACL info
173-
$acl = $this->server->getPlugin('acl');
174-
175-
// If there's no ACL support, we allow everything
176-
if ($acl) {
177-
/** @var \Sabre\DAVACL\Plugin $acl */
178-
$acl->checkPrivileges($path, '{DAV:}write');
179-
180-
$limitSharingToOwner = $this->config->getAppValue('dav', 'limitAddressBookAndCalendarSharingToOwner', 'no') === 'yes';
181-
$isOwner = $acl->getCurrentUserPrincipal() === $node->getOwner();
182-
if ($limitSharingToOwner && !$isOwner) {
183-
return;
184-
}
185-
}
156+
$this->canChangeSharing($path, true);
186157

187158
$node->setPublishStatus(true);
188159

@@ -204,20 +175,7 @@ public function httpPost(RequestInterface $request, ResponseInterface $response)
204175
}
205176
$this->server->transactionType = 'post-unpublish-calendar';
206177

207-
// Getting ACL info
208-
$acl = $this->server->getPlugin('acl');
209-
210-
// If there's no ACL support, we allow everything
211-
if ($acl) {
212-
/** @var \Sabre\DAVACL\Plugin $acl */
213-
$acl->checkPrivileges($path, '{DAV:}write');
214-
215-
$limitSharingToOwner = $this->config->getAppValue('dav', 'limitAddressBookAndCalendarSharingToOwner', 'no') === 'yes';
216-
$isOwner = $acl->getCurrentUserPrincipal() === $node->getOwner();
217-
if ($limitSharingToOwner && !$isOwner) {
218-
return;
219-
}
220-
}
178+
$this->canChangeSharing($path, true);
221179

222180
$node->setPublishStatus(false);
223181

@@ -232,4 +190,12 @@ public function httpPost(RequestInterface $request, ResponseInterface $response)
232190

233191
}
234192
}
193+
194+
private function canChangeSharing(string $path, bool $throwExceptions = false): bool {
195+
$acl = $this->server->getPlugin('acl');
196+
if (!$acl instanceof \Sabre\DAVACL\Plugin) {
197+
return true;
198+
}
199+
return $acl->checkPrivileges($path, '{DAV:}write-acl', \Sabre\DAVACL\Plugin::R_PARENT, $throwExceptions);
200+
}
235201
}

‎apps/dav/lib/CardDAV/AddressBook.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
namespace OCA\DAV\CardDAV;
99

1010
use OCA\DAV\DAV\Sharing\IShareable;
11+
use OCA\DAV\DAV\Sharing\SharingPrivilegeSetTrait;
1112
use OCA\DAV\Exception\UnsupportedLimitOnInitialSyncException;
1213
use OCP\DB\Exception;
1314
use OCP\IL10N;
@@ -27,6 +28,8 @@
2728
* @property CardDavBackend $carddavBackend
2829
*/
2930
class AddressBook extends \Sabre\CardDAV\AddressBook implements IShareable, IMoveTarget {
31+
use SharingPrivilegeSetTrait;
32+
3033
/**
3134
* AddressBook constructor.
3235
*
@@ -97,6 +100,11 @@ public function getACL() {
97100
'principal' => $this->getOwner(),
98101
'protected' => true,
99102
],
103+
[
104+
'privilege' => '{DAV:}write-acl',
105+
'principal' => $this->getOwner(),
106+
'protected' => true,
107+
],
100108
[
101109
'privilege' => '{DAV:}write-properties',
102110
'principal' => $this->getOwner(),

‎apps/dav/lib/DAV/Sharing/Plugin.php‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
use OCA\DAV\Connector\Sabre\Auth;
1313
use OCA\DAV\DAV\Sharing\Xml\Invite;
1414
use OCA\DAV\DAV\Sharing\Xml\ShareRequest;
15-
use OCP\IConfig;
1615
use OCP\IRequest;
1716
use Sabre\DAV\Exception\NotFound;
1817
use Sabre\DAV\INode;
@@ -32,20 +31,15 @@ class Plugin extends ServerPlugin {
3231
/** @var IRequest */
3332
private $request;
3433

35-
/** @var IConfig */
36-
private $config;
37-
3834
/**
3935
* Plugin constructor.
4036
*
4137
* @param Auth $authBackEnd
4238
* @param IRequest $request
43-
* @param IConfig $config
4439
*/
45-
public function __construct(Auth $authBackEnd, IRequest $request, IConfig $config) {
40+
public function __construct(Auth $authBackEnd, IRequest $request) {
4641
$this->auth = $authBackEnd;
4742
$this->request = $request;
48-
$this->config = $config;
4943
}
5044

5145
/**
@@ -154,13 +148,7 @@ public function httpPost(RequestInterface $request, ResponseInterface $response)
154148
// If there's no ACL support, we allow everything
155149
if ($acl) {
156150
/** @var \Sabre\DAVACL\Plugin $acl */
157-
$acl->checkPrivileges($path, '{DAV:}write');
158-
159-
$limitSharingToOwner = $this->config->getAppValue('dav', 'limitAddressBookAndCalendarSharingToOwner', 'no') === 'yes';
160-
$isOwner = $acl->getCurrentUserPrincipal() === $node->getOwner();
161-
if ($limitSharingToOwner && !$isOwner) {
162-
return;
163-
}
151+
$acl->checkPrivileges($path, '{DAV:}write-acl');
164152
}
165153

166154
$node->updateShares($message->set, $message->remove);

‎apps/dav/lib/DAV/Sharing/SharingPrivilegeSetTrait.php‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\DAV\DAV\Sharing;
11+
12+
trait SharingPrivilegeSetTrait {
13+
#[\Override]
14+
public function getSupportedPrivilegeSet(): array {
15+
return [
16+
'{DAV:}read' => [
17+
'abstract' => false,
18+
'aggregates' => [
19+
'{DAV:}read-acl' => ['abstract' => false, 'aggregates' => []],
20+
'{DAV:}read-current-user-privilege-set' => ['abstract' => false, 'aggregates' => []],
21+
],
22+
],
23+
'{DAV:}write' => [
24+
'abstract' => false,
25+
'aggregates' => [
26+
'{DAV:}write-properties' => ['abstract' => false, 'aggregates' => []],
27+
'{DAV:}write-content' => ['abstract' => false, 'aggregates' => []],
28+
'{DAV:}unlock' => ['abstract' => false, 'aggregates' => []],
29+
'{DAV:}bind' => ['abstract' => false, 'aggregates' => []],
30+
'{DAV:}unbind' => ['abstract' => false, 'aggregates' => []],
31+
],
32+
],
33+
'{DAV:}write-acl' => ['abstract' => false, 'aggregates' => []],
34+
];
35+
}
36+
}

‎apps/dav/lib/Server.php‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ public function __construct(IRequest $request, string $baseUri) {
162162

163163
// calendar plugins
164164
if ($this->requestIsForSubtree(['calendars', 'public-calendars', 'system-calendars', 'principals'])) {
165-
$this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OC::$server->getRequest(), \OC::$server->getConfig()));
165+
$this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OC::$server->getRequest()));
166166
$this->server->addPlugin(new \OCA\DAV\CalDAV\Plugin());
167167
$this->server->addPlugin(new \OCA\DAV\CalDAV\ICSExportPlugin\ICSExportPlugin(\OC::$server->getConfig(), $logger));
168168
$this->server->addPlugin(new \OCA\DAV\CalDAV\Schedule\Plugin(\OC::$server->getConfig(), \OC::$server->get(LoggerInterface::class), \OC::$server->get(DefaultCalendarValidator::class)));
@@ -175,7 +175,6 @@ public function __construct(IRequest $request, string $baseUri) {
175175

176176
$this->server->addPlugin(new \Sabre\CalDAV\Notifications\Plugin());
177177
$this->server->addPlugin(new \OCA\DAV\CalDAV\Publishing\PublishPlugin(
178-
\OC::$server->getConfig(),
179178
\OC::$server->getURLGenerator()
180179
));
181180

@@ -185,7 +184,7 @@ public function __construct(IRequest $request, string $baseUri) {
185184

186185
// addressbook plugins
187186
if ($this->requestIsForSubtree(['addressbooks', 'principals'])) {
188-
$this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OC::$server->getRequest(), \OC::$server->getConfig()));
187+
$this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OC::$server->getRequest()));
189188
$this->server->addPlugin(new \OCA\DAV\CardDAV\Plugin());
190189
$this->server->addPlugin(new VCFExportPlugin());
191190
$this->server->addPlugin(new MultiGetExportPlugin());

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

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,14 @@
1010
use OCA\DAV\CalDAV\BirthdayService;
1111
use OCA\DAV\CalDAV\CalDavBackend;
1212
use OCA\DAV\CalDAV\Calendar;
13+
use OCA\DAV\Connector\Sabre\DavAclPlugin;
1314
use OCP\IConfig;
1415
use OCP\IL10N;
1516
use PHPUnit\Framework\MockObject\MockObject;
1617
use Psr\Log\LoggerInterface;
1718
use Sabre\DAV\PropPatch;
19+
use Sabre\DAV\Server;
20+
use Sabre\DAV\SimpleCollection;
1821
use Sabre\VObject\Reader;
1922
use Test\TestCase;
2023

@@ -232,6 +235,11 @@ public function testAcl($expectsWrite, $readOnlyValue, $hasOwnerSet, $uri = 'def
232235
'principal' => ($hasOwnerSet ? 'user1' : 'user2') . '/calendar-proxy-write',
233236
'protected' => true
234237
];
238+
$expectedAcl[] = [
239+
'privilege' => '{DAV:}write-acl',
240+
'principal' => $hasOwnerSet ? 'user1' : 'user2',
241+
'protected' => true
242+
];
235243
}
236244

237245
$expectedAcl[] = [
@@ -276,6 +284,59 @@ public function providesReadOnlyInfo() {
276284
];
277285
}
278286

287+
/**
288+
* @dataProvider providesSharingPrivileges
289+
*/
290+
public function testOnlyOwnerHoldsWriteAcl(string $principal, bool $readOnly, bool $expectsWrite, bool $expectsWriteAcl): void {
291+
/** @var CalDavBackend&MockObject $backend */
292+
$backend = $this->createMock(CalDavBackend::class);
293+
$backend->method('applyShareAcl')->willReturnArgument(1);
294+
$calendarInfo = [
295+
'{DAV:}displayname' => 'Test',
296+
'{http://owncloud.org/ns}owner-principal' => 'user1',
297+
'{http://owncloud.org/ns}read-only' => $readOnly,
298+
'principaluri' => 'user2',
299+
'id' => 666,
300+
'uri' => 'cal',
301+
];
302+
$calendar = new Calendar($backend, $calendarInfo, $this->l10n, $this->config, $this->logger);
303+
304+
$aclPlugin = new class($principal) extends DavAclPlugin {
305+
public function __construct(
306+
private string $principal,
307+
) {
308+
parent::__construct();
309+
}
310+
311+
#[\Override]
312+
public function getCurrentUserPrincipal() {
313+
return $this->principal;
314+
}
315+
316+
#[\Override]
317+
public function getPrincipalMembership($mainPrincipal) {
318+
return [];
319+
}
320+
};
321+
$server = new Server(new SimpleCollection('root'));
322+
$server->addPlugin($aclPlugin);
323+
324+
$privileges = $aclPlugin->getCurrentUserPrivilegeSet($calendar);
325+
326+
$this->assertSame($expectsWrite, in_array('{DAV:}write', $privileges, true));
327+
$this->assertSame($expectsWriteAcl, in_array('{DAV:}write-acl', $privileges, true));
328+
}
329+
330+
public static function providesSharingPrivileges(): array {
331+
return [
332+
'owner' => ['user1', false, true, true],
333+
'owner of read-only share' => ['user1', true, true, true],
334+
'owner write proxy' => ['user1/calendar-proxy-write', false, true, false],
335+
'read-write sharee' => ['user2', false, true, false],
336+
'read-only sharee' => ['user2', true, false, false],
337+
];
338+
}
339+
279340
/**
280341
* @dataProvider providesConfidentialClassificationData
281342
* @param int $expectedChildren

0 commit comments

Comments
 (0)