From c27af238ad99c0330eb4b55398f44be28e6f0485 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Mon, 10 Dec 2018 14:58:46 -0500 Subject: [PATCH] Fix target used in nova.policy.check_is_admin The target passed to Enforcer.authorize should be a dict, similar to the target dict to the RequestContext.can method. However, we were passing an instance of _DeprecatedPolicyValues because that is ultimately what comes out of RequestContext.to_policy_values(). As of change I4642c57990b145c0e691140970574412682e66a5 in oslo.policy, that incorrect type for the target parameter results in an error in the debug logs for the policy check: cannot format data, exception: Expected a dictionary, got instead. This resolves the issue by using the same default target dict that RequestContext.can uses if a target is not supplied. Note that we get here from NovaKeystoneContext via API middleware before any request handler is invoked in the wsgi stack, so there is no context from the request as to what to pass for the target besides the user_id/project_id. Change-Id: I4442a7b95d15233f76f7795d45b18ac440ddb831 Closes-Bug: #1807747 --- nova/context.py | 6 ++++-- nova/policy.py | 2 +- nova/tests/unit/test_policy.py | 11 +++++++++++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/nova/context.py b/nova/context.py index f290a02be7fb..3543f2dde4f5 100644 --- a/nova/context.py +++ b/nova/context.py @@ -250,8 +250,7 @@ class RequestContext(context.RequestContext): authorized and False if not authorized and fatal is False. """ if target is None: - target = {'project_id': self.project_id, - 'user_id': self.user_id} + target = self.default_target() try: return policy.authorize(self, action, target) @@ -260,6 +259,9 @@ class RequestContext(context.RequestContext): raise return False + def default_target(self): + return {'project_id': self.project_id, 'user_id': self.user_id} + def to_policy_values(self): policy = super(RequestContext, self).to_policy_values() policy['is_admin'] = self.is_admin diff --git a/nova/policy.py b/nova/policy.py index cfdb1097a3a3..cad1f16f5c28 100644 --- a/nova/policy.py +++ b/nova/policy.py @@ -176,7 +176,7 @@ def check_is_admin(context): init() # the target is user-self credentials = context.to_policy_values() - target = credentials + target = context.default_target() return _ENFORCER.authorize('context_is_admin', target, credentials) diff --git a/nova/tests/unit/test_policy.py b/nova/tests/unit/test_policy.py index 5ddf5d20343c..33f838ccb9d2 100644 --- a/nova/tests/unit/test_policy.py +++ b/nova/tests/unit/test_policy.py @@ -244,6 +244,17 @@ class IsAdminCheckTestCase(test.NoDBTestCase): self.assertTrue(check('target', dict(is_admin=False), policy._ENFORCER)) + def test_check_is_admin(self): + ctxt = context.RequestContext( + user_id='fake-user', project_id='fake-project') + with mock.patch('oslo_policy.policy.Enforcer.authorize') as mock_auth: + result = policy.check_is_admin(ctxt) + self.assertEqual(mock_auth.return_value, result) + mock_auth.assert_called_once_with( + 'context_is_admin', + {'user_id': 'fake-user', 'project_id': 'fake-project'}, + ctxt.to_policy_values()) + class AdminRolePolicyTestCase(test.NoDBTestCase): def setUp(self):