From 55ae1a5d5397638de7cfd2aec732a7685cc39f10 Mon Sep 17 00:00:00 2001 From: Ghanshyam Mann Date: Thu, 1 Aug 2019 13:43:30 +0000 Subject: [PATCH] Pass RequestContext to oslo_policy In nova.policy.authorize() method, it convert the context object to policy value by context.to_policy_values() and then pass that into oslo.policy enforcer authorize() method. This is fine till now and does not cause any issue but when scope_type is set on policy and context then scope info is not passed correctly. In case of system scope, oslo.policy check for a key called 'system' in creds. The oslo.context library uses `system_scope` instead[1], and the compatibility between both oslo.policy and oslo.context are handled when complete context is passed into oslo_policy[2]. If nova convert the context object to policy values then system scope info is not passed into the oslo_policy. Better way is to pass the complete context object to oslo_policy and let oslo_policy fetch the system scope info in correct way. Update the lower constraints for oslo.policy and oslo.context to have system scope checks feature. Partial implement blueprint policy-defaults-refresh [1] https://github.com/openstack/oslo.context/blob/f65408df5cd5924f2879c3ee94d07fd27cb2cf73/oslo_context/context.py#L321 [2] https://github.com/openstack/oslo.policy/blob/b9fd10e2612f26c93d49c168a0408aba6d20e5bf/oslo_policy/policy.py#L994 Change-Id: I847fc44e62065e3d26e5595e178b83912ab5d19b --- lower-constraints.txt | 4 ++-- nova/policy.py | 15 ++++++++++----- nova/tests/unit/test_policy.py | 18 +++++++++++++++++- requirements.txt | 4 ++-- 4 files changed, 31 insertions(+), 10 deletions(-) diff --git a/lower-constraints.txt b/lower-constraints.txt index ae2a119bdfa2..fb7bc1e63abc 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -76,13 +76,13 @@ osc-lib==1.10.0 oslo.cache==1.26.0 oslo.concurrency==3.26.0 oslo.config==6.1.0 -oslo.context==2.19.2 +oslo.context==2.21.0 oslo.db==4.44.0 oslo.i18n==3.15.3 oslo.log==3.36.0 oslo.messaging==7.0.0 oslo.middleware==3.31.0 -oslo.policy==1.35.0 +oslo.policy==1.38.0 oslo.privsep==1.33.2 oslo.reports==1.18.0 oslo.rootwrap==5.8.0 diff --git a/nova/policy.py b/nova/policy.py index 4f211872b3ff..1117b08d42f3 100644 --- a/nova/policy.py +++ b/nova/policy.py @@ -153,7 +153,6 @@ def authorize(context, action, target=None, do_raise=True, exc=None): do_raise is False. """ init() - credentials = context.to_policy_values() if not exc: exc = exception.PolicyNotAuthorized @@ -163,16 +162,23 @@ def authorize(context, action, target=None, do_raise=True, exc=None): target = default_target(context) try: - result = _ENFORCER.authorize(action, target, credentials, + result = _ENFORCER.authorize(action, target, context, do_raise=do_raise, exc=exc, action=action) except policy.PolicyNotRegistered: with excutils.save_and_reraise_exception(): LOG.exception(_LE('Policy not registered')) + except policy.InvalidScope: + LOG.debug('Policy check for %(action)s failed with scope check ' + '%(credentials)s', + {'action': action, + 'credentials': context.to_policy_values()}) + raise exc(action=action) except Exception: with excutils.save_and_reraise_exception(): LOG.debug('Policy check for %(action)s failed with credentials ' '%(credentials)s', - {'action': action, 'credentials': credentials}) + {'action': action, + 'credentials': context.to_policy_values()}) return result @@ -187,9 +193,8 @@ def check_is_admin(context): init() # the target is user-self - credentials = context.to_policy_values() target = default_target(context) - return _ENFORCER.authorize('context_is_admin', target, credentials) + return _ENFORCER.authorize('context_is_admin', target, context) @policy.register('is_admin') diff --git a/nova/tests/unit/test_policy.py b/nova/tests/unit/test_policy.py index 3a77b632f54f..73a96622ada1 100644 --- a/nova/tests/unit/test_policy.py +++ b/nova/tests/unit/test_policy.py @@ -185,6 +185,22 @@ class PolicyTestCase(test.NoDBTestCase): "project_id:%(project_id)s")]) mock_warning.assert_not_called() + @requests_mock.mock() + def test_authorize_raise_invalid_scope(self, req_mock): + req_mock.post('http://www.example.com/', + text='False') + action = "example:get_http" + target = {} + with mock.patch('oslo_policy.policy.Enforcer.authorize') as auth_mock: + auth_mock.side_effect = oslo_policy.InvalidScope( + action, self.context.system_scope, 'invalid_scope') + exc = self.assertRaises(exception.PolicyNotAuthorized, + policy.authorize, self.context, + action, target) + self.assertEqual( + "Policy doesn't allow %s to be performed." % action, + exc.format_message()) + @mock.patch.object(policy.LOG, 'warning') def test_verify_deprecated_policy_using_old_action(self, mock_warning): @@ -256,7 +272,7 @@ class IsAdminCheckTestCase(test.NoDBTestCase): mock_auth.assert_called_once_with( 'context_is_admin', {'user_id': 'fake-user', 'project_id': 'fake-project'}, - ctxt.to_policy_values()) + ctxt) class AdminRolePolicyTestCase(test.NoDBTestCase): diff --git a/requirements.txt b/requirements.txt index 57e45ceba487..f623b60bff72 100644 --- a/requirements.txt +++ b/requirements.txt @@ -37,7 +37,7 @@ websockify>=0.8.0 # LGPLv3 oslo.cache>=1.26.0 # Apache-2.0 oslo.concurrency>=3.26.0 # Apache-2.0 oslo.config>=6.1.0 # Apache-2.0 -oslo.context>=2.19.2 # Apache-2.0 +oslo.context>=2.21.0 # Apache-2.0 oslo.log>=3.36.0 # Apache-2.0 oslo.reports>=1.18.0 # Apache-2.0 oslo.serialization!=2.19.1,>=2.21.1 # Apache-2.0 @@ -46,7 +46,7 @@ oslo.utils>=3.40.2 # Apache-2.0 oslo.db>=4.44.0 # Apache-2.0 oslo.rootwrap>=5.8.0 # Apache-2.0 oslo.messaging>=7.0.0 # Apache-2.0 -oslo.policy>=1.35.0 # Apache-2.0 +oslo.policy>=1.38.0 # Apache-2.0 oslo.privsep>=1.33.2 # Apache-2.0 oslo.i18n>=3.15.3 # Apache-2.0 oslo.service>=1.40.1 # Apache-2.0