Skip to content

Commit 47576d1

Browse files
Arusekkst3iny
Arusekk
authored and
st3iny
committed
fix(caldav): show confidential event if writable
If a party can edit the calendar/event, just display it instead of hiding the details and risking overwrites. This might be considered a change impacting privacy, but it actually improves semantics. Relevant test updates included, improving assertion correctness. I think all the relevant use cases are solved by this. Closes #5551 Closes nextcloud/calendar#4044 Closes #11214 Signed-off-by: Arusekk <[email protected]> Signed-off-by: Richard Steinmetz <[email protected]>
1 parent 7ce484e commit 47576d1

File tree

5 files changed

+281
-17
lines changed

5 files changed

+281
-17
lines changed

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ public function get() {
5252
}
5353

5454
// shows as busy if event is declared confidential
55-
if ($this->objectData['classification'] === CalDavBackend::CLASSIFICATION_CONFIDENTIAL) {
55+
if ($this->objectData['classification'] === CalDavBackend::CLASSIFICATION_CONFIDENTIAL
56+
&& ($this->isPublic() || !$this->canWrite())) {
5657
$this->createConfidentialObject($vObject);
5758
}
5859

@@ -134,6 +135,10 @@ private function canWrite() {
134135
return true;
135136
}
136137

138+
private function isPublic(): bool {
139+
return $this->calendarInfo['{http://owncloud.org/ns}public'] ?? false;
140+
}
141+
137142
public function getCalendarId(): int {
138143
return (int)$this->objectData['calendarid'];
139144
}

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

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
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\Tests\unit\CalDAV;
11+
12+
use OCA\DAV\CalDAV\CalDavBackend;
13+
use OCA\DAV\CalDAV\CalendarObject;
14+
use OCP\IL10N;
15+
use PHPUnit\Framework\MockObject\MockObject;
16+
use Sabre\VObject\Component\VCalendar;
17+
use Sabre\VObject\Component\VEvent;
18+
use Sabre\VObject\Reader as VObjectReader;
19+
use Test\TestCase;
20+
21+
class CalendarObjectTest extends TestCase {
22+
private readonly CalDavBackend&MockObject $calDavBackend;
23+
private readonly IL10N&MockObject $l10n;
24+
25+
protected function setUp(): void {
26+
parent::setUp();
27+
28+
$this->calDavBackend = $this->createMock(CalDavBackend::class);
29+
$this->l10n = $this->createMock(IL10N::class);
30+
31+
$this->l10n->method('t')
32+
->willReturnArgument(0);
33+
}
34+
35+
public static function provideConfidentialObjectData(): array {
36+
return [
37+
// Shared writable
38+
[
39+
false,
40+
[
41+
'principaluri' => 'user1',
42+
'{http://owncloud.org/ns}owner-principal' => 'user2',
43+
],
44+
],
45+
[
46+
false,
47+
[
48+
'principaluri' => 'user1',
49+
'{http://owncloud.org/ns}owner-principal' => 'user2',
50+
'{http://owncloud.org/ns}read-only' => 0,
51+
],
52+
],
53+
[
54+
false,
55+
[
56+
'principaluri' => 'user1',
57+
'{http://owncloud.org/ns}owner-principal' => 'user2',
58+
'{http://owncloud.org/ns}read-only' => false,
59+
],
60+
],
61+
// Shared read-only
62+
[
63+
true,
64+
[
65+
'principaluri' => 'user1',
66+
'{http://owncloud.org/ns}owner-principal' => 'user2',
67+
'{http://owncloud.org/ns}read-only' => 1,
68+
],
69+
],
70+
[
71+
true,
72+
[
73+
'principaluri' => 'user1',
74+
'{http://owncloud.org/ns}owner-principal' => 'user2',
75+
'{http://owncloud.org/ns}read-only' => true,
76+
],
77+
],
78+
];
79+
}
80+
81+
/**
82+
* @dataProvider provideConfidentialObjectData
83+
*/
84+
public function testGetWithConfidentialObject(
85+
bool $expectConfidential,
86+
array $calendarInfo,
87+
): void {
88+
$ics = <<<EOF
89+
BEGIN:VCALENDAR
90+
CALSCALE:GREGORIAN
91+
VERSION:2.0
92+
PRODID:-//IDN nextcloud.com//Calendar app 5.5.0-dev.1//EN
93+
BEGIN:VEVENT
94+
CREATED:20250820T102647Z
95+
DTSTAMP:20250820T103038Z
96+
LAST-MODIFIED:20250820T103038Z
97+
SEQUENCE:4
98+
UID:a0f55f1f-4f0e-4db8-a54b-1e8b53846591
99+
DTSTART;TZID=Europe/Berlin:20250822T110000
100+
DTEND;TZID=Europe/Berlin:20250822T170000
101+
STATUS:CONFIRMED
102+
SUMMARY:confidential-event
103+
CLASS:CONFIDENTIAL
104+
LOCATION:A location
105+
DESCRIPTION:A description
106+
END:VEVENT
107+
END:VCALENDAR
108+
EOF;
109+
VObjectReader::read($ics);
110+
111+
$calendarObject = new CalendarObject(
112+
$this->calDavBackend,
113+
$this->l10n,
114+
$calendarInfo,
115+
[
116+
'uri' => 'a0f55f1f-4f0e-4db8-a54b-1e8b53846591.ics',
117+
'calendardata' => $ics,
118+
'classification' => 2, // CalDavBackend::CLASSIFICATION_CONFIDENTIAL
119+
],
120+
);
121+
122+
$actualIcs = $calendarObject->get();
123+
$vObject = VObjectReader::read($actualIcs);
124+
125+
$this->assertInstanceOf(VCalendar::class, $vObject);
126+
$vEvent = $vObject->getBaseComponent('VEVENT');
127+
$this->assertInstanceOf(VEvent::class, $vEvent);
128+
129+
if ($expectConfidential) {
130+
$this->assertEquals('Busy', $vEvent->SUMMARY?->getValue());
131+
$this->assertNull($vEvent->DESCRIPTION);
132+
$this->assertNull($vEvent->LOCATION);
133+
} else {
134+
$this->assertEquals('confidential-event', $vEvent->SUMMARY?->getValue());
135+
$this->assertNotNull($vEvent->DESCRIPTION);
136+
$this->assertNotNull($vEvent->LOCATION);
137+
}
138+
}
139+
}

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

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -310,9 +310,9 @@ public function testPrivateClassification($expectedChildren, $isShared): void {
310310
}
311311
$c = new Calendar($backend, $calendarInfo, $this->l10n, $this->config, $this->logger);
312312
$children = $c->getChildren();
313-
$this->assertEquals($expectedChildren, count($children));
313+
$this->assertCount($expectedChildren, $children);
314314
$children = $c->getMultipleChildren(['event-0', 'event-1', 'event-2']);
315-
$this->assertEquals($expectedChildren, count($children));
315+
$this->assertCount($expectedChildren, $children);
316316

317317
$this->assertEquals(!$isShared, $c->childExists('event-2'));
318318
}
@@ -392,9 +392,13 @@ public function testConfidentialClassification($expectedChildren, $isShared): vo
392392
'id' => 666,
393393
'uri' => 'cal',
394394
];
395+
396+
if ($isShared) {
397+
$calendarInfo['{http://owncloud.org/ns}read-only'] = true;
398+
}
395399
$c = new Calendar($backend, $calendarInfo, $this->l10n, $this->config, $this->logger);
396400

397-
$this->assertEquals(count($c->getChildren()), $expectedChildren);
401+
$this->assertCount($expectedChildren, $c->getChildren());
398402

399403
// test private event
400404
$privateEvent = $c->getChild('event-1');
@@ -599,24 +603,24 @@ public function testRemoveVAlarms(): void {
599603
$this->assertCount(2, $roCalendar->getChildren());
600604

601605
// calendar data shall not be altered for the owner
602-
$this->assertEquals($ownerCalendar->getChild('event-0')->get(), $publicObjectData);
603-
$this->assertEquals($ownerCalendar->getChild('event-1')->get(), $confidentialObjectData);
606+
$this->assertEquals($publicObjectData, $ownerCalendar->getChild('event-0')->get());
607+
$this->assertEquals($confidentialObjectData, $ownerCalendar->getChild('event-1')->get());
604608

605609
// valarms shall not be removed for read-write shares
606610
$this->assertEquals(
607-
$this->fixLinebreak($rwCalendar->getChild('event-0')->get()),
608-
$this->fixLinebreak($publicObjectData));
611+
$this->fixLinebreak($publicObjectData),
612+
$this->fixLinebreak($rwCalendar->getChild('event-0')->get()));
609613
$this->assertEquals(
610-
$this->fixLinebreak($rwCalendar->getChild('event-1')->get()),
611-
$this->fixLinebreak($confidentialObjectCleaned));
614+
$this->fixLinebreak($confidentialObjectData),
615+
$this->fixLinebreak($rwCalendar->getChild('event-1')->get()));
612616

613617
// valarms shall be removed for read-only shares
614618
$this->assertEquals(
615-
$this->fixLinebreak($roCalendar->getChild('event-0')->get()),
616-
$this->fixLinebreak($publicObjectDataWithoutVAlarm));
619+
$this->fixLinebreak($publicObjectDataWithoutVAlarm),
620+
$this->fixLinebreak($roCalendar->getChild('event-0')->get()));
617621
$this->assertEquals(
618-
$this->fixLinebreak($roCalendar->getChild('event-1')->get()),
619-
$this->fixLinebreak($confidentialObjectCleaned));
622+
$this->fixLinebreak($confidentialObjectCleaned),
623+
$this->fixLinebreak($roCalendar->getChild('event-1')->get()));
620624
}
621625

622626
private function fixLinebreak($str) {

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

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
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\Tests\unit\CalDAV;
11+
12+
use OCA\DAV\CalDAV\CalDavBackend;
13+
use OCA\DAV\CalDAV\PublicCalendarObject;
14+
use OCP\IL10N;
15+
use PHPUnit\Framework\MockObject\MockObject;
16+
use Sabre\VObject\Component\VCalendar;
17+
use Sabre\VObject\Component\VEvent;
18+
use Sabre\VObject\Reader as VObjectReader;
19+
use Test\TestCase;
20+
21+
class PublicCalendarObjectTest extends TestCase {
22+
private readonly CalDavBackend&MockObject $calDavBackend;
23+
private readonly IL10N&MockObject $l10n;
24+
25+
protected function setUp(): void {
26+
parent::setUp();
27+
28+
$this->calDavBackend = $this->createMock(CalDavBackend::class);
29+
$this->l10n = $this->createMock(IL10N::class);
30+
31+
$this->l10n->method('t')
32+
->willReturnArgument(0);
33+
}
34+
35+
public static function provideConfidentialObjectData(): array {
36+
// For some reason, the CalDavBackend always sets read-only to false. Hence, we test for
37+
// both cases as the property should not matter anyway.
38+
// Ref \OCA\DAV\CalDAV\CalDavBackend::getPublicCalendars (approximately in line 538)
39+
return [
40+
[
41+
[
42+
'{http://owncloud.org/ns}read-only' => true,
43+
'{http://owncloud.org/ns}public' => true,
44+
],
45+
],
46+
[
47+
[
48+
'{http://owncloud.org/ns}read-only' => false,
49+
'{http://owncloud.org/ns}public' => true,
50+
],
51+
],
52+
[
53+
[
54+
'{http://owncloud.org/ns}read-only' => 1,
55+
'{http://owncloud.org/ns}public' => true,
56+
],
57+
],
58+
[
59+
[
60+
'{http://owncloud.org/ns}read-only' => 0,
61+
'{http://owncloud.org/ns}public' => true,
62+
],
63+
],
64+
];
65+
}
66+
67+
/**
68+
* @dataProvider provideConfidentialObjectData
69+
*/
70+
public function testGetWithConfidentialObject(array $calendarInfo): void {
71+
$ics = <<<EOF
72+
BEGIN:VCALENDAR
73+
CALSCALE:GREGORIAN
74+
VERSION:2.0
75+
PRODID:-//IDN nextcloud.com//Calendar app 5.5.0-dev.1//EN
76+
BEGIN:VEVENT
77+
CREATED:20250820T102647Z
78+
DTSTAMP:20250820T103038Z
79+
LAST-MODIFIED:20250820T103038Z
80+
SEQUENCE:4
81+
UID:a0f55f1f-4f0e-4db8-a54b-1e8b53846591
82+
DTSTART;TZID=Europe/Berlin:20250822T110000
83+
DTEND;TZID=Europe/Berlin:20250822T170000
84+
STATUS:CONFIRMED
85+
SUMMARY:confidential-event
86+
CLASS:CONFIDENTIAL
87+
LOCATION:A location
88+
DESCRIPTION:A description
89+
END:VEVENT
90+
END:VCALENDAR
91+
EOF;
92+
93+
$calendarObject = new PublicCalendarObject(
94+
$this->calDavBackend,
95+
$this->l10n,
96+
$calendarInfo,
97+
[
98+
'uri' => 'a0f55f1f-4f0e-4db8-a54b-1e8b53846591.ics',
99+
'calendardata' => $ics,
100+
'classification' => 2, // CalDavBackend::CLASSIFICATION_CONFIDENTIAL
101+
],
102+
);
103+
104+
$actualIcs = $calendarObject->get();
105+
$vObject = VObjectReader::read($actualIcs);
106+
107+
$this->assertInstanceOf(VCalendar::class, $vObject);
108+
$vEvent = $vObject->getBaseComponent('VEVENT');
109+
$this->assertInstanceOf(VEvent::class, $vEvent);
110+
111+
$this->assertEquals('Busy', $vEvent->SUMMARY?->getValue());
112+
$this->assertNull($vEvent->DESCRIPTION);
113+
$this->assertNull($vEvent->LOCATION);
114+
}
115+
}

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,9 @@ public function testPrivateClassification($expectedChildren, $isShared): void {
5050
$logger = $this->createMock(LoggerInterface::class);
5151
$c = new PublicCalendar($backend, $calendarInfo, $this->l10n, $config, $logger);
5252
$children = $c->getChildren();
53-
$this->assertEquals(2, count($children));
53+
$this->assertCount(2, $children);
5454
$children = $c->getMultipleChildren(['event-0', 'event-1', 'event-2']);
55-
$this->assertEquals(2, count($children));
55+
$this->assertCount(2, $children);
5656

5757
$this->assertFalse($c->childExists('event-2'));
5858
}
@@ -131,14 +131,15 @@ public function testConfidentialClassification($expectedChildren, $isShared): vo
131131
'principaluri' => 'user2',
132132
'id' => 666,
133133
'uri' => 'cal',
134+
'{http://owncloud.org/ns}public' => true,
134135
];
135136
/** @var MockObject | IConfig $config */
136137
$config = $this->createMock(IConfig::class);
137138
/** @var MockObject | LoggerInterface $logger */
138139
$logger = $this->createMock(LoggerInterface::class);
139140
$c = new PublicCalendar($backend, $calendarInfo, $this->l10n, $config, $logger);
140141

141-
$this->assertEquals(count($c->getChildren()), 2);
142+
$this->assertCount(2, $c->getChildren());
142143

143144
// test private event
144145
$privateEvent = $c->getChild('event-1');

0 commit comments

Comments
 (0)