diff options
author | Zane Bitter <zbitter@redhat.com> | 2019-12-12 11:03:30 -0500 |
---|---|---|
committer | Zane Bitter <zbitter@redhat.com> | 2019-12-13 02:24:54 +0000 |
commit | b93f51c1aa85ca4735118668d1f890ca0fbe941d (patch) | |
tree | cc267e04e8e3df880c49268de9ee3734a2eeac3b | |
parent | 98d32e085ae904c8d454916694be6ee154a9dea3 (diff) | |
download | oslo-policy-b93f51c1aa85ca4735118668d1f890ca0fbe941d.tar.gz |
Don't use string processing to combine deprecated rules2.4.1
Constructing a policy string by sticking ' or ' between the new and
deprecated check_str values is error-prone. Construct the policy
programmatically instead by parsing the check_str values separately and
combining them into an OrCheck.
Change-Id: Ia2ee05aa08326c6daa214a7b1156baa6efe43dc0
Closes-Bug: #1856207
-rw-r--r-- | oslo_policy/policy.py | 7 | ||||
-rw-r--r-- | oslo_policy/tests/test_policy.py | 64 |
2 files changed, 67 insertions, 4 deletions
diff --git a/oslo_policy/policy.py b/oslo_policy/policy.py index 3429ea4..61a3597 100644 --- a/oslo_policy/policy.py +++ b/oslo_policy/policy.py @@ -699,10 +699,9 @@ class Enforcer(object): if (deprecated_rule.check_str != default.check_str and default.name not in self.file_rules): - default.check = _parser.parse_rule( - default.check_str + ' or ' + - deprecated_rule.check_str - ) + default.check = OrCheck([_parser.parse_rule(cs) for cs in + [default.check_str, + deprecated_rule.check_str]]) if not self.suppress_deprecation_warnings: warnings.warn(deprecated_msg) diff --git a/oslo_policy/tests/test_policy.py b/oslo_policy/tests/test_policy.py index 88d363a..bc41054 100644 --- a/oslo_policy/tests/test_policy.py +++ b/oslo_policy/tests/test_policy.py @@ -1190,6 +1190,70 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase): enforcer.load_rules() mock_warn.assert_called_once_with(expected_msg) + self.assertTrue( + enforcer.enforce('foo:create_bar', {}, {'roles': ['bang']}) + ) + self.assertTrue( + enforcer.enforce('foo:create_bar', {}, {'roles': ['fizz']}) + ) + self.assertFalse( + enforcer.enforce('foo:create_bar', {}, {'roles': ['baz']}) + ) + + def test_deprecate_an_empty_policy_check_string(self): + deprecated_rule = policy.DeprecatedRule( + name='foo:create_bar', + check_str='' + ) + + rule_list = [policy.DocumentedRuleDefault( + name='foo:create_bar', + check_str='role:bang', + description='Create a bar.', + operations=[{'path': '/v1/bars', 'method': 'POST'}], + deprecated_rule=deprecated_rule, + deprecated_reason='because of reasons', + deprecated_since='N' + )] + enforcer = policy.Enforcer(self.conf) + enforcer.register_defaults(rule_list) + + with mock.patch('warnings.warn') as mock_warn: + enforcer.load_rules() + mock_warn.assert_called_once() + + enforcer.enforce('foo:create_bar', {}, {'roles': ['bang']}, + do_raise=True) + enforcer.enforce('foo:create_bar', {}, {'roles': ['fizz']}, + do_raise=True) + + def test_deprecate_replace_with_empty_policy_check_string(self): + deprecated_rule = policy.DeprecatedRule( + name='foo:create_bar', + check_str='role:fizz' + ) + + rule_list = [policy.DocumentedRuleDefault( + name='foo:create_bar', + check_str='', + description='Create a bar.', + operations=[{'path': '/v1/bars', 'method': 'POST'}], + deprecated_rule=deprecated_rule, + deprecated_reason='because of reasons', + deprecated_since='N' + )] + enforcer = policy.Enforcer(self.conf) + enforcer.register_defaults(rule_list) + + with mock.patch('warnings.warn') as mock_warn: + enforcer.load_rules() + mock_warn.assert_called_once() + + enforcer.enforce('foo:create_bar', {}, {'roles': ['fizz']}, + do_raise=True) + enforcer.enforce('foo:create_bar', {}, {'roles': ['bang']}, + do_raise=True) + def test_deprecate_a_policy_name(self): deprecated_rule = policy.DeprecatedRule( name='foo:bar', |