From f7d1505ca02b42ef800df2e76638448a91a4314d Mon Sep 17 00:00:00 2001 From: provokateurin Date: Mon, 14 Oct 2024 17:51:14 +0200 Subject: [PATCH] fix(Trash): Fix original location for deleting shared item Signed-off-by: provokateurin --- lib/Trash/TrashBackend.php | 8 +++- tests/Trash/TrashBackendTest.php | 71 ++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/lib/Trash/TrashBackend.php b/lib/Trash/TrashBackend.php index e964815f5..382359bd9 100644 --- a/lib/Trash/TrashBackend.php +++ b/lib/Trash/TrashBackend.php @@ -24,6 +24,7 @@ use OCP\Files\Node; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; +use OCP\Files\Storage\ISharedStorage; use OCP\Files\Storage\IStorage; use OCP\IUser; use OCP\IUserManager; @@ -256,7 +257,12 @@ public function moveToTrash(IStorage $storage, string $internalPath): bool { $result = $trashStorage->moveFromStorage($storage, $internalPath, $targetInternalPath); } if ($result) { - $this->trashManager->addTrashItem($folderId, $name, $time, $internalPath, $fileEntry->getId(), $user->getUID()); + $originalLocation = $internalPath; + if ($storage->instanceOfStorage(ISharedStorage::class)) { + $originalLocation = $storage->getWrapperStorage()->getUnjailedPath($originalLocation); + } + + $this->trashManager->addTrashItem($folderId, $name, $time, $originalLocation, $fileEntry->getId(), $user->getUID()); // some storage backends (object/encryption) can either already move the cache item or cause the target to be scanned // so we only conditionally do the cache move here diff --git a/tests/Trash/TrashBackendTest.php b/tests/Trash/TrashBackendTest.php index 72b1fba14..8dec722e7 100644 --- a/tests/Trash/TrashBackendTest.php +++ b/tests/Trash/TrashBackendTest.php @@ -19,9 +19,12 @@ use OCA\GroupFolders\Folder\FolderManager; use OCA\GroupFolders\Mount\GroupFolderStorage; use OCA\GroupFolders\Trash\TrashBackend; +use OCP\Constants; use OCP\Files\Folder; use OCP\Files\IRootFolder; use OCP\IUser; +use OCP\Server; +use OCP\Share; use Test\TestCase; use Test\Traits\UserTrait; @@ -213,4 +216,72 @@ public function testHideDeletedTrashItemInDeletedParentFolderAcl(): void { $this->logout(); } + + public function testWrongOriginalLocation(): void { + $shareManager = Server::get(Share\IManager::class); + + $userA = $this->createUser('A', 'test'); + $userAFolder = Server::get(IRootFolder::class)->getUserFolder('A'); + + $userB = $this->createUser('B', 'test'); + $userBFolder = Server::get(IRootFolder::class)->getUserFolder('B'); + + $groupBackend = Server::get(Database::class); + $groupBackend->createGroup('C'); + $groupBackend->addToGroup('A', 'C'); + $groupBackend->addToGroup('B', 'C'); + $this->assertCount(2, $groupBackend->usersInGroup('C')); + + $groupFolderId = $this->folderManager->createFolder('D'); + $this->folderManager->setFolderACL($groupFolderId, true); + $this->folderManager->addApplicableGroup($groupFolderId, 'C'); + $this->folderManager->setGroupPermissions($groupFolderId, 'C', Constants::PERMISSION_ALL); + $this->assertInstanceOf(Folder::class, $userAFolder->get('D')); + $this->assertInstanceOf(Folder::class, $userBFolder->get('D')); + + $this->loginAsUser('A'); + $userAFolder->newFolder('D/E/F'); + $userAFolder->newFile('D/E/F/G', 'foo'); + + $this->ruleManager->saveRule(new Rule(new UserMapping('group', 'C'), $userAFolder->get('D/E')->getId(), Constants::PERMISSION_READ, 0)); + $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)); + $this->ruleManager->saveRule(new Rule(new UserMapping('user', 'A'), $userAFolder->get('D/E/F')->getId(), Constants::PERMISSION_ALL, Constants::PERMISSION_ALL)); + + $folderShare = $shareManager->newShare(); + $folderShare->setShareType(Share\IShare::TYPE_USER); + $folderShare->setSharedWith('B'); + $folderShare->setSharedBy('A'); + $folderShare->setPermissions(Constants::PERMISSION_ALL); + $folderShare->setNode($userAFolder->get('D/E/F')); + $folderShare = $shareManager->createShare($folderShare); + $this->assertNotEmpty($folderShare->getId()); + + $fileShare = $shareManager->newShare(); + $fileShare->setShareType(Share\IShare::TYPE_USER); + $fileShare->setSharedWith('B'); + $fileShare->setSharedBy('A'); + $fileShare->setPermissions(19); + $fileShare->setNode($userAFolder->get('D/E/F/G')); + $fileShare = $shareManager->createShare($fileShare); + $this->assertNotEmpty($fileShare->getId()); + + $this->loginAsUser('B'); + $this->assertTrue($userBFolder->get('F/G')->isDeletable()); + $userBFolder->get('F/G')->delete(); + + $this->assertFalse($userAFolder->nodeExists('D/E/F/G')); + + $trashedOfUserA = $this->trashBackend->listTrashRoot($userA); + $this->assertCount(1, $trashedOfUserA); + // Ensure original location inside share is correct + $this->assertSame('D/E/F/G', $trashedOfUserA[0]->getOriginalLocation()); + + $trashedOfUserB = $this->trashBackend->listTrashRoot($userB); + // Deleting share only unshares it, so no trash here + $this->assertCount(0, $trashedOfUserB); + + // Restoring to original location works + $this->trashBackend->restoreItem($trashedOfUserA[0]); + $this->assertTrue($userAFolder->nodeExists('D/E/F/G')); + } }