From b887adf386063a8264fbb11d2d4380528e380d01 Mon Sep 17 00:00:00 2001 From: Georg Ehrke Date: Sat, 25 Feb 2017 14:26:02 +0100 Subject: [PATCH] allow sharees to edit certain calendar properties for themselves Signed-off-by: Georg Ehrke --- apps/dav/lib/CalDAV/Calendar.php | 9 +- .../dav/lib/Files/CustomPropertiesBackend.php | 22 +++ apps/dav/tests/unit/CalDAV/CalendarTest.php | 17 +- .../Files/CustomPropertiesBackendTest.php | 170 ++++++++++++++++++ 4 files changed, 205 insertions(+), 13 deletions(-) create mode 100644 apps/dav/tests/unit/Files/CustomPropertiesBackendTest.php diff --git a/apps/dav/lib/CalDAV/Calendar.php b/apps/dav/lib/CalDAV/Calendar.php index ef8e21be83..d5e41c3c8c 100644 --- a/apps/dav/lib/CalDAV/Calendar.php +++ b/apps/dav/lib/CalDAV/Calendar.php @@ -175,12 +175,11 @@ class Calendar extends \Sabre\CalDAV\Calendar implements IShareable { } function propPatch(PropPatch $propPatch) { - $mutations = $propPatch->getMutations(); - // If this is a shared calendar, the user can only change the enabled property, to hide it. - if ($this->isShared() && (sizeof($mutations) !== 1 || !isset($mutations['{http://owncloud.org/ns}calendar-enabled']))) { - throw new Forbidden(); + // parent::propPatch will only update calendars table + // if calendar is shared, changes have to be made to the properties table + if (!$this->isShared()) { + parent::propPatch($propPatch); } - parent::propPatch($propPatch); } function getChild($name) { diff --git a/apps/dav/lib/Files/CustomPropertiesBackend.php b/apps/dav/lib/Files/CustomPropertiesBackend.php index 43c02e7d6e..397c7bb22e 100644 --- a/apps/dav/lib/Files/CustomPropertiesBackend.php +++ b/apps/dav/lib/Files/CustomPropertiesBackend.php @@ -1,9 +1,11 @@ * * @author Robin Appelman * @author Thomas Müller + * @author Georg Ehrke * * @license AGPL-3.0 * @@ -102,6 +104,26 @@ class CustomPropertiesBackend implements BackendInterface { $this->ignoredProperties ); + // substr of calendars/ => path is inside the CalDAV component + // two '/' => this a calendar (no calendar-home nor calendar object) + if (substr($path, 0, 10) === 'calendars/' && substr_count($path, '/') === 2) { + $allRequestedProps = $propFind->getRequestedProperties(); + $customPropertiesForShares = [ + '{DAV:}displayname', + '{urn:ietf:params:xml:ns:caldav}calendar-description', + '{urn:ietf:params:xml:ns:caldav}calendar-timezone', + '{http://apple.com/ns/ical/}calendar-order', + '{http://apple.com/ns/ical/}calendar-color', + '{urn:ietf:params:xml:ns:caldav}schedule-calendar-transp', + ]; + + foreach ($customPropertiesForShares as $customPropertyForShares) { + if (in_array($customPropertyForShares, $allRequestedProps)) { + $requestedProps[] = $customPropertyForShares; + } + } + } + if (empty($requestedProps)) { return; } diff --git a/apps/dav/tests/unit/CalDAV/CalendarTest.php b/apps/dav/tests/unit/CalDAV/CalendarTest.php index 70a072f04d..112b2c598d 100644 --- a/apps/dav/tests/unit/CalDAV/CalendarTest.php +++ b/apps/dav/tests/unit/CalDAV/CalendarTest.php @@ -109,7 +109,7 @@ class CalendarTest extends TestCase { ['user1', 'user2', [], true], ['user1', 'user2', [ '{http://owncloud.org/ns}calendar-enabled' => true, - ], false], + ], true], ['user1', 'user2', [ '{DAV:}displayname' => true, ], true], @@ -134,7 +134,7 @@ class CalendarTest extends TestCase { /** * @dataProvider dataPropPatch */ - public function testPropPatch($ownerPrincipal, $principalUri, $mutations, $throws) { + public function testPropPatch($ownerPrincipal, $principalUri, $mutations, $shared) { /** @var \PHPUnit_Framework_MockObject_MockObject | CalDavBackend $backend */ $backend = $this->getMockBuilder(CalDavBackend::class)->disableOriginalConstructor()->getMock(); $calendarInfo = [ @@ -144,14 +144,15 @@ class CalendarTest extends TestCase { 'uri' => 'default' ]; $c = new Calendar($backend, $calendarInfo, $this->l10n); + $propPatch = new PropPatch($mutations); - if ($throws) { - $this->setExpectedException('\Sabre\DAV\Exception\Forbidden'); - } - $c->propPatch(new PropPatch($mutations)); - if (!$throws) { - $this->assertTrue(true); + if (!$shared) { + $backend->expects($this->once()) + ->method('updateCalendar') + ->with(666, $propPatch); } + $c->propPatch($propPatch); + $this->assertTrue(true); } /** diff --git a/apps/dav/tests/unit/Files/CustomPropertiesBackendTest.php b/apps/dav/tests/unit/Files/CustomPropertiesBackendTest.php new file mode 100644 index 0000000000..4f3e22aea6 --- /dev/null +++ b/apps/dav/tests/unit/Files/CustomPropertiesBackendTest.php @@ -0,0 +1,170 @@ + + * + * @author Georg Ehrke + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ +namespace OCA\DAV\Tests\Files; + +use OCA\DAV\Files\CustomPropertiesBackend; +use OCP\IDBConnection; +use OCP\IUser; +use Sabre\DAV\PropFind; +use Sabre\DAV\PropPatch; +use Sabre\DAV\Tree; +use Test\TestCase; + +class CustomPropertiesBackendTest extends TestCase { + + /** @var Tree | \PHPUnit_Framework_MockObject_MockObject */ + private $tree; + + /** @var IDBConnection | \PHPUnit_Framework_MockObject_MockObject */ + private $dbConnection; + + /** @var IUser | \PHPUnit_Framework_MockObject_MockObject */ + private $user; + + /** @var CustomPropertiesBackend | \PHPUnit_Framework_MockObject_MockObject */ + private $backend; + + public function setUp() { + parent::setUp(); + + $this->tree = $this->createMock(Tree::class); + $this->dbConnection = $this->createMock(IDBConnection::class); + $this->user = $this->createMock(IUser::class); + $this->user->expects($this->once()) + ->method('getUID') + ->with() + ->will($this->returnValue('dummy_user_42')); + + $this->backend = new CustomPropertiesBackend($this->tree, + $this->dbConnection, $this->user); + } + + public function testPropFindNoDbCalls() { + $propFind = $this->createMock(PropFind::class); + $propFind->expects($this->at(0)) + ->method('get404Properties') + ->with() + ->will($this->returnValue([ + '{http://owncloud.org/ns}permissions', + '{http://owncloud.org/ns}downloadURL', + '{http://owncloud.org/ns}dDC', + '{http://owncloud.org/ns}size', + ])); + + $this->dbConnection->expects($this->never()) + ->method($this->anything()); + + $this->backend->propFind('foo_bar_path_1337_0', $propFind); + } + + public function testPropFindCalendarCall() { + $propFind = $this->createMock(PropFind::class); + $propFind->expects($this->at(0)) + ->method('get404Properties') + ->with() + ->will($this->returnValue([ + '{DAV:}getcontentlength', + '{DAV:}getcontenttype', + '{DAV:}getetag', + '{abc}def' + ])); + + $propFind->expects($this->at(1)) + ->method('getRequestedProperties') + ->with() + ->will($this->returnValue([ + '{DAV:}getcontentlength', + '{DAV:}getcontenttype', + '{DAV:}getetag', + '{DAV:}displayname', + '{urn:ietf:params:xml:ns:caldav}calendar-description', + '{urn:ietf:params:xml:ns:caldav}calendar-timezone', + '{abc}def' + ])); + + $statement = $this->createMock('\Doctrine\DBAL\Driver\Statement'); + $this->dbConnection->expects($this->once()) + ->method('executeQuery') + ->with('SELECT * FROM `*PREFIX*properties` WHERE `userid` = ? AND `propertypath` = ? AND `propertyname` in (?)', + ['dummy_user_42', 'calendars/foo/bar_path_1337_0', [ + 3 => '{abc}def', + 4 => '{DAV:}displayname', + 5 => '{urn:ietf:params:xml:ns:caldav}calendar-description', + 6 => '{urn:ietf:params:xml:ns:caldav}calendar-timezone']], + [null, null, 102]) + ->will($this->returnValue($statement)); + + $this->backend->propFind('calendars/foo/bar_path_1337_0', $propFind); + } + + /** + * @dataProvider propPatchProvider + */ + public function testPropPatch($path, $propPatch) { + $propPatch->expects($this->once()) + ->method('handleRemaining'); + + $this->backend->propPatch($path, $propPatch); + } + + public function propPatchProvider() { + $propPatchMock = $this->createMock(PropPatch::class); + return [ + ['foo_bar_path_1337', $propPatchMock], + ]; + } + + public function testDelete() { + $statement = $this->createMock('\Doctrine\DBAL\Driver\Statement'); + $statement->expects($this->at(0)) + ->method('execute') + ->with(['dummy_user_42', 'foo_bar_path_1337']); + $statement->expects($this->at(1)) + ->method('closeCursor') + ->with(); + + $this->dbConnection->expects($this->at(0)) + ->method('prepare') + ->with('DELETE FROM `*PREFIX*properties` WHERE `userid` = ? AND `propertypath` = ?') + ->will($this->returnValue($statement)); + + $this->backend->delete('foo_bar_path_1337'); + } + + public function testMove() { + $statement = $this->createMock('\Doctrine\DBAL\Driver\Statement'); + $statement->expects($this->at(0)) + ->method('execute') + ->with(['bar_foo_path_7331', 'dummy_user_42', 'foo_bar_path_1337']); + $statement->expects($this->at(1)) + ->method('closeCursor') + ->with(); + + $this->dbConnection->expects($this->at(0)) + ->method('prepare') + ->with('UPDATE `*PREFIX*properties` SET `propertypath` = ? WHERE `userid` = ? AND `propertypath` = ?') + ->will($this->returnValue($statement)); + + $this->backend->move('foo_bar_path_1337', 'bar_foo_path_7331'); + } +}