From cd0b96176ac8e51a88fc6f388b31f3758089d87c Mon Sep 17 00:00:00 2001 From: Ghanshyam Mann Date: Tue, 31 Mar 2020 01:28:09 -0500 Subject: [PATCH] Fix unpause server policy to be admin_or_owner unpause server API policy is default to admin_or_owner[1] but API is allowed for everyone. We can see the test trying with other project context can access the API - https://review.opendev.org/#/c/716161/ This is because API does not pass the server project_id in policy target[2] and if no target is passed then, policy.py add the default targets which is nothing but context.project_id (allow for everyone who try to access)[3] This commit fix this policy by passing the server's project_id in policy target. Closes-bug: #1869841 Partial implement blueprint policy-defaults-refresh [1] - https://github.com/openstack/nova/blob/eb6bd04e4c27c70b5239dbe7c17607b37f4e87dd/nova/policies/pause_server.py#L38 [2] - https://github.com/openstack/nova/blob/eb6bd04e4c27c70b5239dbe7c17607b37f4e87dd/nova/api/openstack/compute/pause_server.py#L58 [3] - https://github.com/openstack/nova/blob/c16315165ce307c605cf4b608b2df3aa06f46982/nova/policy.py#L191 Change-Id: Iacfaec63eb380863657b44c7f5ff14f6209e3857 --- nova/api/openstack/compute/pause_server.py | 3 ++- nova/tests/unit/api/openstack/compute/test_pause_server.py | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/nova/api/openstack/compute/pause_server.py b/nova/api/openstack/compute/pause_server.py index cc0a186b3662..4d6bbc6cf147 100644 --- a/nova/api/openstack/compute/pause_server.py +++ b/nova/api/openstack/compute/pause_server.py @@ -55,8 +55,9 @@ class PauseServerController(wsgi.Controller): def _unpause(self, req, id, body): """Permit Admins to unpause the server.""" ctxt = req.environ['nova.context'] - ctxt.can(ps_policies.POLICY_ROOT % 'unpause') server = common.get_instance(self.compute_api, ctxt, id) + ctxt.can(ps_policies.POLICY_ROOT % 'unpause', + target={'project_id': server.project_id}) try: self.compute_api.unpause(ctxt, server) except exception.InstanceIsLocked as e: diff --git a/nova/tests/unit/api/openstack/compute/test_pause_server.py b/nova/tests/unit/api/openstack/compute/test_pause_server.py index 2f8df8850904..ec58587fa2d0 100644 --- a/nova/tests/unit/api/openstack/compute/test_pause_server.py +++ b/nova/tests/unit/api/openstack/compute/test_pause_server.py @@ -114,7 +114,12 @@ class PauseServerPolicyEnforcementV21(test.NoDBTestCase): pause_mock.assert_called_once_with(self.req.environ['nova.context'], instance) - def test_unpause_policy_failed(self): + @mock.patch('nova.api.openstack.common.get_instance') + def test_unpause_policy_failed(self, get_instance_mock): + instance = fake_instance.fake_instance_obj( + self.req.environ['nova.context'], + user_id=self.req.environ['nova.context'].user_id) + get_instance_mock.return_value = instance rule_name = "os_compute_api:os-pause-server:unpause" self.policy.set_rules({rule_name: "project:non_fake"}) exc = self.assertRaises(