From 2dd4d19ddc4abe35bafa4c46157b8b22dc262675 Mon Sep 17 00:00:00 2001 From: Lisa Lamplmair Date: Tue, 17 Dec 2024 11:14:34 +0100 Subject: [PATCH 1/3] [BUG] fix permission check in GridHelperService for wrong query column if type is asset --- src/Helper/GridHelperService.php | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/src/Helper/GridHelperService.php b/src/Helper/GridHelperService.php index 51ad8d82e..114c9664d 100644 --- a/src/Helper/GridHelperService.php +++ b/src/Helper/GridHelperService.php @@ -315,8 +315,8 @@ public function getFilterCondition(string $filterJson, ClassDefinition $class, ? $fieldConditions = []; foreach ($filter['value'] as $filterValue) { $brickCondition = '(' . $brickField->getFilterCondition($filterValue, $operator, - ['brickPrefix' => $brickPrefix] - ) . ' AND ' . $brickType . '.fieldname = ' . $db->quote($brickFilterField) . ')'; + ['brickPrefix' => $brickPrefix] + ) . ' AND ' . $brickType . '.fieldname = ' . $db->quote($brickFilterField) . ')'; $fieldConditions[] = $brickCondition; } @@ -325,7 +325,7 @@ public function getFilterCondition(string $filterJson, ClassDefinition $class, ? } } else { $brickCondition = '(' . $brickField->getFilterCondition($filter['value'], $operator, - ['brickPrefix' => $brickPrefix]) . ' AND ' . $brickType . '.fieldname = ' . $db->quote($brickFilterField) . ')'; + ['brickPrefix' => $brickPrefix]) . ' AND ' . $brickType . '.fieldname = ' . $db->quote($brickFilterField) . ')'; $conditionPartsFilters[] = $brickCondition; } } elseif ($field instanceof ClassDefinition\Data\UrlSlug) { @@ -926,16 +926,17 @@ public function createXlsxExportFile(FilesystemOperator $storage, string $fileHa /** * A more performant alternative to "CONCAT(`path`,`key`) LIKE $fullpath" */ - private function optimizedConcatLike(string $fullpath): string + private function optimizedConcatLike(string $fullpath, string $type = 'object'): string { $pathParts = explode('/', $fullpath); $leaf = array_pop($pathParts); $path = implode('/', $pathParts); + $queryColumn = $type === 'asset' ? '`filename`' : '`key`'; return '( - (`path` = "' . $path . '/" AND `key` = "' . $leaf . '") + (`path` = "' . $path . '/" AND ' . $queryColumn . ' = "' . $leaf . '") OR - `path` LIKE "' . $fullpath . '%" + `path` LIKE "' . $fullpath . '/%" )'; } @@ -943,18 +944,23 @@ private function optimizedConcatLike(string $fullpath): string * A more performant alternative to "CONCAT(`path`,`key`) NOT LIKE $fullpath" * Set $onlyChildren to true when you want to exclude the folder/element itself */ - private function optimizedConcatNotLike(string $fullpath, bool $onlyChildren = false): string + private function optimizedConcatNotLike( + string $fullpath, + bool $onlyChildren = false, + string $type = 'object' + ): string { $pathParts = explode('/', $fullpath); $leaf = array_pop($pathParts); $path = implode('/', $pathParts); + $queryColumn = $type === 'asset' ? '`filename`' : '`key`'; if ($onlyChildren) { return '`path` NOT LIKE "' . $fullpath . '/%"'; } return '( - (`path` != "' . $path . '/" AND `key` != "' . $leaf . '") + (`path` != "' . $path . '/" AND ' . $queryColumn . ' != "' . $leaf . '") AND `path` NOT LIKE "' . $fullpath . '/%" )'; @@ -983,27 +989,27 @@ protected function getPermittedPathsByUser(string $type, User $user): string if ($exceptionsConcat !== '') { $exceptionsConcat.= ' OR '; } - $exceptionsConcat.= $this->optimizedConcatLike($path); + $exceptionsConcat.= $this->optimizedConcatLike($path, $type); } $exceptions = ' OR (' . $exceptionsConcat . ')'; //if any allowed child is found, the current folder can be listed but its content is still blocked $onlyChildren = true; } - $forbiddenPathSql[] = $this->optimizedConcatNotLike($forbiddenPath, $onlyChildren) . $exceptions; + $forbiddenPathSql[] = $this->optimizedConcatNotLike($forbiddenPath, $onlyChildren, $type) . $exceptions; } foreach ($elementPaths['allowed'] as $allowedPaths) { - $allowedPathSql[] = $this->optimizedConcatLike($allowedPaths); + $allowedPathSql[] = $this->optimizedConcatLike($allowedPaths, $type); } // this is to avoid query error when implode is empty. // the result would be like `(((path1 OR path2) AND (not_path3 AND not_path4)))` $forbiddenAndAllowedSql = '('; - if (!empty($allowedPathSql) || !empty($forbiddenPathSql)) { + if ($allowedPathSql || $forbiddenPathSql) { $forbiddenAndAllowedSql .= '('; $forbiddenAndAllowedSql .= $allowedPathSql ? '( ' . implode(' OR ', $allowedPathSql) . ' )' : ''; - if (!empty($forbiddenPathSql)) { + if ($forbiddenPathSql) { //if $allowedPathSql "implosion" is present, we need `AND` in between $forbiddenAndAllowedSql .= $allowedPathSql ? ' AND ' : ''; $forbiddenAndAllowedSql .= implode(' AND ', $forbiddenPathSql); From 9b5af24a8e3a001cbeaec9bb3348a05a8ec9ef95 Mon Sep 17 00:00:00 2001 From: Lisa Lamplmair Date: Tue, 17 Dec 2024 11:17:49 +0100 Subject: [PATCH 2/3] add fixes for slashes and empty checks --- src/Helper/GridHelperService.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Helper/GridHelperService.php b/src/Helper/GridHelperService.php index 114c9664d..4a78487d5 100644 --- a/src/Helper/GridHelperService.php +++ b/src/Helper/GridHelperService.php @@ -936,7 +936,7 @@ private function optimizedConcatLike(string $fullpath, string $type = 'object'): return '( (`path` = "' . $path . '/" AND ' . $queryColumn . ' = "' . $leaf . '") OR - `path` LIKE "' . $fullpath . '/%" + `path` LIKE "' . $fullpath . '%" )'; } @@ -962,7 +962,7 @@ private function optimizedConcatNotLike( return '( (`path` != "' . $path . '/" AND ' . $queryColumn . ' != "' . $leaf . '") AND - `path` NOT LIKE "' . $fullpath . '/%" + `path` NOT LIKE "' . $fullpath . '%" )'; } @@ -1005,11 +1005,11 @@ protected function getPermittedPathsByUser(string $type, User $user): string // the result would be like `(((path1 OR path2) AND (not_path3 AND not_path4)))` $forbiddenAndAllowedSql = '('; - if ($allowedPathSql || $forbiddenPathSql) { + if (!empty($allowedPathSql) || !empty($forbiddenPathSql)) { $forbiddenAndAllowedSql .= '('; $forbiddenAndAllowedSql .= $allowedPathSql ? '( ' . implode(' OR ', $allowedPathSql) . ' )' : ''; - if ($forbiddenPathSql) { + if (!empty($forbiddenPathSql)) { //if $allowedPathSql "implosion" is present, we need `AND` in between $forbiddenAndAllowedSql .= $allowedPathSql ? ' AND ' : ''; $forbiddenAndAllowedSql .= implode(' AND ', $forbiddenPathSql); From ac3f184f2a79105bcffe6feef99154893349d846 Mon Sep 17 00:00:00 2001 From: Christian Fasching Date: Tue, 17 Dec 2024 14:30:06 +0100 Subject: [PATCH 3/3] revert / --- src/Helper/GridHelperService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Helper/GridHelperService.php b/src/Helper/GridHelperService.php index 4a78487d5..eff35920e 100644 --- a/src/Helper/GridHelperService.php +++ b/src/Helper/GridHelperService.php @@ -962,7 +962,7 @@ private function optimizedConcatNotLike( return '( (`path` != "' . $path . '/" AND ' . $queryColumn . ' != "' . $leaf . '") AND - `path` NOT LIKE "' . $fullpath . '%" + `path` NOT LIKE "' . $fullpath . '/%" )'; }