Skip to content

Commit f9b96ca

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 <floss@arusekk.pl> Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
1 parent de46e39 commit f9b96ca

File tree

5 files changed

+276
-17
lines changed

5 files changed

+276
-17
lines changed

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

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

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

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

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

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

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
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\Attributes\DataProvider;
16+
use PHPUnit\Framework\MockObject\MockObject;
17+
use Sabre\VObject\Component\VCalendar;
18+
use Sabre\VObject\Component\VEvent;
19+
use Sabre\VObject\Reader as VObjectReader;
20+
use Test\TestCase;
21+
22+
class CalendarObjectTest extends TestCase {
23+
private readonly CalDavBackend&MockObject $calDavBackend;
24+
private readonly IL10N&MockObject $l10n;
25+
26+
protected function setUp(): void {
27+
parent::setUp();
28+
29+
$this->calDavBackend = $this->createMock(CalDavBackend::class);
30+
$this->l10n = $this->createMock(IL10N::class);
31+
32+
$this->l10n->method('t')
33+
->willReturnArgument(0);
34+
}
35+
36+
public static function provideConfidentialObjectData(): array {
37+
return [
38+
// Shared writable
39+
[
40+
false,
41+
[
42+
'principaluri' => 'user1',
43+
'{http://owncloud.org/ns}owner-principal' => 'user2',
44+
],
45+
],
46+
[
47+
false,
48+
[
49+
'principaluri' => 'user1',
50+
'{http://owncloud.org/ns}owner-principal' => 'user2',
51+
'{http://owncloud.org/ns}read-only' => 0,
52+
],
53+
],
54+
// Shared read-only
55+
[
56+
true,
57+
[
58+
'principaluri' => 'user1',
59+
'{http://owncloud.org/ns}owner-principal' => 'user2',
60+
'{http://owncloud.org/ns}read-only' => 1,
61+
],
62+
],
63+
];
64+
}
65+
66+
#[DataProvider('provideConfidentialObjectData')]
67+
public function testGetWithConfidentialObject(
68+
bool $expectConfidential,
69+
array $calendarInfo,
70+
): 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+
VObjectReader::read($ics);
93+
94+
$calendarObject = new CalendarObject(
95+
$this->calDavBackend,
96+
$this->l10n,
97+
$calendarInfo,
98+
[
99+
'uri' => 'a0f55f1f-4f0e-4db8-a54b-1e8b53846591.ics',
100+
'calendardata' => $ics,
101+
'classification' => 2, // CalDavBackend::CLASSIFICATION_CONFIDENTIAL
102+
],
103+
);
104+
105+
$actualIcs = $calendarObject->get();
106+
$vObject = VObjectReader::read($actualIcs);
107+
108+
$this->assertInstanceOf(VCalendar::class, $vObject);
109+
$vEvent = $vObject->getBaseComponent('VEVENT');
110+
$this->assertInstanceOf(VEvent::class, $vEvent);
111+
112+
if ($expectConfidential) {
113+
$this->assertEquals('Busy', $vEvent->SUMMARY?->getValue());
114+
$this->assertNull($vEvent->DESCRIPTION);
115+
$this->assertNull($vEvent->LOCATION);
116+
} else {
117+
$this->assertEquals('confidential-event', $vEvent->SUMMARY?->getValue());
118+
$this->assertNotNull($vEvent->DESCRIPTION);
119+
$this->assertNotNull($vEvent->LOCATION);
120+
}
121+
}
122+
}

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

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -297,9 +297,9 @@ public function testPrivateClassification(int $expectedChildren, bool $isShared)
297297
}
298298
$c = new Calendar($backend, $calendarInfo, $this->l10n, $this->config, $this->logger);
299299
$children = $c->getChildren();
300-
$this->assertEquals($expectedChildren, count($children));
300+
$this->assertCount($expectedChildren, $children);
301301
$children = $c->getMultipleChildren(['event-0', 'event-1', 'event-2']);
302-
$this->assertEquals($expectedChildren, count($children));
302+
$this->assertCount($expectedChildren, $children);
303303

304304
$this->assertEquals(!$isShared, $c->childExists('event-2'));
305305
}
@@ -375,9 +375,13 @@ public function testConfidentialClassification(int $expectedChildren, bool $isSh
375375
'id' => 666,
376376
'uri' => 'cal',
377377
];
378+
379+
if ($isShared) {
380+
$calendarInfo['{http://owncloud.org/ns}read-only'] = true;
381+
}
378382
$c = new Calendar($backend, $calendarInfo, $this->l10n, $this->config, $this->logger);
379383

380-
$this->assertEquals(count($c->getChildren()), $expectedChildren);
384+
$this->assertCount($expectedChildren, $c->getChildren());
381385

382386
// test private event
383387
$privateEvent = $c->getChild('event-1');
@@ -582,24 +586,24 @@ public function testRemoveVAlarms(): void {
582586
$this->assertCount(2, $roCalendar->getChildren());
583587

584588
// calendar data shall not be altered for the owner
585-
$this->assertEquals($ownerCalendar->getChild('event-0')->get(), $publicObjectData);
586-
$this->assertEquals($ownerCalendar->getChild('event-1')->get(), $confidentialObjectData);
589+
$this->assertEquals($publicObjectData, $ownerCalendar->getChild('event-0')->get());
590+
$this->assertEquals($confidentialObjectData, $ownerCalendar->getChild('event-1')->get());
587591

588592
// valarms shall not be removed for read-write shares
589593
$this->assertEquals(
590-
$this->fixLinebreak($rwCalendar->getChild('event-0')->get()),
591-
$this->fixLinebreak($publicObjectData));
594+
$this->fixLinebreak($publicObjectData),
595+
$this->fixLinebreak($rwCalendar->getChild('event-0')->get()));
592596
$this->assertEquals(
593-
$this->fixLinebreak($rwCalendar->getChild('event-1')->get()),
594-
$this->fixLinebreak($confidentialObjectCleaned));
597+
$this->fixLinebreak($confidentialObjectData),
598+
$this->fixLinebreak($rwCalendar->getChild('event-1')->get()));
595599

596600
// valarms shall be removed for read-only shares
597601
$this->assertEquals(
598-
$this->fixLinebreak($roCalendar->getChild('event-0')->get()),
599-
$this->fixLinebreak($publicObjectDataWithoutVAlarm));
602+
$this->fixLinebreak($publicObjectDataWithoutVAlarm),
603+
$this->fixLinebreak($roCalendar->getChild('event-0')->get()));
600604
$this->assertEquals(
601-
$this->fixLinebreak($roCalendar->getChild('event-1')->get()),
602-
$this->fixLinebreak($confidentialObjectCleaned));
605+
$this->fixLinebreak($confidentialObjectCleaned),
606+
$this->fixLinebreak($roCalendar->getChild('event-1')->get()));
603607
}
604608

605609
private function fixLinebreak(string $str): string {

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

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

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

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

5555
$this->assertFalse($c->childExists('event-2'));
5656
}
@@ -125,14 +125,15 @@ public function testConfidentialClassification(int $expectedChildren, bool $isSh
125125
'principaluri' => 'user2',
126126
'id' => 666,
127127
'uri' => 'cal',
128+
'{http://owncloud.org/ns}public' => true,
128129
];
129130
/** @var IConfig&MockObject $config */
130131
$config = $this->createMock(IConfig::class);
131132
/** @var LoggerInterface&MockObject $logger */
132133
$logger = $this->createMock(LoggerInterface::class);
133134
$c = new PublicCalendar($backend, $calendarInfo, $this->l10n, $config, $logger);
134135

135-
$this->assertEquals(count($c->getChildren()), 2);
136+
$this->assertCount(2, $c->getChildren());
136137

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

0 commit comments

Comments
 (0)