diff --git a/ironic_inspector/policy.py b/ironic_inspector/policy.py index acac1589f..f2aeb6509 100644 --- a/ironic_inspector/policy.py +++ b/ironic_inspector/policy.py @@ -37,12 +37,12 @@ opts.set_defaults(cfg.CONF, DEFAULT_POLICY_FILE) # applicable (e.g., cleaning up baremetal hosts) SYSTEM_ADMIN = 'role:admin and system_scope:all' -# Generic policy check string for system users who don't require all the -# authorization that system administrators typically have. This persona, or -# check string, typically isn't used by default, but it's existence it useful -# in the event a deployment wants to offload some administrative action from -# system administrator to system members -SYSTEM_MEMBER = 'role:member and system_scope:all' +# Policy check to allow for the OpenStack community change in RBAC direction, +# where "admin" is still admin across all projects, but "manager" is the +# project delegated administrative account scoped to the project. +# Also adds service role access for the service to authenticate which was +# generally missed with inspector as well. +ADMIN = '(' + SYSTEM_ADMIN + ') or (role:admin) or (role:service)' # Generic policy check string for read-only access to system-level resources. # This persona is useful for someone who needs access for auditing or even @@ -51,6 +51,12 @@ SYSTEM_MEMBER = 'role:member and system_scope:all' # project they belong to). SYSTEM_READER = 'role:reader and system_scope:all' +# Policy check to allow the OpenStack community change in RBAC direction, +# where "admin" is still admin aross all projects, but "manager" is the +# delegated level of access for project scoped administrative use. +# also adds the ability for a service role to access +READER = '(' + SYSTEM_READER + ') or (role:admin) or (role:service)' + deprecated_node_reason = """ The inspector API is now aware of system scope and default roles. """ @@ -154,7 +160,7 @@ introspection_policies = [ ), policy.DocumentedRuleDefault( name='introspection:status', - check_str=SYSTEM_READER, + check_str=READER, description='Get introspection status', operations=[{'path': '/introspection', 'method': 'GET'}, {'path': '/introspection/{node_id}', 'method': 'GET'}], @@ -162,14 +168,14 @@ introspection_policies = [ ), policy.DocumentedRuleDefault( name='introspection:start', - check_str=SYSTEM_ADMIN, + check_str=ADMIN, description='Start introspection', operations=[{'path': '/introspection/{node_id}', 'method': 'POST'}], deprecated_rule=deprecated_introspection_start ), policy.DocumentedRuleDefault( name='introspection:abort', - check_str=SYSTEM_ADMIN, + check_str=ADMIN, description='Abort introspection', operations=[{'path': '/introspection/{node_id}/abort', 'method': 'POST'}], @@ -177,7 +183,7 @@ introspection_policies = [ ), policy.DocumentedRuleDefault( name='introspection:data', - check_str=SYSTEM_ADMIN, + check_str=ADMIN, description='Get introspection data', operations=[{'path': '/introspection/{node_id}/data', 'method': 'GET'}], @@ -185,7 +191,7 @@ introspection_policies = [ ), policy.DocumentedRuleDefault( name='introspection:reapply', - check_str=SYSTEM_ADMIN, + check_str=ADMIN, description='Reapply introspection on stored data', operations=[{'path': '/introspection/{node_id}/data/unprocessed', 'method': 'POST'}], @@ -196,7 +202,7 @@ introspection_policies = [ rule_policies = [ policy.DocumentedRuleDefault( name='introspection:rule:get', - check_str=SYSTEM_ADMIN, + check_str=ADMIN, description='Get introspection rule(s)', operations=[{'path': '/rules', 'method': 'GET'}, {'path': '/rules/{rule_id}', 'method': 'GET'}], @@ -204,7 +210,7 @@ rule_policies = [ ), policy.DocumentedRuleDefault( name='introspection:rule:delete', - check_str=SYSTEM_ADMIN, + check_str=ADMIN, description='Delete introspection rule(s)', operations=[{'path': '/rules', 'method': 'DELETE'}, {'path': '/rules/{rule_id}', 'method': 'DELETE'}], @@ -212,7 +218,7 @@ rule_policies = [ ), policy.DocumentedRuleDefault( name='introspection:rule:create', - check_str=SYSTEM_ADMIN, + check_str=ADMIN, description='Create introspection rule', operations=[{'path': '/rules', 'method': 'POST'}], deprecated_rule=deprecated_introspection_rule_create diff --git a/ironic_inspector/test/unit/test_acl.py b/ironic_inspector/test/unit/test_acl.py index 4429dadd3..71df94837 100644 --- a/ironic_inspector/test/unit/test_acl.py +++ b/ironic_inspector/test/unit/test_acl.py @@ -40,6 +40,12 @@ admin_context = oslo_context.RequestContext( roles=['admin', 'member', 'reader'], ) +MANAGER_TOKEN = uuid.uuid4().hex +manager_context = oslo_context.RequestContext( + user_id=MANAGER_TOKEN, + roles=['manager', 'member', 'reader'], +) + MEMBER_TOKEN = uuid.uuid4().hex member_context = oslo_context.RequestContext( user_id=MEMBER_TOKEN, @@ -58,6 +64,12 @@ no_role_context = oslo_context.RequestContext( roles=[], ) +SERVICE_TOKEN = uuid.uuid4().hex +service_context = oslo_context.RequestContext( + user_id=SERVICE_TOKEN, + roles=['service'] +) + # Tokens for deprecated policy tests BM_ADMIN_TOKEN = uuid.uuid4().hex bm_admin_context = oslo_context.RequestContext( @@ -73,6 +85,8 @@ bm_observer_context = oslo_context.RequestContext( USERS = { ADMIN_TOKEN: admin_context.to_dict(), + MANAGER_TOKEN: manager_context.to_dict(), + SERVICE_TOKEN: service_context.to_dict(), MEMBER_TOKEN: member_context.to_dict(), READER_TOKEN: reader_context.to_dict(), NO_ROLE_TOKEN: no_role_context.to_dict(), @@ -83,6 +97,8 @@ USERS = { class BasePolicyTest(test_base.BaseTest): + system_scope = False + def init_app(self): CONF.set_override('auth_strategy', 'keystone') main._app.testing = True @@ -132,7 +148,7 @@ class BasePolicyTest(test_base.BaseTest): 'X-Auth-Token': token, 'X-Roles': ','.join(self.fake_token['roles']) } - if cfg.CONF.oslo_policy.enforce_scope: + if self.system_scope: headers['OpenStack-System-Scope'] = 'all' return headers @@ -265,6 +281,8 @@ class TestACLDeprecated(BasePolicyTest): class TestRBACScoped(BasePolicyTest): + system_scope = True + def setUp(self): super(TestRBACScoped, self).setUp() cfg.CONF.set_override('enforce_scope', True, group='oslo_policy') @@ -275,6 +293,10 @@ class TestRBACScoped(BasePolicyTest): self.assert_status(200, ADMIN_TOKEN, self.app.get, '/') self.assert_status(200, ADMIN_TOKEN, self.app.get, '/v1') + def test_root_system_service(self): + self.assert_status(200, SERVICE_TOKEN, self.app.get, '/') + self.assert_status(200, SERVICE_TOKEN, self.app.get, '/v1') + def test_root_system_member(self): self.assert_status(200, MEMBER_TOKEN, self.app.get, '/') self.assert_status(200, MEMBER_TOKEN, self.app.get, '/v1') @@ -291,6 +313,10 @@ class TestRBACScoped(BasePolicyTest): self.assert_status(202, ADMIN_TOKEN, self.app.post, '/v1/introspection/%s' % self.uuid) + def test_introspect_system_service(self): + self.assert_status(202, SERVICE_TOKEN, self.app.post, + '/v1/introspection/%s' % self.uuid) + def test_introspect_system_member(self): self.assert_status(403, MEMBER_TOKEN, self.app.post, '/v1/introspection/%s' % self.uuid) @@ -303,6 +329,10 @@ class TestRBACScoped(BasePolicyTest): self.assert_status(202, ADMIN_TOKEN, self.app.post, '/v1/introspection/%s/abort' % self.uuid) + def test_abort_system_service(self): + self.assert_status(202, SERVICE_TOKEN, self.app.post, + '/v1/introspection/%s/abort' % self.uuid) + def test_abort_system_member(self): self.assert_status(403, MEMBER_TOKEN, self.app.post, '/v1/introspection/%s/abort' % self.uuid) @@ -315,6 +345,10 @@ class TestRBACScoped(BasePolicyTest): self.assert_status(200, ADMIN_TOKEN, self.app.get, '/v1/introspection/%s' % self.uuid) + def test_status_system_service(self): + self.assert_status(200, SERVICE_TOKEN, self.app.get, + '/v1/introspection/%s' % self.uuid) + def test_status_system_member(self): self.assert_status(200, MEMBER_TOKEN, self.app.get, '/v1/introspection/%s' % self.uuid) @@ -327,6 +361,10 @@ class TestRBACScoped(BasePolicyTest): self.assert_status(200, ADMIN_TOKEN, self.app.get, '/v1/introspection') + def test_list_system_service(self): + self.assert_status(200, SERVICE_TOKEN, self.app.get, + '/v1/introspection') + def test_list_system_member(self): self.assert_status(200, MEMBER_TOKEN, self.app.get, '/v1/introspection') @@ -339,6 +377,10 @@ class TestRBACScoped(BasePolicyTest): self.assert_status(404, ADMIN_TOKEN, self.app.get, '/v1/introspection/%s/data' % self.uuid) + def test_data_system_service(self): + self.assert_status(404, SERVICE_TOKEN, self.app.get, + '/v1/introspection/%s/data' % self.uuid) + def test_data_system_member(self): self.assert_status(403, MEMBER_TOKEN, self.app.get, '/v1/introspection/%s/data' % self.uuid) @@ -352,6 +394,11 @@ class TestRBACScoped(BasePolicyTest): '/v1/introspection/%s/data/unprocessed' % self.uuid, data={'foo': 'bar'}) + def test_data_unprocessed_system_service(self): + self.assert_status(400, SERVICE_TOKEN, self.app.post, + '/v1/introspection/%s/data/unprocessed' % self.uuid, + data={'foo': 'bar'}) + def test_data_unprocessed_system_member(self): self.assert_status(403, MEMBER_TOKEN, self.app.post, '/v1/introspection/%s/data/unprocessed' % self.uuid, @@ -366,6 +413,10 @@ class TestRBACScoped(BasePolicyTest): self.assert_status(200, ADMIN_TOKEN, self.app.get, '/v1/rules') + def test_rule_list_system_service(self): + self.assert_status(200, SERVICE_TOKEN, self.app.get, + '/v1/rules') + def test_rule_list_system_member(self): self.assert_status(403, MEMBER_TOKEN, self.app.get, '/v1/rules') @@ -378,6 +429,10 @@ class TestRBACScoped(BasePolicyTest): self.assert_status(404, ADMIN_TOKEN, self.app.get, '/v1/rules/foo') + def test_rule_get_system_service(self): + self.assert_status(404, SERVICE_TOKEN, self.app.get, + '/v1/rules/foo') + def test_rule_get_system_member(self): self.assert_status(403, MEMBER_TOKEN, self.app.get, '/v1/rules/foo') @@ -390,6 +445,10 @@ class TestRBACScoped(BasePolicyTest): self.assert_status(204, ADMIN_TOKEN, self.app.delete, '/v1/rules') + def test_rule_delete_all_system_service(self): + self.assert_status(204, SERVICE_TOKEN, self.app.delete, + '/v1/rules') + def test_rule_delete_all_system_member(self): self.assert_status(403, MEMBER_TOKEN, self.app.delete, '/v1/rules') @@ -402,6 +461,10 @@ class TestRBACScoped(BasePolicyTest): self.assert_status(404, ADMIN_TOKEN, self.app.delete, '/v1/rules/foo') + def test_rule_delete_system_service(self): + self.assert_status(404, SERVICE_TOKEN, self.app.delete, + '/v1/rules/foo') + def test_rule_delete_system_member(self): self.assert_status(403, MEMBER_TOKEN, self.app.delete, '/v1/rules/foo') @@ -419,6 +482,15 @@ class TestRBACScoped(BasePolicyTest): 'actions': 'act' }) + def test_rule_create_system_service(self): + self.assert_status(500, SERVICE_TOKEN, self.app.post, + '/v1/rules', + data={ + 'uuid': self.uuid, + 'conditions': 'cond', + 'actions': 'act' + }) + def test_rule_create_system_member(self): self.assert_status(403, MEMBER_TOKEN, self.app.post, '/v1/rules', @@ -436,3 +508,288 @@ class TestRBACScoped(BasePolicyTest): 'conditions': 'cond', 'actions': 'act' }) + + +class TestRBACProjectScope(BasePolicyTest): + + system_scope = False + + def setUp(self): + super(TestRBACProjectScope, self).setUp() + cfg.CONF.set_override('enforce_scope', True, group='oslo_policy') + cfg.CONF.set_override('enforce_new_defaults', True, + group='oslo_policy') + + def test_root_project_admin(self): + self.assert_status(200, ADMIN_TOKEN, self.app.get, '/') + self.assert_status(200, ADMIN_TOKEN, self.app.get, '/v1') + + def test_root_project_manager(self): + self.assert_status(200, MANAGER_TOKEN, self.app.get, '/') + self.assert_status(200, MANAGER_TOKEN, self.app.get, '/v1') + + def test_root_project_service(self): + self.assert_status(200, SERVICE_TOKEN, self.app.get, '/') + self.assert_status(200, SERVICE_TOKEN, self.app.get, '/v1') + + def test_root_project_member(self): + self.assert_status(200, MEMBER_TOKEN, self.app.get, '/') + self.assert_status(200, MEMBER_TOKEN, self.app.get, '/v1') + + def test_root_project_reader(self): + self.assert_status(200, READER_TOKEN, self.app.get, '/') + self.assert_status(200, READER_TOKEN, self.app.get, '/v1') + + def test_root_project_no_role(self): + self.assert_status(200, NO_ROLE_TOKEN, self.app.get, '/') + self.assert_status(200, NO_ROLE_TOKEN, self.app.get, '/v1') + + def test_introspect_project_admin(self): + self.assert_status(202, ADMIN_TOKEN, self.app.post, + '/v1/introspection/%s' % self.uuid) + + def test_introspect_project_manager(self): + self.assert_status(403, MANAGER_TOKEN, self.app.post, + '/v1/introspection/%s' % self.uuid) + + def test_introspect_project_service(self): + self.assert_status(202, SERVICE_TOKEN, self.app.post, + '/v1/introspection/%s' % self.uuid) + + def test_introspect_project_member(self): + self.assert_status(403, MEMBER_TOKEN, self.app.post, + '/v1/introspection/%s' % self.uuid) + + def test_introspect_project_reader(self): + self.assert_status(403, READER_TOKEN, self.app.post, + '/v1/introspection/%s/abort' % self.uuid) + + def test_abort_project_admin(self): + self.assert_status(202, ADMIN_TOKEN, self.app.post, + '/v1/introspection/%s/abort' % self.uuid) + + def test_abort_project_manager(self): + self.assert_status(403, MANAGER_TOKEN, self.app.post, + '/v1/introspection/%s/abort' % self.uuid) + + def test_abort_project_service(self): + self.assert_status(202, SERVICE_TOKEN, self.app.post, + '/v1/introspection/%s/abort' % self.uuid) + + def test_abort_project_member(self): + self.assert_status(403, MEMBER_TOKEN, self.app.post, + '/v1/introspection/%s/abort' % self.uuid) + + def test_abort_project_reader(self): + self.assert_status(403, READER_TOKEN, self.app.post, + '/v1/introspection/%s/abort' % self.uuid) + + def test_status_project_admin(self): + self.assert_status(200, ADMIN_TOKEN, self.app.get, + '/v1/introspection/%s' % self.uuid) + + def test_status_project_manager(self): + self.assert_status(403, MANAGER_TOKEN, self.app.get, + '/v1/introspection/%s' % self.uuid) + + def test_status_project_service(self): + self.assert_status(200, SERVICE_TOKEN, self.app.get, + '/v1/introspection/%s' % self.uuid) + + def test_status_project_member(self): + self.assert_status(403, MEMBER_TOKEN, self.app.get, + '/v1/introspection/%s' % self.uuid) + + def test_status_project_reader(self): + self.assert_status(403, READER_TOKEN, self.app.get, + '/v1/introspection/%s' % self.uuid) + + def test_list_project_admin(self): + self.assert_status(200, ADMIN_TOKEN, self.app.get, + '/v1/introspection') + + def test_list_project_manager(self): + self.assert_status(403, MANAGER_TOKEN, self.app.get, + '/v1/introspection') + + def test_list_project_service(self): + self.assert_status(200, SERVICE_TOKEN, self.app.get, + '/v1/introspection') + + def test_list_project_member(self): + self.assert_status(403, MEMBER_TOKEN, self.app.get, + '/v1/introspection') + + def test_list_project_reader(self): + self.assert_status(403, READER_TOKEN, self.app.get, + '/v1/introspection') + + def test_data_project_admin(self): + self.assert_status(404, ADMIN_TOKEN, self.app.get, + '/v1/introspection/%s/data' % self.uuid) + + def test_data_project_manager(self): + self.assert_status(403, MANAGER_TOKEN, self.app.get, + '/v1/introspection/%s/data' % self.uuid) + + def test_data_project_service(self): + self.assert_status(404, SERVICE_TOKEN, self.app.get, + '/v1/introspection/%s/data' % self.uuid) + + def test_data_project_member(self): + self.assert_status(403, MEMBER_TOKEN, self.app.get, + '/v1/introspection/%s/data' % self.uuid) + + def test_data_project_reader(self): + self.assert_status(403, READER_TOKEN, self.app.get, + '/v1/introspection/%s/data' % self.uuid) + + def test_data_unprocessed_project_admin(self): + self.assert_status(400, ADMIN_TOKEN, self.app.post, + '/v1/introspection/%s/data/unprocessed' % self.uuid, + data={'foo': 'bar'}) + + def test_data_unprocessed_project_manager(self): + self.assert_status(403, MANAGER_TOKEN, self.app.post, + '/v1/introspection/%s/data/unprocessed' % self.uuid, + data={'foo': 'bar'}) + + def test_data_unprocessed_project_service(self): + self.assert_status(400, SERVICE_TOKEN, self.app.post, + '/v1/introspection/%s/data/unprocessed' % self.uuid, + data={'foo': 'bar'}) + + def test_data_unprocessed_project_member(self): + self.assert_status(403, MEMBER_TOKEN, self.app.post, + '/v1/introspection/%s/data/unprocessed' % self.uuid, + data={'foo': 'bar'}) + + def test_data_unprocessed_project_reader(self): + self.assert_status(403, READER_TOKEN, self.app.post, + '/v1/introspection/%s/data/unprocessed' % self.uuid, + data={'foo': 'bar'}) + + def test_rule_list_project_admin(self): + self.assert_status(200, ADMIN_TOKEN, self.app.get, + '/v1/rules') + + def test_rule_list_project_manager(self): + self.assert_status(403, MANAGER_TOKEN, self.app.get, + '/v1/rules') + + def test_rule_list_project_service(self): + self.assert_status(200, SERVICE_TOKEN, self.app.get, + '/v1/rules') + + def test_rule_list_project_member(self): + self.assert_status(403, MEMBER_TOKEN, self.app.get, + '/v1/rules') + + def test_rule_list_project_reader(self): + self.assert_status(403, READER_TOKEN, self.app.get, + '/v1/rules') + + def test_rule_get_project_admin(self): + self.assert_status(404, ADMIN_TOKEN, self.app.get, + '/v1/rules/foo') + + def test_rule_get_project_manager(self): + self.assert_status(403, MANAGER_TOKEN, self.app.get, + '/v1/rules/foo') + + def test_rule_get_project_service(self): + self.assert_status(404, SERVICE_TOKEN, self.app.get, + '/v1/rules/foo') + + def test_rule_get_project_member(self): + self.assert_status(403, MEMBER_TOKEN, self.app.get, + '/v1/rules/foo') + + def test_rule_get_project_reader(self): + self.assert_status(403, READER_TOKEN, self.app.get, + '/v1/rules/foo') + + def test_rule_delete_all_project_admin(self): + self.assert_status(204, ADMIN_TOKEN, self.app.delete, + '/v1/rules') + + def test_rule_delete_all_project_manager(self): + self.assert_status(403, MANAGER_TOKEN, self.app.delete, + '/v1/rules') + + def test_rule_delete_all_project_service(self): + self.assert_status(204, SERVICE_TOKEN, self.app.delete, + '/v1/rules') + + def test_rule_delete_all_project_member(self): + self.assert_status(403, MEMBER_TOKEN, self.app.delete, + '/v1/rules') + + def test_rule_delete_all_project_reader(self): + self.assert_status(403, READER_TOKEN, self.app.delete, + '/v1/rules') + + def test_rule_delete_project_admin(self): + self.assert_status(404, ADMIN_TOKEN, self.app.delete, + '/v1/rules/foo') + + def test_rule_delete_project_manager(self): + self.assert_status(403, MANAGER_TOKEN, self.app.delete, + '/v1/rules/foo') + + def test_rule_delete_project_service(self): + self.assert_status(404, SERVICE_TOKEN, self.app.delete, + '/v1/rules/foo') + + def test_rule_delete_project_member(self): + self.assert_status(403, MEMBER_TOKEN, self.app.delete, + '/v1/rules/foo') + + def test_rule_delete_project_reader(self): + self.assert_status(403, READER_TOKEN, self.app.delete, + '/v1/rules/foo') + + def test_rule_create_project_admin(self): + self.assert_status(500, ADMIN_TOKEN, self.app.post, + '/v1/rules', + data={ + 'uuid': self.uuid, + 'conditions': 'cond', + 'actions': 'act' + }) + + def test_rule_create_project_manager(self): + self.assert_status(403, MANAGER_TOKEN, self.app.post, + '/v1/rules', + data={ + 'uuid': self.uuid, + 'conditions': 'cond', + 'actions': 'act' + }) + + def test_rule_create_project_service(self): + self.assert_status(500, SERVICE_TOKEN, self.app.post, + '/v1/rules', + data={ + 'uuid': self.uuid, + 'conditions': 'cond', + 'actions': 'act' + }) + + def test_rule_create_project_member(self): + self.assert_status(403, MEMBER_TOKEN, self.app.post, + '/v1/rules', + data={ + 'uuid': self.uuid, + 'conditions': 'cond', + 'actions': 'act' + }) + + def test_rule_create_project_reader(self): + self.assert_status(403, READER_TOKEN, self.app.post, + '/v1/rules', + data={ + 'uuid': self.uuid, + 'conditions': 'cond', + 'actions': 'act' + }) diff --git a/releasenotes/notes/role-service-for-openstack-rbac-changes-7ca8533f76e504d5.yaml b/releasenotes/notes/role-service-for-openstack-rbac-changes-7ca8533f76e504d5.yaml new file mode 100644 index 000000000..76e080a0c --- /dev/null +++ b/releasenotes/notes/role-service-for-openstack-rbac-changes-7ca8533f76e504d5.yaml @@ -0,0 +1,14 @@ +--- +fixes: + - | + Fixes the Role Based Access Control state and capabilities to align with + OpenStack Community RBAC goals which includes support for a ``service`` + role by default to enable inter-service communication to be configured + without an ``admin`` username. In large part, these changes were missed + as the Inspector service is considered an "admin-only" service. + + Also in alignment with overall community position changes, where the + ``admin`` role is sufficent without an explicit ``system`` scope. To + help ensure a high level of security, explicit testing was also added + for the ``manager`` role, which is unavailable as that role is reserved + for administrative functions inside of a tenant's project.