Skip to content

Commit b94da13

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 <[email protected]>
1 parent 7448885 commit b94da13

File tree

3 files changed

+206
-96
lines changed

3 files changed

+206
-96
lines changed

apps/files_sharing/lib/Controller/ShareAPIController.php

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

682682
$share->setSharedBy($this->currentUser);
683-
$this->checkInheritedAttributes($share);
684683

685684
if ($shareType === IShare::TYPE_USER) {
686685
// Valid user is required to share
@@ -792,6 +791,7 @@ public function createShare(
792791
}
793792

794793
$share->setShareType($shareType);
794+
$this->checkInheritedAttributes($share);
795795

796796
if ($note !== '') {
797797
$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
/**
12781277
* expiration date, password and publicUpload only make sense for link shares
@@ -1351,6 +1350,7 @@ public function updateShare(
13511350
}
13521351

13531352
try {
1353+
$this->checkInheritedAttributes($share);
13541354
$share = $this->shareManager->updateShare($share);
13551355
} catch (HintException $e) {
13561356
$code = $e->getCode() === 0 ? 403 : $e->getCode();
@@ -2055,30 +2055,48 @@ private function checkInheritedAttributes(IShare $share): void {
20552055
if (!$share->getSharedBy()) {
20562056
return; // Probably in a test
20572057
}
2058+
2059+
$canDownload = false;
2060+
$hideDownload = true;
2061+
20582062
$userFolder = $this->rootFolder->getUserFolder($share->getSharedBy());
2059-
$node = $userFolder->getFirstNodeById($share->getNodeId());
2060-
if (!$node) {
2061-
return;
2062-
}
2063-
if ($node->getStorage()->instanceOfStorage(SharedStorage::class)) {
2064-
$storage = $node->getStorage();
2065-
if ($storage instanceof Wrapper) {
2066-
$storage = $storage->getInstanceOfStorage(SharedStorage::class);
2067-
if ($storage === null) {
2068-
throw new \RuntimeException('Should not happen, instanceOfStorage but getInstanceOfStorage return null');
2069-
}
2070-
} else {
2071-
throw new \RuntimeException('Should not happen, instanceOfStorage but not a wrapper');
2063+
$nodes = $userFolder->getById($share->getNodeId());
2064+
foreach ($nodes as $node) {
2065+
// Owner always can download it - so allow it and break
2066+
if ($node->getOwner()?->getUID() === $share->getSharedBy()) {
2067+
$canDownload = true;
2068+
$hideDownload = false;
2069+
break;
20722070
}
2073-
/** @var \OCA\Files_Sharing\SharedStorage $storage */
2074-
$inheritedAttributes = $storage->getShare()->getAttributes();
2075-
if ($inheritedAttributes !== null && $inheritedAttributes->getAttribute('permissions', 'download') === false) {
2076-
$share->setHideDownload(true);
2077-
$attributes = $share->getAttributes();
2078-
if ($attributes) {
2079-
$attributes->setAttribute('permissions', 'download', false);
2080-
$share->setAttributes($attributes);
2071+
2072+
if ($node->getStorage()->instanceOfStorage(SharedStorage::class)) {
2073+
$storage = $node->getStorage();
2074+
if ($storage instanceof Wrapper) {
2075+
$storage = $storage->getInstanceOfStorage(SharedStorage::class);
2076+
if ($storage === null) {
2077+
throw new \RuntimeException('Should not happen, instanceOfStorage but getInstanceOfStorage return null');
2078+
}
2079+
} else {
2080+
throw new \RuntimeException('Should not happen, instanceOfStorage but not a wrapper');
20812081
}
2082+
2083+
/** @var SharedStorage $storage */
2084+
$originalShare = $storage->getShare();
2085+
$inheritedAttributes = $originalShare->getAttributes();
2086+
// hide if hidden and also the current share enforces hide (can only be false if one share is false or user is owner)
2087+
$hideDownload = $hideDownload && $originalShare->getHideDownload();
2088+
// allow download if already allowed by previous share or when the current share allows downloading
2089+
$canDownload = $canDownload || $inheritedAttributes === null || $inheritedAttributes->getAttribute('permissions', 'download') !== false;
2090+
}
2091+
}
2092+
2093+
if ($hideDownload || !$canDownload) {
2094+
$share->setHideDownload(true);
2095+
2096+
if (!$canDownload) {
2097+
$attributes = $share->getAttributes() ?? $share->newAttributes();
2098+
$attributes->setAttribute('permissions', 'download', false);
2099+
$share->setAttributes($attributes);
20822100
}
20832101
}
20842102

0 commit comments

Comments
 (0)