From 46b495701c1949ee13f4893cb5c23cb08326f372 Mon Sep 17 00:00:00 2001 From: Git'Fellow <12234510+solracsf@users.noreply.github.com> Date: Fri, 10 Jul 2026 09:04:34 +0200 Subject: [PATCH] fix(trash): allow restoring trashed items in "no permission" folders Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com> --- lib/ACL/ACLManager.php | 12 +++++++----- lib/Folder/FolderManager.php | 2 ++ lib/Trash/TrashBackend.php | 15 ++++++++++++--- tests/Folder/FolderManagerTest.php | 15 +++++++++++++++ tests/Trash/TrashBackendTest.php | 29 +++++++++++++++++++++++++++++ 5 files changed, 65 insertions(+), 8 deletions(-) diff --git a/lib/ACL/ACLManager.php b/lib/ACL/ACLManager.php index c007322cf..572bf8acd 100644 --- a/lib/ACL/ACLManager.php +++ b/lib/ACL/ACLManager.php @@ -147,11 +147,11 @@ public function getRelevantRulesForPath(int $storageId, array $paths, bool $cach return $this->getRules($storageId, array_keys($allPaths), $cache); } - public function getACLPermissionsForPath(int $folderId, int $storageId, string $path, string $basePath = ''): int { + public function getACLPermissionsForPath(int $folderId, int $storageId, string $path, string $basePath = '', ?int $baseOverride = null): int { $path = ltrim($path, '/'); $rules = $this->getRules($storageId, $this->getRelevantPaths($path, $basePath)); - return $this->calculatePermissionsForPath($folderId, $rules); + return $this->calculatePermissionsForPath($folderId, $rules, $baseOverride); } /** @@ -187,8 +187,10 @@ public function getPermissionsForPathFromRules(int $folderId, string $path, arra /** * @param array $rules list of rules per path, sorted parent first + * @param ?int $baseOverride start from this permission set instead of the folder's default base permission */ - private function calculatePermissionsForPath(int $folderId, array $rules): int { + private function calculatePermissionsForPath(int $folderId, array $rules, ?int $baseOverride = null): int { + $basePermission = $baseOverride ?? $this->getBasePermission($folderId); // given the following rules // // | Folder Rule | Read | Update | Share | Delete | @@ -224,14 +226,14 @@ private function calculatePermissionsForPath(int $folderId, array $rules): int { $mergedRule = Rule::mergeRules($rulesPerMapping); - return $mergedRule->applyPermissions($this->getBasePermission($folderId)); + return $mergedRule->applyPermissions($basePermission); } else { // first combine all rules with the same path, then apply them on top of the current permissions // since $rules is sorted parent first rules for subfolders overwrite the rules from the parent return array_reduce($rules, function (int $permissions, array $rules): int { $mergedRule = Rule::mergeRules($rules); return $mergedRule->applyPermissions($permissions); - }, $this->getBasePermission($folderId)); + }, $basePermission); } } diff --git a/lib/Folder/FolderManager.php b/lib/Folder/FolderManager.php index 332027819..c62e5f217 100644 --- a/lib/Folder/FolderManager.php +++ b/lib/Folder/FolderManager.php @@ -836,6 +836,8 @@ public function createFolder(string $mountPoint, array $options = [], bool $aclD ->values([ 'mount_point' => $query->createNamedParameter($mountPoint), 'quota' => self::SPACE_DEFAULT, + // The deny-by-default flag is an ACL concept, so enabling it enables ACL. + 'acl' => $query->createNamedParameter((int)$aclDefaultNoPermission, IQueryBuilder::PARAM_INT), 'acl_default_no_permission' => $query->createNamedParameter($aclDefaultNoPermission, IQueryBuilder::PARAM_BOOL), 'options' => $query->createNamedParameter(json_encode([ 'separate-storage' => $seperateStorage, diff --git a/lib/Trash/TrashBackend.php b/lib/Trash/TrashBackend.php index 6318b0c35..9ab1e792d 100644 --- a/lib/Trash/TrashBackend.php +++ b/lib/Trash/TrashBackend.php @@ -330,18 +330,27 @@ private function userHasAccessToItem( int $permission = Constants::PERMISSION_READ, string $pathInsideItem = '', ): bool { + // Without ACL there are no per-path restrictions to enforce; access to + // the trash item follows the folder-level permission checks + if (!$item->folder->acl) { + return true; + } + try { $aclManager = $this->aclManagerFactory->getACLManager($item->getUser()); - $trashPath = $this->getUnJailedPath($item->getTrashNode()) . $pathInsideItem; - $activePermissions = $aclManager->getACLPermissionsForPath($item->folder->id, $item->getGroupTrashFolderStorageId(), $trashPath); + // Reconstruct the permissions the item would have at its original + // location as a single evaluation: the folder base and the rules of + // the original parent tree first, then the item's own rules on top. $originalPath = $item->folder->rootCacheEntry->getPath() . '/' . $item->getInternalOriginalLocation() . $pathInsideItem; $originalLocationPermissions = $aclManager->getACLPermissionsForPath($item->folder->id, $item->getGroupFolderStorageId(), $originalPath); + $trashPath = $this->getUnJailedPath($item->getTrashNode()) . $pathInsideItem; + $permissions = $aclManager->getACLPermissionsForPath($item->folder->id, $item->getGroupTrashFolderStorageId(), $trashPath, baseOverride: $originalLocationPermissions); } catch (\Exception $e) { $this->logger->warning("Failed to get permissions for {$item->getPath()}", ['exception' => $e]); return false; } - return (bool)($activePermissions & $permission & $originalLocationPermissions); + return (bool)($permissions & $permission); } private function getNodeForTrashItem(IUser $user, ITrashItem $trashItem): ?Node { diff --git a/tests/Folder/FolderManagerTest.php b/tests/Folder/FolderManagerTest.php index 983c39b48..fe025a986 100644 --- a/tests/Folder/FolderManagerTest.php +++ b/tests/Folder/FolderManagerTest.php @@ -122,6 +122,21 @@ public function testCreateFolder(): void { ]); } + public function testCreateFolderWithAclDefaultNoPermissionEnablesAcl(): void { + $this->config->expects($this->any()) + ->method('getSystemValueInt') + ->with('groupfolders.quota.default', FileInfo::SPACE_UNLIMITED) + ->willReturn(FileInfo::SPACE_UNLIMITED); + + $folderId = $this->manager->createFolder('foo', [], true); + + $folder = $this->manager->getFolder($folderId); + $this->assertNotNull($folder); + // The deny-by-default flag is meaningless without ACL, so it enables ACL. + $this->assertTrue($folder->acl); + $this->assertTrue($folder->aclDefaultNoPermission); + } + public function testSetMountpoint(): void { $this->config->expects($this->any()) ->method('getSystemValueInt') diff --git a/tests/Trash/TrashBackendTest.php b/tests/Trash/TrashBackendTest.php index 52300a863..272f74087 100644 --- a/tests/Trash/TrashBackendTest.php +++ b/tests/Trash/TrashBackendTest.php @@ -366,4 +366,33 @@ public function testRestoreNoOriginalLocation(): void { $this->trashBackend->restoreItem($trashItem); $this->assertTrue($folder->nodeExists('sub')); } + + public function testRestoreWithAclDefaultNoPermissionWhileAclDisabled(): void { + $folderId = $this->folderManager->createFolder('flag-without-acl'); + $this->folderManager->addApplicableGroup($folderId, 'gf_normal'); + + // Reproduce the inconsistent state older versions allowed at creation + // (flag set while ACL disabled); newer versions prevent it, but existing + // installations still have such folders. + $query = Server::get(IDBConnection::class)->getQueryBuilder(); + $query->update('group_folders') + ->set('acl_default_no_permission', $query->createNamedParameter(true, IQueryBuilder::PARAM_BOOL)) + ->where($query->expr()->eq('folder_id', $query->createNamedParameter($folderId, IQueryBuilder::PARAM_INT))) + ->executeStatement(); + + $this->loginAsUser('normal'); + $normalUserFolder = Server::get(IRootFolder::class)->getUserFolder('normal'); + $file = $normalUserFolder->newFile('flag-without-acl/hello.txt', 'content'); + $this->trashBackend->moveToTrash($file->getStorage(), $file->getInternalPath()); + $this->assertFalse($normalUserFolder->nodeExists('flag-without-acl/hello.txt')); + + $trashItems = $this->trashBackend->listTrashRoot($this->normalUser); + $this->assertCount(1, $trashItems); + + $this->trashBackend->restoreItem($trashItems[0]); + $this->assertTrue($normalUserFolder->nodeExists('flag-without-acl/hello.txt')); + + $this->logout(); + $this->folderManager->removeFolder($folderId); + } }