Merge pull request #3620 from nextcloud/feature/1463/editable_color_name_for_shared_calendars

allow sharees to edit certain calendar properties for themselves

5 files changed, 207 insertions, 15 deletions

diff --git a/apps/dav/lib/CalDAV/Calendar.php b/apps/dav/lib/CalDAV/Calendar.php
index ef8e21be83a..d5e41c3c8c5 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/DAV/CustomPropertiesBackend.php
index 43c02e7d6eb..ee0d0407015 100644
--- a/apps/dav/lib/Files/CustomPropertiesBackend.php
+++ b/apps/dav/lib/DAV/CustomPropertiesBackend.php
@@ -1,9 +1,11 @@
 <?php
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
+ * @copyright Copyright (c) 2017, Georg Ehrke <oc.list@georgehrke.com>
  *
  * @author Robin Appelman <robin@icewind.nl>
  * @author Thomas Müller <thomas.mueller@tmit.eu>
+ * @author Georg Ehrke <oc.list@georgehrke.com>
  *
  * @license AGPL-3.0
  *
@@ -21,7 +23,7 @@
  *
  */
 
-namespace OCA\DAV\Files;
+namespace OCA\DAV\DAV;
 
 use OCP\IDBConnection;
 use OCP\IUser;
@@ -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/lib/Server.php b/apps/dav/lib/Server.php
index 031bc1d3d81..a5c1379eb7e 100644
--- a/apps/dav/lib/Server.php
+++ b/apps/dav/lib/Server.php
@@ -42,9 +42,9 @@ use OCA\DAV\Connector\Sabre\FilesPlugin;
 use OCA\DAV\Connector\Sabre\FilesReportPlugin;
 use OCA\DAV\Connector\Sabre\SharesPlugin;
 use OCA\DAV\DAV\PublicAuth;
+use OCA\DAV\DAV\CustomPropertiesBackend;
 use OCA\DAV\Connector\Sabre\QuotaPlugin;
 use OCA\DAV\Files\BrowserErrorPagePlugin;
-use OCA\DAV\Files\CustomPropertiesBackend;
 use OCA\DAV\SystemTag\SystemTagPlugin;
 use OCP\IRequest;
 use OCP\SabrePluginEvent;
diff --git a/apps/dav/tests/unit/CalDAV/CalendarTest.php b/apps/dav/tests/unit/CalDAV/CalendarTest.php
index 70a072f04de..112b2c598df 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/DAV/CustomPropertiesBackendTest.php b/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php
new file mode 100644
index 00000000000..780324abaa6
--- /dev/null
+++ b/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php
@@ -0,0 +1,170 @@
+<?php
+/**
+ * @copyright Copyright (c) 2017, Georg Ehrke <oc.list@georgehrke.com>
+ *
+ * @author Georg Ehrke <oc.list@georgehrke.com>
+ *
+ * @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 <http://www.gnu.org/licenses/>.
+ *
+ */
+namespace OCA\DAV\Tests\DAV;
+
+use OCA\DAV\DAV\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');
+	}
+}