From bdd0f204f0cbdad99aa84782d792440659a35b97 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Mon, 1 Feb 2021 15:04:36 +0100 Subject: [PATCH] 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',