Skip to content

Commit

Permalink
Merge pull request #3076 from nextcloud/more-acl-trashbin-fixes
Browse files Browse the repository at this point in the history
More acl trashbin fixes
  • Loading branch information
icewind1991 authored Jul 25, 2024
2 parents 576f85e + f7638c0 commit 4ca7326
Show file tree
Hide file tree
Showing 2 changed files with 240 additions and 3 deletions.
45 changes: 42 additions & 3 deletions lib/Trash/TrashBackend.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
use OCA\GroupFolders\Mount\MountProvider;
use OCA\GroupFolders\Versions\VersionsBackend;
use OCP\Constants;
use OCP\Files\Cache\ICacheEntry;
use OCP\Files\Folder;
use OCP\Files\IRootFolder;
use OCP\Files\Node;
Expand Down Expand Up @@ -309,6 +310,8 @@ private function getTrashFolder(int $folderId): Folder {
}

/**
* @param IUser $user
* @param array{folder_id: int, mount_point: string, permissions: int, quota: int, acl: bool, rootCacheEntry: ?ICacheEntry}[] $folders
* @return list<ITrashItem>
*/
private function getTrashForFolders(IUser $user, array $folders): array {
Expand All @@ -317,29 +320,51 @@ private function getTrashForFolders(IUser $user, array $folders): array {
}, $folders);
$rows = $this->trashManager->listTrashForFolders($folderIds);
$indexedRows = [];
$trashItemsByOriginalLocation = [];
foreach ($rows as $row) {
$key = $row['folder_id'] . '/' . $row['name'] . '/' . $row['deleted_time'];
$indexedRows[$key] = $row;
$trashItemsByOriginalLocation[$row['original_location']] = $row;
}
$items = [];
foreach ($folders as $folder) {
$folderId = $folder['folder_id'];
$folderHasAcl = $folder['acl'];
$mountPoint = $folder['mount_point'];
$trashFolder = $this->getTrashFolder($folderId);
$content = $trashFolder->getDirectoryListing();
$userCanManageAcl = $this->folderManager->canManageACL($folderId, $user);
$this->aclManagerFactory->getACLManager($user)->preloadRulesForFolder($trashFolder->getPath());
foreach ($content as $item) {
/** @var \OC\Files\Node\Node $item */
$pathParts = pathinfo($item->getName());
$timestamp = (int)substr($pathParts['extension'], 1);
$name = $pathParts['filename'];
$key = $folderId . '/' . $name . '/' . $timestamp;
if (!$this->userHasAccessToPath($user, $item->getPath())) {
continue;

$originalLocation = isset($indexedRows[$key]) ? $indexedRows[$key]['original_location'] : '';

if ($folderHasAcl) {
// if we for any reason lost track of the original location, hide the item for non-managers as a fail-safe
if ($originalLocation === '' && !$userCanManageAcl) {
continue;
}
if (!$this->userHasAccessToPath($user, $item->getPath())) {
continue;
}
// if a parent of the original location has also been deleted, we also need to check it based on the now-deleted parent path
foreach ($this->getParentOriginalPaths($originalLocation, $trashItemsByOriginalLocation) as $parentOriginalPath) {
$parentTrashItem = $trashItemsByOriginalLocation[$parentOriginalPath];
$relativePath = substr($originalLocation, strlen($parentOriginalPath));
$parentTrashItemPath = "__groupfolders/trash/{$parentTrashItem['folder_id']}/{$parentTrashItem['name']}.d{$parentTrashItem['deleted_time']}";
if (!$this->userHasAccessToPath($user, $parentTrashItemPath . $relativePath)) {
continue 2;
}
}
}

$info = $item->getFileInfo();
$info['name'] = $name;
$originalLocation = isset($indexedRows[$key]) ? $indexedRows[$key]['original_location'] : '';
$items[] = new GroupTrashItem(
$this,
$originalLocation,
Expand All @@ -354,6 +379,20 @@ private function getTrashForFolders(IUser $user, array $folders): array {
return $items;
}

private function getParentOriginalPaths(string $path, array $trashItemsByOriginalPath): array {
$parentPaths = [];
while ($path !== '') {
$path = dirname($path);

if ($path === '.' || $path === '/') {
break;
} elseif (isset($trashItemsByOriginalPath[$path])) {
$parentPaths[] = $path;
}
}
return $parentPaths;
}

public function getTrashNodeById(IUser $user, int $fileId): ?Node {
try {
/** @var Folder $trashFolder */
Expand Down
198 changes: 198 additions & 0 deletions tests/Trash/TrashBackendTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2024 Robin Appelman <robin@icewind.nl>
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\GroupFolders\Tests\Trash;

use OC\Files\SetupManager;
use OC\Group\Database;
use OCA\GroupFolders\ACL\ACLManager;
use OCA\GroupFolders\ACL\ACLManagerFactory;
use OCA\GroupFolders\ACL\Rule;
use OCA\GroupFolders\ACL\RuleManager;
use OCA\GroupFolders\ACL\UserMapping\UserMapping;
use OCA\GroupFolders\Folder\FolderManager;
use OCA\GroupFolders\Mount\GroupFolderStorage;
use OCA\GroupFolders\Trash\TrashBackend;
use OCP\Files\Folder;
use OCP\Files\IRootFolder;
use OCP\IUser;
use Test\TestCase;
use Test\Traits\UserTrait;

/**
* @group DB
*/
class TrashBackendTest extends TestCase {
use UserTrait;

private string $folderName;
private TrashBackend $trashBackend;
private FolderManager $folderManager;
private ACLManager $aclManager;
private RuleManager $ruleManager;
private int $folderId;
private Folder $managerUserFolder;
private Folder $normalUserFolder;
private IUser $managerUser;
private IUser $normalUser;

public function setUp(): void {
parent::setUp();

$this->folderName = "gf";
$this->managerUser = $this->createUser("manager", "test");
$this->normalUser = $this->createUser("normal", "test");

/** @var Database $groupBackend */
$groupBackend = \OC::$server->get(Database::class);
$groupBackend->createGroup("gf_manager");
$groupBackend->createGroup("gf_normal");
$groupBackend->addToGroup("manager", "gf_manager");
$groupBackend->addToGroup("normal", "gf_normal");

$this->trashBackend = \OC::$server->get(TrashBackend::class);
$this->folderManager = \OC::$server->get(FolderManager::class);
/** @var ACLManagerFactory $aclManagerFactory */
$aclManagerFactory = \OC::$server->get(ACLManagerFactory::class);
$this->aclManager = $aclManagerFactory->getACLManager($this->managerUser);
$this->ruleManager = \OC::$server->get(RuleManager::class);

$this->folderId = $this->folderManager->createFolder($this->folderName);
$this->folderManager->addApplicableGroup($this->folderId, "gf_manager");
$this->folderManager->addApplicableGroup($this->folderId, "gf_normal");
$this->folderManager->setFolderACL($this->folderId, true);
$this->folderManager->setManageACL($this->folderId, "user", "manager", true);

/** @var IRootFolder $rootFolder */
$rootFolder = \OC::$server->get(IRootFolder::class);

$this->managerUserFolder = $rootFolder->getUserFolder("manager");
$this->normalUserFolder = $rootFolder->getUserFolder("normal");

$this->assertTrue($this->managerUserFolder->nodeExists($this->folderName));
$this->assertTrue($this->normalUserFolder->nodeExists($this->folderName));

/** @var GroupFolderStorage $groupFolderStorage */
$groupFolderStorage = $this->managerUserFolder->get($this->folderName)->getStorage();
$this->assertTrue($groupFolderStorage->instanceOfStorage(GroupFolderStorage::class));
$this->assertEquals($this->folderId, $groupFolderStorage->getFolderId());

}

protected function tearDown(): void {
$this->trashBackend->cleanTrashFolder($this->folderId);
$this->folderManager->removeFolder($this->folderId);

/** @var SetupManager $setupManager */
$setupManager = \OC::$server->get(SetupManager::class);
$setupManager->tearDown();
parent::tearDown();
}


private function createNoReadRule(string $userId, int $fileId): Rule {
return new Rule(
new UserMapping("user", $userId),
$fileId,
1,
0,
);
}

public function testHideTrashItemAcl() {
$restricted = $this->managerUserFolder->newFile("{$this->folderName}/restricted.txt", "content");
$this->ruleManager->saveRule($this->createNoReadRule("normal", $restricted->getId()));

$this->assertTrue($this->managerUserFolder->nodeExists("{$this->folderName}/restricted.txt"));
$this->assertFalse($this->normalUserFolder->nodeExists("{$this->folderName}/restricted.txt"));

$this->trashBackend->moveToTrash($restricted->getStorage(), $restricted->getInternalPath());

$this->assertFalse($this->managerUserFolder->nodeExists("{$this->folderName}/restricted.txt"));
$this->assertFalse($this->normalUserFolder->nodeExists("{$this->folderName}/restricted.txt"));

// only the manager can see the deleted file
$this->assertCount(1, $this->trashBackend->listTrashRoot($this->managerUser));
$this->assertCount(0, $this->trashBackend->listTrashRoot($this->normalUser));
}

public function testHideItemInDeletedFolderAcl() {
$folder = $this->managerUserFolder->newFolder("{$this->folderName}/folder");
$folder->newFile("file.txt", "content1");
$restrictedChild = $folder->newFile("restricted.txt", "content2");
$this->ruleManager->saveRule($this->createNoReadRule("normal", $restrictedChild->getId()));

$this->assertTrue($this->managerUserFolder->nodeExists("{$this->folderName}/folder/restricted.txt"));
$this->assertFalse($this->normalUserFolder->nodeExists("{$this->folderName}/folder/restricted.txt"));

$this->trashBackend->moveToTrash($folder->getStorage(), $folder->getInternalPath());

$this->assertFalse($this->managerUserFolder->nodeExists("{$this->folderName}/folder"));
$this->assertFalse($this->normalUserFolder->nodeExists("{$this->folderName}/folder"));

// everyone can see the parent folder
$this->assertCount(1, $this->trashBackend->listTrashRoot($this->managerUser));
$managerTrashFolder = current($this->trashBackend->listTrashRoot($this->managerUser));
$this->assertCount(1, $this->trashBackend->listTrashRoot($this->normalUser));
$normalTrashFolder = current($this->trashBackend->listTrashRoot($this->normalUser));

// only the manager can see the restricted child, both can see the un-restricted child
$this->assertCount(2, $this->trashBackend->listTrashFolder($managerTrashFolder));
$this->assertCount(1, $this->trashBackend->listTrashFolder($normalTrashFolder));
}

public function testHideDeletedTrashItemInDeletedFolderAcl() {
$folder = $this->managerUserFolder->newFolder("{$this->folderName}/restricted");
$child = $folder->newFile("file.txt", "content1");
$this->ruleManager->saveRule($this->createNoReadRule("normal", $folder->getId()));

$this->assertTrue($this->managerUserFolder->nodeExists("{$this->folderName}/restricted/file.txt"));
$this->assertFalse($this->normalUserFolder->nodeExists("{$this->folderName}/restricted/file.txt"));

$this->trashBackend->moveToTrash($child->getStorage(), $child->getInternalPath());

$this->assertFalse($this->managerUserFolder->nodeExists("{$this->folderName}/restricted/file.txt"));
$this->assertFalse($this->normalUserFolder->nodeExists("{$this->folderName}/restricted/file.txt"));

// only the manager can see the deleted child
$this->assertCount(1, $this->trashBackend->listTrashRoot($this->managerUser));
$this->assertCount(0, $this->trashBackend->listTrashRoot($this->normalUser));

$this->trashBackend->moveToTrash($folder->getStorage(), $folder->getInternalPath());

// only the manager can see the deleted items
$this->assertCount(2, $this->trashBackend->listTrashRoot($this->managerUser));
$this->assertCount(0, $this->trashBackend->listTrashRoot($this->normalUser));
}

public function testHideDeletedTrashItemInDeletedParentFolderAcl() {
$parent = $this->managerUserFolder->newFolder("{$this->folderName}/parent");
$folder = $parent->newFolder("restricted");
$child = $folder->newFile("file.txt", "content1");
$this->ruleManager->saveRule($this->createNoReadRule("normal", $folder->getId()));

$this->assertTrue($this->managerUserFolder->nodeExists("{$this->folderName}/parent/restricted/file.txt"));
$this->assertFalse($this->normalUserFolder->nodeExists("{$this->folderName}/parent/restricted/file.txt"));

$this->trashBackend->moveToTrash($child->getStorage(), $child->getInternalPath());

$this->assertFalse($this->managerUserFolder->nodeExists("{$this->folderName}/restricted/file.txt"));
$this->assertFalse($this->normalUserFolder->nodeExists("{$this->folderName}/restricted/file.txt"));

// only the manager can see the deleted child
$this->assertCount(1, $this->trashBackend->listTrashRoot($this->managerUser));
$this->assertCount(0, $this->trashBackend->listTrashRoot($this->normalUser));

$this->trashBackend->moveToTrash($parent->getStorage(), $parent->getInternalPath());

// only the manager can see the deleted child, both can see the deleted parent
$this->assertCount(2, $this->trashBackend->listTrashRoot($this->managerUser));
$this->assertCount(1, $this->trashBackend->listTrashRoot($this->normalUser));
}
}

0 comments on commit 4ca7326

Please sign in to comment.