Skip to content

Commit

Permalink
fix(Trash): Fix original location for deleting shared item
Browse files Browse the repository at this point in the history
Signed-off-by: provokateurin <[email protected]>
  • Loading branch information
provokateurin committed Jan 28, 2025
1 parent 6158d31 commit f7d1505
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 1 deletion.
8 changes: 7 additions & 1 deletion lib/Trash/TrashBackend.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
71 changes: 71 additions & 0 deletions tests/Trash/TrashBackendTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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'));
}
}

0 comments on commit f7d1505

Please sign in to comment.