Skip to content

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

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

Open
kesselb wants to merge 1 commit into
base: master
Choose a base branch
from
+104 −48
Open

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

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 31 additions & 34 deletions lib/private/Calendar/Manager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,10 @@
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 {
/**
Expand Down Expand Up @@ -250,9 +252,14 @@ public function handleIMip(

$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;
}

Expand All @@ -273,7 +280,7 @@ public function handleIMip(
$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;
}

Expand All @@ -290,22 +297,14 @@ public function handleIMip(
}

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;
Expand All @@ -316,26 +315,12 @@ public function handleIMip(
}

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']);

Expand Down Expand Up @@ -367,6 +352,18 @@ public function handleIMip(
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
*
Expand Down
87 changes: 73 additions & 14 deletions tests/lib/Calendar/ManagerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -342,7 +349,7 @@ public function testHandleImipWithNoCalendars(): void {
->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;
Expand All @@ -356,6 +363,12 @@ public function testHandleImipWithNoCalendars(): void {
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)
Expand Down Expand Up @@ -433,6 +446,12 @@ public function testHandleImipMissingOrganizerWithRecipient(): void {
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)
Expand Down Expand Up @@ -467,6 +486,12 @@ public function testHandleImipMissingOrganizerNoRecipient(): void {
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)
Expand All @@ -487,7 +512,7 @@ public function testHandleImipWithNoUid(): void {
->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;
Expand Down Expand Up @@ -542,6 +567,42 @@ public function testHandleImipWithNoMatch(): void {
$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);
Expand Down Expand Up @@ -586,10 +647,10 @@ public function testHandleImip(): void {
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())
Expand Down Expand Up @@ -683,12 +744,11 @@ public function testHandleImipWithAbsentIgnoreOption(): void {
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)
Expand All @@ -707,12 +767,11 @@ public function testHandleImipWithAbsentCreateNoWritableCalendar(): void {
$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;
Expand Down Expand Up @@ -789,10 +848,10 @@ public function testHandleImipWithAbsentCreateUsesPrimaryCalendar(): void {
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())
Expand Down
Loading