open-dsi
/
nextcloud-server
mirror of https://github.com/nextcloud/server
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

fix(imip): don't report success when a calendar cannot process iMip

Browse Source 
handleIMip() checked IHandleImipMessage only inside the match, so a
writable calendar unable to process iMip returned true without doing
anything.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
pull/62722/head
Daniel Kesselberg 3 weeks ago
parent c80389a718
commit cf01026fc6
No known key found for this signature in database
GPG Key ID: 4A81C29F63464E8F
2 changed files with 104 additions and 48 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 65
      lib/private/Calendar/Manager.php
  2. 87
      tests/lib/Calendar/ManagerTest.php

65
lib/private/Calendar/Manager.php
Unescape View File

@ -37,8 +37,10 @@ use Sabre\VObject\Component\VFreeBusy;
use Sabre\VObject\ParseException;
use Sabre\VObject\Reader;
use Throwable;
use function array_filter;
use function array_map;
use function array_merge;
use function array_values;
class Manager implements IManager {
/**
@ -250,9 +252,14 @@ class Manager implements IManager {
$userUri = 'principals/users/' . $userId;
$userCalendars = $this->getCalendarsForPrincipal($userUri);
if (empty($userCalendars)) {
$this->logger->warning('iMip message could not be processed because user has no calendars', $logContext);
/** @var list<ICalendarIsWritable&IHandleImipMessage> $userCalendars */
$userCalendars = array_values(array_filter(
$this->getCalendarsForPrincipal($userUri),
fn (ICalendar $calendar): bool => $this->canHandleImip($calendar),
));
if ($userCalendars === []) {
$this->logger->warning('iMip message could not be processed because user has no calendar that can process iMip messages', $logContext);
return false;
}
@ -273,7 +280,7 @@ class Manager implements IManager {
$vEvent = $vObject->VEVENT;
if (!isset($vEvent->UID)) {
$this->logger->warning('iMip message event dose not contains a UID', $logContext);
$this->logger->warning('iMip message event does not contains a UID', $logContext);
return false;
}
@ -290,22 +297,14 @@ class Manager implements IManager {
}
if (!isset($vEvent->ATTENDEE)) {
$this->logger->warning('iMip message event dose not contains any attendees', $logContext);
$this->logger->warning('iMip message event does not contains any attendees', $logContext);
return false;
}
foreach ($userCalendars as $calendar) {
if (!$calendar instanceof ICalendarIsWritable) {
continue;
}
if ($calendar->isDeleted() || !$calendar->isWritable()) {
continue;
}
if (!empty($calendar->search('', [], ['uid' => $vEvent->UID->getValue()]))) {
try {
if ($calendar instanceof IHandleImipMessage) {
$calendar->handleIMipMessage($userId, $vObject->serialize());
}
$calendar->handleIMipMessage($userId, $vObject->serialize());
return true;
} catch (CalendarException $e) {
$logContext['exception'] = $e;
@ -316,26 +315,12 @@ class Manager implements IManager {
}
if (isset($options['absent']) && $options['absent'] === 'create') {
// retrieve the primary calendar for the user
$calendar = $this->getPrimaryCalendar($userId);
if ($calendar !== null && (
!$calendar instanceof IHandleImipMessage || !$calendar instanceof ICalendarIsWritable || $calendar->isDeleted() || !$calendar->isWritable()
)) {
$calendar = null;
}
// if no primary calendar is set, use the first writable calendar
if ($calendar === null) {
foreach ($userCalendars as $userCalendar) {
if ($userCalendar instanceof IHandleImipMessage && $userCalendar instanceof ICalendarIsWritable && !$userCalendar->isDeleted() && $userCalendar->isWritable()) {
$calendar = $userCalendar;
break;
}
}
}
if ($calendar === null) {
$this->logger->warning('iMip message could not be processed because no writable calendar was found', $logContext);
return false;
}
// use the primary calendar of the user, otherwise the first one that can process iMip messages
$primaryCalendar = $this->getPrimaryCalendar($userId);
$calendar = $primaryCalendar !== null && $this->canHandleImip($primaryCalendar)
? $primaryCalendar
: $userCalendars[0];
if (!empty($options['absentCreateStatus'])) {
$status = strtoupper($options['absentCreateStatus']);
@ -367,6 +352,18 @@ class Manager implements IManager {
return false;
}
/**
* Determines if a calendar can be used to process an iMip message
*
* @psalm-assert-if-true ICalendarIsWritable&IHandleImipMessage $calendar
*/
private function canHandleImip(ICalendar $calendar): bool {
return $calendar instanceof ICalendarIsWritable
&& $calendar instanceof IHandleImipMessage
&& $calendar->isWritable()
&& !$calendar->isDeleted();
}
/**
* @since 31.0.0
*

87
tests/lib/Calendar/ManagerTest.php
Unescape View File

@ -41,6 +41,13 @@ interface ITestCalendar extends ICreateFromString, IHandleImipMessage, ICalendar
}
/*
* A writable calendar that is unable to process iMip messages
*/
interface ITestCalendarWithoutImip extends ICreateFromString, ICalendarIsWritable {
}
class ManagerTest extends TestCase {
/** @var Coordinator&MockObject */
private $coordinator;
@ -342,7 +349,7 @@ class ManagerTest extends TestCase {
->willReturn([]);
// construct logger returns
$this->logger->expects(self::once())->method('warning')
->with('iMip message could not be processed because user has no calendars');
->with('iMip message could not be processed because user has no calendar that can process iMip messages');
// construct parameters
$userId = 'attendee1';
$calendar = $this->vCalendar1a;
@ -356,6 +363,12 @@ class ManagerTest extends TestCase {
public function testHandleImipWithNoEvent(): void {
// construct mock user calendar
$userCalendar = $this->createMock(ITestCalendar::class);
$userCalendar->expects(self::once())
->method('isDeleted')
->willReturn(false);
$userCalendar->expects(self::once())
->method('isWritable')
->willReturn(true);
// construct mock calendar manager and returns
/** @var Manager&MockObject $manager */
$manager = $this->getMockBuilder(Manager::class)
@ -433,6 +446,12 @@ class ManagerTest extends TestCase {
public function testHandleImipMissingOrganizerNoRecipient(): void {
// construct mock user calendar
$userCalendar = $this->createMock(ITestCalendar::class);
$userCalendar->expects(self::once())
->method('isDeleted')
->willReturn(false);
$userCalendar->expects(self::once())
->method('isWritable')
->willReturn(true);
// construct mock calendar manager and returns
/** @var Manager&MockObject $manager */
$manager = $this->getMockBuilder(Manager::class)
@ -467,6 +486,12 @@ class ManagerTest extends TestCase {
public function testHandleImipWithNoUid(): void {
// construct mock user calendar
$userCalendar = $this->createMock(ITestCalendar::class);
$userCalendar->expects(self::once())
->method('isDeleted')
->willReturn(false);
$userCalendar->expects(self::once())
->method('isWritable')
->willReturn(true);
// construct mock calendar manager and returns
/** @var Manager&MockObject $manager */
$manager = $this->getMockBuilder(Manager::class)
@ -487,7 +512,7 @@ class ManagerTest extends TestCase {
->willReturn([$userCalendar]);
// construct logger returns
$this->logger->expects(self::once())->method('warning')
->with('iMip message event dose not contains a UID');
->with('iMip message event does not contains a UID');
// construct parameters
$userId = 'attendee1';
$calendar = $this->vCalendar1a;
@ -542,6 +567,42 @@ class ManagerTest extends TestCase {
$this->assertFalse($result);
}
public function testHandleImipWithCalendarUnableToHandleImip(): void {
// construct mock user calendar which is writable but can not process iMip messages
$userCalendar = $this->createMock(ITestCalendarWithoutImip::class);
$userCalendar->expects(self::never())
->method('search');
// construct mock calendar manager and returns
/** @var Manager&MockObject $manager */
$manager = $this->getMockBuilder(Manager::class)
->setConstructorArgs([
$this->coordinator,
$this->container,
$this->logger,
$this->time,
$this->secureRandom,
$this->userManager,
$this->serverFactory,
$this->propertyMapper,
])
->onlyMethods(['getCalendarsForPrincipal'])
->getMock();
$manager->expects(self::once())
->method('getCalendarsForPrincipal')
->willReturn([$userCalendar]);
// construct logger returns
$this->logger->expects(self::once())->method('warning')
->with('iMip message could not be processed because user has no calendar that can process iMip messages');
// construct parameters
$userId = 'attendee1';
$calendar = $this->vCalendar1a;
$calendar->add('METHOD', 'REQUEST');
// test method
$result = $manager->handleIMip($userId, $calendar->serialize());
// Assert
$this->assertFalse($result);
}
public function testHandleImip(): void {
// construct mock user calendar
$userCalendar = $this->createMock(ITestCalendar::class);
@ -586,10 +647,10 @@ class ManagerTest extends TestCase {
public function testHandleImipWithAbsentCreateOption(): void {
// construct mock user calendar (no matching event found)
$userCalendar = $this->createMock(ITestCalendar::class);
$userCalendar->expects(self::exactly(2))
$userCalendar->expects(self::once())
->method('isDeleted')
->willReturn(false);
$userCalendar->expects(self::exactly(2))
$userCalendar->expects(self::once())
->method('isWritable')
->willReturn(true);
$userCalendar->expects(self::once())
@ -683,12 +744,11 @@ class ManagerTest extends TestCase {
public function testHandleImipWithAbsentCreateNoWritableCalendar(): void {
// construct mock user calendar (not writable)
$userCalendar = $this->createMock(ITestCalendar::class);
$userCalendar->expects(self::exactly(2))
->method('isDeleted')
->willReturn(false);
$userCalendar->expects(self::exactly(2))
$userCalendar->expects(self::once())
->method('isWritable')
->willReturn(false);
$userCalendar->expects(self::never())
->method('search');
// construct mock calendar manager and returns
/** @var Manager&MockObject $manager */
$manager = $this->getMockBuilder(Manager::class)
@ -707,12 +767,11 @@ class ManagerTest extends TestCase {
$manager->expects(self::once())
->method('getCalendarsForPrincipal')
->willReturn([$userCalendar]);
$manager->expects(self::once())
->method('getPrimaryCalendar')
->willReturn(null);
$manager->expects(self::never())
->method('getPrimaryCalendar');
// construct logger returns
$this->logger->expects(self::once())->method('warning')
->with('iMip message could not be processed because no writable calendar was found');
->with('iMip message could not be processed because user has no calendar that can process iMip messages');
// construct parameters
$userId = 'attendee1';
$calendar = $this->vCalendar1a;
@ -789,10 +848,10 @@ class ManagerTest extends TestCase {
public function testHandleImipWithAbsentCreateOverwritesExistingStatus(): void {
// construct mock user calendar (no matching event found)
$userCalendar = $this->createMock(ITestCalendar::class);
$userCalendar->expects(self::exactly(2))
$userCalendar->expects(self::once())
->method('isDeleted')
->willReturn(false);
$userCalendar->expects(self::exactly(2))
$userCalendar->expects(self::once())
->method('isWritable')
->willReturn(true);
$userCalendar->expects(self::once())

Write Preview
Loading…
Cancel
Save