Merge pull request #9268 from nextcloud/bugfix/6711

Do not load calendar/addressbook plugins if not needed

apps/dav/appinfo/info.xml

@@ -5,7 +5,7 @@
 	<name>WebDAV</name>
 	<summary>WebDAV endpoint</summary>
 	<description>WebDAV endpoint</description>
-	<version>1.5.2</version>
+	<version>1.5.3</version>
 	<licence>agpl</licence>
 	<author>owncloud.org</author>
 	<namespace>DAV</namespace>
@@ -36,6 +36,7 @@
 		<command>OCA\DAV\Command\CreateCalendar</command>
 		<command>OCA\DAV\Command\SyncBirthdayCalendar</command>
 		<command>OCA\DAV\Command\SyncSystemAddressBook</command>
+		<command>OCA\DAV\Command\RemoveInvalidShares</command>
 	</commands>
 
 	<settings>

apps/dav/composer/composer/autoload_classmap.php

@@ -65,6 +65,7 @@ return array(
     'OCA\\DAV\\CardDAV\\Xml\\Groups' => $baseDir . '/../lib/CardDAV/Xml/Groups.php',
     'OCA\\DAV\\Command\\CreateAddressBook' => $baseDir . '/../lib/Command/CreateAddressBook.php',
     'OCA\\DAV\\Command\\CreateCalendar' => $baseDir . '/../lib/Command/CreateCalendar.php',
+    'OCA\\DAV\\Command\\RemoveInvalidShares' => $baseDir . '/../lib/Command/RemoveInvalidShares.php',
     'OCA\\DAV\\Command\\SyncBirthdayCalendar' => $baseDir . '/../lib/Command/SyncBirthdayCalendar.php',
     'OCA\\DAV\\Command\\SyncSystemAddressBook' => $baseDir . '/../lib/Command/SyncSystemAddressBook.php',
     'OCA\\DAV\\Comments\\CommentNode' => $baseDir . '/../lib/Comments/CommentNode.php',

apps/dav/composer/composer/autoload_static.php

@@ -80,6 +80,7 @@ class ComposerStaticInitDAV
         'OCA\\DAV\\CardDAV\\Xml\\Groups' => __DIR__ . '/..' . '/../lib/CardDAV/Xml/Groups.php',
         'OCA\\DAV\\Command\\CreateAddressBook' => __DIR__ . '/..' . '/../lib/Command/CreateAddressBook.php',
         'OCA\\DAV\\Command\\CreateCalendar' => __DIR__ . '/..' . '/../lib/Command/CreateCalendar.php',
+        'OCA\\DAV\\Command\\RemoveInvalidShares' => __DIR__ . '/..' . '/../lib/Command/RemoveInvalidShares.php',
         'OCA\\DAV\\Command\\SyncBirthdayCalendar' => __DIR__ . '/..' . '/../lib/Command/SyncBirthdayCalendar.php',
         'OCA\\DAV\\Command\\SyncSystemAddressBook' => __DIR__ . '/..' . '/../lib/Command/SyncSystemAddressBook.php',
         'OCA\\DAV\\Comments\\CommentNode' => __DIR__ . '/..' . '/../lib/Comments/CommentNode.php',

apps/dav/lib/CalDAV/Calendar.php

@@ -203,7 +203,7 @@ class Calendar extends \Sabre\CalDAV\Calendar implements IShareable {
 			}
 
 			$this->caldavBackend->updateShares($this, [], [
-				'href' => $principal
+				$principal
 			]);
 			return;
 		}

apps/dav/lib/CardDAV/AddressBook.php

@@ -181,7 +181,7 @@ class AddressBook extends \Sabre\CardDAV\AddressBook implements IShareable {
 			}
 
 			$this->carddavBackend->updateShares($this, [], [
-				'href' => $principal
+				$principal
 			]);
 			return;
 		}

apps/dav/lib/Command/RemoveInvalidShares.php

@@ -0,0 +1,82 @@
+<?php
+declare(strict_types=1);
+/**
+ * @author Thomas Müller <thomas.mueller@tmit.eu>
+ *
+ * @copyright Copyright (c) 2018, ownCloud GmbH
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * 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, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OCA\DAV\Command;
+
+use OCA\DAV\Connector\Sabre\Principal;
+use OCP\IDBConnection;
+use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+
+/**
+ * Class RemoveInvalidShares - removes shared calendars and addressbook which
+ * have no matching principal. Happened because of a bug in the calendar app.
+ */
+class RemoveInvalidShares extends Command {
+
+	/** @var IDBConnection */
+	private $connection;
+	/** @var Principal */
+	private $principalBackend;
+
+	public function __construct(IDBConnection $connection,
+								Principal $principalBackend) {
+		parent::__construct();
+
+		$this->connection = $connection;
+		$this->principalBackend = $principalBackend;
+	}
+
+	protected function configure() {
+		$this
+			->setName('dav:remove-invalid-shares')
+			->setDescription('Remove invalid dav shares');
+	}
+
+	protected function execute(InputInterface $input, OutputInterface $output) {
+		$query = $this->connection->getQueryBuilder();
+		$result = $query->selectDistinct('principaluri')
+			->from('dav_shares')
+			->execute();
+
+		while($row = $result->fetch()) {
+			$principaluri = $row['principaluri'];
+			$p = $this->principalBackend->getPrincipalByPath($principaluri);
+			if ($p === null) {
+				$this->deleteSharesForPrincipal($principaluri);
+			}
+		}
+
+		$result->closeCursor();
+	}
+
+	/**
+	 * @param string $principaluri
+	 */
+	private function deleteSharesForPrincipal($principaluri) {
+		$delete = $this->connection->getQueryBuilder();
+		$delete->delete('dav_shares')
+			->where($delete->expr()->eq('principaluri', $delete->createNamedParameter($principaluri)));
+		$delete->execute();
+	}
+}

apps/dav/lib/Connector/Sabre/Principal.php

@@ -324,6 +324,13 @@ class Principal implements BackendInterface {
 				return $this->principalPrefix . '/' . $user->getUID();
 			}
 		}
+		if (substr($uri, 0, 10) === 'principal:') {
+			$principal = substr($uri, 10);
+			$principal = $this->getPrincipalByPath($principal);
+			if ($principal !== null) {
+				return $principal['uri'];
+			}
+		}
 
 		return null;
 	}

apps/dav/lib/DAV/Sharing/Backend.php

@@ -67,12 +67,18 @@ class Backend {
 	 * @param string[] $add
 	 * @param string[] $remove
 	 */
-	public function updateShares($shareable, $add, $remove) {
+	public function updateShares(IShareable $shareable, array $add, array $remove) {
 		foreach($add as $element) {
-			$this->shareWith($shareable, $element);
+			$principal = $this->principalBackend->findByUri($element['href'], '');
+			if ($principal !== '') {
+				$this->shareWith($shareable, $element);
+			}
 		}
 		foreach($remove as $element) {
-			$this->unshare($shareable, $element);
+			$principal = $this->principalBackend->findByUri($element, '');
+			if ($principal !== '') {
+				$this->unshare($shareable, $element);
+			}
 		}
 	}
 

apps/dav/lib/Server.php

@@ -71,14 +71,13 @@ class Server {
 	private $baseUri;
 
 	/** @var Connector\Sabre\Server  */
-	private $server;
+	public $server;
 
 	public function __construct(IRequest $request, $baseUri) {
 		$this->request = $request;
 		$this->baseUri = $baseUri;
 		$logger = \OC::$server->getLogger();
 		$dispatcher = \OC::$server->getEventDispatcher();
-		$sendInvitations = \OC::$server->getConfig()->getAppValue('dav', 'sendInvitations', 'yes') === 'yes';
 
 		$root = new RootCollection();
 		$this->server = new \OCA\DAV\Connector\Sabre\Server(new CachingTree($root));
@@ -137,24 +136,29 @@ class Server {
 		$this->server->addPlugin($acl);
 
 		// calendar plugins
-		$this->server->addPlugin(new \OCA\DAV\CalDAV\Plugin());
-		$this->server->addPlugin(new \Sabre\CalDAV\ICSExportPlugin());
-		$this->server->addPlugin(new \OCA\DAV\CalDAV\Schedule\Plugin());
-		if ($sendInvitations) {
-			$this->server->addPlugin(\OC::$server->query(\OCA\DAV\CalDAV\Schedule\IMipPlugin::class));
+		if ($this->requestIsForSubtree(['calendars', 'principals'])) {
+			$this->server->addPlugin(new \OCA\DAV\CalDAV\Plugin());
+			$this->server->addPlugin(new \Sabre\CalDAV\ICSExportPlugin());
+			$this->server->addPlugin(new \OCA\DAV\CalDAV\Schedule\Plugin());
+			if (\OC::$server->getConfig()->getAppValue('dav', 'sendInvitations', 'yes') === 'yes') {
+				$this->server->addPlugin(\OC::$server->query(\OCA\DAV\CalDAV\Schedule\IMipPlugin::class));
+			}
+			$this->server->addPlugin(new \Sabre\CalDAV\Subscriptions\Plugin());
+			$this->server->addPlugin(new \Sabre\CalDAV\Notifications\Plugin());
+			$this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OC::$server->getRequest()));
+			$this->server->addPlugin(new \OCA\DAV\CalDAV\Publishing\PublishPlugin(
+				\OC::$server->getConfig(),
+				\OC::$server->getURLGenerator()
+			));
 		}
-		$this->server->addPlugin(new \Sabre\CalDAV\Subscriptions\Plugin());
-		$this->server->addPlugin(new \Sabre\CalDAV\Notifications\Plugin());
-		$this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OC::$server->getRequest()));
-		$this->server->addPlugin(new \OCA\DAV\CalDAV\Publishing\PublishPlugin(
-			\OC::$server->getConfig(),
-			\OC::$server->getURLGenerator()
-		));
 
 		// addressbook plugins
-		$this->server->addPlugin(new \OCA\DAV\CardDAV\Plugin());
-		$this->server->addPlugin(new VCFExportPlugin());
-		$this->server->addPlugin(new ImageExportPlugin(new PhotoCache(\OC::$server->getAppDataDir('dav-photocache'))));
+		if ($this->requestIsForSubtree(['addressbooks', 'principals'])) {
+			$this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OC::$server->getRequest()));
+			$this->server->addPlugin(new \OCA\DAV\CardDAV\Plugin());
+			$this->server->addPlugin(new VCFExportPlugin());
+			$this->server->addPlugin(new ImageExportPlugin(new PhotoCache(\OC::$server->getAppDataDir('dav-photocache'))));
+		}
 
 		// system tags plugins
 		$this->server->addPlugin(new SystemTagPlugin(
@@ -280,4 +284,14 @@ class Server {
 	public function exec() {
 		$this->server->exec();
 	}
+
+	private function requestIsForSubtree(array $subTrees): bool {
+		foreach ($subTrees as $subTree) {
+			$subTree = trim($subTree, ' /');
+			if (strpos($this->server->getRequestUri(), $subTree.'/') === 0) {
+				return true;
+			}
+		}
+		return false;
+	}
 }

apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php

@@ -30,7 +30,9 @@ use OCA\DAV\Connector\Sabre\Principal;
 use OCP\IGroupManager;
 use OCP\ILogger;
 use OCP\IUserManager;
+use OCP\IUserSession;
 use OCP\Security\ISecureRandom;
+use OCP\Share\IManager as ShareManager;
 use Sabre\CalDAV\Xml\Property\SupportedCalendarComponentSet;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Test\TestCase;
@@ -73,7 +75,12 @@ abstract class AbstractCalDavBackend extends TestCase {
 		$this->groupManager = $this->createMock(IGroupManager::class);
 		$this->dispatcher = $this->createMock(EventDispatcherInterface::class);
 		$this->principal = $this->getMockBuilder(Principal::class)
-			->disableOriginalConstructor()
+			->setConstructorArgs([
+				$this->userManager,
+				$this->groupManager,
+				$this->createMock(ShareManager::class),
+				$this->createMock(IUserSession::class),
+			])
 			->setMethods(['getPrincipalByPath', 'getGroupMembership'])
 			->getMock();
 		$this->principal->expects($this->any())->method('getPrincipalByPath')

apps/dav/tests/unit/CardDAV/CardDavBackendTest.php

@@ -40,6 +40,8 @@ use OCP\IDBConnection;
 use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\IUserManager;
+use OCP\IUserSession;
+use OCP\Share\IManager as ShareManager;
 use Sabre\DAV\PropPatch;
 use Sabre\VObject\Component\VCard;
 use Sabre\VObject\Property\Text;
@@ -90,7 +92,12 @@ class CardDavBackendTest extends TestCase {
 		$this->userManager = $this->createMock(IUserManager::class);
 		$this->groupManager = $this->createMock(IGroupManager::class);
 		$this->principal = $this->getMockBuilder(Principal::class)
-			->disableOriginalConstructor()
+			->setConstructorArgs([
+				$this->userManager,
+				$this->groupManager,
+				$this->createMock(ShareManager::class),
+				$this->createMock(IUserSession::class),
+				])
 			->setMethods(['getPrincipalByPath', 'getGroupMembership'])
 			->getMock();
 		$this->principal->method('getPrincipalByPath')

apps/dav/tests/unit/Command/RemoveInvalidSharesTest.php

@@ -0,0 +1,71 @@
+<?php
+/**
+ * @author Thomas Müller <thomas.mueller@tmit.eu>
+ *
+ * @copyright Copyright (c) 2018, ownCloud GmbH
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * 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, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+
+namespace OCA\DAV\Tests\Unit\Command;
+
+
+use OCA\DAV\Connector\Sabre\Principal;
+use OCA\DAV\Command\RemoveInvalidShares;
+use OCP\Migration\IOutput;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+use Test\TestCase;
+
+/**
+ * Class RemoveInvalidSharesTest
+ *
+ * @package OCA\DAV\Tests\Unit\Repair
+ * @group DB
+ */
+class RemoveInvalidSharesTest extends TestCase {
+
+	public function setUp() {
+		parent::setUp();
+		$db = \OC::$server->getDatabaseConnection();
+
+		$db->insertIfNotExist('*PREFIX*dav_shares', [
+			'principaluri' => 'principal:unknown',
+			'type' => 'calendar',
+			'access' => 2,
+			'resourceid' => 666,
+		]);
+	}
+
+	public function test() {
+		$db = \OC::$server->getDatabaseConnection();
+		/** @var Principal | \PHPUnit_Framework_MockObject_MockObject $principal */
+		$principal = $this->createMock(Principal::class);
+
+		/** @var IOutput | \PHPUnit_Framework_MockObject_MockObject $output */
+		$output = $this->createMock(IOutput::class);
+
+		$repair = new RemoveInvalidShares($db, $principal);
+		$this->invokePrivate($repair, 'run', [$this->createMock(InputInterface::class), $this->createMock(OutputInterface::class)]);
+
+		$query = $db->getQueryBuilder();
+		$result = $query->select('*')->from('dav_shares')
+			->where($query->expr()->eq('principaluri', $query->createNamedParameter('principal:unknown')))->execute();
+		$data = $result->fetchAll();
+		$result->closeCursor();
+		$this->assertEquals(0, count($data));
+	}
+}

apps/dav/tests/unit/ServerTest.php

@@ -38,12 +38,24 @@ use OCA\DAV\AppInfo\PluginManager;
  */
 class ServerTest extends \Test\TestCase {
 
-	public function test() {
-		/** @var IRequest $r */
+	/**
+	 * @dataProvider providesUris
+	 */
+	public function test($uri, array $plugins) {
+		/** @var IRequest | \PHPUnit_Framework_MockObject_MockObject $r */
 		$r = $this->createMock(IRequest::class);
-		$r->method('getRequestUri')
-			->willReturn('/');
+		$r->expects($this->any())->method('getRequestUri')->willReturn($uri);
 		$s = new Server($r, '/');
-		$this->assertInstanceOf('OCA\DAV\Server', $s);
+		$this->assertNotNull($s->server);
+		foreach ($plugins as $plugin) {
+			$this->assertNotNull($s->server->getPlugin($plugin));
+		}
+	}
+	public function providesUris() {
+		return [
+			'principals' => ['principals/users/admin', ['caldav', 'oc-resource-sharing', 'carddav']],
+			'calendars' => ['calendars/admin', ['caldav', 'oc-resource-sharing']],
+			'addressbooks' => ['addressbooks/admin', ['carddav', 'oc-resource-sharing']],
+		];
 	}
 }

lib/private/Repair.php

@@ -47,6 +47,8 @@ use OC\Repair\RepairMimeTypes;
 use OC\Repair\RepairInvalidShares;
 use OC\Template\JSCombiner;
 use OC\Template\SCSSCacher;
+use OCA\DAV\Connector\Sabre\Principal;
+use OCA\DAV\Repair\RemoveInvalidShares;
 use OCP\AppFramework\QueryException;
 use OCP\Migration\IOutput;
 use OCP\Migration\IRepairStep;
@@ -146,7 +148,7 @@ class Repair implements IOutput{
 	 */
 	public static function getExpensiveRepairSteps() {
 		return [
-			new OldGroupMembershipShares(\OC::$server->getDatabaseConnection(), \OC::$server->getGroupManager())
+			new OldGroupMembershipShares(\OC::$server->getDatabaseConnection(), \OC::$server->getGroupManager()),
 		];
 	}