Skip to content

Commit 6c6e823

Browse files
provokateurinbackportbot[bot]
authored andcommitted
fix(Trash): Fix original location for deleting shared item
Signed-off-by: provokateurin <[email protected]>
1 parent 9f454ff commit 6c6e823

File tree

2 files changed

+78
-1
lines changed

2 files changed

+78
-1
lines changed

lib/Trash/TrashBackend.php

+7-1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
use OCP\Files\Node;
2525
use OCP\Files\NotFoundException;
2626
use OCP\Files\NotPermittedException;
27+
use OCP\Files\Storage\ISharedStorage;
2728
use OCP\Files\Storage\IStorage;
2829
use OCP\IUser;
2930
use OCP\IUserManager;
@@ -256,7 +257,12 @@ public function moveToTrash(IStorage $storage, string $internalPath): bool {
256257
$result = $trashStorage->moveFromStorage($storage, $internalPath, $targetInternalPath);
257258
}
258259
if ($result) {
259-
$this->trashManager->addTrashItem($folderId, $name, $time, $internalPath, $fileEntry->getId(), $user->getUID());
260+
$originalLocation = $internalPath;
261+
if ($storage->instanceOfStorage(ISharedStorage::class)) {
262+
$originalLocation = $storage->getWrapperStorage()->getUnjailedPath($originalLocation);
263+
}
264+
265+
$this->trashManager->addTrashItem($folderId, $name, $time, $originalLocation, $fileEntry->getId(), $user->getUID());
260266

261267
// some storage backends (object/encryption) can either already move the cache item or cause the target to be scanned
262268
// so we only conditionally do the cache move here

tests/Trash/TrashBackendTest.php

+71
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,12 @@
1919
use OCA\GroupFolders\Folder\FolderManager;
2020
use OCA\GroupFolders\Mount\GroupFolderStorage;
2121
use OCA\GroupFolders\Trash\TrashBackend;
22+
use OCP\Constants;
2223
use OCP\Files\Folder;
2324
use OCP\Files\IRootFolder;
2425
use OCP\IUser;
26+
use OCP\Server;
27+
use OCP\Share;
2528
use Test\TestCase;
2629
use Test\Traits\UserTrait;
2730

@@ -211,4 +214,72 @@ public function testHideDeletedTrashItemInDeletedParentFolderAcl(): void {
211214

212215
$this->logout();
213216
}
217+
218+
public function testWrongOriginalLocation(): void {
219+
$shareManager = Server::get(Share\IManager::class);
220+
221+
$userA = $this->createUser('A', 'test');
222+
$userAFolder = Server::get(IRootFolder::class)->getUserFolder('A');
223+
224+
$userB = $this->createUser('B', 'test');
225+
$userBFolder = Server::get(IRootFolder::class)->getUserFolder('B');
226+
227+
$groupBackend = Server::get(Database::class);
228+
$groupBackend->createGroup('C');
229+
$groupBackend->addToGroup('A', 'C');
230+
$groupBackend->addToGroup('B', 'C');
231+
$this->assertCount(2, $groupBackend->usersInGroup('C'));
232+
233+
$groupFolderId = $this->folderManager->createFolder('D');
234+
$this->folderManager->setFolderACL($groupFolderId, true);
235+
$this->folderManager->addApplicableGroup($groupFolderId, 'C');
236+
$this->folderManager->setGroupPermissions($groupFolderId, 'C', Constants::PERMISSION_ALL);
237+
$this->assertInstanceOf(Folder::class, $userAFolder->get('D'));
238+
$this->assertInstanceOf(Folder::class, $userBFolder->get('D'));
239+
240+
$this->loginAsUser('A');
241+
$userAFolder->newFolder('D/E/F');
242+
$userAFolder->newFile('D/E/F/G', 'foo');
243+
244+
$this->ruleManager->saveRule(new Rule(new UserMapping('group', 'C'), $userAFolder->get('D/E')->getId(), Constants::PERMISSION_READ, 0));
245+
$this->ruleManager->saveRule(new Rule(new UserMapping('user', 'A'), $userAFolder->get('D/E')->getId(), Constants::PERMISSION_ALL, Constants::PERMISSION_READ | Constants::PERMISSION_UPDATE | Constants::PERMISSION_CREATE));
246+
$this->ruleManager->saveRule(new Rule(new UserMapping('user', 'A'), $userAFolder->get('D/E/F')->getId(), Constants::PERMISSION_ALL, Constants::PERMISSION_ALL));
247+
248+
$folderShare = $shareManager->newShare();
249+
$folderShare->setShareType(Share\IShare::TYPE_USER);
250+
$folderShare->setSharedWith('B');
251+
$folderShare->setSharedBy('A');
252+
$folderShare->setPermissions(Constants::PERMISSION_ALL);
253+
$folderShare->setNode($userAFolder->get('D/E/F'));
254+
$folderShare = $shareManager->createShare($folderShare);
255+
$this->assertNotEmpty($folderShare->getId());
256+
257+
$fileShare = $shareManager->newShare();
258+
$fileShare->setShareType(Share\IShare::TYPE_USER);
259+
$fileShare->setSharedWith('B');
260+
$fileShare->setSharedBy('A');
261+
$fileShare->setPermissions(19);
262+
$fileShare->setNode($userAFolder->get('D/E/F/G'));
263+
$fileShare = $shareManager->createShare($fileShare);
264+
$this->assertNotEmpty($fileShare->getId());
265+
266+
$this->loginAsUser('B');
267+
$this->assertTrue($userBFolder->get('F/G')->isDeletable());
268+
$userBFolder->get('F/G')->delete();
269+
270+
$this->assertFalse($userAFolder->nodeExists('D/E/F/G'));
271+
272+
$trashedOfUserA = $this->trashBackend->listTrashRoot($userA);
273+
$this->assertCount(1, $trashedOfUserA);
274+
// Ensure original location inside share is correct
275+
$this->assertSame('D/E/F/G', $trashedOfUserA[0]->getOriginalLocation());
276+
277+
$trashedOfUserB = $this->trashBackend->listTrashRoot($userB);
278+
// Deleting share only unshares it, so no trash here
279+
$this->assertCount(0, $trashedOfUserB);
280+
281+
// Restoring to original location works
282+
$this->trashBackend->restoreItem($trashedOfUserA[0]);
283+
$this->assertTrue($userAFolder->nodeExists('D/E/F/G'));
284+
}
214285
}

0 commit comments

Comments
 (0)