Skip to content

Commit 44cefb3

Browse files
AndyScherzingerAndyScherzinger
authored
Merge pull request #54816 from nextcloud/backport/54814/stable29
[stable29] fix(dav): ensure moving or copying a file is possible
2 parents 7b72e18 + 6f189cc commit 44cefb3

File tree

2 files changed

+158
-0
lines changed

2 files changed

+158
-0
lines changed

‎apps/dav/lib/Connector/Sabre/SharesPlugin.php‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,17 @@
2929
namespace OCA\DAV\Connector\Sabre;
3030

3131
use OC\Share20\Exception\BackendError;
32+
use OCA\DAV\Connector\Sabre\Exception\Forbidden;
3233
use OCA\DAV\Connector\Sabre\Node as DavNode;
34+
use OCA\Files_Sharing\ISharedStorage;
35+
use OCA\Files_Sharing\SharedStorage;
3336
use OCP\Files\Folder;
3437
use OCP\Files\Node;
3538
use OCP\Files\NotFoundException;
3639
use OCP\IUserSession;
3740
use OCP\Share\IManager;
3841
use OCP\Share\IShare;
42+
use Sabre\DAV\Exception\NotFound;
3943
use Sabre\DAV\PropFind;
4044
use Sabre\DAV\Server;
4145
use Sabre\DAV\Tree;
@@ -89,6 +93,8 @@ public function initialize(Server $server) {
8993

9094
$this->server = $server;
9195
$this->server->on('propFind', [$this, 'handleGetProperties']);
96+
$this->server->on('beforeCopy', [$this, 'validateMoveOrCopy']);
97+
$this->server->on('beforeMove', [$this, 'validateMoveOrCopy']);
9298
}
9399

94100
/**
@@ -225,4 +231,49 @@ public function handleGetProperties(
225231
return new ShareeList($shares);
226232
});
227233
}
234+
235+
/**
236+
* Ensure that when copying or moving a node it is not transferred from one share to another,
237+
* if the user is neither the owner nor has re-share permissions.
238+
* For share creation we already ensure this in the share manager.
239+
*/
240+
public function validateMoveOrCopy(string $source, string $target): bool {
241+
try {
242+
$targetNode = $this->tree->getNodeForPath($target);
243+
} catch (NotFound) {
244+
[$targetPath,] = \Sabre\Uri\split($target);
245+
$targetNode = $this->tree->getNodeForPath($targetPath);
246+
}
247+
248+
$sourceNode = $this->tree->getNodeForPath($source);
249+
if ((!$sourceNode instanceof DavNode) || (!$targetNode instanceof DavNode)) {
250+
return true;
251+
}
252+
253+
$sourceNode = $sourceNode->getNode();
254+
if ($sourceNode->isShareable()) {
255+
return true;
256+
}
257+
258+
$targetShares = $this->getShare($targetNode->getNode());
259+
if (empty($targetShares)) {
260+
// Target is not a share so no re-sharing inprogress
261+
return true;
262+
}
263+
264+
$sourceStorage = $sourceNode->getStorage();
265+
if ($sourceStorage->instanceOfStorage(ISharedStorage::class)) {
266+
// source is also a share - check if it is the same share
267+
268+
/** @var SharedStorage $sourceStorage */
269+
$sourceShare = $sourceStorage->getShare();
270+
foreach ($targetShares as $targetShare) {
271+
if ($targetShare->getId() === $sourceShare->getId()) {
272+
return true;
273+
}
274+
}
275+
}
276+
277+
throw new Forbidden('You cannot move a non-shareable node into a share');
278+
}
228279
}

‎build/integration/sharing_features/sharing-v1-part4.feature‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,3 +180,110 @@ Scenario: publicUpload overrides permissions
180180
| uid_file_owner | user0 |
181181
| share_type | 3 |
182182
| permissions | 1 |
183+
184+
Scenario: Cannot copy files from share without share permission into other share
185+
Given user "user0" exists
186+
Given user "user1" exists
187+
Given user "user2" exists
188+
And As an "user0"
189+
And user "user0" created a folder "/share"
190+
When creating a share with
191+
| path | share |
192+
| shareType | 0 |
193+
| shareWith | user1 |
194+
| permissions | 15 |
195+
Then the HTTP status code should be "200"
196+
And the OCS status code should be "100"
197+
And User "user0" uploads file with content "test" to "/share/test.txt"
198+
And As an "user1"
199+
And user "user1" created a folder "/re-share"
200+
When creating a share with
201+
| path | re-share |
202+
| shareType | 0 |
203+
| shareWith | user2 |
204+
| permissions | 31 |
205+
Then the HTTP status code should be "200"
206+
And the OCS status code should be "100"
207+
When User "user1" copies file "/share/test.txt" to "/re-share/copytest.txt"
208+
Then the HTTP status code should be "403"
209+
210+
Scenario: Cannot move files from share without share permission into other share
211+
Given user "user0" exists
212+
Given user "user1" exists
213+
Given user "user2" exists
214+
And As an "user0"
215+
And user "user0" created a folder "/share"
216+
When creating a share with
217+
| path | share |
218+
| shareType | 0 |
219+
| shareWith | user1 |
220+
| permissions | 15 |
221+
Then the HTTP status code should be "200"
222+
And the OCS status code should be "100"
223+
And User "user0" uploads file with content "test" to "/share/test.txt"
224+
And As an "user1"
225+
And user "user1" created a folder "/re-share"
226+
When creating a share with
227+
| path | re-share |
228+
| shareType | 0 |
229+
| shareWith | user2 |
230+
| permissions | 31 |
231+
Then the HTTP status code should be "200"
232+
And the OCS status code should be "100"
233+
When User "user1" moves file "/share/test.txt" to "/re-share/movetest.txt"
234+
Then the HTTP status code should be "403"
235+
236+
Scenario: Cannot move folder containing share without share permission into other share
237+
Given user "user0" exists
238+
Given user "user1" exists
239+
Given user "user2" exists
240+
And As an "user0"
241+
And user "user0" created a folder "/share"
242+
When creating a share with
243+
| path | share |
244+
| shareType | 0 |
245+
| shareWith | user1 |
246+
| permissions | 15 |
247+
Then the HTTP status code should be "200"
248+
And the OCS status code should be "100"
249+
And User "user0" uploads file with content "test" to "/share/test.txt"
250+
And As an "user1"
251+
And user "user1" created a folder "/contains-share"
252+
When User "user1" moves file "/share" to "/contains-share/share"
253+
Then the HTTP status code should be "201"
254+
And user "user1" created a folder "/re-share"
255+
When creating a share with
256+
| path | re-share |
257+
| shareType | 0 |
258+
| shareWith | user2 |
259+
| permissions | 31 |
260+
Then the HTTP status code should be "200"
261+
And the OCS status code should be "100"
262+
When User "user1" moves file "/contains-share" to "/re-share/movetest"
263+
Then the HTTP status code should be "403"
264+
265+
Scenario: Can copy file between shares if share permissions
266+
Given user "user0" exists
267+
Given user "user1" exists
268+
Given user "user2" exists
269+
And As an "user0"
270+
And user "user0" created a folder "/share"
271+
When creating a share with
272+
| path | share |
273+
| shareType | 0 |
274+
| shareWith | user1 |
275+
| permissions | 31 |
276+
Then the HTTP status code should be "200"
277+
And the OCS status code should be "100"
278+
And User "user0" uploads file with content "test" to "/share/test.txt"
279+
And As an "user1"
280+
And user "user1" created a folder "/re-share"
281+
When creating a share with
282+
| path | re-share |
283+
| shareType | 0 |
284+
| shareWith | user2 |
285+
| permissions | 31 |
286+
Then the HTTP status code should be "200"
287+
And the OCS status code should be "100"
288+
When User "user1" copies file "/share/test.txt" to "/re-share/movetest.txt"
289+
Then the HTTP status code should be "201"

0 commit comments

Comments
 (0)