Merge pull request #3358 from nextcloud/fix/trash/empty-original-location

Author: provokateurin

lib/Trash/TrashBackend.php

@@ -23,6 +23,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;
@@ -254,7 +255,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

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'));
+	}
 }