diff --git a/lower-constraints.txt b/lower-constraints.txt index 57f22de724..7486b72fbd 100644 --- a/lower-constraints.txt +++ b/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 diff --git a/manila/api/extensions.py b/manila/api/extensions.py index bea35bec94..6bbdca258e 100644 --- a/manila/api/extensions.py +++ b/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 diff --git a/manila/policy.py b/manila/policy.py index 05ac7be479..f0501999a0 100644 --- a/manila/policy.py +++ b/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) diff --git a/manila/tests/test_policy.py b/manila/tests/test_policy.py index a0d8bb4569..cab315f41d 100644 --- a/manila/tests/test_policy.py +++ b/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() diff --git a/requirements.txt b/requirements.txt index b9b71ca5e1..638607c8bc 100644 --- a/requirements.txt +++ b/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