From 01948df1a0d9b5ac0e31f14d4050f494e135ce82 Mon Sep 17 00:00:00 2001 From: Ghanshyam Mann Date: Sat, 8 Feb 2020 12:30:02 -0600 Subject: [PATCH] Add new default roles in os-atttach-inerfaces policies This adds new defaults roles in os-attach-interfaces API policies. - GET rules are made granular and default to System or project reader - Other policies are default to system admin or project member. 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 Change-Id: Ic405544560ae75ad88c10b8ff6d5048b3728cd2b --- .../openstack/compute/attach_interfaces.py | 4 +- nova/policies/attach_interfaces.py | 45 ++++-- nova/policies/base.py | 5 +- nova/tests/unit/fake_policy.py | 5 +- nova/tests/unit/policies/base.py | 2 + .../unit/policies/test_attach_interfaces.py | 131 +++++++++++++++++- nova/tests/unit/test_policy.py | 3 +- 7 files changed, 175 insertions(+), 20 deletions(-) diff --git a/nova/api/openstack/compute/attach_interfaces.py b/nova/api/openstack/compute/attach_interfaces.py index 23bb19053b68..50e82d679364 100644 --- a/nova/api/openstack/compute/attach_interfaces.py +++ b/nova/api/openstack/compute/attach_interfaces.py @@ -68,7 +68,7 @@ class InterfaceAttachmentController(wsgi.Controller): """Returns the list of interface attachments for a given instance.""" context = req.environ['nova.context'] instance = common.get_instance(self.compute_api, context, server_id) - context.can(ai_policies.BASE_POLICY_NAME, + context.can(ai_policies.POLICY_ROOT % 'list', target={'project_id': instance.project_id}) search_opts = {'device_id': instance.uuid} @@ -110,7 +110,7 @@ class InterfaceAttachmentController(wsgi.Controller): """Return data about the given interface attachment.""" context = req.environ['nova.context'] instance = common.get_instance(self.compute_api, context, server_id) - context.can(ai_policies.BASE_POLICY_NAME, + context.can(ai_policies.POLICY_ROOT % 'show', target={'project_id': instance.project_id}) port_id = id diff --git a/nova/policies/attach_interfaces.py b/nova/policies/attach_interfaces.py index 8b0cfd6a9d76..be751af157c2 100644 --- a/nova/policies/attach_interfaces.py +++ b/nova/policies/attach_interfaces.py @@ -20,28 +20,49 @@ from nova.policies import base BASE_POLICY_NAME = 'os_compute_api:os-attach-interfaces' POLICY_ROOT = 'os_compute_api:os-attach-interfaces:%s' +DEPRECATED_INTERFACES_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. +""" attach_interfaces_policies = [ policy.DocumentedRuleDefault( - name=BASE_POLICY_NAME, - check_str=base.RULE_ADMIN_OR_OWNER, - description="List port interfaces or show details of a port interface " - "attached to a server", + name=POLICY_ROOT % 'list', + check_str=base.PROJECT_READER_OR_SYSTEM_READER, + description="List port interfaces attached to a server", operations=[ { 'method': 'GET', 'path': '/servers/{server_id}/os-interface' }, + ], + scope_types=['system', 'project'], + deprecated_rule=DEPRECATED_INTERFACES_POLICY, + deprecated_reason=DEPRECATED_REASON, + deprecated_since='21.0.0'), + policy.DocumentedRuleDefault( + name=POLICY_ROOT % 'show', + check_str=base.PROJECT_READER_OR_SYSTEM_READER, + description="Show details of a port interface attached to a server", + operations=[ { 'method': 'GET', 'path': '/servers/{server_id}/os-interface/{port_id}' } ], - scope_types=['system', 'project']), + scope_types=['system', 'project'], + deprecated_rule=DEPRECATED_INTERFACES_POLICY, + deprecated_reason=DEPRECATED_REASON, + deprecated_since='21.0.0'), policy.DocumentedRuleDefault( name=POLICY_ROOT % 'create', - check_str=base.RULE_ADMIN_OR_OWNER, + check_str=base.PROJECT_MEMBER_OR_SYSTEM_ADMIN, description="Attach an interface to a server", operations=[ { @@ -49,10 +70,13 @@ attach_interfaces_policies = [ 'path': '/servers/{server_id}/os-interface' } ], - scope_types=['system', 'project']), + scope_types=['system', 'project'], + deprecated_rule=DEPRECATED_INTERFACES_POLICY, + deprecated_reason=DEPRECATED_REASON, + deprecated_since='21.0.0'), policy.DocumentedRuleDefault( name=POLICY_ROOT % 'delete', - check_str=base.RULE_ADMIN_OR_OWNER, + check_str=base.PROJECT_MEMBER_OR_SYSTEM_ADMIN, description="Detach an interface from a server", operations=[ { @@ -60,7 +84,10 @@ attach_interfaces_policies = [ 'path': '/servers/{server_id}/os-interface/{port_id}' } ], - scope_types=['system', 'project']) + scope_types=['system', 'project'], + deprecated_rule=DEPRECATED_INTERFACES_POLICY, + deprecated_reason=DEPRECATED_REASON, + deprecated_since='21.0.0') ] diff --git a/nova/policies/base.py b/nova/policies/base.py index ba4b67d79ef3..96b9d8e91a3b 100644 --- a/nova/policies/base.py +++ b/nova/policies/base.py @@ -130,7 +130,10 @@ rules = [ policy.RuleDefault( "system_or_project_reader", "rule:system_reader_api or rule:project_reader_api", - "Default rule for System+Project read only APIs.") + "Default rule for System+Project read only APIs.", + deprecated_rule=DEPRECATED_ADMIN_OR_OWNER_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 8883e92b819b..8512c78b2f78 100644 --- a/nova/tests/unit/fake_policy.py +++ b/nova/tests/unit/fake_policy.py @@ -26,7 +26,10 @@ policy_data = """ "os_compute_api:os-admin-actions:reset_state": "", "os_compute_api:os-admin-password": "", "os_compute_api:os-agents": "", - "os_compute_api:os-attach-interfaces": "", + "os_compute_api:os-attach-interfaces:list": "", + "os_compute_api:os-attach-interfaces:show": "", + "os_compute_api:os-attach-interfaces:create": "", + "os_compute_api:os-attach-interfaces:delete": "", "os_compute_api:os-baremetal-nodes": "", "os_compute_api:os-console-output": "", "os_compute_api:os-remote-consoles": "", diff --git a/nova/tests/unit/policies/base.py b/nova/tests/unit/policies/base.py index 19679fe8eb61..8f38ec68aefd 100644 --- a/nova/tests/unit/policies/base.py +++ b/nova/tests/unit/policies/base.py @@ -103,6 +103,8 @@ class BasePolicyTest(test.TestCase): self.rules_without_deprecation.update({ "system_admin_or_owner": "rule:system_admin_api or rule:project_member_api", + "system_or_project_reader": + "rule:system_reader_api or rule:project_reader_api", "system_admin_api": "role:admin and system_scope:all", "system_reader_api": diff --git a/nova/tests/unit/policies/test_attach_interfaces.py b/nova/tests/unit/policies/test_attach_interfaces.py index 4bfd04017c9e..8f3794025222 100644 --- a/nova/tests/unit/policies/test_attach_interfaces.py +++ b/nova/tests/unit/policies/test_attach_interfaces.py @@ -17,9 +17,13 @@ from oslo_utils import timeutils from nova.api.openstack.compute import attach_interfaces from nova.compute import vm_states +from nova import exception +from nova.policies import attach_interfaces as ai_policies +from nova.policies import base as base_policy from nova.tests.unit.api.openstack import fakes from nova.tests.unit import fake_instance from nova.tests.unit.policies import base +from nova.tests.unit import policy_fixture class AttachInterfacesPolicyTest(base.BasePolicyTest): @@ -55,19 +59,31 @@ class AttachInterfacesPolicyTest(base.BasePolicyTest): self.other_project_member_context ] + self.reader_authorized_contexts = [ + self.legacy_admin_context, self.system_admin_context, + self.project_admin_context, self.system_member_context, + self.system_reader_context, self.project_reader_context, + self.project_member_context, self.project_foo_context + ] + + self.reader_unauthorized_contexts = [ + self.system_foo_context, + self.other_project_member_context + ] + @mock.patch('nova.compute.api.API.get') @mock.patch('nova.network.neutron.API.list_ports') def test_index_interfaces_policy(self, mock_port, mock_get): - rule_name = "os_compute_api:os-attach-interfaces" - self.common_policy_check(self.admin_authorized_contexts, - self.admin_unauthorized_contexts, + rule_name = "os_compute_api:os-attach-interfaces:list" + self.common_policy_check(self.reader_authorized_contexts, + self.reader_unauthorized_contexts, rule_name, self.controller.index, self.req, uuids.fake_id) @mock.patch('nova.compute.api.API.get') @mock.patch('nova.network.neutron.API.show_port') def test_show_interface_policy(self, mock_port, mock_get): - rule_name = "os_compute_api:os-attach-interfaces" + rule_name = "os_compute_api:os-attach-interfaces:show" server_id = uuids.fake_id port_id = uuids.fake_id mock_port.return_value = {'port': { @@ -79,8 +95,8 @@ class AttachInterfacesPolicyTest(base.BasePolicyTest): "fixed_ips": ["10.0.2.2"], "device_id": server_id, }} - self.common_policy_check(self.admin_authorized_contexts, - self.admin_unauthorized_contexts, + self.common_policy_check(self.reader_authorized_contexts, + self.reader_unauthorized_contexts, rule_name, self.controller.show, self.req, server_id, port_id) @@ -120,3 +136,106 @@ class AttachInterfacesScopeTypePolicyTest(AttachInterfacesPolicyTest): def setUp(self): super(AttachInterfacesScopeTypePolicyTest, self).setUp() self.flags(enforce_scope=True, group="oslo_policy") + + +class AttachInterfacesDeprecatedPolicyTest(base.BasePolicyTest): + """Test Attach Interfaces APIs Deprecated policies. + This class checks if deprecated policy rules are + overridden by user on policy.json file then they + still work because oslo.policy add deprecated rules + in logical OR condition and enforce them for policy + checks if overridden. + """ + + def setUp(self): + super(AttachInterfacesDeprecatedPolicyTest, self).setUp() + self.controller = attach_interfaces.InterfaceAttachmentController() + self.admin_req = fakes.HTTPRequest.blank('') + self.admin_req.environ['nova.context'] = self.project_admin_context + self.reader_req = fakes.HTTPRequest.blank('') + self.reader_req.environ['nova.context'] = self.project_reader_context + self.deprecated_policy = "os_compute_api:os-attach-interfaces" + # Overridde rule with different checks than defaults so that we can + # verify the rule overridden case. + override_rules = {self.deprecated_policy: base_policy.RULE_ADMIN_API} + # NOTE(gmann): Only override the deprecated rule in policy file so + # that + # we can verify if overridden checks are considered by oslo.policy. + # Oslo.policy will consider the overridden rules if: + # 1. overridden deprecated rule's checks are different than defaults + # 2. new rules are not present in policy file + self.policy = self.useFixture(policy_fixture.OverridePolicyFixture( + rules_in_file=override_rules)) + + @mock.patch('nova.compute.api.API.get') + @mock.patch('nova.network.neutron.API.list_ports') + def test_deprecated_policy_overridden_rule_is_checked(self, mock_port, + mock_get): + # Test to verify if deprecatd overridden policy is working. + + # check for success as admin role. Deprecated rule + # has been overridden with admin checks in policy.json + # If admin role pass it means overridden rule is enforced by + # olso.policy because new default is system or project reader and the + # old default is admin. + self.controller.index(self.admin_req, uuids.fake_id) + + # check for failure with reader context. + exc = self.assertRaises(exception.PolicyNotAuthorized, + self.controller.index, self.reader_req, + uuids.fake_id) + self.assertEqual( + "Policy doesn't allow os_compute_api:os-attach-interfaces:list" + " to be performed.", + exc.format_message()) + + +class AttachInterfacesNoLegacyPolicyTest(AttachInterfacesPolicyTest): + """Test Attach Interfaces APIs policies with system scope enabled, + and no more deprecated rules that allow the legacy admin API to + access system_admin_or_owner APIs. + """ + without_deprecated_rules = True + rules_without_deprecation = { + ai_policies.POLICY_ROOT % 'list': + base_policy.PROJECT_READER_OR_SYSTEM_READER, + ai_policies.POLICY_ROOT % 'show': + base_policy.PROJECT_READER_OR_SYSTEM_READER, + ai_policies.POLICY_ROOT % 'create': + base_policy.PROJECT_MEMBER_OR_SYSTEM_ADMIN, + ai_policies.POLICY_ROOT % 'delete': + base_policy.PROJECT_MEMBER_OR_SYSTEM_ADMIN} + + def setUp(self): + super(AttachInterfacesNoLegacyPolicyTest, self).setUp() + self.flags(enforce_scope=True, group="oslo_policy") + + # Check that system or projct admin or owner is able to + # create or delete interfaces. + self.admin_authorized_contexts = [ + self.system_admin_context, + self.project_admin_context, self.project_member_context] + # Check that non-system and non-admin/owner is not able to + # create or delete interfaces. + self.admin_unauthorized_contexts = [ + self.legacy_admin_context, self.project_reader_context, + self.project_foo_context, + self.system_member_context, self.system_reader_context, + self.system_foo_context, + self.other_project_member_context] + + # Check that system reader or projct is able to + # create or delete interfaces. + self.reader_authorized_contexts = [ + self.system_admin_context, + self.project_admin_context, self.system_member_context, + self.system_reader_context, self.project_reader_context, + self.project_member_context, + ] + + # Check that non-system reader nd non-admin/owner is not able to + # create or delete interfaces. + self.reader_unauthorized_contexts = [ + self.legacy_admin_context, self.project_foo_context, + self.system_foo_context, self.other_project_member_context + ] diff --git a/nova/tests/unit/test_policy.py b/nova/tests/unit/test_policy.py index e44ce0e4cd15..64e5885c3a14 100644 --- a/nova/tests/unit/test_policy.py +++ b/nova/tests/unit/test_policy.py @@ -415,7 +415,6 @@ class RealRolePolicyTestCase(test.NoDBTestCase): "os_compute_api:servers:update", "os_compute_api:servers:create_image:allow_volume_backed", "os_compute_api:os-admin-password", -"os_compute_api:os-attach-interfaces", "os_compute_api:os-attach-interfaces:create", "os_compute_api:os-attach-interfaces:delete", "os_compute_api:os-console-output", @@ -459,6 +458,8 @@ class RealRolePolicyTestCase(test.NoDBTestCase): self.system_reader_rules = ( "os_compute_api:os-services:list", +"os_compute_api:os-attach-interfaces:list", +"os_compute_api:os-attach-interfaces:show", ) self.allow_nobody_rules = (