Teach Enforcer.enforce to deal with context objects

The ``creds`` dictionary passed into oslo.policy's enforce() method
assumes a lot of the same values already specified by oslo.context
RequestContext objects.

This commit teaches enforce() to handle being passed an instance of
a RequestContext object, and populate credential values accordingly.

Change-Id: Ia74bf6c40b1e05a1c958f4325e00f68be28d91b9
Closes-Bug: 1779172

lower-constraints.txt

@@ -28,6 +28,7 @@ netifaces==0.10.4
 openstackdocstheme==1.18.1
 os-client-config==1.28.0
 oslo.config==5.2.0
+oslo.context==2.21.0
 oslo.i18n==3.15.3
 oslo.serialization==2.18.0
 oslo.utils==3.33.0

oslo_policy/policy.py

@@ -221,12 +221,14 @@ by setting the ``policy_default_rule`` configuration setting to the
 desired rule name.
 """
 
+import collections
 import copy
 import logging
 import os
 import warnings
 
 from oslo_config import cfg
+from oslo_context import context
 from oslo_serialization import jsonutils
 import six
 import yaml
@@ -342,6 +344,13 @@ class InvalidRuleDefault(Exception):
         super(InvalidRuleDefault, self).__init__(msg)
 
 
+class InvalidContextObject(Exception):
+    def __init__(self, error):
+        msg = (_('Invalid context object: '
+                 '%(error)s.') % {'error': error})
+        super(InvalidContextObject, self).__init__(msg)
+
+
 def parse_file_contents(data):
     """Parse the raw contents of a policy file.
 
@@ -789,7 +798,8 @@ class Enforcer(object):
                             the Mapping abstract base class and deep
                             copying.
         :param dict creds: As much information about the user performing the
-                           action as possible.
+                           action as possible. This parameter can also be an
+                           instance of ``oslo_context.context.RequestContext``.
         :param do_raise: Whether to raise an exception or not if check
                         fails.
         :param exc: Class of the exception to raise if the check fails.
@@ -807,6 +817,23 @@ class Enforcer(object):
 
         self.load_rules()
 
+        if isinstance(creds, context.RequestContext):
+            creds = self._map_context_attributes_into_creds(creds)
+        # NOTE(lbragstad): The oslo.context library exposes the ability to call
+        # a method on RequestContext objects that converts attributes of the
+        # context object to policy values. However, ``to_policy_values()``
+        # doesn't actually return a dictionary, it's a subclass of
+        # collections.MutableMapping, which behaves like a dictionary but
+        # doesn't pass the type check.
+        elif not isinstance(creds, collections.MutableMapping):
+            msg = (
+                'Expected type oslo_context.context.RequestContext, dict, or  '
+                'the output of '
+                'oslo_context.context.RequestContext.to_policy_values but '
+                'got %(creds_type)s instead' % {'creds_type': type(creds)}
+            )
+            raise InvalidContextObject(msg)
+
         # Allow the rule to be a Check tree
         if isinstance(rule, _checks.BaseCheck):
             # If the thing we're given is a Check, we don't know the
@@ -881,6 +908,27 @@ class Enforcer(object):
 
         return result
 
+    def _map_context_attributes_into_creds(self, context):
+        creds = {}
+        # port public context attributes into the creds dictionary so long as
+        # the attribute isn't callable
+        context_values = context.to_policy_values()
+        for k, v in context_values.items():
+            creds[k] = v
+
+        # NOTE(lbragstad): We unfortunately have to special case this
+        # attribute. Originally when the system scope when into oslo.policy, we
+        # checked for a key called 'system' in creds. The oslo.context library
+        # uses `system_scope` instead, and the compatibility between
+        # oslo.policy and oslo.context was an afterthought. We'll have to
+        # support services who've been setting creds['system'], but we can do
+        # that by making sure we populate it with what's in the context object
+        # if it has a system_scope attribute.
+        if context.system_scope:
+            creds['system'] = context.system_scope
+
+        return creds
+
     def register_default(self, default):
         """Registers a RuleDefault.
 

oslo_policy/tests/test_policy.py

@@ -19,6 +19,7 @@ import os
 
 import mock
 from oslo_config import cfg
+from oslo_context import context
 from oslo_serialization import jsonutils
 from oslotest import base as test_base
 import six
@@ -646,6 +647,89 @@ class EnforcerTest(base.PolicyBaseTestCase):
                           self.enforcer.authorize, 'test', {},
                           {'roles': ['test']})
 
+    def test_enforcer_accepts_context_objects(self):
+        rule = policy.RuleDefault(name='fake_rule', check_str='role:test')
+        self.enforcer.register_default(rule)
+
+        request_context = context.RequestContext()
+        target_dict = {}
+        self.enforcer.enforce('fake_rule', target_dict, request_context)
+
+    def test_enforcer_accepts_subclassed_context_objects(self):
+        rule = policy.RuleDefault(name='fake_rule', check_str='role:test')
+        self.enforcer.register_default(rule)
+
+        class SpecializedContext(context.RequestContext):
+            pass
+
+        request_context = SpecializedContext()
+        target_dict = {}
+        self.enforcer.enforce('fake_rule', target_dict, request_context)
+
+    def test_enforcer_rejects_non_context_objects(self):
+        rule = policy.RuleDefault(name='fake_rule', check_str='role:test')
+        self.enforcer.register_default(rule)
+
+        class InvalidContext(object):
+            pass
+
+        request_context = InvalidContext()
+        target_dict = {}
+        self.assertRaises(
+            policy.InvalidContextObject, self.enforcer.enforce, 'fake_rule',
+            target_dict, request_context
+        )
+
+    @mock.patch.object(policy.Enforcer, '_map_context_attributes_into_creds')
+    def test_enforcer_call_map_context_attributes(self, map_mock):
+        rule = policy.RuleDefault(name='fake_rule', check_str='role:test')
+        self.enforcer.register_default(rule)
+
+        request_context = context.RequestContext()
+        target_dict = {}
+        self.enforcer.enforce('fake_rule', target_dict, request_context)
+        map_mock.assert_called_once_with(request_context)
+
+    def test_enforcer_consolidates_context_attributes_with_creds(self):
+        request_context = context.RequestContext()
+        expected_creds = request_context.to_policy_values()
+
+        creds = self.enforcer._map_context_attributes_into_creds(
+            request_context
+        )
+
+        # We don't use self.assertDictEqual here because to_policy_values
+        # actaully returns a non-dict object that just behaves like a
+        # dictionary, but does some special handling when people access
+        # deprecated policy values.
+        for k, v in expected_creds.items():
+            self.assertEqual(expected_creds[k], creds[k])
+
+    def test_map_context_attributes_populated_system(self):
+        request_context = context.RequestContext(system_scope='all')
+        expected_creds = request_context.to_policy_values()
+        expected_creds['system'] = 'all'
+
+        creds = self.enforcer._map_context_attributes_into_creds(
+            request_context
+        )
+
+        # We don't use self.assertDictEqual here because to_policy_values
+        # actaully returns a non-dict object that just behaves like a
+        # dictionary, but does some special handling when people access
+        # deprecated policy values.
+        for k, v in expected_creds.items():
+            self.assertEqual(expected_creds[k], creds[k])
+
+    def test_enforcer_accepts_policy_values_from_context(self):
+        rule = policy.RuleDefault(name='fake_rule', check_str='role:test')
+        self.enforcer.register_default(rule)
+
+        request_context = context.RequestContext()
+        policy_values = request_context.to_policy_values()
+        target_dict = {}
+        self.enforcer.enforce('fake_rule', target_dict, policy_values)
+
 
 class EnforcerNoPolicyFileTest(base.PolicyBaseTestCase):
     def setUp(self):

releasenotes/notes/bug-1779172-c1323c0f647bc44c.yaml

@@ -0,0 +1,19 @@
+---
+features:
+  - |
+    [`bug 1779172 <https://bugs.launchpad.net/keystone/+bug/1779172>`_]
+    The ``enforce()`` method now supports the ability to parse ``oslo.context``
+    objects if passed into ``enforce()`` as ``creds``. This provides more
+    consistent policy enforcement for service developers by ensuring the
+    attributes provided in policy enforcement are standardized. In this case
+    they are being standardized through the
+    ``oslo_context.context.RequestContext.to_policy_values()`` method.
+fixes:
+  - |
+    [`bug 1779172 <https://bugs.launchpad.net/keystone/+bug/1779172>`_]
+    The ``enforce()`` method now supports the ability to parse ``oslo.context``
+    objects if passed into ``enforce()`` as ``creds``. This provides more
+    consistent policy enforcement for service developers by ensuring the
+    attributes provided in policy enforcement are standardized. In this case
+    they are being standardized through the
+    ``oslo_context.context.RequestContext.to_policy_values()`` method.

requirements.txt

@@ -4,6 +4,7 @@
 
 requests>=2.14.2 # Apache-2.0
 oslo.config>=5.2.0 # Apache-2.0
+oslo.context>=2.21.0 # Apache-2.0
 oslo.i18n>=3.15.3 # Apache-2.0
 oslo.serialization!=2.19.1,>=2.18.0 # Apache-2.0
 PyYAML>=3.12 # MIT

test-requirements.txt

@@ -5,6 +5,7 @@ hacking!=0.13.0,<0.14,>=0.12.0 # Apache-2.0
 oslotest>=3.2.0 # Apache-2.0
 requests-mock>=1.1.0 # Apache-2.0
 stestr>=2.0.0 # Apache-2.0
+oslo.context>=2.21.0 # Apache-2.0
 
 # computes code coverage percentages
 coverage!=4.4,>=4.0 # Apache-2.0