From 51abb44ee7125f52f4c7be47473402107b1f7e05 Mon Sep 17 00:00:00 2001 From: Ghanshyam Mann Date: Wed, 18 Mar 2020 06:56:05 -0500 Subject: [PATCH] Add new default roles in os-flavor-access policies This adds new defaults roles in os-flavor-access API policies. This policy is default to SYSTEM_ADMIN role for add/remove tenant access and SYSTEM_READER for list the access information. Also add tests to simulates the future where we drop the deprecation fall back in the policy by overriding the rules with a version where there are no deprecated rule options. Operators can do the same by adding overrides in their policy files that match the default but stop the rule deprecation fallback from happening. Partial implement blueprint policy-defaults-refresh Closes-Bug: #1867840 Change-Id: Ieeaafe923b78f03ddcbec18d8759aa1d76bcfcb1 --- nova/policies/flavor_access.py | 34 +++++++---- nova/tests/unit/fake_policy.py | 1 + .../tests/unit/policies/test_flavor_access.py | 59 +++++++++++++++++-- 3 files changed, 78 insertions(+), 16 deletions(-) diff --git a/nova/policies/flavor_access.py b/nova/policies/flavor_access.py index 512072ac3873..ee9d469bb3b3 100644 --- a/nova/policies/flavor_access.py +++ b/nova/policies/flavor_access.py @@ -22,11 +22,28 @@ from nova.policies import base BASE_POLICY_NAME = 'os_compute_api:os-flavor-access' POLICY_ROOT = 'os_compute_api:os-flavor-access:%s' +# NOTE(gmann): Deprecating this policy explicitly as old defaults +# admin or owner is not suitable for that which should be admin (Bug#1867840) +# but changing that will break old deployment so let's keep supporting +# the old default also and new default can be SYSTEM_READER +# SYSTEM_READER rule in base class is defined with the deprecated rule of admin +# not admin or owner which is the main reason that we need to explicitly +# deprecate this policy here. +DEPRECATED_FLAVOR_ACCESS_POLICY = policy.DeprecatedRule( + BASE_POLICY_NAME, + base.RULE_ADMIN_OR_OWNER, +) + +DEPRECATED_REASON = """ +Nova API policies are introducing new default roles with scope_type +capabilities. Old policies are deprecated and silently going to be ignored +in nova 23.0.0 release. +""" flavor_access_policies = [ policy.DocumentedRuleDefault( name=POLICY_ROOT % 'add_tenant_access', - check_str=base.RULE_ADMIN_API, + check_str=base.SYSTEM_ADMIN, description="Add flavor access to a tenant", operations=[ { @@ -37,7 +54,7 @@ flavor_access_policies = [ scope_types=['system']), policy.DocumentedRuleDefault( name=POLICY_ROOT % 'remove_tenant_access', - check_str=base.RULE_ADMIN_API, + check_str=base.SYSTEM_ADMIN, description="Remove flavor access from a tenant", operations=[ { @@ -48,7 +65,7 @@ flavor_access_policies = [ scope_types=['system']), policy.DocumentedRuleDefault( name=BASE_POLICY_NAME, - check_str=base.RULE_ADMIN_OR_OWNER, + check_str=base.SYSTEM_READER, description="""List flavor access information Allows access to the full list of tenants that have access @@ -60,13 +77,10 @@ to a flavor via an os-flavor-access API. 'path': '/flavors/{flavor_id}/os-flavor-access' }, ], - # NOTE(gmann): This policy is admin_or_owner by default but allowed - # for everyone, bug#1867840. There can be multiple project with access - # to specific flavorso we cannot say there is single owner of flavor. - # Only admin should be able to list the projects having access to any - # flavor. We should change this policy defaults to admin only. I am - # seeting scope as 'system' only and new defaults can be SYSTEM_ADMIN. - scope_types=['system']), + scope_types=['system'], + deprecated_rule=DEPRECATED_FLAVOR_ACCESS_POLICY, + deprecated_reason=DEPRECATED_REASON, + deprecated_since='21.0.0'), ] diff --git a/nova/tests/unit/fake_policy.py b/nova/tests/unit/fake_policy.py index 8fccf2b610c2..19967b8b78b4 100644 --- a/nova/tests/unit/fake_policy.py +++ b/nova/tests/unit/fake_policy.py @@ -42,6 +42,7 @@ policy_data = """ "os_compute_api:extensions": "", "os_compute_api:os-flavor-access:remove_tenant_access": "", "os_compute_api:os-flavor-access:add_tenant_access": "", + "os_compute_api:os-flavor-access": "", "os_compute_api:os-flavor-extra-specs:index": "", "os_compute_api:os-flavor-extra-specs:show": "", "os_compute_api:os-flavor-manage:create": "", diff --git a/nova/tests/unit/policies/test_flavor_access.py b/nova/tests/unit/policies/test_flavor_access.py index 21ce26e54de4..b2f632d8cc37 100644 --- a/nova/tests/unit/policies/test_flavor_access.py +++ b/nova/tests/unit/policies/test_flavor_access.py @@ -15,6 +15,7 @@ import mock from oslo_utils.fixture import uuidsentinel as uuids from nova.api.openstack.compute import flavor_access +from nova.policies import base as base_policy from nova.policies import flavor_access as fa_policy from nova.tests.unit.api.openstack import fakes from nova.tests.unit import fake_flavor @@ -64,7 +65,7 @@ class FlavorAccessPolicyTest(base.BasePolicyTest): # Check that everyone is able to list flavor access # information which is nothing but bug#1867840. - self.admin_or_owner_authorized_contexts = [ + self.reader_authorized_contexts = [ self.legacy_admin_context, self.system_admin_context, self.project_admin_context, self.project_member_context, self.project_reader_context, self.project_foo_context, @@ -72,13 +73,13 @@ class FlavorAccessPolicyTest(base.BasePolicyTest): self.system_foo_context, self.other_project_member_context] - self.admin_or_owner_unauthorized_contexts = [ + self.reader_unauthorized_contexts = [ ] def test_list_flavor_access_policy(self): rule_name = fa_policy.BASE_POLICY_NAME - self.common_policy_check(self.admin_or_owner_authorized_contexts, - self.admin_or_owner_unauthorized_contexts, + self.common_policy_check(self.reader_authorized_contexts, + self.reader_unauthorized_contexts, rule_name, self.controller_index.index, self.req, '1') @@ -134,13 +135,59 @@ class FlavorAccessScopeTypePolicyTest(FlavorAccessPolicyTest): # Check that system user is able to list flavor access # information. - self.admin_or_owner_authorized_contexts = [ + self.reader_authorized_contexts = [ self.system_admin_context, self.system_member_context, self.system_reader_context, self.system_foo_context] # Check that non-system is not able to list flavor access # information. - self.admin_or_owner_unauthorized_contexts = [ + self.reader_unauthorized_contexts = [ self.legacy_admin_context, self.other_project_member_context, self.project_admin_context, self.project_member_context, self.project_reader_context, self.project_foo_context] + + +class FlavorAccessNoLegacyPolicyTest(FlavorAccessPolicyTest): + """Test FlavorAccess APIs policies with system scope enabled, + and no more deprecated rules that allow the legacy admin API to + access system_redear APIs. + """ + without_deprecated_rules = True + rules_without_deprecation = { + fa_policy.POLICY_ROOT % "add_tenant_access": + base_policy.SYSTEM_ADMIN, + fa_policy.POLICY_ROOT % "remove_tenant_access": + base_policy.SYSTEM_ADMIN, + fa_policy.BASE_POLICY_NAME: + base_policy.SYSTEM_READER} + + def setUp(self): + super(FlavorAccessNoLegacyPolicyTest, self).setUp() + self.flags(enforce_scope=True, group="oslo_policy") + + # Check that system admin is able to add/remove flavor access + # to a tenant. + self.admin_authorized_contexts = [ + self.system_admin_context] + # Check that non-system-admin is not able to add/remove flavor access + # to a tenant. + self.admin_unauthorized_contexts = [ + self.legacy_admin_context, self.system_member_context, + self.system_reader_context, self.project_admin_context, + self.system_foo_context, self.project_member_context, + self.other_project_member_context, + self.project_foo_context, self.project_reader_context + ] + + # Check that system reader is able to list flavor access + # information. + self.reader_authorized_contexts = [ + self.system_admin_context, + self.system_member_context, self.system_reader_context] + # Check that non-system-reader is not able to list flavor access + # information. + self.reader_unauthorized_contexts = [ + self.legacy_admin_context, self.other_project_member_context, + self.project_admin_context, self.project_member_context, + self.project_reader_context, self.project_foo_context, + self.system_foo_context]