openstack/manila
Code Issues Proposed changes

Clean up some policy code

Browse Source 
oslo policy handles the mapping of
credentials from a context object to values
that oslo policy cares about. This mapping
includes some deprecations and compatibility
handling code that we must take advantage of [1].
So, stop mapping context to policy values
on our end, and rely on oslo.policy handling
this for us.

enforce and authorize methods in policy.py
do the same thing, but with a subtle
difference. The "enforce" method doesn't
raise errors when unregistered policies are
invoked - this shouldn't ever be the case
for any policies written/maintained within
manila - however, we support API extensions
and don't dictate what must be done there. So
add docstrings to clarify that we shouldn't
invoke enforce, ever.

Also handle InvalidPolicyScope exceptions
and raise the oslo.policy library version
since some test enhancements have been
committed in the latest version.

[1] d3185debdb/oslo_policy/policy.py (L1077-L1096)

Change-Id: I069bf7143d6ff66b3dcdc34c9b52d48f5808481b
Signed-off-by: Goutham Pacha Ravi <gouthampravi@gmail.com>
This commit is contained in:
Goutham Pacha Ravi 2021-02-25 23:41:57 -08:00
parent 00133f93ae
commit 8553962997
5 changed files with 71 additions and 27 deletions

2
lower-constraints.txt

@ -64,7 +64,7 @@ oslo.i18n==5.0.1
oslo.log==4.4.0
oslo.messaging==12.5.0
oslo.middleware==4.1.1
oslo.policy==3.6.0
oslo.policy==3.6.2
oslo.reports==2.2.0
oslo.rootwrap==6.2.0
oslo.serialization==4.0.1

9
manila/api/extensions.py

@ -22,10 +22,9 @@ from oslo_utils import importutils
import webob.dec
import webob.exc
import manila.api.openstack
from manila.api.openstack import wsgi
from manila import exception
import manila.policy
from manila import policy
CONF = cfg.CONF
LOG = log.getLogger(__name__)
@ -332,12 +331,10 @@ def load_standard_extensions(ext_mgr, logger, path, package, ext_list=None):
def extension_authorizer(api_name, extension_name):
def authorize(context, target=None, action=None):
if target is None:
target = {'project_id': context.project_id,
'user_id': context.user_id}
target = target or policy.default_target(context)
if action is None:
act = '%s_extension:%s' % (api_name, extension_name)
else:
act = '%s_extension:%s:%s' % (api_name, extension_name, action)
manila.policy.enforce(context, act, target)
policy.enforce(context, act, target)
return authorize

50
manila/policy.py

@ -69,6 +69,10 @@ def init(rules=None, use_conf=True):
def enforce(context, action, target, do_raise=True):
"""Verifies that the action is valid on the target in this context.
**IMPORTANT** ONLY for use in API extensions. This method ignores
unregistered rules and applies a default rule on them; there should
be no unregistered rules in first party manila APIs.
:param context: manila context
:param action: string representing the action to be checked,
this should be colon separated for clarity.
@ -88,12 +92,15 @@ def enforce(context, action, target, do_raise=True):
"""
init()
return _ENFORCER.enforce(action,
target,
context.to_policy_values(),
do_raise=do_raise,
exc=exception.PolicyNotAuthorized,
action=action)
try:
return _ENFORCER.enforce(action,
target,
context,
do_raise=do_raise,
exc=exception.PolicyNotAuthorized,
action=action)
except policy.InvalidScope:
raise exception.PolicyNotAuthorized(action=action)
def set_rules(rules, overwrite=True, use_conf=False):
@ -165,32 +172,41 @@ def authorize(context, action, target, do_raise=True, exc=None):
do_raise is False.
"""
init()
credentials = context.to_policy_values()
if not exc:
exc = exception.PolicyNotAuthorized
target = target or 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('Policy not registered')
except policy.InvalidScope:
raise exception.PolicyNotAuthorized(action=action)
except Exception:
with excutils.save_and_reraise_exception():
msg_args = {
'action': action,
'credentials': context.to_policy_values(),
}
LOG.debug('Policy check for %(action)s failed with credentials '
'%(credentials)s',
{'action': action, 'credentials': credentials})
'%(credentials)s', msg_args)
return result
def default_target(context):
return {'project_id': context.project_id, 'user_id': context.user_id}
def check_is_admin(context):
"""Whether or not user is admin according to policy setting.
"""
init()
credentials = context.to_policy_values()
target = credentials
return _ENFORCER.authorize('context_is_admin', target, credentials)
# the target is user-self
target = default_target(context)
return _ENFORCER.authorize('context_is_admin', target, context)
def wrap_check_policy(resource):
@ -206,10 +222,6 @@ def wrap_check_policy(resource):
def check_policy(context, resource, action, target_obj=None, do_raise=True):
target = {
'project_id': context.project_id,
'user_id': context.user_id,
}
target.update(target_obj or {})
target = target_obj or default_target(context)
_action = '%s:%s' % (resource, action)
return authorize(context, _action, target, do_raise=do_raise)

35
manila/tests/test_policy.py

@ -15,6 +15,7 @@
"""Test of Policy Engine For Manila."""
import ddt
from oslo_config import cfg
from oslo_policy import policy as common_policy
@ -26,6 +27,7 @@ from manila import test
CONF = cfg.CONF
@ddt.ddt
class PolicyTestCase(test.TestCase):
def setUp(self):
super(PolicyTestCase, self).setUp()
@ -97,8 +99,41 @@ class PolicyTestCase(test.TestCase):
policy.authorize(admin_context, lowercase_action, self.target)
policy.authorize(admin_context, uppercase_action, self.target)
@ddt.data('enforce', 'authorize')
def test_authorize_properly_handles_invalid_scope_exception(self, method):
self.fixture.config(enforce_scope=True, group='oslo_policy')
project_context = context.RequestContext(project_id='fake-project-id',
roles=['bar'])
policy.reset()
policy.init()
rule = common_policy.RuleDefault('foo', 'role:bar',
scope_types=['system'])
policy._ENFORCER.register_defaults([rule])
self.assertRaises(exception.PolicyNotAuthorized,
getattr(policy, method),
project_context, 'foo', {})
@ddt.data('enforce', 'authorize')
def test_authorize_does_not_raise_forbidden(self, method):
self.fixture.config(enforce_scope=False, group='oslo_policy')
project_context = context.RequestContext(project_id='fake-project-id',
roles=['bar'])
policy.reset()
policy.init()
rule = common_policy.RuleDefault('foo', 'role:bar',
scope_types=['system'])
policy._ENFORCER.register_defaults([rule])
self.assertTrue(getattr(policy, method)(project_context, 'foo', {}))
class DefaultPolicyTestCase(test.TestCase):
"""This test case calls into the "enforce" method in policy
enforce() in contrast with authorize() allows "default" rules to apply
to policies that have not been registered.
"""
def setUp(self):
super(DefaultPolicyTestCase, self).setUp()

2
requirements.txt

@ -17,7 +17,7 @@ oslo.i18n>=5.0.1 # Apache-2.0
oslo.log>=4.4.0 # Apache-2.0
oslo.messaging>=12.5.0 # Apache-2.0
oslo.middleware>=4.1.1 # Apache-2.0
oslo.policy>=3.6.0 # Apache-2.0
oslo.policy>=3.6.2 # Apache-2.0
oslo.reports>=2.2.0 # Apache-2.0
oslo.rootwrap>=6.2.0 # Apache-2.0
oslo.serialization>=4.0.1 # Apache-2.0