From 562d9b7ea81c1b39dd2d089cf9ef08ab409801aa Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Thu, 28 Jan 2021 12:27:23 +0100 Subject: [PATCH 01/26] configuration.php: Provide restrictions --- configuration.php | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/configuration.php b/configuration.php index 600d790a0..50386faa1 100644 --- a/configuration.php +++ b/configuration.php @@ -13,6 +13,21 @@ $this->provideSetupWizard('Icinga\Module\Icingadb\Setup\IcingaDbWizard'); + $this->provideRestriction( + 'icingadb/filter/objects', + $this->translate('Restrict access to the Icinga objects that match the filter') + ); + + $this->provideRestriction( + 'icingadb/filter/hosts', + $this->translate('Restrict access to the Icinga hosts and services that match the filter') + ); + + $this->provideRestriction( + 'icingadb/filter/services', + $this->translate('Restrict access to the Icinga services that match the filter') + ); + if (! $this::exists('monitoring')) { /** * Search urls From e500de5f00d120979dfcfbccea9261bd5b32e7d4 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Thu, 28 Jan 2021 12:28:29 +0100 Subject: [PATCH 02/26] Auth: Add method `applyRestrictions()` --- library/Icingadb/Common/Auth.php | 128 +++++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) diff --git a/library/Icingadb/Common/Auth.php b/library/Icingadb/Common/Auth.php index 613f5bcfe..c9fd3474e 100644 --- a/library/Icingadb/Common/Auth.php +++ b/library/Icingadb/Common/Auth.php @@ -4,10 +4,138 @@ namespace Icinga\Module\Icingadb\Common; +use Icinga\Exception\ConfigurationError; +use ipl\Orm\Compat\FilterProcessor; +use ipl\Orm\Query; +use ipl\Orm\UnionQuery; +use ipl\Sql\Filter\IsNull; +use ipl\Stdlib\Filter; +use ipl\Web\Filter\QueryString; + trait Auth { public function getAuth() { return \Icinga\Authentication\Auth::getInstance(); } + + /** + * Apply Icinga DB Web's restrictions depending on what is queried + * + * This will apply `icingadb/filter/objects` in any case. `icingadb/filter/services` is only + * applied to queries fetching services and `icingadb/filter/hosts` is applied to queries + * fetching either hosts or services. + * + * @param Query $query + * + * @return void + */ + public function applyRestrictions(Query $query) + { + if ($query instanceof UnionQuery) { + $queries = $query->getUnions(); + } else { + $queries = [$query]; + } + + foreach ($queries as $query) { + $relations = [$query->getModel()->getTableName()]; + foreach ($query->getWith() as $relation) { + $relations[] = $relation->getTarget()->getTableName(); + } + + $applyServiceRestriction = in_array('service', $relations, true); + $applyHostRestriction = in_array('host', $relations, true) + // Hosts and services have a special relation as a service can't exist without its host. + // Hence why the hosts restriction is also applied if only services are queried. + || $applyServiceRestriction; + + $queryFilter = Filter::any(); + foreach ($this->getAuth()->getUser()->getRoles() as $role) { + $roleFilter = Filter::all(); + + if (($restriction = $role->getRestrictions('icingadb/filter/objects'))) { + $roleFilter->add($this->parseRestriction($restriction, 'icingadb/filter/objects')); + } + + if ($applyHostRestriction && ($restriction = $role->getRestrictions('icingadb/filter/hosts'))) { + $roleFilter->add($this->parseRestriction($restriction, 'icingadb/filter/hosts')); + } + + if ($applyServiceRestriction && ($restriction = $role->getRestrictions('icingadb/filter/services'))) { + $roleFilter->add( + Filter::any( + new IsNull('service.id'), + $this->parseRestriction($restriction, 'icingadb/filter/services') + ) + ); + } + + // TODO: Should we allow full access if there's a role that doesn't define any restriction? + $queryFilter->add($roleFilter); + } + + FilterProcessor::apply($queryFilter, $query); + } + } + + /** + * Parse the given restriction + * + * @param string $queryString + * @param string $restriction The name of the restriction + * + * @return Filter\Rule + */ + protected function parseRestriction($queryString, $restriction) + { + $allowedColumns = [ + 'host.name', + 'hostgroup.name', + 'service.name', + 'servicegroup.name', + '(host|service).vars.' => function ($c) { + return preg_match('/^(?:host|service)\.vars\./i', $c); + } + ]; + + return QueryString::fromString($queryString) + ->on( + QueryString::ON_CONDITION, + function (Filter\Condition $condition) use ( + $restriction, + $queryString, + $allowedColumns + ) { + foreach ($allowedColumns as $column) { + if (is_callable($column)) { + if ($column($condition->getColumn())) { + return; + } + } elseif ($column === $condition->getColumn()) { + return; + } + } + + throw new ConfigurationError( + t( + 'Cannot apply restriction %s using the filter %s.' + . ' You can only use the following columns: %s' + ), + $restriction, + $queryString, + join( + ', ', + array_map( + function ($k, $v) { + return is_string($k) ? $k : $v; + }, + array_keys($allowedColumns), + $allowedColumns + ) + ) + ); + } + )->parse(); + } } From 989fbfe48707ec2d6b1250c9e3ae1b92c5b358be Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Thu, 28 Jan 2021 12:38:03 +0100 Subject: [PATCH 03/26] Apply our own restrictions instead of `monitoring/filter/objects` --- application/controllers/CommentController.php | 2 +- .../controllers/DowntimeController.php | 2 +- application/controllers/HealthController.php | 4 ++-- application/controllers/HostController.php | 4 ++-- .../controllers/HostgroupController.php | 4 ++-- application/controllers/ServiceController.php | 2 +- .../controllers/ServicegroupController.php | 4 ++-- application/controllers/UserController.php | 2 +- .../controllers/UsergroupController.php | 2 +- library/Icingadb/Web/Controller.php | 21 +++---------------- 10 files changed, 16 insertions(+), 31 deletions(-) diff --git a/application/controllers/CommentController.php b/application/controllers/CommentController.php index 18f6c246f..d8f6b4abb 100644 --- a/application/controllers/CommentController.php +++ b/application/controllers/CommentController.php @@ -41,7 +41,7 @@ public function init() $query->getSelectBase() ->where(['comment.name = ?' => $name]); - $this->applyMonitoringRestriction($query); + $this->applyRestrictions($query); $comment = $query->first(); if ($comment === null) { diff --git a/application/controllers/DowntimeController.php b/application/controllers/DowntimeController.php index 2a93dba85..1cdc7e95c 100644 --- a/application/controllers/DowntimeController.php +++ b/application/controllers/DowntimeController.php @@ -36,7 +36,7 @@ public function init() $query->getSelectBase() ->where(['downtime.name = ?' => $name]); - $this->applyMonitoringRestriction($query); + $this->applyRestrictions($query); $downtime = $query->first(); if ($downtime === null) { diff --git a/application/controllers/HealthController.php b/application/controllers/HealthController.php index 1004ddb7d..e16491bbc 100644 --- a/application/controllers/HealthController.php +++ b/application/controllers/HealthController.php @@ -29,8 +29,8 @@ public function indexAction() $hoststateSummary = HoststateSummary::on($db)->with('state'); $servicestateSummary = ServicestateSummary::on($db)->with('state'); - $this->applyMonitoringRestriction($hoststateSummary); - $this->applyMonitoringRestriction($servicestateSummary); + $this->applyRestrictions($hoststateSummary); + $this->applyRestrictions($servicestateSummary); yield $this->export($instance, $hoststateSummary, $servicestateSummary); diff --git a/application/controllers/HostController.php b/application/controllers/HostController.php index a64167c0c..88223bb45 100644 --- a/application/controllers/HostController.php +++ b/application/controllers/HostController.php @@ -37,7 +37,7 @@ public function init() $query->getSelectBase() ->where(['host.name = ?' => $name]); - $this->applyMonitoringRestriction($query); + $this->applyRestrictions($query); /** @var Host $host */ $host = $query->first(); @@ -203,7 +203,7 @@ public function servicesAction() ->getSelectBase() ->where(['service_host.id = ?' => $this->host->id]); - $this->applyMonitoringRestriction($services); + $this->applyRestrictions($services); $limitControl = $this->createLimitControl(); $paginationControl = $this->createPaginationControl($services); diff --git a/application/controllers/HostgroupController.php b/application/controllers/HostgroupController.php index 11f5fb256..516a1daa6 100644 --- a/application/controllers/HostgroupController.php +++ b/application/controllers/HostgroupController.php @@ -31,7 +31,7 @@ public function init() $query ); - $this->applyMonitoringRestriction($query); + $this->applyRestrictions($query); $hostgroup = $query->first(); if ($hostgroup === null) { @@ -48,7 +48,7 @@ public function indexAction() $hosts = Host::on($db)->with('state')->utilize('hostgroup'); $hosts->getSelectBase()->where(['host_hostgroup.id = ?' => $this->hostgroup->id]); - $this->applyMonitoringRestriction($hosts); + $this->applyRestrictions($hosts); $limitControl = $this->createLimitControl(); $paginationControl = $this->createPaginationControl($hosts); diff --git a/application/controllers/ServiceController.php b/application/controllers/ServiceController.php index d45d5506f..52261c221 100644 --- a/application/controllers/ServiceController.php +++ b/application/controllers/ServiceController.php @@ -41,7 +41,7 @@ public function init() ->where(['service.name = ?' => $name]) ->where(['service_host.name = ?' => $hostName]); - $this->applyMonitoringRestriction($query); + $this->applyRestrictions($query); /** @var Service $service */ $service = $query->first(); diff --git a/application/controllers/ServicegroupController.php b/application/controllers/ServicegroupController.php index 93e4167af..d5e86a281 100644 --- a/application/controllers/ServicegroupController.php +++ b/application/controllers/ServicegroupController.php @@ -31,7 +31,7 @@ public function init() $query ); - $this->applyMonitoringRestriction($query); + $this->applyRestrictions($query); $servicegroup = $query->first(); if ($servicegroup === null) { @@ -52,7 +52,7 @@ public function indexAction() ])->utilize('servicegroup'); $services->getSelectBase()->where(['service_servicegroup.id = ?' => $this->servicegroup->id]); - $this->applyMonitoringRestriction($services); + $this->applyRestrictions($services); $limitControl = $this->createLimitControl(); $paginationControl = $this->createPaginationControl($services); diff --git a/application/controllers/UserController.php b/application/controllers/UserController.php index 60d25b8cc..079dd72b4 100644 --- a/application/controllers/UserController.php +++ b/application/controllers/UserController.php @@ -30,7 +30,7 @@ public function init() $query->getSelectBase() ->where(['user.name = ?' => $name]); - $this->applyMonitoringRestriction($query); + $this->applyRestrictions($query); $user = $query->first(); if ($user === null) { diff --git a/application/controllers/UsergroupController.php b/application/controllers/UsergroupController.php index 285ea158d..40481e3e4 100644 --- a/application/controllers/UsergroupController.php +++ b/application/controllers/UsergroupController.php @@ -30,7 +30,7 @@ public function init() $query->getSelectBase() ->where(['usergroup.name = ?' => $name]); - $this->applyMonitoringRestriction($query); + $this->applyRestrictions($query); $usergroup = $query->first(); if ($usergroup === null) { diff --git a/library/Icingadb/Web/Controller.php b/library/Icingadb/Web/Controller.php index 479e84df4..65189ed1f 100644 --- a/library/Icingadb/Web/Controller.php +++ b/library/Icingadb/Web/Controller.php @@ -7,9 +7,8 @@ use Generator; use GuzzleHttp\Psr7\ServerRequest; use Icinga\Application\Icinga; +use Icinga\Module\Icingadb\Common\Auth; use Icinga\Module\Icingadb\Common\Database; -use Icinga\Module\Icingadb\Compat\MonitoringRestrictions; -use Icinga\Module\Icingadb\Compat\UrlMigrator; use Icinga\Module\Icingadb\Web\Control\SearchBar\ObjectSuggestions; use Icinga\Module\Icingadb\Widget\BaseItemList; use Icinga\Module\Icingadb\Widget\ViewModeSwitcher; @@ -31,6 +30,7 @@ class Controller extends CompatController { + use Auth; use Database; /** @var Filter Filter from query string parameters */ @@ -349,28 +349,13 @@ protected function addContent(ValidHtml $content) public function filter(Query $query, Filter\Rule $filter = null) { - $this->applyMonitoringRestriction($query); + $this->applyRestrictions($query); FilterProcessor::apply($filter ?: $this->getFilter(), $query); return $this; } - public function applyMonitoringRestriction(Query $query, $queryTransformer = null) - { - if ($queryTransformer === null || UrlMigrator::hasQueryTransformer($queryTransformer)) { - $restriction = UrlMigrator::transformFilter( - MonitoringRestrictions::getRestriction('monitoring/filter/objects'), - $queryTransformer - ); - if ($restriction) { - FilterProcessor::apply($restriction, $query); - } - } - - return $this; - } - public function preDispatch() { parent::preDispatch(); From 7ab9ae4f712a1caf05bb21b0c0aed7a5ff564816 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Thu, 28 Jan 2021 12:39:29 +0100 Subject: [PATCH 04/26] host/index: Apply restrictions to the service statistics widget --- application/controllers/HostController.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/application/controllers/HostController.php b/application/controllers/HostController.php index 88223bb45..e7313a88f 100644 --- a/application/controllers/HostController.php +++ b/application/controllers/HostController.php @@ -56,6 +56,8 @@ public function indexAction() $serviceSummary->getSelectBase() ->where(['host_id = ?' => $this->host->id]); + $this->applyRestrictions($serviceSummary); + if ($this->host->state->is_overdue) { $this->controls->addAttributes(['class' => 'overdue']); } From 1522dfa53534713777bb4e9788e8b009aab26297 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Thu, 28 Jan 2021 12:41:04 +0100 Subject: [PATCH 05/26] hosts/details: Only show host comment count Previously, also service comments were counted. --- application/controllers/HostsController.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/application/controllers/HostsController.php b/application/controllers/HostsController.php index be933a768..b254f59c8 100644 --- a/application/controllers/HostsController.php +++ b/application/controllers/HostsController.php @@ -144,6 +144,8 @@ public function detailsAction() $comments = Host::on($db)->with(['comment']); $comments->getWith()['host.comment']->setJoinType('INNER'); + // TODO: This should be automatically done by the model/resolver and added as ON condition + FilterProcessor::apply(Filter::equal('comment.object_type', 'host'), $comments); $this->filter($comments); $summary->comments_total = $comments->count(); From 5e61ddc38b06c8a7361a65f2a494bed38d831141 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Thu, 28 Jan 2021 12:54:00 +0100 Subject: [PATCH 06/26] ObjectDetail: Only show host comments and downtimes Previously also service comments and downtimes were shown. --- .../Icingadb/Widget/Detail/ObjectDetail.php | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/library/Icingadb/Widget/Detail/ObjectDetail.php b/library/Icingadb/Widget/Detail/ObjectDetail.php index 52574ccb6..2bf362f86 100644 --- a/library/Icingadb/Widget/Detail/ObjectDetail.php +++ b/library/Icingadb/Widget/Detail/ObjectDetail.php @@ -32,7 +32,9 @@ use ipl\Html\Html; use ipl\Html\HtmlElement; use ipl\Html\HtmlString; +use ipl\Orm\Compat\FilterProcessor; use ipl\Orm\ResultSet; +use ipl\Stdlib\Filter; use ipl\Web\Widget\Icon; use Zend_View_Helper_Perfdata; @@ -112,8 +114,15 @@ protected function createComments() $relations = ['service', 'service.state', 'service.host', 'service.host.state']; } + $comments = $this->object->comment + ->with($relations) + ->limit(3) + ->peekAhead(); + // TODO: This should be automatically done by the model/resolver and added as ON condition + FilterProcessor::apply(Filter::equal('object_type', $this->objectType), $comments); + + $comments = $comments->execute(); /** @var ResultSet $comments */ - $comments = $this->object->comment->with($relations)->limit(3)->peekAhead()->execute(); $content = [Html::tag('h2', t('Comments'))]; @@ -158,7 +167,14 @@ protected function createDowntimes() $link = ServiceLinks::downtimes($this->object, $this->object->host); } - $downtimes = $this->object->downtime->limit(3)->peekAhead()->execute(); + $downtimes = $this->object->downtime + ->limit(3) + ->peekAhead(); + // TODO: This should be automatically done by the model/resolver and added as ON condition + FilterProcessor::apply(Filter::equal('object_type', $this->objectType), $downtimes); + + $downtimes = $downtimes->execute(); + /** @var ResultSet $downtimes */ $content = [Html::tag('h2', t('Downtimes'))]; From 40e2950052c1404f17845aad5ea0a9e32f1c3c21 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Thu, 28 Jan 2021 12:54:57 +0100 Subject: [PATCH 07/26] hosts/detail: Filter and restrict feature status query --- application/controllers/HostsController.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/application/controllers/HostsController.php b/application/controllers/HostsController.php index b254f59c8..b5e8ef235 100644 --- a/application/controllers/HostsController.php +++ b/application/controllers/HostsController.php @@ -208,6 +208,9 @@ public function getCommandTargetsUrl() protected function getFeatureStatus() { - return new FeatureStatus('host', HoststateSummary::on($this->getDb())->with(['state'])->first()); + $summary = HoststateSummary::on($this->getDb())->with(['state']); + $this->filter($summary); + + return new FeatureStatus('host', $summary->first()); } } From 12e67204b0c2c3a61a54ed72aa21797346f2a88a Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Thu, 28 Jan 2021 12:55:18 +0100 Subject: [PATCH 08/26] services/detail: Filter and restrict feature status query --- application/controllers/ServicesController.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/application/controllers/ServicesController.php b/application/controllers/ServicesController.php index 0cfb05f03..07dd08c84 100644 --- a/application/controllers/ServicesController.php +++ b/application/controllers/ServicesController.php @@ -219,6 +219,9 @@ public function getCommandTargetsUrl() protected function getFeatureStatus() { - return new FeatureStatus('service', ServicestateSummary::on($this->getDb())->with(['state'])->first()); + $summary = ServicestateSummary::on($this->getDb())->with(['state']); + $this->filter($summary); + + return new FeatureStatus('service', $summary->first()); } } From bf8a1b65d0ecb5fe2958fc868fcb01b4300e66b8 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Thu, 28 Jan 2021 12:56:02 +0100 Subject: [PATCH 09/26] ObjectSuggestions: Restrict customvar suggestions and value suggestions --- .../Control/SearchBar/ObjectSuggestions.php | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/library/Icingadb/Web/Control/SearchBar/ObjectSuggestions.php b/library/Icingadb/Web/Control/SearchBar/ObjectSuggestions.php index 2e61b4b6c..e2955be9f 100644 --- a/library/Icingadb/Web/Control/SearchBar/ObjectSuggestions.php +++ b/library/Icingadb/Web/Control/SearchBar/ObjectSuggestions.php @@ -4,6 +4,7 @@ namespace Icinga\Module\Icingadb\Web\Control\SearchBar; +use Icinga\Module\Icingadb\Common\Auth; use Icinga\Module\Icingadb\Common\Database; use Icinga\Module\Icingadb\Model\Behavior\ReRoute; use Icinga\Module\Icingadb\Model\CustomvarFlat; @@ -24,6 +25,7 @@ class ObjectSuggestions extends Suggestions { + use Auth; use Database; /** @var Model */ @@ -160,6 +162,7 @@ protected function fetchValueSuggestions($column, $searchTerm, Filter\Chain $sea } FilterProcessor::apply($searchFilter, $query); + $this->applyRestrictions($query); try { return (new Cursor($query->getDb(), $query->assembleSelect()->distinct())) @@ -234,22 +237,19 @@ protected function queryCustomvarConfig($searchTerm) $tableName = $customVars->getModel()->getTableName(); $columns = ['flatname']; + $aggregates = ['flatname']; foreach ($customVars->getResolver()->getRelations($customVars->getModel()) as $name => $relation) { if (isset($this->customVarSources[$name]) && $relation instanceof BelongsToMany) { - $junction = $relation->getThrough(); - - $foreignKey = $customVars->getResolver()->qualifyColumn( - $customVars->getResolver()->getRelations($junction)->get($tableName)->getForeignKey(), - $junction->getTableName() - ); - $candidateKey = $customVars->getResolver()->qualifyColumn( - $relation->getCandidateKey(), - $tableName + $query = $customVars->createSubQuery( + $relation->getTarget(), + $customVars->getResolver()->qualifyPath($name, $tableName) ); - $columns[$name] = $junction::on($customVars->getDb())->assembleSelect() + $this->applyRestrictions($query); + + $aggregates[$name] = new Expression("MAX($name)"); + $columns[$name] = $query->assembleSelect() ->resetColumns()->columns(new Expression('1')) - ->where("$foreignKey = $candidateKey") ->limit(1); } } @@ -258,10 +258,11 @@ protected function queryCustomvarConfig($searchTerm) $customVars = $customVars->assembleSelect(); $customVars->resetColumns()->columns($columns); - $customVars->groupBy('flatname'); + $customVars->groupBy('id'); $customVars->limit(static::DEFAULT_LIMIT); - return $customVars; + // This outer query exists only because there's no way to combine aggregates and sub queries (yet) + return (new Select())->columns($aggregates)->from(['results' => $customVars])->groupBy('flatname'); } /** From 10e81525a0283a9d8ec19fb65cc025915dcaa908 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Thu, 28 Jan 2021 12:56:45 +0100 Subject: [PATCH 10/26] HostProblemsBadge: Apply restrictions to the count query --- .../Web/Navigation/Renderer/HostProblemsBadge.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/library/Icingadb/Web/Navigation/Renderer/HostProblemsBadge.php b/library/Icingadb/Web/Navigation/Renderer/HostProblemsBadge.php index b3ce54cd4..be062c600 100644 --- a/library/Icingadb/Web/Navigation/Renderer/HostProblemsBadge.php +++ b/library/Icingadb/Web/Navigation/Renderer/HostProblemsBadge.php @@ -4,14 +4,20 @@ namespace Icinga\Module\Icingadb\Web\Navigation\Renderer; +use Icinga\Module\Icingadb\Common\Auth; use Icinga\Module\Icingadb\Common\Links; use Icinga\Module\Icingadb\Model\HoststateSummary; class HostProblemsBadge extends ProblemsBadge { + use Auth; + protected function fetchProblemsCount() { - return HoststateSummary::on($this->getDb())->with('state')->first()->hosts_down_unhandled; + $summary = HoststateSummary::on($this->getDb())->with('state'); + $this->applyRestrictions($summary); + + return $summary->first()->hosts_down_unhandled; } protected function getUrl() From 542df951343510e8ef8e6181ebc012e0e2803352 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Thu, 28 Jan 2021 12:57:39 +0100 Subject: [PATCH 11/26] ServiceProblemsBadge: Apply restrictions to the count query --- .../Web/Navigation/Renderer/ServiceProblemsBadge.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/library/Icingadb/Web/Navigation/Renderer/ServiceProblemsBadge.php b/library/Icingadb/Web/Navigation/Renderer/ServiceProblemsBadge.php index e8fa9dc66..8006e2e01 100644 --- a/library/Icingadb/Web/Navigation/Renderer/ServiceProblemsBadge.php +++ b/library/Icingadb/Web/Navigation/Renderer/ServiceProblemsBadge.php @@ -4,14 +4,20 @@ namespace Icinga\Module\Icingadb\Web\Navigation\Renderer; +use Icinga\Module\Icingadb\Common\Auth; use Icinga\Module\Icingadb\Common\Links; use Icinga\Module\Icingadb\Model\ServicestateSummary; class ServiceProblemsBadge extends ProblemsBadge { + use Auth; + protected function fetchProblemsCount() { - return ServicestateSummary::on($this->getDb())->with('state')->first()->services_critical_unhandled; + $summary = ServicestateSummary::on($this->getDb())->with('state'); + $this->applyRestrictions($summary); + + return $summary->first()->services_critical_unhandled; } protected function getUrl() From 844be61fc24438c8be3586bfda1ecae16ce9123a Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Thu, 28 Jan 2021 12:58:19 +0100 Subject: [PATCH 12/26] ObjectDetail: Apply restrictions to hostgroup, servicegroup, user and usergroup listings --- .../Icingadb/Widget/Detail/ObjectDetail.php | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/library/Icingadb/Widget/Detail/ObjectDetail.php b/library/Icingadb/Widget/Detail/ObjectDetail.php index 2bf362f86..2e144b47c 100644 --- a/library/Icingadb/Widget/Detail/ObjectDetail.php +++ b/library/Icingadb/Widget/Detail/ObjectDetail.php @@ -193,9 +193,11 @@ protected function createGroups() $groups = [Html::tag('h2', t('Groups'))]; if ($this->objectType === 'host') { - $hostgroupList = new TagList(); + $hostgroups = $this->object->hostgroup; + $this->applyRestrictions($hostgroups); - foreach ($this->object->hostgroup as $hostgroup) { + $hostgroupList = new TagList(); + foreach ($hostgroups as $hostgroup) { $hostgroupList->addLink($hostgroup->display_name, Links::hostgroup($hostgroup)); } @@ -206,9 +208,11 @@ protected function createGroups() : new EmptyState(t('Not a member of any host group.')) ); } else { - $servicegroupList = new TagList(); + $servicegroups = $this->object->servicegroup; + $this->applyRestrictions($servicegroups); - foreach ($this->object->servicegroup as $servicegroup) { + $servicegroupList = new TagList(); + foreach ($servicegroups as $servicegroup) { $servicegroupList->addLink($servicegroup->display_name, Links::servicegroup($servicegroup)); } @@ -401,11 +405,15 @@ protected function getUsersAndUsergroups() || ! $this->getAuth()->hasPermission('no-monitoring/contacts') ) { foreach ($this->object->notification as $notification) { - foreach ($notification->user as $user) { + $recipients = $notification->user; + $this->applyRestrictions($recipients); + foreach ($recipients as $user) { $users[$user->name] = $user; } - foreach ($notification->usergroup as $usergroup) { + $recipientGroups = $notification->usergroup; + $this->applyRestrictions($recipientGroups); + foreach ($recipientGroups as $usergroup) { $usergroups[$usergroup->name] = $usergroup; } } From da2069b1bbf1cf7db431cb56b6d71497766eef39 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Thu, 28 Jan 2021 12:59:12 +0100 Subject: [PATCH 13/26] Hostgroupsummary: Fetch hostgroups in a separate query Otherwise not all hostgroups are shown if there's a host restriction active --- library/Icingadb/Model/Hostgroupsummary.php | 24 +++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/library/Icingadb/Model/Hostgroupsummary.php b/library/Icingadb/Model/Hostgroupsummary.php index b7c38fd2f..c8bce66a7 100644 --- a/library/Icingadb/Model/Hostgroupsummary.php +++ b/library/Icingadb/Model/Hostgroupsummary.php @@ -97,6 +97,22 @@ public function getDefaultSort() public function getUnions() { $unions = [ + [ + Hostgroup::class, + [], + [ + 'hostgroup_id' => 'hostgroup.id', + 'hostgroup_name' => 'hostgroup.name', + 'hostgroup_display_name' => 'hostgroup.display_name', + 'host_id' => new Expression('NULL'), + 'host_state' => new Expression('NULL'), + 'host_handled' => new Expression('NULL'), + 'host_severity' => new Expression('0'), + 'service_id' => new Expression('NULL'), + 'service_state' => new Expression('NULL'), + 'service_handled' => new Expression('NULL') + ] + ], [ Host::class, [ @@ -105,8 +121,8 @@ public function getUnions() ], [ 'hostgroup_id' => 'hostgroup.id', - 'hostgroup_name' => 'hostgroup.name', - 'hostgroup_display_name' => 'hostgroup.display_name', + 'hostgroup_name' => new Expression('NULL'), + 'hostgroup_display_name' => new Expression('NULL'), 'host_id' => 'host.id', 'host_state' => 'state.soft_state', 'host_handled' => 'state.is_handled', @@ -124,8 +140,8 @@ public function getUnions() ], [ 'hostgroup_id' => 'hostgroup.id', - 'hostgroup_name' => 'hostgroup.name', - 'hostgroup_display_name' => 'hostgroup.display_name', + 'hostgroup_name' => new Expression('NULL'), + 'hostgroup_display_name' => new Expression('NULL'), 'host_id' => new Expression('NULL'), 'host_state' => new Expression('NULL'), 'host_handled' => new Expression('NULL'), From adfeb079ba97933e93c3d87446840d18935d3d62 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Thu, 28 Jan 2021 13:00:19 +0100 Subject: [PATCH 14/26] Servicegroupsummary: Fetch servicegroups in a separate query Otherwise not all servicegroups are shown if there's a service restriction active --- library/Icingadb/Model/ServicegroupSummary.php | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/library/Icingadb/Model/ServicegroupSummary.php b/library/Icingadb/Model/ServicegroupSummary.php index 0be61015e..8ac0779f5 100644 --- a/library/Icingadb/Model/ServicegroupSummary.php +++ b/library/Icingadb/Model/ServicegroupSummary.php @@ -73,6 +73,19 @@ public function getDefaultSort() public function getUnions() { $unions = [ + [ + Servicegroup::class, + [], + [ + 'servicegroup_id' => 'servicegroup.id', + 'servicegroup_name' => 'servicegroup.name', + 'servicegroup_display_name' => 'servicegroup.display_name', + 'service_id' => new Expression('NULL'), + 'service_state' => new Expression('NULL'), + 'service_handled' => new Expression('NULL'), + 'service_severity' => new Expression('0') + ] + ], [ Service::class, [ @@ -81,8 +94,8 @@ public function getUnions() ], [ 'servicegroup_id' => 'servicegroup.id', - 'servicegroup_name' => 'servicegroup.name', - 'servicegroup_display_name' => 'servicegroup.display_name', + 'servicegroup_name' => new Expression('NULL'), + 'servicegroup_display_name' => new Expression('NULL'), 'service_id' => 'service.id', 'service_state' => 'state.soft_state', 'service_handled' => 'state.is_handled', From ddb8ac16225f4e13980e58d028c44fdc239367c4 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Thu, 28 Jan 2021 14:41:21 +0100 Subject: [PATCH 15/26] ObjectDetail: Avoid unnecessary queries when listing downtimes --- library/Icingadb/Widget/Detail/ObjectDetail.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/Icingadb/Widget/Detail/ObjectDetail.php b/library/Icingadb/Widget/Detail/ObjectDetail.php index 2e144b47c..24acc8c96 100644 --- a/library/Icingadb/Widget/Detail/ObjectDetail.php +++ b/library/Icingadb/Widget/Detail/ObjectDetail.php @@ -163,11 +163,14 @@ protected function createDowntimes() { if ($this->objectType === 'host') { $link = HostLinks::downtimes($this->object); + $relations = ['host', 'host.state']; } else { $link = ServiceLinks::downtimes($this->object, $this->object->host); + $relations = ['service', 'service.state', 'service.host', 'service.host.state']; } $downtimes = $this->object->downtime + ->with($relations) ->limit(3) ->peekAhead(); // TODO: This should be automatically done by the model/resolver and added as ON condition From da055ac5c2d90806fd1f1374bef3281f8bf67db3 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Thu, 28 Jan 2021 14:42:07 +0100 Subject: [PATCH 16/26] ObjectDetail: Fetch contacts by exactly two queries... ...and not with as much queries as there are notifications*recipients --- .../Icingadb/Widget/Detail/ObjectDetail.php | 33 ++++++++++++------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/library/Icingadb/Widget/Detail/ObjectDetail.php b/library/Icingadb/Widget/Detail/ObjectDetail.php index 24acc8c96..1cd548712 100644 --- a/library/Icingadb/Widget/Detail/ObjectDetail.php +++ b/library/Icingadb/Widget/Detail/ObjectDetail.php @@ -7,6 +7,7 @@ use Icinga\Application\Config; use Icinga\Application\Icinga; use Icinga\Module\Icingadb\Common\Auth; +use Icinga\Module\Icingadb\Common\Database; use Icinga\Module\Icingadb\Common\HostLinks; use Icinga\Module\Icingadb\Common\Icons; use Icinga\Module\Icingadb\Common\Links; @@ -17,6 +18,8 @@ use Icinga\Module\Icingadb\Compat\CustomvarFilter; use Icinga\Module\Icingadb\Forms\Command\Object\ToggleObjectFeaturesForm; use Icinga\Module\Icingadb\Model\Host; +use Icinga\Module\Icingadb\Model\User; +use Icinga\Module\Icingadb\Model\Usergroup; use Icinga\Module\Icingadb\Widget\DowntimeList; use Icinga\Module\Icingadb\Widget\EmptyState; use Icinga\Module\Icingadb\Widget\HorizontalKeyValue; @@ -41,6 +44,7 @@ class ObjectDetail extends BaseHtmlElement { use Auth; + use Database; protected $object; @@ -407,18 +411,23 @@ protected function getUsersAndUsergroups() $this->getAuth()->hasPermission('*') || ! $this->getAuth()->hasPermission('no-monitoring/contacts') ) { - foreach ($this->object->notification as $notification) { - $recipients = $notification->user; - $this->applyRestrictions($recipients); - foreach ($recipients as $user) { - $users[$user->name] = $user; - } - - $recipientGroups = $notification->usergroup; - $this->applyRestrictions($recipientGroups); - foreach ($recipientGroups as $usergroup) { - $usergroups[$usergroup->name] = $usergroup; - } + $objectFilter = Filter::equal( + 'notification.' . ($this->objectType === 'host' ? 'host_id' : 'service_id'), + $this->object->id + ); + + $userQuery = User::on($this->getDb()); + FilterProcessor::apply($objectFilter, $userQuery); + $this->applyRestrictions($userQuery); + foreach ($userQuery as $user) { + $users[$user->name] = $user; + } + + $usergroupQuery = Usergroup::on($this->getDb()); + FilterProcessor::apply($objectFilter, $usergroupQuery); + $this->applyRestrictions($usergroupQuery); + foreach ($usergroupQuery as $usergroup) { + $usergroups[$usergroup->name] = $usergroup; } } From e9d132a5597abca5423cb40427b2b9814eee02bd Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 29 Jan 2021 10:36:21 +0100 Subject: [PATCH 17/26] Drop class `MonitoringRestrictions` --- .../Compat/MonitoringRestrictions.php | 84 ------------------- 1 file changed, 84 deletions(-) delete mode 100644 library/Icingadb/Compat/MonitoringRestrictions.php diff --git a/library/Icingadb/Compat/MonitoringRestrictions.php b/library/Icingadb/Compat/MonitoringRestrictions.php deleted file mode 100644 index 78e2232e6..000000000 --- a/library/Icingadb/Compat/MonitoringRestrictions.php +++ /dev/null @@ -1,84 +0,0 @@ -getRestrictions($name); - } - - /** - * Get a restriction of the authenticated user - * - * @param string $name Name of the restriction - * - * @return Filter\Rule Filter rule - * @throws ConfigurationError If the restriction contains invalid filter columns - */ - public static function getRestriction($name) - { - $restriction = Filter::any(); - - $allowedColumns = [ - 'host_name', - 'hostgroup_name', - 'instance_name', - 'service_description', - 'servicegroup_name', - '_(host|service)_' => function ($c) { - return preg_match('/^_(?:host|service)_/i', $c); - } - ]; - - foreach (self::getRestrictions($name) as $queryString) { - if ($queryString === '*') { - return Filter::all(); - } - - $restriction->add(QueryString::fromString($queryString) - ->on(QueryString::ON_CONDITION, function (Filter\Condition $condition) use ( - $name, - $queryString, - $allowedColumns - ) { - foreach ($allowedColumns as $column) { - if (is_callable($column)) { - if ($column($condition->getColumn())) { - return; - } - } elseif ($column === $condition->getColumn()) { - return; - } - } - - throw new ConfigurationError( - t('Cannot apply restriction %s using the filter %s.' - . ' You can only use the following columns: %s'), - $name, - $queryString, - join(', ', array_map(function ($k, $v) { - return is_string($k) ? $k : $v; - }, array_keys($allowedColumns), $allowedColumns)) - ); - })->parse()); - } - - return $restriction->isEmpty() ? Filter::all() : $restriction; - } -} From dadc02dd1b2979aa132f5e70277faf1ae7253386 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Tue, 9 Mar 2021 11:27:50 +0100 Subject: [PATCH 18/26] Auth: Don't apply restrictions if the user is unrestricted --- library/Icingadb/Common/Auth.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/library/Icingadb/Common/Auth.php b/library/Icingadb/Common/Auth.php index c9fd3474e..ee3c27977 100644 --- a/library/Icingadb/Common/Auth.php +++ b/library/Icingadb/Common/Auth.php @@ -32,6 +32,10 @@ public function getAuth() */ public function applyRestrictions(Query $query) { + if ($this->getAuth()->getUser()->isUnrestricted()) { + return; + } + if ($query instanceof UnionQuery) { $queries = $query->getUnions(); } else { @@ -71,7 +75,6 @@ public function applyRestrictions(Query $query) ); } - // TODO: Should we allow full access if there's a role that doesn't define any restriction? $queryFilter->add($roleFilter); } From c1a289b23b570f2fe275d07dacbe80b0fcc08b66 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Tue, 9 Mar 2021 15:42:25 +0100 Subject: [PATCH 19/26] ObjectDetail: Fetch vars from `customvar_flat` instead of `customvar` This is a preparation for the custom variable restrictions as they're supposed to be applied on the query. --- library/Icingadb/Compat/CompatObject.php | 29 +++++++------- library/Icingadb/Model/CustomvarFlat.php | 38 +++++++++++++++++++ .../Icingadb/Widget/Detail/CustomVarTable.php | 25 +++++++----- .../Icingadb/Widget/Detail/ObjectDetail.php | 16 ++------ 4 files changed, 71 insertions(+), 37 deletions(-) diff --git a/library/Icingadb/Compat/CompatObject.php b/library/Icingadb/Compat/CompatObject.php index ee2baf256..46a4cfa56 100644 --- a/library/Icingadb/Compat/CompatObject.php +++ b/library/Icingadb/Compat/CompatObject.php @@ -4,7 +4,6 @@ namespace Icinga\Module\Icingadb\Compat; -use Icinga\Application\Config; use Icinga\Exception\NotImplementedError; use Icinga\Module\Icingadb\Common\Auth; use Icinga\Module\Icingadb\Model\Host; @@ -61,9 +60,9 @@ public function fetch() return true; } - public function fetchCustomvars() + protected function fetchRawCustomvars() { - if ($this->customvars !== null) { + if ($this->rawCustomvars !== null) { return $this; } @@ -76,19 +75,19 @@ public function fetchCustomvars() $this->rawCustomvars = $customVars; - $filter = new CustomvarFilter( - $vars, - $this->type, - $this->getAuth()->getRestrictions('monitoring/blacklist/properties'), - Config::module('monitoring')->get('security', 'protected_customvars', '') - ); + return $this; + } - $obscuredVars = []; - foreach ($filter as $row) { - $obscuredVars[$row->name] = $row->value; + public function fetchCustomvars() + { + if ($this->customvars !== null) { + return $this; } - $this->customvars = $obscuredVars; + $this->customvars = $this->object->customvar_flat->getModel()->unflattenVars( + $this->object->customvar_flat + ); + return $this; } @@ -106,10 +105,10 @@ public function __get($name) if (preg_match('/^_(host|service)_(.+)/i', $name, $matches)) { switch (strtolower($matches[1])) { case $this->type: - $customvars = $this->fetchCustomvars()->rawCustomvars; + $customvars = $this->fetchRawCustomvars()->rawCustomvars; break; case self::TYPE_HOST: - $customvars = $this->getHost()->fetchCustomvars()->rawCustomvars; + $customvars = $this->getHost()->fetchRawCustomvars()->rawCustomvars; break; case self::TYPE_SERVICE: throw new LogicException('Cannot fetch service custom variables for non-service objects'); diff --git a/library/Icingadb/Model/CustomvarFlat.php b/library/Icingadb/Model/CustomvarFlat.php index 1a3653b3a..43fab0b32 100644 --- a/library/Icingadb/Model/CustomvarFlat.php +++ b/library/Icingadb/Model/CustomvarFlat.php @@ -8,6 +8,7 @@ use ipl\Orm\Behaviors; use ipl\Orm\Model; use ipl\Orm\Relations; +use Traversable; class CustomvarFlat extends Model { @@ -76,4 +77,41 @@ public function createBehaviors(Behaviors $behaviors) { $behaviors->add(new FlattenedObjectVars()); } + + /** + * Restore flattened custom variables to their previous structure + * + * @param Traversable $flattenedVars + * + * @return array + */ + public function unFlattenVars(Traversable $flattenedVars) + { + $registerValue = function (&$data, $path, $value) use (&$registerValue) { + $step = array_shift($path); + $pos = null; + if (preg_match('/\[(\d+)]$/', $step, $m)) { + $step = substr($step, 0, -strlen($m[0])); + array_unshift($path, $m[1]); + } + + if (! empty($path)) { + if (! isset($data[$step])) { + $data[$step] = []; + } + + $registerValue($data[$step], $path, $value); + } else { + $data[$step] = $value; + } + }; + + $vars = []; + foreach ($flattenedVars as $var) { + $path = explode('.', $var->flatname); + $registerValue($vars, $path, $var->flatvalue); + } + + return $vars; + } } diff --git a/library/Icingadb/Widget/Detail/CustomVarTable.php b/library/Icingadb/Widget/Detail/CustomVarTable.php index 2b38fa0a0..81fc5458d 100644 --- a/library/Icingadb/Widget/Detail/CustomVarTable.php +++ b/library/Icingadb/Widget/Detail/CustomVarTable.php @@ -35,14 +35,16 @@ protected function addRow($name, $value) protected function renderVar($name, $value) { + $isArray = is_array($value); switch (true) { - case is_array($value): - return $this->renderArray($name, $value); - case is_object($value): - return $this->renderObject($name, $value); - case $value === null: + case $isArray && is_int(key($value)): + $this->renderArray($name, $value); + break; + case $isArray: + $this->renderObject($name, $value); + break; default: - return $this->renderScalar($name, $value); + $this->renderScalar($name, $value); } } @@ -53,6 +55,7 @@ protected function renderArray($name, array $array) ++$this->level; + ksort($array); foreach ($array as $key => $value) { $this->renderVar("[$key]", $value); } @@ -60,13 +63,14 @@ protected function renderArray($name, array $array) --$this->level; } - protected function renderObject($name, $object) + protected function renderObject($name, array $object) { - $numItems = count(get_object_vars($object)); + $numItems = count($object); $this->addRow($name, sprintf(tp('%d item', '%d items', $numItems), $numItems)); ++$this->level; + ksort($object); foreach ($object as $key => $value) { $this->renderVar($key, $value); } @@ -81,8 +85,9 @@ protected function renderScalar($name, $value) protected function assemble() { - foreach ($this->data as $customvar) { - $this->renderVar($customvar->name, $customvar->value); + ksort($this->data); + foreach ($this->data as $name => $value) { + $this->renderVar($name, $value); } } } diff --git a/library/Icingadb/Widget/Detail/ObjectDetail.php b/library/Icingadb/Widget/Detail/ObjectDetail.php index 1cd548712..35ebd8e60 100644 --- a/library/Icingadb/Widget/Detail/ObjectDetail.php +++ b/library/Icingadb/Widget/Detail/ObjectDetail.php @@ -4,7 +4,6 @@ namespace Icinga\Module\Icingadb\Widget\Detail; -use Icinga\Application\Config; use Icinga\Application\Icinga; use Icinga\Module\Icingadb\Common\Auth; use Icinga\Module\Icingadb\Common\Database; @@ -15,7 +14,6 @@ use Icinga\Module\Icingadb\Common\ServiceLinks; use Icinga\Module\Icingadb\Compat\CompatObject; use Icinga\Module\Icingadb\Compat\CompatPluginOutput; -use Icinga\Module\Icingadb\Compat\CustomvarFilter; use Icinga\Module\Icingadb\Forms\Command\Object\ToggleObjectFeaturesForm; use Icinga\Module\Icingadb\Model\Host; use Icinga\Module\Icingadb\Model\User; @@ -28,7 +26,6 @@ use Icinga\Module\Icingadb\Widget\TagList; use Icinga\Module\Monitoring\Hook\DetailviewExtensionHook; use Icinga\Module\Monitoring\Hook\ObjectActionsHook; -use Icinga\Web\Helper\Markdown; use Icinga\Web\Hook; use Icinga\Web\Navigation\Navigation; use ipl\Html\BaseHtmlElement; @@ -143,16 +140,11 @@ protected function createComments() protected function createCustomVars() { $content = [Html::tag('h2', t('Custom Variables'))]; - $vars = $this->object->customvar->execute(); - - if ($vars->hasResult()) { - $vars = new CustomvarFilter( - $vars, - $this->objectType, - $this->getAuth()->getRestrictions('monitoring/blacklist/properties'), - Config::module('monitoring')->get('security', 'protected_customvars', '') - ); + $vars = $this->object->customvar_flat->getModel()->unflattenVars( + $this->object->customvar_flat + ); + if (! empty($vars)) { $customvarTable = new CustomVarTable($vars); $customvarTable->setAttribute('id', $this->objectType . '-customvars'); $content[] = $customvarTable; From 989d6fcccc992880dcf091084977cb5d9bf5d805 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Thu, 11 Mar 2021 16:37:52 +0100 Subject: [PATCH 20/26] configuration.php: Add custom variable restrictions --- configuration.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/configuration.php b/configuration.php index 50386faa1..33c11bdb9 100644 --- a/configuration.php +++ b/configuration.php @@ -28,6 +28,16 @@ $this->translate('Restrict access to the Icinga services that match the filter') ); + $this->provideRestriction( + 'icingadb/blacklist/variables', + $this->translate('Hide custom variables of Icinga objects that are part of the list') + ); + + $this->provideRestriction( + 'icingadb/protect/variables', + $this->translate('Obfuscate custom variable values of Icinga objects that are part of the list') + ); + if (! $this::exists('monitoring')) { /** * Search urls From 5a808ae6d2063fba8fd667b214ad4550eb7f08fe Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Thu, 11 Mar 2021 16:39:46 +0100 Subject: [PATCH 21/26] Apply custom variable restrictions when necessary --- library/Icingadb/Common/Auth.php | 127 +++++++++++++++--- .../Control/SearchBar/ObjectSuggestions.php | 22 ++- .../Icingadb/Widget/Detail/ObjectDetail.php | 6 +- 3 files changed, 125 insertions(+), 30 deletions(-) diff --git a/library/Icingadb/Common/Auth.php b/library/Icingadb/Common/Auth.php index ee3c27977..15c2e481c 100644 --- a/library/Icingadb/Common/Auth.php +++ b/library/Icingadb/Common/Auth.php @@ -8,6 +8,7 @@ use ipl\Orm\Compat\FilterProcessor; use ipl\Orm\Query; use ipl\Orm\UnionQuery; +use ipl\Sql\Expression; use ipl\Sql\Filter\IsNull; use ipl\Stdlib\Filter; use ipl\Web\Filter\QueryString; @@ -24,7 +25,8 @@ public function getAuth() * * This will apply `icingadb/filter/objects` in any case. `icingadb/filter/services` is only * applied to queries fetching services and `icingadb/filter/hosts` is applied to queries - * fetching either hosts or services. + * fetching either hosts or services. It also applies custom variable restrictions and + * obfuscations. (`icingadb/blacklist/variables` and `icingadb/protect/variables`) * * @param Query $query * @@ -44,38 +46,115 @@ public function applyRestrictions(Query $query) foreach ($queries as $query) { $relations = [$query->getModel()->getTableName()]; - foreach ($query->getWith() as $relation) { - $relations[] = $relation->getTarget()->getTableName(); + foreach ($query->getWith() as $relationPath => $relation) { + $relations[$relationPath] = $relation->getTarget()->getTableName(); } + $customVarRelationName = array_search('customvar_flat', $relations, true); $applyServiceRestriction = in_array('service', $relations, true); $applyHostRestriction = in_array('host', $relations, true) // Hosts and services have a special relation as a service can't exist without its host. // Hence why the hosts restriction is also applied if only services are queried. || $applyServiceRestriction; + $resolver = $query->getResolver(); + $queryFilter = Filter::any(); + $obfuscationRules = Filter::any(); foreach ($this->getAuth()->getUser()->getRoles() as $role) { $roleFilter = Filter::all(); - if (($restriction = $role->getRestrictions('icingadb/filter/objects'))) { - $roleFilter->add($this->parseRestriction($restriction, 'icingadb/filter/objects')); + if ($customVarRelationName !== false) { + if (($restriction = $role->getRestrictions('icingadb/blacklist/variables'))) { + $roleFilter->add($this->parseBlacklist( + $restriction, + $customVarRelationName + ? $resolver->qualifyColumn('flatname', $customVarRelationName) + : 'flatname' + )); + } + + if (($restriction = $role->getRestrictions('icingadb/protect/variables'))) { + $obfuscationRules->add($this->parseBlacklist( + $restriction, + $customVarRelationName + ? $resolver->qualifyColumn('flatname', $customVarRelationName) + : 'flatname' + )); + } + } + + if ($customVarRelationName === false || count($relations) > 1) { + if (($restriction = $role->getRestrictions('icingadb/filter/objects'))) { + $roleFilter->add($this->parseRestriction($restriction, 'icingadb/filter/objects')); + } + + if ($applyHostRestriction && ($restriction = $role->getRestrictions('icingadb/filter/hosts'))) { + $roleFilter->add($this->parseRestriction($restriction, 'icingadb/filter/hosts')); + } + + if ( + $applyServiceRestriction + && ($restriction = $role->getRestrictions('icingadb/filter/services')) + ) { + $roleFilter->add( + Filter::any( + new IsNull('service.id'), + $this->parseRestriction($restriction, 'icingadb/filter/services') + ) + ); + } } - if ($applyHostRestriction && ($restriction = $role->getRestrictions('icingadb/filter/hosts'))) { - $roleFilter->add($this->parseRestriction($restriction, 'icingadb/filter/hosts')); + if (! $roleFilter->isEmpty()) { + $queryFilter->add($roleFilter); } + } - if ($applyServiceRestriction && ($restriction = $role->getRestrictions('icingadb/filter/services'))) { - $roleFilter->add( - Filter::any( - new IsNull('service.id'), - $this->parseRestriction($restriction, 'icingadb/filter/services') - ) - ); + if (! $obfuscationRules->isEmpty()) { + $flatvaluePath = $customVarRelationName + ? $resolver->qualifyColumn('flatvalue', $customVarRelationName) + : 'flatvalue'; + + $columns = $query->getColumns(); + if (empty($columns)) { + $columns = [$flatvaluePath, '*']; } - $queryFilter->add($roleFilter); + $flatvalue = null; + if (isset($columns[$flatvaluePath])) { + $flatvalue = $columns[$flatvaluePath]; + } else { + $flatvaluePathAt = array_search($flatvaluePath, $columns, true); + if ($flatvaluePathAt !== false) { + $flatvalue = $columns[$flatvaluePathAt]; + if (is_int($flatvaluePathAt)) { + unset($columns[$flatvaluePathAt]); + } else { + $flatvaluePath = $flatvaluePathAt; + } + } + } + + if ($flatvalue !== null) { + // TODO: The four lines below are needed because there is still no way to postpone filter column + // qualification. (i.e. Just like the expression, filter rules need to be handled the same + // so that their columns are qualified lazily when assembling the query) + $queryClone = clone $query; + $queryClone->getSelectBase()->resetWhere(); + FilterProcessor::apply($obfuscationRules, $queryClone); + $where = $queryClone->getSelectBase()->getWhere(); + + $values = []; + $rendered = $this->getDb()->getQueryBuilder()->buildCondition($where, $values); + $columns[$flatvaluePath] = new Expression( + "CASE WHEN (" . $rendered . ") THEN (%s) ELSE '***' END", + [$flatvalue], + ...$values + ); + + $query->setColumns($columns); + } } FilterProcessor::apply($queryFilter, $query); @@ -141,4 +220,22 @@ function ($k, $v) { } )->parse(); } + + /** + * Parse the given blacklist + * + * @param string $blacklist Comma separated list of column names + * @param string $column The column which should not equal any of the blacklisted names + * + * @return Filter\None + */ + protected function parseBlacklist($blacklist, $column) + { + $filter = Filter::none(); + foreach (explode(',', $blacklist) as $value) { + $filter->add(Filter::equal($column, trim($value))); + } + + return $filter; + } } diff --git a/library/Icingadb/Web/Control/SearchBar/ObjectSuggestions.php b/library/Icingadb/Web/Control/SearchBar/ObjectSuggestions.php index e2955be9f..6b916d9d9 100644 --- a/library/Icingadb/Web/Control/SearchBar/ObjectSuggestions.php +++ b/library/Icingadb/Web/Control/SearchBar/ObjectSuggestions.php @@ -93,9 +93,6 @@ protected function createQuickSearchFilter($searchTerm) return $quickFilter; } - /** - * @todo Don't suggest obfuscated (protected) custom variables - */ protected function fetchValueSuggestions($column, $searchTerm, Filter\Chain $searchFilter) { $model = $this->getModel(); @@ -172,9 +169,6 @@ protected function fetchValueSuggestions($column, $searchTerm, Filter\Chain $sea } } - /** - * @todo Don't suggest blacklisted custom variables - */ protected function fetchColumnSuggestions($searchTerm) { // Ordinary columns first @@ -235,30 +229,34 @@ protected function queryCustomvarConfig($searchTerm) { $customVars = CustomvarFlat::on($this->getDb()); $tableName = $customVars->getModel()->getTableName(); + $resolver = $customVars->getResolver(); - $columns = ['flatname']; + $scalarQueries = []; $aggregates = ['flatname']; - foreach ($customVars->getResolver()->getRelations($customVars->getModel()) as $name => $relation) { + foreach ($resolver->getRelations($customVars->getModel()) as $name => $relation) { if (isset($this->customVarSources[$name]) && $relation instanceof BelongsToMany) { $query = $customVars->createSubQuery( $relation->getTarget(), - $customVars->getResolver()->qualifyPath($name, $tableName) + $resolver->qualifyPath($name, $tableName) ); $this->applyRestrictions($query); $aggregates[$name] = new Expression("MAX($name)"); - $columns[$name] = $query->assembleSelect() + $scalarQueries[$name] = $query->assembleSelect() ->resetColumns()->columns(new Expression('1')) ->limit(1); } } + $customVars->columns('flatname'); + $this->applyRestrictions($customVars); FilterProcessor::apply(Filter::equal('flatname', $searchTerm), $customVars); + $idColumn = $resolver->qualifyColumnsAndAliases((array) 'id', $customVars->getModel(), false); $customVars = $customVars->assembleSelect(); - $customVars->resetColumns()->columns($columns); - $customVars->groupBy('id'); + $customVars->columns($scalarQueries); + $customVars->groupBy($idColumn); $customVars->limit(static::DEFAULT_LIMIT); // This outer query exists only because there's no way to combine aggregates and sub queries (yet) diff --git a/library/Icingadb/Widget/Detail/ObjectDetail.php b/library/Icingadb/Widget/Detail/ObjectDetail.php index 35ebd8e60..7d98e8627 100644 --- a/library/Icingadb/Widget/Detail/ObjectDetail.php +++ b/library/Icingadb/Widget/Detail/ObjectDetail.php @@ -140,10 +140,10 @@ protected function createComments() protected function createCustomVars() { $content = [Html::tag('h2', t('Custom Variables'))]; - $vars = $this->object->customvar_flat->getModel()->unflattenVars( - $this->object->customvar_flat - ); + $flattenedVars = $this->object->customvar_flat; + $this->applyRestrictions($flattenedVars); + $vars = $this->object->customvar_flat->getModel()->unflattenVars($flattenedVars); if (! empty($vars)) { $customvarTable = new CustomVarTable($vars); $customvarTable->setAttribute('id', $this->objectType . '-customvars'); From 3a14e2e0b4a2ddc5427092c458a61c37c0722b69 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Thu, 11 Mar 2021 16:40:59 +0100 Subject: [PATCH 22/26] Drop class `Icinga\Module\Icingadb\Compat\CustomvarFilter` --- library/Icingadb/Compat/CustomvarFilter.php | 101 -------------------- 1 file changed, 101 deletions(-) delete mode 100644 library/Icingadb/Compat/CustomvarFilter.php diff --git a/library/Icingadb/Compat/CustomvarFilter.php b/library/Icingadb/Compat/CustomvarFilter.php deleted file mode 100644 index 660f7cde7..000000000 --- a/library/Icingadb/Compat/CustomvarFilter.php +++ /dev/null @@ -1,101 +0,0 @@ -objectType = $objectType; - - if (! empty($restrictions)) { - $this->filter = new GlobFilter($restrictions); - } - - if ($toObfuscate) { - $patterns = []; - foreach (explode(',', $toObfuscate) as $pattern) { - $nonWildcards = []; - foreach (explode('*', $pattern) as $nonWildcard) { - $nonWildcards[] = preg_quote($nonWildcard, '/'); - } - - $patterns[] = implode('.*', $nonWildcards); - } - - $this->toObfuscate = '/^(' . join('|', $patterns) . ')$/i'; - } - - parent::__construct($iterator); - } - - public function accept() - { - if ($this->filter === null) { - return true; - } - - $model = $this->getInnerIterator()->current(); - - $this->currentResult = $this->filter->removeMatching([ - $this->objectType => [ - 'vars' => [ - $model->name => $model->value - ] - ] - ]); - - return isset($this->currentResult[$this->objectType]['vars'][$model->name]); - } - - public function current() - { - $model = parent::current(); - - if ($this->filter !== null) { - $model->value = $this->currentResult[$this->objectType]['vars'][$model->name]; - } - - if ($this->toObfuscate !== null) { - $model->value = $this->obfuscate($model->name, $model->value); - } - - return $model; - } - - protected function obfuscate($name, $value) - { - if (preg_match($this->toObfuscate, $name)) { - return '***'; - } elseif (is_scalar($value)) { - return $value; - } - - $obfuscatedVars = []; - foreach (arrayval($value) as $nestedName => $nestedValue) { - $obfuscatedVars[$nestedName] = $this->obfuscate($nestedName, $nestedValue); - } - - return $value instanceof stdClass ? (object) $obfuscatedVars : $obfuscatedVars; - } -} From 96b9a796ae54e0afb43a5b17dd7aa1c46740f51c Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 12 Mar 2021 10:16:40 +0100 Subject: [PATCH 23/26] config: Remove obsolete `Security` tab --- application/controllers/ConfigController.php | 13 ------------- configuration.php | 5 ----- 2 files changed, 18 deletions(-) diff --git a/application/controllers/ConfigController.php b/application/controllers/ConfigController.php index a56c7c2fb..9b6069daf 100644 --- a/application/controllers/ConfigController.php +++ b/application/controllers/ConfigController.php @@ -8,7 +8,6 @@ use Icinga\Module\Icingadb\Forms\DatabaseConfigForm; use Icinga\Module\Icingadb\Forms\RedisConfigForm; use Icinga\Module\Icingadb\Web\Controller; -use Icinga\Module\Monitoring\Forms\Config\SecurityConfigForm; use Icinga\Web\Form; use Icinga\Web\Widget\Tab; use Icinga\Web\Widget\Tabs; @@ -47,18 +46,6 @@ public function redisAction() $this->addFormToContent($form); } - public function securityAction() - { - $form = (new SecurityConfigForm()) - ->setIniConfig(Config::module('monitoring')); - - $form->handleRequest(); - - $this->mergeTabs($this->Module()->getConfigTabs()->activate('security')); - - $this->addFormToContent($form); - } - protected function addFormToContent(Form $form) { $this->addContent(new HtmlString($form->render())); diff --git a/configuration.php b/configuration.php index 33c11bdb9..8bfc3142e 100644 --- a/configuration.php +++ b/configuration.php @@ -239,11 +239,6 @@ 'title' => t('Configure command transports'), 'url' => 'command-transport' ]); - $this->provideConfigTab('security', [ - 'label' => t('Security'), - 'title' => t('Configure security related settings'), - 'url' => 'config/security' - ]); try { // TODO: Remove this before the stable release!!!1!11 From 73f0b39b17c61bf682f001ca9cb3dcf48815ee61 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 12 Mar 2021 10:26:49 +0100 Subject: [PATCH 24/26] Auth: Prefer `!=*` check instead of `IsNull` condition --- library/Icingadb/Common/Auth.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library/Icingadb/Common/Auth.php b/library/Icingadb/Common/Auth.php index 15c2e481c..7c2423478 100644 --- a/library/Icingadb/Common/Auth.php +++ b/library/Icingadb/Common/Auth.php @@ -9,7 +9,6 @@ use ipl\Orm\Query; use ipl\Orm\UnionQuery; use ipl\Sql\Expression; -use ipl\Sql\Filter\IsNull; use ipl\Stdlib\Filter; use ipl\Web\Filter\QueryString; @@ -99,7 +98,7 @@ public function applyRestrictions(Query $query) ) { $roleFilter->add( Filter::any( - new IsNull('service.id'), + Filter::unequal('service.id', '*'), $this->parseRestriction($restriction, 'icingadb/filter/services') ) ); From 1c079ef0f8dc7c54b80718d23a171d9a3a81d1b2 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Thu, 21 Jan 2021 15:06:46 +0100 Subject: [PATCH 25/26] ObjectSuggestions: Make value suggestions work better This depends upon https://github.com/Icinga/ipl-orm/pull/13 --- .../Icingadb/Web/Control/SearchBar/ObjectSuggestions.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/library/Icingadb/Web/Control/SearchBar/ObjectSuggestions.php b/library/Icingadb/Web/Control/SearchBar/ObjectSuggestions.php index 6b916d9d9..77b49e81d 100644 --- a/library/Icingadb/Web/Control/SearchBar/ObjectSuggestions.php +++ b/library/Icingadb/Web/Control/SearchBar/ObjectSuggestions.php @@ -147,15 +147,17 @@ protected function fetchValueSuggestions($column, $searchTerm, Filter\Chain $sea FilterProcessor::apply($flatnameFilter, $query); } + $inputFilter = Filter::equal($columnPath, $searchTerm); + $inputFilter->noOptimization = true; $query->columns($columnPath); // This had so many iterations, if it still doesn't work, consider removing it entirely :( if ($searchFilter instanceof Filter\None) { - FilterProcessor::apply(Filter::equal($columnPath, $searchTerm), $query); + FilterProcessor::apply($inputFilter, $query); } elseif ($searchFilter instanceof Filter\All) { - $searchFilter->add(Filter::equal($columnPath, $searchTerm)); + $searchFilter->add($inputFilter); } else { - $searchFilter = Filter::equal($columnPath, $searchTerm); + $searchFilter = $inputFilter; } FilterProcessor::apply($searchFilter, $query); From aae4c01cbf6aed4b851715382c7f07a3b72bfab6 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 12 Mar 2021 11:18:45 +0100 Subject: [PATCH 26/26] Auth: Prevent filter optimizations for variable blacklists This depends upon https://github.com/Icinga/ipl-orm/pull/13 --- library/Icingadb/Common/Auth.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/library/Icingadb/Common/Auth.php b/library/Icingadb/Common/Auth.php index 7c2423478..21b73bb4b 100644 --- a/library/Icingadb/Common/Auth.php +++ b/library/Icingadb/Common/Auth.php @@ -232,7 +232,13 @@ protected function parseBlacklist($blacklist, $column) { $filter = Filter::none(); foreach (explode(',', $blacklist) as $value) { - $filter->add(Filter::equal($column, trim($value))); + $columnFilter = Filter::equal($column, trim($value)); + + // For an explanation, check class ObjectSuggestions. This does + // not expect other blacklists than those for custom variables. + $columnFilter->noOptimization = true; + + $filter->add($columnFilter); } return $filter;