openstack/oslo.policy
Code Issues Proposed changes

Don't use string processing to combine deprecated rules

Browse Source 
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
This commit is contained in:
Zane Bitter 2019-12-12 11:03:30 -05:00
parent 98d32e085a
commit b93f51c1aa
2 changed files with 67 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
oslo_policy/policy.py
View File

@ -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)

64
oslo_policy/tests/test_policy.py
View File

@ -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',