diff --git a/cypress/e2e/groupfolders.cy.ts b/cypress/e2e/groupfolders.cy.ts index 1fed11450..c7219b381 100644 --- a/cypress/e2e/groupfolders.cy.ts +++ b/cypress/e2e/groupfolders.cy.ts @@ -114,11 +114,15 @@ describe('Groupfolders ACLs and trashbin behavior', () => { fileOrFolderDoesNotExist('subfolder1') // Delete files + cy.log('Deleting the files') cy.login(managerUser) cy.visit('/apps/files') enterFolder(groupFolderName) enterFolder('subfolder1') deleteFile('file1.txt') + cy.visit('/apps/files') + enterFolder(groupFolderName) + enterFolder('subfolder1') deleteFile('subfolder2') // User1 sees it in trash @@ -135,6 +139,7 @@ describe('Groupfolders ACLs and trashbin behavior', () => { fileOrFolderDoesNotExistInTrashbin('subfolder2') // Restore files + cy.log('Restoring the files') cy.login(managerUser) cy.visit('/apps/files/trashbin') fileOrFolderExistsInTrashbin('file1.txt') @@ -159,7 +164,7 @@ describe('Groupfolders ACLs and trashbin behavior', () => { fileOrFolderDoesNotExist('subfolder1') }) - it.skip('ACL directly on deleted folder', () => { + it('ACL directly on deleted folder', () => { // Create a subfolders and a file as manager cy.login(managerUser) cy.mkdir(managerUser, `/${groupFolderName}/subfolder1`) diff --git a/cypress/e2e/groupfoldersUtils.ts b/cypress/e2e/groupfoldersUtils.ts index 42cf61295..38b6bfbca 100644 --- a/cypress/e2e/groupfoldersUtils.ts +++ b/cypress/e2e/groupfoldersUtils.ts @@ -95,17 +95,23 @@ export function fileOrFolderDoesNotExistInTrashbin(name: string) { } export function enterFolder(name: string) { + cy.intercept({ times: 1, method: 'PROPFIND', url: `**/dav/files/**/${name}` }).as('propFindFolder') cy.get(`[data-cy-files-list] [data-cy-files-list-row-name="${name}"]`).click() + cy.wait('@propFindFolder') } export function enterFolderInTrashbin(name: string) { + cy.intercept({ times: 1, method: 'PROPFIND', url: `**/dav/trashbin/**/${name}.d*` }).as('propFindFolder') cy.get(`[data-cy-files-list] [data-cy-files-list-row-name^="${name}.d"]`).click() + cy.wait('@propFindFolder') } export function deleteFile(name: string) { + cy.intercept({ times: 1, method: 'DELETE', url: `**/dav/files/**/${name}` }).as('delete') cy.get(`[data-cy-files-list] [data-cy-files-list-row-name="${name}"] [data-cy-files-list-row-actions]`).click() cy.get(`[data-cy-files-list] [data-cy-files-list-row-action="delete"]`).scrollIntoView() cy.get(`[data-cy-files-list] [data-cy-files-list-row-action="delete"]`).click() + cy.wait('@delete') } export function restoreFile(name: string) { diff --git a/lib/ACL/ACLManager.php b/lib/ACL/ACLManager.php index 7c5115d03..44dcd03f8 100644 --- a/lib/ACL/ACLManager.php +++ b/lib/ACL/ACLManager.php @@ -24,24 +24,25 @@ namespace OCA\GroupFolders\ACL; use OC\Cache\CappedMemoryCache; +use OCA\GroupFolders\Trash\TrashManager; use OCP\Constants; use OCP\Files\IRootFolder; use OCP\IUser; class ACLManager { - private RuleManager $ruleManager; private CappedMemoryCache $ruleCache; - private IUser $user; - private ?int $rootStorageId; /** @var callable */ private $rootFolderProvider; - public function __construct(RuleManager $ruleManager, IUser $user, callable $rootFolderProvider, ?int $rootStorageId = null) { - $this->ruleManager = $ruleManager; + public function __construct( + private RuleManager $ruleManager, + private TrashManager $trashManager, + private IUser $user, + callable $rootFolderProvider, + private ?int $rootStorageId = null, + ) { $this->ruleCache = new CappedMemoryCache(); - $this->user = $user; $this->rootFolderProvider = $rootFolderProvider; - $this->rootStorageId = $rootStorageId; } private function getRootStorageId(): int { @@ -98,13 +99,31 @@ private function getRules(array $paths, bool $cache = true): array { * @return string[] */ private function getRelevantPaths(string $path): array { - $paths = [$path]; + $paths = []; + $fromTrashbin = str_starts_with($path, '__groupfolders/trash/'); + if ($fromTrashbin) { + /* Exploded path will look like ["__groupfolders", "trash", "1", "folderName.d2345678", "rest/of/the/path.txt"] */ + [,,$groupFolderId,$rootTrashedItemName] = explode('/', $path, 5); + $groupFolderId = (int)$groupFolderId; + /* Remove the date part */ + $separatorPos = strrpos($rootTrashedItemName, '.d'); + $rootTrashedItemDate = (int)substr($rootTrashedItemName, $separatorPos + 2); + $rootTrashedItemName = substr($rootTrashedItemName, 0, $separatorPos); + } while ($path !== '') { + $paths[] = $path; $path = dirname($path); + if ($fromTrashbin && ($path === '__groupfolders/trash')) { + /* We are in trash and hit the root folder, continue looking for ACLs on parent folders in original location */ + $trashItemRow = $this->trashManager->getTrashItemByFileName($groupFolderId, $rootTrashedItemName, $rootTrashedItemDate); + $path = dirname('__groupfolders/' . $groupFolderId . '/' . $trashItemRow['original_location']); + $fromTrashbin = false; + continue; + } + if ($path === '.' || $path === '/') { $path = ''; } - $paths[] = $path; } return $paths; diff --git a/lib/ACL/ACLManagerFactory.php b/lib/ACL/ACLManagerFactory.php index ac45dc1c9..805d78a3d 100644 --- a/lib/ACL/ACLManagerFactory.php +++ b/lib/ACL/ACLManagerFactory.php @@ -23,18 +23,21 @@ namespace OCA\GroupFolders\ACL; +use OCA\GroupFolders\Trash\TrashManager; use OCP\IUser; class ACLManagerFactory { - private $ruleManager; private $rootFolderProvider; - public function __construct(RuleManager $ruleManager, callable $rootFolderProvider) { - $this->ruleManager = $ruleManager; + public function __construct( + private RuleManager $ruleManager, + private TrashManager $trashManager, + callable $rootFolderProvider, + ) { $this->rootFolderProvider = $rootFolderProvider; } public function getACLManager(IUser $user, ?int $rootStorageId = null): ACLManager { - return new ACLManager($this->ruleManager, $user, $this->rootFolderProvider, $rootStorageId); + return new ACLManager($this->ruleManager, $this->trashManager, $user, $this->rootFolderProvider, $rootStorageId); } } diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index cb7e8b040..3b1d22f3a 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -219,6 +219,7 @@ public function register(IRegistrationContext $context): void { }; return new ACLManagerFactory( $c->get(RuleManager::class), + $c->get(TrashManager::class), $rootFolderProvider ); }); diff --git a/lib/Trash/TrashBackend.php b/lib/Trash/TrashBackend.php index 948fc9706..78abfe241 100644 --- a/lib/Trash/TrashBackend.php +++ b/lib/Trash/TrashBackend.php @@ -121,7 +121,7 @@ public function restoreItem(ITrashItem $item) { if ($node === null) { throw new NotFoundException(); } - if (!$this->userHasAccessToPath($item->getUser(), $folderId . '/' . $item->getOriginalLocation(), Constants::PERMISSION_UPDATE)) { + if (!$this->userHasAccessToPath($item->getUser(), $item->getPath(), Constants::PERMISSION_UPDATE)) { throw new NotPermittedException(); } $folderPermissions = $this->folderManager->getFolderPermissionsForUser($item->getUser(), (int)$folderId); @@ -191,7 +191,7 @@ public function removeItem(ITrashItem $item) { if ($node->getStorage()->unlink($node->getInternalPath()) === false) { throw new \Exception('Failed to remove item from trashbin'); } - if (!$this->userHasAccessToPath($item->getUser(), $folderId . '/' . $item->getOriginalLocation(), Constants::PERMISSION_DELETE)) { + if (!$this->userHasAccessToPath($item->getUser(), $item->getPath(), Constants::PERMISSION_DELETE)) { throw new NotPermittedException(); } @@ -253,7 +253,7 @@ private function userHasAccessToPath( int $permission = Constants::PERMISSION_READ ): bool { $activePermissions = $this->aclManagerFactory->getACLManager($user) - ->getACLPermissionsForPath('__groupfolders/' . ltrim($path, '/')); + ->getACLPermissionsForPath($path); return (bool)($activePermissions & $permission); } @@ -265,7 +265,7 @@ private function getNodeForTrashItem(IUser $user, ITrashItem $trashItem): ?Node $trashRoot = $this->getTrashFolder((int)$folderId); try { $node = $trashRoot->get($path); - if (!$this->userHasAccessToPath($user, $folderId . '/' . $trashItem->getOriginalLocation())) { + if (!$this->userHasAccessToPath($user, $trashItem->getPath())) { return null; } return $node; @@ -321,12 +321,12 @@ private function getTrashForFolders(IUser $user, array $folders): array { $timestamp = (int)substr($pathParts['extension'], 1); $name = $pathParts['filename']; $key = $folderId . '/' . $name . '/' . $timestamp; - $originalLocation = isset($indexedRows[$key]) ? $indexedRows[$key]['original_location'] : ''; - if (!$this->userHasAccessToPath($user, $folderId . '/' . $originalLocation)) { + if (!$this->userHasAccessToPath($user, $item->getPath())) { continue; } $info = $item->getFileInfo(); $info['name'] = $name; + $originalLocation = isset($indexedRows[$key]) ? $indexedRows[$key]['original_location'] : ''; $items[] = new GroupTrashItem( $this, $originalLocation, @@ -354,13 +354,7 @@ public function getTrashNodeById(IUser $user, int $fileId): ?Node { $relativePath = $trashFolder->getRelativePath($absolutePath); [, $folderId, $nameAndTime] = explode('/', $relativePath); - if (substr_count($relativePath, "/") > 2) { - // fileid is a file inside a deleted folder - $fileId = $trashFolder->get($folderId . "/" . $nameAndTime)->getId(); - } - $trashItem = $this->trashManager->getTrashItemByFileId($fileId); - $originalPath = $folderId . '/' . ($trashItem ? $trashItem['original_location'] : '/'); - if ($this->userHasAccessToFolder($user, (int)$folderId) && $this->userHasAccessToPath($user, $originalPath)) { + if ($this->userHasAccessToFolder($user, (int)$folderId) && $this->userHasAccessToPath($user, $absolutePath)) { return $trashFolder->get($relativePath); } else { return null; diff --git a/lib/Trash/TrashManager.php b/lib/Trash/TrashManager.php index 0f6ad1871..a769da298 100644 --- a/lib/Trash/TrashManager.php +++ b/lib/Trash/TrashManager.php @@ -65,6 +65,16 @@ public function getTrashItemByFileId(int $fileId): ?array { return $query->executeQuery()->fetch() ?: null; } + public function getTrashItemByFileName(int $folderId, string $name, int $deletedTime): ?array { + $query = $this->connection->getQueryBuilder(); + $query->select(['trash_id', 'name', 'deleted_time', 'original_location', 'folder_id']) + ->from('group_folders_trash') + ->where($query->expr()->eq('folder_id', $query->createNamedParameter($folderId, IQueryBuilder::PARAM_INT))) + ->andWhere($query->expr()->eq('name', $query->createNamedParameter($name))) + ->andWhere($query->expr()->eq('deleted_time', $query->createNamedParameter($deletedTime, IQueryBuilder::PARAM_INT))); + return $query->executeQuery()->fetch() ?: null; + } + public function removeItem(int $folderId, string $name, int $deletedTime): void { $query = $this->connection->getQueryBuilder(); $query->delete('group_folders_trash') diff --git a/tests/ACL/ACLManagerTest.php b/tests/ACL/ACLManagerTest.php index 5a97cdb1c..eacb23a8e 100644 --- a/tests/ACL/ACLManagerTest.php +++ b/tests/ACL/ACLManagerTest.php @@ -27,6 +27,7 @@ use OCA\GroupFolders\ACL\Rule; use OCA\GroupFolders\ACL\RuleManager; use OCA\GroupFolders\ACL\UserMapping\IUserMapping; +use OCA\GroupFolders\Trash\TrashManager; use OCP\Constants; use OCP\Files\IRootFolder; use OCP\Files\Mount\IMountPoint; @@ -34,8 +35,10 @@ use Test\TestCase; class ACLManagerTest extends TestCase { - /** @var RuleManager|\PHPUnit_Framework_MockObject_MockObject */ + /** @var RuleManager */ private $ruleManager; + /** @var TrashManager */ + private $trashManager; /** @var IUser */ private $user; /** @var ACLManager */ @@ -56,7 +59,8 @@ protected function setUp(): void { $rootFolder = $this->createMock(IRootFolder::class); $rootFolder->method('getMountPoint') ->willReturn($rootMountPoint); - $this->aclManager = new ACLManager($this->ruleManager, $this->user, function () use ($rootFolder) { + $this->trashManager = $this->createMock(TrashManager::class); + $this->aclManager = new ACLManager($this->ruleManager, $this->trashManager, $this->user, function () use ($rootFolder) { return $rootFolder; }); $this->dummyMapping = $this->createMock(IUserMapping::class); @@ -95,4 +99,32 @@ public function testGetACLPermissionsForPath() { $this->assertEquals(Constants::PERMISSION_ALL - Constants::PERMISSION_SHARE, $this->aclManager->getACLPermissionsForPath('foo/bar')); $this->assertEquals(Constants::PERMISSION_ALL, $this->aclManager->getACLPermissionsForPath('foo/bar/sub')); } + + public function testGetACLPermissionsForPathInTrashbin() { + $this->rules = [ + '__groupfolders/1' => [ + new Rule($this->dummyMapping, 10, Constants::PERMISSION_READ + Constants::PERMISSION_UPDATE, Constants::PERMISSION_READ), // read only + new Rule($this->dummyMapping, 10, Constants::PERMISSION_SHARE, 0) // deny share + ], + '__groupfolders/1/subfolder' => [ + new Rule($this->dummyMapping, 10, Constants::PERMISSION_UPDATE, Constants::PERMISSION_UPDATE) // add write + ], + '__groupfolders/trash/1/subfolder2.d1700752274' => [ + new Rule($this->dummyMapping, 10, Constants::PERMISSION_SHARE, Constants::PERMISSION_SHARE) // add share + ] + ]; + + $this->trashManager + ->expects($this->once()) + ->method('getTrashItemByFileName') + ->with(1, 'subfolder2', 1700752274) + ->willReturn([ + 'trash_id' => 3, + 'name' => 'subfolder2', + 'deleted_time' => '1700752274', + 'original_location' => 'subfolder/subfolder2', + 'folder_id' => '1', + ]); + $this->assertEquals(Constants::PERMISSION_ALL, $this->aclManager->getACLPermissionsForPath('__groupfolders/trash/1/subfolder2.d1700752274/coucou.md')); + } }