Rename rbac_policy_parser to policy_authority
This change is a follow-up to commit
I8ba89ab5e134b15e97ac20a7aacbfd70896e192f
which introduced an abstract class from which (previously)
rbac_policy_parser and requirements authority inherit, providing
rbac_rule_validation with 2 ways of validating RBAC.
For the sake of naming consistency, rbac_policy_parser is renamed
to policy_authority. This naming scheme is better because
"policy parser" is implementation-specific and doesn't convey
what the file (and class name) do from a high-level perspective.
Because this file is only used internally to Patrole, it can be
changed without backward-compatibility concerns.
This commit also includes documentation for the policy authority
module and the rbac_rule_validation module.
Change-Id: Ie09fc2d884f9211244b062fdd5fe018970c2bb2d
diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst
deleted file mode 100644
index 243a9c0..0000000
--- a/CONTRIBUTING.rst
+++ /dev/null
@@ -1,17 +0,0 @@
-If you would like to contribute to the development of OpenStack, you must
-follow the steps in this page:
-
- https://docs.openstack.org/infra/manual/developers.html
-
-If you already have a good understanding of how the system works and your
-OpenStack accounts are set up, you can skip to the development workflow
-section of this documentation to learn how changes to OpenStack should be
-submitted for review via the Gerrit tool:
-
- https://docs.openstack.org/infra/manual/developers.html#development-workflow
-
-Pull requests submitted through GitHub will be ignored.
-
-Bugs should be filed on Launchpad, not GitHub:
-
- https://bugs.launchpad.net/patrole
diff --git a/doc/source/HACKING.rst b/doc/source/HACKING.rst
new file mode 100644
index 0000000..1847447
--- /dev/null
+++ b/doc/source/HACKING.rst
@@ -0,0 +1,4 @@
+=======
+Hacking
+=======
+.. include:: ../../HACKING.rst
diff --git a/doc/source/contributing.rst b/doc/source/contributing.rst
deleted file mode 100644
index 1728a61..0000000
--- a/doc/source/contributing.rst
+++ /dev/null
@@ -1,4 +0,0 @@
-============
-Contributing
-============
-.. include:: ../../CONTRIBUTING.rst
diff --git a/doc/source/index.rst b/doc/source/index.rst
index f58ee7f..e2cc0bd 100644
--- a/doc/source/index.rst
+++ b/doc/source/index.rst
@@ -20,9 +20,17 @@
=================
.. toctree::
+ :maxdepth: 1
+
+ HACKING
+
+Framework
+---------
+
+.. toctree::
:maxdepth: 2
- contributing
+ rbac_validation
Indices and tables
==================
diff --git a/doc/source/rbac_validation.rst b/doc/source/rbac_validation.rst
new file mode 100644
index 0000000..ccaf3c8
--- /dev/null
+++ b/doc/source/rbac_validation.rst
@@ -0,0 +1,64 @@
+.. _rbac-validation:
+
+RBAC Testing Validation
+=======================
+
+--------
+Overview
+--------
+
+RBAC Testing Validation is broken up into 3 stages:
+
+ 1. "Expected" stage. Determine whether the test should be able to succeed
+ or fail based on the test role defined by ``[patrole] rbac_test_role``)
+ and the policy action that the test enforces.
+ 2. "Actual" stage. Run the test by calling the API endpoint that enforces
+ the expected policy action using the test role.
+ 3. Comparing the outputs from both stages for consistency. A "consistent"
+ result is treated as a pass and an "inconsistent" result is treated
+ as a failure. "Consistent" (or successful) cases include:
+
+ * Expected result is ``True`` and the test passes.
+ * Expected result is ``False`` and the test fails.
+
+ "Inconsistent" (or failing) cases include:
+
+ * Expected result is ``False`` and the test passes. This results in an
+ ``RbacOverPermission`` exception getting thrown.
+ * Expected result is ``True`` and the test fails. This results in a
+ ``Forbidden`` exception getting thrown.
+
+ For example, a 200 from the API call and a ``True`` result from
+ ``oslo.policy`` or a 403 from the API call and a ``False`` result from
+ ``oslo.policy`` are successful results.
+
+-------------------------------
+The RBAC Rule Validation Module
+-------------------------------
+
+High-level module that implements decorator inside which the "Expected" stage
+is initiated.
+
+.. automodule:: patrole_tempest_plugin.rbac_rule_validation
+ :members:
+
+---------------------------
+The Policy Authority Module
+---------------------------
+
+Using the Policy Authority Module, policy verification is performed by:
+
+1. Pooling together the default `in-code` policy rules.
+2. Overriding the defaults with custom policy rules located in a policy.json,
+ if the policy file exists and the custom policy definition is explicitly
+ defined therein.
+3. Confirming that the policy action -- for example, "list_users" -- exists.
+ (``oslo.policy`` otherwise claims that role "foo" is allowed to
+ perform policy action "bar", for example, because it defers to the
+ "default" policy rule and oftentimes the default can be "anyone allowed").
+4. Performing a call with all necessary data to ``oslo.policy`` and returning
+ the expected result back to ``rbac_rule_validation`` decorator.
+
+.. automodule:: patrole_tempest_plugin.policy_authority
+ :members:
+ :special-members:
diff --git a/patrole_tempest_plugin/rbac_policy_parser.py b/patrole_tempest_plugin/policy_authority.py
similarity index 99%
rename from patrole_tempest_plugin/rbac_policy_parser.py
rename to patrole_tempest_plugin/policy_authority.py
index aff4e66..af227c4 100644
--- a/patrole_tempest_plugin/rbac_policy_parser.py
+++ b/patrole_tempest_plugin/policy_authority.py
@@ -31,7 +31,7 @@
LOG = logging.getLogger(__name__)
-class RbacPolicyParser(RbacAuthority):
+class PolicyAuthority(RbacAuthority):
"""A class for parsing policy rules into lists of allowed roles.
RBAC testing requires that each rule in a policy file be broken up into
diff --git a/patrole_tempest_plugin/rbac_rule_validation.py b/patrole_tempest_plugin/rbac_rule_validation.py
index 69a98e3..69274b3 100644
--- a/patrole_tempest_plugin/rbac_rule_validation.py
+++ b/patrole_tempest_plugin/rbac_rule_validation.py
@@ -23,8 +23,8 @@
from tempest.lib import exceptions
from tempest import test
+from patrole_tempest_plugin import policy_authority
from patrole_tempest_plugin import rbac_exceptions
-from patrole_tempest_plugin import rbac_policy_parser
from patrole_tempest_plugin import rbac_utils
from patrole_tempest_plugin import requirements_authority
@@ -237,14 +237,14 @@
try:
role = CONF.patrole.rbac_test_role
- # Test RBAC against custom requirements. Otherwise use oslo.policy
+ # Test RBAC against custom requirements. Otherwise use oslo.policy.
if CONF.patrole.test_custom_requirements:
authority = requirements_authority.RequirementsAuthority(
CONF.patrole.custom_requirements_file, service)
else:
formatted_target_data = _format_extra_target_data(
test_obj, extra_target_data)
- authority = rbac_policy_parser.RbacPolicyParser(
+ authority = policy_authority.PolicyAuthority(
project_id, user_id, service,
extra_target_data=formatted_target_data)
is_allowed = authority.allowed(rule, role)
diff --git a/patrole_tempest_plugin/tests/unit/test_rbac_policy_parser.py b/patrole_tempest_plugin/tests/unit/test_policy_authority.py
similarity index 75%
rename from patrole_tempest_plugin/tests/unit/test_rbac_policy_parser.py
rename to patrole_tempest_plugin/tests/unit/test_policy_authority.py
index 5fa26df..2a8da9d 100644
--- a/patrole_tempest_plugin/tests/unit/test_rbac_policy_parser.py
+++ b/patrole_tempest_plugin/tests/unit/test_policy_authority.py
@@ -20,13 +20,14 @@
from tempest import config
from tempest.tests import base
+from patrole_tempest_plugin import policy_authority
from patrole_tempest_plugin import rbac_exceptions
-from patrole_tempest_plugin import rbac_policy_parser
+from patrole_tempest_plugin.tests.unit import fixtures
CONF = config.CONF
-class RbacPolicyTest(base.TestCase):
+class PolicyAuthorityTest(base.TestCase):
services = {
'services': [
@@ -39,9 +40,9 @@
}
def setUp(self):
- super(RbacPolicyTest, self).setUp()
- self.patchobject(rbac_policy_parser, 'credentials')
- m_creds = self.patchobject(rbac_policy_parser, 'clients')
+ super(PolicyAuthorityTest, self).setUp()
+ self.patchobject(policy_authority, 'credentials')
+ m_creds = self.patchobject(policy_authority, 'clients')
m_creds.Manager().identity_services_client.list_services.\
return_value = self.services
m_creds.Manager().identity_services_v3_client.list_services.\
@@ -63,36 +64,29 @@
self.conf_policy_path = os.path.join(
current_directory, 'resources', '%s.json')
- CONF.set_override(
- 'custom_policy_files', [self.conf_policy_path], group='patrole')
- self.addCleanup(CONF.clear_override, 'custom_policy_files',
- group='patrole')
+ self.useFixture(fixtures.ConfPatcher(
+ custom_policy_files=[self.conf_policy_path], group='patrole'))
+ self.useFixture(fixtures.ConfPatcher(
+ api_v3=True, api_v2=False, group='identity-feature-enabled'))
# Guarantee a blank slate for each test.
for attr in ('available_services', 'policy_files'):
- if attr in dir(rbac_policy_parser.RbacPolicyParser):
- delattr(rbac_policy_parser.RbacPolicyParser, attr)
-
- # TODO(fm577c): Use fixture for setting/clearing CONF.
- CONF.set_override('api_v3', True, group='identity-feature-enabled')
- self.addCleanup(CONF.clear_override, 'api_v2',
- group='identity-feature-enabled')
- self.addCleanup(CONF.clear_override, 'api_v3',
- group='identity-feature-enabled')
+ if attr in dir(policy_authority.PolicyAuthority):
+ delattr(policy_authority.PolicyAuthority, attr)
def _get_fake_policy_rule(self, name, rule):
fake_rule = mock.Mock(check=rule, __name__='foo')
fake_rule.name = name
return fake_rule
- @mock.patch.object(rbac_policy_parser, 'LOG', autospec=True)
+ @mock.patch.object(policy_authority, 'LOG', autospec=True)
def test_custom_policy(self, m_log):
default_roles = ['zero', 'one', 'two', 'three', 'four',
'five', 'six', 'seven', 'eight', 'nine']
test_tenant_id = mock.sentinel.tenant_id
test_user_id = mock.sentinel.user_id
- parser = rbac_policy_parser.RbacPolicyParser(
+ authority = policy_authority.PolicyAuthority(
test_tenant_id, test_user_id, "custom_rbac_policy")
expected = {
@@ -107,14 +101,14 @@
for rule, role_list in expected.items():
for role in role_list:
- self.assertTrue(parser.allowed(rule, role))
+ self.assertTrue(authority.allowed(rule, role))
for role in set(default_roles) - set(role_list):
- self.assertFalse(parser.allowed(rule, role))
+ self.assertFalse(authority.allowed(rule, role))
def test_admin_policy_file_with_admin_role(self):
test_tenant_id = mock.sentinel.tenant_id
test_user_id = mock.sentinel.user_id
- parser = rbac_policy_parser.RbacPolicyParser(
+ authority = policy_authority.PolicyAuthority(
test_tenant_id, test_user_id, "admin_rbac_policy")
role = 'admin'
@@ -124,17 +118,17 @@
disallowed_rules = ['non_admin_rule']
for rule in allowed_rules:
- allowed = parser.allowed(rule, role)
+ allowed = authority.allowed(rule, role)
self.assertTrue(allowed)
for rule in disallowed_rules:
- allowed = parser.allowed(rule, role)
+ allowed = authority.allowed(rule, role)
self.assertFalse(allowed)
def test_admin_policy_file_with_member_role(self):
test_tenant_id = mock.sentinel.tenant_id
test_user_id = mock.sentinel.user_id
- parser = rbac_policy_parser.RbacPolicyParser(
+ authority = policy_authority.PolicyAuthority(
test_tenant_id, test_user_id, "admin_rbac_policy")
role = 'Member'
@@ -145,17 +139,17 @@
'admin_rule', 'is_admin_rule', 'alt_admin_rule']
for rule in allowed_rules:
- allowed = parser.allowed(rule, role)
+ allowed = authority.allowed(rule, role)
self.assertTrue(allowed)
for rule in disallowed_rules:
- allowed = parser.allowed(rule, role)
+ allowed = authority.allowed(rule, role)
self.assertFalse(allowed)
def test_alt_admin_policy_file_with_context_is_admin(self):
test_tenant_id = mock.sentinel.tenant_id
test_user_id = mock.sentinel.user_id
- parser = rbac_policy_parser.RbacPolicyParser(
+ authority = policy_authority.PolicyAuthority(
test_tenant_id, test_user_id, "alt_admin_rbac_policy")
role = 'fake_admin'
@@ -163,11 +157,11 @@
disallowed_rules = ['admin_rule']
for rule in allowed_rules:
- allowed = parser.allowed(rule, role)
+ allowed = authority.allowed(rule, role)
self.assertTrue(allowed)
for rule in disallowed_rules:
- allowed = parser.allowed(rule, role)
+ allowed = authority.allowed(rule, role)
self.assertFalse(allowed)
role = 'super_admin'
@@ -175,11 +169,11 @@
disallowed_rules = ['non_admin_rule']
for rule in allowed_rules:
- allowed = parser.allowed(rule, role)
+ allowed = authority.allowed(rule, role)
self.assertTrue(allowed)
for rule in disallowed_rules:
- allowed = parser.allowed(rule, role)
+ allowed = authority.allowed(rule, role)
self.assertFalse(allowed)
def test_tenant_user_policy(self):
@@ -191,28 +185,28 @@
"""
test_tenant_id = mock.sentinel.tenant_id
test_user_id = mock.sentinel.user_id
- parser = rbac_policy_parser.RbacPolicyParser(
+ authority = policy_authority.PolicyAuthority(
test_tenant_id, test_user_id, "tenant_rbac_policy")
# Check whether Member role can perform expected actions.
allowed_rules = ['rule1', 'rule2', 'rule3', 'rule4']
for rule in allowed_rules:
- allowed = parser.allowed(rule, 'Member')
+ allowed = authority.allowed(rule, 'Member')
self.assertTrue(allowed)
disallowed_rules = ['admin_tenant_rule', 'admin_user_rule']
for disallowed_rule in disallowed_rules:
- self.assertFalse(parser.allowed(disallowed_rule, 'Member'))
+ self.assertFalse(authority.allowed(disallowed_rule, 'Member'))
# Check whether admin role can perform expected actions.
allowed_rules.extend(disallowed_rules)
for rule in allowed_rules:
- allowed = parser.allowed(rule, 'admin')
+ allowed = authority.allowed(rule, 'admin')
self.assertTrue(allowed)
# Check whether _try_rule is called with the correct target dictionary.
with mock.patch.object(
- parser, '_try_rule', return_value=True, autospec=True) \
+ authority, '_try_rule', return_value=True, autospec=True) \
as mock_try_rule:
expected_target = {
@@ -232,20 +226,20 @@
}
for rule in allowed_rules:
- allowed = parser.allowed(rule, 'Member')
+ allowed = authority.allowed(rule, 'Member')
self.assertTrue(allowed)
mock_try_rule.assert_called_once_with(
rule, expected_target, expected_access_data, mock.ANY)
mock_try_rule.reset_mock()
- @mock.patch.object(rbac_policy_parser, 'LOG', autospec=True)
+ @mock.patch.object(policy_authority, 'LOG', autospec=True)
def test_invalid_service_raises_exception(self, m_log):
test_tenant_id = mock.sentinel.tenant_id
test_user_id = mock.sentinel.user_id
service = 'invalid_service'
self.assertRaises(rbac_exceptions.RbacInvalidService,
- rbac_policy_parser.RbacPolicyParser,
+ policy_authority.PolicyAuthority,
test_tenant_id,
test_user_id,
service)
@@ -253,25 +247,25 @@
m_log.debug.assert_called_once_with(
'%s is NOT a valid service.', service)
- @mock.patch.object(rbac_policy_parser, 'LOG', autospec=True)
+ @mock.patch.object(policy_authority, 'LOG', autospec=True)
def test_service_is_none_raises_exception(self, m_log):
test_tenant_id = mock.sentinel.tenant_id
test_user_id = mock.sentinel.user_id
service = None
self.assertRaises(rbac_exceptions.RbacInvalidService,
- rbac_policy_parser.RbacPolicyParser,
+ policy_authority.PolicyAuthority,
test_tenant_id,
test_user_id,
service)
m_log.debug.assert_called_once_with('%s is NOT a valid service.', None)
- @mock.patch.object(rbac_policy_parser, 'LOG', autospec=True)
+ @mock.patch.object(policy_authority, 'LOG', autospec=True)
def test_invalid_policy_rule_throws_rbac_parsing_exception(self, m_log):
test_tenant_id = mock.sentinel.tenant_id
test_user_id = mock.sentinel.user_id
- parser = rbac_policy_parser.RbacPolicyParser(
+ authority = policy_authority.PolicyAuthority(
test_tenant_id, test_user_id, "custom_rbac_policy")
fake_rule = 'fake_rule'
@@ -279,18 +273,18 @@
.format(fake_rule, self.custom_policy_file)
e = self.assertRaises(rbac_exceptions.RbacParsingException,
- parser.allowed, fake_rule, None)
+ authority.allowed, fake_rule, None)
self.assertIn(expected_message, str(e))
m_log.debug.assert_called_once_with(expected_message)
- @mock.patch.object(rbac_policy_parser, 'LOG', autospec=True)
+ @mock.patch.object(policy_authority, 'LOG', autospec=True)
def test_unknown_exception_throws_rbac_parsing_exception(self, m_log):
test_tenant_id = mock.sentinel.tenant_id
test_user_id = mock.sentinel.user_id
- parser = rbac_policy_parser.RbacPolicyParser(
+ authority = policy_authority.PolicyAuthority(
test_tenant_id, test_user_id, "custom_rbac_policy")
- parser.rules = mock.MagicMock(
+ authority.rules = mock.MagicMock(
__name__='foo',
**{'__getitem__.return_value.side_effect': Exception(
mock.sentinel.error)})
@@ -300,11 +294,11 @@
self.custom_policy_file)
e = self.assertRaises(rbac_exceptions.RbacParsingException,
- parser.allowed, mock.sentinel.rule, None)
+ authority.allowed, mock.sentinel.rule, None)
self.assertIn(expected_message, str(e))
m_log.debug.assert_called_once_with(expected_message)
- @mock.patch.object(rbac_policy_parser, 'stevedore', autospec=True)
+ @mock.patch.object(policy_authority, 'stevedore', autospec=True)
def test_get_policy_data_from_file_and_from_code(self, mock_stevedore):
fake_policy_rules = [
self._get_fake_policy_rule('code_policy_action_1',
@@ -323,10 +317,10 @@
test_tenant_id = mock.sentinel.tenant_id
test_user_id = mock.sentinel.user_id
- parser = rbac_policy_parser.RbacPolicyParser(
+ authority = policy_authority.PolicyAuthority(
test_tenant_id, test_user_id, "tenant_rbac_policy")
- policy_data = parser._get_policy_data('fake_service')
+ policy_data = authority._get_policy_data('fake_service')
self.assertIsInstance(policy_data, str)
actual_policy_data = json.loads(policy_data)
@@ -344,7 +338,7 @@
self.assertEqual(expected_policy_data, actual_policy_data)
- @mock.patch.object(rbac_policy_parser, 'stevedore', autospec=True)
+ @mock.patch.object(policy_authority, 'stevedore', autospec=True)
def test_get_policy_data_from_file_and_from_code_with_overwrite(
self, mock_stevedore):
# The custom policy file should overwrite default rules rule1 and rule2
@@ -365,9 +359,9 @@
test_tenant_id = mock.sentinel.tenant_id
test_user_id = mock.sentinel.user_id
- parser = rbac_policy_parser.RbacPolicyParser(
+ authority = policy_authority.PolicyAuthority(
test_tenant_id, test_user_id, 'tenant_rbac_policy')
- policy_data = parser._get_policy_data('fake_service')
+ policy_data = authority._get_policy_data('fake_service')
self.assertIsInstance(policy_data, str)
actual_policy_data = json.loads(policy_data)
@@ -383,11 +377,11 @@
self.assertEqual(expected_policy_data, actual_policy_data)
- @mock.patch.object(rbac_policy_parser, 'stevedore', autospec=True)
+ @mock.patch.object(policy_authority, 'stevedore', autospec=True)
def test_get_policy_data_cannot_find_policy(self, mock_stevedore):
mock_stevedore.named.NamedExtensionManager.return_value = None
e = self.assertRaises(rbac_exceptions.RbacParsingException,
- rbac_policy_parser.RbacPolicyParser,
+ policy_authority.PolicyAuthority,
None, None, 'test_service')
expected_error = \
@@ -398,8 +392,8 @@
self.assertIn(expected_error, str(e))
- @mock.patch.object(rbac_policy_parser, 'json', autospec=True)
- @mock.patch.object(rbac_policy_parser, 'stevedore', autospec=True)
+ @mock.patch.object(policy_authority, 'json', autospec=True)
+ @mock.patch.object(policy_authority, 'stevedore', autospec=True)
def test_get_policy_data_without_valid_policy(self, mock_stevedore,
mock_json):
test_policy_action = mock.Mock(check='rule:bar', __name__='foo')
@@ -414,7 +408,7 @@
mock_json.dumps.side_effect = ValueError
e = self.assertRaises(rbac_exceptions.RbacParsingException,
- rbac_policy_parser.RbacPolicyParser,
+ policy_authority.PolicyAuthority,
None, None, 'test_service')
expected_error = "Policy file for {0} service is invalid."\
@@ -428,14 +422,14 @@
invoke_on_load=True,
warn_on_missing_entrypoint=False)
- @mock.patch.object(rbac_policy_parser, 'json', autospec=True)
- @mock.patch.object(rbac_policy_parser, 'stevedore', autospec=True)
+ @mock.patch.object(policy_authority, 'json', autospec=True)
+ @mock.patch.object(policy_authority, 'stevedore', autospec=True)
def test_get_policy_data_from_file_not_json(self, mock_stevedore,
mock_json):
mock_stevedore.named.NamedExtensionManager.return_value = None
mock_json.loads.side_effect = ValueError
e = self.assertRaises(rbac_exceptions.RbacParsingException,
- rbac_policy_parser.RbacPolicyParser,
+ policy_authority.PolicyAuthority,
None, None, 'tenant_rbac_policy')
expected_error = (
@@ -445,22 +439,22 @@
self.assertIn(expected_error, str(e))
def test_discover_policy_files(self):
- policy_parser = rbac_policy_parser.RbacPolicyParser(
+ policy_parser = policy_authority.PolicyAuthority(
None, None, 'tenant_rbac_policy')
# Ensure that "policy_files" is set at class and instance levels.
self.assertIn('policy_files',
- dir(rbac_policy_parser.RbacPolicyParser))
+ dir(policy_authority.PolicyAuthority))
self.assertIn('policy_files', dir(policy_parser))
self.assertIn('tenant_rbac_policy', policy_parser.policy_files)
self.assertEqual(self.conf_policy_path % 'tenant_rbac_policy',
policy_parser.policy_files['tenant_rbac_policy'])
- @mock.patch.object(rbac_policy_parser, 'policy', autospec=True)
- @mock.patch.object(rbac_policy_parser.RbacPolicyParser, '_get_policy_data',
+ @mock.patch.object(policy_authority, 'policy', autospec=True)
+ @mock.patch.object(policy_authority.PolicyAuthority, '_get_policy_data',
autospec=True)
- @mock.patch.object(rbac_policy_parser, 'clients', autospec=True)
- @mock.patch.object(rbac_policy_parser, 'os', autospec=True)
+ @mock.patch.object(policy_authority, 'clients', autospec=True)
+ @mock.patch.object(policy_authority, 'os', autospec=True)
def test_discover_policy_files_with_many_invalid_one_valid(self, m_os,
m_creds, *args):
# Only the 3rd path is valid.
@@ -472,16 +466,16 @@
'services': [{'name': 'test_service'}]}
# The expected policy will be 'baz/test_service'.
- CONF.set_override(
- 'custom_policy_files', ['foo/%s', 'bar/%s', 'baz/%s'],
- group='patrole')
+ self.useFixture(fixtures.ConfPatcher(
+ custom_policy_files=['foo/%s', 'bar/%s', 'baz/%s'],
+ group='patrole'))
- policy_parser = rbac_policy_parser.RbacPolicyParser(
+ policy_parser = policy_authority.PolicyAuthority(
None, None, 'test_service')
# Ensure that "policy_files" is set at class and instance levels.
self.assertIn('policy_files',
- dir(rbac_policy_parser.RbacPolicyParser))
+ dir(policy_authority.PolicyAuthority))
self.assertIn('policy_files', dir(policy_parser))
self.assertIn('test_service', policy_parser.policy_files)
self.assertEqual('baz/test_service',
@@ -493,19 +487,19 @@
[self.conf_policy_path % 'test_service'])
e = self.assertRaises(rbac_exceptions.RbacParsingException,
- rbac_policy_parser.RbacPolicyParser,
+ policy_authority.PolicyAuthority,
None, None, 'test_service')
self.assertIn(expected_error, str(e))
self.assertIn('policy_files',
- dir(rbac_policy_parser.RbacPolicyParser))
+ dir(policy_authority.PolicyAuthority))
self.assertNotIn(
'test_service',
- rbac_policy_parser.RbacPolicyParser.policy_files.keys())
+ policy_authority.PolicyAuthority.policy_files.keys())
def _test_validate_service(self, v2_services, v3_services,
expected_failure=False, expected_services=None):
- with mock.patch.object(rbac_policy_parser, 'clients') as m_creds:
+ with mock.patch.object(policy_authority, 'clients') as m_creds:
m_creds.Manager().identity_services_client.list_services.\
return_value = v2_services
m_creds.Manager().identity_services_v3_client.list_services.\
@@ -514,15 +508,15 @@
test_tenant_id = mock.sentinel.tenant_id
test_user_id = mock.sentinel.user_id
- mock_os = self.patchobject(rbac_policy_parser, 'os')
+ mock_os = self.patchobject(policy_authority, 'os')
mock_os.path.join.return_value = self.admin_policy_file
if not expected_services:
expected_services = [s['name'] for s in self.services['services']]
# Guarantee a blank slate for this test.
- if hasattr(rbac_policy_parser.RbacPolicyParser, 'available_services'):
- delattr(rbac_policy_parser.RbacPolicyParser,
+ if hasattr(policy_authority.PolicyAuthority, 'available_services'):
+ delattr(policy_authority.PolicyAuthority,
'available_services')
if expected_failure:
@@ -531,10 +525,10 @@
expected_exception = 'invalid_service is NOT a valid service'
with self.assertRaisesRegex(rbac_exceptions.RbacInvalidService,
expected_exception):
- rbac_policy_parser.RbacPolicyParser(
+ policy_authority.PolicyAuthority(
test_tenant_id, test_user_id, "INVALID_SERVICE")
else:
- policy_parser = rbac_policy_parser.RbacPolicyParser(
+ policy_parser = policy_authority.PolicyAuthority(
test_tenant_id, test_user_id, "tenant_rbac_policy")
# Check that the attribute is available at object and class levels.
@@ -543,11 +537,11 @@
self.assertTrue(hasattr(policy_parser, 'available_services'))
self.assertEqual(expected_services,
policy_parser.available_services)
- self.assertTrue(hasattr(rbac_policy_parser.RbacPolicyParser,
+ self.assertTrue(hasattr(policy_authority.PolicyAuthority,
'available_services'))
self.assertEqual(
expected_services,
- rbac_policy_parser.RbacPolicyParser.available_services)
+ policy_authority.PolicyAuthority.available_services)
def test_validate_service(self):
"""Positive test case to ensure ``validate_service`` works.
@@ -557,16 +551,16 @@
2) Identity v2 API enabled.
3) Both are enabled.
"""
- CONF.set_override('api_v2', True, group='identity-feature-enabled')
- CONF.set_override('api_v3', False, group='identity-feature-enabled')
+ self.useFixture(fixtures.ConfPatcher(
+ api_v2=True, api_v3=False, group='identity-feature-enabled'))
self._test_validate_service(self.services, [], False)
- CONF.set_override('api_v2', False, group='identity-feature-enabled')
- CONF.set_override('api_v3', True, group='identity-feature-enabled')
+ self.useFixture(fixtures.ConfPatcher(
+ api_v2=False, api_v3=True, group='identity-feature-enabled'))
self._test_validate_service([], self.services, False)
- CONF.set_override('api_v2', True, group='identity-feature-enabled')
- CONF.set_override('api_v3', True, group='identity-feature-enabled')
+ self.useFixture(fixtures.ConfPatcher(
+ api_v2=True, api_v3=True, group='identity-feature-enabled'))
self._test_validate_service(self.services, self.services, False)
def test_validate_service_except_invalid_service(self):
@@ -578,18 +572,18 @@
3) Both are enabled.
4) Neither are enabled.
"""
- CONF.set_override('api_v2', True, group='identity-feature-enabled')
- CONF.set_override('api_v3', False, group='identity-feature-enabled')
+ self.useFixture(fixtures.ConfPatcher(
+ api_v2=True, api_v3=False, group='identity-feature-enabled'))
self._test_validate_service(self.services, [], True)
- CONF.set_override('api_v2', False, group='identity-feature-enabled')
- CONF.set_override('api_v3', True, group='identity-feature-enabled')
+ self.useFixture(fixtures.ConfPatcher(
+ api_v2=False, api_v3=True, group='identity-feature-enabled'))
self._test_validate_service([], self.services, True)
- CONF.set_override('api_v2', True, group='identity-feature-enabled')
- CONF.set_override('api_v3', True, group='identity-feature-enabled')
+ self.useFixture(fixtures.ConfPatcher(
+ api_v2=True, api_v3=True, group='identity-feature-enabled'))
self._test_validate_service(self.services, self.services, True)
- CONF.set_override('api_v2', False, group='identity-feature-enabled')
- CONF.set_override('api_v3', False, group='identity-feature-enabled')
+ self.useFixture(fixtures.ConfPatcher(
+ api_v2=False, api_v3=False, group='identity-feature-enabled'))
self._test_validate_service([], [], True, [])
diff --git a/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py b/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
index 0dcaaa7..94a2306 100644
--- a/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
+++ b/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
@@ -44,8 +44,8 @@
rbac_rv.RBACLOG, 'info', autospec=False).start()
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
- @mock.patch.object(rbac_rv, 'rbac_policy_parser', autospec=True)
- def test_rule_validation_have_permission_no_exc(self, mock_policy,
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+ def test_rule_validation_have_permission_no_exc(self, mock_authority,
mock_log):
"""Test that having permission and no exception thrown is success.
@@ -56,7 +56,8 @@
mock_function = mock.Mock(__name__='foo')
wrapper = decorator(mock_function)
- mock_policy.RbacPolicyParser.return_value.allowed.return_value = True
+ mock_authority.PolicyAuthority.return_value.allowed\
+ .return_value = True
result = wrapper(self.mock_args)
@@ -65,8 +66,8 @@
mock_log.error.assert_not_called()
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
- @mock.patch.object(rbac_rv, 'rbac_policy_parser', autospec=True)
- def test_rule_validation_lack_permission_throw_exc(self, mock_policy,
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+ def test_rule_validation_lack_permission_throw_exc(self, mock_authority,
mock_log):
"""Test that having no permission and exception thrown is success.
@@ -78,7 +79,8 @@
mock_function.side_effect = exceptions.Forbidden
wrapper = decorator(mock_function)
- mock_policy.RbacPolicyParser.return_value.allowed.return_value = False
+ mock_authority.PolicyAuthority.return_value.allowed\
+ .return_value = False
result = wrapper(self.mock_args)
@@ -87,8 +89,9 @@
mock_log.error.assert_not_called()
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
- @mock.patch.object(rbac_rv, 'rbac_policy_parser', autospec=True)
- def test_rule_validation_forbidden_negative(self, mock_policy, mock_log):
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+ def test_rule_validation_forbidden_negative(self, mock_authority,
+ mock_log):
"""Test Forbidden error is thrown and have permission fails.
Negative test case: if Forbidden is thrown and the user should be
@@ -100,7 +103,8 @@
mock_function.side_effect = exceptions.Forbidden
wrapper = decorator(mock_function)
- mock_policy.RbacPolicyParser.return_value.allowed.return_value = True
+ mock_authority.PolicyAuthority.return_value.allowed\
+ .return_value = True
e = self.assertRaises(exceptions.Forbidden, wrapper, self.mock_args)
self.assertIn(
@@ -110,9 +114,9 @@
" perform sentinel.action.")
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
- @mock.patch.object(rbac_rv, 'rbac_policy_parser', autospec=True)
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
def test_rule_validation_rbac_malformed_response_positive(
- self, mock_policy, mock_log):
+ self, mock_authority_authority, mock_log):
"""Test RbacMalformedResponse error is thrown without permission passes.
Positive test case: if RbacMalformedResponse is thrown and the user is
@@ -123,7 +127,8 @@
mock_function.side_effect = rbac_exceptions.RbacMalformedResponse
wrapper = decorator(mock_function)
- mock_policy.RbacPolicyParser.return_value.allowed.return_value = False
+ (mock_authority_authority.PolicyAuthority.return_value.allowed
+ .return_value) = False
result = wrapper(self.mock_args)
@@ -132,9 +137,9 @@
mock_log.warning.assert_not_called()
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
- @mock.patch.object(rbac_rv, 'rbac_policy_parser', autospec=True)
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
def test_rule_validation_rbac_malformed_response_negative(
- self, mock_policy, mock_log):
+ self, mock_authority_authority, mock_log):
"""Test RbacMalformedResponse error is thrown with permission fails.
Negative test case: if RbacMalformedResponse is thrown and the user is
@@ -145,7 +150,8 @@
mock_function.side_effect = rbac_exceptions.RbacMalformedResponse
wrapper = decorator(mock_function)
- mock_policy.RbacPolicyParser.return_value.allowed.return_value = True
+ (mock_authority_authority.PolicyAuthority.return_value.allowed
+ .return_value) = True
e = self.assertRaises(exceptions.Forbidden, wrapper, self.mock_args)
self.assertIn(
@@ -156,9 +162,9 @@
" perform sentinel.action.")
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
- @mock.patch.object(rbac_rv, 'rbac_policy_parser', autospec=True)
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
def test_rule_validation_rbac_conflicting_policies_positive(
- self, mock_policy, mock_log):
+ self, mock_authority_authority, mock_log):
"""Test RbacConflictingPolicies error is thrown without permission passes.
Positive test case: if RbacConflictingPolicies is thrown and the user
@@ -169,7 +175,8 @@
mock_function.side_effect = rbac_exceptions.RbacConflictingPolicies
wrapper = decorator(mock_function)
- mock_policy.RbacPolicyParser.return_value.allowed.return_value = False
+ (mock_authority_authority.PolicyAuthority.return_value.allowed
+ .return_value) = False
result = wrapper(self.mock_args)
@@ -178,9 +185,10 @@
mock_log.warning.assert_not_called()
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
- @mock.patch.object(rbac_rv, 'rbac_policy_parser', autospec=True)
- def test_rule_validation_rbac_conflicting_policies_negative(
- self, mock_policy, mock_log):
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+ def test_rule_validation_rbac_conflicting_policies_negative(self,
+ mock_authority,
+ mock_log):
"""Test RbacConflictingPolicies error is thrown with permission fails.
Negative test case: if RbacConflictingPolicies is thrown and the user
@@ -191,7 +199,8 @@
mock_function.side_effect = rbac_exceptions.RbacConflictingPolicies
wrapper = decorator(mock_function)
- mock_policy.RbacPolicyParser.return_value.allowed.return_value = True
+ mock_authority.PolicyAuthority.return_value.allowed\
+ .return_value = True
e = self.assertRaises(exceptions.Forbidden, wrapper, self.mock_args)
self.assertIn(
@@ -202,8 +211,8 @@
" perform sentinel.action.")
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
- @mock.patch.object(rbac_rv, 'rbac_policy_parser', autospec=True)
- def test_expect_not_found_but_raises_forbidden(self, mock_policy,
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+ def test_expect_not_found_but_raises_forbidden(self, mock_authority,
mock_log):
"""Test that expecting 404 but getting 403 works for all scenarios.
@@ -223,9 +232,9 @@
expected_error = "An unexpected exception has occurred during test: "\
"foo, Exception was: Forbidden\nDetails: Random message."
- for permission in [True, False]:
- mock_policy.RbacPolicyParser.return_value.allowed.return_value =\
- permission
+ for allowed in [True, False]:
+ mock_authority.PolicyAuthority.return_value.allowed.\
+ return_value = allowed
e = self.assertRaises(exceptions.Forbidden, wrapper,
self.mock_args)
@@ -234,8 +243,8 @@
mock_log.error.reset_mock()
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
- @mock.patch.object(rbac_rv, 'rbac_policy_parser', autospec=True)
- def test_expect_not_found_and_raise_not_found(self, mock_policy,
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+ def test_expect_not_found_and_raise_not_found(self, mock_authority,
mock_log):
"""Test that expecting 404 and getting 404 works for all scenarios.
@@ -257,9 +266,9 @@
"Role Member was not allowed to perform sentinel.action.", None
]
- for pos, permission in enumerate([True, False]):
- mock_policy.RbacPolicyParser.return_value.allowed.return_value =\
- permission
+ for pos, allowed in enumerate([True, False]):
+ mock_authority.PolicyAuthority.return_value.allowed\
+ .return_value = allowed
expected_error = expected_errors[pos]
@@ -282,8 +291,8 @@
mock_log.error.reset_mock()
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
- @mock.patch.object(rbac_rv, 'rbac_policy_parser', autospec=True)
- def test_rule_validation_overpermission_negative(self, mock_policy,
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+ def test_rule_validation_overpermission_negative(self, mock_authority,
mock_log):
"""Test that OverPermission is correctly handled.
@@ -295,7 +304,8 @@
mock_function = mock.Mock(__name__='foo')
wrapper = decorator(mock_function)
- mock_policy.RbacPolicyParser.return_value.allowed.return_value = False
+ mock_authority.PolicyAuthority.return_value.allowed\
+ .return_value = False
e = self.assertRaises(rbac_exceptions.RbacOverPermission, wrapper,
self.mock_args)
@@ -305,15 +315,15 @@
'Role %s was allowed to perform %s', 'Member',
mock.sentinel.action)
- @mock.patch.object(rbac_rv, 'rbac_policy_parser', autospec=True)
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
def test_invalid_policy_rule_throws_parsing_exception(
- self, mock_rbac_policy_parser):
+ self, mock_authority_authority):
"""Test that invalid policy action causes test to be skipped."""
CONF.set_override('strict_policy_check', True, group='patrole')
self.addCleanup(CONF.clear_override, 'strict_policy_check',
group='patrole')
- mock_rbac_policy_parser.RbacPolicyParser.return_value.allowed.\
+ mock_authority_authority.PolicyAuthority.return_value.allowed.\
side_effect = rbac_exceptions.RbacParsingException
decorator = rbac_rv.action(mock.sentinel.service,
@@ -325,12 +335,12 @@
self.assertEqual('Attempted to test an invalid policy file or action',
str(e))
- mock_rbac_policy_parser.RbacPolicyParser.assert_called_once_with(
+ mock_authority_authority.PolicyAuthority.assert_called_once_with(
mock.sentinel.project_id, mock.sentinel.user_id,
mock.sentinel.service, extra_target_data={})
- @mock.patch.object(rbac_rv, 'rbac_policy_parser', autospec=True)
- def test_get_exception_type_404(self, mock_policy):
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+ def test_get_exception_type_404(self, _):
"""Test that getting a 404 exception type returns NotFound."""
expected_exception = exceptions.NotFound
expected_irregular_msg = ("NotFound exception was caught for policy "
@@ -343,8 +353,8 @@
self.assertEqual(expected_exception, actual_exception)
self.assertEqual(expected_irregular_msg, actual_irregular_msg)
- @mock.patch.object(rbac_rv, 'rbac_policy_parser', autospec=True)
- def test_get_exception_type_403(self, mock_policy):
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+ def test_get_exception_type_403(self, _):
"""Test that getting a 404 exception type returns Forbidden."""
expected_exception = exceptions.Forbidden
expected_irregular_msg = None
@@ -355,10 +365,9 @@
self.assertEqual(expected_exception, actual_exception)
self.assertEqual(expected_irregular_msg, actual_irregular_msg)
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
- @mock.patch.object(rbac_rv, 'rbac_policy_parser', autospec=True)
- def test_exception_thrown_when_type_is_not_int(self, mock_policy,
- mock_log):
+ def test_exception_thrown_when_type_is_not_int(self, mock_log, _):
"""Test that non-integer exception type raises error."""
self.assertRaises(rbac_exceptions.RbacInvalidErrorCode,
rbac_rv._get_exception_type, "403")
@@ -367,10 +376,9 @@
"code. Currently supported "
"codes: [403, 404]")
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
- @mock.patch.object(rbac_rv, 'rbac_policy_parser', autospec=True)
- def test_exception_thrown_when_type_is_403_or_404(self, mock_policy,
- mock_log):
+ def test_exception_thrown_when_type_is_403_or_404(self, mock_log, _):
"""Test that unsupported exceptions throw error."""
invalid_exceptions = [200, 400, 500]
for exc in invalid_exceptions:
@@ -383,8 +391,8 @@
mock_log.error.reset_mock()
@mock.patch.object(rbac_rv, 'RBACLOG', autospec=True)
- @mock.patch.object(rbac_rv, 'rbac_policy_parser', autospec=True)
- def test_rbac_report_logging_disabled(self, mock_policy, mock_rbaclog):
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+ def test_rbac_report_logging_disabled(self, mock_authority, mock_rbaclog):
"""Test case to ensure that we DON'T write logs when
enable_reporting is False
"""
@@ -397,15 +405,15 @@
mock_function = mock.Mock(__name__='foo-nolog')
wrapper = decorator(mock_function)
- mock_policy.RbacPolicyParser.return_value.allowed.return_value = True
+ mock_authority.PolicyAuthority.return_value.allowed.return_value = True
wrapper(self.mock_args)
self.assertFalse(mock_rbaclog.info.called)
@mock.patch.object(rbac_rv, 'RBACLOG', autospec=True)
- @mock.patch.object(rbac_rv, 'rbac_policy_parser', autospec=True)
- def test_rbac_report_logging_enabled(self, mock_policy, mock_rbaclog):
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+ def test_rbac_report_logging_enabled(self, mock_authority, mock_rbaclog):
"""Test case to ensure that we DO write logs when
enable_reporting is True
"""
@@ -418,7 +426,7 @@
mock_function = mock.Mock(__name__='foo-log')
wrapper = decorator(mock_function)
- mock_policy.RbacPolicyParser.return_value.allowed.return_value = True
+ mock_authority.PolicyAuthority.return_value.allowed.return_value = True
wrapper(self.mock_args)