From d74365969e8c3736103b32458d53b176a3c68428 Mon Sep 17 00:00:00 2001 From: Ghanshyam Mann Date: Mon, 9 Sep 2024 14:34:10 -0700 Subject: [PATCH] Drop system scope from RBAC In new RBAC rules, all openstack services except ironic and keystone have dropped the system scope. - https://governance.openstack.org/tc/goals/selected/consistent-and-secure-rbac.html#phase-1 All the policy rules should be scoped to project and system scoped token should not be allowed to access the Cyborg APIs. As oslo.policy 4.4.0 enable the scope check and new defaults, it break cyborg tests and to fix that we need to drop the system scope from the cyborg policies. Ref: - https://lists.openstack.org/archives/list/openstack-discuss@lists.openstack.org/thread/6IDPXIXZ6FBKONNHFRU3ZPSXIIQJXUIE/ - https://lists.openstack.org/archives/list/openstack-discuss@lists.openstack.org/message/MPHSVG222OFHJL2AQD2A7CJGTH57SRCJ/ Change-Id: Iafbd5470e55e10ffb6bcddd232a698eb042d7eed --- cyborg/policies/base.py | 136 ++++++++---------- cyborg/policies/device_profiles.py | 20 +-- .../unit/policies/test_device_profiles.py | 12 +- 3 files changed, 76 insertions(+), 92 deletions(-) diff --git a/cyborg/policies/base.py b/cyborg/policies/base.py index f6961c12..7a438010 100644 --- a/cyborg/policies/base.py +++ b/cyborg/policies/base.py @@ -19,75 +19,6 @@ from oslo_policy import policy # All legacy policy and new policy mapping for all V2 APIs can be found # here:https://wiki.openstack.org/wiki/Cyborg/Policy - -# TODO(yumeng) Special string ``system_scope:all`` -# We are explicitly setting system_scope:all in these check strings because -# they provide backwards compatibility in the event a deployment sets -# ``cyborg.conf [oslo_policy] enforce_scope = False``, which the default. -# Otherwise, this might open up APIs to be more permissive unintentionally if a -# deployment isn't enforcing scope. For example, the new rule for action -# 'cyborg:device_profile:create' will be System Scoped Admin with -# ``role:admin`` and scope_type=['system']. However, it would be possible for -# users with the ``admin`` role on a project to access the -# 'cyborg:device_profile:create' until enforce_scope=True is set by default. -# Once cyborg defaults ``cyborg.conf [oslo_policy] enforce_scope = True``, -# the the ``system_scope:all`` bits of these check strings -# can be removed since that will be handled automatically by scope_types in -# oslo.policy's RuleDefault objects. -SYSTEM_ADMIN = 'rule:system_admin_api' -SYSTEM_READER = 'rule:system_reader_api' -PROJECT_ADMIN = 'rule:project_admin_api' -PROJECT_MEMBER = 'rule:project_member_api' -PROJECT_READER = 'rule:project_reader_api' -PROJECT_MEMBER_OR_SYSTEM_ADMIN = 'rule:system_admin_or_owner' -PROJECT_READER_OR_SYSTEM_READER = 'rule:system_or_project_reader' - -# NOTE(yumeng): Keystone already support implied roles means assignment -# of one role implies the assignment of another. New defaults roles -# `reader`, `member` also has been added in bootstrap. If the bootstrap -# process is re-run, and a `reader`, `member`, or `admin` role already -# exists, a role implication chain will be created: `admin` implies -# `member` implies `reader`. -# For example: If we give access to 'reader' it means the 'admin' and -# 'member' also get access. - -# NOTE(yumeng) the rules listed include both old rules and new rules. -# legacy rules list: 'public_api','allow','deny','admin_api','is_admin', -# 'admin_or_owner','admin_or_user'. -# new rules list: system_admin_api,system_reader_api,project_admin_api, -# project_member_api, project_reader_api, system_admin_or_owner, -# system_or_project_reader . -default_policies = [ - policy.RuleDefault( - name="system_admin_api", - check_str='role:admin and system_scope:all', - description="Default rule for System Admin APIs."), - policy.RuleDefault( - name="system_reader_api", - check_str="role:reader and system_scope:all", - description="Default rule for System level read only APIs."), - policy.RuleDefault( - name="project_admin_api", - check_str="role:admin and project_id:%(project_id)s", - description="Default rule for Project level admin APIs."), - policy.RuleDefault( - name="project_member_api", - check_str="role:member and project_id:%(project_id)s", - description="Default rule for Project level non admin APIs."), - policy.RuleDefault( - name="project_reader_api", - check_str="role:reader and project_id:%(project_id)s", - description="Default rule for Project level read only APIs."), - policy.RuleDefault( - name="system_admin_or_owner", - check_str="rule:system_admin_api or rule:project_member_api", - description="Default rule for system_admin+owner APIs."), - policy.RuleDefault( - name="system_or_project_reader", - check_str="rule:system_reader_api or rule:project_reader_api", - description="Default rule for System+Project read only APIs.") -] - DEPRECATED_REASON = """ Cyborg API policies are introducing new default roles with scope_type capabilities. We will start to deprecate old policies from WALLABY release, @@ -98,6 +29,16 @@ relying on overrides in your deployment for the policy API. deprecated_default = 'rule:admin_or_owner' deprecated_is_admin = 'rule:is_admin' + +ADMIN_OR_OWNER = 'rule:admin_or_owner' + +DEPRECATED_ADMIN_OR_OWNER = policy.DeprecatedRule( + name=ADMIN_OR_OWNER, + check_str='is_admin:True or project_id:%(project_id)s', + deprecated_reason=DEPRECATED_REASON, + deprecated_since=versionutils.deprecated.WALLABY +) + deprecated_default_policies = [ # is_public_api is set in the environment from AuthTokenMiddleware policy.RuleDefault( @@ -131,13 +72,6 @@ deprecated_default_policies = [ deprecated_for_removal=True, deprecated_reason=DEPRECATED_REASON, deprecated_since=versionutils.deprecated.WALLABY), - policy.RuleDefault( - name='admin_api', - check_str='role:admin or role:administrator', - description='Legacy rule for cloud admin access', - deprecated_for_removal=True, - deprecated_reason=DEPRECATED_REASON, - deprecated_since=versionutils.deprecated.WALLABY), policy.RuleDefault( name='is_admin', check_str='rule:admin_api', @@ -161,6 +95,56 @@ deprecated_default_policies = [ deprecated_since=versionutils.deprecated.WALLABY), ] +ADMIN = 'rule:admin_api' +PROJECT_ADMIN = 'rule:project_admin_api' +PROJECT_MEMBER = 'rule:project_member_api' +PROJECT_READER = 'rule:project_reader_api' +PROJECT_MEMBER_OR_ADMIN = 'rule:project_member_or_admin' +PROJECT_READER_OR_ADMIN = 'rule:project_reader_or_admin' + +# NOTE(yumeng): Keystone already support implied roles means assignment +# of one role implies the assignment of another. New defaults roles +# `reader`, `member` also has been added in bootstrap. If the bootstrap +# process is re-run, and a `reader`, `member`, or `admin` role already +# exists, a role implication chain will be created: `admin` implies +# `member` implies `reader`. +# For example: If we give access to 'reader' it means the 'admin' and +# 'member' also get access. + +# NOTE(yumeng) the rules listed include both old rules and new rules. +# legacy rules list: 'public_api','allow','deny','admin_api','is_admin', +# 'admin_or_owner','admin_or_user'. +# new rules list: project_admin_api, project_member_api, project_reader_api, +# project_member_or_admin, project_reader_or_admin . +default_policies = [ + policy.RuleDefault( + name='admin_api', + check_str='role:admin or role:administrator', + description='Legacy rule for cloud admin access'), + policy.RuleDefault( + name="project_admin_api", + check_str="role:admin and project_id:%(project_id)s", + description="Default rule for Project level admin APIs."), + policy.RuleDefault( + name="project_member_api", + check_str="role:member and project_id:%(project_id)s", + description="Default rule for Project level non admin APIs."), + policy.RuleDefault( + name="project_reader_api", + check_str="role:reader and project_id:%(project_id)s", + description="Default rule for Project level read only APIs."), + policy.RuleDefault( + "project_member_or_admin", + "rule:project_member_api or rule:admin_api", + "Default rule for Project Member or admin APIs.", + deprecated_rule=DEPRECATED_ADMIN_OR_OWNER), + policy.RuleDefault( + "project_reader_or_admin", + "rule:project_reader_api or rule:admin_api", + "Default rule for Project reader or admin APIs.", + deprecated_rule=DEPRECATED_ADMIN_OR_OWNER) +] + def list_policies(): return default_policies \ diff --git a/cyborg/policies/device_profiles.py b/cyborg/policies/device_profiles.py index 26b8b992..9332cd07 100644 --- a/cyborg/policies/device_profiles.py +++ b/cyborg/policies/device_profiles.py @@ -46,53 +46,53 @@ deprecated_create = policy.DeprecatedRule( name='cyborg:device_profile:create', check_str=base.deprecated_is_admin, deprecated_reason=('project_admin_or_owner is too permissive, ' - 'introduce system_scoped admin for creation'), + 'introduce admin for creation'), deprecated_since=versionutils.deprecated.WALLABY) deprecated_delete = policy.DeprecatedRule( name='cyborg:device_profile:delete', check_str=base.deprecated_default, deprecated_reason=('project_admin_or_owner is too permissive, ' - 'introduce system_scoped admin for deletion'), + 'introduce admin for deletion'), deprecated_since=versionutils.deprecated.WALLABY) # new device_profile policies device_profile_policies = [ policy.DocumentedRuleDefault( name='cyborg:device_profile:get_all', - check_str=base.PROJECT_READER_OR_SYSTEM_READER, + check_str=base.PROJECT_READER_OR_ADMIN, description='Retrieve all device_profiles', operations=[ { 'path': '/v2/device_profiles', 'method': 'GET' }], - scope_types=['system', 'project'], + scope_types=['project'], deprecated_rule=deprecated_get_all), policy.DocumentedRuleDefault( name='cyborg:device_profile:get_one', - check_str=base.PROJECT_READER_OR_SYSTEM_READER, + check_str=base.PROJECT_READER_OR_ADMIN, description='Retrieve a specific device_profile', operations=[ { 'path': '/v2/device_profiles/{device_profiles_uuid}', 'method': 'GET' }], - scope_types=['system', 'project'], + scope_types=['project'], deprecated_rule=deprecated_get_one), policy.DocumentedRuleDefault( name='cyborg:device_profile:create', - check_str=base.SYSTEM_ADMIN, + check_str=base.ADMIN, description='Create a device_profile', operations=[ { 'path': '/v2/device_profiles', 'method': 'POST' }], - scope_types=['system'], + scope_types=['project'], deprecated_rule=deprecated_create), policy.DocumentedRuleDefault( name='cyborg:device_profile:delete', - check_str=base.SYSTEM_ADMIN, + check_str=base.ADMIN, description='Delete device_profile(s)', operations=[ { @@ -102,7 +102,7 @@ device_profile_policies = [ 'path': '/v2/device_profiles?value={device_profile_name1}', 'method': 'DELETE'}, ], - scope_types=['system'], + scope_types=['project'], deprecated_rule=deprecated_delete), ] diff --git a/cyborg/tests/unit/policies/test_device_profiles.py b/cyborg/tests/unit/policies/test_device_profiles.py index 8bc64dd4..2d1557a4 100644 --- a/cyborg/tests/unit/policies/test_device_profiles.py +++ b/cyborg/tests/unit/policies/test_device_profiles.py @@ -153,17 +153,17 @@ class DeviceProfileScopeTypePolicyTest(DeviceProfilePolicyTest): def setUp(self): super(DeviceProfileScopeTypePolicyTest, self).setUp() self.flags(enforce_scope=True, group="oslo_policy") - - # check that system_admin is able to do create and delete operations. + # check that admin is able to do create and delete operations. self.create_authorized_contexts = [ - self.system_admin_context] + self.legacy_admin_context, + self.project_admin_context] self.delete_authorized_contexts = self.create_authorized_contexts - # Check that non-system or non-admin is not able to perform the system + # Check that system or non-admin is not able to perform the system # level actions on device_profiles. self.create_unauthorized_contexts = [ - self.legacy_admin_context, self.system_member_context, + self.system_admin_context, self.system_member_context, self.system_reader_context, self.system_foo_context, - self.project_admin_context, self.project_member_context, + self.project_member_context, self.other_project_member_context, self.project_foo_context, self.project_reader_context ]