Skip to content

Commit 4bb070a

Browse files
kesselbbackportbot[bot]
kesselb
authored and
backportbot[bot]
committed
fix(dav): restrict calendar and address book sharing to the owner
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> [skip ci]
1 parent 11a8826 commit 4bb070a

12 files changed

Lines changed: 187 additions & 62 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: 6 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;
@@ -135,6 +136,11 @@ public function getACL() {
135136
'principal' => $this->getOwner() . '/calendar-proxy-write',
136137
'protected' => true,
137138
];
139+
$acl[] = [
140+
'privilege' => '{DAV:}write-acl',
141+
'principal' => $this->getOwner(),
142+
'protected' => true,
143+
];
138144
} else {
139145
$acl[] = [
140146
'privilege' => '{DAV:}write-properties',

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

Lines changed: 11 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,7 @@ public function propFind(PropFind $propFind, INode $node) {
112112
$canShare = (!$node->isSubscription() && $node->canWrite());
113113
$canPublish = (!$node->isSubscription() && $node->canWrite());
114114

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);
115+
return new AllowedSharingModes($canShare, $canShare);
121116
});
122117
}
123118
}
@@ -169,20 +164,7 @@ public function httpPost(RequestInterface $request, ResponseInterface $response)
169164
}
170165
$this->server->transactionType = 'post-publish-calendar';
171166

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-
}
167+
$this->canChangeSharing($path, true);
186168

187169
$node->setPublishStatus(true);
188170

@@ -204,20 +186,7 @@ public function httpPost(RequestInterface $request, ResponseInterface $response)
204186
}
205187
$this->server->transactionType = 'post-unpublish-calendar';
206188

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-
}
189+
$this->canChangeSharing($path, true);
221190

222191
$node->setPublishStatus(false);
223192

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

233202
}
234203
}
204+
205+
private function canChangeSharing(string $path, bool $throwExceptions = false): bool {
206+
$acl = $this->server->getPlugin('acl');
207+
if (!$acl instanceof \Sabre\DAVACL\Plugin) {
208+
return true;
209+
}
210+
return $acl->checkPrivileges($path, '{DAV:}write-acl', \Sabre\DAVACL\Plugin::R_PARENT, $throwExceptions);
211+
}
235212
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
* @property CardDavBackend $carddavBackend
2828
*/
2929
class AddressBook extends \Sabre\CardDAV\AddressBook implements IShareable, IMoveTarget {
30+
use SharingPrivilegeSetTrait;
31+
3032
/**
3133
* AddressBook constructor.
3234
*
@@ -97,6 +99,11 @@ public function getACL() {
9799
'principal' => $this->getOwner(),
98100
'protected' => true,
99101
],
102+
[
103+
'privilege' => '{DAV:}write-acl',
104+
'principal' => $this->getOwner(),
105+
'protected' => true,
106+
],
100107
[
101108
'privilege' => '{DAV:}write-properties',
102109
'principal' => $this->getOwner(),

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

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -154,13 +154,7 @@ public function httpPost(RequestInterface $request, ResponseInterface $response)
154154
// If there's no ACL support, we allow everything
155155
if ($acl) {
156156
/** @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-
}
157+
$acl->checkPrivileges($path, '{DAV:}write-acl');
164158
}
165159

166160
$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: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
use OCP\IConfig;
6666
use OCP\IPreview;
6767
use OCP\IRequest;
68+
use OCP\IURLGenerator;
6869
use OCP\IUserSession;
6970
use OCP\Profiler\IProfiler;
7071
use OCP\SabrePluginEvent;
@@ -162,7 +163,7 @@ public function __construct(IRequest $request, string $baseUri) {
162163

163164
// calendar plugins
164165
if ($this->requestIsForSubtree(['calendars', 'public-calendars', 'system-calendars', 'principals'])) {
165-
$this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OC::$server->getRequest(), \OC::$server->getConfig()));
166+
$this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OC::$server->getRequest()));
166167
$this->server->addPlugin(new \OCA\DAV\CalDAV\Plugin());
167168
$this->server->addPlugin(new \OCA\DAV\CalDAV\ICSExportPlugin\ICSExportPlugin(\OC::$server->getConfig(), $logger));
168169
$this->server->addPlugin(new \OCA\DAV\CalDAV\Schedule\Plugin(\OC::$server->getConfig(), \OC::$server->get(LoggerInterface::class), \OC::$server->get(DefaultCalendarValidator::class)));
@@ -185,7 +186,7 @@ public function __construct(IRequest $request, string $baseUri) {
185186

186187
// addressbook plugins
187188
if ($this->requestIsForSubtree(['addressbooks', 'principals'])) {
188-
$this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OC::$server->getRequest(), \OC::$server->getConfig()));
189+
$this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OC::$server->getRequest()));
189190
$this->server->addPlugin(new \OCA\DAV\CardDAV\Plugin());
190191
$this->server->addPlugin(new VCFExportPlugin());
191192
$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

‎apps/dav/tests/unit/CalDAV/Publishing/PublishingTest.php‎

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@
88

99
use OCA\DAV\CalDAV\Calendar;
1010
use OCA\DAV\CalDAV\Publishing\PublishPlugin;
11-
use OCP\IConfig;
12-
use OCP\IRequest;
1311
use OCP\IURLGenerator;
1412
use Sabre\DAV\Server;
1513
use Sabre\DAV\SimpleCollection;
@@ -25,27 +23,15 @@ class PublishingTest extends TestCase {
2523
private $server;
2624
/** @var Calendar | \PHPUnit\Framework\MockObject\MockObject */
2725
private $book;
28-
/** @var IConfig | \PHPUnit\Framework\MockObject\MockObject */
29-
private $config;
3026
/** @var IURLGenerator | \PHPUnit\Framework\MockObject\MockObject */
3127
private $urlGenerator;
3228

3329
protected function setUp(): void {
3430
parent::setUp();
3531

36-
$this->config = $this->getMockBuilder(IConfig::class)->
37-
disableOriginalConstructor()->
38-
getMock();
39-
$this->config->expects($this->any())->method('getSystemValue')
40-
->with($this->equalTo('secret'))
41-
->willReturn('mysecret');
42-
43-
$this->urlGenerator = $this->getMockBuilder(IURLGenerator::class)->
44-
disableOriginalConstructor()->
45-
getMock();
32+
$this->urlGenerator = $this->createMock(IURLGenerator::class);
4633

47-
/** @var IRequest $request */
48-
$this->plugin = new PublishPlugin($this->config, $this->urlGenerator);
34+
$this->plugin = new PublishPlugin($this->urlGenerator);
4935

5036
$root = new SimpleCollection('calendars');
5137
$this->server = new Server($root);

0 commit comments

Comments
 (0)