diff --git a/doc/source/authentication.rst b/doc/source/authentication.rst index e1746d65b2..5175e8b9f3 100644 --- a/doc/source/authentication.rst +++ b/doc/source/authentication.rst @@ -44,7 +44,7 @@ To enable tenant-scoped access to privileged actions, see the Zuul Web Server component's section. To set access rules for a tenant, see :ref:`the documentation about tenant -definition `. +definition `. Most of the time, only one authenticator will be needed in Zuul's configuration; namely the configuration matching a third party identity provider service like diff --git a/doc/source/developer/model-changelog.rst b/doc/source/developer/model-changelog.rst index d27ec8351f..c6d8fedbd7 100644 --- a/doc/source/developer/model-changelog.rst +++ b/doc/source/developer/model-changelog.rst @@ -93,3 +93,10 @@ Version 9 :Prior Zuul version: 6.3.0 :Description: Adds nodeset_alternatives and nodeset_index to frozen job. Removes nodset from frozen job. Affects schedulers and executors. + +Version 10 +---------- + +:Prior Zuul version: 6.4.0 +:Description: Renames admin_rules to authz_rules in unparsed abide. + Affects schedulers and web. diff --git a/doc/source/examples/keycloak/etc_zuul/main.yaml b/doc/source/examples/keycloak/etc_zuul/main.yaml index 9006399b6f..0305fcc289 100644 --- a/doc/source/examples/keycloak/etc_zuul/main.yaml +++ b/doc/source/examples/keycloak/etc_zuul/main.yaml @@ -1,8 +1,8 @@ -- admin-rule: +- authorization-rule: name: tenant-group conditions: - groups: "{tenant.name}-admin" -- admin-rule: +- authorization-rule: name: admin-user conditions: - preferred_username: admin diff --git a/doc/source/tenants.rst b/doc/source/tenants.rst index fbbb458a55..56d0e5666c 100644 --- a/doc/source/tenants.rst +++ b/doc/source/tenants.rst @@ -16,14 +16,14 @@ A project may appear in more than one tenant; this may be useful if you wish to use common job definitions across multiple tenants. Actions normally available to the Zuul operator only can be performed by specific -users on Zuul's REST API, if admin rules are listed for the tenant. Admin rules +users on Zuul's REST API if admin rules are listed for the tenant. Authorization rules are also defined in the tenant configuration file. The tenant configuration file is specified by the :attr:`scheduler.tenant_config` setting in ``zuul.conf``. It is a YAML file which, like other Zuul configuration files, is a list of -configuration objects, though only two types of objects are supported: -``tenant`` and ``admin-rule``. +configuration objects, though only a few types of objects (described +below) are supported. Alternatively the :attr:`scheduler.tenant_config_script` can be the path to an executable that will be executed and its stdout @@ -398,11 +398,12 @@ configuration. Some examples of tenant definitions are: .. attr:: admin-rules - A list of access rules for the tenant. These rules are checked to grant - privileged actions to users at the tenant level, through Zuul's REST API. + A list of authorization rules to be checked in order to grant + administrative access to the tenant through Zuul's REST API and + web interface. - At least one rule in the list must match for the user to be allowed the - privileged action. + At least one rule in the list must match for the user to be allowed to + execute privileged actions. More information on tenant-scoped actions can be found in :ref:`authentication`. @@ -474,16 +475,18 @@ An example definition looks similar to the normal semaphore object: The maximum number of running jobs which can use this semaphore. -.. _admin_rule_definition: +.. _authz_rule_definition: -Access Rule ------------ +Authorization Rule +------------------ -An access rule is a set of conditions the claims of a user's JWT must match -in order to be allowed to perform protected actions at a tenant's level. +An authorization rule is a set of conditions the claims of a user's +JWT must match in order to be allowed to perform actions at a tenant's +level. -The protected actions available at tenant level are **autohold**, **enqueue**, -**dequeue** or **promote**. +When an authorization rule is included in the tenant's `admin-rules`, +the protected actions available are **autohold**, **enqueue**, +**dequeue** and **promote**. .. note:: @@ -491,11 +494,11 @@ The protected actions available at tenant level are **autohold**, **enqueue**, an authenticator configuration where `allow_authz_override` is set to true. See :ref:`authentication` for more details. -Below are some examples of how access rules can be defined: +Below are some examples of how authorization rules can be defined: .. code-block:: yaml - - admin-rule: + - authorization-rule: name: affiliate_or_admin conditions: - resources_access: @@ -503,14 +506,17 @@ Below are some examples of how access rules can be defined: roles: "affiliate" iss: external_institution - resources_access.account.roles: "admin" - - admin-rule: + - authorization-rule: name: alice_or_bob conditions: - zuul_uid: alice - zuul_uid: bob +Zuul previously used ``admin-rule`` for these definitions. That form +is still permitted for backwards compatibility, but is deprecated and +will be removed in a future version of Zuul. -.. attr:: admin-rule +.. attr:: authorization-rule The following attributes are supported: @@ -591,8 +597,8 @@ Below are some examples of how access rules can be defined: }, } -Access Rule Templating ----------------------- +Authorization Rule Templating +----------------------------- The special word "{tenant.name}" can be used in conditions' values. It will be automatically substituted for the relevant tenant when evaluating authorizations for a given @@ -600,7 +606,7 @@ set of claims. For example, consider the following rule: .. code-block:: yaml - - admin-rule: + - authorization-rule: name: tenant_in_groups conditions: - groups: "{tenant.name}" diff --git a/releasenotes/notes/authz-rule-55a6db54340f2e08.yaml b/releasenotes/notes/authz-rule-55a6db54340f2e08.yaml new file mode 100644 index 0000000000..d3b3fa5f12 --- /dev/null +++ b/releasenotes/notes/authz-rule-55a6db54340f2e08.yaml @@ -0,0 +1,24 @@ +--- +upgrade: + - | + In preparation for expanded access control features in the web + interface, and REST API, the ``admin-rule`` tenant configuration + object has been renamed to ``authorization-rule``. When applied + to a tenant, the tenant attribute is still ``admin-rules`` since + it determines admin access to that tenant. This change will allow + similar rules to be applied to non-admin level access in the + future. + + Tenant configs should now follow this example: + + .. code-block:: yaml + + - authorization-rule: + name: example-rule + - tenant: + name: example-tenant + admin-rules: + - example-rule + + The old form is still permitted for backwards compatability, but + will be removed in a later version of Zuul. diff --git a/tests/fixtures/config/authorization/rules-templating/main.yaml b/tests/fixtures/config/authorization/rules-templating/main.yaml index c621115a3e..f1c85ea25f 100644 --- a/tests/fixtures/config/authorization/rules-templating/main.yaml +++ b/tests/fixtures/config/authorization/rules-templating/main.yaml @@ -1,4 +1,4 @@ -- admin-rule: +- authorization-rule: name: tenant-admin conditions: - groups: "{tenant.name}" diff --git a/tests/fixtures/config/authorization/single-tenant/main.yaml b/tests/fixtures/config/authorization/single-tenant/main.yaml index 136ecd2660..52e1fbdd78 100644 --- a/tests/fixtures/config/authorization/single-tenant/main.yaml +++ b/tests/fixtures/config/authorization/single-tenant/main.yaml @@ -1,7 +1,8 @@ -- admin-rule: +- authorization-rule: name: venkman_rule conditions: - zuul_uid: venkman +# TODO: rename to authorization-rule; this tests backwards compat for now - admin-rule: name: columbia_rule conditions: diff --git a/tests/fixtures/config/tenant-parser/authorizations-templating.yaml b/tests/fixtures/config/tenant-parser/authorizations-templating.yaml index 2b30ea5abe..87b34fa501 100644 --- a/tests/fixtures/config/tenant-parser/authorizations-templating.yaml +++ b/tests/fixtures/config/tenant-parser/authorizations-templating.yaml @@ -1,8 +1,8 @@ -- admin-rule: +- authorization-rule: name: tenant-admin conditions: - group: "{tenant.name}-admin" -- admin-rule: +- authorization-rule: name: tenant-admin-complex conditions: - path.to.group: "{tenant.name}-admin" diff --git a/tests/fixtures/config/tenant-parser/authorizations.yaml b/tests/fixtures/config/tenant-parser/authorizations.yaml index d84e7d8a6d..442355b91a 100644 --- a/tests/fixtures/config/tenant-parser/authorizations.yaml +++ b/tests/fixtures/config/tenant-parser/authorizations.yaml @@ -1,9 +1,9 @@ -- admin-rule: +- authorization-rule: name: auth-rule-one conditions: - sub: venkman - sub: zeddemore -- admin-rule: +- authorization-rule: name: auth-rule-two conditions: - sub: gozer diff --git a/tests/unit/test_configloader.py b/tests/unit/test_configloader.py index 571df75500..1593292944 100644 --- a/tests/unit/test_configloader.py +++ b/tests/unit/test_configloader.py @@ -46,7 +46,7 @@ class TestConfigLoader(ZuulTestCase): keystorage=sched.keystore) abide = Abide() loader.loadTPCs(abide, unparsed_abide) - loader.loadAdminRules(abide, unparsed_abide) + loader.loadAuthzRules(abide, unparsed_abide) for tenant_name in unparsed_abide.tenants: tlock = tenant_read_lock(self.zk_client, tenant_name) @@ -886,7 +886,7 @@ class TestAuthorizationRuleParser(ZuulTestCase): tenant_config_file = 'config/tenant-parser/authorizations.yaml' def test_rules_are_loaded(self): - rules = self.scheds.first.sched.abide.admin_rules + rules = self.scheds.first.sched.abide.authz_rules self.assertTrue('auth-rule-one' in rules, self.scheds.first.sched.abide) self.assertTrue('auth-rule-two' in rules, @@ -1022,7 +1022,7 @@ class TestAuthorizationRuleParserWithTemplating(ZuulTestCase): tenant_config_file = 'config/tenant-parser/authorizations-templating.yaml' def test_rules_are_loaded(self): - rules = self.scheds.first.sched.abide.admin_rules + rules = self.scheds.first.sched.abide.authz_rules self.assertTrue('tenant-admin' in rules, self.scheds.first.sched.abide) self.assertTrue('tenant-admin-complex' in rules, self.scheds.first.sched.abide) @@ -1030,7 +1030,7 @@ class TestAuthorizationRuleParserWithTemplating(ZuulTestCase): def test_tenant_substitution(self): claims_1 = {'group': 'tenant-one-admin'} claims_2 = {'group': 'tenant-two-admin'} - rules = self.scheds.first.sched.abide.admin_rules + rules = self.scheds.first.sched.abide.authz_rules tenant_one = self.scheds.first.sched.abide.tenants.get('tenant-one') tenant_two = self.scheds.first.sched.abide.tenants.get('tenant-two') self.assertTrue(rules['tenant-admin'](claims_1, tenant_one)) @@ -1041,7 +1041,7 @@ class TestAuthorizationRuleParserWithTemplating(ZuulTestCase): def test_tenant_substitution_in_list(self): claims_1 = {'group': ['tenant-one-admin', 'some-other-tenant']} claims_2 = {'group': ['tenant-two-admin', 'some-other-tenant']} - rules = self.scheds.first.sched.abide.admin_rules + rules = self.scheds.first.sched.abide.authz_rules tenant_one = self.scheds.first.sched.abide.tenants.get('tenant-one') tenant_two = self.scheds.first.sched.abide.tenants.get('tenant-two') self.assertTrue(rules['tenant-admin'](claims_1, tenant_one)) @@ -1057,7 +1057,7 @@ class TestAuthorizationRuleParserWithTemplating(ZuulTestCase): } } } - rules = self.scheds.first.sched.abide.admin_rules + rules = self.scheds.first.sched.abide.authz_rules tenant_one = self.scheds.first.sched.abide.tenants.get('tenant-one') tenant_two = self.scheds.first.sched.abide.tenants.get('tenant-two') self.assertTrue(not rules['tenant-admin-complex'](claims_2, diff --git a/tests/unit/test_zk.py b/tests/unit/test_zk.py index fc6a47b938..e9c70d680d 100644 --- a/tests/unit/test_zk.py +++ b/tests/unit/test_zk.py @@ -47,7 +47,7 @@ from zuul.zk.sharding import ( NODE_BYTE_SIZE_LIMIT, ) from zuul.zk.components import ( - BaseComponent, ComponentRegistry, ExecutorComponent + BaseComponent, ComponentRegistry, ExecutorComponent, COMPONENT_REGISTRY ) from tests.base import ( BaseTestCase, HoldableExecutorApi, HoldableMergerApi, @@ -74,6 +74,9 @@ class ZooKeeperBaseTestCase(BaseTestCase): self.addCleanup(self.zk_client.disconnect) self.zk_client.connect() self.component_registry = ComponentRegistry(self.zk_client) + # We don't have any other component to initialize the global + # registry in these tests, so we do it ourselves. + COMPONENT_REGISTRY.create(self.zk_client) class TestZookeeperClient(ZooKeeperBaseTestCase): @@ -1347,7 +1350,7 @@ class TestSystemConfigCache(ZooKeeperBaseTestCase): def test_set_get(self): uac = model.UnparsedAbideConfig() uac.tenants = {"foo": "bar"} - uac.admin_rules = ["bar", "foo"] + uac.authz_rules = ["bar", "foo"] attrs = model.SystemAttributes.fromDict({ "use_relative_priority": True, "max_hold_expiration": 7200, @@ -1362,7 +1365,7 @@ class TestSystemConfigCache(ZooKeeperBaseTestCase): uac_cached, cached_attrs = self.config_cache.get() self.assertEqual(uac.uuid, uac_cached.uuid) self.assertEqual(uac.tenants, uac_cached.tenants) - self.assertEqual(uac.admin_rules, uac_cached.admin_rules) + self.assertEqual(uac.authz_rules, uac_cached.authz_rules) self.assertEqual(attrs, cached_attrs) def test_cache_empty(self): diff --git a/zuul/configloader.py b/zuul/configloader.py index a92f5337cc..9b695ecd94 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -1460,11 +1460,9 @@ class AuthorizationRuleParser(object): self.schema = self.getSchema() def getSchema(self): - authRule = {vs.Required('name'): str, vs.Required('conditions'): to_list(dict) } - return vs.Schema(authRule) def fromYaml(self, conf): @@ -1654,7 +1652,7 @@ class TenantParser(object): tenant.exclude_unprotected_branches = \ conf['exclude-unprotected-branches'] if conf.get('admin-rules') is not None: - tenant.authorization_rules = conf['admin-rules'] + tenant.admin_rules = conf['admin-rules'] if conf.get('authentication-realm') is not None: tenant.default_auth_realm = conf['authentication-realm'] if conf.get('semaphores') is not None: @@ -2530,7 +2528,7 @@ class ConfigLoader(object): self.tenant_parser = TenantParser( connections, zk_client, scheduler, merger, keystorage, zuul_globals, statsd) - self.admin_rule_parser = AuthorizationRuleParser() + self.authz_rule_parser = AuthorizationRuleParser() self.global_semaphore_parser = GlobalSemaphoreParser() def expandConfigPath(self, config_path): @@ -2582,11 +2580,11 @@ class ConfigLoader(object): self.tenant_parser.getSchema()(unparsed_abide.tenants[tenant_name]) return unparsed_abide - def loadAdminRules(self, abide, unparsed_abide): - abide.admin_rules.clear() - for conf_admin_rule in unparsed_abide.admin_rules: - admin_rule = self.admin_rule_parser.fromYaml(conf_admin_rule) - abide.admin_rules[admin_rule.name] = admin_rule + def loadAuthzRules(self, abide, unparsed_abide): + abide.authz_rules.clear() + for conf_authz_rule in unparsed_abide.authz_rules: + authz_rule = self.authz_rule_parser.fromYaml(conf_authz_rule) + abide.authz_rules[authz_rule.name] = authz_rule def loadSemaphores(self, abide, unparsed_abide): abide.semaphores.clear() diff --git a/zuul/model.py b/zuul/model.py index 67b5e0117b..4f53e2490a 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -7103,13 +7103,13 @@ class UnparsedAbideConfig(object): self.uuid = uuid4().hex self.ltime = -1 self.tenants = {} - self.admin_rules = [] + self.authz_rules = [] self.semaphores = [] def extend(self, conf): if isinstance(conf, UnparsedAbideConfig): self.tenants.update(conf.tenants) - self.admin_rules.extend(conf.admin_rules) + self.authz_rules.extend(conf.authz_rules) self.semaphores.extend(conf.semaphores) return @@ -7127,20 +7127,27 @@ class UnparsedAbideConfig(object): raise Exception("Duplicate configuration for " f"tenant {value['name']}") self.tenants[value["name"]] = value + # TODO: remove deprecated "admin-rule" elif key == 'admin-rule': - self.admin_rules.append(value) + self.authz_rules.append(value) + elif key == 'authorization-rule': + self.authz_rules.append(value) elif key == 'global-semaphore': self.semaphores.append(value) else: raise ConfigItemUnknownError(item) def toDict(self): - return { + d = { "uuid": self.uuid, "tenants": self.tenants, - "admin_rules": self.admin_rules, "semaphores": self.semaphores, } + if (COMPONENT_REGISTRY.model_api < 10): + d["admin_rules"] = self.authz_rules + else: + d["authz_rules"] = self.authz_rules + return d @classmethod def fromDict(cls, data, ltime): @@ -7148,7 +7155,9 @@ class UnparsedAbideConfig(object): unparsed_abide.uuid = data["uuid"] unparsed_abide.ltime = ltime unparsed_abide.tenants = data["tenants"] - unparsed_abide.admin_rules = data["admin_rules"] + unparsed_abide.authz_rules = data.get('authz_rules', + data.get('admin_rules', + [])) unparsed_abide.semaphores = data.get("semaphores", []) return unparsed_abide @@ -7918,7 +7927,7 @@ class Tenant(object): # The per tenant default ansible version self.default_ansible_version = None - self.authorization_rules = [] + self.admin_rules = [] self.default_auth_realm = None self.global_semaphores = set() @@ -8128,7 +8137,7 @@ class UnparsedBranchCache(object): class Abide(object): def __init__(self): - self.admin_rules = {} + self.authz_rules = {} self.semaphores = {} self.tenants = {} # tenant -> project -> list(tpcs) diff --git a/zuul/model_api.py b/zuul/model_api.py index 27d4a2b07a..9542cf4153 100644 --- a/zuul/model_api.py +++ b/zuul/model_api.py @@ -14,4 +14,4 @@ # When making ZK schema changes, increment this and add a record to # docs/developer/model-changelog.rst -MODEL_API = 9 +MODEL_API = 10 diff --git a/zuul/scheduler.py b/zuul/scheduler.py index d37ef95399..5ccef556a2 100644 --- a/zuul/scheduler.py +++ b/zuul/scheduler.py @@ -1313,7 +1313,7 @@ class Scheduler(threading.Thread): try: abide = Abide() - loader.loadAdminRules(abide, unparsed_abide) + loader.loadAuthzRules(abide, unparsed_abide) loader.loadSemaphores(abide, unparsed_abide) loader.loadTPCs(abide, unparsed_abide) for tenant_name in tenants_to_load: @@ -1372,7 +1372,7 @@ class Scheduler(threading.Thread): for tenant_name in deleted_tenants: self.abide.clearTPCs(tenant_name) - loader.loadAdminRules(self.abide, self.unparsed_abide) + loader.loadAuthzRules(self.abide, self.unparsed_abide) loader.loadSemaphores(self.abide, self.unparsed_abide) loader.loadTPCs(self.abide, self.unparsed_abide) @@ -2044,7 +2044,7 @@ class Scheduler(threading.Thread): tenant_config, from_script=script) self.system_config_cache.set(self.unparsed_abide, self.globals) - loader.loadAdminRules(self.abide, self.unparsed_abide) + loader.loadAuthzRules(self.abide, self.unparsed_abide) loader.loadSemaphores(self.abide, self.unparsed_abide) loader.loadTPCs(self.abide, self.unparsed_abide) @@ -2065,7 +2065,7 @@ class Scheduler(threading.Thread): for tenant_name in deleted_tenants: self.abide.clearTPCs(tenant_name) - loader.loadAdminRules(self.abide, self.unparsed_abide) + loader.loadAuthzRules(self.abide, self.unparsed_abide) loader.loadSemaphores(self.abide, self.unparsed_abide) loader.loadTPCs(self.abide, self.unparsed_abide) diff --git a/zuul/web/__init__.py b/zuul/web/__init__.py index 09706057b0..b62af00e5e 100755 --- a/zuul/web/__init__.py +++ b/zuul/web/__init__.py @@ -871,8 +871,8 @@ class ZuulWebAPI(object): (isinstance(override, list) and tenant.name in override)): return True - for rule_name in tenant.authorization_rules: - rule = self.zuulweb.abide.admin_rules.get(rule_name) + for rule_name in tenant.admin_rules: + rule = self.zuulweb.abide.authz_rules.get(rule_name) if not rule: self.log.error('Undefined rule "%s"', rule_name) continue @@ -2170,7 +2170,7 @@ class ZuulWeb(object): for tenant_name in deleted_tenants: self.abide.clearTPCs(tenant_name) - loader.loadAdminRules(self.abide, self.unparsed_abide) + loader.loadAuthzRules(self.abide, self.unparsed_abide) loader.loadSemaphores(self.abide, self.unparsed_abide) loader.loadTPCs(self.abide, self.unparsed_abide)