From c0541d70e9f6dfa7b398d6fd5031abc40a15cd7d Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 22 Jan 2021 15:47:36 +0100 Subject: [PATCH 1/8] Move permission match code from class `User` to `Role` --- library/Icinga/Authentication/Role.php | 37 ++++++++++++++++++++++++++ library/Icinga/User.php | 23 ++-------------- 2 files changed, 39 insertions(+), 21 deletions(-) diff --git a/library/Icinga/Authentication/Role.php b/library/Icinga/Authentication/Role.php index f00d063e2..acebae26a 100644 --- a/library/Icinga/Authentication/Role.php +++ b/library/Icinga/Authentication/Role.php @@ -106,4 +106,41 @@ class Role $this->restrictions = $restrictions; return $this; } + + /** + * Whether this role grants the given permission + * + * @param string $permission + * + * @return bool + */ + public function grants($permission) + { + $requiredWildcard = strpos($permission, '*'); + foreach ($this->permissions as $grantedPermission) { + if ($grantedPermission === '*' || $grantedPermission === $permission) { + return true; + } + + if ($requiredWildcard !== false) { + if (($grantedWildcard = strpos($grantedPermission, '*')) !== false) { + $wildcard = min($requiredWildcard, $grantedWildcard); + } else { + $wildcard = $requiredWildcard; + } + } else { + $wildcard = strpos($grantedPermission, '*'); + } + + if ($wildcard !== false && $wildcard > 0) { + if (substr($permission, 0, $wildcard) === substr($grantedPermission, 0, $wildcard)) { + return true; + } + } elseif ($permission === $grantedPermission) { + return true; + } + } + + return false; + } } diff --git a/library/Icinga/User.php b/library/Icinga/User.php index ef5a0ff62..c5652ab62 100644 --- a/library/Icinga/User.php +++ b/library/Icinga/User.php @@ -563,27 +563,8 @@ class User */ public function can($requiredPermission) { - if (isset($this->permissions['*']) || isset($this->permissions[$requiredPermission])) { - return true; - } - - $requiredWildcard = strpos($requiredPermission, '*'); - foreach ($this->permissions as $grantedPermission) { - if ($requiredWildcard !== false) { - if (($grantedWildcard = strpos($grantedPermission, '*')) !== false) { - $wildcard = min($requiredWildcard, $grantedWildcard); - } else { - $wildcard = $requiredWildcard; - } - } else { - $wildcard = strpos($grantedPermission, '*'); - } - - if ($wildcard !== false && $wildcard > 0) { - if (substr($requiredPermission, 0, $wildcard) === substr($grantedPermission, 0, $wildcard)) { - return true; - } - } elseif ($requiredPermission === $grantedPermission) { + foreach ($this->getRoles() as $role) { + if ($role->grants($requiredPermission)) { return true; } } From b2f7c3788d08bb1cd5cd070e90e0bc297d9fab2d Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 22 Jan 2021 15:59:01 +0100 Subject: [PATCH 2/8] test: Roles have permissions, not users --- test/php/library/Icinga/UserTest.php | 11 ++++++++--- .../Icinga/Web/Widget/SearchDashboardTest.php | 13 +++++++++++-- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/test/php/library/Icinga/UserTest.php b/test/php/library/Icinga/UserTest.php index 00230b827..d73ed6b43 100644 --- a/test/php/library/Icinga/UserTest.php +++ b/test/php/library/Icinga/UserTest.php @@ -3,6 +3,7 @@ namespace Tests\Icinga; +use Icinga\Authentication\Role; use Mockery; use DateTimeZone; use Icinga\User; @@ -62,14 +63,18 @@ class UserTest extends BaseTestCase public function testPermissions() { - $user = new User('test'); - $user->setPermissions(array( + $role = new Role(); + $role->setPermissions([ 'test', 'test/some/specific', 'test/more/*', 'test/wildcard-with-wildcard/*', 'test/even-more/specific-with-wildcard/*' - )); + ]); + + $user = new User('test'); + $user->setRoles([$role]); + $this->assertTrue($user->can('test')); $this->assertTrue($user->can('test/some/specific')); $this->assertTrue($user->can('test/more/everything')); diff --git a/test/php/library/Icinga/Web/Widget/SearchDashboardTest.php b/test/php/library/Icinga/Web/Widget/SearchDashboardTest.php index e5e0f1c62..7370338e5 100644 --- a/test/php/library/Icinga/Web/Widget/SearchDashboardTest.php +++ b/test/php/library/Icinga/Web/Widget/SearchDashboardTest.php @@ -3,6 +3,7 @@ namespace Tests\Icinga\Web; +use Icinga\Authentication\Role; use Mockery; use Icinga\Test\BaseTestCase; use Icinga\User; @@ -47,8 +48,12 @@ class SearchDashboardTest extends BaseTestCase public function testWhetherSearchLoadsSearchDashletsFromModules() { + $role = new Role(); + $role->setPermissions(['*']); + $user = new User('test'); - $user->setPermissions(array('*' => '*')); + $user->setRoles([$role]); + $dashboard = new SearchDashboard(); $dashboard->setUser($user); $dashboard = $dashboard->search('pending'); @@ -60,8 +65,12 @@ class SearchDashboardTest extends BaseTestCase public function testWhetherSearchProvidesHintWhenSearchStringIsEmpty() { + $role = new Role(); + $role->setPermissions(['*']); + $user = new User('test'); - $user->setPermissions(array('*' => '*')); + $user->setRoles([$role]); + $dashboard = new SearchDashboard(); $dashboard->setUser($user); $dashboard = $dashboard->search(); From 87d741265e260e130b6119eccd450d2181102daf Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 29 Jan 2021 15:48:48 +0100 Subject: [PATCH 3/8] Auth: Add support for denied permissions --- application/forms/Security/RoleForm.php | 52 +++++++- .../Icinga/Authentication/AdmissionLoader.php | 3 + library/Icinga/Authentication/Role.php | 114 ++++++++++++++---- library/Icinga/Authentication/RolesConfig.php | 1 + library/Icinga/User.php | 11 +- public/css/icinga/widgets.less | 19 +++ test/php/library/Icinga/UserTest.php | 45 +++++++ 7 files changed, 215 insertions(+), 30 deletions(-) diff --git a/application/forms/Security/RoleForm.php b/application/forms/Security/RoleForm.php index b50cb8f8b..bb2e237eb 100644 --- a/application/forms/Security/RoleForm.php +++ b/application/forms/Security/RoleForm.php @@ -23,6 +23,11 @@ class RoleForm extends RepositoryForm */ const WILDCARD_NAME = 'allAndEverything'; + /** + * The prefix used to deny a permission + */ + const DENY_PREFIX = 'no-'; + /** * Provided permissions by currently installed modules * @@ -196,8 +201,15 @@ class RoleForm extends RepositoryForm $elements[] = 'permission_header'; $this->addElement('note', 'permission_header', [ - 'value' => '

' . $this->translate('Permissions') . '

', - 'decorators' => ['ViewHelper'] + 'decorators' => [['Callback', ['callback' => function () { + return '

' . $this->translate('Permissions') . '

' + . $this->getView()->icon('ok', $this->translate( + 'Grant access by toggling a switch below' + )) + . $this->getView()->icon('cancel', $this->translate( + 'Deny access by toggling a switch below' + )); + }]], ['HtmlTag', ['tag' => 'div']]] ]); $hasFullPerm = false; @@ -207,6 +219,17 @@ class RoleForm extends RepositoryForm $elementName .= '_fake'; } + $denyCheckbox = null; + if (! isset($spec['isFullPerm']) + && substr($spec['name'], 0, strlen(self::DENY_PREFIX)) !== self::DENY_PREFIX + ) { + $denyCheckbox = $this->createElement('checkbox', self::DENY_PREFIX . $name, [ + 'decorators' => ['ViewHelper'] + ]); + $this->addElement($denyCheckbox); + $this->removeFromIteration($denyCheckbox->getName()); + } + $elements[] = $elementName; $this->addElement( 'checkbox', @@ -222,7 +245,14 @@ class RoleForm extends RepositoryForm '/​', isset($spec['label']) ? $spec['label'] : $spec['name'] ), - 'description' => isset($spec['description']) ? $spec['description'] : $spec['name'] + 'description' => isset($spec['description']) ? $spec['description'] : $spec['name'], + 'decorators' => array_merge( + array_slice(self::$defaultElementDecorators, 0, 3), + [['Callback', ['callback' => function () use ($denyCheckbox) { + return $denyCheckbox ? $denyCheckbox->render() : ''; + }]]], + array_slice(self::$defaultElementDecorators, 3) + ) ] ) ->getElement($elementName) @@ -298,13 +328,18 @@ class RoleForm extends RepositoryForm self::WILDCARD_NAME => (bool) preg_match('~(?permissions) ]; - if (! empty($role->permissions) && $role->permissions !== '*') { + if (! empty($role->permissions) || ! empty($role->refusals)) { $permissions = StringHelper::trimSplit($role->permissions); + $refusals = StringHelper::trimSplit($role->refusals); foreach ($this->providedPermissions as $moduleName => $permissionList) { foreach ($permissionList as $name => $spec) { if (in_array($spec['name'], $permissions, true)) { $values[$name] = 1; } + + if (in_array($spec['name'], $refusals, true)) { + $values[$this->filterName(self::DENY_PREFIX . $name)] = 1; + } } } } @@ -338,17 +373,24 @@ class RoleForm extends RepositoryForm $permissions[] = '*'; } + $refusals = []; foreach ($this->providedPermissions as $moduleName => $permissionList) { foreach ($permissionList as $name => $spec) { if (isset($values[$name]) && $values[$name]) { $permissions[] = $spec['name']; } - unset($values[$name]); + $denyName = $this->filterName(self::DENY_PREFIX . $name); + if (isset($values[$denyName]) && $values[$denyName]) { + $refusals[] = $spec['name']; + } + + unset($values[$name], $values[$denyName]); } } unset($values[self::WILDCARD_NAME]); + $values['refusals'] = join(',', $refusals); $values['permissions'] = join(',', $permissions); return ConfigForm::transformEmptyValuesToNull($values); } diff --git a/library/Icinga/Authentication/AdmissionLoader.php b/library/Icinga/Authentication/AdmissionLoader.php index 8ee43dbfb..b9a9919db 100644 --- a/library/Icinga/Authentication/AdmissionLoader.php +++ b/library/Icinga/Authentication/AdmissionLoader.php @@ -71,6 +71,7 @@ class AdmissionLoader foreach ($roles as $roleName => $role) { if ($this->match($username, $userGroups, $role)) { $permissionsFromRole = StringHelper::trimSplit($role->permissions); + $refusals = StringHelper::trimSplit($role->refusals); $permissions = array_merge( $permissions, array_diff($permissionsFromRole, $permissions) @@ -78,6 +79,7 @@ class AdmissionLoader $restrictionsFromRole = $role->toArray(); unset($restrictionsFromRole['users']); unset($restrictionsFromRole['groups']); + unset($restrictionsFromRole['refusals']); unset($restrictionsFromRole['permissions']); foreach ($restrictionsFromRole as $name => $restriction) { if (! isset($restrictions[$name])) { @@ -89,6 +91,7 @@ class AdmissionLoader $roleObj = new Role(); $roleObjs[] = $roleObj ->setName($roleName) + ->setRefusals($refusals) ->setPermissions($permissionsFromRole) ->setRestrictions($restrictionsFromRole); } diff --git a/library/Icinga/Authentication/Role.php b/library/Icinga/Authentication/Role.php index acebae26a..a1d166f64 100644 --- a/library/Icinga/Authentication/Role.php +++ b/library/Icinga/Authentication/Role.php @@ -17,14 +17,21 @@ class Role * * @var string[] */ - protected $permissions = array(); + protected $permissions = []; + + /** + * Refusals of the role + * + * @var string[] + */ + protected $refusals = []; /** * Restrictions of the role * * @var string[] */ - protected $restrictions = array(); + protected $restrictions = []; /** * Get the name of the role @@ -46,6 +53,7 @@ class Role public function setName($name) { $this->name = $name; + return $this; } @@ -69,6 +77,31 @@ class Role public function setPermissions(array $permissions) { $this->permissions = $permissions; + + return $this; + } + + /** + * Get the refusals of the role + * + * @return string[] + */ + public function getRefusals() + { + return $this->refusals; + } + + /** + * Set the refusals of the role + * + * @param array $refusals + * + * @return $this + */ + public function setRefusals(array $refusals) + { + $this->refusals = $refusals; + return $this; } @@ -104,6 +137,7 @@ class Role public function setRestrictions(array $restrictions) { $this->restrictions = $restrictions; + return $this; } @@ -116,31 +150,67 @@ class Role */ public function grants($permission) { - $requiredWildcard = strpos($permission, '*'); foreach ($this->permissions as $grantedPermission) { - if ($grantedPermission === '*' || $grantedPermission === $permission) { - return true; - } - - if ($requiredWildcard !== false) { - if (($grantedWildcard = strpos($grantedPermission, '*')) !== false) { - $wildcard = min($requiredWildcard, $grantedWildcard); - } else { - $wildcard = $requiredWildcard; - } - } else { - $wildcard = strpos($grantedPermission, '*'); - } - - if ($wildcard !== false && $wildcard > 0) { - if (substr($permission, 0, $wildcard) === substr($grantedPermission, 0, $wildcard)) { - return true; - } - } elseif ($permission === $grantedPermission) { + if ($this->match($grantedPermission, $permission)) { return true; } } return false; } + + /** + * Whether this role denies the given permission + * + * @param string $permission + * + * @return bool + */ + public function denies($permission) + { + foreach ($this->refusals as $refusedPermission) { + if ($this->match($refusedPermission, $permission, false)) { + return true; + } + } + + return false; + } + + /** + * Get whether the role expression matches the required permission + * + * @param string $roleExpression + * @param string $requiredPermission + * @param bool $cascadeUpwards `false` if `foo/bar/*` and `foo/bar/raboof` should not match `foo/*` + * + * @return bool + */ + protected function match($roleExpression, $requiredPermission, $cascadeUpwards = true) + { + if ($roleExpression === '*' || $roleExpression === $requiredPermission) { + return true; + } + + $requiredWildcard = strpos($requiredPermission, '*'); + if ($requiredWildcard !== false) { + if (($grantedWildcard = strpos($roleExpression, '*')) !== false) { + $wildcard = $cascadeUpwards ? min($requiredWildcard, $grantedWildcard) : $grantedWildcard; + } else { + $wildcard = $cascadeUpwards ? $requiredWildcard : false; + } + } else { + $wildcard = strpos($roleExpression, '*'); + } + + if ($wildcard !== false && $wildcard > 0) { + if (substr($requiredPermission, 0, $wildcard) === substr($roleExpression, 0, $wildcard)) { + return true; + } + } elseif ($requiredPermission === $roleExpression) { + return true; + } + + return false; + } } diff --git a/library/Icinga/Authentication/RolesConfig.php b/library/Icinga/Authentication/RolesConfig.php index 51f07cc3b..a0aeee823 100644 --- a/library/Icinga/Authentication/RolesConfig.php +++ b/library/Icinga/Authentication/RolesConfig.php @@ -22,6 +22,7 @@ class RolesConfig extends IniRepository 'name', 'users', 'groups', + 'refusals', 'permissions', 'application/share/users', 'application/share/groups' diff --git a/library/Icinga/User.php b/library/Icinga/User.php index c5652ab62..833a4c84b 100644 --- a/library/Icinga/User.php +++ b/library/Icinga/User.php @@ -563,13 +563,18 @@ class User */ public function can($requiredPermission) { + $granted = false; foreach ($this->getRoles() as $role) { - if ($role->grants($requiredPermission)) { - return true; + if ($role->denies($requiredPermission)) { + return false; + } + + if (! $granted && $role->grants($requiredPermission)) { + $granted = true; } } - return false; + return $granted; } /** diff --git a/public/css/icinga/widgets.less b/public/css/icinga/widgets.less index 8ae3efcae..d8410178b 100644 --- a/public/css/icinga/widgets.less +++ b/public/css/icinga/widgets.less @@ -242,7 +242,26 @@ form.role-form { } h4 { + display: inline-block; + width: 20em; margin-top: 1.5em; + padding-right: .5625em; + text-align: right; + + & ~ i { + display: inline-block; + width: 2.625em; + margin-right: 1em; + text-align: center; + + &.icon-ok { + color: @color-ok; + } + + &.icon-cancel { + color: @color-critical; + } + } } .collapsible-control { diff --git a/test/php/library/Icinga/UserTest.php b/test/php/library/Icinga/UserTest.php index d73ed6b43..7798aee50 100644 --- a/test/php/library/Icinga/UserTest.php +++ b/test/php/library/Icinga/UserTest.php @@ -76,6 +76,7 @@ class UserTest extends BaseTestCase $user->setRoles([$role]); $this->assertTrue($user->can('test')); + $this->assertTrue($user->can('test/some/*')); $this->assertTrue($user->can('test/some/specific')); $this->assertTrue($user->can('test/more/everything')); $this->assertTrue($user->can('test/wildcard-with-wildcard/*')); @@ -85,4 +86,48 @@ class UserTest extends BaseTestCase $this->assertFalse($user->can('test/some/not/so/specific')); $this->assertFalse($user->can('test/wildcard2/*')); } + + public function testRefusals() + { + $role = new Role(); + $role->setPermissions([ + 'a', + 'a/b/*', + 'a/b/c/d', + 'c/*', + 'd/*' + ]); + $role->setRefusals([ + 'a/b/c', + 'a/b/e', + 'c/b/a', + 'c/d/*', + 'd/f', + 'e/g' + ]); + + $user = new User('test'); + $user->setRoles([$role]); + + $this->assertFalse($user->can('a/b/c')); + $this->assertFalse($user->can('a/b/e')); + $this->assertTrue($user->can('a/b/d')); + $this->assertTrue($user->can('a/b/c/d')); + $this->assertFalse($user->can('c/b/a')); + $this->assertTrue($user->can('c/b/d')); + $this->assertFalse($user->can('c/d/u')); + $this->assertFalse($user->can('c/d/*')); + $this->assertTrue($user->can('c/*')); + $this->assertTrue($user->can('d/*')); + $this->assertFalse($user->can('e/*')); + + $secondRole = new Role(); + $role->setRefusals(['a/b/*']); + + $user->setRoles([$role, $secondRole]); + + $this->assertFalse($user->can('a/b/d')); + $this->assertFalse($user->can('a/b/c/d')); + $this->assertTrue($user->can('c/b/d')); + } } From bdd0f204f0cbdad99aa84782d792440659a35b97 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Mon, 1 Feb 2021 15:04:36 +0100 Subject: [PATCH 4/8] Auth: Support single inheritance in roles --- application/forms/Security/RoleForm.php | 52 ++++++ application/views/scripts/role/list.phtml | 2 + .../Icinga/Authentication/AdmissionLoader.php | 148 +++++++++++++----- library/Icinga/Authentication/Role.php | 90 ++++++++++- library/Icinga/Authentication/RolesConfig.php | 1 + 5 files changed, 250 insertions(+), 43 deletions(-) diff --git a/application/forms/Security/RoleForm.php b/application/forms/Security/RoleForm.php index bb2e237eb..d1e8fd03f 100644 --- a/application/forms/Security/RoleForm.php +++ b/application/forms/Security/RoleForm.php @@ -157,6 +157,19 @@ class RoleForm extends RepositoryForm 'description' => $this->translate('The name of the role') ] ); + $this->addElement( + 'select', + 'parent', + [ + 'label' => $this->translate('Inherit From'), + 'description' => $this->translate('Choose a role from which to inherit privileges'), + 'value' => '', + 'multiOptions' => array_merge( + ['' => $this->translate('None', 'parent role')], + $this->collectRoles() + ) + ] + ); $this->addElement( 'textarea', 'users', @@ -322,6 +335,7 @@ class RoleForm extends RepositoryForm } $values = [ + 'parent' => $role->parent, 'name' => $role->name, 'users' => $role->users, 'groups' => $role->groups, @@ -431,6 +445,36 @@ class RoleForm extends RepositoryForm }); } + protected function collectRoles() + { + // Function to get all connected children. Used to avoid reference loops + $getChildren = function ($name, $children = []) use (&$getChildren) { + foreach ($this->repository->select()->where('parent', $name) as $child) { + if (isset($children[$child->name])) { + // Don't follow already established loops here, + // the user should be able to solve such in the UI + continue; + } + + $children[$child->name] = true; + $children = $getChildren($child->name, $children); + } + + return $children; + }; + + $children = $this->getIdentifier() !== null ? $getChildren($this->getIdentifier()) : []; + + $names = []; + foreach ($this->repository->select() as $role) { + if ($role->name !== $this->getIdentifier() && ! isset($children[$role->name])) { + $names[] = $role->name; + } + } + + return array_combine($names, $names); + } + public function isValid($formData) { $valid = parent::isValid($formData); @@ -452,6 +496,14 @@ class RoleForm extends RepositoryForm return false; } + if (($newName = $this->getValue('name')) !== $this->getIdentifier()) { + $this->repository->update( + $this->getBaseTable(), + ['parent' => $newName], + Filter::where('parent', $this->getIdentifier()) + ); + } + if (ConfigFormEventsHook::runOnSuccess($this) === false) { Notification::error($this->translate( 'Configuration successfully stored. Though, one or more module hooks failed to run.' diff --git a/application/views/scripts/role/list.phtml b/application/views/scripts/role/list.phtml index 9045d0e3b..352e3e224 100644 --- a/application/views/scripts/role/list.phtml +++ b/application/views/scripts/role/list.phtml @@ -28,6 +28,7 @@ translate('Name') ?> translate('Users') ?> translate('Groups') ?> + translate('Inherits From') ?> @@ -44,6 +45,7 @@ escape($role->users) ?> escape($role->groups) ?> + escape($role->parent) ?> qlink( '', diff --git a/library/Icinga/Authentication/AdmissionLoader.php b/library/Icinga/Authentication/AdmissionLoader.php index b9a9919db..049b37c36 100644 --- a/library/Icinga/Authentication/AdmissionLoader.php +++ b/library/Icinga/Authentication/AdmissionLoader.php @@ -3,9 +3,10 @@ namespace Icinga\Authentication; +use Generator; use Icinga\Application\Config; use Icinga\Application\Logger; -use Icinga\Authentication\Role; +use Icinga\Exception\ConfigurationError; use Icinga\Exception\NotReadableError; use Icinga\Data\ConfigObject; use Icinga\User; @@ -16,7 +17,24 @@ use Icinga\Util\StringHelper; */ class AdmissionLoader { + /** @var Role[] */ + protected $roles; + + /** @var ConfigObject */ + protected $roleConfig; + + public function __construct() + { + try { + $this->roleConfig = Config::app('roles'); + } catch (NotReadableError $e) { + Logger::error('Can\'t access roles configuration. An exception was thrown:', $e); + } + } + /** + * Whether the user or groups are a member of the role + * * @param string $username * @param array $userGroups * @param ConfigObject $section @@ -31,10 +49,12 @@ class AdmissionLoader if (in_array('*', $users)) { return true; } + if (in_array($username, $users)) { return true; } } + if (! empty($section->groups)) { $groups = array_map('strtolower', StringHelper::trimSplit($section->groups)); foreach ($userGroups as $userGroup) { @@ -43,9 +63,70 @@ class AdmissionLoader } } } + return false; } + /** + * Process role configuration and yield resulting roles + * + * This will also resolve any parent-child relationships. + * + * @param string $name + * @param ConfigObject $section + * + * @return Generator + * @throws ConfigurationError + */ + protected function loadRole($name, ConfigObject $section) + { + if (! isset($this->roles[$name])) { + $permissions = StringHelper::trimSplit($section->permissions); + $refusals = StringHelper::trimSplit($section->refusals); + + $restrictions = $section->toArray(); + unset($restrictions['users'], $restrictions['groups']); + unset($restrictions['refusals'], $restrictions['permissions']); + + $role = new Role(); + $this->roles[$name] = $role + ->setName($name) + ->setRefusals($refusals) + ->setPermissions($permissions) + ->setRestrictions($restrictions); + + if (isset($section->parent)) { + $parentName = $section->parent; + if (! $this->roleConfig->hasSection($parentName)) { + Logger::error( + 'Failed to parse authentication configuration: Missing parent role "%s" (required by "%s")', + $parentName, + $name + ); + throw new ConfigurationError( + t('Unable to parse authentication configuration. Check the log for more details.') + ); + } + + foreach ($this->loadRole($parentName, $this->roleConfig->getSection($parentName)) as $parent) { + if ($parent->getName() === $parentName) { + $role->setParent($parent); + $parent->addChild($role); + + // Only yield main role once fully assembled + yield $role; + } + + yield $parent; + } + } else { + yield $role; + } + } else { + yield $this->roles[$name]; + } + } + /** * Apply permissions, restrictions and roles to the given user * @@ -53,51 +134,36 @@ class AdmissionLoader */ public function applyRoles(User $user) { - $username = $user->getUsername(); - try { - $roles = Config::app('roles'); - } catch (NotReadableError $e) { - Logger::error( - 'Can\'t get permissions and restrictions for user \'%s\'. An exception was thrown:', - $username, - $e - ); + if ($this->roleConfig === null) { return; } - $userGroups = $user->getGroups(); - $permissions = array(); - $restrictions = array(); - $roleObjs = array(); - foreach ($roles as $roleName => $role) { - if ($this->match($username, $userGroups, $role)) { - $permissionsFromRole = StringHelper::trimSplit($role->permissions); - $refusals = StringHelper::trimSplit($role->refusals); - $permissions = array_merge( - $permissions, - array_diff($permissionsFromRole, $permissions) - ); - $restrictionsFromRole = $role->toArray(); - unset($restrictionsFromRole['users']); - unset($restrictionsFromRole['groups']); - unset($restrictionsFromRole['refusals']); - unset($restrictionsFromRole['permissions']); - foreach ($restrictionsFromRole as $name => $restriction) { - if (! isset($restrictions[$name])) { - $restrictions[$name] = array(); - } - $restrictions[$name][] = $restriction; - } - $roleObj = new Role(); - $roleObjs[] = $roleObj - ->setName($roleName) - ->setRefusals($refusals) - ->setPermissions($permissionsFromRole) - ->setRestrictions($restrictionsFromRole); + $username = $user->getUsername(); + $userGroups = $user->getGroups(); + + $roles = []; + $permissions = []; + $restrictions = []; + foreach ($this->roleConfig as $roleName => $roleConfig) { + if (! isset($roles[$roleName]) && $this->match($username, $userGroups, $roleConfig)) { + foreach ($this->loadRole($roleName, $roleConfig) as $role) { + /** @var Role $role */ + $roles[$role->getName()] = $role; + + $permissions = array_merge( + $permissions, + array_diff($role->getPermissions(), $permissions) + ); + + foreach ($role->getRestrictions() as $name => $restriction) { + $restrictions[$name][] = $restriction; + } + } } } - $user->setPermissions($permissions); + $user->setRestrictions($restrictions); - $user->setRoles($roleObjs); + $user->setPermissions($permissions); + $user->setRoles(array_values($roles)); } } diff --git a/library/Icinga/Authentication/Role.php b/library/Icinga/Authentication/Role.php index a1d166f64..ef2792d83 100644 --- a/library/Icinga/Authentication/Role.php +++ b/library/Icinga/Authentication/Role.php @@ -12,6 +12,20 @@ class Role */ protected $name; + /** + * The role from which to inherit privileges + * + * @var Role + */ + protected $parent; + + /** + * The roles to which privileges are inherited + * + * @var Role[] + */ + protected $children; + /** * Permissions of the role * @@ -57,6 +71,68 @@ class Role return $this; } + /** + * Get the role from which privileges are inherited + * + * @return Role + */ + public function getParent() + { + return $this->parent; + } + + /** + * Set the role from which to inherit privileges + * + * @param Role $parent + * + * @return $this + */ + public function setParent(Role $parent) + { + $this->parent = $parent; + + return $this; + } + + /** + * Get the roles to which privileges are inherited + * + * @return Role[] + */ + public function getChildren() + { + return $this->children; + } + + /** + * Set the roles to which inherit privileges + * + * @param Role[] $children + * + * @return $this + */ + public function setChildren(array $children) + { + $this->children = $children; + + return $this; + } + + /** + * Add a role to which inherit privileges + * + * @param Role $role + * + * @return $this + */ + public function addChild(Role $role) + { + $this->children[] = $role; + + return $this; + } + /** * Get the permissions of the role * @@ -145,10 +221,11 @@ class Role * Whether this role grants the given permission * * @param string $permission + * @param bool $ignoreParent Only evaluate the role's own permissions * * @return bool */ - public function grants($permission) + public function grants($permission, $ignoreParent = false) { foreach ($this->permissions as $grantedPermission) { if ($this->match($grantedPermission, $permission)) { @@ -156,6 +233,10 @@ class Role } } + if (! $ignoreParent && $this->getParent() !== null) { + return $this->getParent()->grants($permission); + } + return false; } @@ -163,10 +244,11 @@ class Role * Whether this role denies the given permission * * @param string $permission + * @param bool $ignoreParent Only evaluate the role's own refusals * * @return bool */ - public function denies($permission) + public function denies($permission, $ignoreParent = false) { foreach ($this->refusals as $refusedPermission) { if ($this->match($refusedPermission, $permission, false)) { @@ -174,6 +256,10 @@ class Role } } + if (! $ignoreParent && $this->getParent() !== null) { + return $this->getParent()->denies($permission); + } + return false; } diff --git a/library/Icinga/Authentication/RolesConfig.php b/library/Icinga/Authentication/RolesConfig.php index a0aeee823..896405069 100644 --- a/library/Icinga/Authentication/RolesConfig.php +++ b/library/Icinga/Authentication/RolesConfig.php @@ -19,6 +19,7 @@ class RolesConfig extends IniRepository { $columns = [ 'roles' => [ + 'parent', 'name', 'users', 'groups', From 6eb0139446082d852cfd594cee4028cc60f7c02a Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Mon, 1 Feb 2021 15:19:17 +0100 Subject: [PATCH 5/8] User: Move `$user:local_name$` handling to class `AdmissionLoader` This way it also adjusts the roles directly, and not just their copies for the user object --- library/Icinga/Authentication/AdmissionLoader.php | 6 +++++- library/Icinga/User.php | 4 ---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/library/Icinga/Authentication/AdmissionLoader.php b/library/Icinga/Authentication/AdmissionLoader.php index 049b37c36..38ad18dd3 100644 --- a/library/Icinga/Authentication/AdmissionLoader.php +++ b/library/Icinga/Authentication/AdmissionLoader.php @@ -155,9 +155,13 @@ class AdmissionLoader array_diff($role->getPermissions(), $permissions) ); - foreach ($role->getRestrictions() as $name => $restriction) { + $roleRestrictions = $role->getRestrictions(); + foreach ($roleRestrictions as $name => & $restriction) { + $restriction = str_replace('$user:local_name$', $user->getLocalUsername(), $restriction); $restrictions[$name][] = $restriction; } + + $role->setRestrictions($roleRestrictions); } } } diff --git a/library/Icinga/User.php b/library/Icinga/User.php index 833a4c84b..2bc9ebec8 100644 --- a/library/Icinga/User.php +++ b/library/Icinga/User.php @@ -252,10 +252,6 @@ class User */ public function setRestrictions(array $restrictions) { - foreach ($restrictions as $name => $restriction) { - $restrictions[$name] = str_replace('$user:local_name$', $this->getLocalUsername(), $restriction); - } - $this->restrictions = $restrictions; return $this; } From 155604e5b1f8d9efd2e63525a722e12d96e3e6f9 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Tue, 2 Feb 2021 10:28:01 +0100 Subject: [PATCH 6/8] docs: Update security chapter --- doc/06-Security.md | 228 +++++++++++++++++---------------------------- 1 file changed, 85 insertions(+), 143 deletions(-) diff --git a/doc/06-Security.md b/doc/06-Security.md index e0848f405..ac2a120cf 100644 --- a/doc/06-Security.md +++ b/doc/06-Security.md @@ -1,190 +1,143 @@ -# Security +# Security -Access control is a vital part of configuring Icinga Web 2 in a secure way. -It is important that not every user that has access to Icinga Web 2 is able -to do any action or to see any host and service. For example, it is useful to allow -only a small group of administrators to change the Icinga Web 2 configuration, -to prevent misconfiguration or security breaches. Another important use case is -creating groups of users which can only see the fraction of the monitoring -environment they are in charge of. +Access control is a vital part of configuring Icinga Web 2 securely. It is important that not every user that has +access to Icinga Web 2 can perform any action or see any host and service. Allow only a small group of administrators +to change the Icinga Web 2 configuration to prevent mis-configuration and security breaches. Define different rules +to users and groups of users which should only see a part of the monitoring environment they're in charge of. -This chapter will describe how to do the security configuration of Icinga Web 2 -and how to apply permissions and restrictions to users or groups of users. +This chapter will describe how to configure such rules in Icinga Web 2 and how permissions, refusals, restrictions +and role inheritance work. -## Basics +## Basics -Icinga Web 2 access control is done by defining **roles** that associate permissions -and restrictions with **users** and **groups**. There are two general kinds of -things to which access can be managed: actions and objects. +Icinga Web 2 access control is done by defining **roles** that associate privileges with **users** and **groups**. +Privileges of a role consist of **permissions**, **refusals** and **restrictions**. A role can **inherit** privileges +from another role. +### Role Memberships -### Actions - -Actions are all the things an Icinga Web 2 user can do, like changing a certain configuration, -changing permissions or sending a command to an Icinga 2 instance. -All actions must be be **allowed explicitly** using permissions. - -A permission is a simple list of identifiers of actions a user is -allowed to do. Permissions are described in greater detail in the -section [Permissions](06-Security.md#permissions). - -### Objects - -There are all kinds of different objects in Icinga Web 2: Hosts, Services, Notifications, Downtimes and Events. - -By default, a user can **see everything**, but it is possible to **explicitly restrict** what each user can see using restrictions. - -Restrictions are complex filter queries that describe what objects should be displayed to a user. Restrictions are described -in greater detail in the section [Restrictions](06-Security.md#restrictions). - -### Users - -Anyone who can **login** to Icinga Web 2 is considered a user and can be referenced to by the -**user name** used during login. -For example, there might be user called **jdoe** authenticated -using Active Directory, and a user **icingaadmin** that is authenticated using a MySQL-Database as backend. -In the configuration, both can be referenced to by using their user names **icingaadmin** or **jdoe**. - -Icinga Web 2 users and groups are not configured by a configuration file, but provided by -an **authentication backend**. For extended information on setting up authentication backends and managing users, please read the chapter [Authentication](05-Authentication.md#authentication). - +A role is tied to users or groups of users. Upon login, a user's roles are identified by the username or names of +groups the user is a member of. > **Note** > -> Since Icinga Web 2, users in the Icinga configuration and the web authentication are separated, to allow -> use of external authentication providers. This means that users and groups defined in the Icinga configuration are not available to Icinga Web 2. Instead it uses its own authentication -> backend to fetch users and groups from, which must be configured separately. +> Since Icinga Web 2, users in the Icinga configuration and the web authentication are separated, to allow use of +> external authentication providers. This means that users and groups defined in the Icinga configuration are not +> available to Icinga Web 2. It uses its own authentication backend to fetch users and groups from, +> [which must be configured separately](05-Authentication.md#authentication). -#### Managing Users +### Privileges -When using a [Database -as authentication backend](05-Authentication.md#authentication-configuration-db-authentication), it is possible to create, add and delete users directly in the frontend. This configuration -can be found at **Configuration > Authentication > Users **. +Permissions are used to grant access. Whether this means that a user can see a certain area or perform a distinct +action is fully up to the permission in question. Without granting a permission, the user will lack access and won't +see the area or perform the action. -### Groups +Refusals are used to deny access. So they're the exact opposite of permissions. Most permissions can be refused. +Refusing a permission will block the user's access no matter if another role grants the permission. Refusals +override permissions. -If there is a big amount of users to manage, it would be tedious to specify each user -separately when regularly referring to the same group of users. Because of that, it is possible to group users. -A user can be member of multiple groups and will inherit all permissions and restrictions. +Restrictions are expressions that limit access. What this exactly means is up to how the restriction is being utilized. +Without any restriction, a user is supposed to see *everything*. A user that occupies multiple roles, which all define +a restriction of the same type, will see *more*. -Like users, groups are identified solely by their **name** that is provided by - a **group backend**. For extended information on setting up group backends, - please read the chapter [Authentication](05-Authentication.md#authentication). +## Roles +A user can occupy multiple roles. Permissions and restrictions stack up in this case, thus will grant *more* access. +Refusals still override permissions however. A refusal of one role negates the granted permission of any other role. -#### Managing Groups +### Configuration -When using a [Database as an authentication backend](05-Authentication.md#authentication-configuration-db-authentication), -it is possible to manage groups and group memberships directly in the frontend. This configuration -can be found at **Configuration > Authentication > User Groups **. +Roles can be changed either through the UI, by navigating to the page **Configuration > Authentication > Roles**, +or by editing the configuration file `/etc/icingaweb2/roles.ini`. -## Roles +#### Example -A role defines a set of **permissions** and **restrictions** and assigns -those to **users** and **groups**. For example, a role **admins** could define that certain -users have access to all configuration options, or another role **support** -could define that a list of users or groups is restricted to see only hosts and services -that match a specific query. - -The actual permission of a certain user will be determined by merging the permissions -and restrictions of the user itself and all the groups the user is member of. Permissions can -be simply added up, while restrictions follow a slighty more complex pattern, that is described -in the section [Stacking Filters](06-Security.md#stacking-filters). - -### Configuration - -Roles can be changed either through the icingaweb2 interface, by navigation -to the page **Configuration > Authentication > Roles**, or through editing the -configuration file: - -``` -vim /etc/icingaweb2/roles.ini -``` - -#### Introducing Example - -To get you a quick start, here is an example of what a role definition could look like: +The following shows a role definition from the configuration file mentioned above: ``` [winadmin] users = "jdoe, janedoe" groups = "admin" -permissions = "config/*, monitoring/commands/schedule-check" +permissions = "config/*, module/monitoring, monitoring/commands/schedule-check" +refusals = "config/authentication" monitoring/filter/objects = "host_name=*win*" ``` +This describes a role with the name `winadmin`. The users `jdoe` and `janedoe` are members of it. Just like the +members of group `admin` are. Full configuration access is granted, except of the authentication configuration, +which is forbidden. It also grants access to the *monitoring* module which includes the ability to re-schedule +checks, but only on objects related to hosts whose name contain `win`. -This example creates a role called **winadmin**, that grants all permissions in `config/*` and `monitoring/commands/schedule-check` and additionally only -allows the hosts and services that match the filter `host_name=*win*` to be displayed. The users -**jdoe** and **janedoe** and all members of the group **admin** will be affected -by this role. - - -#### Syntax +#### Syntax Each role is defined as a section, with the name of the role as section name. The following -attributes can be defined for each role in a default Icinga Web 2 installation: - +options can be defined for each role in a default Icinga Web 2 installation: Name | Description --------------------------|----------------------------------------------- -users | Comma-separated list of user **user names** that are affected by this role. -groups | Comma-separated list of **group names** that are affected by this role. +parent | The name of the role from which to inherit privileges. +users | Comma-separated list of **usernames** that should occupy this role. +groups | Comma-separated list of **group names** whose users should occupy this role. permissions | Comma-separated list of **permissions** granted by this role. -monitoring/filter/objects | **Filter expression** that restricts the access to services and hosts. +refusals | Comma-separated list of **permissions** refused by this role. +monitoring/filter/objects | **Filter expression** that restricts the access to monitoring objects. +### Inheritance +A role can inherit privileges from another role. Privileges are then combined the same way as if a user occupies +all roles in the inheritance path. Or to rephrase that, each role shares its members with all of its parents. -## Permissions +## Permissions -Permissions can be used to allow users or groups certain **actions**. By default, -all actions are **prohibited** and must be allowed explicitly by a role for any user. +Each permission in Icinga Web 2 is denoted by a **namespaced key**, which is used to group permissions. All permissions +that affect the configuration of Icinga Web 2, are in a namespace called **config**, while all configuration options +that affect modules are covered by the permission `config/modules`. -Each action in Icinga Web 2 is denoted by a **namespaced key**, which is used to order and -group those actions. All actions that affect the configuration of Icinga Web 2, are in a -namespace called **config**, while all configurations that affect modules -are in the namespace `config/modules` - -**Wildcards** can be used to grant permission for all actions in a certain namespace. -The permission `config/*` would grant permission to all configuration actions, -while just specifying a wildcard `*` would give permission for all actions. +**Wildcards** can be used to grant all permissions in a certain namespace. The permission `config/*` grants access to +all configuration options. Just specifying a wildcard `*` will grant all permissions. Access to modules is restricted to users who have the related module permission granted. Icinga Web 2 provides a module permission in the format `module/` for each installed module. -When multiple roles assign permissions to the same user (either directly or indirectly -through a group) all permissions are added together to get the users actual permission set. - -### Global Permissions +### General Permissions Name | Permits --------------------------|----------------------------------------------- \* | allow everything, including module-specific permissions config/\* | allow all configuration actions config/modules | allow enabling or disabling modules -module/<moduleName> | allow access to module <moduleName> +module/`` | allow access to module `` (e.g. `module/monitoring`) - -### Monitoring Module Permissions +### Monitoring Module Permissions The built-in monitoring module defines an additional set of permissions, that is described in detail in the monitoring module documentation. +## Restrictions -## Restrictions +Restrictions can be used to define what a user can see by specifying an expression that applies to a defined set of +data. By default, when no restrictions are defined, a user will be able to see the entire data that is available. -Restrictions can be used to define what a user or group can see by specifying -a filter expression that applies to a defined set of data. By default, when no -restrictions are defined, a user will be able to see every information that is available. +The syntax of the expression used to define a particular restriction varies. This can be a comma-separated list of +terms, or [a full-blown filter](06-Security.md#filter-expressions). For more details on particular restrictions, +check the table below or the module's documentation providing the restriction. -A restrictions is always specified for a certain **filter directive**, that defines what -data the filter is applied to. The **filter directive** is a simple identifier, that was -defined in an Icinga Web 2 module. The only filter directive that is available -in a default installation, is the `monitoring/filter/objects` directive, defined by the monitoring module, -that can be used to apply filter to hosts and services. This directive was previously -mentioned in the section [Syntax](06-Security.md#syntax). +### General Restrictions -### Filter Expressions +Name | Applies to +--------------------------|------------------------------------------------------------------------------------------ +application/share/users | which users a user can share navigation items with (comma-separated list of usernames) +application/share/groups | which groups a user can share navigation items with (comma-separated list of group names) + +### Username placeholder + +It is possible to reference the local username (without the domain part) of the user in restrictions. To accomplish +this, put the macro `$user:local_name$` in the restriction where you want it to appear. + +This can come in handy if you have e.g. an attribute on hosts or services defining which user is responsible for it: +`_host_deputy=$user:local_name$|_service_deputy=$user:local_name$` + +### Filter Expressions Filters operate on columns. A complete list of all available filter columns on hosts and services can be found in the monitoring module documentation. @@ -214,18 +167,7 @@ results of this query instead: | +--- service_handled = 0 -#### Username placeholder - -The string `$user:local_name$` is replaced by the local username (without the domain part) of the logged on user while evaluating restrictions. -This can come in handy if you have some kind of attribute on host or service level defining which user is responsible for a certain host or service. - -**Example** - -``` -monitoring/filter/objects = (_responsible=$user:local_name$|_deputy=$user:local_name$) -``` - -#### Stacking Filters +#### Stacking Filters When multiple roles assign restrictions to the same user, either directly or indirectly through a group, all filters will be combined using an **OR-Clause**, resulting in the final @@ -246,7 +188,7 @@ expression: As a result, a user is be able to see hosts that are matched by **ANY** of the filter expressions. The following examples will show the usefulness of this behavior: -#### Example 1: Negation +#### Example 1: Negation ``` [winadmin] @@ -266,7 +208,7 @@ Will only match hosts and services whose host name does **not** contain **win** Notice that because of the behavior of two stacking filters, a user that is member of **windows-admins** and **web-admins**, will now be able to see both, Windows and non-Windows hosts and services. -#### Example 2: Hostgroups +#### Example 2: Hostgroups ``` [unix-server] From 429a70f05f311654cf406c83a9816624ab0e3a9f Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Tue, 2 Feb 2021 13:20:29 +0100 Subject: [PATCH 7/8] Auth: Allow to ignore any and all restrictions --- application/forms/Security/RoleForm.php | 16 +++++++++- doc/06-Security.md | 9 ++++++ .../Icinga/Authentication/AdmissionLoader.php | 10 ++++-- library/Icinga/Authentication/Role.php | 31 +++++++++++++++++++ library/Icinga/Authentication/RolesConfig.php | 1 + 5 files changed, 64 insertions(+), 3 deletions(-) diff --git a/application/forms/Security/RoleForm.php b/application/forms/Security/RoleForm.php index d1e8fd03f..39346c945 100644 --- a/application/forms/Security/RoleForm.php +++ b/application/forms/Security/RoleForm.php @@ -195,8 +195,19 @@ class RoleForm extends RepositoryForm 'description' => $this->translate('Everything is allowed') ] ); + $this->addElement( + 'checkbox', + 'unrestricted', + [ + 'autosubmit' => true, + 'uncheckedValue' => null, + 'label' => $this->translate('Unrestricted Access'), + 'description' => $this->translate('Access to any data is completely unrestricted') + ] + ); $hasAdminPerm = isset($formData[self::WILDCARD_NAME]) && $formData[self::WILDCARD_NAME]; + $isUnrestricted = isset($formData['unrestricted']) && $formData['unrestricted']; foreach ($this->providedPermissions as $moduleName => $permissionList) { $this->sortPermissions($permissionList); @@ -301,7 +312,9 @@ class RoleForm extends RepositoryForm '/​', isset($spec['label']) ? $spec['label'] : $spec['name'] ), - 'description' => $spec['description'] + 'description' => $spec['description'], + 'style' => $isUnrestricted ? 'text-decoration:line-through;' : '', + 'readonly' => $isUnrestricted ?: null ] ) ->getElement($name) @@ -339,6 +352,7 @@ class RoleForm extends RepositoryForm 'name' => $role->name, 'users' => $role->users, 'groups' => $role->groups, + 'unrestricted' => $role->unrestricted, self::WILDCARD_NAME => (bool) preg_match('~(?permissions) ]; diff --git a/doc/06-Security.md b/doc/06-Security.md index ac2a120cf..0fb1d9e63 100644 --- a/doc/06-Security.md +++ b/doc/06-Security.md @@ -80,8 +80,17 @@ users | Comma-separated list of **usernames** that should oc groups | Comma-separated list of **group names** whose users should occupy this role. permissions | Comma-separated list of **permissions** granted by this role. refusals | Comma-separated list of **permissions** refused by this role. +unrestricted | If set to `1`, owners of this role are not restricted in any way (Default: `0`) monitoring/filter/objects | **Filter expression** that restricts the access to monitoring objects. +### Administrative Roles + +Roles that have the wildcard `*` as permission, have full access and don't need any further permissions. However, +they are still affected by refusals. + +Unrestricted roles are supposed to allow users to access data without being limited to a subset of it. Once a user +occupies an unrestricted role, restrictions of the same and any other role are ignored. + ### Inheritance A role can inherit privileges from another role. Privileges are then combined the same way as if a user occupies diff --git a/library/Icinga/Authentication/AdmissionLoader.php b/library/Icinga/Authentication/AdmissionLoader.php index 38ad18dd3..1b8bb239e 100644 --- a/library/Icinga/Authentication/AdmissionLoader.php +++ b/library/Icinga/Authentication/AdmissionLoader.php @@ -93,7 +93,8 @@ class AdmissionLoader ->setName($name) ->setRefusals($refusals) ->setPermissions($permissions) - ->setRestrictions($restrictions); + ->setRestrictions($restrictions) + ->setIsUnrestricted($section->get('unrestricted', false)); if (isset($section->parent)) { $parentName = $section->parent; @@ -144,6 +145,7 @@ class AdmissionLoader $roles = []; $permissions = []; $restrictions = []; + $isUnrestricted = false; foreach ($this->roleConfig as $roleName => $roleConfig) { if (! isset($roles[$roleName]) && $this->match($username, $userGroups, $roleConfig)) { foreach ($this->loadRole($roleName, $roleConfig) as $role) { @@ -162,11 +164,15 @@ class AdmissionLoader } $role->setRestrictions($roleRestrictions); + + if (! $isUnrestricted) { + $isUnrestricted = $role->isUnrestricted(); + } } } } - $user->setRestrictions($restrictions); + $user->setRestrictions($isUnrestricted ? [] : $restrictions); $user->setPermissions($permissions); $user->setRoles(array_values($roles)); } diff --git a/library/Icinga/Authentication/Role.php b/library/Icinga/Authentication/Role.php index ef2792d83..1fca29b9d 100644 --- a/library/Icinga/Authentication/Role.php +++ b/library/Icinga/Authentication/Role.php @@ -26,6 +26,13 @@ class Role */ protected $children; + /** + * Whether restrictions should not apply to owners of the role + * + * @var bool + */ + protected $unrestricted = false; + /** * Permissions of the role * @@ -133,6 +140,30 @@ class Role return $this; } + /** + * Get whether restrictions should not apply to owners of the role + * + * @return bool + */ + public function isUnrestricted() + { + return $this->unrestricted; + } + + /** + * Set whether restrictions should not apply to owners of the role + * + * @param bool $state + * + * @return $this + */ + public function setIsUnrestricted($state) + { + $this->unrestricted = (bool) $state; + + return $this; + } + /** * Get the permissions of the role * diff --git a/library/Icinga/Authentication/RolesConfig.php b/library/Icinga/Authentication/RolesConfig.php index 896405069..ac5695f11 100644 --- a/library/Icinga/Authentication/RolesConfig.php +++ b/library/Icinga/Authentication/RolesConfig.php @@ -25,6 +25,7 @@ class RolesConfig extends IniRepository 'groups', 'refusals', 'permissions', + 'unrestricted', 'application/share/users', 'application/share/groups' ] From cc65164a6705ecd6b007f1ee475b7505a1a85f31 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Thu, 18 Feb 2021 08:52:57 +0100 Subject: [PATCH 8/8] Adjust global permissions --- application/controllers/AccountController.php | 2 +- .../controllers/AnnouncementsController.php | 6 +- application/controllers/ConfigController.php | 90 +++++++++++-------- application/controllers/GroupController.php | 61 +++++++------ .../controllers/NavigationController.php | 6 +- application/controllers/RoleController.php | 64 ++++++++----- application/controllers/UserController.php | 63 +++++++------ .../UsergroupbackendController.php | 2 +- .../forms/Navigation/NavigationConfigForm.php | 8 +- application/forms/PreferenceForm.php | 2 +- application/forms/Security/RoleForm.php | 81 ++++++++++++----- .../views/scripts/announcements/index.phtml | 6 +- .../scripts/config/userbackend/reorder.phtml | 4 + application/views/scripts/group/list.phtml | 4 +- application/views/scripts/group/show.phtml | 6 +- application/views/scripts/user/list.phtml | 4 +- application/views/scripts/user/show.phtml | 4 +- doc/06-Security.md | 25 ++++-- library/Icinga/Application/Web.php | 2 +- .../Icinga/Authentication/AdmissionLoader.php | 52 +++++++++++ library/Icinga/User.php | 10 +++ library/Icinga/Web/Menu.php | 13 ++- .../controllers/ConfigController.php | 2 +- 23 files changed, 345 insertions(+), 172 deletions(-) diff --git a/application/controllers/AccountController.php b/application/controllers/AccountController.php index de1faf153..795935fb5 100644 --- a/application/controllers/AccountController.php +++ b/application/controllers/AccountController.php @@ -43,7 +43,7 @@ class AccountController extends Controller $config = Config::app()->getSection('global'); $user = $this->Auth()->getUser(); if ($user->getAdditional('backend_type') === 'db') { - if ($user->can('*') || ! $user->can('no-user/password-change')) { + if ($user->can('user/password-change')) { try { $userBackend = UserBackend::create($user->getAdditional('backend_name')); } catch (ConfigurationError $e) { diff --git a/application/controllers/AnnouncementsController.php b/application/controllers/AnnouncementsController.php index 82d012fb6..ee7fd4c74 100644 --- a/application/controllers/AnnouncementsController.php +++ b/application/controllers/AnnouncementsController.php @@ -61,7 +61,7 @@ class AnnouncementsController extends Controller */ public function newAction() { - $this->assertPermission('admin'); + $this->assertPermission('application/announcements'); $form = $this->prepareForm()->add(); $form->handleRequest(); @@ -73,7 +73,7 @@ class AnnouncementsController extends Controller */ public function updateAction() { - $this->assertPermission('admin'); + $this->assertPermission('application/announcements'); $form = $this->prepareForm()->edit($this->params->getRequired('id')); try { @@ -89,7 +89,7 @@ class AnnouncementsController extends Controller */ public function removeAction() { - $this->assertPermission('admin'); + $this->assertPermission('application/announcements'); $form = $this->prepareForm()->remove($this->params->getRequired('id')); try { diff --git a/application/controllers/ConfigController.php b/application/controllers/ConfigController.php index 9e070a6da..29e75d27d 100644 --- a/application/controllers/ConfigController.php +++ b/application/controllers/ConfigController.php @@ -35,24 +35,33 @@ class ConfigController extends Controller public function createApplicationTabs() { $tabs = $this->getTabs(); - $tabs->add('general', array( - 'title' => $this->translate('Adjust the general configuration of Icinga Web 2'), - 'label' => $this->translate('General'), - 'url' => 'config/general', - 'baseTarget' => '_main' - )); - $tabs->add('resource', array( - 'title' => $this->translate('Configure which resources are being utilized by Icinga Web 2'), - 'label' => $this->translate('Resources'), - 'url' => 'config/resource', - 'baseTarget' => '_main' - )); - $tabs->add('authentication', array( - 'title' => $this->translate('Configure the user and group backends'), - 'label' => $this->translate('Authentication'), - 'url' => 'config/userbackend', - 'baseTarget' => '_main' - )); + if ($this->hasPermission('config/general')) { + $tabs->add('general', array( + 'title' => $this->translate('Adjust the general configuration of Icinga Web 2'), + 'label' => $this->translate('General'), + 'url' => 'config/general', + 'baseTarget' => '_main' + )); + } + if ($this->hasPermission('config/resources')) { + $tabs->add('resource', array( + 'title' => $this->translate('Configure which resources are being utilized by Icinga Web 2'), + 'label' => $this->translate('Resources'), + 'url' => 'config/resource', + 'baseTarget' => '_main' + )); + } + if ($this->hasPermission('config/access-control/users') + || $this->hasPermission('config/access-control/groups') + ) { + $tabs->add('authentication', array( + 'title' => $this->translate('Configure the user and group backends'), + 'label' => $this->translate('Access Control Backends'), + 'url' => 'config/userbackend', + 'baseTarget' => '_main' + )); + } + return $tabs; } @@ -66,7 +75,15 @@ class ConfigController extends Controller */ public function indexAction() { - $this->redirectNow('config/general'); + if ($this->hasPermission('config/general')) { + $this->redirectNow('config/general'); + } elseif ($this->hasPermission('config/resources')) { + $this->redirectNow('config/resource'); + } elseif ($this->hasPermission('config/access-control/*')) { + $this->redirectNow('config/userbackend'); + } else { + throw new SecurityException('No permission to configure Icinga Web 2'); + } } /** @@ -76,7 +93,7 @@ class ConfigController extends Controller */ public function generalAction() { - $this->assertPermission('config/application/general'); + $this->assertPermission('config/general'); $form = new GeneralConfigForm(); $form->setIniConfig(Config::app()); $form->handleRequest(); @@ -207,14 +224,17 @@ class ConfigController extends Controller */ public function userbackendAction() { - $this->assertPermission('config/application/userbackend'); - $this->assertPermission('config/application/usergroupbackend'); - $form = new UserBackendReorderForm(); - $form->setIniConfig(Config::app('authentication')); - $form->handleRequest(); + if ($this->hasPermission('config/access-control/users')) { + $form = new UserBackendReorderForm(); + $form->setIniConfig(Config::app('authentication')); + $form->handleRequest(); + $this->view->form = $form; + } + + if ($this->hasPermission('config/access-control/groups')) { + $this->view->backendNames = Config::app('groups'); + } - $this->view->form = $form; - $this->view->backendNames = Config::app('groups'); $this->createApplicationTabs()->activate('authentication'); $this->view->title = $this->translate('Authentication'); $this->render('userbackend/reorder'); @@ -225,7 +245,7 @@ class ConfigController extends Controller */ public function createuserbackendAction() { - $this->assertPermission('config/application/userbackend'); + $this->assertPermission('config/access-control/users'); $form = new UserBackendConfigForm(); $form ->setRedirectUrl('config/userbackend') @@ -238,7 +258,7 @@ class ConfigController extends Controller try { $form->setResourceConfig(ResourceFactory::getResourceConfigs()); } catch (ConfigurationError $e) { - if ($this->hasPermission('config/application/resources')) { + if ($this->hasPermission('config/resources')) { Notification::error($e->getMessage()); $this->redirectNow('config/createresource'); } @@ -272,7 +292,7 @@ class ConfigController extends Controller */ public function edituserbackendAction() { - $this->assertPermission('config/application/userbackend'); + $this->assertPermission('config/access-control/users'); $backendName = $this->params->getRequired('backend'); $form = new UserBackendConfigForm(); @@ -311,7 +331,7 @@ class ConfigController extends Controller */ public function removeuserbackendAction() { - $this->assertPermission('config/application/userbackend'); + $this->assertPermission('config/access-control/users'); $backendName = $this->params->getRequired('backend'); $backendForm = new UserBackendConfigForm(); @@ -344,7 +364,7 @@ class ConfigController extends Controller */ public function resourceAction() { - $this->assertPermission('config/application/resources'); + $this->assertPermission('config/resources'); $this->view->resources = Config::app('resources', true)->getConfigObject() ->setKeyColumn('name') ->select() @@ -358,7 +378,7 @@ class ConfigController extends Controller */ public function createresourceAction() { - $this->assertPermission('config/application/resources'); + $this->assertPermission('config/resources'); $this->getTabs()->add('resources/new', array( 'label' => $this->translate('New Resource'), 'url' => Url::fromRequest() @@ -379,7 +399,7 @@ class ConfigController extends Controller */ public function editresourceAction() { - $this->assertPermission('config/application/resources'); + $this->assertPermission('config/resources'); $this->getTabs()->add('resources/update', array( 'label' => $this->translate('Update Resource'), 'url' => Url::fromRequest() @@ -399,7 +419,7 @@ class ConfigController extends Controller */ public function removeresourceAction() { - $this->assertPermission('config/application/resources'); + $this->assertPermission('config/resources'); $this->getTabs()->add('resources/remove', array( 'label' => $this->translate('Remove Resource'), 'url' => Url::fromRequest() diff --git a/application/controllers/GroupController.php b/application/controllers/GroupController.php index c91c033a2..7e3da5ae6 100644 --- a/application/controllers/GroupController.php +++ b/application/controllers/GroupController.php @@ -33,7 +33,7 @@ class GroupController extends AuthBackendController */ public function listAction() { - $this->assertPermission('config/authentication/groups/show'); + $this->assertPermission('config/access-control/groups'); $this->createListTabs()->activate('group/list'); $backendNames = array_map( function ($b) { @@ -90,7 +90,7 @@ class GroupController extends AuthBackendController */ public function showAction() { - $this->assertPermission('config/authentication/groups/show'); + $this->assertPermission('config/access-control/groups'); $groupName = $this->params->getRequired('group'); $backend = $this->getUserGroupBackend($this->params->getRequired('backend')); @@ -125,7 +125,7 @@ class GroupController extends AuthBackendController $this->view->members = $members; $this->createShowTabs($backend->getName(), $groupName)->activate('group/show'); - if ($this->hasPermission('config/authentication/groups/edit') && $backend instanceof Reducible) { + if ($this->hasPermission('config/access-control/groups') && $backend instanceof Reducible) { $removeForm = new Form(); $removeForm->setUidDisabled(); $removeForm->setAttrib('class', 'inline'); @@ -161,7 +161,7 @@ class GroupController extends AuthBackendController */ public function addAction() { - $this->assertPermission('config/authentication/groups/add'); + $this->assertPermission('config/access-control/groups'); $backend = $this->getUserGroupBackend($this->params->getRequired('backend'), 'Icinga\Data\Extensible'); $form = new UserGroupForm(); $form->setRedirectUrl(Url::fromPath('group/list', array('backend' => $backend->getName()))); @@ -176,7 +176,7 @@ class GroupController extends AuthBackendController */ public function editAction() { - $this->assertPermission('config/authentication/groups/edit'); + $this->assertPermission('config/access-control/groups'); $groupName = $this->params->getRequired('group'); $backend = $this->getUserGroupBackend($this->params->getRequired('backend'), 'Icinga\Data\Updatable'); @@ -200,7 +200,7 @@ class GroupController extends AuthBackendController */ public function removeAction() { - $this->assertPermission('config/authentication/groups/remove'); + $this->assertPermission('config/access-control/groups'); $groupName = $this->params->getRequired('group'); $backend = $this->getUserGroupBackend($this->params->getRequired('backend'), 'Icinga\Data\Reducible'); @@ -222,7 +222,7 @@ class GroupController extends AuthBackendController */ public function addmemberAction() { - $this->assertPermission('config/authentication/groups/edit'); + $this->assertPermission('config/access-control/groups'); $groupName = $this->params->getRequired('group'); $backend = $this->getUserGroupBackend($this->params->getRequired('backend'), 'Icinga\Data\Extensible'); @@ -249,7 +249,7 @@ class GroupController extends AuthBackendController */ public function removememberAction() { - $this->assertPermission('config/authentication/groups/edit'); + $this->assertPermission('config/access-control/groups'); $this->assertHttpMethod('POST'); $groupName = $this->params->getRequired('group'); $backend = $this->getUserGroupBackend($this->params->getRequired('backend'), 'Icinga\Data\Reducible'); @@ -368,26 +368,32 @@ class GroupController extends AuthBackendController protected function createListTabs() { $tabs = $this->getTabs(); - $tabs->add( - 'role/list', - array( - 'baseTarget' => '_main', - 'label' => $this->translate('Roles'), - 'title' => $this->translate( - 'Configure roles to permit or restrict users and groups accessing Icinga Web 2' - ), - 'url' => 'role/list' - ) - ); - $tabs->add( - 'user/list', - array( - 'title' => $this->translate('List users of authentication backends'), - 'label' => $this->translate('Users'), - 'url' => 'user/list' - ) - ); + if ($this->hasPermission('config/access-control/roles')) { + $tabs->add( + 'role/list', + array( + 'baseTarget' => '_main', + 'label' => $this->translate('Roles'), + 'title' => $this->translate( + 'Configure roles to permit or restrict users and groups accessing Icinga Web 2' + ), + 'url' => 'role/list' + ) + ); + } + + if ($this->hasPermission('config/access-control/users')) { + $tabs->add( + 'user/list', + array( + 'title' => $this->translate('List users of authentication backends'), + 'label' => $this->translate('Users'), + 'url' => 'user/list' + ) + ); + } + $tabs->add( 'group/list', array( @@ -396,6 +402,7 @@ class GroupController extends AuthBackendController 'url' => 'group/list' ) ); + return $tabs; } } diff --git a/application/controllers/NavigationController.php b/application/controllers/NavigationController.php index 6bcdee76a..403c8acbf 100644 --- a/application/controllers/NavigationController.php +++ b/application/controllers/NavigationController.php @@ -161,7 +161,7 @@ class NavigationController extends Controller */ public function sharedAction() { - $this->assertPermission('config/application/navigation'); + $this->assertPermission('config/navigation'); $ds = new ArrayDatasource($this->fetchSharedNavigationItemConfigs()); $query = $ds->select(); @@ -259,7 +259,7 @@ class NavigationController extends Controller $referrer = $this->params->get('referrer', 'index'); $user = $this->Auth()->getUser(); - if ($user->can('config/application/navigation')) { + if ($user->can('config/navigation')) { $itemOwner = $this->params->get('owner', $user->getUsername()); } else { $itemOwner = $user->getUsername(); @@ -354,7 +354,7 @@ class NavigationController extends Controller */ public function unshareAction() { - $this->assertPermission('config/application/navigation'); + $this->assertPermission('config/navigation'); $this->assertHttpMethod('POST'); // TODO: I'd like these being form fields diff --git a/application/controllers/RoleController.php b/application/controllers/RoleController.php index a65510471..ea35fcf07 100644 --- a/application/controllers/RoleController.php +++ b/application/controllers/RoleController.php @@ -6,6 +6,7 @@ namespace Icinga\Controllers; use Icinga\Authentication\RolesConfig; use Icinga\Exception\NotFoundError; use Icinga\Forms\Security\RoleForm; +use Icinga\Security\SecurityException; use Icinga\Web\Controller\AuthBackendController; /** @@ -22,6 +23,19 @@ class RoleController extends AuthBackendController parent::init(); } + public function indexAction() + { + if ($this->hasPermission('config/access-control/roles')) { + $this->redirectNow('role/list'); + } elseif ($this->hasPermission('config/access-control/users')) { + $this->redirectNow('user/list'); + } elseif ($this->hasPermission('config/access-control/groups')) { + $this->redirectNow('group/list'); + } else { + throw new SecurityException('No permission to configure Icinga Web 2'); + } + } + /** * List roles * @@ -29,7 +43,7 @@ class RoleController extends AuthBackendController */ public function listAction() { - $this->assertPermission('config/authentication/roles/show'); + $this->assertPermission('config/access-control/roles'); $this->createListTabs()->activate('role/list'); $this->view->roles = (new RolesConfig()) ->select(); @@ -54,7 +68,7 @@ class RoleController extends AuthBackendController */ public function addAction() { - $this->assertPermission('config/authentication/roles/add'); + $this->assertPermission('config/access-control/roles'); $role = new RoleForm(); $role->setRedirectUrl('role/list'); @@ -72,7 +86,7 @@ class RoleController extends AuthBackendController */ public function editAction() { - $this->assertPermission('config/authentication/roles/edit'); + $this->assertPermission('config/access-control/roles'); $name = $this->params->getRequired('role'); $role = new RoleForm(); @@ -95,7 +109,7 @@ class RoleController extends AuthBackendController */ public function removeAction() { - $this->assertPermission('config/authentication/roles/remove'); + $this->assertPermission('config/access-control/roles'); $name = $this->params->getRequired('role'); $role = new RoleForm(); @@ -128,25 +142,31 @@ class RoleController extends AuthBackendController 'Configure roles to permit or restrict users and groups accessing Icinga Web 2' ), 'url' => 'role/list' + ) + ); + + if ($this->hasPermission('config/access-control/users')) { + $tabs->add( + 'user/list', + array( + 'title' => $this->translate('List users of authentication backends'), + 'label' => $this->translate('Users'), + 'url' => 'user/list' + ) + ); + } + + if ($this->hasPermission('config/access-control/groups')) { + $tabs->add( + 'group/list', + array( + 'title' => $this->translate('List groups of user group backends'), + 'label' => $this->translate('User Groups'), + 'url' => 'group/list' + ) + ); + } - ) - ); - $tabs->add( - 'user/list', - array( - 'title' => $this->translate('List users of authentication backends'), - 'label' => $this->translate('Users'), - 'url' => 'user/list' - ) - ); - $tabs->add( - 'group/list', - array( - 'title' => $this->translate('List groups of user group backends'), - 'label' => $this->translate('User Groups'), - 'url' => 'group/list' - ) - ); return $tabs; } } diff --git a/application/controllers/UserController.php b/application/controllers/UserController.php index c10b19b53..39b056b1e 100644 --- a/application/controllers/UserController.php +++ b/application/controllers/UserController.php @@ -33,7 +33,7 @@ class UserController extends AuthBackendController */ public function listAction() { - $this->assertPermission('config/authentication/users/show'); + $this->assertPermission('config/access-control/users'); $this->createListTabs()->activate('user/list'); $backendNames = array_map( function ($b) { @@ -91,7 +91,7 @@ class UserController extends AuthBackendController */ public function showAction() { - $this->assertPermission('config/authentication/users/show'); + $this->assertPermission('config/access-control/users'); $userName = $this->params->getRequired('user'); $backend = $this->getUserBackend($this->params->getRequired('backend')); @@ -127,7 +127,7 @@ class UserController extends AuthBackendController $memberships ); - if ($this->hasPermission('config/authentication/groups/edit')) { + if ($this->hasPermission('config/access-control/groups')) { $extensibleBackends = $this->loadUserGroupBackends('Icinga\Data\Extensible'); $this->view->showCreateMembershipLink = ! empty($extensibleBackends); } else { @@ -139,7 +139,7 @@ class UserController extends AuthBackendController $this->view->memberships = $memberships; $this->createShowTabs($backend->getName(), $userName)->activate('user/show'); - if ($this->hasPermission('config/authentication/groups/edit')) { + if ($this->hasPermission('config/access-control/groups')) { $removeForm = new Form(); $removeForm->setUidDisabled(); $removeForm->setAttrib('class', 'inline'); @@ -170,7 +170,7 @@ class UserController extends AuthBackendController $admissionLoader = new AdmissionLoader(); $admissionLoader->applyRoles($userObj); $this->view->userObj = $userObj; - $this->view->allowedToEditRoles = $this->hasPermission('config/authentication/roles/edit'); + $this->view->allowedToEditRoles = $this->hasPermission('config/access-control/groups'); } /** @@ -178,7 +178,7 @@ class UserController extends AuthBackendController */ public function addAction() { - $this->assertPermission('config/authentication/users/add'); + $this->assertPermission('config/access-control/users'); $backend = $this->getUserBackend($this->params->getRequired('backend'), 'Icinga\Data\Extensible'); $form = new UserForm(); $form->setRedirectUrl(Url::fromPath('user/list', array('backend' => $backend->getName()))); @@ -193,7 +193,7 @@ class UserController extends AuthBackendController */ public function editAction() { - $this->assertPermission('config/authentication/users/edit'); + $this->assertPermission('config/access-control/users'); $userName = $this->params->getRequired('user'); $backend = $this->getUserBackend($this->params->getRequired('backend'), 'Icinga\Data\Updatable'); @@ -215,7 +215,7 @@ class UserController extends AuthBackendController */ public function removeAction() { - $this->assertPermission('config/authentication/users/remove'); + $this->assertPermission('config/access-control/users'); $userName = $this->params->getRequired('user'); $backend = $this->getUserBackend($this->params->getRequired('backend'), 'Icinga\Data\Reducible'); @@ -237,7 +237,7 @@ class UserController extends AuthBackendController */ public function createmembershipAction() { - $this->assertPermission('config/authentication/groups/edit'); + $this->assertPermission('config/access-control/groups'); $userName = $this->params->getRequired('user'); $backend = $this->getUserBackend($this->params->getRequired('backend')); @@ -325,18 +325,21 @@ class UserController extends AuthBackendController protected function createListTabs() { $tabs = $this->getTabs(); - $tabs->add( - 'role/list', - array( - 'baseTarget' => '_main', - 'label' => $this->translate('Roles'), - 'title' => $this->translate( - 'Configure roles to permit or restrict users and groups accessing Icinga Web 2' - ), - 'url' => 'role/list' - ) - ); + if ($this->hasPermission('config/access-control/roles')) { + $tabs->add( + 'role/list', + array( + 'baseTarget' => '_main', + 'label' => $this->translate('Roles'), + 'title' => $this->translate( + 'Configure roles to permit or restrict users and groups accessing Icinga Web 2' + ), + 'url' => 'role/list' + ) + ); + } + $tabs->add( 'user/list', array( @@ -345,14 +348,18 @@ class UserController extends AuthBackendController 'url' => 'user/list' ) ); - $tabs->add( - 'group/list', - array( - 'title' => $this->translate('List groups of user group backends'), - 'label' => $this->translate('User Groups'), - 'url' => 'group/list' - ) - ); + + if ($this->hasPermission('config/access-control/groups')) { + $tabs->add( + 'group/list', + array( + 'title' => $this->translate('List groups of user group backends'), + 'label' => $this->translate('User Groups'), + 'url' => 'group/list' + ) + ); + } + return $tabs; } } diff --git a/application/controllers/UsergroupbackendController.php b/application/controllers/UsergroupbackendController.php index 123997a05..a96ab75d5 100644 --- a/application/controllers/UsergroupbackendController.php +++ b/application/controllers/UsergroupbackendController.php @@ -21,7 +21,7 @@ class UsergroupbackendController extends Controller */ public function init() { - $this->assertPermission('config/application/usergroupbackend'); + $this->assertPermission('config/access-control/users'); } /** diff --git a/application/forms/Navigation/NavigationConfigForm.php b/application/forms/Navigation/NavigationConfigForm.php index 9ae87b42a..e9fecc885 100644 --- a/application/forms/Navigation/NavigationConfigForm.php +++ b/application/forms/Navigation/NavigationConfigForm.php @@ -299,7 +299,7 @@ class NavigationConfigForm extends ConfigForm $shared = false; $config = $this->getUserConfig($data['type']); if ((isset($data['users']) && $data['users']) || (isset($data['groups']) && $data['groups'])) { - if ($this->getUser()->can('application/share/navigation')) { + if ($this->getUser()->can('user/share/navigation')) { $data['owner'] = $this->getUser()->getUsername(); $config = $this->getShareConfig($data['type']); $shared = true; @@ -370,7 +370,7 @@ class NavigationConfigForm extends ConfigForm $config = $this->unshare($name, isset($data['parent']) ? $data['parent'] : null); } } elseif ((isset($data['users']) && $data['users']) || (isset($data['groups']) && $data['groups'])) { - if ($this->getUser()->can('application/share/navigation')) { + if ($this->getUser()->can('user/share/navigation')) { // It is not shared yet but should be $this->secondaryConfig = $config; $config = $this->getShareConfig(); @@ -580,7 +580,7 @@ class NavigationConfigForm extends ConfigForm ); if ((! $itemForm->requiresParentSelection() || ! isset($formData['parent']) || ! $formData['parent']) - && $this->getUser()->can('application/share/navigation') + && $this->getUser()->can('user/share/navigation') ) { $checked = isset($formData['shared']) ? null : (isset($formData['users']) || isset($formData['groups'])); @@ -783,7 +783,7 @@ class NavigationConfigForm extends ConfigForm return $this->getUserConfig(); } elseif ($this->getShareConfig()->hasSection($name)) { if ($this->getShareConfig()->get($name, 'owner') === $this->getUser()->getUsername() - || $this->getUser()->can('config/application/navigation') + || $this->getUser()->can('user/share/navigation') ) { return $this->getShareConfig(); } diff --git a/application/forms/PreferenceForm.php b/application/forms/PreferenceForm.php index 64c0ee977..9df7d1bcb 100644 --- a/application/forms/PreferenceForm.php +++ b/application/forms/PreferenceForm.php @@ -255,7 +255,7 @@ class PreferenceForm extends Form ) ); - if (Auth::getInstance()->hasPermission('application/stacktraces')) { + if (Auth::getInstance()->hasPermission('user/application/stacktraces')) { $this->addElement( 'checkbox', 'show_stacktraces', diff --git a/application/forms/Security/RoleForm.php b/application/forms/Security/RoleForm.php index 39346c945..b7773ca8c 100644 --- a/application/forms/Security/RoleForm.php +++ b/application/forms/Security/RoleForm.php @@ -6,6 +6,7 @@ namespace Icinga\Forms\Security; use Icinga\Application\Hook\ConfigFormEventsHook; use Icinga\Application\Icinga; use Icinga\Application\Modules\Manager; +use Icinga\Authentication\AdmissionLoader; use Icinga\Data\Filter\Filter; use Icinga\Forms\ConfigForm; use Icinga\Forms\RepositoryForm; @@ -50,33 +51,67 @@ class RoleForm extends RepositoryForm $view = $this->getView(); $this->providedPermissions['application'] = [ - $helper->filterName('no-user/password-change') => [ - 'name' => 'no-user/password-change', - 'description' => $this->translate('Prohibit password changes in the account preferences') - ], - $helper->filterName('application/share/navigation') => [ - 'name' => 'application/share/navigation', - 'description' => $this->translate('Allow to share navigation items') - ], - $helper->filterName('application/stacktraces') => [ - 'name' => 'application/stacktraces', - 'description' => $this->translate( - 'Allow to adjust in the preferences whether to show stacktraces' - ) + $helper->filterName('application/announcements') => [ + 'name' => 'application/announcements', + 'description' => $this->translate('Allow to manage announcements') ], $helper->filterName('application/log') => [ 'name' => 'application/log', 'description' => $this->translate('Allow to view the application log') ], - $helper->filterName('admin') => [ - 'name' => 'admin', - 'description' => $this->translate( - 'Grant admin permissions, e.g. manage announcements' - ) - ], $helper->filterName('config/*') => [ 'name' => 'config/*', - 'description' => $this->translate('Allow config access') + 'description' => $this->translate('Allow full config access') + ], + $helper->filterName('config/general') => [ + 'name' => 'config/general', + 'description' => $this->translate('Allow to adjust the general configuration') + ], + $helper->filterName('config/modules') => [ + 'name' => 'config/modules', + 'description' => $this->translate('Allow to enable/disable and configure modules') + ], + $helper->filterName('config/resources') => [ + 'name' => 'config/resources', + 'description' => $this->translate('Allow to manage resources') + ], + $helper->filterName('config/navigation') => [ + 'name' => 'config/navigation', + 'description' => $this->translate('Allow to view and adjust shared navigation items') + ], + $helper->filterName('config/access-control/*') => [ + 'name' => 'config/access-control/*', + 'description' => $this->translate('Allow to fully manage access-control') + ], + $helper->filterName('config/access-control/users') => [ + 'name' => 'config/access-control/users', + 'description' => $this->translate('Allow to manage user accounts') + ], + $helper->filterName('config/access-control/groups') => [ + 'name' => 'config/access-control/groups', + 'description' => $this->translate('Allow to manage user groups') + ], + $helper->filterName('config/access-control/roles') => [ + 'name' => 'config/access-control/roles', + 'description' => $this->translate('Allow to manage roles') + ], + $helper->filterName('user/*') => [ + 'name' => 'user/*', + 'description' => $this->translate('Allow all account related functionalities') + ], + $helper->filterName('user/password-change') => [ + 'name' => 'user/password-change', + 'description' => $this->translate('Allow password changes in the account preferences') + ], + $helper->filterName('user/application/stacktraces') => [ + 'name' => 'user/application/stacktraces', + 'description' => $this->translate( + 'Allow to adjust in the preferences whether to show stacktraces' + ) + ], + $helper->filterName('user/share/navigation') => [ + 'name' => 'user/share/navigation', + 'description' => $this->translate('Allow to share navigation items') ] ]; @@ -359,6 +394,12 @@ class RoleForm extends RepositoryForm if (! empty($role->permissions) || ! empty($role->refusals)) { $permissions = StringHelper::trimSplit($role->permissions); $refusals = StringHelper::trimSplit($role->refusals); + + list($permissions, $newRefusals) = AdmissionLoader::migrateLegacyPermissions($permissions); + if (! empty($newRefusals)) { + array_push($refusals, ...$newRefusals); + } + foreach ($this->providedPermissions as $moduleName => $permissionList) { foreach ($permissionList as $name => $spec) { if (in_array($spec['name'], $permissions, true)) { diff --git a/application/views/scripts/announcements/index.phtml b/application/views/scripts/announcements/index.phtml index 8635116a3..ff87c66bd 100644 --- a/application/views/scripts/announcements/index.phtml +++ b/application/views/scripts/announcements/index.phtml @@ -10,7 +10,7 @@
+ auth()->hasPermission('config/access-control/users')): ?>

translate('User Backends') ?>

qlink( $this->translate('Create a New User Backend') , @@ -15,7 +16,9 @@ ) ) ?> + + auth()->hasPermission('config/access-control/groups')): ?>

translate('User Group Backends') ?>

qlink( $this->translate('Create a New User Group Backend') , @@ -68,4 +71,5 @@ +
diff --git a/application/views/scripts/group/list.phtml b/application/views/scripts/group/list.phtml index 84d75018c..d362db4e3 100644 --- a/application/views/scripts/group/list.phtml +++ b/application/views/scripts/group/list.phtml @@ -22,8 +22,8 @@ if (! isset($backend)) { echo $this->translate('No backend found which is able to list user groups') . ''; return; } else { - $extensible = $this->hasPermission('config/authentication/groups/add') && $backend instanceof Extensible; - $reducible = $this->hasPermission('config/authentication/groups/remove') && $backend instanceof Reducible; + $extensible = $this->hasPermission('config/access-control/groups') && $backend instanceof Extensible; + $reducible = $this->hasPermission('config/access-control/groups') && $backend instanceof Reducible; } ?> diff --git a/application/views/scripts/group/show.phtml b/application/views/scripts/group/show.phtml index f64e1bae4..75f0b7554 100644 --- a/application/views/scripts/group/show.phtml +++ b/application/views/scripts/group/show.phtml @@ -3,10 +3,10 @@ use Icinga\Data\Extensible; use Icinga\Data\Updatable; -$extensible = $this->hasPermission('config/authentication/groups/add') && $backend instanceof Extensible; +$extensible = $this->hasPermission('config/access-control/groups') && $backend instanceof Extensible; $editLink = null; -if ($this->hasPermission('config/authentication/groups/edit') && $backend instanceof Updatable) { +if ($this->hasPermission('config/access-control/groups') && $backend instanceof Updatable) { $editLink = $this->qlink( null, 'group/edit', @@ -83,7 +83,7 @@ if ($this->hasPermission('config/authentication/groups/edit') && $backend instan hasPermission('config/authentication/users/show') + $this->hasPermission('config/access-control/users') && ($userBackend = $backend->getUserBackendName($member->user_name)) !== null ): ?> qlink($member->user_name, 'user/show', array( diff --git a/application/views/scripts/user/list.phtml b/application/views/scripts/user/list.phtml index 7e045d8a9..bdb5f1a3c 100644 --- a/application/views/scripts/user/list.phtml +++ b/application/views/scripts/user/list.phtml @@ -22,8 +22,8 @@ if (! isset($backend)) { echo $this->translate('No backend found which is able to list users') . ''; return; } else { - $extensible = $this->hasPermission('config/authentication/users/add') && $backend instanceof Extensible; - $reducible = $this->hasPermission('config/authentication/users/remove') && $backend instanceof Reducible; + $extensible = $this->hasPermission('config/access-control/users') && $backend instanceof Extensible; + $reducible = $this->hasPermission('config/access-control/users') && $backend instanceof Reducible; } ?> diff --git a/application/views/scripts/user/show.phtml b/application/views/scripts/user/show.phtml index 543e667fa..b19c15a27 100644 --- a/application/views/scripts/user/show.phtml +++ b/application/views/scripts/user/show.phtml @@ -11,7 +11,7 @@ use Icinga\Data\Selectable;

escape($user->user_name) ?>

hasPermission('config/authentication/users/edit') && $backend instanceof Updatable) { + if ($this->hasPermission('config/access-control/users') && $backend instanceof Updatable) { echo $this->qlink( $this->translate('Edit User'), 'user/edit', @@ -110,7 +110,7 @@ use Icinga\Data\Selectable; - hasPermission('config/authentication/groups/show') && $membership->backend instanceof Selectable): ?> + hasPermission('config/access-control/groups') && $membership->backend instanceof Selectable): ?> qlink($membership->group_name, 'group/show', array( 'backend' => $membership->backend->getName(), 'group' => $membership->group_name diff --git a/doc/06-Security.md b/doc/06-Security.md index 0fb1d9e63..6a3b67c1f 100644 --- a/doc/06-Security.md +++ b/doc/06-Security.md @@ -110,12 +110,25 @@ a module permission in the format `module/` for each installed modul ### General Permissions -Name | Permits ---------------------------|----------------------------------------------- -\* | allow everything, including module-specific permissions -config/\* | allow all configuration actions -config/modules | allow enabling or disabling modules -module/`` | allow access to module `` (e.g. `module/monitoring`) +Name | Permits +-----------------------------|----------------------------------------------- +\* | allow everything, including module-specific permissions +application/announcements | allow to manage announcements +application/log | allow to view the application log +config/\* | allow full config access +config/access-control/\* | allow to fully manage access control +config/access-control/groups | allow to manage groups +config/access-control/roles | allow to manage roles +config/access-control/users | allow to manage user accounts +config/general | allow to adjust the general configuration +config/modules | allow to enable/disable and configure modules +config/navigation | allow to view and adjust shared navigation items +config/resources | allow to manage resources +user/\* | allow all account related functionalities +user/application/stacktraces | allow to adjust in the preferences whether to show stacktraces +user/password-change | allow password changes in the account preferences +user/share/navigation | allow to share navigation items +module/`` | allow access to module `` (e.g. `module/monitoring`) ### Monitoring Module Permissions diff --git a/library/Icinga/Application/Web.php b/library/Icinga/Application/Web.php index a865c7d3a..43f2e5a8a 100644 --- a/library/Icinga/Application/Web.php +++ b/library/Icinga/Application/Web.php @@ -336,7 +336,7 @@ class Web extends EmbeddedWeb $this->getRequest()->setUser($user); $this->user = $user; - if ($user->can('application/stacktraces')) { + if ($user->can('user/application/stacktraces')) { $displayExceptions = $this->user->getPreferences()->getValue( 'icingaweb', 'show_stacktraces' diff --git a/library/Icinga/Authentication/AdmissionLoader.php b/library/Icinga/Authentication/AdmissionLoader.php index 1b8bb239e..6d755992b 100644 --- a/library/Icinga/Authentication/AdmissionLoader.php +++ b/library/Icinga/Authentication/AdmissionLoader.php @@ -17,6 +17,35 @@ use Icinga\Util\StringHelper; */ class AdmissionLoader { + const LEGACY_PERMISSIONS = [ + 'admin' => 'application/announcements', + 'application/stacktraces' => 'user/application/stacktraces', + 'application/share/navigation' => 'user/share/navigation', + // Migrating config/application/* would include config/modules, so that's skipped + //'config/application/*' => 'config/*', + 'config/application/general' => 'config/general', + 'config/application/resources' => 'config/resources', + 'config/application/navigation' => 'config/navigation', + 'config/application/userbackend' => 'config/access-control/users', + 'config/application/usergroupbackend' => 'config/access-control/groups', + 'config/authentication/*' => 'config/access-control/*', + 'config/authentication/users/*' => 'config/access-control/users', + 'config/authentication/users/show' => 'config/access-control/users', + 'config/authentication/users/add' => 'config/access-control/users', + 'config/authentication/users/edit' => 'config/access-control/users', + 'config/authentication/users/remove' => 'config/access-control/users', + 'config/authentication/groups/*' => 'config/access-control/groups', + 'config/authentication/groups/show' => 'config/access-control/groups', + 'config/authentication/groups/edit' => 'config/access-control/groups', + 'config/authentication/groups/add' => 'config/access-control/groups', + 'config/authentication/groups/remove' => 'config/access-control/groups', + 'config/authentication/roles/*' => 'config/access-control/roles', + 'config/authentication/roles/show' => 'config/access-control/roles', + 'config/authentication/roles/add' => 'config/access-control/roles', + 'config/authentication/roles/edit' => 'config/access-control/roles', + 'config/authentication/roles/remove' => 'config/access-control/roles' + ]; + /** @var Role[] */ protected $roles; @@ -84,6 +113,11 @@ class AdmissionLoader $permissions = StringHelper::trimSplit($section->permissions); $refusals = StringHelper::trimSplit($section->refusals); + list($permissions, $newRefusals) = self::migrateLegacyPermissions($permissions); + if (! empty($newRefusals)) { + array_push($refusals, ...$newRefusals); + } + $restrictions = $section->toArray(); unset($restrictions['users'], $restrictions['groups']); unset($restrictions['refusals'], $restrictions['permissions']); @@ -176,4 +210,22 @@ class AdmissionLoader $user->setPermissions($permissions); $user->setRoles(array_values($roles)); } + + public static function migrateLegacyPermissions(array $permissions) + { + $migratedGrants = []; + $refusals = []; + + foreach ($permissions as $permission) { + if (array_key_exists($permission, self::LEGACY_PERMISSIONS)) { + $migratedGrants[] = self::LEGACY_PERMISSIONS[$permission]; + } elseif ($permission === 'no-user/password-change') { + $refusals[] = 'user/password-change'; + } else { + $migratedGrants[] = $permission; + } + } + + return [$migratedGrants, $refusals]; + } } diff --git a/library/Icinga/User.php b/library/Icinga/User.php index 2bc9ebec8..491f29660 100644 --- a/library/Icinga/User.php +++ b/library/Icinga/User.php @@ -4,6 +4,7 @@ namespace Icinga; use DateTimeZone; +use Icinga\Authentication\AdmissionLoader; use InvalidArgumentException; use Icinga\Application\Config; use Icinga\Authentication\Role; @@ -559,6 +560,15 @@ class User */ public function can($requiredPermission) { + list($permissions, $refusals) = AdmissionLoader::migrateLegacyPermissions([$requiredPermission]); + if (! empty($permissions)) { + $requiredPermission = array_pop($permissions); + } elseif (! empty($refusals)) { + throw new InvalidArgumentException( + 'Refusals are not supported anymore. Check for a grant instead!' + ); + } + $granted = false; foreach ($this->getRoles() as $role) { if ($role->denies($requiredPermission)) { diff --git a/library/Icinga/Web/Menu.php b/library/Icinga/Web/Menu.php index c7e4a7177..5f822fa2b 100644 --- a/library/Icinga/Web/Menu.php +++ b/library/Icinga/Web/Menu.php @@ -67,24 +67,23 @@ class Menu extends Navigation 'icon' => 'wrench', 'description' => t('Open application configuration'), 'label' => t('Application'), - 'url' => 'config/general', - 'permission' => 'config/application/*', + 'url' => 'config', 'priority' => 810 ], 'authentication' => [ 'icon' => 'users', - 'description' => t('Open authentication configuration'), - 'label' => t('Authentication'), - 'permission' => 'config/authentication/*', + 'description' => t('Open access control configuration'), + 'label' => t('Access Control'), + 'permission' => 'config/access-control/*', 'priority' => 830, - 'url' => 'role/list' + 'url' => 'role' ], 'navigation' => [ 'icon' => 'sitemap', 'description' => t('Open shared navigation configuration'), 'label' => t('Shared Navigation'), 'url' => 'navigation/shared', - 'permission' => 'config/application/navigation', + 'permission' => 'config/navigation', 'priority' => 840, ], 'modules' => [ diff --git a/modules/monitoring/application/controllers/ConfigController.php b/modules/monitoring/application/controllers/ConfigController.php index 2d281fee1..92444b654 100644 --- a/modules/monitoring/application/controllers/ConfigController.php +++ b/modules/monitoring/application/controllers/ConfigController.php @@ -101,7 +101,7 @@ class ConfigController extends Controller try { $form->setResourceConfig(ResourceFactory::getResourceConfigs()); } catch (ConfigurationError $e) { - if ($this->hasPermission('config/application/resources')) { + if ($this->hasPermission('config/resources')) { Notification::error($e->getMessage()); $this->redirectNow('config/createresource'); }