Skip to content

Commit e0e7c2d

Browse files
susnuxsusnux
authored
Merge pull request #54815 from nextcloud/backport/54801/stable31
[stable31] fix(dav): ensure moving or copying a file is possible
2 parents f188935 + bd397ae commit e0e7c2d

File tree

2 files changed

+158
-1
lines changed

2 files changed

+158
-1
lines changed

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

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,16 @@
88
namespace OCA\DAV\Connector\Sabre;
99

1010
use OC\Share20\Exception\BackendError;
11+
use OCA\DAV\Connector\Sabre\Exception\Forbidden;
1112
use OCA\DAV\Connector\Sabre\Node as DavNode;
1213
use OCP\Files\Folder;
1314
use OCP\Files\Node;
1415
use OCP\Files\NotFoundException;
16+
use OCP\Files\Storage\ISharedStorage;
1517
use OCP\IUserSession;
1618
use OCP\Share\IManager;
1719
use OCP\Share\IShare;
20+
use Sabre\DAV\Exception\NotFound;
1821
use Sabre\DAV\PropFind;
1922
use Sabre\DAV\Server;
2023
use Sabre\DAV\Tree;
@@ -67,7 +70,9 @@ public function initialize(Server $server) {
6770
$server->protectedProperties[] = self::SHAREES_PROPERTYNAME;
6871

6972
$this->server = $server;
70-
$this->server->on('propFind', [$this, 'handleGetProperties']);
73+
$this->server->on('propFind', $this->handleGetProperties(...));
74+
$this->server->on('beforeCopy', $this->validateMoveOrCopy(...));
75+
$this->server->on('beforeMove', $this->validateMoveOrCopy(...));
7176
}
7277

7378
/**
@@ -204,4 +209,49 @@ public function handleGetProperties(
204209
return new ShareeList($shares);
205210
});
206211
}
212+
213+
/**
214+
* Ensure that when copying or moving a node it is not transferred from one share to another,
215+
* if the user is neither the owner nor has re-share permissions.
216+
* For share creation we already ensure this in the share manager.
217+
*/
218+
public function validateMoveOrCopy(string $source, string $target): bool {
219+
try {
220+
$targetNode = $this->tree->getNodeForPath($target);
221+
} catch (NotFound) {
222+
[$targetPath,] = \Sabre\Uri\split($target);
223+
$targetNode = $this->tree->getNodeForPath($targetPath);
224+
}
225+
226+
$sourceNode = $this->tree->getNodeForPath($source);
227+
if ((!$sourceNode instanceof DavNode) || (!$targetNode instanceof DavNode)) {
228+
return true;
229+
}
230+
231+
$sourceNode = $sourceNode->getNode();
232+
if ($sourceNode->isShareable()) {
233+
return true;
234+
}
235+
236+
$targetShares = $this->getShare($targetNode->getNode());
237+
if (empty($targetShares)) {
238+
// Target is not a share so no re-sharing inprogress
239+
return true;
240+
}
241+
242+
$sourceStorage = $sourceNode->getStorage();
243+
if ($sourceStorage->instanceOfStorage(ISharedStorage::class)) {
244+
// source is also a share - check if it is the same share
245+
246+
/** @var ISharedStorage $sourceStorage */
247+
$sourceShare = $sourceStorage->getShare();
248+
foreach ($targetShares as $targetShare) {
249+
if ($targetShare->getId() === $sourceShare->getId()) {
250+
return true;
251+
}
252+
}
253+
}
254+
255+
throw new Forbidden('You cannot move a non-shareable node into a share');
256+
}
207257
}

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

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

0 commit comments

Comments
 (0)