From 0c30356d52b692c62a4d85a59000440f98f93334 Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Thu, 12 Mar 2015 13:59:24 +0100 Subject: [PATCH 01/12] Add Menu::getPermission() and Menu::setPermission() Those two functions are required for filtering menu items based on a user's grants. refs #8720 --- library/Icinga/Web/Menu.php | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/library/Icinga/Web/Menu.php b/library/Icinga/Web/Menu.php index 8d62563be..872fd3249 100644 --- a/library/Icinga/Web/Menu.php +++ b/library/Icinga/Web/Menu.php @@ -75,6 +75,17 @@ class Menu implements RecursiveIterator */ protected $parent; + /** + * Permission a user is required to have granted to display the menu item + * + * If a permission is set, authentication is of course required. + * + * Note that only one required permission can be set yet. + * + * @var string|null + */ + protected $permission; + /** * Create a new menu * @@ -442,15 +453,27 @@ class Menu implements RecursiveIterator } /** - * Set required Permissions + * Get the permission a user is required to have granted to display the menu item * - * @param $permission + * @return string|null + */ + public function getPermission() + { + return $this->permission; + } + + /** + * Set permission a user is required to have granted to display the menu item + * + * If a permission is set, authentication is of course required. + * + * @param string $permission * * @return $this */ - public function requirePermission($permission) + public function setPermission($permission) { - // Not implemented yet + $this->permission = (string) $permission; return $this; } From 956bc3c07a7d1d428d0d087e121aa3507e959a01 Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Thu, 12 Mar 2015 14:40:37 +0100 Subject: [PATCH 02/12] Security: Rename system/config/application to config/application/general Module config permission and application config permission have to be separeted. Application config related permissions will be added beneath config/application and module related config permissions will be added beneath config/modules. refs #8720 --- application/controllers/ConfigController.php | 4 ++-- application/controllers/RolesController.php | 2 +- application/forms/Security/RoleForm.php | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/application/controllers/ConfigController.php b/application/controllers/ConfigController.php index f7543cc03..307ce3b00 100644 --- a/application/controllers/ConfigController.php +++ b/application/controllers/ConfigController.php @@ -37,7 +37,7 @@ class ConfigController extends ActionController $tabs = $this->getTabs(); $auth = $this->Auth(); $allowedActions = array(); - if ($auth->hasPermission('system/config/application')) { + if ($auth->hasPermission('config/application/general')) { $tabs->add('application', array( 'title' => $this->translate('Adjust the general configuration of Icinga Web 2'), 'label' => $this->translate('Application'), @@ -103,7 +103,7 @@ class ConfigController extends ActionController */ public function applicationAction() { - $this->assertPermission('system/config/application'); + $this->assertPermission('config/application/general'); $form = new GeneralConfigForm(); $form->setIniConfig(Config::app()); $form->handleRequest(); diff --git a/application/controllers/RolesController.php b/application/controllers/RolesController.php index 2b7b520c5..95440ca96 100644 --- a/application/controllers/RolesController.php +++ b/application/controllers/RolesController.php @@ -23,7 +23,7 @@ class RolesController extends ActionController $this->assertPermission('system/config/roles'); $tabs = $this->getTabs(); $auth = $this->Auth(); - if ($auth->hasPermission('system/config/application')) { + if ($auth->hasPermission('config/application/general')) { $tabs->add('application', array( 'title' => $this->translate('Adjust the general configuration of Icinga Web 2'), 'label' => $this->translate('Application'), diff --git a/application/forms/Security/RoleForm.php b/application/forms/Security/RoleForm.php index 85691de24..d77210510 100644 --- a/application/forms/Security/RoleForm.php +++ b/application/forms/Security/RoleForm.php @@ -23,7 +23,7 @@ class RoleForm extends ConfigForm protected $providedPermissions = array( '*' => '*', 'system/config/*' => 'system/config/*', - 'system/config/application' => 'system/config/application', + 'config/application/general' => 'config/application/general', 'system/config/authentication' => 'system/config/authentication', 'system/config/modules' => 'system/config/modules', 'system/config/resources' => 'system/config/resources', From a4e81c320a6b0d165b053fcf68cdcbbbfa3da65f Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Thu, 12 Mar 2015 14:44:45 +0100 Subject: [PATCH 03/12] Security: Rename system/config/authentication to config/application/authentication Module config permission and application config permission have to be separeted. Application config related permissions will be added beneath config/application and module related config permissions will be added beneath config/modules. refs #8720 --- application/controllers/ConfigController.php | 10 +++++----- application/controllers/RolesController.php | 2 +- application/forms/Security/RoleForm.php | 14 +++++++------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/application/controllers/ConfigController.php b/application/controllers/ConfigController.php index 307ce3b00..21f359226 100644 --- a/application/controllers/ConfigController.php +++ b/application/controllers/ConfigController.php @@ -45,7 +45,7 @@ class ConfigController extends ActionController )); $allowedActions[] = 'application'; } - if ($auth->hasPermission('system/config/authentication')) { + if ($auth->hasPermission('config/application/authentication')) { $tabs->add('authentication', array( 'title' => $this->translate('Configure how users authenticate with and log into Icinga Web 2'), 'label' => $this->translate('Authentication'), @@ -199,7 +199,7 @@ class ConfigController extends ActionController */ public function authenticationAction() { - $this->assertPermission('system/config/authentication'); + $this->assertPermission('config/application/authentication'); $form = new AuthenticationBackendReorderForm(); $form->setIniConfig(Config::app('authentication')); $form->handleRequest(); @@ -214,7 +214,7 @@ class ConfigController extends ActionController */ public function createauthenticationbackendAction() { - $this->assertPermission('system/config/authentication'); + $this->assertPermission('config/application/authentication'); $form = new AuthenticationBackendConfigForm(); $form->setTitle($this->translate('Create New Authentication Backend')); $form->addDescription($this->translate( @@ -236,7 +236,7 @@ class ConfigController extends ActionController */ public function editauthenticationbackendAction() { - $this->assertPermission('system/config/authentication'); + $this->assertPermission('config/application/authentication'); $form = new AuthenticationBackendConfigForm(); $form->setTitle($this->translate('Edit Backend')); $form->setIniConfig(Config::app('authentication')); @@ -254,7 +254,7 @@ class ConfigController extends ActionController */ public function removeauthenticationbackendAction() { - $this->assertPermission('system/config/authentication'); + $this->assertPermission('config/application/authentication'); $form = new ConfirmRemovalForm(array( 'onSuccess' => function ($form) { $configForm = new AuthenticationBackendConfigForm(); diff --git a/application/controllers/RolesController.php b/application/controllers/RolesController.php index 95440ca96..57987e60d 100644 --- a/application/controllers/RolesController.php +++ b/application/controllers/RolesController.php @@ -30,7 +30,7 @@ class RolesController extends ActionController 'url' => 'config' )); } - if ($auth->hasPermission('system/config/authentication')) { + if ($auth->hasPermission('config/application/authentication')) { $tabs->add('authentication', array( 'title' => $this->translate('Configure how users authenticate with and log into Icinga Web 2'), 'label' => $this->translate('Authentication'), diff --git a/application/forms/Security/RoleForm.php b/application/forms/Security/RoleForm.php index d77210510..f9d306698 100644 --- a/application/forms/Security/RoleForm.php +++ b/application/forms/Security/RoleForm.php @@ -21,13 +21,13 @@ class RoleForm extends ConfigForm * @type array */ protected $providedPermissions = array( - '*' => '*', - 'system/config/*' => 'system/config/*', - 'config/application/general' => 'config/application/general', - 'system/config/authentication' => 'system/config/authentication', - 'system/config/modules' => 'system/config/modules', - 'system/config/resources' => 'system/config/resources', - 'system/config/roles' => 'system/config/roles' + '*' => '*', + 'system/config/*' => 'system/config/*', + 'config/application/general' => 'config/application/general', + 'config/application/authentication' => 'config/application/authentication', + 'system/config/modules' => 'system/config/modules', + 'system/config/resources' => 'system/config/resources', + 'system/config/roles' => 'system/config/roles' ); /** From 08abbda15247975b1ca97da9dfa09f292515d439 Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Thu, 12 Mar 2015 14:46:56 +0100 Subject: [PATCH 04/12] Security: Rename system/config/resources to config/application/resources Module config permission and application config permission have to be separeted. Application config related permissions will be added beneath config/application and module related config permissions will be added beneath config/modules. refs #8720 --- application/controllers/ConfigController.php | 10 +++++----- application/controllers/RolesController.php | 2 +- application/forms/Security/RoleForm.php | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/application/controllers/ConfigController.php b/application/controllers/ConfigController.php index 21f359226..dc7afd887 100644 --- a/application/controllers/ConfigController.php +++ b/application/controllers/ConfigController.php @@ -53,7 +53,7 @@ class ConfigController extends ActionController )); $allowedActions[] = 'authentication'; } - if ($auth->hasPermission('system/config/resources')) { + if ($auth->hasPermission('config/application/resources')) { $tabs->add('resource', array( 'title' => $this->translate('Configure which resources are being utilized by Icinga Web 2'), 'label' => $this->translate('Resources'), @@ -292,7 +292,7 @@ class ConfigController extends ActionController */ public function resourceAction() { - $this->assertPermission('system/config/resources'); + $this->assertPermission('config/application/resources'); $this->view->resources = Config::app('resources', true)->keys(); $this->view->tabs->activate('resource'); } @@ -302,7 +302,7 @@ class ConfigController extends ActionController */ public function createresourceAction() { - $this->assertPermission('system/config/resources'); + $this->assertPermission('config/application/resources'); $form = new ResourceConfigForm(); $form->setTitle($this->translate('Create A New Resource')); $form->addDescription($this->translate('Resources are entities that provide data to Icinga Web 2.')); @@ -319,7 +319,7 @@ class ConfigController extends ActionController */ public function editresourceAction() { - $this->assertPermission('system/config/resources'); + $this->assertPermission('config/application/resources'); $form = new ResourceConfigForm(); $form->setTitle($this->translate('Edit Existing Resource')); $form->setIniConfig(Config::app('resources')); @@ -335,7 +335,7 @@ class ConfigController extends ActionController */ public function removeresourceAction() { - $this->assertPermission('system/config/resources'); + $this->assertPermission('config/application/resources'); $form = new ConfirmRemovalForm(array( 'onSuccess' => function ($form) { $configForm = new ResourceConfigForm(); diff --git a/application/controllers/RolesController.php b/application/controllers/RolesController.php index 57987e60d..e14426bd7 100644 --- a/application/controllers/RolesController.php +++ b/application/controllers/RolesController.php @@ -37,7 +37,7 @@ class RolesController extends ActionController 'url' => 'config/authentication' )); } - if ($auth->hasPermission('system/config/resources')) { + if ($auth->hasPermission('config/application/resources')) { $tabs->add('resource', array( 'title' => $this->translate('Configure which resources are being utilized by Icinga Web 2'), 'label' => $this->translate('Resources'), diff --git a/application/forms/Security/RoleForm.php b/application/forms/Security/RoleForm.php index f9d306698..bc93c39d4 100644 --- a/application/forms/Security/RoleForm.php +++ b/application/forms/Security/RoleForm.php @@ -26,7 +26,7 @@ class RoleForm extends ConfigForm 'config/application/general' => 'config/application/general', 'config/application/authentication' => 'config/application/authentication', 'system/config/modules' => 'system/config/modules', - 'system/config/resources' => 'system/config/resources', + 'config/application/resources' => 'config/application/resources', 'system/config/roles' => 'system/config/roles' ); From f6e95512351e21c6e897e0507534664a63fa5c9d Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Thu, 12 Mar 2015 14:48:45 +0100 Subject: [PATCH 05/12] Security: Rename system/config/roles to config/application/roles Module config permission and application config permission have to be separeted. Application config related permissions will be added beneath config/application and module related config permissions will be added beneath config/modules. refs #8720 --- application/controllers/ConfigController.php | 2 +- application/controllers/RolesController.php | 2 +- application/forms/Security/RoleForm.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/application/controllers/ConfigController.php b/application/controllers/ConfigController.php index dc7afd887..9bc26f726 100644 --- a/application/controllers/ConfigController.php +++ b/application/controllers/ConfigController.php @@ -61,7 +61,7 @@ class ConfigController extends ActionController )); $allowedActions[] = 'resource'; } - if ($auth->hasPermission('system/config/roles')) { + if ($auth->hasPermission('config/application/roles')) { $tabs->add('roles', array( 'title' => $this->translate( 'Configure roles to permit or restrict users and groups accessing Icinga Web 2' diff --git a/application/controllers/RolesController.php b/application/controllers/RolesController.php index e14426bd7..68d23d84f 100644 --- a/application/controllers/RolesController.php +++ b/application/controllers/RolesController.php @@ -20,7 +20,7 @@ class RolesController extends ActionController */ public function init() { - $this->assertPermission('system/config/roles'); + $this->assertPermission('config/application/roles'); $tabs = $this->getTabs(); $auth = $this->Auth(); if ($auth->hasPermission('config/application/general')) { diff --git a/application/forms/Security/RoleForm.php b/application/forms/Security/RoleForm.php index bc93c39d4..9f951deb2 100644 --- a/application/forms/Security/RoleForm.php +++ b/application/forms/Security/RoleForm.php @@ -27,7 +27,7 @@ class RoleForm extends ConfigForm 'config/application/authentication' => 'config/application/authentication', 'system/config/modules' => 'system/config/modules', 'config/application/resources' => 'config/application/resources', - 'system/config/roles' => 'system/config/roles' + 'config/application/roles' => 'config/application/roles' ); /** From 195fe6f04b7935f2afadf12add6c7804662914ba Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Thu, 12 Mar 2015 14:53:20 +0100 Subject: [PATCH 06/12] Security: Rename system/config/modules to config/modules Module config permission and application config permission have to be separeted. Application config related permissions were added beneath config/application and module related config permissions will be config/modules for now. refs #8720 --- application/controllers/ConfigController.php | 4 ++-- application/forms/Security/RoleForm.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/application/controllers/ConfigController.php b/application/controllers/ConfigController.php index 9bc26f726..1942826bb 100644 --- a/application/controllers/ConfigController.php +++ b/application/controllers/ConfigController.php @@ -159,7 +159,7 @@ class ConfigController extends ActionController */ public function moduleenableAction() { - $this->assertPermission('system/config/modules'); + $this->assertPermission('config/modules'); $module = $this->getParam('name'); $manager = Icinga::app()->getModuleManager(); try { @@ -179,7 +179,7 @@ class ConfigController extends ActionController */ public function moduledisableAction() { - $this->assertPermission('system/config/modules'); + $this->assertPermission('config/modules'); $module = $this->getParam('name'); $manager = Icinga::app()->getModuleManager(); try { diff --git a/application/forms/Security/RoleForm.php b/application/forms/Security/RoleForm.php index 9f951deb2..671f7738f 100644 --- a/application/forms/Security/RoleForm.php +++ b/application/forms/Security/RoleForm.php @@ -25,9 +25,9 @@ class RoleForm extends ConfigForm 'system/config/*' => 'system/config/*', 'config/application/general' => 'config/application/general', 'config/application/authentication' => 'config/application/authentication', - 'system/config/modules' => 'system/config/modules', 'config/application/resources' => 'config/application/resources', - 'config/application/roles' => 'config/application/roles' + 'config/application/roles' => 'config/application/roles', + 'config/modules' => 'config/modules' ); /** From 517437eb1b57fe6ae1b2ad69e48525f3dcf83cbf Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Thu, 12 Mar 2015 14:54:57 +0100 Subject: [PATCH 07/12] Security: Add wildcard permissions to the config/ tree refs #8720 --- application/forms/Security/RoleForm.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/application/forms/Security/RoleForm.php b/application/forms/Security/RoleForm.php index 671f7738f..39d7ae2c6 100644 --- a/application/forms/Security/RoleForm.php +++ b/application/forms/Security/RoleForm.php @@ -22,7 +22,8 @@ class RoleForm extends ConfigForm */ protected $providedPermissions = array( '*' => '*', - 'system/config/*' => 'system/config/*', + 'config/*' => 'config/*', + 'config/application/*' => 'config/application/*', 'config/application/general' => 'config/application/general', 'config/application/authentication' => 'config/application/authentication', 'config/application/resources' => 'config/application/resources', From 33112f6a187a6a9d464b2d97091a27044298ef6e Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Thu, 12 Mar 2015 14:56:17 +0100 Subject: [PATCH 08/12] Use {@inheritdoc} in the RoleForm refs #8720 --- application/forms/Security/RoleForm.php | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/application/forms/Security/RoleForm.php b/application/forms/Security/RoleForm.php index 39d7ae2c6..0600e04f2 100644 --- a/application/forms/Security/RoleForm.php +++ b/application/forms/Security/RoleForm.php @@ -39,8 +39,7 @@ class RoleForm extends ConfigForm protected $providedRestrictions = array(); /** - * (non-PHPDoc) - * @see \Icinga\Web\Form::init() For the method documentation. + * {@inheritdoc} */ public function init() { @@ -69,8 +68,7 @@ class RoleForm extends ConfigForm } /** - * (non-PHPDoc) - * @see \Icinga\Web\Form::createElements() For the method documentation. + * {@inheritdoc} */ public function createElements(array $formData = array()) { @@ -254,8 +252,7 @@ class RoleForm extends ConfigForm } /** - * (non-PHPDoc) - * @see \Zend_Form::getValues() For the method documentation. + * {@inheritdoc} */ public function getValues($suppressArrayNotation = false) { From 7f010102f641e0f46ed013d1d9eff80730444e51 Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Thu, 12 Mar 2015 15:27:44 +0100 Subject: [PATCH 09/12] Add wildcard support to the permission passed to User::can() refs #8720 --- library/Icinga/User.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/library/Icinga/User.php b/library/Icinga/User.php index 3bf0d0aa2..18d44af96 100644 --- a/library/Icinga/User.php +++ b/library/Icinga/User.php @@ -422,8 +422,16 @@ class User if (isset($this->permissions['*']) || isset($this->permissions[$permission])) { return true; } + // If the permission to check contains a wildcard, grant the permission if any permit related to the permission + // matches + $any = strpos($permission, '*'); foreach ($this->permissions as $permitted) { - $wildcard = strpos($permitted, '*'); + if ($any !== false) { + $wildcard = $any; + } else { + // If the permit contains a wildcard, grant the permission if it's related to the permit + $wildcard = strpos($permitted, '*'); + } if ($wildcard !== false) { if (substr($permission, 0, $wildcard) === substr($permitted, 0, $wildcard)) { return true; From e6a60e214c29cd73167fd943550a72aad1ab5f2a Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Thu, 12 Mar 2015 15:28:27 +0100 Subject: [PATCH 10/12] lib: Add PermittedMenuItemFilter ... ... for iterating over menu items the user is allowed to display refs #8720 --- .../Web/Menu/PermittedMenuItemFilter.php | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 library/Icinga/Web/Menu/PermittedMenuItemFilter.php diff --git a/library/Icinga/Web/Menu/PermittedMenuItemFilter.php b/library/Icinga/Web/Menu/PermittedMenuItemFilter.php new file mode 100644 index 000000000..7224c3589 --- /dev/null +++ b/library/Icinga/Web/Menu/PermittedMenuItemFilter.php @@ -0,0 +1,33 @@ +current(); + /** @var Menu $item */ + if (($permission = $item->getPermission()) !== null) { + $auth = Manager::getInstance(); + if (! $auth->isAuthenticated()) { + // Don't accept menu item because user is not authenticated and the menu item requires a permission + return false; + } + if (! $auth->getUser()->can($permission)) { + return false; + } + } + // Accept menu item if it does not require a permission + return true; + } +} From da16bfcef30790ae08ac3c211d08c3d13c069a5e Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Thu, 12 Mar 2015 15:30:10 +0100 Subject: [PATCH 11/12] Security: Use PermittedMenuItemFilter in the MenuRenderer refs #8720 --- library/Icinga/Web/MenuRenderer.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/Icinga/Web/MenuRenderer.php b/library/Icinga/Web/MenuRenderer.php index 78e910f61..7d2cd5ad3 100644 --- a/library/Icinga/Web/MenuRenderer.php +++ b/library/Icinga/Web/MenuRenderer.php @@ -6,6 +6,7 @@ namespace Icinga\Web; use Exception; use RecursiveIteratorIterator; use Icinga\Application\Logger; +use Icinga\Web\Menu\PermittedMenuItemFilter; /** * A renderer to draw a menu with its sub-menus using an unordered html list @@ -44,7 +45,7 @@ class MenuRenderer extends RecursiveIteratorIterator } else { $this->url = Url::fromPath($url); } - parent::__construct($menu, RecursiveIteratorIterator::CHILD_FIRST); + parent::__construct(new PermittedMenuItemFilter($menu), RecursiveIteratorIterator::CHILD_FIRST); } /** From 093dfd627ea563e61189782f3390dea0b6744290 Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Thu, 12 Mar 2015 15:37:56 +0100 Subject: [PATCH 12/12] Security: Hide config menu items if the user lacks the required permission refs #8720 --- library/Icinga/Web/Menu.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/library/Icinga/Web/Menu.php b/library/Icinga/Web/Menu.php index 872fd3249..3e7882bfc 100644 --- a/library/Icinga/Web/Menu.php +++ b/library/Icinga/Web/Menu.php @@ -232,12 +232,14 @@ class Menu implements RecursiveIterator 'priority' => 200 )); $section->add(t('Configuration'), array( - 'url' => 'config', - 'priority' => 300 + 'url' => 'config', + 'permission' => 'config/application/*', + 'priority' => 300 )); $section->add(t('Modules'), array( - 'url' => 'config/modules', - 'priority' => 400 + 'url' => 'config/modules', + 'permission' => 'config/modules', + 'priority' => 400 )); if (Logger::writesToFile()) {