Check if the user/group exists for dav shares

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

apps/dav/appinfo/v1/caldav.php

@@ -49,7 +49,7 @@ $db = \OC::$server->getDatabaseConnection();
 $userManager = \OC::$server->getUserManager();
 $random = \OC::$server->getSecureRandom();
 $dispatcher = \OC::$server->getEventDispatcher();
-$calDavBackend = new CalDavBackend($db, $principalBackend, $userManager, $random, $dispatcher, true);
+$calDavBackend = new CalDavBackend($db, $principalBackend, $userManager, \OC::$server->getGroupManager(), $random, $dispatcher, true);
 
 $debugging = \OC::$server->getConfig()->getSystemValue('debug', false);
 

apps/dav/appinfo/v1/carddav.php

@@ -48,7 +48,7 @@ $principalBackend = new Principal(
 	'principals/'
 );
 $db = \OC::$server->getDatabaseConnection();
-$cardDavBackend = new CardDavBackend($db, $principalBackend, \OC::$server->getUserManager(), \OC::$server->getEventDispatcher());
+$cardDavBackend = new CardDavBackend($db, $principalBackend, \OC::$server->getUserManager(), \OC::$server->getGroupManager(), \OC::$server->getEventDispatcher());
 
 $debugging = \OC::$server->getConfig()->getSystemValue('debug', false);
 

apps/dav/lib/CalDAV/CalDavBackend.php

@@ -32,6 +32,7 @@ use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCA\DAV\Connector\Sabre\Principal;
 use OCA\DAV\DAV\Sharing\Backend;
 use OCP\IDBConnection;
+use OCP\IGroupManager;
 use OCP\IUser;
 use OCP\IUserManager;
 use OCP\Security\ISecureRandom;
@@ -158,6 +159,7 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
 	 * @param IDBConnection $db
 	 * @param Principal $principalBackend
 	 * @param IUserManager $userManager
+	 * @param IGroupManager $groupManager
 	 * @param ISecureRandom $random
 	 * @param EventDispatcherInterface $dispatcher
 	 * @param bool $legacyEndpoint
@@ -165,13 +167,14 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
 	public function __construct(IDBConnection $db,
 								Principal $principalBackend,
 								IUserManager $userManager,
+								IGroupManager $groupManager,
 								ISecureRandom $random,
 								EventDispatcherInterface $dispatcher,
 								$legacyEndpoint = false) {
 		$this->db = $db;
 		$this->principalBackend = $principalBackend;
 		$this->userManager = $userManager;
-		$this->sharingBackend = new Backend($this->db, $principalBackend, 'calendar');
+		$this->sharingBackend = new Backend($this->db, $this->userManager, $groupManager, $principalBackend, 'calendar');
 		$this->random = $random;
 		$this->dispatcher = $dispatcher;
 		$this->legacyEndpoint = $legacyEndpoint;

apps/dav/lib/CardDAV/CardDavBackend.php

@@ -33,6 +33,7 @@ use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCA\DAV\DAV\Sharing\Backend;
 use OCA\DAV\DAV\Sharing\IShareable;
 use OCP\IDBConnection;
+use OCP\IGroupManager;
 use OCP\IUser;
 use OCP\IUserManager;
 use PDO;
@@ -88,17 +89,19 @@ class CardDavBackend implements BackendInterface, SyncSupport {
 	 * @param IDBConnection $db
 	 * @param Principal $principalBackend
 	 * @param IUserManager $userManager
+	 * @param IGroupManager $groupManager
 	 * @param EventDispatcherInterface $dispatcher
 	 */
 	public function __construct(IDBConnection $db,
 								Principal $principalBackend,
 								IUserManager $userManager,
+								IGroupManager $groupManager,
 								EventDispatcherInterface $dispatcher) {
 		$this->db = $db;
 		$this->principalBackend = $principalBackend;
 		$this->userManager = $userManager;
 		$this->dispatcher = $dispatcher;
-		$this->sharingBackend = new Backend($this->db, $principalBackend, 'addressbook');
+		$this->sharingBackend = new Backend($this->db, $this->userManager, $groupManager, $principalBackend, 'addressbook');
 	}
 
 	/**

apps/dav/lib/Command/CreateCalendar.php

@@ -79,7 +79,7 @@ class CreateCalendar extends Command {
 		$dispatcher = \OC::$server->getEventDispatcher();
 
 		$name = $input->getArgument('name');
-		$caldav = new CalDavBackend($this->dbConnection, $principalBackend, $this->userManager, $random, $dispatcher);
+		$caldav = new CalDavBackend($this->dbConnection, $principalBackend, $this->userManager, $this->groupManager, $random, $dispatcher);
 		$caldav->createCalendar("principals/users/$user", $name, []);
 	}
 }

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

@@ -26,11 +26,17 @@ namespace OCA\DAV\DAV\Sharing;
 
 use OCA\DAV\Connector\Sabre\Principal;
 use OCP\IDBConnection;
+use OCP\IGroupManager;
+use OCP\IUserManager;
 
 class Backend {
 
 	/** @var IDBConnection */
 	private $db;
+	/** @var IUserManager */
+	private $userManager;
+	/** @var IGroupManager */
+	private $groupManager;
 	/** @var Principal */
 	private $principalBackend;
 	/** @var string */
@@ -42,11 +48,15 @@ class Backend {
 
 	/**
 	 * @param IDBConnection $db
+	 * @param IUserManager $userManager
+	 * @param IGroupManager $groupManager
 	 * @param Principal $principalBackend
 	 * @param string $resourceType
 	 */
-	public function __construct(IDBConnection $db, Principal $principalBackend, $resourceType) {
+	public function __construct(IDBConnection $db, IUserManager $userManager, IGroupManager $groupManager, Principal $principalBackend, $resourceType) {
 		$this->db = $db;
+		$this->userManager = $userManager;
+		$this->groupManager = $groupManager;
 		$this->principalBackend = $principalBackend;
 		$this->resourceType = $resourceType;
 	}
@@ -81,6 +91,18 @@ class Backend {
 			return;
 		}
 
+		$principal = explode('/', $parts[1], 3);
+		if (count($principal) !== 3 || $principal[0] !== 'principals' || !in_array($principal[1], ['users', 'groups'], true)) {
+			// Invalid principal
+			return;
+		}
+
+		if (($principal[1] === 'users' && !$this->userManager->userExists($principal[2])) ||
+			($principal[1] === 'groups' && !$this->groupManager->groupExists($principal[2]))) {
+			// User or group does not exist
+			return;
+		}
+
 		// remove the share if it already exists
 		$this->unshare($shareable, $element['href']);
 		$access = self::ACCESS_READ;

apps/dav/lib/RootCollection.php

@@ -41,15 +41,14 @@ class RootCollection extends SimpleCollection {
 		$config = \OC::$server->getConfig();
 		$random = \OC::$server->getSecureRandom();
 		$userManager = \OC::$server->getUserManager();
+		$groupManager = \OC::$server->getGroupManager();
 		$db = \OC::$server->getDatabaseConnection();
 		$dispatcher = \OC::$server->getEventDispatcher();
 		$userPrincipalBackend = new Principal(
 			$userManager,
-			\OC::$server->getGroupManager()
-		);
-		$groupPrincipalBackend = new GroupPrincipalBackend(
-			\OC::$server->getGroupManager()
+			$groupManager
 		);
+		$groupPrincipalBackend = new GroupPrincipalBackend($groupManager);
 		// as soon as debug mode is enabled we allow listing of principals
 		$disableListing = !$config->getSystemValue('debug', false);
 
@@ -62,7 +61,7 @@ class RootCollection extends SimpleCollection {
 		$systemPrincipals->disableListing = $disableListing;
 		$filesCollection = new Files\RootCollection($userPrincipalBackend, 'principals/users');
 		$filesCollection->disableListing = $disableListing;
-		$caldavBackend = new CalDavBackend($db, $userPrincipalBackend, $userManager, $random, $dispatcher);
+		$caldavBackend = new CalDavBackend($db, $userPrincipalBackend, $userManager, $groupManager, $random, $dispatcher);
 		$calendarRoot = new CalendarRoot($userPrincipalBackend, $caldavBackend, 'principals/users');
 		$calendarRoot->disableListing = $disableListing;
 		$publicCalendarRoot = new PublicCalendarRoot($caldavBackend);
@@ -71,28 +70,28 @@ class RootCollection extends SimpleCollection {
 		$systemTagCollection = new SystemTag\SystemTagsByIdCollection(
 			\OC::$server->getSystemTagManager(),
 			\OC::$server->getUserSession(),
-			\OC::$server->getGroupManager()
+			$groupManager
 		);
 		$systemTagRelationsCollection = new SystemTag\SystemTagsRelationsCollection(
 			\OC::$server->getSystemTagManager(),
 			\OC::$server->getSystemTagObjectMapper(),
 			\OC::$server->getUserSession(),
-			\OC::$server->getGroupManager(),
+			$groupManager,
 			\OC::$server->getEventDispatcher()
 		);
 		$commentsCollection = new Comments\RootCollection(
 			\OC::$server->getCommentsManager(),
-			\OC::$server->getUserManager(),
+			$userManager,
 			\OC::$server->getUserSession(),
 			\OC::$server->getEventDispatcher(),
 			\OC::$server->getLogger()
 		);
 
-		$usersCardDavBackend = new CardDavBackend($db, $userPrincipalBackend, \OC::$server->getUserManager(), $dispatcher);
+		$usersCardDavBackend = new CardDavBackend($db, $userPrincipalBackend, $userManager, $groupManager, $dispatcher);
 		$usersAddressBookRoot = new AddressBookRoot($userPrincipalBackend, $usersCardDavBackend, 'principals/users');
 		$usersAddressBookRoot->disableListing = $disableListing;
 
-		$systemCardDavBackend = new CardDavBackend($db, $userPrincipalBackend, \OC::$server->getUserManager(), $dispatcher);
+		$systemCardDavBackend = new CardDavBackend($db, $userPrincipalBackend, $userManager, $groupManager, $dispatcher);
 		$systemAddressBookRoot = new AddressBookRoot(new SystemPrincipalBackend(), $systemCardDavBackend, 'principals/system');
 		$systemAddressBookRoot->disableListing = $disableListing;
 

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

@@ -24,6 +24,7 @@ namespace OCA\DAV\Tests\unit\CalDAV;
 
 use OCA\DAV\CalDAV\CalDavBackend;
 use OCA\DAV\Connector\Sabre\Principal;
+use OCP\IGroupManager;
 use OCP\IUserManager;
 use OCP\Security\ISecureRandom;
 use Sabre\CalDAV\Xml\Property\SupportedCalendarComponentSet;
@@ -37,7 +38,7 @@ use Test\TestCase;
  *
  * @package OCA\DAV\Tests\unit\CalDAV
  */
-abstract class AbstractCalDavBackendTest extends TestCase {
+abstract class AbstractCalDavBackend extends TestCase {
 
 	/** @var CalDavBackend */
 	protected $backend;
@@ -46,6 +47,8 @@ abstract class AbstractCalDavBackendTest extends TestCase {
 	protected $principal;
 	/** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */
 	protected $userManager;
+	/** @var IGroupManager|\PHPUnit_Framework_MockObject_MockObject */
+	protected $groupManager;
 	/** @var EventDispatcherInterface|\PHPUnit_Framework_MockObject_MockObject */
 	protected $dispatcher;
 
@@ -61,6 +64,7 @@ abstract class AbstractCalDavBackendTest extends TestCase {
 		parent::setUp();
 
 		$this->userManager = $this->createMock(IUserManager::class);
+		$this->groupManager = $this->createMock(IGroupManager::class);
 		$this->dispatcher = $this->createMock(EventDispatcherInterface::class);
 		$this->principal = $this->getMockBuilder('OCA\DAV\Connector\Sabre\Principal')
 			->disableOriginalConstructor()
@@ -77,7 +81,7 @@ abstract class AbstractCalDavBackendTest extends TestCase {
 
 		$db = \OC::$server->getDatabaseConnection();
 		$this->random = \OC::$server->getSecureRandom();
-		$this->backend = new CalDavBackend($db, $this->principal, $this->userManager, $this->random, $this->dispatcher);
+		$this->backend = new CalDavBackend($db, $this->principal, $this->userManager, $this->groupManager, $this->random, $this->dispatcher);
 
 		$this->cleanUpBackend();
 	}

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

@@ -42,7 +42,7 @@ use Sabre\DAVACL\IACL;
  *
  * @package OCA\DAV\Tests\unit\CalDAV
  */
-class CalDavBackendTest extends AbstractCalDavBackendTest {
+class CalDavBackendTest extends AbstractCalDavBackend {
 
 	public function testCalendarOperations() {
 
@@ -130,6 +130,14 @@ class CalDavBackendTest extends AbstractCalDavBackendTest {
 				return vsprintf($text, $parameters);
 			}));
 
+		$this->userManager->expects($this->any())
+			->method('userExists')
+			->willReturn(true);
+
+		$this->userManager->expects($this->any())
+			->method('groupExists')
+			->willReturn(true);
+
 		$calendarId = $this->createTestCalendar();
 		$calendars = $this->backend->getCalendarsForUser(self::UNIT_TEST_USER);
 		$this->assertCount(1, $calendars);

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

@@ -5,6 +5,7 @@ namespace OCA\DAV\Tests\unit\CalDAV;
 use OCA\DAV\CalDAV\Calendar;
 use OCA\DAV\CalDAV\PublicCalendar;
 use OCA\DAV\Connector\Sabre\Principal;
+use OCP\IGroupManager;
 use OCP\IL10N;
 use OCA\DAV\CalDAV\CalDavBackend;
 use OCA\DAV\CalDAV\PublicCalendarRoot;
@@ -33,6 +34,8 @@ class PublicCalendarRootTest extends TestCase {
 	private $principal;
 	/** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */
 	protected $userManager;
+	/** @var IGroupManager|\PHPUnit_Framework_MockObject_MockObject */
+	protected $groupManager;
 
 	/** @var ISecureRandom */
 	private $random;
@@ -43,6 +46,7 @@ class PublicCalendarRootTest extends TestCase {
 		$db = \OC::$server->getDatabaseConnection();
 		$this->principal = $this->createMock('OCA\DAV\Connector\Sabre\Principal');
 		$this->userManager = $this->createMock(IUserManager::class);
+		$this->groupManager = $this->createMock(IGroupManager::class);
 		$this->random = \OC::$server->getSecureRandom();
 		$dispatcher = $this->createMock(EventDispatcherInterface::class);
 
@@ -54,6 +58,7 @@ class PublicCalendarRootTest extends TestCase {
 			$db,
 			$this->principal,
 			$this->userManager,
+			$this->groupManager,
 			$this->random,
 			$dispatcher
 		);

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

@@ -33,6 +33,7 @@ use OCA\DAV\CardDAV\CardDavBackend;
 use OCA\DAV\Connector\Sabre\Principal;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\IDBConnection;
+use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\IUserManager;
 use Sabre\DAV\PropPatch;
@@ -60,6 +61,9 @@ class CardDavBackendTest extends TestCase {
 	/** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */
 	private $userManager;
 
+	/** @var IGroupManager|\PHPUnit_Framework_MockObject_MockObject */
+	private $groupManager;
+
 	/** @var EventDispatcherInterface|\PHPUnit_Framework_MockObject_MockObject */
 	private $dispatcher;
 
@@ -80,6 +84,7 @@ class CardDavBackendTest extends TestCase {
 		parent::setUp();
 
 		$this->userManager = $this->createMock(IUserManager::class);
+		$this->groupManager = $this->createMock(IGroupManager::class);
 		$this->principal = $this->getMockBuilder('OCA\DAV\Connector\Sabre\Principal')
 			->disableOriginalConstructor()
 			->setMethods(['getPrincipalByPath', 'getGroupMembership'])
@@ -96,7 +101,7 @@ class CardDavBackendTest extends TestCase {
 
 		$this->db = \OC::$server->getDatabaseConnection();
 
-		$this->backend = new CardDavBackend($this->db, $this->principal, $this->userManager, $this->dispatcher);
+		$this->backend = new CardDavBackend($this->db, $this->principal, $this->userManager, $this->groupManager, $this->dispatcher);
 		// start every test with a empty cards_properties and cards table
 		$query = $this->db->getQueryBuilder();
 		$query->delete('cards_properties')->execute();
@@ -154,6 +159,14 @@ class CardDavBackendTest extends TestCase {
 
 	public function testAddressBookSharing() {
 
+		$this->userManager->expects($this->any())
+			->method('userExists')
+			->willReturn(true);
+
+		$this->groupManager->expects($this->any())
+			->method('groupExists')
+			->willReturn(true);
+
 		$this->backend->createAddressBook(self::UNIT_TEST_USER, 'Example', []);
 		$books = $this->backend->getAddressBooksForUser(self::UNIT_TEST_USER);
 		$this->assertEquals(1, count($books));
@@ -180,7 +193,7 @@ class CardDavBackendTest extends TestCase {
 
 		/** @var CardDavBackend | \PHPUnit_Framework_MockObject_MockObject $backend */
 		$backend = $this->getMockBuilder(CardDavBackend::class)
-				->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher])
+				->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->groupManager, $this->dispatcher])
 				->setMethods(['updateProperties', 'purgeProperties'])->getMock();
 
 		// create a new address book
@@ -253,7 +266,7 @@ class CardDavBackendTest extends TestCase {
 	public function testMultiCard() {
 
 		$this->backend = $this->getMockBuilder(CardDavBackend::class)
-			->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher])
+			->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->groupManager, $this->dispatcher])
 			->setMethods(['updateProperties'])->getMock();
 
 		// create a new address book
@@ -299,7 +312,7 @@ class CardDavBackendTest extends TestCase {
 
 	public function testDeleteWithoutCard() {
 		$this->backend = $this->getMockBuilder(CardDavBackend::class)
-			->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher])
+			->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->groupManager, $this->dispatcher])
 			->setMethods([
 				'getCardId',
 				'addChange',
@@ -339,7 +352,7 @@ class CardDavBackendTest extends TestCase {
 
 	public function testSyncSupport() {
 		$this->backend = $this->getMockBuilder(CardDavBackend::class)
-			->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher])
+			->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->groupManager, $this->dispatcher])
 			->setMethods(['updateProperties'])->getMock();
 
 		// create a new address book
@@ -362,32 +375,41 @@ class CardDavBackendTest extends TestCase {
 	}
 
 	public function testSharing() {
+
+		$this->userManager->expects($this->any())
+			->method('userExists')
+			->willReturn(true);
+
+		$this->groupManager->expects($this->any())
+			->method('groupExists')
+			->willReturn(true);
+
 		$this->backend->createAddressBook(self::UNIT_TEST_USER, 'Example', []);
 		$books = $this->backend->getAddressBooksForUser(self::UNIT_TEST_USER);
 		$this->assertEquals(1, count($books));
 
 		$l = $this->createMock(IL10N::class);
 		$exampleBook = new AddressBook($this->backend, $books[0], $l);
-		$this->backend->updateShares($exampleBook, [['href' => 'principal:principals/best-friend']], []);
+		$this->backend->updateShares($exampleBook, [['href' => 'principal:' . self::UNIT_TEST_USER1]], []);
 
 		$shares = $this->backend->getShares($exampleBook->getResourceId());
 		$this->assertEquals(1, count($shares));
 
 		// adding the same sharee again has no effect
-		$this->backend->updateShares($exampleBook, [['href' => 'principal:principals/best-friend']], []);
+		$this->backend->updateShares($exampleBook, [['href' => 'principal:' . self::UNIT_TEST_USER1]], []);
 
 		$shares = $this->backend->getShares($exampleBook->getResourceId());
 		$this->assertEquals(1, count($shares));
 
-		$books = $this->backend->getAddressBooksForUser('principals/best-friend');
+		$books = $this->backend->getAddressBooksForUser(self::UNIT_TEST_USER1);
 		$this->assertEquals(1, count($books));
 
-		$this->backend->updateShares($exampleBook, [], ['principal:principals/best-friend']);
+		$this->backend->updateShares($exampleBook, [], ['principal:' . self::UNIT_TEST_USER1]);
 
 		$shares = $this->backend->getShares($exampleBook->getResourceId());
 		$this->assertEquals(0, count($shares));
 
-		$books = $this->backend->getAddressBooksForUser('principals/best-friend');
+		$books = $this->backend->getAddressBooksForUser(self::UNIT_TEST_USER1);
 		$this->assertEquals(0, count($books));
 	}
 
@@ -398,7 +420,7 @@ class CardDavBackendTest extends TestCase {
 		$cardId = 2;
 
 		$backend = $this->getMockBuilder(CardDavBackend::class)
-			->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher])
+			->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->groupManager, $this->dispatcher])
 			->setMethods(['getCardId'])->getMock();
 
 		$backend->expects($this->any())->method('getCardId')->willReturn($cardId);