From 4b14ca672ffc4388297a278b87c30ffa2c94561d Mon Sep 17 00:00:00 2001 From: =?utf8?q?Thomas=20M=C3=BCller?= Date: Thu, 28 Jan 2016 20:30:40 +0100 Subject: [PATCH] Fix ACLs on shared calendars --- apps/dav/lib/caldav/caldavbackend.php | 2 - apps/dav/lib/caldav/calendar.php | 17 --- apps/dav/lib/dav/sharing/backend.php | 2 +- .../tests/unit/caldav/caldavbackendtest.php | 107 ++++++++++++++++-- 4 files changed, 98 insertions(+), 30 deletions(-) diff --git a/apps/dav/lib/caldav/caldavbackend.php b/apps/dav/lib/caldav/caldavbackend.php index 1bd1000a731..4820116e9a6 100644 --- a/apps/dav/lib/caldav/caldavbackend.php +++ b/apps/dav/lib/caldav/caldavbackend.php @@ -188,7 +188,6 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription $fields[] = 'a.components'; $fields[] = 'a.principaluri'; $fields[] = 'a.transparent'; - $fields[] = 's.access'; $query = $this->db->getQueryBuilder(); $result = $query->select($fields) ->from('dav_shares', 's') @@ -216,7 +215,6 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription '{' . Plugin::NS_CALDAV . '}supported-calendar-component-set' => new SupportedCalendarComponentSet($components), '{' . Plugin::NS_CALDAV . '}schedule-calendar-transp' => new ScheduleCalendarTransp($row['transparent']?'transparent':'opaque'), '{' . \OCA\DAV\DAV\Sharing\Plugin::NS_OWNCLOUD . '}owner-principal' => $row['principaluri'], - '{' . \OCA\DAV\DAV\Sharing\Plugin::NS_OWNCLOUD . '}read-only' => $row['access'] === Backend::ACCESS_READ, ]; foreach($this->propertyMap as $xmlName=>$dbName) { diff --git a/apps/dav/lib/caldav/calendar.php b/apps/dav/lib/caldav/calendar.php index b4a47418350..7822c703e91 100644 --- a/apps/dav/lib/caldav/calendar.php +++ b/apps/dav/lib/caldav/calendar.php @@ -58,23 +58,6 @@ class Calendar extends \Sabre\CalDAV\Calendar implements IShareable { function getACL() { $acl = parent::getACL(); - // add the current user - if (isset($this->calendarInfo['{' . \OCA\DAV\DAV\Sharing\Plugin::NS_OWNCLOUD . '}owner-principal'])) { - $owner = $this->calendarInfo['{' . \OCA\DAV\DAV\Sharing\Plugin::NS_OWNCLOUD . '}owner-principal']; - $acl[] = [ - 'privilege' => '{DAV:}read', - 'principal' => $owner, - 'protected' => true, - ]; - if ($this->calendarInfo['{' . \OCA\DAV\DAV\Sharing\Plugin::NS_OWNCLOUD . '}read-only']) { - $acl[] = [ - 'privilege' => '{DAV:}write', - 'principal' => $owner, - 'protected' => true, - ]; - } - } - /** @var CalDavBackend $caldavBackend */ $caldavBackend = $this->caldavBackend; return $caldavBackend->applyShareAcl($this->getResourceId(), $acl); diff --git a/apps/dav/lib/dav/sharing/backend.php b/apps/dav/lib/dav/sharing/backend.php index fee864ffe6f..2d810a43f9d 100644 --- a/apps/dav/lib/dav/sharing/backend.php +++ b/apps/dav/lib/dav/sharing/backend.php @@ -136,7 +136,7 @@ class Backend { 'href' => "principal:${row['principaluri']}", // 'commonName' => isset($p['{DAV:}displayname']) ? $p['{DAV:}displayname'] : '', 'status' => 1, - 'readOnly' => ($row['access'] === self::ACCESS_READ), + 'readOnly' => ($row['access'] == self::ACCESS_READ), '{'.\OCA\DAV\DAV\Sharing\Plugin::NS_OWNCLOUD.'}principal' => $row['principaluri'] ]; } diff --git a/apps/dav/tests/unit/caldav/caldavbackendtest.php b/apps/dav/tests/unit/caldav/caldavbackendtest.php index 30148211a43..aece738166a 100644 --- a/apps/dav/tests/unit/caldav/caldavbackendtest.php +++ b/apps/dav/tests/unit/caldav/caldavbackendtest.php @@ -28,6 +28,7 @@ use OCA\DAV\Connector\Sabre\Principal; use Sabre\CalDAV\Xml\Property\SupportedCalendarComponentSet; use Sabre\DAV\PropPatch; use Sabre\DAV\Xml\Property\Href; +use Sabre\DAVACL\IACL; use Test\TestCase; /** @@ -108,22 +109,80 @@ class CalDavBackendTest extends TestCase { $this->assertEquals(0, count($books)); } - public function testCalendarSharing() { + public function providesSharingData() { + return [ + [true, true, true, false, [ + [ + 'href' => 'principal:' . self::UNIT_TEST_USER1, + 'readOnly' => false + ], + [ + 'href' => 'principal:' . self::UNIT_TEST_GROUP, + 'readOnly' => true + ] + ]], + [true, false, false, false, [ + [ + 'href' => 'principal:' . self::UNIT_TEST_USER1, + 'readOnly' => true + ], + ]], + + ]; + } + + /** + * @dataProvider providesSharingData + */ + public function testCalendarSharing($userCanRead, $userCanWrite, $groupCanRead, $groupCanWrite, $add) { - $this->createTestCalendar(); + $calendarId = $this->createTestCalendar(); $books = $this->backend->getCalendarsForUser(self::UNIT_TEST_USER); $this->assertEquals(1, count($books)); $calendar = new Calendar($this->backend, $books[0]); - $this->backend->updateShares($calendar, [ - [ - 'href' => 'principal:' . self::UNIT_TEST_USER1, - ], - [ - 'href' => 'principal:' . self::UNIT_TEST_GROUP, - ] - ], []); + $this->backend->updateShares($calendar, $add, []); $books = $this->backend->getCalendarsForUser(self::UNIT_TEST_USER1); $this->assertEquals(1, count($books)); + $calendar = new Calendar($this->backend, $books[0]); + $acl = $calendar->getACL(); + $this->assertAcl(self::UNIT_TEST_USER, '{DAV:}read', $acl); + $this->assertAcl(self::UNIT_TEST_USER, '{DAV:}write', $acl); + $this->assertAccess($userCanRead, self::UNIT_TEST_USER1, '{DAV:}read', $acl); + $this->assertAccess($userCanWrite, self::UNIT_TEST_USER1, '{DAV:}write', $acl); + $this->assertAccess($groupCanRead, self::UNIT_TEST_GROUP, '{DAV:}read', $acl); + $this->assertAccess($groupCanWrite, self::UNIT_TEST_GROUP, '{DAV:}write', $acl); + $this->assertEquals(self::UNIT_TEST_USER, $calendar->getOwner()); + + // test acls on the child + $uri = $this->getUniqueID('calobj'); + $calData = <<<'EOD' +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:ownCloud Calendar +BEGIN:VEVENT +CREATED;VALUE=DATE-TIME:20130910T125139Z +UID:47d15e3ec8 +LAST-MODIFIED;VALUE=DATE-TIME:20130910T125139Z +DTSTAMP;VALUE=DATE-TIME:20130910T125139Z +SUMMARY:Test Event +DTSTART;VALUE=DATE-TIME:20130912T130000Z +DTEND;VALUE=DATE-TIME:20130912T140000Z +CLASS:PUBLIC +END:VEVENT +END:VCALENDAR +EOD; + + $this->backend->createCalendarObject($calendarId, $uri, $calData); + + /** @var IACL $child */ + $child = $calendar->getChild($uri); + $acl = $child->getACL(); + $this->assertAcl(self::UNIT_TEST_USER, '{DAV:}read', $acl); + $this->assertAcl(self::UNIT_TEST_USER, '{DAV:}write', $acl); + $this->assertAccess($userCanRead, self::UNIT_TEST_USER1, '{DAV:}read', $acl); + $this->assertAccess($userCanWrite, self::UNIT_TEST_USER1, '{DAV:}write', $acl); + $this->assertAccess($groupCanRead, self::UNIT_TEST_GROUP, '{DAV:}read', $acl); + $this->assertAccess($groupCanWrite, self::UNIT_TEST_GROUP, '{DAV:}write', $acl); // delete the address book $this->backend->deleteCalendar($books[0]['id']); @@ -386,4 +445,32 @@ EOD; $sos = $this->backend->getSchedulingObjects(self::UNIT_TEST_USER); $this->assertEquals(0, count($sos)); } + + private function assertAcl($principal, $privilege, $acl) { + foreach($acl as $a) { + if ($a['principal'] === $principal && $a['privilege'] === $privilege) { + $this->assertTrue(true); + return; + } + } + $this->fail("ACL does not contain $principal / $privilege"); + } + + private function assertNotAcl($principal, $privilege, $acl) { + foreach($acl as $a) { + if ($a['principal'] === $principal && $a['privilege'] === $privilege) { + $this->fail("ACL contains $principal / $privilege"); + return; + } + } + $this->assertTrue(true); + } + + private function assertAccess($shouldHaveAcl, $principal, $privilege, $acl) { + if ($shouldHaveAcl) { + $this->assertAcl($principal, $privilege, $acl); + } else { + $this->assertNotAcl($principal, $privilege, $acl); + } + } } -- 2.39.5