Skip to content

Commit

Permalink
Revert "[stable28] fix(dav): Prevent out-of-office event time drifts"
Browse files Browse the repository at this point in the history
  • Loading branch information
@blizzz
blizzz committed Dec 11, 2023
1 parent c471de2 commit a33ffeb
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 92 deletions.
33 changes: 23 additions & 10 deletions apps/dav/lib/Listener/OutOfOfficeListener.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,9 @@
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 @@ -42,6 +40,9 @@
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 @@ -54,7 +55,6 @@ 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;
}
$tzId = $this->timezoneService->getUserTimezone($userId) ?? $this->timezoneService->getDefaultTimezone();
$vCalendarEvent = $this->createVCalendarEvent($event->getData(), $tzId);
$tz = $calendarNode->getProperties([])['{urn:ietf:params:xml:ns:caldav}calendar-timezone'] ?? null;
$vCalendarEvent = $this->createVCalendarEvent($event->getData(), $tz);
$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;
}
$tzId = $this->timezoneService->getUserTimezone($userId) ?? $this->timezoneService->getDefaultTimezone();
$vCalendarEvent = $this->createVCalendarEvent($event->getData(), $tzId);
$tz = $calendarNode->getProperties([])['{urn:ietf:params:xml:ns:caldav}calendar-timezone'] ?? null;
$vCalendarEvent = $this->createVCalendarEvent($event->getData(), $tz);
try {
$oldEvent = $calendarNode->getChild($this->getEventFileName($event->getData()->getId()));
$oldEvent->put($vCalendarEvent->serialize());
Expand Down Expand Up @@ -169,15 +169,13 @@ private function getEventFileName(string $id): string {
return "out_of_office_$id.ics";
}

private function createVCalendarEvent(IOutOfOfficeData $data, string $tzId): VCalendar {
private function createVCalendarEvent(IOutOfOfficeData $data, ?string $timeZoneData): 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 @@ -190,6 +188,21 @@ private function createVCalendarEvent(IOutOfOfficeData $data, string $tzId): VCa
'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));

Check notice

Code scanning / Psalm

PossiblyNullReference Note

Cannot call method setDateTime on possibly null value
/** @psalm-suppress UndefinedMethod */
$vEvent->DTEND->setDateTime($end->setTimezone($calendarTimeZone)->setTime(0, 0));

Check notice

Code scanning / Psalm

PossiblyNullReference Note

Cannot call method setDateTime on possibly null value
}
return $vCalendar;
}
}
88 changes: 6 additions & 82 deletions apps/dav/tests/unit/Listener/OutOfOfficeListenerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,11 @@

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 @@ -48,9 +45,6 @@
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 @@ -61,23 +55,20 @@ class OutOfOfficeListenerTest extends TestCase {
private ServerFactory|MockObject $serverFactory;
private IConfig|MockObject $appConfig;
private LoggerInterface|MockObject $loggerInterface;
private MockObject|TimezoneService $timezoneService;
private IManager|MockObject $manager;
private OutOfOfficeListener $listener;
private IManager|MockObject $manager;

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 @@ -175,15 +166,11 @@ public function testHandleSchedulingPersonalCalendarNotFound(): void {
$this->listener->handle($event);
}

public function testHandleSchedulingWithDefaultTimezone(): void {
public function testHandleScheduling(): 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 @@ -208,28 +195,12 @@ public function testHandleSchedulingWithDefaultTimezone(): 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')
->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());
});
->method('createFile');
$event = new OutOfOfficeScheduledEvent($data);

$this->listener->handle($event);
Expand Down Expand Up @@ -324,10 +295,6 @@ 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 @@ -352,13 +319,6 @@ 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 @@ -367,33 +327,17 @@ public function testHandleChangeRecreate(): void {
->method('getChild')
->willThrowException(new NotFound());
$calendar->expects(self::once())
->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());
});
->method('createFile');
$event = new OutOfOfficeChangedEvent($data);

$this->listener->handle($event);
}

public function testHandleChangeWithoutTimezone(): void {
public function testHandleChange(): 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 @@ -423,31 +367,11 @@ public function testHandleChangeWithoutTimezone(): 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')
->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());
});
->method('put');
$calendar->expects(self::never())
->method('createFile');
$event = new OutOfOfficeChangedEvent($data);
Expand Down

0 comments on commit a33ffeb

Please sign in to comment.