Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[stable28] fix(dav): Prevent out-of-office event time drifts #42163

Merged
merged 1 commit into from Dec 11, 2023
Merged

[stable28] fix(dav): Prevent out-of-office event time drifts #42163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 10 additions & 23 deletions apps/dav/lib/Listener/OutOfOfficeListener.php
View file
Open in desktop
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -55,6 +54,7 @@ class OutOfOfficeListener implements IEventListener {
public function __construct(
private ServerFactory $serverFactory,
private IConfig $appConfig,
private TimezoneService $timezoneService,
private LoggerInterface $logger
) {
}
Expand All @@ -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());
Expand All @@ -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());
Expand Down Expand Up @@ -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);
Expand All @@ -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;
}
}
88 changes: 82 additions & 6 deletions apps/dav/tests/unit/Listener/OutOfOfficeListenerTest.php
View file
Open in desktop
Expand Up @@ -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;
Expand All @@ -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;

/**
Expand All @@ -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
);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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')
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down