Skip to content

[stable30] fix(dav): ensure moving or copying a file is possible #54814

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 2, 2025
Merged

[stable30] fix(dav): ensure moving or copying a file is possible #54814

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
52 changes: 51 additions & 1 deletion apps/dav/lib/Connector/Sabre/SharesPlugin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,16 @@
namespace OCA\DAV\Connector\Sabre;

use OC\Share20\Exception\BackendError;
use OCA\DAV\Connector\Sabre\Exception\Forbidden;
use OCA\DAV\Connector\Sabre\Node as DavNode;
use OCP\Files\Folder;
use OCP\Files\Node;
use OCP\Files\NotFoundException;
use OCP\Files\Storage\ISharedStorage;
use OCP\IUserSession;
use OCP\Share\IManager;
use OCP\Share\IShare;
use Sabre\DAV\Exception\NotFound;
use Sabre\DAV\PropFind;
use Sabre\DAV\Server;
use Sabre\DAV\Tree;
Expand Down Expand Up @@ -67,7 +70,9 @@ public function initialize(Server $server) {
$server->protectedProperties[] = self::SHAREES_PROPERTYNAME;

$this->server = $server;
$this->server->on('propFind', [$this, 'handleGetProperties']);
$this->server->on('propFind', $this->handleGetProperties(...));
$this->server->on('beforeCopy', $this->validateMoveOrCopy(...));
$this->server->on('beforeMove', $this->validateMoveOrCopy(...));
}

/**
Expand Down Expand Up @@ -204,4 +209,49 @@ public function handleGetProperties(
return new ShareeList($shares);
});
}

/**
* Ensure that when copying or moving a node it is not transferred from one share to another,
* if the user is neither the owner nor has re-share permissions.
* For share creation we already ensure this in the share manager.
*/
public function validateMoveOrCopy(string $source, string $target): bool {
try {
$targetNode = $this->tree->getNodeForPath($target);
} catch (NotFound) {
[$targetPath,] = \Sabre\Uri\split($target);
$targetNode = $this->tree->getNodeForPath($targetPath);
}

$sourceNode = $this->tree->getNodeForPath($source);
if ((!$sourceNode instanceof DavNode) || (!$targetNode instanceof DavNode)) {
return true;
}

$sourceNode = $sourceNode->getNode();
if ($sourceNode->isShareable()) {
return true;
}

$targetShares = $this->getShare($targetNode->getNode());
if (empty($targetShares)) {
// Target is not a share so no re-sharing inprogress
return true;
}

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

/** @var ISharedStorage $sourceStorage */
$sourceShare = $sourceStorage->getShare();
foreach ($targetShares as $targetShare) {
if ($targetShare->getId() === $sourceShare->getId()) {
return true;
}
}
}

throw new Forbidden('You cannot move a non-shareable node into a share');
}
}
107 changes: 107 additions & 0 deletions build/integration/sharing_features/sharing-v1-part4.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,3 +182,110 @@ Scenario: publicUpload overrides permissions
| uid_file_owner | user0 |
| share_type | 3 |
| permissions | 1 |

Scenario: Cannot copy files from share without share permission into other share
Given user "user0" exists
Given user "user1" exists
Given user "user2" exists
And As an "user0"
And user "user0" created a folder "/share"
When creating a share with
| path | share |
| shareType | 0 |
| shareWith | user1 |
| permissions | 15 |
Then the HTTP status code should be "200"
And the OCS status code should be "100"
And User "user0" uploads file with content "test" to "/share/test.txt"
And As an "user1"
And user "user1" created a folder "/re-share"
When creating a share with
| path | re-share |
| shareType | 0 |
| shareWith | user2 |
| permissions | 31 |
Then the HTTP status code should be "200"
And the OCS status code should be "100"
When User "user1" copies file "/share/test.txt" to "/re-share/copytest.txt"
Then the HTTP status code should be "403"

Scenario: Cannot move files from share without share permission into other share
Given user "user0" exists
Given user "user1" exists
Given user "user2" exists
And As an "user0"
And user "user0" created a folder "/share"
When creating a share with
| path | share |
| shareType | 0 |
| shareWith | user1 |
| permissions | 15 |
Then the HTTP status code should be "200"
And the OCS status code should be "100"
And User "user0" uploads file with content "test" to "/share/test.txt"
And As an "user1"
And user "user1" created a folder "/re-share"
When creating a share with
| path | re-share |
| shareType | 0 |
| shareWith | user2 |
| permissions | 31 |
Then the HTTP status code should be "200"
And the OCS status code should be "100"
When User "user1" moves file "/share/test.txt" to "/re-share/movetest.txt"
Then the HTTP status code should be "403"

Scenario: Cannot move folder containing share without share permission into other share
Given user "user0" exists
Given user "user1" exists
Given user "user2" exists
And As an "user0"
And user "user0" created a folder "/share"
When creating a share with
| path | share |
| shareType | 0 |
| shareWith | user1 |
| permissions | 15 |
Then the HTTP status code should be "200"
And the OCS status code should be "100"
And User "user0" uploads file with content "test" to "/share/test.txt"
And As an "user1"
And user "user1" created a folder "/contains-share"
When User "user1" moves file "/share" to "/contains-share/share"
Then the HTTP status code should be "201"
And user "user1" created a folder "/re-share"
When creating a share with
| path | re-share |
| shareType | 0 |
| shareWith | user2 |
| permissions | 31 |
Then the HTTP status code should be "200"
And the OCS status code should be "100"
When User "user1" moves file "/contains-share" to "/re-share/movetest"
Then the HTTP status code should be "403"

Scenario: Can copy file between shares if share permissions
Given user "user0" exists
Given user "user1" exists
Given user "user2" exists
And As an "user0"
And user "user0" created a folder "/share"
When creating a share with
| path | share |
| shareType | 0 |
| shareWith | user1 |
| permissions | 31 |
Then the HTTP status code should be "200"
And the OCS status code should be "100"
And User "user0" uploads file with content "test" to "/share/test.txt"
And As an "user1"
And user "user1" created a folder "/re-share"
When creating a share with
| path | re-share |
| shareType | 0 |
| shareWith | user2 |
| permissions | 31 |
Then the HTTP status code should be "200"
And the OCS status code should be "100"
When User "user1" copies file "/share/test.txt" to "/re-share/movetest.txt"
Then the HTTP status code should be "201"
Loading