From 9774108cf91408e9cb825b317f48a3a3f856e161 Mon Sep 17 00:00:00 2001
From: Michael Johnson <johnsomor@gmail.com>
Date: Thu, 15 Jul 2021 21:42:54 +0000
Subject: Map system_scope in creds dictionary

An earlier patch[1] added a mapping for context 'system_scope'
to 'system' when enforce was called with a RequestContext
object. However, enforce can also be called with a creds dictionary
that may contain the context 'system_scope' element. When this
occured, 'system_scope' was not mapped to 'system' and the enforce
would fail with an InvalidScope exception.
This patch moves the 'system_scope' mapping from only occuring
with RequestContext objects to also map it when a creds dictonary
is passed to enforce.

[1] https://review.opendev.org/c/openstack/oslo.policy/+/578995

Change-Id: I83a22c3f825bad0c88018118f8630a20a445965e
---
 oslo_policy/policy.py            | 22 +++++++++++-----------
 oslo_policy/tests/test_policy.py | 31 ++++++++++++++-----------------
 2 files changed, 25 insertions(+), 28 deletions(-)

(limited to 'oslo_policy')

diff --git a/oslo_policy/policy.py b/oslo_policy/policy.py
index 4491eca..53815d6 100644
--- a/oslo_policy/policy.py
+++ b/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):
diff --git a/oslo_policy/tests/test_policy.py b/oslo_policy/tests/test_policy.py
index d5c86c3..d2c313e 100644
--- a/oslo_policy/tests/test_policy.py
+++ b/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(
-- 
cgit v1.2.1