From b00d01e85707c5aa01a1151c7646672333afae91 Mon Sep 17 00:00:00 2001 From: Claus-Justus Heine Date: Thu, 16 Feb 2023 19:03:21 +0100 Subject: [PATCH] feat: enable invitations for shared calendars This is basically the original commit, but with the addition that shared calendars in to account and adjusts their uri s.t. the Sabre library can find the returned object uri in the user's calendar collection. Signed-off-by: Claus-Justus Heine Try to fix some psalm things. No change to the code apart from that. Signed-off-by: Claus-Justus Heine getCalendarObjectByUID(): prefer objects in shared writable calendars over private objects. Signed-off-by: Claus-Justus Heine fix: include the principalUri for dav_shares clause Signed-off-by: Daniel Kesselberg feat: use getPrincipalURI only for shared calendars Signed-off-by: Daniel Kesselberg --- apps/dav/lib/CalDAV/CalDavBackend.php | 55 ++++++-- apps/dav/lib/CalDAV/Schedule/Plugin.php | 94 +++++++++++++- .../tests/unit/CalDAV/Schedule/PluginTest.php | 117 ++++++++++++++++++ 3 files changed, 253 insertions(+), 13 deletions(-) diff --git a/apps/dav/lib/CalDAV/CalDavBackend.php b/apps/dav/lib/CalDAV/CalDavBackend.php index a948c54ad5829..52f283773437b 100644 --- a/apps/dav/lib/CalDAV/CalDavBackend.php +++ b/apps/dav/lib/CalDAV/CalDavBackend.php @@ -2171,21 +2171,58 @@ public function searchPrincipalUri(string $principalUri, * @return string|null */ public function getCalendarObjectByUID($principalUri, $uid) { + // query for shared writable calendars + $principals = array_merge( + [$principalUri], + $this->principalBackend->getGroupMembership($principalUri, true), + $this->principalBackend->getCircleMembership($principalUri) + ); + $query = $this->db->getQueryBuilder(); - $query->selectAlias('c.uri', 'calendaruri')->selectAlias('co.uri', 'objecturi') + $query + ->selectAlias('c.id', 'calendarid') + ->selectAlias('c.principaluri', 'principaluri') + ->selectAlias('c.uri', 'calendaruri') + ->selectAlias('co.uri', 'objecturi') + ->selectAlias('ds.access', 'access') + ->selectAlias('ds.principaluri', 'shareprincipal') ->from('calendarobjects', 'co') ->leftJoin('co', 'calendars', 'c', $query->expr()->eq('co.calendarid', 'c.id')) - ->where($query->expr()->eq('c.principaluri', $query->createNamedParameter($principalUri))) - ->andWhere($query->expr()->eq('co.uid', $query->createNamedParameter($uid))) - ->andWhere($query->expr()->isNull('co.deleted_at')); + ->leftJoin('co', 'dav_shares', 'ds', $query->expr()->eq('co.calendarid', 'ds.resourceid')) + ->where($query->expr()->eq('co.uid', $query->createNamedParameter($uid))) + ->andWhere($query->expr()->isNull('co.deleted_at')) + ->andWhere($query->expr()->orX( + $query->expr()->eq('c.principaluri', $query->createNamedParameter($principalUri)), + $query->expr()->andX( + $query->expr()->in('ds.principaluri', $query->createNamedParameter($principals, IQueryBuilder::PARAM_STR_ARRAY)), + $query->expr()->eq('ds.type', $query->createNamedParameter('calendar')), + $query->expr()->eq('ds.access', $query->createNamedParameter(Backend::ACCESS_READ_WRITE)), + ) + )); $stmt = $query->executeQuery(); - $row = $stmt->fetch(); - $stmt->closeCursor(); - if ($row) { - return $row['calendaruri'] . '/' . $row['objecturi']; + $calendarObjectUri = null; + while ($row = $stmt->fetch()) { + if ($row['principaluri'] != $principalUri && !empty($row['shareprincipal']) && $row['access'] == Backend::ACCESS_READ_WRITE) { + /** + * This seeems to be a false positive: we have "use Sabre\Uri" and Uri\split() IS defined. + * + * @psalm-suppress UndefinedFunction + */ + [, $name] = Uri\split($row['principaluri']); + $calendarUri = $row['calendaruri'] . '_shared_by_' . $name; + } elseif (!empty($calendarObjectUri)) { + // There could be multiple entries for the UID if the share + // permissions have been changed "in between". In this case we + // prefer the shared calendar object. + continue; + } else { + $calendarUri = $row['calendaruri']; + } + $calendarObjectUri = $calendarUri . '/' . $row['objecturi']; } + $stmt->closeCursor(); - return null; + return $calendarObjectUri; } public function getCalendarObjectById(string $principalUri, int $id): ?array { diff --git a/apps/dav/lib/CalDAV/Schedule/Plugin.php b/apps/dav/lib/CalDAV/Schedule/Plugin.php index 0751638b697e6..12ba4f036f6d7 100644 --- a/apps/dav/lib/CalDAV/Schedule/Plugin.php +++ b/apps/dav/lib/CalDAV/Schedule/Plugin.php @@ -148,9 +148,6 @@ protected function getAddressesForPrincipal($principal) { } /** - * @param RequestInterface $request - * @param ResponseInterface $response - * @param VCalendar $vCal * @param mixed $calendarPath * @param mixed $modified * @param mixed $isNew @@ -161,7 +158,96 @@ public function calendarObjectChange(RequestInterface $request, ResponseInterfac $this->pathOfCalendarObjectChange = $request->getPath(); } - parent::calendarObjectChange($request, $response, $vCal, $calendarPath, $modified, $isNew); + if (!$this->scheduleReply($this->server->httpRequest)) { + return; + } + + /** @var Calendar $calendarNode */ + $calendarNode = $this->server->tree->getNodeForPath($calendarPath); + + /** @var bool $isSharedCalendar is the calendar shared? */ + $isSharedCalendar = str_contains($calendarPath, '_shared_by_'); + + /** + * Calendar "Alice & Bob" shared from Alice (owner) to Bob (can edit) + * + * Alice adds an event with Jane as attendee + * - $isSharedCalendar = false, because Alice is the owner + * - $principal = principals/users/alice + * + * Bob adds an event with John as attendee + * - $isSharedCalendar = true, because Alice is the owner + * - $principal = principals/users/bob + */ + if ($isSharedCalendar) { + $principal = $calendarNode->getPrincipalURI(); + } else { + $principal = $calendarNode->getOwner(); + } + + /** + * In order for scheduling to work, $addresses must contain the email address of the event organizer. + * + * In Sabre\VObject\ITip\Broker.parseEvent is a conditional whether the + * event organizer is included in $addresses respectively $userHref [1]. + * + * Yes, treat the iTip message as an update from the event organizer + * and deliver it to the other attendees [2]. + * + * No, treat the iTip message as an update from an attendee to the event organizer, + * usually a reply to an event invitation [3]. + * + * The annotated return type for Sabre\CalDAV\Calendar.getOwner is string|null [4], + * but getOwner should not return null in our world. + * + * [1]: https://github.com/sabre-io/vobject/blob/ac56915f9b88a99118c0ee7f25d4338798514251/lib/ITip/Broker.php#L249-L260 + * [2]: https://github.com/sabre-io/vobject/blob/ac56915f9b88a99118c0ee7f25d4338798514251/lib/ITip/Broker.php#L437-L445 + * [3]: https://github.com/sabre-io/vobject/blob/ac56915f9b88a99118c0ee7f25d4338798514251/lib/ITip/Broker.php#L607-L616 + * [4]: https://github.com/sabre-io/dav/blob/85b33f7c4b597bdb2a90e6886a48c5723e767062/lib/CalDAV/Calendar.php#L229-L236 + */ + if ($principal === null) { + $addresses = []; + } else { + $addresses = $this->getAddressesForPrincipal($principal); + } + + /** @var VCalendar $oldObj */ + if (!$isNew) { + /** @var \Sabre\CalDAV\CalendarObject $node */ + $node = $this->server->tree->getNodeForPath($request->getPath()); + $oldObj = Reader::read($node->get()); + } else { + $oldObj = null; + } + + /** + * Sabre has several issues with faulty argument type specifications + * in its doc-block comments. Passing null is ok here. + * + * @psalm-suppress PossiblyNullArgument + * @psalm-suppress ArgumentTypeCoercion + */ + $this->processICalendarChange($oldObj, $vCal, $addresses, [], $modified); + + if ($oldObj) { + // Destroy circular references so PHP will GC the object. + $oldObj->destroy(); + } + } + + /** + * This method checks the 'Schedule-Reply' header + * and returns false if it's 'F', otherwise true. + * + * Copied from Sabre/DAV's Schedule plugin, because it's + * private for whatever reason + * + * @param RequestInterface $request + * @return bool + */ + private function scheduleReply(RequestInterface $request) { + $scheduleReply = $request->getHeader('Schedule-Reply'); + return $scheduleReply !== 'F'; } /** diff --git a/apps/dav/tests/unit/CalDAV/Schedule/PluginTest.php b/apps/dav/tests/unit/CalDAV/Schedule/PluginTest.php index 8f315eac0ee52..c13ea7b6cf86b 100644 --- a/apps/dav/tests/unit/CalDAV/Schedule/PluginTest.php +++ b/apps/dav/tests/unit/CalDAV/Schedule/PluginTest.php @@ -36,15 +36,23 @@ use OCP\IL10N; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; +use Sabre\CalDAV\Backend\BackendInterface; use Sabre\DAV\PropFind; use Sabre\DAV\Server; use Sabre\DAV\Tree; use Sabre\DAV\Xml\Property\Href; use Sabre\DAV\Xml\Property\LocalHref; use Sabre\DAVACL\IPrincipal; +use Sabre\HTTP\Request; +use Sabre\HTTP\Response; use Sabre\HTTP\ResponseInterface; +use Sabre\VObject\Component\VCalendar; +use Sabre\VObject\Component\VEvent; +use Sabre\VObject\ITip\Message; use Sabre\VObject\Parameter; use Sabre\VObject\Property\ICalendar\CalAddress; +use Sabre\VObject\Reader; use Sabre\Xml\Service; use Test\TestCase; @@ -408,4 +416,113 @@ public function testPropFindDefaultCalendarUrl(string $principalUri, ?string $ca $result = $propFind->get(Plugin::SCHEDULE_DEFAULT_CALENDAR_URL); $this->assertEquals('/remote.php/dav/'. $calendarHome . '/' . $calendarUri, $result->getHref()); } + + public function testCalendarObjectChangeShared() { + // Calendar + $calendarNode = new Calendar( + $this->createMock(BackendInterface::class), + [ + 'uri' => 'alice-bob_shared_by_alice', + 'principaluri' => 'principals/users/bob', + '{http://owncloud.org/ns}owner-principal' => 'principals/users/alice' + ], + $this->createMock(IL10N::class), + $this->createMock(IConfig::class), + new NullLogger() + ); + + // Tree + $tree = $this->createMock(Tree::class); + $tree->expects($this->once()) + ->method('getNodeForPath') + ->with('calendars/bob/alice-bob_shared_by_alice') + ->willReturn($calendarNode); + + $this->server->tree = $tree; + + // Request + $request = new Request( + 'PUT', + '/remote.php/dav/calendars/bob/alice-bob_shared_by_alice/B0DC78AE-6DD7-47E3-80BE-89F23E6D5383.ics' + ); + $request->setBaseUrl('/remote.php/dav/'); + + $this->server->httpRequest = $request; + + // Server.getProperties + $addresses = new LocalHref([ + 'mailto:bob@mail.localhost', + '/remote.php/dav/principals/users/bob/' + ]); + + $this->server->expects($this->once()) + ->method('getProperties') + ->with('principals/users/bob', ['{urn:ietf:params:xml:ns:caldav}calendar-user-address-set']) + ->willReturn([ + '{urn:ietf:params:xml:ns:caldav}calendar-user-address-set' => $addresses + ]); + + $response = new Response(); + + // VCalendar / VEvent + $data = 'BEGIN:VCALENDAR +CALSCALE:GREGORIAN +VERSION:2.0 +PRODID:-//IDN nextcloud.com//Calendar app 4.5.0-alpha.1//EN +BEGIN:VEVENT +CREATED:20230808T153326Z +DTSTAMP:20230808T164811Z +LAST-MODIFIED:20230808T164811Z +UID:B0DC78AE-6DD7-47E3-80BE-89F23E6D5383 +DTSTART:20330810T150000 +DTEND:20330810T153000 +SUMMARY:Event in shared calendar +ATTENDEE;CN=Jane;CUTYPE=INDIVIDUAL;PARTSTAT=NEEDS-ACTION;ROLE=REQ-PARTICIPANT;RSVP=TRUE;LANGUAGE=en:mailto:jane@mail.localhost +ATTENDEE;CN=John;CUTYPE=INDIVIDUAL;PARTSTAT=NEEDS-ACTION;ROLE=REQ-PARTICIPANT;RSVP=TRUE;LANGUAGE=en:mailto:john@mail.localhost +ORGANIZER;CN=Bob:mailto:bob@mail.localhost +END:VEVENT +END:VCALENDAR +'; + + /** @var VCalendar $vCal */ + $vCal = Reader::read($data); + + $modified = false; + $isNew = true; + + /** @var Message[] $iTipMessages */ + $iTipMessages = []; + + $this->server->expects($this->exactly(2)) + ->method('emit') + ->willReturnCallback(function (string $eventName, array $arguments = [], callable $continueCallBack = null) use (&$iTipMessages) { + $this->assertEquals('schedule', $eventName); + $this->assertCount(1, $arguments); + $iTipMessages[] = $arguments[0]; + return true; + }); + + $this->plugin->calendarObjectChange( + $request, + $response, + $vCal, + 'calendars/bob/alice-bob_shared_by_alice', + $modified, + $isNew + ); + + /** + * VCalendar contains an event organized by Bob with Jane and John as attendees. + * The expected outcome is that for Jane and John an iTip message is generated. + */ + $this->assertCount(2, $iTipMessages); + + $this->assertEquals('mailto:bob@mail.localhost', $iTipMessages[0]->sender); + $this->assertEquals('mailto:jane@mail.localhost', $iTipMessages[0]->recipient); + $this->assertTrue($iTipMessages[0]->significantChange); + + $this->assertEquals('mailto:bob@mail.localhost', $iTipMessages[1]->sender); + $this->assertEquals('mailto:john@mail.localhost', $iTipMessages[1]->recipient); + $this->assertTrue($iTipMessages[1]->significantChange); + } }