Merge pull request #50646 from nextcloud/backport/50642/stable30

[stable30] fix(sharing): better handle file share attributes
nextcloud · Feb 4, 2025 · 290d630 · 290d630
2 parents 3d9d99d + f9bd0ed
commit 290d630
Show file tree

Hide file tree

Showing 3 changed files with 220 additions and 110 deletions.
diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php
@@ -649,7 +649,6 @@ public function createShare(
 		}
 
 		$share->setSharedBy($this->currentUser);
-		$this->checkInheritedAttributes($share);
 
 		// Handle mail send
 		if (is_null($sendMail)) {
@@ -779,6 +778,7 @@ public function createShare(
 		}
 
 		$share->setShareType($shareType);
+		$this->checkInheritedAttributes($share);
 
 		if ($note !== '') {
 			$share->setNote($note);
@@ -1261,7 +1261,6 @@ public function updateShare(
 		if ($attributes !== null) {
 			$share = $this->setShareAttributes($share, $attributes);
 		}
-		$this->checkInheritedAttributes($share);
 
 		// Handle mail send
 		if ($sendMail === 'true' || $sendMail === 'false') {
@@ -1345,6 +1344,7 @@ public function updateShare(
 		}
 
 		try {
+			$this->checkInheritedAttributes($share);
 			$share = $this->shareManager->updateShare($share);
 		} catch (HintException $e) {
 			$code = $e->getCode() === 0 ? 403 : $e->getCode();
@@ -2047,30 +2047,48 @@ private function checkInheritedAttributes(IShare $share): void {
 		if (!$share->getSharedBy()) {
 			return; // Probably in a test
 		}
+
+		$canDownload = false;
+		$hideDownload = true;
+
 		$userFolder = $this->rootFolder->getUserFolder($share->getSharedBy());
-		$node = $userFolder->getFirstNodeById($share->getNodeId());
-		if (!$node) {
-			return;
-		}
-		if ($node->getStorage()->instanceOfStorage(SharedStorage::class)) {
-			$storage = $node->getStorage();
-			if ($storage instanceof Wrapper) {
-				$storage = $storage->getInstanceOfStorage(SharedStorage::class);
-				if ($storage === null) {
-					throw new \RuntimeException('Should not happen, instanceOfStorage but getInstanceOfStorage return null');
-				}
-			} else {
-				throw new \RuntimeException('Should not happen, instanceOfStorage but not a wrapper');
+		$nodes = $userFolder->getById($share->getNodeId());
+		foreach ($nodes as $node) {
+			// Owner always can download it - so allow it and break
+			if ($node->getOwner()?->getUID() === $share->getSharedBy()) {
+				$canDownload = true;
+				$hideDownload = false;
+				break;
 			}
-			/** @var \OCA\Files_Sharing\SharedStorage $storage */
-			$inheritedAttributes = $storage->getShare()->getAttributes();
-			if ($inheritedAttributes !== null && $inheritedAttributes->getAttribute('permissions', 'download') === false) {
-				$share->setHideDownload(true);
-				$attributes = $share->getAttributes();
-				if ($attributes) {
-					$attributes->setAttribute('permissions', 'download', false);
-					$share->setAttributes($attributes);
+
+			if ($node->getStorage()->instanceOfStorage(SharedStorage::class)) {
+				$storage = $node->getStorage();
+				if ($storage instanceof Wrapper) {
+					$storage = $storage->getInstanceOfStorage(SharedStorage::class);
+					if ($storage === null) {
+						throw new \RuntimeException('Should not happen, instanceOfStorage but getInstanceOfStorage return null');
+					}
+				} else {
+					throw new \RuntimeException('Should not happen, instanceOfStorage but not a wrapper');
 				}
+
+				/** @var SharedStorage $storage */
+				$originalShare = $storage->getShare();
+				$inheritedAttributes = $originalShare->getAttributes();
+				// hide if hidden and also the current share enforces hide (can only be false if one share is false or user is owner)
+				$hideDownload = $hideDownload && $originalShare->getHideDownload();
+				// allow download if already allowed by previous share or when the current share allows downloading
+				$canDownload = $canDownload || $inheritedAttributes === null || $inheritedAttributes->getAttribute('permissions', 'download') !== false;
+			}
+		}
+
+		if ($hideDownload || !$canDownload) {
+			$share->setHideDownload(true);
+
+			if (!$canDownload) {
+				$attributes = $share->getAttributes() ?? $share->newAttributes();
+				$attributes->setAttribute('permissions', 'download', false);
+				$share->setAttributes($attributes);
 			}
 		}
 	}