Merge pull request #31405 from owncloud/bugfix/31402

group principals are now verified as well when sharing…

apps/dav/appinfo/v1/caldav.php

@@ -45,10 +45,13 @@
 	\OC::$server->getGroupManager(),
 	'principals/'
 );
+$groupPrincipalBackend = new \OCA\DAV\DAV\GroupPrincipalBackend(
+	\OC::$server->getGroupManager()
+);
 $db = \OC::$server->getDatabaseConnection();
 $config = \OC::$server->getConfig();
 $random = \OC::$server->getSecureRandom();
-$calDavBackend = new CalDavBackend($db, $principalBackend, $config, $random, true);
+$calDavBackend = new CalDavBackend($db, $principalBackend, $groupPrincipalBackend, $random, true);
 
 $debugging = \OC::$server->getConfig()->getSystemValue('debug', false);
 

apps/dav/appinfo/v1/carddav.php

@@ -45,8 +45,11 @@
 	\OC::$server->getGroupManager(),
 	'principals/'
 );
+$groupPrincipalBackend = new \OCA\DAV\DAV\GroupPrincipalBackend(
+	\OC::$server->getGroupManager()
+);
 $db = \OC::$server->getDatabaseConnection();
-$cardDavBackend = new CardDavBackend($db, $principalBackend, null, true);
+$cardDavBackend = new CardDavBackend($db, $principalBackend, $groupPrincipalBackend, null, true);
 
 $debugging = \OC::$server->getConfig()->getSystemValue('debug', false);
 

apps/dav/lib/CalDAV/CalDavBackend.php

@@ -26,6 +26,7 @@
 
 use Doctrine\DBAL\Connection;
 use OCA\DAV\Connector\Sabre\Principal;
+use OCA\DAV\DAV\GroupPrincipalBackend;
 use OCA\DAV\DAV\Sharing\Backend;
 use OCA\DAV\DAV\Sharing\IShareable;
 use OCP\DB\QueryBuilder\IQueryBuilder;
@@ -65,12 +66,12 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
 	 * in 2038-01-19 to avoid problems when the date is converted
 	 * to a unix timestamp.
 	 */
-	const MAX_DATE = '2038-01-01';
+	public const MAX_DATE = '2038-01-01';
 
-	const ACCESS_PUBLIC = 4;
-	const CLASSIFICATION_PUBLIC = 0;
-	const CLASSIFICATION_PRIVATE = 1;
-	const CLASSIFICATION_CONFIDENTIAL = 2;
+	public const ACCESS_PUBLIC = 4;
+	public const CLASSIFICATION_PUBLIC = 0;
+	public const CLASSIFICATION_PRIVATE = 1;
+	public const CLASSIFICATION_CONFIDENTIAL = 2;
 
 	/**
 	 * List of CalDAV properties, and how they map to database field names
@@ -112,11 +113,9 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
 	/** @var Principal */
 	private $principalBackend;
 
-	/** @var IConfig */
-	private $config;
-
 	/** @var ISecureRandom */
 	private $random;
+
 	/** @var bool */
 	private $legacyMode;
 
@@ -125,19 +124,18 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
 	 *
 	 * @param IDBConnection $db
 	 * @param Principal $principalBackend
-	 * @param IConfig $config
+	 * @param GroupPrincipalBackend $groupPrincipalBackend
 	 * @param ISecureRandom $random
 	 * @param bool $legacyMode
 	 */
 	public function __construct(IDBConnection $db,
 								Principal $principalBackend,
-								IConfig $config,
+								GroupPrincipalBackend $groupPrincipalBackend,
 								ISecureRandom $random,
 								$legacyMode = false) {
 		$this->db = $db;
 		$this->principalBackend = $principalBackend;
-		$this->sharingBackend = new Backend($this->db, $principalBackend, 'calendar');
-		$this->config = $config;
+		$this->sharingBackend = new Backend($this->db, $principalBackend, $groupPrincipalBackend, 'calendar');
 		$this->random = $random;
 		$this->legacyMode = $legacyMode;
 	}
@@ -816,6 +814,8 @@ public function getMultipleCalendarObjects($calendarId, array $uris) {
 	 * @param string $calendarData
 	 * @return string
 	 * @throws DAV\Exception\BadRequest
+	 * @throws \Sabre\VObject\Recur\MaxInstancesExceededException
+	 * @throws \Sabre\VObject\Recur\NoInstancesException
 	 */
 	public function createCalendarObject($calendarId, $objectUri, $calendarData) {
 		$extraData = $this->getDenormalizedData($calendarData);
@@ -860,6 +860,8 @@ public function createCalendarObject($calendarId, $objectUri, $calendarData) {
 	 * @param string $calendarData
 	 * @return string
 	 * @throws DAV\Exception\BadRequest
+	 * @throws \Sabre\VObject\Recur\MaxInstancesExceededException
+	 * @throws \Sabre\VObject\Recur\NoInstancesException
 	 */
 	public function updateCalendarObject($calendarId, $objectUri, $calendarData) {
 		$extraData = $this->getDenormalizedData($calendarData);
@@ -891,7 +893,7 @@ public function updateCalendarObject($calendarId, $objectUri, $calendarData) {
 	public function setClassification($calendarObjectId, $classification) {
 		if (!\in_array($classification, [
 			self::CLASSIFICATION_PUBLIC, self::CLASSIFICATION_PRIVATE, self::CLASSIFICATION_CONFIDENTIAL
-		])) {
+		], true)) {
 			throw new \InvalidArgumentException();
 		}
 		$query = $this->db->getQueryBuilder();
@@ -1278,7 +1280,7 @@ public function createSubscription($principalUri, $uri, array $properties) {
 		foreach ($this->subscriptionPropertyMap as $xmlName=>$dbName) {
 			if (\array_key_exists($xmlName, $properties)) {
 				$values[$dbName] = $properties[$xmlName];
-				if (\in_array($dbName, $propertiesBoolean)) {
+				if (\in_array($dbName, $propertiesBoolean, true)) {
 					$values[$dbName] = true;
 				}
 			}
@@ -1501,8 +1503,9 @@ protected function addChange($calendarId, $objectUri, $operation) {
 	 *
 	 * @param string $calendarData
 	 * @return array
-	 * @throws \Sabre\VObject\Recur\MaxInstancesExceededException
 	 * @throws DAV\Exception\BadRequest
+	 * @throws \Sabre\VObject\Recur\MaxInstancesExceededException
+	 * @throws \Sabre\VObject\Recur\NoInstancesException
 	 */
 	public function getDenormalizedData($calendarData) {
 		$vObject = Reader::read($calendarData);
@@ -1520,7 +1523,7 @@ public function getDenormalizedData($calendarData) {
 			}
 		}
 		if (!$componentType) {
-			throw new \Sabre\DAV\Exception\BadRequest('Calendar objects must have a VJOURNAL, VEVENT or VTODO component');
+			throw new DAV\Exception\BadRequest('Calendar objects must have a VJOURNAL, VEVENT or VTODO component');
 		}
 		if ($componentType === 'VEVENT' && $component->DTSTART) {
 			$firstOccurrence = $component->DTSTART->getDateTime()->getTimeStamp();

apps/dav/lib/CardDAV/CardDavBackend.php

@@ -28,6 +28,7 @@
 
 use OC\Cache\CappedMemoryCache;
 use OCA\DAV\Connector\Sabre\Principal;
+use OCA\DAV\DAV\GroupPrincipalBackend;
 use OCA\DAV\DAV\Sharing\Backend;
 use OCA\DAV\DAV\Sharing\IShareable;
 use OCP\DB\QueryBuilder\IQueryBuilder;
@@ -78,16 +79,19 @@ class CardDavBackend implements BackendInterface, SyncSupport {
 	 *
 	 * @param IDBConnection $db
 	 * @param Principal $principalBackend
+	 * @param GroupPrincipalBackend $groupPrincipalBackend
 	 * @param EventDispatcherInterface $dispatcher
+	 * @param bool $legacyMode
 	 */
 	public function __construct(IDBConnection $db,
 								Principal $principalBackend,
+								GroupPrincipalBackend $groupPrincipalBackend,
 								EventDispatcherInterface $dispatcher = null,
 								$legacyMode = false) {
 		$this->db = $db;
 		$this->principalBackend = $principalBackend;
 		$this->dispatcher = $dispatcher;
-		$this->sharingBackend = new Backend($this->db, $principalBackend, 'addressbook');
+		$this->sharingBackend = new Backend($this->db, $principalBackend, $groupPrincipalBackend, 'addressbook');
 		$this->legacyMode = $legacyMode;
 		$this->idCache = new CappedMemoryCache();
 	}

apps/dav/lib/Command/CreateCalendar.php

@@ -23,6 +23,7 @@
 
 use OCA\DAV\CalDAV\CalDavBackend;
 use OCA\DAV\Connector\Sabre\Principal;
+use OCA\DAV\DAV\GroupPrincipalBackend;
 use OCP\IDBConnection;
 use OCP\IGroupManager;
 use OCP\IUserManager;
@@ -75,11 +76,14 @@ protected function execute(InputInterface $input, OutputInterface $output) {
 			$this->userManager,
 			$this->groupManager
 		);
+		$groupPrincipalBackend = new GroupPrincipalBackend(
+			$this->groupManager
+		);
 		$config = \OC::$server->getConfig();
 		$random = \OC::$server->getSecureRandom();
 
 		$name = $input->getArgument('name');
-		$caldav = new CalDavBackend($this->dbConnection, $principalBackend, $config, $random);
+		$caldav = new CalDavBackend($this->dbConnection, $principalBackend, $groupPrincipalBackend, $random);
 		$caldav->createCalendar("principals/users/$user", $name, []);
 	}
 }

apps/dav/lib/DAV/GroupPrincipalBackend.php

@@ -28,7 +28,7 @@
 use Sabre\DAVACL\PrincipalBackend\BackendInterface;
 
 class GroupPrincipalBackend implements BackendInterface {
-	const PRINCIPAL_PREFIX = 'principals/groups';
+	public const PRINCIPAL_PREFIX = 'principals/groups';
 
 	/** @var IGroupManager */
 	private $groupManager;
@@ -96,7 +96,6 @@ public function getPrincipalByPath($path) {
 	 *
 	 * @param string $principal
 	 * @return string[]
-	 * @throws Exception
 	 */
 	public function getGroupMemberSet($principal) {
 		$elements = \explode('/', $principal);
@@ -123,7 +122,6 @@ public function getGroupMemberSet($principal) {
 	 *
 	 * @param string $principal
 	 * @return array
-	 * @throws Exception
 	 */
 	public function getGroupMembership($principal) {
 		return [];
@@ -167,6 +165,13 @@ public function searchPrincipals($prefixPath, array $searchProperties, $test = '
 	 * @return string
 	 */
 	public function findByUri($uri, $principalPrefix) {
+		if (\strpos($uri, 'principal:') === 0) {
+			$principal = \substr($uri, 10);
+			$principal = $this->getPrincipalByPath($principal);
+			if ($principal !== null) {
+				return $principal['uri'];
+			}
+		}
 		return '';
 	}
 

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

@@ -25,6 +25,7 @@
 namespace OCA\DAV\DAV\Sharing;
 
 use OCA\DAV\Connector\Sabre\Principal;
+use OCA\DAV\DAV\GroupPrincipalBackend;
 use OCP\IDBConnection;
 
 class Backend {
@@ -33,21 +34,25 @@ class Backend {
 	private $db;
 	/** @var Principal */
 	private $principalBackend;
+	/** @var GroupPrincipalBackend */
+	private $groupPrincipalBackend;
 	/** @var string */
 	private $resourceType;
 
-	const ACCESS_OWNER = 1;
-	const ACCESS_READ_WRITE = 2;
-	const ACCESS_READ = 3;
+	public const ACCESS_OWNER = 1;
+	public const ACCESS_READ_WRITE = 2;
+	public const ACCESS_READ = 3;
 
 	/**
 	 * @param IDBConnection $db
 	 * @param Principal $principalBackend
+	 * @param GroupPrincipalBackend $groupPrincipalBackend
 	 * @param string $resourceType
 	 */
-	public function __construct(IDBConnection $db, Principal $principalBackend, $resourceType) {
+	public function __construct(IDBConnection $db, Principal $principalBackend, GroupPrincipalBackend $groupPrincipalBackend, $resourceType) {
 		$this->db = $db;
 		$this->principalBackend = $principalBackend;
+		$this->groupPrincipalBackend = $groupPrincipalBackend;
 		$this->resourceType = $resourceType;
 	}
 
@@ -58,19 +63,27 @@ public function __construct(IDBConnection $db, Principal $principalBackend, $res
 	 */
 	public function updateShares($shareable, $add, $remove) {
 		foreach ($add as $element) {
-			$principal = $this->principalBackend->findByUri($element['href'], '');
+			$principal = $this->findByUri($element['href']);
 			if ($principal !== '') {
 				$this->shareWith($shareable, $element);
 			}
 		}
 		foreach ($remove as $element) {
-			$principal = $this->principalBackend->findByUri($element, '');
+			$principal = $this->findByUri($element);
 			if ($principal !== '') {
 				$this->unshare($shareable, $element);
 			}
 		}
 	}
 
+	private function findByUri($uri) {
+		$principal = $this->principalBackend->findByUri($uri, '');
+		if ($principal !== '') {
+			return $principal;
+		}
+		return $this->groupPrincipalBackend->findByUri($uri, '');
+	}
+
 	/**
 	 * @param IShareable $shareable
 	 * @param string $element
@@ -174,9 +187,9 @@ public function getShares($resourceId) {
 			$p = $this->principalBackend->getPrincipalByPath($row['principaluri']);
 			$shares[]= [
 				'href' => "principal:${row['principaluri']}",
-				'commonName' => isset($p['{DAV:}displayname']) ? $p['{DAV:}displayname'] : '',
+				'commonName' => $p['{DAV:}displayname'] ?? '',
 				'status' => 1,
-				'readOnly' => ($row['access'] == self::ACCESS_READ),
+				'readOnly' => $row['access'] == self::ACCESS_READ,
 				'{http://owncloud.org/ns}principal' => $row['principaluri'],
 				'{http://owncloud.org/ns}group-share' => $p === null
 			];
@@ -197,21 +210,21 @@ public function applyShareAcl($resourceId, $acl) {
 		foreach ($shares as $share) {
 			$acl[] = [
 				'privilege' => '{DAV:}read',
-				'principal' => $share['{' . \OCA\DAV\DAV\Sharing\Plugin::NS_OWNCLOUD . '}principal'],
+				'principal' => $share['{' . Plugin::NS_OWNCLOUD . '}principal'],
 				'protected' => true,
 			];
 			if (!$share['readOnly']) {
 				$acl[] = [
 					'privilege' => '{DAV:}write',
-					'principal' => $share['{' . \OCA\DAV\DAV\Sharing\Plugin::NS_OWNCLOUD . '}principal'],
+					'principal' => $share['{' . Plugin::NS_OWNCLOUD . '}principal'],
 					'protected' => true,
 				];
 			} elseif ($this->resourceType === 'calendar') {
 				// Allow changing the properties of read only calendars,
 				// so users can change the visibility.
 				$acl[] = [
 					'privilege' => '{DAV:}write-properties',
-					'principal' => $share['{' . \OCA\DAV\DAV\Sharing\Plugin::NS_OWNCLOUD . '}principal'],
+					'principal' => $share['{' . Plugin::NS_OWNCLOUD . '}principal'],
 					'protected' => true,
 				];
 			}

apps/dav/lib/RootCollection.php

@@ -58,7 +58,7 @@ public function __construct() {
 		$systemPrincipals->disableListing = $disableListing;
 		$filesCollection = new Files\RootCollection($userPrincipalBackend, 'principals/users');
 		$filesCollection->disableListing = $disableListing;
-		$caldavBackend = new CalDavBackend($db, $userPrincipalBackend, $config, $random);
+		$caldavBackend = new CalDavBackend($db, $userPrincipalBackend, $groupPrincipalBackend, $random);
 		$calendarRoot = new CalendarRoot($userPrincipalBackend, $caldavBackend, 'principals/users');
 		$calendarRoot->disableListing = $disableListing;
 		$publicCalendarRoot = new PublicCalendarRoot($caldavBackend);
@@ -77,11 +77,11 @@ public function __construct() {
 			\OC::$server->getRootFolder()
 		);
 
-		$usersCardDavBackend = new CardDavBackend($db, $userPrincipalBackend, $dispatcher);
+		$usersCardDavBackend = new CardDavBackend($db, $userPrincipalBackend, $groupPrincipalBackend, $dispatcher);
 		$usersAddressBookRoot = new AddressBookRoot($userPrincipalBackend, $usersCardDavBackend, 'principals/users');
 		$usersAddressBookRoot->disableListing = $disableListing;
 
-		$systemCardDavBackend = new CardDavBackend($db, $userPrincipalBackend, $dispatcher);
+		$systemCardDavBackend = new CardDavBackend($db, $userPrincipalBackend, $groupPrincipalBackend, $dispatcher);
 		$systemAddressBookRoot = new AddressBookRoot(new SystemPrincipalBackend(), $systemCardDavBackend, 'principals/system');
 		$systemAddressBookRoot->disableListing = $disableListing;
 

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

@@ -24,6 +24,7 @@
 
 use OCA\DAV\CalDAV\CalDavBackend;
 use OCA\DAV\Connector\Sabre\Principal;
+use OCA\DAV\DAV\GroupPrincipalBackend;
 use OCP\IConfig;
 use OCP\Security\ISecureRandom;
 use Sabre\CalDAV\Xml\Property\SupportedCalendarComponentSet;
@@ -44,6 +45,9 @@ abstract class AbstractCalDavBackendTest extends TestCase {
 	/** @var Principal | \PHPUnit_Framework_MockObject_MockObject */
 	protected $principal;
 
+	/** @var GroupPrincipalBackend | \PHPUnit_Framework_MockObject_MockObject */
+	protected $groupPrincipal;
+
 	/** @var IConfig */
 	protected $config;
 
@@ -69,10 +73,11 @@ public function setUp() {
 			->withAnyParameters()
 			->willReturn([self::UNIT_TEST_GROUP]);
 
+		$this->groupPrincipal = $this->createMock(GroupPrincipalBackend::class);
 		$db = \OC::$server->getDatabaseConnection();
 		$this->config = \OC::$server->getConfig();
 		$this->random = \OC::$server->getSecureRandom();
-		$this->backend = new CalDavBackend($db, $this->principal, $this->config, $this->random);
+		$this->backend = new CalDavBackend($db, $this->principal, $this->groupPrincipal, $this->random);
 
 		$this->tearDown();
 	}