From 0eb50d7fdc8491a247a44fbac8bcd845974a94ac Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Mon, 11 Dec 2023 11:02:21 +0100 Subject: [PATCH] fix(dav): Prevent out-of-office event time drifts Signed-off-by: Christoph Wurst --- apps/dav/lib/Listener/OutOfOfficeListener.php | 33 +++---- .../unit/Listener/OutOfOfficeListenerTest.php | 88 +++++++++++++++++-- 2 files changed, 92 insertions(+), 29 deletions(-) diff --git a/apps/dav/lib/Listener/OutOfOfficeListener.php b/apps/dav/lib/Listener/OutOfOfficeListener.php index fe8377d17fda3..6adc42ceeba38 100644 --- a/apps/dav/lib/Listener/OutOfOfficeListener.php +++ b/apps/dav/lib/Listener/OutOfOfficeListener.php @@ -26,9 +26,11 @@ namespace OCA\DAV\Listener; use DateTimeImmutable; +use DateTimeZone; use OCA\DAV\CalDAV\CalDavBackend; use OCA\DAV\CalDAV\Calendar; use OCA\DAV\CalDAV\CalendarHome; +use OCA\DAV\CalDAV\TimezoneService; use OCA\DAV\ServerFactory; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; @@ -40,9 +42,6 @@ use Psr\Log\LoggerInterface; use Sabre\DAV\Exception\NotFound; use Sabre\VObject\Component\VCalendar; -use Sabre\VObject\Component\VEvent; -use Sabre\VObject\Component\VTimeZone; -use Sabre\VObject\Reader; use function fclose; use function fopen; use function fwrite; @@ -55,6 +54,7 @@ class OutOfOfficeListener implements IEventListener { public function __construct( private ServerFactory $serverFactory, private IConfig $appConfig, + private TimezoneService $timezoneService, private LoggerInterface $logger ) { } @@ -67,8 +67,8 @@ public function handle(Event $event): void { if ($calendarNode === null) { return; } - $tz = $calendarNode->getProperties([])['{urn:ietf:params:xml:ns:caldav}calendar-timezone'] ?? null; - $vCalendarEvent = $this->createVCalendarEvent($event->getData(), $tz); + $tzId = $this->timezoneService->getUserTimezone($userId) ?? $this->timezoneService->getDefaultTimezone(); + $vCalendarEvent = $this->createVCalendarEvent($event->getData(), $tzId); $stream = fopen('php://memory', 'rb+'); try { fwrite($stream, $vCalendarEvent->serialize()); @@ -87,8 +87,8 @@ public function handle(Event $event): void { if ($calendarNode === null) { return; } - $tz = $calendarNode->getProperties([])['{urn:ietf:params:xml:ns:caldav}calendar-timezone'] ?? null; - $vCalendarEvent = $this->createVCalendarEvent($event->getData(), $tz); + $tzId = $this->timezoneService->getUserTimezone($userId) ?? $this->timezoneService->getDefaultTimezone(); + $vCalendarEvent = $this->createVCalendarEvent($event->getData(), $tzId); try { $oldEvent = $calendarNode->getChild($this->getEventFileName($event->getData()->getId())); $oldEvent->put($vCalendarEvent->serialize()); @@ -169,13 +169,15 @@ private function getEventFileName(string $id): string { return "out_of_office_$id.ics"; } - private function createVCalendarEvent(IOutOfOfficeData $data, ?string $timeZoneData): VCalendar { + private function createVCalendarEvent(IOutOfOfficeData $data, string $tzId): VCalendar { $shortMessage = $data->getShortMessage(); $longMessage = $data->getMessage(); $start = (new DateTimeImmutable) + ->setTimezone(new DateTimeZone($tzId)) ->setTimestamp($data->getStartDate()) ->setTime(0, 0); $end = (new DateTimeImmutable()) + ->setTimezone(new DateTimeZone($tzId)) ->setTimestamp($data->getEndDate()) ->modify('+ 1 days') ->setTime(0, 0); @@ -188,21 +190,6 @@ private function createVCalendarEvent(IOutOfOfficeData $data, ?string $timeZoneD 'DTEND' => $end, 'X-NEXTCLOUD-OUT-OF-OFFICE' => $data->getId(), ]); - /** @var VEvent $vEvent */ - $vEvent = $vCalendar->VEVENT; - if ($timeZoneData !== null) { - /** @var VCalendar $vtimezoneObj */ - $vtimezoneObj = Reader::read($timeZoneData); - /** @var VTimeZone $vtimezone */ - $vtimezone = $vtimezoneObj->VTIMEZONE; - $calendarTimeZone = $vtimezone->getTimeZone(); - $vCalendar->add($vtimezone); - - /** @psalm-suppress UndefinedMethod */ - $vEvent->DTSTART->setDateTime($start->setTimezone($calendarTimeZone)->setTime(0, 0)); - /** @psalm-suppress UndefinedMethod */ - $vEvent->DTEND->setDateTime($end->setTimezone($calendarTimeZone)->setTime(0, 0)); - } return $vCalendar; } } diff --git a/apps/dav/tests/unit/Listener/OutOfOfficeListenerTest.php b/apps/dav/tests/unit/Listener/OutOfOfficeListenerTest.php index 919cf0a9ccf35..7d13dd22f69e4 100644 --- a/apps/dav/tests/unit/Listener/OutOfOfficeListenerTest.php +++ b/apps/dav/tests/unit/Listener/OutOfOfficeListenerTest.php @@ -25,11 +25,14 @@ namespace OCA\DAV\Tests\Unit\Listener; +use DateTimeImmutable; +use InvalidArgumentException; use OCA\DAV\CalDAV\Calendar; use OCA\DAV\CalDAV\CalendarHome; use OCA\DAV\CalDAV\CalendarObject; use OCA\DAV\CalDAV\InvitationResponse\InvitationResponseServer; use OCA\DAV\CalDAV\Plugin; +use OCA\DAV\CalDAV\TimezoneService; use OCA\DAV\Connector\Sabre\Server; use OCA\DAV\Listener\OutOfOfficeListener; use OCA\DAV\ServerFactory; @@ -45,6 +48,9 @@ use Psr\Log\LoggerInterface; use Sabre\DAV\Exception\NotFound; use Sabre\DAV\Tree; +use Sabre\VObject\Component\VCalendar; +use Sabre\VObject\Component\VEvent; +use Sabre\VObject\Reader; use Test\TestCase; /** @@ -55,20 +61,23 @@ class OutOfOfficeListenerTest extends TestCase { private ServerFactory|MockObject $serverFactory; private IConfig|MockObject $appConfig; private LoggerInterface|MockObject $loggerInterface; - private OutOfOfficeListener $listener; + private MockObject|TimezoneService $timezoneService; private IManager|MockObject $manager; + private OutOfOfficeListener $listener; protected function setUp(): void { parent::setUp(); $this->serverFactory = $this->createMock(ServerFactory::class); $this->appConfig = $this->createMock(IConfig::class); + $this->timezoneService = $this->createMock(TimezoneService::class); $this->loggerInterface = $this->createMock(LoggerInterface::class); $this->manager = $this->createMock(IManager::class); $this->listener = new OutOfOfficeListener( $this->serverFactory, $this->appConfig, + $this->timezoneService, $this->loggerInterface, $this->manager ); @@ -166,11 +175,15 @@ public function testHandleSchedulingPersonalCalendarNotFound(): void { $this->listener->handle($event); } - public function testHandleScheduling(): void { + public function testHandleSchedulingWithDefaultTimezone(): void { $user = $this->createMock(IUser::class); $user->method('getUID')->willReturn('user123'); $data = $this->createMock(IOutOfOfficeData::class); $data->method('getUser')->willReturn($user); + $data->method('getStartDate') + ->willReturn((new DateTimeImmutable('2023-12-12T00:00:00Z'))->getTimestamp()); + $data->method('getEndDate') + ->willReturn((new DateTimeImmutable('2023-12-13T00:00:00Z'))->getTimestamp()); $davServer = $this->createMock(Server::class); $invitationServer = $this->createMock(InvitationResponseServer::class); $invitationServer->method('getServer')->willReturn($davServer); @@ -195,12 +208,28 @@ public function testHandleScheduling(): void { ->with('user123', 'dav', 'defaultCalendar', 'personal') ->willReturn('personal-1'); $calendar = $this->createMock(Calendar::class); + $this->timezoneService->expects(self::once()) + ->method('getUserTimezone') + ->with('user123') + ->willReturn('Europe/Prague'); $calendarHome->expects(self::once()) ->method('getChild') ->with('personal-1') ->willReturn($calendar); $calendar->expects(self::once()) - ->method('createFile'); + ->method('createFile') + ->willReturnCallback(function ($name, $data) { + $vcalendar = Reader::read($data); + if (!($vcalendar instanceof VCalendar)) { + throw new InvalidArgumentException('Calendar data should be a VCALENDAR'); + } + $vevent = $vcalendar->VEVENT; + if ($vevent === null || !($vevent instanceof VEvent)) { + throw new InvalidArgumentException('Calendar data should contain a VEVENT'); + } + self::assertSame('Europe/Prague', $vevent->DTSTART['TZID']?->getValue()); + self::assertSame('Europe/Prague', $vevent->DTEND['TZID']?->getValue()); + }); $event = new OutOfOfficeScheduledEvent($data); $this->listener->handle($event); @@ -295,6 +324,10 @@ public function testHandleChangeRecreate(): void { $user->method('getUID')->willReturn('user123'); $data = $this->createMock(IOutOfOfficeData::class); $data->method('getUser')->willReturn($user); + $data->method('getStartDate') + ->willReturn((new DateTimeImmutable('2023-12-12T00:00:00Z'))->getTimestamp()); + $data->method('getEndDate') + ->willReturn((new DateTimeImmutable('2023-12-14T00:00:00Z'))->getTimestamp()); $davServer = $this->createMock(Server::class); $invitationServer = $this->createMock(InvitationResponseServer::class); $invitationServer->method('getServer')->willReturn($davServer); @@ -319,6 +352,13 @@ public function testHandleChangeRecreate(): void { ->with('user123', 'dav', 'defaultCalendar', 'personal') ->willReturn('personal-1'); $calendar = $this->createMock(Calendar::class); + $this->timezoneService->expects(self::once()) + ->method('getUserTimezone') + ->with('user123') + ->willReturn(null); + $this->timezoneService->expects(self::once()) + ->method('getDefaultTimezone') + ->willReturn('Europe/Berlin'); $calendarHome->expects(self::once()) ->method('getChild') ->with('personal-1') @@ -327,17 +367,33 @@ public function testHandleChangeRecreate(): void { ->method('getChild') ->willThrowException(new NotFound()); $calendar->expects(self::once()) - ->method('createFile'); + ->method('createFile') + ->willReturnCallback(function ($name, $data) { + $vcalendar = Reader::read($data); + if (!($vcalendar instanceof VCalendar)) { + throw new InvalidArgumentException('Calendar data should be a VCALENDAR'); + } + $vevent = $vcalendar->VEVENT; + if ($vevent === null || !($vevent instanceof VEvent)) { + throw new InvalidArgumentException('Calendar data should contain a VEVENT'); + } + self::assertSame('Europe/Berlin', $vevent->DTSTART['TZID']?->getValue()); + self::assertSame('Europe/Berlin', $vevent->DTEND['TZID']?->getValue()); + }); $event = new OutOfOfficeChangedEvent($data); $this->listener->handle($event); } - public function testHandleChange(): void { + public function testHandleChangeWithoutTimezone(): void { $user = $this->createMock(IUser::class); $user->method('getUID')->willReturn('user123'); $data = $this->createMock(IOutOfOfficeData::class); $data->method('getUser')->willReturn($user); + $data->method('getStartDate') + ->willReturn((new DateTimeImmutable('2023-01-12T00:00:00Z'))->getTimestamp()); + $data->method('getEndDate') + ->willReturn((new DateTimeImmutable('2023-12-14T00:00:00Z'))->getTimestamp()); $davServer = $this->createMock(Server::class); $invitationServer = $this->createMock(InvitationResponseServer::class); $invitationServer->method('getServer')->willReturn($davServer); @@ -367,11 +423,31 @@ public function testHandleChange(): void { ->with('personal-1') ->willReturn($calendar); $eventNode = $this->createMock(CalendarObject::class); + $this->timezoneService->expects(self::once()) + ->method('getUserTimezone') + ->with('user123') + ->willReturn(null); + $this->timezoneService->expects(self::once()) + ->method('getDefaultTimezone') + ->willReturn('UTC'); $calendar->expects(self::once()) ->method('getChild') ->willReturn($eventNode); $eventNode->expects(self::once()) - ->method('put'); + ->method('put') + ->willReturnCallback(function ($data) { + $vcalendar = Reader::read($data); + if (!($vcalendar instanceof VCalendar)) { + throw new InvalidArgumentException('Calendar data should be a VCALENDAR'); + } + $vevent = $vcalendar->VEVENT; + if ($vevent === null || !($vevent instanceof VEvent)) { + throw new InvalidArgumentException('Calendar data should contain a VEVENT'); + } + // UTC datetimes are stored without a TZID + self::assertSame(null, $vevent->DTSTART['TZID']?->getValue()); + self::assertSame(null, $vevent->DTEND['TZID']?->getValue()); + }); $calendar->expects(self::never()) ->method('createFile'); $event = new OutOfOfficeChangedEvent($data);