Skip to content

Commit bd491b9

Browse files
committed
fix(caldav): prevent unshare entry creation for owner unsharing
- Introduces a `unshare` method in `CalDavBackend` to handle user unshares. - Implements check to determine if unshare entry is needed based on group/circle membership. - Ensures `updateShares` is only used when the calendar owner manages shares. - Resolves issue where unsharing a calendar as owner created an unshare entry in `oc_dav_shares`. Related PRs: - #43117 - #47737 Signed-off-by: Daniel Kesselberg <[email protected]>
1 parent cd6e5ab commit bd491b9

File tree

18 files changed

+846
-118
lines changed

18 files changed

+846
-118
lines changed

apps/dav/appinfo/info.xml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,15 @@
5555
</repair-steps>
5656

5757
<commands>
58+
<command>OCA\DAV\Command\ClearCalendarUnshares</command>
5859
<command>OCA\DAV\Command\CreateAddressBook</command>
5960
<command>OCA\DAV\Command\CreateCalendar</command>
6061
<command>OCA\DAV\Command\CreateSubscription</command>
6162
<command>OCA\DAV\Command\DeleteCalendar</command>
6263
<command>OCA\DAV\Command\DeleteSubscription</command>
6364
<command>OCA\DAV\Command\FixCalendarSyncCommand</command>
6465
<command>OCA\DAV\Command\ListAddressbooks</command>
66+
<command>OCA\DAV\Command\ListCalendarShares</command>
6567
<command>OCA\DAV\Command\ListCalendars</command>
6668
<command>OCA\DAV\Command\ListSubscriptions</command>
6769
<command>OCA\DAV\Command\MoveCalendar</command>

apps/dav/composer/composer/autoload_classmap.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,13 +153,15 @@
153153
'OCA\\DAV\\CardDAV\\UserAddressBooks' => $baseDir . '/../lib/CardDAV/UserAddressBooks.php',
154154
'OCA\\DAV\\CardDAV\\Validation\\CardDavValidatePlugin' => $baseDir . '/../lib/CardDAV/Validation/CardDavValidatePlugin.php',
155155
'OCA\\DAV\\CardDAV\\Xml\\Groups' => $baseDir . '/../lib/CardDAV/Xml/Groups.php',
156+
'OCA\\DAV\\Command\\ClearCalendarUnshares' => $baseDir . '/../lib/Command/ClearCalendarUnshares.php',
156157
'OCA\\DAV\\Command\\CreateAddressBook' => $baseDir . '/../lib/Command/CreateAddressBook.php',
157158
'OCA\\DAV\\Command\\CreateCalendar' => $baseDir . '/../lib/Command/CreateCalendar.php',
158159
'OCA\\DAV\\Command\\CreateSubscription' => $baseDir . '/../lib/Command/CreateSubscription.php',
159160
'OCA\\DAV\\Command\\DeleteCalendar' => $baseDir . '/../lib/Command/DeleteCalendar.php',
160161
'OCA\\DAV\\Command\\DeleteSubscription' => $baseDir . '/../lib/Command/DeleteSubscription.php',
161162
'OCA\\DAV\\Command\\FixCalendarSyncCommand' => $baseDir . '/../lib/Command/FixCalendarSyncCommand.php',
162163
'OCA\\DAV\\Command\\ListAddressbooks' => $baseDir . '/../lib/Command/ListAddressbooks.php',
164+
'OCA\\DAV\\Command\\ListCalendarShares' => $baseDir . '/../lib/Command/ListCalendarShares.php',
163165
'OCA\\DAV\\Command\\ListCalendars' => $baseDir . '/../lib/Command/ListCalendars.php',
164166
'OCA\\DAV\\Command\\ListSubscriptions' => $baseDir . '/../lib/Command/ListSubscriptions.php',
165167
'OCA\\DAV\\Command\\MoveCalendar' => $baseDir . '/../lib/Command/MoveCalendar.php',

apps/dav/composer/composer/autoload_static.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,13 +168,15 @@ class ComposerStaticInitDAV
168168
'OCA\\DAV\\CardDAV\\UserAddressBooks' => __DIR__ . '/..' . '/../lib/CardDAV/UserAddressBooks.php',
169169
'OCA\\DAV\\CardDAV\\Validation\\CardDavValidatePlugin' => __DIR__ . '/..' . '/../lib/CardDAV/Validation/CardDavValidatePlugin.php',
170170
'OCA\\DAV\\CardDAV\\Xml\\Groups' => __DIR__ . '/..' . '/../lib/CardDAV/Xml/Groups.php',
171+
'OCA\\DAV\\Command\\ClearCalendarUnshares' => __DIR__ . '/..' . '/../lib/Command/ClearCalendarUnshares.php',
171172
'OCA\\DAV\\Command\\CreateAddressBook' => __DIR__ . '/..' . '/../lib/Command/CreateAddressBook.php',
172173
'OCA\\DAV\\Command\\CreateCalendar' => __DIR__ . '/..' . '/../lib/Command/CreateCalendar.php',
173174
'OCA\\DAV\\Command\\CreateSubscription' => __DIR__ . '/..' . '/../lib/Command/CreateSubscription.php',
174175
'OCA\\DAV\\Command\\DeleteCalendar' => __DIR__ . '/..' . '/../lib/Command/DeleteCalendar.php',
175176
'OCA\\DAV\\Command\\DeleteSubscription' => __DIR__ . '/..' . '/../lib/Command/DeleteSubscription.php',
176177
'OCA\\DAV\\Command\\FixCalendarSyncCommand' => __DIR__ . '/..' . '/../lib/Command/FixCalendarSyncCommand.php',
177178
'OCA\\DAV\\Command\\ListAddressbooks' => __DIR__ . '/..' . '/../lib/Command/ListAddressbooks.php',
179+
'OCA\\DAV\\Command\\ListCalendarShares' => __DIR__ . '/..' . '/../lib/Command/ListCalendarShares.php',
178180
'OCA\\DAV\\Command\\ListCalendars' => __DIR__ . '/..' . '/../lib/Command/ListCalendars.php',
179181
'OCA\\DAV\\Command\\ListSubscriptions' => __DIR__ . '/..' . '/../lib/Command/ListSubscriptions.php',
180182
'OCA\\DAV\\Command\\MoveCalendar' => __DIR__ . '/..' . '/../lib/Command/MoveCalendar.php',

apps/dav/lib/CalDAV/CalDavBackend.php

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,19 @@
8888
* Code is heavily inspired by https://github.com/fruux/sabre-dav/blob/master/lib/CalDAV/Backend/PDO.php
8989
*
9090
* @package OCA\DAV\CalDAV
91+
*
92+
* @psalm-type CalendarInfo = array{
93+
* id: int,
94+
* uri: string,
95+
* principaluri: string,
96+
* '{http://calendarserver.org/ns/}getctag': string,
97+
* '{http://sabredav.org/ns}sync-token': int,
98+
* '{urn:ietf:params:xml:ns:caldav}supported-calendar-component-set': \Sabre\CalDAV\Xml\Property\SupportedCalendarComponentSet,
99+
* '{urn:ietf:params:xml:ns:caldav}schedule-calendar-transp': \Sabre\CalDAV\Xml\Property\ScheduleCalendarTransp,
100+
* '{DAV:}displayname': string,
101+
* '{urn:ietf:params:xml:ns:caldav}calendar-timezone': ?string,
102+
* '{http://nextcloud.com/ns}owner-displayname': string,
103+
* }
91104
*/
92105
class CalDavBackend extends AbstractBackend implements SyncSupport, SubscriptionSupport, SchedulingSupport {
93106
use TTransactional;
@@ -649,7 +662,8 @@ public function getCalendarByUri($principal, $uri) {
649662
}
650663

651664
/**
652-
* @return array{id: int, uri: string, '{http://calendarserver.org/ns/}getctag': string, '{http://sabredav.org/ns}sync-token': int, '{urn:ietf:params:xml:ns:caldav}supported-calendar-component-set': SupportedCalendarComponentSet, '{urn:ietf:params:xml:ns:caldav}schedule-calendar-transp': ScheduleCalendarTransp, '{urn:ietf:params:xml:ns:caldav}calendar-timezone': ?string }|null
665+
* @psalm-return CalendarInfo|null
666+
* @return array|null
653667
*/
654668
public function getCalendarById(int $calendarId): ?array {
655669
$fields = array_column($this->propertyMap, 0);
@@ -3628,4 +3642,26 @@ protected function purgeObjectInvitations(string $eventId): void {
36283642
->where($cmd->expr()->eq('uid', $cmd->createNamedParameter($eventId, IQueryBuilder::PARAM_STR), IQueryBuilder::PARAM_STR));
36293643
$cmd->executeStatement();
36303644
}
3645+
3646+
public function unshare(IShareable $shareable, string $principal): void {
3647+
$this->atomic(function () use ($shareable, $principal): void {
3648+
$calendarData = $this->getCalendarById($shareable->getResourceId());
3649+
if ($calendarData === null) {
3650+
throw new \RuntimeException('Trying to update shares for non-existing calendar: ' . $shareable->getResourceId());
3651+
}
3652+
3653+
$oldShares = $this->getShares($shareable->getResourceId());
3654+
$unshare = $this->calendarSharingBackend->unshare($shareable, $principal);
3655+
3656+
if ($unshare) {
3657+
$this->dispatcher->dispatchTyped(new CalendarShareUpdatedEvent(
3658+
$shareable->getResourceId(),
3659+
$calendarData,
3660+
$oldShares,
3661+
[],
3662+
[$principal]
3663+
));
3664+
}
3665+
}, $this->db);
3666+
}
36313667
}

apps/dav/lib/CalDAV/Calendar.php

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -214,12 +214,8 @@ public function getOwner(): ?string {
214214
}
215215

216216
public function delete() {
217-
if (isset($this->calendarInfo['{http://owncloud.org/ns}owner-principal']) &&
218-
$this->calendarInfo['{http://owncloud.org/ns}owner-principal'] !== $this->calendarInfo['principaluri']) {
219-
$principal = 'principal:' . parent::getOwner();
220-
$this->caldavBackend->updateShares($this, [], [
221-
$principal
222-
]);
217+
if ($this->isShared()) {
218+
$this->caldavBackend->unshare($this, 'principal:' . $this->getPrincipalURI());
223219
return;
224220
}
225221

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\DAV\Command;
11+
12+
use OCA\DAV\CalDAV\CalDavBackend;
13+
use OCA\DAV\CalDAV\Sharing\Backend;
14+
use OCA\DAV\CalDAV\Sharing\Service;
15+
use OCA\DAV\Connector\Sabre\Principal;
16+
use OCA\DAV\DAV\Sharing\Backend as BackendAlias;
17+
use OCA\DAV\DAV\Sharing\SharingMapper;
18+
use OCP\IAppConfig;
19+
use OCP\IUserManager;
20+
use Symfony\Component\Console\Attribute\AsCommand;
21+
use Symfony\Component\Console\Command\Command;
22+
use Symfony\Component\Console\Helper\QuestionHelper;
23+
use Symfony\Component\Console\Helper\Table;
24+
use Symfony\Component\Console\Input\InputArgument;
25+
use Symfony\Component\Console\Input\InputInterface;
26+
use Symfony\Component\Console\Output\OutputInterface;
27+
use Symfony\Component\Console\Question\ConfirmationQuestion;
28+
29+
#[AsCommand(
30+
name: 'dav:clear-calendar-unshares',
31+
description: 'Clear calendar unshares for a user',
32+
hidden: false,
33+
)]
34+
class ClearCalendarUnshares extends Command {
35+
public function __construct(
36+
private IUserManager $userManager,
37+
private IAppConfig $appConfig,
38+
private Principal $principal,
39+
private CalDavBackend $caldav,
40+
private Backend $sharingBackend,
41+
private Service $sharingService,
42+
private SharingMapper $mapper,
43+
) {
44+
parent::__construct();
45+
}
46+
47+
protected function configure(): void {
48+
$this->addArgument(
49+
'uid',
50+
InputArgument::REQUIRED,
51+
'User whose unshares to clear'
52+
);
53+
}
54+
55+
protected function execute(InputInterface $input, OutputInterface $output): int {
56+
$user = (string)$input->getArgument('uid');
57+
if (!$this->userManager->userExists($user)) {
58+
throw new \InvalidArgumentException("User $user is unknown");
59+
}
60+
61+
$principal = $this->principal->getPrincipalByPath('principals/users/' . $user);
62+
if ($principal === null) {
63+
throw new \InvalidArgumentException("Unable to fetch principal for user $user ");
64+
}
65+
66+
$shares = $this->mapper->getSharesByPrincipals([$principal['uri']], 'calendar');
67+
$unshares = array_filter($shares, static fn ($share) => $share['access'] === BackendAlias::ACCESS_UNSHARED);
68+
69+
if (count($unshares) === 0) {
70+
$output->writeln("User $user has no calendar unshares");
71+
return self::SUCCESS;
72+
}
73+
74+
$rows = array_map(fn ($share) => $this->formatCalendarUnshare($share), $shares);
75+
76+
$table = new Table($output);
77+
$table
78+
->setHeaders(['Share Id', 'Calendar Id', 'Calendar URI', 'Calendar Name'])
79+
->setRows($rows)
80+
->render();
81+
82+
$output->writeln('');
83+
84+
/** @var QuestionHelper $helper */
85+
$helper = $this->getHelper('question');
86+
$question = new ConfirmationQuestion('Please confirm to delete the above calendar unshare entries [y/n]', false);
87+
88+
if ($helper->ask($input, $output, $question)) {
89+
$this->mapper->deleteUnsharesByPrincipal($principal['uri'], 'calendar');
90+
$output->writeln("Calendar unshares for user $user deleted");
91+
}
92+
93+
return self::SUCCESS;
94+
}
95+
96+
private function formatCalendarUnshare(array $share): array {
97+
$calendarInfo = $this->caldav->getCalendarById($share['resourceid']);
98+
99+
$resourceUri = 'Resource not found';
100+
$resourceName = '';
101+
102+
if ($calendarInfo !== null) {
103+
$resourceUri = $calendarInfo['uri'];
104+
$resourceName = $calendarInfo['{DAV:}displayname'];
105+
}
106+
107+
return [
108+
$share['id'],
109+
$share['resourceid'],
110+
$resourceUri,
111+
$resourceName,
112+
];
113+
}
114+
}
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\DAV\Command;
11+
12+
use OCA\DAV\CalDAV\CalDavBackend;
13+
use OCA\DAV\CalDAV\Sharing\Backend;
14+
use OCA\DAV\Connector\Sabre\Principal;
15+
use OCA\DAV\DAV\Sharing\SharingMapper;
16+
use OCP\IUserManager;
17+
use Symfony\Component\Console\Attribute\AsCommand;
18+
use Symfony\Component\Console\Command\Command;
19+
use Symfony\Component\Console\Helper\Table;
20+
use Symfony\Component\Console\Input\InputArgument;
21+
use Symfony\Component\Console\Input\InputInterface;
22+
use Symfony\Component\Console\Input\InputOption;
23+
use Symfony\Component\Console\Output\OutputInterface;
24+
25+
#[AsCommand(
26+
name: 'dav:list-calendar-shares',
27+
description: 'List all calendar shares for a user',
28+
hidden: false,
29+
)]
30+
class ListCalendarShares extends Command {
31+
public function __construct(
32+
private IUserManager $userManager,
33+
private Principal $principal,
34+
private CalDavBackend $caldav,
35+
private SharingMapper $mapper,
36+
) {
37+
parent::__construct();
38+
}
39+
40+
protected function configure(): void {
41+
$this->addArgument(
42+
'uid',
43+
InputArgument::REQUIRED,
44+
'User whose calendar shares will be listed'
45+
);
46+
$this->addOption(
47+
'calendar-id',
48+
'',
49+
InputOption::VALUE_REQUIRED,
50+
'List only shares for the given calendar id id',
51+
null,
52+
);
53+
}
54+
55+
protected function execute(InputInterface $input, OutputInterface $output): int {
56+
$user = (string)$input->getArgument('uid');
57+
if (!$this->userManager->userExists($user)) {
58+
throw new \InvalidArgumentException("User $user is unknown");
59+
}
60+
61+
$principal = $this->principal->getPrincipalByPath('principals/users/' . $user);
62+
if ($principal === null) {
63+
throw new \InvalidArgumentException("Unable to fetch principal for user $user");
64+
}
65+
66+
$memberships = array_merge(
67+
[$principal['uri']],
68+
$this->principal->getGroupMembership($principal['uri']),
69+
$this->principal->getCircleMembership($principal['uri']),
70+
);
71+
72+
$shares = $this->mapper->getSharesByPrincipals($memberships, 'calendar');
73+
74+
$calendarId = $input->getOption('calendar-id');
75+
if ($calendarId !== null) {
76+
$shares = array_filter($shares, fn ($share) => $share['resourceid'] === (int)$calendarId);
77+
}
78+
79+
$rows = array_map(fn ($share) => $this->formatCalendarShare($share), $shares);
80+
81+
if (count($rows) > 0) {
82+
$table = new Table($output);
83+
$table
84+
->setHeaders(['Share Id', 'Calendar Id', 'Calendar URI', 'Calendar Name', 'Calendar Owner', 'Access By', 'Permissions'])
85+
->setRows($rows)
86+
->render();
87+
} else {
88+
$output->writeln("User $user has no calendar shares");
89+
}
90+
91+
return self::SUCCESS;
92+
}
93+
94+
private function formatCalendarShare(array $share): array {
95+
$calendarInfo = $this->caldav->getCalendarById($share['resourceid']);
96+
97+
$calendarUri = 'Resource not found';
98+
$calendarName = '';
99+
$calendarOwner = '';
100+
101+
if ($calendarInfo !== null) {
102+
$calendarUri = $calendarInfo['uri'];
103+
$calendarName = $calendarInfo['{DAV:}displayname'];
104+
$calendarOwner = $calendarInfo['{http://nextcloud.com/ns}owner-displayname'] . ' (' . $calendarInfo['principaluri'] . ')';
105+
}
106+
107+
$accessBy = match (true) {
108+
str_starts_with($share['principaluri'], 'principals/users/') => 'Individual',
109+
str_starts_with($share['principaluri'], 'principals/groups/') => 'Group (' . $share['principaluri'] . ')',
110+
str_starts_with($share['principaluri'], 'principals/circles/') => 'Team (' . $share['principaluri'] . ')',
111+
default => $share['principaluri'],
112+
};
113+
114+
$permissions = match ($share['access']) {
115+
Backend::ACCESS_READ => 'Read',
116+
Backend::ACCESS_READ_WRITE => 'Read/Write',
117+
Backend::ACCESS_UNSHARED => 'Unshare',
118+
default => $share['access'],
119+
};
120+
121+
return [
122+
$share['id'],
123+
$share['resourceid'],
124+
$calendarUri,
125+
$calendarName,
126+
$calendarOwner,
127+
$accessBy,
128+
$permissions,
129+
];
130+
}
131+
}

0 commit comments

Comments
 (0)