Skip to content

Commit 856f813

Browse files
committed
fix(sharing): Ensure download restrictions are not dropped
When a user receives a share with share-permissions but also with download restrictions (hide download or the modern download permission attribute), then re-shares of that share must always also include those restrictions. Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
1 parent 83e35b6 commit 856f813

3 files changed

Lines changed: 194 additions & 92 deletions

File tree

apps/files_sharing/lib/Controller/ShareAPIController.php

Lines changed: 41 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -654,7 +654,6 @@ public function createShare(
654654
}
655655

656656
$share->setSharedBy($this->userId);
657-
$this->checkInheritedAttributes($share);
658657

659658
// Handle mail send
660659
if (is_null($sendMail)) {
@@ -787,6 +786,7 @@ public function createShare(
787786
}
788787

789788
$share->setShareType($shareType);
789+
$this->checkInheritedAttributes($share);
790790

791791
if ($note !== '') {
792792
$share->setNote($note);
@@ -1272,7 +1272,6 @@ public function updateShare(
12721272
if ($attributes !== null) {
12731273
$share = $this->setShareAttributes($share, $attributes);
12741274
}
1275-
$this->checkInheritedAttributes($share);
12761275

12771276
// Handle mail send
12781277
if ($sendMail === 'true' || $sendMail === 'false') {
@@ -1359,6 +1358,7 @@ public function updateShare(
13591358
}
13601359

13611360
try {
1361+
$this->checkInheritedAttributes($share);
13621362
$share = $this->shareManager->updateShare($share);
13631363
} catch (HintException $e) {
13641364
$code = $e->getCode() === 0 ? 403 : $e->getCode();
@@ -2083,30 +2083,48 @@ private function checkInheritedAttributes(IShare $share): void {
20832083
if (!$share->getSharedBy()) {
20842084
return; // Probably in a test
20852085
}
2086+
2087+
$canDownload = false;
2088+
$hideDownload = true;
2089+
20862090
$userFolder = $this->rootFolder->getUserFolder($share->getSharedBy());
2087-
$node = $userFolder->getFirstNodeById($share->getNodeId());
2088-
if (!$node) {
2089-
return;
2090-
}
2091-
if ($node->getStorage()->instanceOfStorage(SharedStorage::class)) {
2092-
$storage = $node->getStorage();
2093-
if ($storage instanceof Wrapper) {
2094-
$storage = $storage->getInstanceOfStorage(SharedStorage::class);
2095-
if ($storage === null) {
2096-
throw new \RuntimeException('Should not happen, instanceOfStorage but getInstanceOfStorage return null');
2097-
}
2098-
} else {
2099-
throw new \RuntimeException('Should not happen, instanceOfStorage but not a wrapper');
2091+
$nodes = $userFolder->getById($share->getNodeId());
2092+
foreach ($nodes as $node) {
2093+
// Owner always can download it - so allow it and break
2094+
if ($node->getOwner()?->getUID() === $share->getSharedBy()) {
2095+
$canDownload = true;
2096+
$hideDownload = false;
2097+
break;
21002098
}
2101-
/** @var SharedStorage $storage */
2102-
$inheritedAttributes = $storage->getShare()->getAttributes();
2103-
if ($inheritedAttributes !== null && $inheritedAttributes->getAttribute('permissions', 'download') === false) {
2104-
$share->setHideDownload(true);
2105-
$attributes = $share->getAttributes();
2106-
if ($attributes) {
2107-
$attributes->setAttribute('permissions', 'download', false);
2108-
$share->setAttributes($attributes);
2099+
2100+
if ($node->getStorage()->instanceOfStorage(SharedStorage::class)) {
2101+
$storage = $node->getStorage();
2102+
if ($storage instanceof Wrapper) {
2103+
$storage = $storage->getInstanceOfStorage(SharedStorage::class);
2104+
if ($storage === null) {
2105+
throw new \RuntimeException('Should not happen, instanceOfStorage but getInstanceOfStorage return null');
2106+
}
2107+
} else {
2108+
throw new \RuntimeException('Should not happen, instanceOfStorage but not a wrapper');
21092109
}
2110+
2111+
/** @var SharedStorage $storage */
2112+
$originalShare = $storage->getShare();
2113+
$inheritedAttributes = $originalShare->getAttributes();
2114+
// hide if hidden and also the current share enforces hide (can only be false if one share is false or user is owner)
2115+
$hideDownload = $hideDownload && $originalShare->getHideDownload();
2116+
// allow download if already allowed by previous share or when the current share allows downloading
2117+
$canDownload = $canDownload || $inheritedAttributes === null || $inheritedAttributes->getAttribute('permissions', 'download') !== false;
2118+
}
2119+
}
2120+
2121+
if ($hideDownload || !$canDownload) {
2122+
$share->setHideDownload(true);
2123+
2124+
if (!$canDownload) {
2125+
$attributes = $share->getAttributes() ?? $share->newAttributes();
2126+
$attributes->setAttribute('permissions', 'download', false);
2127+
$share->setAttributes($attributes);
21102128
}
21112129
}
21122130
}

0 commit comments

Comments
 (0)