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.

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

[stable27] fix(dav): allow multiple organizers if possible #42375

Merged
merged 1 commit into from Jan 2, 2024
Merged
Changes from all commits
Commits
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
68 changes: 67 additions & 1 deletion apps/dav/lib/CalDAV/Schedule/Plugin.php
Expand Up @@ -36,11 +36,14 @@
use OCP\IConfig;
use Psr\Log\LoggerInterface;
use Sabre\CalDAV\ICalendar;
use Sabre\CalDAV\ICalendarObject;
use Sabre\CalDAV\Schedule\ISchedulingObject;
use Sabre\DAV\INode;
use Sabre\DAV\IProperties;
use Sabre\DAV\PropFind;
use Sabre\DAV\Server;
use Sabre\DAV\Xml\Property\LocalHref;
use Sabre\DAVACL\IACL;
use Sabre\DAVACL\IPrincipal;
use Sabre\HTTP\RequestInterface;
use Sabre\HTTP\ResponseInterface;
Expand All @@ -50,6 +53,7 @@
use Sabre\VObject\DateTimeParser;
use Sabre\VObject\FreeBusyGenerator;
use Sabre\VObject\ITip;
use Sabre\VObject\ITip\SameOrganizerForAllComponentsException;
use Sabre\VObject\Parameter;
use Sabre\VObject\Property;
use Sabre\VObject\Reader;
Expand Down Expand Up @@ -161,7 +165,29 @@
$this->pathOfCalendarObjectChange = $request->getPath();
}

parent::calendarObjectChange($request, $response, $vCal, $calendarPath, $modified, $isNew);
try {
parent::calendarObjectChange($request, $response, $vCal, $calendarPath, $modified, $isNew);
} catch (SameOrganizerForAllComponentsException $e) {
$this->handleSameOrganizerException($e, $vCal, $calendarPath);
}
}

/**
* @inheritDoc
*/
public function beforeUnbind($path): void {
try {
parent::beforeUnbind($path);
} catch (SameOrganizerForAllComponentsException $e) {
$node = $this->server->tree->getNodeForPath($path);
if (!$node instanceof ICalendarObject || $node instanceof ISchedulingObject) {
throw $e;
}

/** @var VCalendar $vCal */
$vCal = Reader::read($node->get());
$this->handleSameOrganizerException($e, $vCal, $path);
}
}

/**
Expand Down Expand Up @@ -630,4 +656,44 @@
'{DAV:}displayname' => $displayName,
]);
}

/**
* Try to handle the given exception gracefully or throw it if necessary.
*
* @throws SameOrganizerForAllComponentsException If the exception should not be ignored
*/
private function handleSameOrganizerException(
SameOrganizerForAllComponentsException $e,
VCalendar $vCal,
string $calendarPath,
): void {
// This is very hacky! However, we want to allow saving events with multiple
// organizers. Those events are not RFC compliant, but sometimes imported from major
// external calendar services (e.g. Google). If the current user is not an organizer of
// the event we ignore the exception as no scheduling messages will be sent anyway.

// It would be cleaner to patch Sabre to validate organizers *after* checking if
// scheduling messages are necessary. Currently, organizers are validated first and
// afterwards the broker checks if messages should be scheduled. So the code will throw
// even if the organizers are not relevant. This is to ensure compliance with RFCs but
// a bit too strict for real world usage.

if (!isset($vCal->VEVENT)) {
throw $e;
}

$calendarNode = $this->server->tree->getNodeForPath($calendarPath);
if (!($calendarNode instanceof IACL)) {
// Should always be an instance of IACL but just to be sure
throw $e;
}

$addresses = $this->getAddressesForPrincipal($calendarNode->getOwner());

Check notice

Code scanning / Psalm

PossiblyNullArgument Note

Argument 1 of OCA\DAV\CalDAV\Schedule\Plugin::getAddressesForPrincipal cannot be null, possibly null value provided
foreach ($vCal->VEVENT as $vevent) {
if (in_array($vevent->ORGANIZER->getNormalizedValue(), $addresses, true)) {
// User is an organizer => throw the exception
throw $e;
}
}
}
}