Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 70 additions & 6 deletions apps/dav/lib/Connector/Sabre/SharesPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use OCA\DAV\Connector\Sabre\Node as DavNode;
use OCP\Files\Folder;
use OCP\Files\IRootFolder;
use OCP\Files\Mount\IShareOwnerlessMount;
use OCP\Files\Node;
use OCP\Files\NotFoundException;
use OCP\Files\Storage\ISharedStorage;
Expand Down Expand Up @@ -137,13 +138,15 @@ private function getSharesForTarget(Node $node): array {
return $shares;
}

// also check the owner side
// also check outgoing shares made by the user (or anyone for IShareOwnerlessMount)
$userRoot = $this->rootFolder->getUserFolder($this->userId);
$outgoingAncestorShares = [];
while (str_starts_with($node->getPath(), $userRoot->getPath() . '/')) {
$shares = array_merge($shares, $this->getShare($node, false));
$outgoingAncestorShares[] = $this->getShare($node, false);
$node = $node->getParent();
}
return $shares;

return array_merge($shares, ...$outgoingAncestorShares);
}

/**
Expand Down Expand Up @@ -268,14 +271,24 @@ public function validateMoveOrCopy(string $source, string $target): bool {
return true;
}

$sourceStorage = $sourceNode->getStorage();
$sourceIsShare = $sourceStorage->instanceOfStorage(ISharedStorage::class);
if (!$sourceIsShare && $sourceNode->getMountPoint() instanceof IShareOwnerlessMount) {
$sourceParent = $sourceNode->getParent();
if (!$this->operationAddsShares($sourceParent, $targetNode->getNode())) {
return true;
}

throw new Forbidden('You cannot move a non-shareable node into a share');
Comment on lines +276 to +282

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this logic also apply to shares with owners?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean to drop the instanceof IShareOwnerlessMount? If so yes, with the caveat that the same check needs to be added in the $sameMount evaluation, since the shortcut there is only safe to do on ownerless mounts (because of the way shares are handled in that case).

}

$targetShares = $this->getSharesForTarget($targetNode->getNode());
if ($targetShares === []) {
// Target is not a share so no re-sharing inprogress
// Target is not a share so no re-sharing in progress
return true;
}

$sourceStorage = $sourceNode->getStorage();
if ($sourceStorage->instanceOfStorage(ISharedStorage::class)) {
if ($sourceIsShare) {
// source is also a share - check if it is the same share

/** @var ISharedStorage $sourceStorage */
Expand All @@ -297,4 +310,55 @@ public function validateMoveOrCopy(string $source, string $target): bool {

throw new Forbidden('You cannot move a non-shareable node into a share');
}

/**
* Whether moving or copying a node from $sourceParent into $targetNode would expose it to recipients
* that cannot already see it.
*
* Walks the target and its ancestors up to the target's mount root, or the user root if it comes
* first, since a share is a jail over a single storage subtree.
* When target and source are within the same mount the walk ends at the lowest ancestor that is or contains
* $sourceParent: any share at or above it covers the source too, so its recipients can already see
* the node where it is now.
*
* NOTE: shares are only looked up on the target and its ancestors. Manager::getSharesBy() returns every share
* on a path only for share-ownerless mounts and otherwise filters by initiator, so the same-mount shortcut is
* only safe on share-ownerless mounts.
*/
private function operationAddsShares(Node $sourceParent, Node $targetNode): bool {
$targetMountPoint = $targetNode->getMountPoint();
$sameMount = $sourceParent->getMountPoint()->getMountPoint() === $targetMountPoint->getMountPoint();
$sourceParentPath = $sourceParent->getPath();
// on share-ownerless mounts the shares of a node already include the ones received by the user
$includeIncoming = !($targetMountPoint instanceof IShareOwnerlessMount);

foreach ($this->getNodeAndAncestorsInMount($targetNode) as $node) {
$path = $node->getPath();
if ($sameMount && ($sourceParentPath === $path || str_starts_with($sourceParentPath, $path . '/'))) {
// shares from here up cover the source already
return false;
}

if ($this->getShare($node, $includeIncoming) !== []) {
return true;
}
}

return false;
}

/**
* @return \Generator<Node> $node and its ancestors, up to the root of the mount of $node or the user folder
*/
private function getNodeAndAncestorsInMount(Node $node): \Generator {
$mountRoot = rtrim($node->getMountPoint()->getMountPoint(), '/');
$userRootPath = $this->rootFolder->getUserFolder($this->userId)->getPath();
while (str_starts_with($node->getPath(), $userRootPath . '/')) {
yield $node;
if ($node->getPath() === $mountRoot) {
return;
}
$node = $node->getParent();
}
}
}
Loading
Loading