Merge "Map system_scope in creds dictionary"

oslo_policy/policy.py

@@ -982,6 +982,17 @@ class Enforcer(object):
             )
             raise InvalidContextObject(msg)
 
+        # 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 creds.get('system_scope'):
+            creds['system'] = creds.get('system_scope')
+
         if LOG.isEnabledFor(logging.DEBUG):
             try:
                 creds_dict = strutils.mask_dict_password(creds)
@@ -1088,17 +1099,6 @@ class Enforcer(object):
         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):

oslo_policy/tests/test_policy.py

@@ -881,23 +881,6 @@ class EnforcerTest(base.PolicyBaseTestCase):
         for k, v in expected_creds.items():
             self.assertEqual(expected_creds[k], creds[k])
 
-    @mock.patch('warnings.warn', new=mock.Mock())
-    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)
@@ -918,6 +901,20 @@ class EnforcerTest(base.PolicyBaseTestCase):
         target_dict = {}
         self.enforcer.enforce('fake_rule', target_dict, ctx)
 
+    def test_enforcer_understands_system_scope_creds_dict(self):
+        self.conf.set_override('enforce_scope', True, group='oslo_policy')
+        rule = policy.RuleDefault(
+            name='fake_rule', check_str='role:test', scope_types=['system']
+        )
+        self.enforcer.register_default(rule)
+
+        ctx = context.RequestContext()
+        creds = ctx.to_dict()
+        creds['system_scope'] = 'all'
+
+        target_dict = {}
+        self.enforcer.enforce('fake_rule', target_dict, creds)
+
     def test_enforcer_raises_invalid_scope_with_system_scope_type(self):
         self.conf.set_override('enforce_scope', True, group='oslo_policy')
         rule = policy.RuleDefault(

releasenotes/notes/Fix-map-system-scope-for-creds-dict-e4cbec2f7495f22e.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Fixes the mapping of 'system_scope' to 'system' when enforce is called
+    with a 'creds' dictionary instead of a RequestContext.