Restrict share handling to the owner only

Otherwise group members can remove the share for the complete group,
remove edit permissions and even single user shares for other users.

Signed-off-by: Joas Schilling <coding@schilljs.com>

3 files changed, 34 insertions, 8 deletions

diff --git a/apps/dav/lib/CalDAV/Calendar.php b/apps/dav/lib/CalDAV/Calendar.php
index d1eff1aeaa3..05c7e635391 100644
--- a/apps/dav/lib/CalDAV/Calendar.php
+++ b/apps/dav/lib/CalDAV/Calendar.php
@@ -61,8 +61,12 @@ class Calendar extends \Sabre\CalDAV\Calendar implements IShareable {
 	 * @param array $add
 	 * @param array $remove
 	 * @return void
+	 * @throws Forbidden
 	 */
-	function updateShares(array $add, array $remove) {
+	public function updateShares(array $add, array $remove) {
+		if ($this->isShared()) {
+			throw new Forbidden();
+		}
 		/** @var CalDavBackend $calDavBackend */
 		$calDavBackend = $this->caldavBackend;
 		$calDavBackend->updateShares($this, $add, $remove);
@@ -80,7 +84,10 @@ class Calendar extends \Sabre\CalDAV\Calendar implements IShareable {
 	 *
 	 * @return array
 	 */
-	function getShares() {
+	public function getShares() {
+		if ($this->isShared()) {
+			return [];
+		}
 		/** @var CalDavBackend $calDavBackend */
 		$calDavBackend = $this->caldavBackend;
 		return $calDavBackend->getShares($this->getResourceId());
@@ -136,6 +143,10 @@ class Calendar extends \Sabre\CalDAV\Calendar implements IShareable {
 			];
 		}
 
+		if ($this->isShared()) {
+			return $acl;
+		}
+
 		/** @var CalDavBackend $calDavBackend */
 		$calDavBackend = $this->caldavBackend;
 		return $calDavBackend->applyShareAcl($this->getResourceId(), $acl);
@@ -156,7 +167,7 @@ class Calendar extends \Sabre\CalDAV\Calendar implements IShareable {
 		if (isset($this->calendarInfo['{http://owncloud.org/ns}owner-principal']) &&
 			$this->calendarInfo['{http://owncloud.org/ns}owner-principal'] !== $this->calendarInfo['principaluri']) {
 			$principal = 'principal:' . parent::getOwner();
-			$shares = $this->getShares();
+			$shares = $this->caldavBackend->getShares($this->getResourceId());
 			$shares = array_filter($shares, function($share) use ($principal){
 				return $share['href'] === $principal;
 			});
diff --git a/apps/dav/lib/CardDAV/AddressBook.php b/apps/dav/lib/CardDAV/AddressBook.php
index 1c13ac00aec..84448c5459e 100644
--- a/apps/dav/lib/CardDAV/AddressBook.php
+++ b/apps/dav/lib/CardDAV/AddressBook.php
@@ -64,8 +64,12 @@ class AddressBook extends \Sabre\CardDAV\AddressBook implements IShareable {
 	 * @param array $add
 	 * @param array $remove
 	 * @return void
+	 * @throws Forbidden
 	 */
 	function updateShares(array $add, array $remove) {
+		if ($this->isShared()) {
+			throw new Forbidden();
+		}
 		/** @var CardDavBackend $carddavBackend */
 		$carddavBackend = $this->carddavBackend;
 		$carddavBackend->updateShares($this, $add, $remove);
@@ -84,6 +88,9 @@ class AddressBook extends \Sabre\CardDAV\AddressBook implements IShareable {
 	 * @return array
 	 */
 	function getShares() {
+		if ($this->isShared()) {
+			return [];
+		}
 		/** @var CardDavBackend $carddavBackend */
 		$carddavBackend = $this->carddavBackend;
 		return $carddavBackend->getShares($this->getResourceId());
@@ -123,6 +130,10 @@ class AddressBook extends \Sabre\CardDAV\AddressBook implements IShareable {
 			];
 		}
 
+		if ($this->isShared()) {
+			return $acl;
+		}
+
 		/** @var CardDavBackend $carddavBackend */
 		$carddavBackend = $this->carddavBackend;
 		return $carddavBackend->applyShareAcl($this->getResourceId(), $acl);
@@ -160,7 +171,7 @@ class AddressBook extends \Sabre\CardDAV\AddressBook implements IShareable {
 	function delete() {
 		if (isset($this->addressBookInfo['{http://owncloud.org/ns}owner-principal'])) {
 			$principal = 'principal:' . parent::getOwner();
-			$shares = $this->getShares();
+			$shares = $this->carddavBackend->getShares($this->getResourceId());
 			$shares = array_filter($shares, function($share) use ($principal){
 				return $share['href'] === $principal;
 			});
@@ -192,6 +203,14 @@ class AddressBook extends \Sabre\CardDAV\AddressBook implements IShareable {
 		return $cardDavBackend->collectCardProperties($this->getResourceId(), 'CATEGORIES');
 	}
 
+	private function isShared() {
+		if (!isset($this->addressBookInfo['{http://owncloud.org/ns}owner-principal'])) {
+			return false;
+		}
+
+		return $this->addressBookInfo['{http://owncloud.org/ns}owner-principal'] !== $this->addressBookInfo['principaluri'];
+	}
+
 	private function canWrite() {
 		if (isset($this->addressBookInfo['{http://owncloud.org/ns}read-only'])) {
 			return !$this->addressBookInfo['{http://owncloud.org/ns}read-only'];
diff --git a/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php b/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php
index 22ef232dac4..63ca03b0d3d 100644
--- a/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php
+++ b/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php
@@ -143,8 +143,6 @@ class CalDavBackendTest extends AbstractCalDavBackendTest {
 		$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
@@ -178,8 +176,6 @@ EOD;
 		$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->dispatcher->expects($this->at(0))