diff --git a/library/Icingadb/Common/Auth.php b/library/Icingadb/Common/Auth.php index c1d12906..46bcdda8 100644 --- a/library/Icingadb/Common/Auth.php +++ b/library/Icingadb/Common/Auth.php @@ -4,6 +4,7 @@ namespace Icinga\Module\Icingadb\Common; +use Icinga\Application\Logger; use Icinga\Authentication\Auth as IcingaAuth; use Icinga\Exception\ConfigurationError; use Icinga\Module\Icingadb\Authentication\ObjectAuthorization; @@ -16,6 +17,7 @@ use ipl\Orm\UnionQuery; use ipl\Sql\Expression; use ipl\Stdlib\Filter; use ipl\Web\Filter\QueryString; +use ipl\Web\Filter\Renderer; trait Auth { @@ -179,11 +181,12 @@ trait Auth if ($query instanceof UnionQuery) { $queries = $query->getUnions(); + $forceGroupFilterOptimization = true; } else { $queries = [$query]; + $forceGroupFilterOptimization = false; } - $orgQuery = $query; foreach ($queries as $query) { $relations = [$query->getModel()->getTableName()]; foreach ($query->getWith() as $relationPath => $relation) { @@ -206,14 +209,14 @@ trait Auth $resolver = $query->getResolver(); $queryFilter = Filter::any(); - $forbiddenVars = Filter::all(); + $forbiddenVars = Filter::none(); $obfuscationRules = Filter::all(); foreach ($this->getAuth()->getUser()->getRoles() as $role) { $roleFilter = Filter::all(); if ($customVarRelationName !== false) { if (($restriction = $role->getRestrictions('icingadb/denylist/variables'))) { - $forbiddenVars->add($this->parseDenylist( + $this->flattenSemanticallyEqualRules($forbiddenVars, $this->parseDenylist( $restriction, $customVarRelationName ? $resolver->qualifyColumn('flatname', $customVarRelationName) @@ -248,7 +251,7 @@ trait Auth if ($applyHostRestriction && ($restriction = $role->getRestrictions('icingadb/filter/hosts'))) { $hostFilter = $this->parseRestriction($restriction, 'icingadb/filter/hosts'); - if ($orgQuery instanceof UnionQuery) { + if ($forceGroupFilterOptimization) { $this->forceQueryOptimization($hostFilter, 'hostgroup.name'); } @@ -264,7 +267,7 @@ trait Auth && ($restriction = $role->getRestrictions('icingadb/filter/services')) ) { $serviceFilter = $this->parseRestriction($restriction, 'icingadb/filter/services'); - if ($orgQuery instanceof UnionQuery) { + if ($forceGroupFilterOptimization) { $this->forceQueryOptimization($serviceFilter, 'servicegroup.name'); } @@ -273,7 +276,19 @@ trait Auth } if (! $roleFilter->isEmpty()) { - $queryFilter->add($roleFilter); + if (Logger::getInstance()->getLevel() === Logger::DEBUG) { + Logger::debug( + 'Preparing restrictions for user %s with role %s: %s; Current query filter: %s', + $this->getAuth()->getUser()->getUsername(), + $role->getName(), + (new Renderer($roleFilter))->setStrict()->render(), + (new Renderer($queryFilter))->setStrict()->render() + ); + } + + if (! $this->flattenSemanticallyEqualRules($queryFilter, $roleFilter)) { + $queryFilter->add($roleFilter); + } } } @@ -356,6 +371,14 @@ trait Auth } } + if (Logger::getInstance()->getLevel() === Logger::DEBUG) { + Logger::debug( + 'Final restrictions for user %s: %s', + $this->getAuth()->getUser()->getUsername(), + (new Renderer($queryFilter))->setStrict()->render() + ); + } + $query->filter($queryFilter) ->filter($forbiddenVars); } @@ -447,6 +470,51 @@ trait Auth return $filter; } + /** + * Flatten the given rule into the given chain if they are semantically equal + * + * This is needed as the same filter may be added multiple times to the same query, which leads to multiple nested + * chains of the same type. This method flattens those chains into a single one, which allows for better + * optimization and therefore enhanced efficiency. + * + * @param Filter\Chain $to + * @param Filter\Chain $from + * + * @return bool Whether the $from rule has been added to the $to chain + */ + private function flattenSemanticallyEqualRules(Filter\Chain $to, Filter\Chain $from): bool + { + $transfer = $from instanceof $to || (! $from instanceof Filter\None && $from->count() === 1); + foreach (iterator_to_array($from) as $rule) { + if ($rule instanceof Filter\Chain) { + if ($transfer) { + if (! $this->flattenSemanticallyEqualRules($to, $rule)) { + $to->add($rule); + } + + $from->remove($rule); + } elseif ($this->flattenSemanticallyEqualRules($from, $rule)) { + $from->remove($rule); + } + } elseif ($transfer) { + $to->add($rule); + $from->remove($rule); + } + } + + if (! $to->has($from)) { + if (! $from->isEmpty()) { + $to->add($from); + } + + return true; + } elseif ($from->isEmpty()) { + $to->remove($from); + } + + return false; + } + /** * Force query optimization on the given service/host filter rule * diff --git a/test/php/library/Icingadb/Common/AuthTest.php b/test/php/library/Icingadb/Common/AuthTest.php new file mode 100644 index 00000000..2b13d946 --- /dev/null +++ b/test/php/library/Icingadb/Common/AuthTest.php @@ -0,0 +1,201 @@ +flattenSemanticallyEqualRules($to, $from); + + $this->assertFilterEquals( + '(a=1|b=2|c=3|d=4|e=5|f=6)', + $to + ); + } + + public function testFlatteningOfNestedAllRules(): void + { + $to = Filter::all(); + $from = Filter::all( + Filter::equal('a', 1), + Filter::equal('b', 2), + Filter::all( + Filter::equal('c', 3), + Filter::equal('d', 4), + Filter::all( + Filter::equal('e', 5), + Filter::equal('f', 6) + ) + ) + ); + + $this->flattenSemanticallyEqualRules($to, $from); + + $this->assertFilterEquals( + '(a=1&b=2&c=3&d=4&e=5&f=6)', + $to + ); + } + + public function testFlatteningOfNestedNoneRules(): void + { + $to = Filter::none(); + $from = Filter::none( + Filter::equal('a', 1), + Filter::equal('b', 2), + Filter::none( + Filter::equal('c', 3), + Filter::equal('d', 4), + Filter::none( + Filter::equal('e', 5), + Filter::equal('f', 6) + ) + ) + ); + + $this->flattenSemanticallyEqualRules($to, $from); + + $this->assertFilterEquals( + '!(a=1|b=2|c=3|d=4|e=5|f=6)', + $to + ); + } + + public function testFlatteningOfNestedMixedRules(): void + { + $to = Filter::any(); + $from = Filter::any( + Filter::equal('a', 1), + Filter::all( + Filter::equal('b', 2), + Filter::equal('c', 3) + ), + Filter::none( + Filter::equal('d', 4), + Filter::equal('e', 5) + ), + Filter::any( + Filter::equal('f', 6), + Filter::equal('g', 7), + Filter::none( + Filter::none( + Filter::equal('h', 8), + Filter::equal('i', 9) + ) + ) + ) + ); + + $this->flattenSemanticallyEqualRules($to, $from); + + $this->assertFilterEquals( + '(a=1|(b=2&c=3)|!(d=4|e=5)|f=6|g=7|!(h=8|i=9))', + $to + ); + } + + public function testFlatteningOfEdgeCases(): void + { + $to = Filter::any(); + $from = Filter::any( + Filter::all( + Filter::equal('a', 1) + ), + Filter::none( + Filter::equal('b', 2) + ), + Filter::any( + Filter::equal('c', 3) + ), + Filter::equal('d', 4) + ); + + $this->flattenSemanticallyEqualRules($to, $from); + + $this->assertFilterEquals( + '(a=1|!(b=2)|c=3|d=4)', + $to + ); + } + + public function testFlatteningOfSemanticallyUnequalRules(): void + { + $to = Filter::any(); + $from = Filter::all( + Filter::equal('a', 1), + Filter::equal('b', 2), + Filter::any( + Filter::equal('c', 3), + Filter::equal('d', 4), + Filter::any( + Filter::equal('e', 5), + Filter::equal('f', 6) + ) + ), + Filter::all( + Filter::equal('g', 7), + ) + ); + + $this->flattenSemanticallyEqualRules($to, $from); + + $this->assertFilterEquals( + '((a=1&b=2&(c=3|d=4|e=5|f=6)&g=7)|)', + $to + ); + + $to = Filter::all(); + $from = Filter::any( + Filter::equal('a', 1), + Filter::all( + Filter::equal('b', 2), + Filter::equal('c', 3) + ), + Filter::none( + Filter::equal('d', 4), + Filter::equal('e', 5) + ), + Filter::any( + Filter::equal('f', 6), + Filter::equal('g', 7) + ) + ); + + $this->flattenSemanticallyEqualRules($to, $from); + + $this->assertFilterEquals( + '((a=1|(b=2&c=3)|!(d=4|e=5)|f=6|g=7))', + $to + ); + } + + private function assertFilterEquals(string $expected, Filter\Rule $actual): void + { + $this->assertEquals( + $expected, + (new Renderer($actual))->setStrict()->render() + ); + } +}