enh(sharing): enable unsharing for sharees for DAV shares (addressboo…

…ks and calendars) Signed-off-by: Anna Larch <anna@nextcloud.com>
nextcloud · Jan 30, 2024 · cd7f2f7 · cd7f2f7
1 parent 8597859
commit cd7f2f7
Show file tree

Hide file tree

Showing 5 changed files with 84 additions and 31 deletions.
diff --git a/apps/dav/lib/CalDAV/CalDavBackend.php b/apps/dav/lib/CalDAV/CalDavBackend.php
@@ -361,7 +361,6 @@ public function getCalendarsForUser($principalUri) {
 			// query for shared calendars
 			$principals = $this->principalBackend->getGroupMembership($principalUriOriginal, true);
 			$principals = array_merge($principals, $this->principalBackend->getCircleMembership($principalUriOriginal));
-
 			$principals[] = $principalUri;
 
 			$fields = array_column($this->propertyMap, 0);
@@ -372,19 +371,28 @@ public function getCalendarsForUser($principalUri) {
 			$fields[] = 'a.principaluri';
 			$fields[] = 'a.transparent';
 			$fields[] = 's.access';
-			$query = $this->db->getQueryBuilder();
-			$query->select($fields)
+
+			$select = $this->db->getQueryBuilder();
+			$subSelect = $this->db->getQueryBuilder();
+
+			$subSelect->select('resourceid')
+				->from('dav_shares', 'd')
+				->where($subSelect->expr()->eq('d.access', $select->createNamedParameter(Backend::ACCESS_UNSHARED)))
+				->andWhere($subSelect->expr()->in('d.principaluri', $select->createNamedParameter($principals, IQueryBuilder::PARAM_STR_ARRAY)));
+
+			$select->select($fields)
 				->from('dav_shares', 's')
-				->join('s', 'calendars', 'a', $query->expr()->eq('s.resourceid', 'a.id'))
-				->where($query->expr()->in('s.principaluri', $query->createParameter('principaluri')))
-				->andWhere($query->expr()->eq('s.type', $query->createParameter('type')))
-				->setParameter('type', 'calendar')
-				->setParameter('principaluri', $principals, \Doctrine\DBAL\Connection::PARAM_STR_ARRAY);
+				->join('s', 'calendars', 'a', $select->expr()->eq('s.resourceid', 'a.id'))
+				->where($select->expr()->in('s.principaluri', $select->createNamedParameter($principals, IQueryBuilder::PARAM_STR_ARRAY)))
+				->andWhere($select->expr()->eq('s.type', $select->createNamedParameter('calendar', IQueryBuilder::PARAM_STR)))
+				->andWhere($select->expr()->notIn('a.id', $select->createFunction('(' . $subSelect->getSQL() . ')'),IQueryBuilder::PARAM_INT_ARRAY));
 
-			$result = $query->executeQuery();
+			$results = $select->executeQuery();
+			$shared = $results->fetchAll();
+			$results->closeCursor();
 
 			$readOnlyPropertyName = '{' . \OCA\DAV\DAV\Sharing\Plugin::NS_OWNCLOUD . '}read-only';
-			while ($row = $result->fetch()) {
+			foreach ($shared as $row) {
 				$row['principaluri'] = (string) $row['principaluri'];
 				if ($row['principaluri'] === $principalUri) {
 					continue;
@@ -393,7 +401,7 @@ public function getCalendarsForUser($principalUri) {
 				$readOnly = (int) $row['access'] === Backend::ACCESS_READ;
 				if (isset($calendars[$row['id']])) {
 					if ($readOnly) {
-						// New share can not have more permissions then the old one.
+						// New share can not have more permissions than the old one.
 						continue;
 					}
 					if (isset($calendars[$row['id']][$readOnlyPropertyName]) &&
@@ -2891,7 +2899,7 @@ public function updateShares(IShareable $shareable, array $add, array $remove):
 			}
 			$oldShares = $this->getShares($calendarId);
 
-			$this->calendarSharingBackend->updateShares($shareable, $add, $remove);
+			$this->calendarSharingBackend->updateShares($shareable, $add, $remove, $oldShares);
 
 			$this->dispatcher->dispatchTyped(new CalendarShareUpdatedEvent($calendarId, $calendarRow, $oldShares, $add, $remove));
 		}, $this->db);

diff --git a/apps/dav/lib/CalDAV/Calendar.php b/apps/dav/lib/CalDAV/Calendar.php
@@ -236,14 +236,6 @@ public function delete() {
 		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->caldavBackend->getShares($this->getResourceId());
-			$shares = array_filter($shares, function ($share) use ($principal) {
-				return $share['href'] === $principal;
-			});
-			if (empty($shares)) {
-				throw new Forbidden();
-			}
-
 			$this->caldavBackend->updateShares($this, [], [
 				$principal
 			]);

diff --git a/apps/dav/lib/CardDAV/CardDavBackend.php b/apps/dav/lib/CardDAV/CardDavBackend.php
@@ -1056,7 +1056,7 @@ public function updateShares(IShareable $shareable, array $add, array $remove):
 			$addressBookData = $this->getAddressBookById($addressBookId);
 			$oldShares = $this->getShares($addressBookId);
 
-			$this->sharingBackend->updateShares($shareable, $add, $remove);
+			$this->sharingBackend->updateShares($shareable, $add, $remove, $oldShares);
 
 			$this->dispatcher->dispatchTyped(new AddressBookShareUpdatedEvent($addressBookId, $addressBookData, $oldShares, $add, $remove));
 		}, $this->db);

diff --git a/apps/dav/lib/Connector/Sabre/DavAclPlugin.php b/apps/dav/lib/Connector/Sabre/DavAclPlugin.php
@@ -58,6 +58,9 @@ public function checkPrivileges($uri, $privileges, $recursion = self::R_PARENT,
 				case AddressBook::class:
 					$type = 'Addressbook';
 					break;
+				case Calendar::class:
+					$type = 'Calendar';
+					break;
 				default:
 					$type = 'Node';
 					break;

diff --git a/apps/dav/lib/DAV/Sharing/Backend.php b/apps/dav/lib/DAV/Sharing/Backend.php
@@ -44,10 +44,12 @@ class Backend {
 	private IGroupManager $groupManager;
 	private Principal $principalBackend;
 	private string $resourceType;
-
 	public const ACCESS_OWNER = 1;
+
 	public const ACCESS_READ_WRITE = 2;
 	public const ACCESS_READ = 3;
+	// 4 is already in use for public calendars
+	const ACCESS_UNSHARED = 5;
 
 	private CappedMemoryCache $shareCache;
 
@@ -64,9 +66,9 @@ public function __construct(IDBConnection $db, IUserManager $userManager, IGroup
 	 * @param list<array{href: string, commonName: string, readOnly: bool}> $add
 	 * @param list<string> $remove
 	 */
-	public function updateShares(IShareable $shareable, array $add, array $remove): void {
+	public function updateShares(IShareable $shareable, array $add, array $remove, array $oldShares = []): void {
 		$this->shareCache->clear();
-		$this->atomic(function () use ($shareable, $add, $remove) {
+		$this->atomic(function () use ($shareable, $add, $remove, $oldShares) {
 			foreach ($add as $element) {
 				$principal = $this->principalBackend->findByUri($element['href'], '');
 				if ($principal !== '') {
@@ -75,7 +77,16 @@ public function updateShares(IShareable $shareable, array $add, array $remove):
 			}
 			foreach ($remove as $element) {
 				$principal = $this->principalBackend->findByUri($element, '');
-				if ($principal !== '') {
+				if ($principal === '') {
+					continue;
+				}
+
+				// Delete any possible direct shares (since the frontend does not separate between them)
+				$this->deleteShare($shareable, $element);
+
+				// Check if a user has a groupshare that they're trying to free themselves from
+				// If so we need to add a self::ACCESS_UNSHARED row
+				if(!str_contains($principal, 'group') && $this->hasGroupShare($oldShares)) {
 					$this->unshare($shareable, $element);
 				}
 			}
@@ -112,7 +123,7 @@ private function shareWith(IShareable $shareable, array $element): void {
 		}
 
 		// remove the share if it already exists
-		$this->unshare($shareable, $element['href']);
+		$this->deleteShare($shareable, $element['href']);
 		$access = self::ACCESS_READ;
 		if (isset($element['readOnly'])) {
 			$access = $element['readOnly'] ? self::ACCESS_READ : self::ACCESS_READ_WRITE;
@@ -147,6 +158,28 @@ public function deleteAllSharesByUser(string $principaluri): void {
 			->executeStatement();
 	}
 
+	private function deleteShare(IShareable $shareable, string $element): void {
+		$this->shareCache->clear();
+		$parts = explode(':', $element, 2);
+		if ($parts[0] !== 'principal') {
+			return;
+		}
+
+		// don't share with owner
+		if ($shareable->getOwner() === $parts[1]) {
+			return;
+		}
+
+		$query = $this->db->getQueryBuilder();
+		$query->delete('dav_shares');
+		$query->where(
+			$query->expr()->eq('resourceid', $query->createNamedParameter($shareable->getResourceId(), IQueryBuilder::PARAM_INT)),
+			$query->expr()->eq('type', $query->createNamedParameter($this->resourceType)),
+			$query->expr()->eq('principaluri', $query->createNamedParameter($parts[1]))
+		);
+		$query->executeStatement();
+	}
+
 	private function unshare(IShareable $shareable, string $element): void {
 		$this->shareCache->clear();
 		$parts = explode(':', $element, 2);
@@ -160,14 +193,31 @@ private function unshare(IShareable $shareable, string $element): void {
 		}
 
 		$query = $this->db->getQueryBuilder();
-		$query->delete('dav_shares')
-			->where($query->expr()->eq('resourceid', $query->createNamedParameter($shareable->getResourceId())))
-			->andWhere($query->expr()->eq('type', $query->createNamedParameter($this->resourceType)))
-			->andWhere($query->expr()->eq('principaluri', $query->createNamedParameter($parts[1])))
-		;
+		$query->insert('dav_shares')
+			->values([
+				'principaluri' => $query->createNamedParameter($parts[1]),
+				'type' => $query->createNamedParameter($this->resourceType),
+				'access' => $query->createNamedParameter(self::ACCESS_UNSHARED),
+				'resourceid' => $query->createNamedParameter($shareable->getResourceId())
+			]);
 		$query->executeStatement();
 	}
 
+	/**
+	 * @param array $oldShares
+	 * @return bool
+	 */
+	private function hasGroupShare(array $oldShares): bool {
+		$group = false;
+		foreach ($oldShares as $share) {
+			if($share['{http://owncloud.org/ns}group-share'] === true) {
+				$group = true;
+				break;
+			}
+		}
+		return $group;
+	}
+
 	/**
 	 * Returns the list of people whom this resource is shared with.
 	 *