From 10c0a44cff56550d5dac09bb47fd49ce80fba86c Mon Sep 17 00:00:00 2001 From: Ghanshyam Mann Date: Wed, 1 Apr 2020 20:30:22 -0500 Subject: [PATCH] Pass the actual target in server diagnostics policy Currently if target is not passed in context.can(), it use defauls target which is context.user_id, context.project_id. These defaults target are not useful as it pass the context's user_id and project_id only which means we tell oslo policy to verify the context data with context data. This commit pass the actual target for server diagnostics policies which is server project_id because policy rule is system and project scoped. Adding tests also to show that rule can be override with project roles. Partial implement blueprint policy-defaults-refresh Change-Id: Ie9af539cf1cdbce14c30dbf89e4676ebc50f3246 --- .../openstack/compute/server_diagnostics.py | 4 +-- .../compute/test_server_diagnostics.py | 4 ++- .../unit/policies/test_server_diagnostics.py | 32 +++++++++++++++++++ 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/nova/api/openstack/compute/server_diagnostics.py b/nova/api/openstack/compute/server_diagnostics.py index f559dd036512..325d81ab063b 100644 --- a/nova/api/openstack/compute/server_diagnostics.py +++ b/nova/api/openstack/compute/server_diagnostics.py @@ -34,9 +34,9 @@ class ServerDiagnosticsController(wsgi.Controller): @wsgi.expected_errors((400, 404, 409, 501)) def index(self, req, server_id): context = req.environ["nova.context"] - context.can(sd_policies.BASE_POLICY_NAME) - instance = common.get_instance(self.compute_api, context, server_id) + context.can(sd_policies.BASE_POLICY_NAME, + target={'project_id': instance.project_id}) try: if api_version_request.is_supported(req, min_version='2.48'): diff --git a/nova/tests/unit/api/openstack/compute/test_server_diagnostics.py b/nova/tests/unit/api/openstack/compute/test_server_diagnostics.py index 9c15f7b30e4a..d215f3e9037d 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_diagnostics.py +++ b/nova/tests/unit/api/openstack/compute/test_server_diagnostics.py @@ -32,7 +32,9 @@ def fake_instance_get(self, _context, instance_uuid, expected_attrs=None, cell_down_support=False): if instance_uuid != UUID: raise Exception("Invalid UUID") - return objects.Instance(uuid=instance_uuid, host='123') + return objects.Instance(uuid=instance_uuid, + project_id=_context.project_id, + host='123') class ServerDiagnosticsTestV21(test.NoDBTestCase): diff --git a/nova/tests/unit/policies/test_server_diagnostics.py b/nova/tests/unit/policies/test_server_diagnostics.py index 5758ff24d413..2d3cf2d1ced0 100644 --- a/nova/tests/unit/policies/test_server_diagnostics.py +++ b/nova/tests/unit/policies/test_server_diagnostics.py @@ -17,6 +17,7 @@ from oslo_utils import timeutils from nova.api.openstack.compute import server_diagnostics from nova.compute import vm_states +from nova.policies import base as base_policy from nova.policies import server_diagnostics as policies from nova.tests.unit.api.openstack import fakes from nova.tests.unit import fake_instance @@ -103,3 +104,34 @@ class ServerDiagnosticsNoLegacyPolicyTest( self.project_reader_context, self.project_foo_context, self.other_project_member_context ] + + +class ServerDiagnosticsOverridePolicyTest(ServerDiagnosticsNoLegacyPolicyTest): + """Test Server Diagnostics APIs policies with system and project scoped + but default to system roles only are allowed for project roles + if override by operators. This test is with system scope enable + and no more deprecated rules. + """ + + def setUp(self): + super(ServerDiagnosticsOverridePolicyTest, self).setUp() + rule = policies.BASE_POLICY_NAME + # NOTE(gmann): override the rule to project member and verify it + # work as policy is system and projct scoped. + self.policy.set_rules({ + rule: base_policy.PROJECT_MEMBER_OR_SYSTEM_ADMIN}, + overwrite=False) + + # Check that system admin or project scoped role as override above + # is able to get server diagnostics. + self.admin_authorized_contexts = [ + self.system_admin_context, + self.project_admin_context, self.project_member_context] + # Check that non-system admin or project role is not able to + # get server diagnostics. + self.admin_unauthorized_contexts = [ + self.legacy_admin_context, self.system_member_context, + self.system_reader_context, self.system_foo_context, + self.other_project_member_context, + self.project_foo_context, self.project_reader_context + ]