Work on copy of registered rule instead of original object3.6.2

When service register their policy rule oslo policy does not
copy the rule and instead work on the original object.

- https://github.com/openstack/oslo.policy/blob/bd9d47aa36ad6f2f4746f09a267d7ce809a820f4/oslo_policy/policy.py#L1104

policy enforcer modify the default rules in
_handle_deprecated_rule().

- https://github.com/openstack/oslo.policy/blob/bd9d47aa36ad6f2f4746f09a267d7ce809a820f4/oslo_policy/policy.py#L767-L774

In any case, oslo policy should make copy of the registered
rules.

Another thing it fix is setting of flag
RuleDefault._deprecated_rule_handled.
Flag _deprecated_rule_handled is set to True when
_handle_deprecated_rule() is called irrespective of it
actually handle the deprecated rule and add it in OR checks.
We should set this flag when acutally deprecated rule is
handled so that if any condition change like config flag or
file rules we correctly handle deprecated rules.

Closes-Bug: #1914095
Closes-Bug: #1914592
Story: 2008556
Task: 41687
Change-Id: I154213dabd4d9eef760f0a4c9a852d504638ca8d

2 files changed, 35 insertions, 9 deletions

diff --git a/oslo_policy/policy.py b/oslo_policy/policy.py
index 046bec0..903f65b 100644
--- a/oslo_policy/policy.py
+++ b/oslo_policy/policy.py
@@ -747,6 +747,7 @@ class Enforcer(object):
                     self.rules[default.name] = self.file_rules[
                         deprecated_rule.name
                     ].check
+                    default._deprecated_rule_handled = True
 
         # In this case, the default check string is changing. We need to let
         # operators know that this is going to change. If they don't want to
@@ -767,12 +768,11 @@ class Enforcer(object):
             default.check = OrCheck([_parser.parse_rule(cs) for cs in
                                      [default.check_str,
                                       deprecated_rule.check_str]])
+            default._deprecated_rule_handled = True
             if not (self.suppress_deprecation_warnings
                     or self.suppress_default_change_warnings):
                 warnings.warn(deprecated_msg)
 
-        default._deprecated_rule_handled = True
-
     def _undefined_check(self, check):
         '''Check if a RuleCheck references an undefined rule.'''
         if isinstance(check, RuleCheck):
@@ -1101,7 +1101,11 @@ class Enforcer(object):
         if default.name in self.registered_rules:
             raise DuplicatePolicyError(default.name)
 
-        self.registered_rules[default.name] = default
+        # NOTE Always make copy of registered rule because policy engine
+        # update these rules in many places (one example is
+        # self._handle_deprecated_rule() ). This will avoid any conflict
+        # in rule object values when running tests in parallel.
+        self.registered_rules[default.name] = copy.deepcopy(default)
 
     def register_defaults(self, defaults):
         """Registers a list of RuleDefaults.
diff --git a/oslo_policy/tests/test_policy.py b/oslo_policy/tests/test_policy.py
index 945b809..513b43d 100644
--- a/oslo_policy/tests/test_policy.py
+++ b/oslo_policy/tests/test_policy.py
@@ -784,6 +784,25 @@ class EnforcerTest(base.PolicyBaseTestCase):
                           policy.RuleDefault(name='owner',
                                              check_str='role:owner'))
 
+    def test_enforcer_does_not_modify_original_registered_rule(self):
+        rule_original = policy.RuleDefault(
+            name='test',
+            check_str='role:owner',)
+        rule_original._deprecated_rule_handled = False
+        self.enforcer.register_default(rule_original)
+        self.enforcer.registered_rules['test'].check_str = 'role:admin'
+        self.enforcer.registered_rules['test'].check = 'role:admin'
+        self.enforcer.registered_rules['test']._deprecated_rule_handled = True
+        self.assertEqual(self.enforcer.registered_rules['test'].check_str,
+                         'role:admin')
+        self.assertEqual(self.enforcer.registered_rules['test'].check,
+                         'role:admin')
+        self.assertTrue(
+            self.enforcer.registered_rules['test']._deprecated_rule_handled)
+        self.assertEqual(rule_original.check_str, 'role:owner')
+        self.assertEqual(rule_original.check.__str__(), 'role:owner')
+        self.assertFalse(rule_original._deprecated_rule_handled)
+
     def test_non_reversible_check(self):
         self.create_config_file('policy.json',
                                 jsonutils.dumps(
@@ -1766,21 +1785,24 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
 
         enforcer = policy.Enforcer(self.conf)
         enforcer.register_defaults([rule])
-
+        registered_default_rule = enforcer.registered_rules['foo:create_bar']
         # Check that rule deprecation handling hasn't been done, yet
-        self.assertFalse(rule._deprecated_rule_handled)
+        self.assertFalse(registered_default_rule._deprecated_rule_handled)
 
         # Loading the rules will modify the rule check string to logically OR
         # the new value with the deprecated value
         enforcer.load_rules()
-        self.assertTrue(rule._deprecated_rule_handled)
+        self.assertTrue(registered_default_rule._deprecated_rule_handled)
 
         # Make sure the original value is used instead of instantiating new
         # OrCheck objects whenever we perform subsequent reloads
-        expected_check = rule.check
+        expected_check = policy.OrCheck([_parser.parse_rule(cs) for cs in
+                                         [rule.check_str,
+                                          deprecated_rule.check_str]])
         enforcer.load_rules()
-        self.assertTrue(rule.check is expected_check)
-        self.assertTrue(rule._deprecated_rule_handled)
+        self.assertEqual(registered_default_rule.check.__str__(),
+                         expected_check.__str__())
+        self.assertTrue(registered_default_rule._deprecated_rule_handled)
 
 
 class DocumentedRuleDefaultTestCase(base.PolicyBaseTestCase):