Fix re-accepting or re-rejecting remote group shares

When accepting a group share, a sub-share entry is created which also
has a different id.

When accepting or rejecting the sub-share, simply update the "accepted"
flag instead of trying to re-insert the entry.

Adjust getShare to also properly validate group share membership
when called on a sub-share id.

Signed-off-by: Vincent Petry <vincent@nextcloud.com>

1 files changed, 97 insertions, 42 deletions

diff --git a/apps/files_sharing/lib/External/Manager.php b/apps/files_sharing/lib/External/Manager.php
index de566692d6b..b67ca675c31 100644
--- a/apps/files_sharing/lib/External/Manager.php
+++ b/apps/files_sharing/lib/External/Manager.php
@@ -227,7 +227,7 @@ class Manager {
 	 * @param int $id share id
 	 * @return mixed share of false
 	 */
-	public function getShare($id) {
+	private function fetchShare($id) {
 		$getShare = $this->connection->prepare('
 			SELECT `id`, `remote`, `remote_id`, `share_token`, `name`, `owner`, `user`, `mountpoint`, `accepted`, `parent`, `share_type`, `password`, `mountpoint_hash`
 			FROM  `*PREFIX*share_external`
@@ -235,14 +235,32 @@ class Manager {
 		$result = $getShare->execute([$id]);
 		$share = $result->fetch();
 		$result->closeCursor();
+		return $share;
+	}
+
+	/**
+	 * get share
+	 *
+	 * @param int $id share id
+	 * @return mixed share of false
+	 */
+	public function getShare($id) {
+		$share = $this->fetchShare($id);
 		$validShare = is_array($share) && isset($share['share_type']) && isset($share['user']);
 
 		// check if the user is allowed to access it
 		if ($validShare && (int)$share['share_type'] === IShare::TYPE_USER && $share['user'] === $this->uid) {
 			return $share;
 		} elseif ($validShare && (int)$share['share_type'] === IShare::TYPE_GROUP) {
+			$parentId = (int)$share['parent'];
+			if ($parentId !== -1) {
+				// we just retrieved a sub-share, switch to the parent entry for verification
+				$groupShare = $this->fetchShare($parentId);
+			} else {
+				$groupShare = $share;
+			}
 			$user = $this->userManager->get($this->uid);
-			if ($this->groupManager->get($share['user'])->inGroup($user)) {
+			if ($this->groupManager->get($groupShare['user'])->inGroup($user)) {
 				return $share;
 			}
 		}
@@ -251,6 +269,20 @@ class Manager {
 	}
 
 	/**
+	 * Updates accepted flag in the database
+	 *
+	 * @param int $id
+	 */
+	private function updateAccepted(int $shareId, bool $accepted) : void {
+		$query = $this->connection->prepare('
+			UPDATE `*PREFIX*share_external`
+			SET `accepted` = ?
+			WHERE `id` = ?');
+		$updateResult = $query->execute([$accepted ? 1 : 0, $shareId]);
+		$updateResult->closeCursor();
+	}
+
+	/**
 	 * accept server-to-server share
 	 *
 	 * @param int $id
@@ -277,21 +309,34 @@ class Manager {
 				WHERE `id` = ? AND `user` = ?');
 				$userShareAccepted = $acceptShare->execute([1, $mountPoint, $hash, $id, $this->uid]);
 			} else {
-				try {
-					$this->writeShareToDb(
-						$share['remote'],
-						$share['share_token'],
-						$share['password'],
-						$share['name'],
-						$share['owner'],
-						$this->uid,
-						$mountPoint, $hash, 1,
-						$share['remote_id'],
-						$id,
-						$share['share_type']);
-					$result = true;
-				} catch (Exception $e) {
-					$result = false;
+				$parentId = (int)$share['parent'];
+				if ($parentId !== -1) {
+					// this is the sub-share, simply update it to re-accept
+					try {
+						$this->updateAccepted((int)$share['id'], true);
+						$result = true;
+					} catch (Exception $e) {
+						$this->logger->logException($e);
+						$result = false;
+					}
+				} else {
+					try {
+						$this->writeShareToDb(
+							$share['remote'],
+							$share['share_token'],
+							$share['password'],
+							$share['name'],
+							$share['owner'],
+							$this->uid,
+							$mountPoint, $hash, 1,
+							$share['remote_id'],
+							$id,
+							$share['share_type']);
+						$result = true;
+					} catch (Exception $e) {
+						$this->logger->logException($e);
+						$result = false;
+					}
 				}
 			}
 			if ($userShareAccepted !== false) {
@@ -327,23 +372,38 @@ class Manager {
 			$this->processNotification($id);
 			$result = true;
 		} elseif ($share && (int)$share['share_type'] === IShare::TYPE_GROUP) {
-			try {
-				$this->writeShareToDb(
-					$share['remote'],
-					$share['share_token'],
-					$share['password'],
-					$share['name'],
-					$share['owner'],
-					$this->uid,
-					$share['mountpoint'],
-					$share['mountpoint_hash'],
-					0,
-					$share['remote_id'],
-					$id,
-					$share['share_type']);
-				$result = true;
-			} catch (Exception $e) {
-				$result = false;
+			$parent = (int)$share['parent'];
+			// can only decline an already accepted/mounted group share,
+			// check if this is the sub-share entry
+			if ($parent !== -1) {
+				try {
+					// this is the sub-share, simply update it to decline
+					$this->updateAccepted((int)$share['id'], false);
+					$result = true;
+				} catch (Exception $e) {
+					$this->logger->logException($e);
+					$result = false;
+				}
+			} else {
+				try {
+					$this->writeShareToDb(
+						$share['remote'],
+						$share['share_token'],
+						$share['password'],
+						$share['name'],
+						$share['owner'],
+						$this->uid,
+						$share['mountpoint'],
+						$share['mountpoint_hash'],
+						0,
+						$share['remote_id'],
+						$id,
+						$share['share_type']);
+					$result = true;
+				} catch (Exception $e) {
+					$this->logger->logException($e);
+					$result = false;
+				}
 			}
 			$this->processNotification($id);
 		}
@@ -528,18 +588,13 @@ class Manager {
 				}
 
 				$query = $this->connection->prepare('
-				DELETE FROM `*PREFIX*share_external`
-				WHERE `id` = ?
+					DELETE FROM `*PREFIX*share_external`
+					WHERE `id` = ?
 				');
 				$deleteResult = $query->execute([(int)$share['id']]);
 				$deleteResult->closeCursor();
 			} elseif ($share !== false && (int)$share['share_type'] === IShare::TYPE_GROUP) {
-				$query = $this->connection->prepare('
-					UPDATE `*PREFIX*share_external`
-					SET `accepted` = ?
-					WHERE `id` = ?');
-				$updateResult = $query->execute([0, (int)$share['id']]);
-				$updateResult->closeCursor();
+				$this->updateAccepted((int)$share['id'], false);
 			}
 
 			$this->removeReShares($id);