Skip to content

Commit d12ce5d

Browse files
susnuxsusnux
committed
fix(dav): ensure moving or copying a file is possible
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
1 parent 2f2049c commit d12ce5d

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\ICollection;
1922
use Sabre\DAV\PropFind;
2023
use Sabre\DAV\Server;
@@ -76,7 +79,9 @@ public function initialize(Server $server) {
7679

7780
$this->server = $server;
7881
$this->server->on('preloadCollection', $this->preloadCollection(...));
79-
$this->server->on('propFind', [$this, 'handleGetProperties']);
82+
$this->server->on('propFind', $this->handleGetProperties(...));
83+
$this->server->on('beforeCopy', $this->validateMoveOrCopy(...));
84+
$this->server->on('beforeMove', $this->validateMoveOrCopy(...));
8085
}
8186

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

‎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)