From 196895b3a7ebdfc5879ad898ba83b5030a9f04df Mon Sep 17 00:00:00 2001 From: Arusekk Date: Thu, 17 Jul 2025 12:25:05 +0200 Subject: [PATCH] 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 https://github.com/nextcloud/server/issues/5551 Closes https://github.com/nextcloud/calendar/issues/4044 Closes https://github.com/nextcloud/server/issues/11214 Signed-off-by: Arusekk Signed-off-by: Richard Steinmetz --- apps/dav/lib/CalDAV/CalendarObject.php | 7 +- .../tests/unit/CalDAV/CalendarObjectTest.php | 139 ++++++++++++++++++ apps/dav/tests/unit/CalDAV/CalendarTest.php | 30 ++-- .../unit/CalDAV/PublicCalendarObjectTest.php | 115 +++++++++++++++ .../tests/unit/CalDAV/PublicCalendarTest.php | 7 +- 5 files changed, 281 insertions(+), 17 deletions(-) create mode 100644 apps/dav/tests/unit/CalDAV/CalendarObjectTest.php create mode 100644 apps/dav/tests/unit/CalDAV/PublicCalendarObjectTest.php diff --git a/apps/dav/lib/CalDAV/CalendarObject.php b/apps/dav/lib/CalDAV/CalendarObject.php index 0a8a0c7af1426..1c8df815b1050 100644 --- a/apps/dav/lib/CalDAV/CalendarObject.php +++ b/apps/dav/lib/CalDAV/CalendarObject.php @@ -55,7 +55,8 @@ public function get() { } // shows as busy if event is declared confidential - if ($this->objectData['classification'] === CalDavBackend::CLASSIFICATION_CONFIDENTIAL) { + if ($this->objectData['classification'] === CalDavBackend::CLASSIFICATION_CONFIDENTIAL + && ($this->isPublic() || !$this->canWrite())) { $this->createConfidentialObject($vObject); } @@ -137,6 +138,10 @@ private function canWrite() { return true; } + private function isPublic(): bool { + return $this->calendarInfo['{http://owncloud.org/ns}public'] ?? false; + } + public function getCalendarId(): int { return (int) $this->objectData['calendarid']; } diff --git a/apps/dav/tests/unit/CalDAV/CalendarObjectTest.php b/apps/dav/tests/unit/CalDAV/CalendarObjectTest.php new file mode 100644 index 0000000000000..cf24c87df7707 --- /dev/null +++ b/apps/dav/tests/unit/CalDAV/CalendarObjectTest.php @@ -0,0 +1,139 @@ +calDavBackend = $this->createMock(CalDavBackend::class); + $this->l10n = $this->createMock(IL10N::class); + + $this->l10n->method('t') + ->willReturnArgument(0); + } + + public static function provideConfidentialObjectData(): array { + return [ + // Shared writable + [ + false, + [ + 'principaluri' => 'user1', + '{http://owncloud.org/ns}owner-principal' => 'user2', + ], + ], + [ + false, + [ + 'principaluri' => 'user1', + '{http://owncloud.org/ns}owner-principal' => 'user2', + '{http://owncloud.org/ns}read-only' => 0, + ], + ], + [ + false, + [ + 'principaluri' => 'user1', + '{http://owncloud.org/ns}owner-principal' => 'user2', + '{http://owncloud.org/ns}read-only' => false, + ], + ], + // Shared read-only + [ + true, + [ + 'principaluri' => 'user1', + '{http://owncloud.org/ns}owner-principal' => 'user2', + '{http://owncloud.org/ns}read-only' => 1, + ], + ], + [ + true, + [ + 'principaluri' => 'user1', + '{http://owncloud.org/ns}owner-principal' => 'user2', + '{http://owncloud.org/ns}read-only' => true, + ], + ], + ]; + } + + /** + * @dataProvider provideConfidentialObjectData + */ + public function testGetWithConfidentialObject( + bool $expectConfidential, + array $calendarInfo, + ): void { + $ics = <<calDavBackend, + $this->l10n, + $calendarInfo, + [ + 'uri' => 'a0f55f1f-4f0e-4db8-a54b-1e8b53846591.ics', + 'calendardata' => $ics, + 'classification' => 2, // CalDavBackend::CLASSIFICATION_CONFIDENTIAL + ], + ); + + $actualIcs = $calendarObject->get(); + $vObject = VObjectReader::read($actualIcs); + + $this->assertInstanceOf(VCalendar::class, $vObject); + $vEvent = $vObject->getBaseComponent('VEVENT'); + $this->assertInstanceOf(VEvent::class, $vEvent); + + if ($expectConfidential) { + $this->assertEquals('Busy', $vEvent->SUMMARY?->getValue()); + $this->assertNull($vEvent->DESCRIPTION); + $this->assertNull($vEvent->LOCATION); + } else { + $this->assertEquals('confidential-event', $vEvent->SUMMARY?->getValue()); + $this->assertNotNull($vEvent->DESCRIPTION); + $this->assertNotNull($vEvent->LOCATION); + } + } +} diff --git a/apps/dav/tests/unit/CalDAV/CalendarTest.php b/apps/dav/tests/unit/CalDAV/CalendarTest.php index 87df1791e8828..972f751bf103a 100644 --- a/apps/dav/tests/unit/CalDAV/CalendarTest.php +++ b/apps/dav/tests/unit/CalDAV/CalendarTest.php @@ -311,9 +311,9 @@ public function testPrivateClassification($expectedChildren, $isShared): void { } $c = new Calendar($backend, $calendarInfo, $this->l10n, $this->config, $this->logger); $children = $c->getChildren(); - $this->assertEquals($expectedChildren, count($children)); + $this->assertCount($expectedChildren, $children); $children = $c->getMultipleChildren(['event-0', 'event-1', 'event-2']); - $this->assertEquals($expectedChildren, count($children)); + $this->assertCount($expectedChildren, $children); $this->assertEquals(!$isShared, $c->childExists('event-2')); } @@ -393,9 +393,13 @@ public function testConfidentialClassification($expectedChildren, $isShared): vo 'id' => 666, 'uri' => 'cal', ]; + + if ($isShared) { + $calendarInfo['{http://owncloud.org/ns}read-only'] = true; + } $c = new Calendar($backend, $calendarInfo, $this->l10n, $this->config, $this->logger); - $this->assertEquals(count($c->getChildren()), $expectedChildren); + $this->assertCount($expectedChildren, $c->getChildren()); // test private event $privateEvent = $c->getChild('event-1'); @@ -600,24 +604,24 @@ public function testRemoveVAlarms() { $this->assertCount(2, $roCalendar->getChildren()); // calendar data shall not be altered for the owner - $this->assertEquals($ownerCalendar->getChild('event-0')->get(), $publicObjectData); - $this->assertEquals($ownerCalendar->getChild('event-1')->get(), $confidentialObjectData); + $this->assertEquals($publicObjectData, $ownerCalendar->getChild('event-0')->get()); + $this->assertEquals($confidentialObjectData, $ownerCalendar->getChild('event-1')->get()); // valarms shall not be removed for read-write shares $this->assertEquals( - $this->fixLinebreak($rwCalendar->getChild('event-0')->get()), - $this->fixLinebreak($publicObjectData)); + $this->fixLinebreak($publicObjectData), + $this->fixLinebreak($rwCalendar->getChild('event-0')->get())); $this->assertEquals( - $this->fixLinebreak($rwCalendar->getChild('event-1')->get()), - $this->fixLinebreak($confidentialObjectCleaned)); + $this->fixLinebreak($confidentialObjectData), + $this->fixLinebreak($rwCalendar->getChild('event-1')->get())); // valarms shall be removed for read-only shares $this->assertEquals( - $this->fixLinebreak($roCalendar->getChild('event-0')->get()), - $this->fixLinebreak($publicObjectDataWithoutVAlarm)); + $this->fixLinebreak($publicObjectDataWithoutVAlarm), + $this->fixLinebreak($roCalendar->getChild('event-0')->get())); $this->assertEquals( - $this->fixLinebreak($roCalendar->getChild('event-1')->get()), - $this->fixLinebreak($confidentialObjectCleaned)); + $this->fixLinebreak($confidentialObjectCleaned), + $this->fixLinebreak($roCalendar->getChild('event-1')->get())); } private function fixLinebreak($str) { diff --git a/apps/dav/tests/unit/CalDAV/PublicCalendarObjectTest.php b/apps/dav/tests/unit/CalDAV/PublicCalendarObjectTest.php new file mode 100644 index 0000000000000..a198039b377cd --- /dev/null +++ b/apps/dav/tests/unit/CalDAV/PublicCalendarObjectTest.php @@ -0,0 +1,115 @@ +calDavBackend = $this->createMock(CalDavBackend::class); + $this->l10n = $this->createMock(IL10N::class); + + $this->l10n->method('t') + ->willReturnArgument(0); + } + + public static function provideConfidentialObjectData(): array { + // For some reason, the CalDavBackend always sets read-only to false. Hence, we test for + // both cases as the property should not matter anyway. + // Ref \OCA\DAV\CalDAV\CalDavBackend::getPublicCalendars (approximately in line 538) + return [ + [ + [ + '{http://owncloud.org/ns}read-only' => true, + '{http://owncloud.org/ns}public' => true, + ], + ], + [ + [ + '{http://owncloud.org/ns}read-only' => false, + '{http://owncloud.org/ns}public' => true, + ], + ], + [ + [ + '{http://owncloud.org/ns}read-only' => 1, + '{http://owncloud.org/ns}public' => true, + ], + ], + [ + [ + '{http://owncloud.org/ns}read-only' => 0, + '{http://owncloud.org/ns}public' => true, + ], + ], + ]; + } + + /** + * @dataProvider provideConfidentialObjectData + */ + public function testGetWithConfidentialObject(array $calendarInfo): void { + $ics = <<calDavBackend, + $this->l10n, + $calendarInfo, + [ + 'uri' => 'a0f55f1f-4f0e-4db8-a54b-1e8b53846591.ics', + 'calendardata' => $ics, + 'classification' => 2, // CalDavBackend::CLASSIFICATION_CONFIDENTIAL + ], + ); + + $actualIcs = $calendarObject->get(); + $vObject = VObjectReader::read($actualIcs); + + $this->assertInstanceOf(VCalendar::class, $vObject); + $vEvent = $vObject->getBaseComponent('VEVENT'); + $this->assertInstanceOf(VEvent::class, $vEvent); + + $this->assertEquals('Busy', $vEvent->SUMMARY?->getValue()); + $this->assertNull($vEvent->DESCRIPTION); + $this->assertNull($vEvent->LOCATION); + } +} diff --git a/apps/dav/tests/unit/CalDAV/PublicCalendarTest.php b/apps/dav/tests/unit/CalDAV/PublicCalendarTest.php index f9523aa77a406..79429f775605b 100644 --- a/apps/dav/tests/unit/CalDAV/PublicCalendarTest.php +++ b/apps/dav/tests/unit/CalDAV/PublicCalendarTest.php @@ -51,9 +51,9 @@ public function testPrivateClassification($expectedChildren, $isShared): void { $logger = $this->createMock(LoggerInterface::class); $c = new PublicCalendar($backend, $calendarInfo, $this->l10n, $config, $logger); $children = $c->getChildren(); - $this->assertEquals(2, count($children)); + $this->assertCount(2, $children); $children = $c->getMultipleChildren(['event-0', 'event-1', 'event-2']); - $this->assertEquals(2, count($children)); + $this->assertCount(2, $children); $this->assertFalse($c->childExists('event-2')); } @@ -132,6 +132,7 @@ public function testConfidentialClassification($expectedChildren, $isShared): vo 'principaluri' => 'user2', 'id' => 666, 'uri' => 'cal', + '{http://owncloud.org/ns}public' => true, ]; /** @var MockObject | IConfig $config */ $config = $this->createMock(IConfig::class); @@ -139,7 +140,7 @@ public function testConfidentialClassification($expectedChildren, $isShared): vo $logger = $this->createMock(LoggerInterface::class); $c = new PublicCalendar($backend, $calendarInfo, $this->l10n, $config, $logger); - $this->assertEquals(count($c->getChildren()), 2); + $this->assertCount(2, $c->getChildren()); // test private event $privateEvent = $c->getChild('event-1');