Reload files in policy_dirs on primary file change

It was determined that rules from policy files located in the directory
specified in the policy_dirs option (/etc/<config_dir>/policy.d by
default) are not re-applied after the rules from the primary policy file
is re-applied due to a change.

This change introduces additional behavior to make sure the rules from
policy_dirs are reapplied if there is a change to the primary policy
file.

Change-Id: I8a6f8e971d881365c41ea409966723319d5b239a
Closes-Bug: #1880959
Related-Bug: #1880847
(cherry picked from commit 75677a3110)
(cherry picked from commit 5904564bf1)
This commit is contained in:
Dmitrii Shcherbakov 2020-05-27 17:06:25 +03:00
parent 20331c3597
commit a49f31d3fc
3 changed files with 85 additions and 7 deletions

View File

@ -550,6 +550,8 @@ class Enforcer(object):
if force_reload: if force_reload:
self.use_conf = force_reload self.use_conf = force_reload
policy_file_rules_changed = False
if self.use_conf: if self.use_conf:
if not self.policy_path: if not self.policy_path:
try: try:
@ -561,18 +563,30 @@ class Enforcer(object):
self._informed_no_policy_file = True self._informed_no_policy_file = True
if self.policy_path: if self.policy_path:
self._load_policy_file(self.policy_path, force_reload, # If the policy file rules have changed any policy.d rules
overwrite=self.overwrite) # also need to be reapplied on top of that change.
policy_file_rules_changed = self._load_policy_file(
self.policy_path,
force_reload,
overwrite=self.overwrite
)
force_reload_policy_dir = force_reload
if policy_file_rules_changed:
force_reload_policy_dir = True
for path in self.conf.oslo_policy.policy_dirs: for path in self.conf.oslo_policy.policy_dirs:
try: try:
path = self._get_policy_path(path) path = self._get_policy_path(path)
except cfg.ConfigFilesNotFoundError: except cfg.ConfigFilesNotFoundError:
continue continue
if (force_reload or self._is_directory_updated( if (self._is_directory_updated(self._policy_dir_mtimes, path)
self._policy_dir_mtimes, path)): or force_reload_policy_dir):
self._walk_through_policy_directory(path, self._walk_through_policy_directory(
self._load_policy_file, path,
force_reload, False) self._load_policy_file,
force_reload_policy_dir, False
)
for default in self.registered_rules.values(): for default in self.registered_rules.values():
if default.deprecated_for_removal: if default.deprecated_for_removal:
@ -755,6 +769,8 @@ class Enforcer(object):
# is in the cache # is in the cache
mtime = 0 mtime = 0
if os.path.exists(path): if os.path.exists(path):
if not os.path.isdir(path):
raise ValueError('{} is not a directory'.format(path))
# Make a list of all the files # Make a list of all the files
files = [path] + [os.path.join(path, file) for file in files = [path] + [os.path.join(path, file) for file in
os.listdir(path)] os.listdir(path)]
@ -793,14 +809,25 @@ class Enforcer(object):
self.file_rules[name] = RuleDefault(name, check_str) self.file_rules[name] = RuleDefault(name, check_str)
def _load_policy_file(self, path, force_reload, overwrite=True): def _load_policy_file(self, path, force_reload, overwrite=True):
"""Load policy rules from the specified policy file.
:param str path: A path of the policy file to load rules from.
:param bool force_reload: Forcefully reload the policy file content.
:param bool overwrite: Replace policy rules instead of updating them.
:return: A bool indicating whether rules have been changed or not.
:rtype: bool
"""
rules_changed = False
reloaded, data = _cache_handler.read_cached_file( reloaded, data = _cache_handler.read_cached_file(
self._file_cache, path, force_reload=force_reload) self._file_cache, path, force_reload=force_reload)
if reloaded or not self.rules: if reloaded or not self.rules:
rules = Rules.load(data, self.default_rule) rules = Rules.load(data, self.default_rule)
self.set_rules(rules, overwrite=overwrite, use_conf=True) self.set_rules(rules, overwrite=overwrite, use_conf=True)
rules_changed = True
self._record_file_rules(data, overwrite) self._record_file_rules(data, overwrite)
self._loaded_files.append(path) self._loaded_files.append(path)
LOG.debug('Reloaded policy file: %(path)s', {'path': path}) LOG.debug('Reloaded policy file: %(path)s', {'path': path})
return rules_changed
def _get_policy_path(self, path): def _get_policy_path(self, path):
"""Locate the policy YAML/JSON data file/path. """Locate the policy YAML/JSON data file/path.

View File

@ -296,6 +296,48 @@ class EnforcerTest(base.PolicyBaseTestCase):
os.path.join('policy.d', 'b.conf'), os.path.join('policy.d', 'b.conf'),
]) ])
def test_load_directory_after_file_update(self):
self.create_config_file(
os.path.join('policy.d', 'a.conf'), POLICY_A_CONTENTS)
self.enforcer.load_rules(True)
self.assertIsNotNone(self.enforcer.rules)
loaded_rules = jsonutils.loads(str(self.enforcer.rules))
self.assertEqual('role:fakeA', loaded_rules['default'])
self.assertEqual('is_admin:True', loaded_rules['admin'])
self.check_loaded_files([
'policy.json',
os.path.join('policy.d', 'a.conf'),
])
new_policy_json_contents = jsonutils.dumps({
"default": "rule:admin",
"admin": "is_admin:True",
"foo": "rule:bar",
})
# Modify the policy.json file and then validate that the rules
# from the policy directory are re-applied on top of the
# new rules from the file.
self.create_config_file('policy.json', new_policy_json_contents)
policy_file_path = self.get_config_file_fullname('policy.json')
# Force the mtime change since the unit test may write to this file
# too fast for mtime to actually change.
stinfo = os.stat(policy_file_path)
os.utime(policy_file_path, (stinfo.st_atime + 42,
stinfo.st_mtime + 42))
self.enforcer.load_rules()
self.assertIsNotNone(self.enforcer.rules)
loaded_rules = jsonutils.loads(str(self.enforcer.rules))
self.assertEqual('role:fakeA', loaded_rules['default'])
self.assertEqual('is_admin:True', loaded_rules['admin'])
self.assertEqual('rule:bar', loaded_rules['foo'])
self.check_loaded_files([
'policy.json',
os.path.join('policy.d', 'a.conf'),
'policy.json',
os.path.join('policy.d', 'a.conf'),
])
def test_load_directory_opts_registered(self): def test_load_directory_opts_registered(self):
self._test_scenario_with_opts_registered(self.test_load_directory) self._test_scenario_with_opts_registered(self.test_load_directory)
@ -421,6 +463,7 @@ class EnforcerTest(base.PolicyBaseTestCase):
[os.path.join('policy.d', 'a.conf')], [os.path.join('policy.d', 'a.conf')],
group='oslo_policy') group='oslo_policy')
self.assertRaises(ValueError, self.enforcer.load_rules, True) self.assertRaises(ValueError, self.enforcer.load_rules, True)
self.assertRaises(ValueError, self.enforcer.load_rules, False)
@mock.patch('oslo_policy.policy.Enforcer.check_rules') @mock.patch('oslo_policy.policy.Enforcer.check_rules')
def test_load_rules_twice(self, mock_check_rules): def test_load_rules_twice(self, mock_check_rules):

View File

@ -0,0 +1,8 @@
---
fixes:
- |
[`bug 1880959 <https://bugs.launchpad.net/keystone/+bug/1880959>`_]
The behavior of policy file reloading from policy directories was fixed.
Previously the rules from policy files located in the directories
specified in the ``policy_dirs`` option were not reapplied after the rules
from the primary policy file have been reapplied due to a change.