From 8e928ca4969d9aa9d0b987489ba7e05aa73322d5 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 | 79 +++++++++++- .../tests/unit/CalDAV/Schedule/PluginTest.php | 117 ++++++++++++++++++ 3 files changed, 238 insertions(+), 13 deletions(-) diff --git a/apps/dav/lib/CalDAV/CalDavBackend.php b/apps/dav/lib/CalDAV/CalDavBackend.php index 1424ee4f9be..9cdd47b6b81 100644 --- a/apps/dav/lib/CalDAV/CalDavBackend.php +++ b/apps/dav/lib/CalDAV/CalDavBackend.php @@ -2244,21 +2244,58 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription * @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 a1297fd2cf1..2579caf6f60 100644 --- a/apps/dav/lib/CalDAV/Schedule/Plugin.php +++ b/apps/dav/lib/CalDAV/Schedule/Plugin.php @@ -159,9 +159,6 @@ class Plugin extends \Sabre\CalDAV\Schedule\Plugin { } /** - * @param RequestInterface $request - * @param ResponseInterface $response - * @param VCalendar $vCal * @param mixed $calendarPath * @param mixed $modified * @param mixed $isNew @@ -173,7 +170,81 @@ class Plugin extends \Sabre\CalDAV\Schedule\Plugin { } try { - 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(); + } } catch (SameOrganizerForAllComponentsException $e) { $this->handleSameOrganizerException($e, $vCal, $calendarPath); } diff --git a/apps/dav/tests/unit/CalDAV/Schedule/PluginTest.php b/apps/dav/tests/unit/CalDAV/Schedule/PluginTest.php index 8f315eac0ee..80179ae8a35 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\IConfig; 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 @@ class PluginTest extends TestCase { $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); + } }