Skip to content

Commit

Permalink
[BUG] fix permission check in GridHelperService for wrong query colum…
Browse files Browse the repository at this point in the history
…n if type is asset
  • Loading branch information
la-lisa committed Dec 17, 2024
1 parent 0b7366e commit 2dd4d19
Showing 1 changed file with 19 additions and 13 deletions.
32 changes: 19 additions & 13 deletions src/Helper/GridHelperService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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) {
Expand Down Expand Up @@ -926,35 +926,41 @@ 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 . '/%"
)';
}

/**
* 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 . '/%"
)';
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 2dd4d19

Please sign in to comment.