From b4e700e5928df030389f9d2657dc71d7d3afa367 Mon Sep 17 00:00:00 2001 From: Ghanshyam Mann Date: Tue, 31 Mar 2020 10:25:43 -0500 Subject: [PATCH] Pass the actual target in unlock override 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 unlock override 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: Ie3e6667df1e8f5d3e96ac291106f7e4b0273f0ef --- nova/api/openstack/compute/lock_server.py | 2 +- nova/tests/unit/policies/test_lock_server.py | 36 ++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/nova/api/openstack/compute/lock_server.py b/nova/api/openstack/compute/lock_server.py index 3f9a4388a6..5615ba15e8 100644 --- a/nova/api/openstack/compute/lock_server.py +++ b/nova/api/openstack/compute/lock_server.py @@ -55,6 +55,6 @@ class LockServerController(wsgi.Controller): target={'project_id': instance.project_id}) if not self.compute_api.is_expected_locked_by(context, instance): context.can(ls_policies.POLICY_ROOT % 'unlock:unlock_override', - instance) + target={'project_id': instance.project_id}) self.compute_api.unlock(context, instance) diff --git a/nova/tests/unit/policies/test_lock_server.py b/nova/tests/unit/policies/test_lock_server.py index f4de7f3d9c..4925214022 100644 --- a/nova/tests/unit/policies/test_lock_server.py +++ b/nova/tests/unit/policies/test_lock_server.py @@ -12,6 +12,7 @@ import fixtures import mock +from nova.policies import base as base_policy from nova.policies import lock_server as ls_policies from oslo_utils.fixture import uuidsentinel as uuids from oslo_utils import timeutils @@ -182,3 +183,38 @@ class LockServerNoLegacyPolicyTest(LockServerScopeTypePolicyTest): self.other_project_member_context, self.project_foo_context, self.project_reader_context ] + + +class LockServerOverridePolicyTest(LockServerNoLegacyPolicyTest): + """Test Lock Server 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(LockServerOverridePolicyTest, self).setUp() + + # Check that system admin or project scoped role as override above + # is able to unlock the server which is locked by other + 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 + # unlock the server which is locked by other + 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 + ] + + def test_unlock_override_server_policy(self): + rule = ls_policies.POLICY_ROOT % 'unlock:unlock_override' + self.policy.set_rules({ + # make unlock allowed for everyone so that we can check unlock + # override policy. + ls_policies.POLICY_ROOT % 'unlock': "@", + rule: base_policy.PROJECT_MEMBER_OR_SYSTEM_ADMIN}, overwrite=False) + super(LockServerOverridePolicyTest, + self).test_unlock_override_server_policy()