Merge "Clean up some policy code"

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

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

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)

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()

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