Skip to content

Commit 0131bd9

Browse files
AndyScherzingerAndyScherzinger
authored
Merge pull request #62793 from nextcloud/backport/62722/stable33
[stable33] fix(imip): don't report success when a calendar cannot process iMip
2 parents cd3147b + b1391a0 commit 0131bd9

2 files changed

Lines changed: 104 additions & 48 deletions

File tree

‎lib/private/Calendar/Manager.php‎

Lines changed: 31 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,10 @@
3737
use Sabre\VObject\Property\VCard\DateTime;
3838
use Sabre\VObject\Reader;
3939
use Throwable;
40+
use function array_filter;
4041
use function array_map;
4142
use function array_merge;
43+
use function array_values;
4244

4345
class Manager implements IManager {
4446
/**
@@ -239,9 +241,14 @@ public function handleIMip(
239241

240242
$userUri = 'principals/users/' . $userId;
241243

242-
$userCalendars = $this->getCalendarsForPrincipal($userUri);
243-
if (empty($userCalendars)) {
244-
$this->logger->warning('iMip message could not be processed because user has no calendars', $logContext);
244+
/** @var list<ICalendarIsWritable&IHandleImipMessage> $userCalendars */
245+
$userCalendars = array_values(array_filter(
246+
$this->getCalendarsForPrincipal($userUri),
247+
fn (ICalendar $calendar): bool => $this->canHandleImip($calendar),
248+
));
249+
250+
if ($userCalendars === []) {
251+
$this->logger->warning('iMip message could not be processed because user has no calendar that can process iMip messages', $logContext);
245252
return false;
246253
}
247254

@@ -262,7 +269,7 @@ public function handleIMip(
262269
$vEvent = $vObject->VEVENT;
263270

264271
if (!isset($vEvent->UID)) {
265-
$this->logger->warning('iMip message event dose not contains a UID', $logContext);
272+
$this->logger->warning('iMip message event does not contains a UID', $logContext);
266273
return false;
267274
}
268275

@@ -279,22 +286,14 @@ public function handleIMip(
279286
}
280287

281288
if (!isset($vEvent->ATTENDEE)) {
282-
$this->logger->warning('iMip message event dose not contains any attendees', $logContext);
289+
$this->logger->warning('iMip message event does not contains any attendees', $logContext);
283290
return false;
284291
}
285292

286293
foreach ($userCalendars as $calendar) {
287-
if (!$calendar instanceof ICalendarIsWritable) {
288-
continue;
289-
}
290-
if ($calendar->isDeleted() || !$calendar->isWritable()) {
291-
continue;
292-
}
293294
if (!empty($calendar->search('', [], ['uid' => $vEvent->UID->getValue()]))) {
294295
try {
295-
if ($calendar instanceof IHandleImipMessage) {
296-
$calendar->handleIMipMessage($userId, $vObject->serialize());
297-
}
296+
$calendar->handleIMipMessage($userId, $vObject->serialize());
298297
return true;
299298
} catch (CalendarException $e) {
300299
$logContext['exception'] = $e;
@@ -305,26 +304,12 @@ public function handleIMip(
305304
}
306305

307306
if (isset($options['absent']) && $options['absent'] === 'create') {
308-
// retrieve the primary calendar for the user
309-
$calendar = $this->getPrimaryCalendar($userId);
310-
if ($calendar !== null && (
311-
!$calendar instanceof IHandleImipMessage || !$calendar instanceof ICalendarIsWritable || $calendar->isDeleted() || !$calendar->isWritable()
312-
)) {
313-
$calendar = null;
314-
}
315-
// if no primary calendar is set, use the first writable calendar
316-
if ($calendar === null) {
317-
foreach ($userCalendars as $userCalendar) {
318-
if ($userCalendar instanceof IHandleImipMessage && $userCalendar instanceof ICalendarIsWritable && !$userCalendar->isDeleted() && $userCalendar->isWritable()) {
319-
$calendar = $userCalendar;
320-
break;
321-
}
322-
}
323-
}
324-
if ($calendar === null) {
325-
$this->logger->warning('iMip message could not be processed because no writable calendar was found', $logContext);
326-
return false;
327-
}
307+
// use the primary calendar of the user, otherwise the first one that can process iMip messages
308+
$primaryCalendar = $this->getPrimaryCalendar($userId);
309+
$calendar = $primaryCalendar !== null && $this->canHandleImip($primaryCalendar)
310+
? $primaryCalendar
311+
: $userCalendars[0];
312+
328313
if (!empty($options['absentCreateStatus'])) {
329314
$status = strtoupper($options['absentCreateStatus']);
330315

@@ -356,6 +341,18 @@ public function handleIMip(
356341
return false;
357342
}
358343

344+
/**
345+
* Determines if a calendar can be used to process an iMip message
346+
*
347+
* @psalm-assert-if-true ICalendarIsWritable&IHandleImipMessage $calendar
348+
*/
349+
private function canHandleImip(ICalendar $calendar): bool {
350+
return $calendar instanceof ICalendarIsWritable
351+
&& $calendar instanceof IHandleImipMessage
352+
&& $calendar->isWritable()
353+
&& !$calendar->isDeleted();
354+
}
355+
359356
/**
360357
* @since 31.0.0
361358
*

‎tests/lib/Calendar/ManagerTest.php‎

Lines changed: 73 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,13 @@ interface ITestCalendar extends ICreateFromString, IHandleImipMessage, ICalendar
4141

4242
}
4343

44+
/*
45+
* A writable calendar that is unable to process iMip messages
46+
*/
47+
interface ITestCalendarWithoutImip extends ICreateFromString, ICalendarIsWritable {
48+
49+
}
50+
4451
class ManagerTest extends TestCase {
4552
/** @var Coordinator&MockObject */
4653
private $coordinator;
@@ -341,7 +348,7 @@ public function testHandleImipWithNoCalendars(): void {
341348
->willReturn([]);
342349
// construct logger returns
343350
$this->logger->expects(self::once())->method('warning')
344-
->with('iMip message could not be processed because user has no calendars');
351+
->with('iMip message could not be processed because user has no calendar that can process iMip messages');
345352
// construct parameters
346353
$userId = 'attendee1';
347354
$calendar = $this->vCalendar1a;
@@ -355,6 +362,12 @@ public function testHandleImipWithNoCalendars(): void {
355362
public function testHandleImipWithNoEvent(): void {
356363
// construct mock user calendar
357364
$userCalendar = $this->createMock(ITestCalendar::class);
365+
$userCalendar->expects(self::once())
366+
->method('isDeleted')
367+
->willReturn(false);
368+
$userCalendar->expects(self::once())
369+
->method('isWritable')
370+
->willReturn(true);
358371
// construct mock calendar manager and returns
359372
/** @var Manager&MockObject $manager */
360373
$manager = $this->getMockBuilder(Manager::class)
@@ -432,6 +445,12 @@ public function testHandleImipMissingOrganizerWithRecipient(): void {
432445
public function testHandleImipMissingOrganizerNoRecipient(): void {
433446
// construct mock user calendar
434447
$userCalendar = $this->createMock(ITestCalendar::class);
448+
$userCalendar->expects(self::once())
449+
->method('isDeleted')
450+
->willReturn(false);
451+
$userCalendar->expects(self::once())
452+
->method('isWritable')
453+
->willReturn(true);
435454
// construct mock calendar manager and returns
436455
/** @var Manager&MockObject $manager */
437456
$manager = $this->getMockBuilder(Manager::class)
@@ -466,6 +485,12 @@ public function testHandleImipMissingOrganizerNoRecipient(): void {
466485
public function testHandleImipWithNoUid(): void {
467486
// construct mock user calendar
468487
$userCalendar = $this->createMock(ITestCalendar::class);
488+
$userCalendar->expects(self::once())
489+
->method('isDeleted')
490+
->willReturn(false);
491+
$userCalendar->expects(self::once())
492+
->method('isWritable')
493+
->willReturn(true);
469494
// construct mock calendar manager and returns
470495
/** @var Manager&MockObject $manager */
471496
$manager = $this->getMockBuilder(Manager::class)
@@ -486,7 +511,7 @@ public function testHandleImipWithNoUid(): void {
486511
->willReturn([$userCalendar]);
487512
// construct logger returns
488513
$this->logger->expects(self::once())->method('warning')
489-
->with('iMip message event dose not contains a UID');
514+
->with('iMip message event does not contains a UID');
490515
// construct parameters
491516
$userId = 'attendee1';
492517
$calendar = $this->vCalendar1a;
@@ -541,6 +566,42 @@ public function testHandleImipWithNoMatch(): void {
541566
$this->assertFalse($result);
542567
}
543568

569+
public function testHandleImipWithCalendarUnableToHandleImip(): void {
570+
// construct mock user calendar which is writable but can not process iMip messages
571+
$userCalendar = $this->createMock(ITestCalendarWithoutImip::class);
572+
$userCalendar->expects(self::never())
573+
->method('search');
574+
// construct mock calendar manager and returns
575+
/** @var Manager&MockObject $manager */
576+
$manager = $this->getMockBuilder(Manager::class)
577+
->setConstructorArgs([
578+
$this->coordinator,
579+
$this->container,
580+
$this->logger,
581+
$this->time,
582+
$this->secureRandom,
583+
$this->userManager,
584+
$this->serverFactory,
585+
$this->propertyMapper,
586+
])
587+
->onlyMethods(['getCalendarsForPrincipal'])
588+
->getMock();
589+
$manager->expects(self::once())
590+
->method('getCalendarsForPrincipal')
591+
->willReturn([$userCalendar]);
592+
// construct logger returns
593+
$this->logger->expects(self::once())->method('warning')
594+
->with('iMip message could not be processed because user has no calendar that can process iMip messages');
595+
// construct parameters
596+
$userId = 'attendee1';
597+
$calendar = $this->vCalendar1a;
598+
$calendar->add('METHOD', 'REQUEST');
599+
// test method
600+
$result = $manager->handleIMip($userId, $calendar->serialize());
601+
// Assert
602+
$this->assertFalse($result);
603+
}
604+
544605
public function testHandleImip(): void {
545606
// construct mock user calendar
546607
$userCalendar = $this->createMock(ITestCalendar::class);
@@ -585,10 +646,10 @@ public function testHandleImip(): void {
585646
public function testHandleImipWithAbsentCreateOption(): void {
586647
// construct mock user calendar (no matching event found)
587648
$userCalendar = $this->createMock(ITestCalendar::class);
588-
$userCalendar->expects(self::exactly(2))
649+
$userCalendar->expects(self::once())
589650
->method('isDeleted')
590651
->willReturn(false);
591-
$userCalendar->expects(self::exactly(2))
652+
$userCalendar->expects(self::once())
592653
->method('isWritable')
593654
->willReturn(true);
594655
$userCalendar->expects(self::once())
@@ -682,12 +743,11 @@ public function testHandleImipWithAbsentIgnoreOption(): void {
682743
public function testHandleImipWithAbsentCreateNoWritableCalendar(): void {
683744
// construct mock user calendar (not writable)
684745
$userCalendar = $this->createMock(ITestCalendar::class);
685-
$userCalendar->expects(self::exactly(2))
686-
->method('isDeleted')
687-
->willReturn(false);
688-
$userCalendar->expects(self::exactly(2))
746+
$userCalendar->expects(self::once())
689747
->method('isWritable')
690748
->willReturn(false);
749+
$userCalendar->expects(self::never())
750+
->method('search');
691751
// construct mock calendar manager and returns
692752
/** @var Manager&MockObject $manager */
693753
$manager = $this->getMockBuilder(Manager::class)
@@ -706,12 +766,11 @@ public function testHandleImipWithAbsentCreateNoWritableCalendar(): void {
706766
$manager->expects(self::once())
707767
->method('getCalendarsForPrincipal')
708768
->willReturn([$userCalendar]);
709-
$manager->expects(self::once())
710-
->method('getPrimaryCalendar')
711-
->willReturn(null);
769+
$manager->expects(self::never())
770+
->method('getPrimaryCalendar');
712771
// construct logger returns
713772
$this->logger->expects(self::once())->method('warning')
714-
->with('iMip message could not be processed because no writable calendar was found');
773+
->with('iMip message could not be processed because user has no calendar that can process iMip messages');
715774
// construct parameters
716775
$userId = 'attendee1';
717776
$calendar = $this->vCalendar1a;
@@ -788,10 +847,10 @@ public function testHandleImipWithAbsentCreateUsesPrimaryCalendar(): void {
788847
public function testHandleImipWithAbsentCreateOverwritesExistingStatus(): void {
789848
// construct mock user calendar (no matching event found)
790849
$userCalendar = $this->createMock(ITestCalendar::class);
791-
$userCalendar->expects(self::exactly(2))
850+
$userCalendar->expects(self::once())
792851
->method('isDeleted')
793852
->willReturn(false);
794-
$userCalendar->expects(self::exactly(2))
853+
$userCalendar->expects(self::once())
795854
->method('isWritable')
796855
->willReturn(true);
797856
$userCalendar->expects(self::once())

0 commit comments

Comments
 (0)