Merge "RequirementsAuthority multi role support enhancement"
diff --git a/doc/source/framework/requirements_authority.rst b/doc/source/framework/requirements_authority.rst
index 628f0c0..cf7c51c 100644
--- a/doc/source/framework/requirements_authority.rst
+++ b/doc/source/framework/requirements_authority.rst
@@ -7,8 +7,9 @@
--------
Requirements-driven approach to declaring the expected RBAC test results
-referenced by Patrole. Uses a high-level YAML syntax to crystallize policy
-requirements concisely and unambiguously.
+referenced by Patrole. These requirements express the *intention* behind the
+policy. A high-level YAML syntax is used to concisely and clearly map each
+policy action to the list of associated roles.
.. note::
@@ -29,10 +30,6 @@
:ref:`custom-requirements-file` which precisely defines the cloud's RBAC
requirements.
-Using a high-level declarative language, the requirements are captured
-unambiguously in the :ref:`custom-requirements-file`, allowing operators to
-validate their requirements against their OpenStack cloud.
-
This validation approach should be used when:
* The cloud has heavily customized policy files that require careful validation
@@ -71,31 +68,87 @@
file must be located on the same host that Patrole runs on. The YAML
file should be written as follows:
+ .. code-block:: yaml
+
+ <service_foo>:
+ <logical_or_example>:
+ - <allowed_role_1>
+ - <allowed_role_2>
+ <logical_and_example>:
+ - <allowed_role_3>, <allowed_role_4>
+ <service_bar>:
+ <logical_not_example>:
+ - <!disallowed_role_5>
+
+Where:
+
+* ``service`` - the service that is being tested (Cinder, Nova, etc.).
+* ``api_action`` - the policy action that is being tested. Examples:
+
+ * volume:create
+ * os_compute_api:servers:start
+ * add_image
+
+* ``allowed_role`` - the ``oslo.policy`` role that is allowed to perform the
+ API.
+
+Each item under ``logical_or_example`` is "logical OR"-ed together. Each role
+in the comma-separated string under ``logical_and_example`` is "logical AND"-ed
+together. And each item prefixed with "!" under ``logical_not_example`` is
+"logical negated".
+
+.. note::
+
+ The custom requirements file only allows policy actions to be mapped to
+ the associated roles that define it. Complex ``oslo.policy`` constructs
+ like ``literals`` or ``GenericChecks`` are not supported. For more
+ information, reference the `oslo.policy documentation`_.
+
+.. _oslo.policy documentation: https://docs.openstack.org/oslo.policy/latest/reference/api/oslo_policy.policy.html#policy-rule-expressions
+
+Examples
+~~~~~~~~
+
+Items within ``api_action`` are considered as logical or, so you may read:
+
.. code-block:: yaml
<service_foo>:
+ # "api_action_a: allowed_role_1 or allowed_role_2 or allowed_role_3"
<api_action_a>:
- <allowed_role_1>
- <allowed_role_2>
- <allowed_role_3>
- <api_action_b>:
- - <allowed_role_2>
- - <allowed_role_4>
- <service_bar>:
- <api_action_c>:
+
+as ``<allowed_role_1> or <allowed_role_2> or <allowed_role_3>``.
+
+Roles within comma-separated items are considered as logic and, so you may
+read:
+
+.. code-block:: yaml
+
+ <service_foo>:
+ # "api_action_a: (allowed_role_1 and allowed_role_2) or allowed_role_3"
+ <api_action_a>:
+ - <allowed_role_1>, <allowed_role_2>
- <allowed_role_3>
-Where:
+as ``<allowed_role_1> and <allowed_role_2> or <allowed_role_3>``.
-service = the service that is being tested (Cinder, Nova, etc.).
+Also negative roles may be defined with an exclamation mark ahead of role:
-api_action = the policy action that is being tested. Examples:
+.. code-block:: yaml
-* volume:create
-* os_compute_api:servers:start
-* add_image
+ <service_foo>:
+ # "api_action_a: (allowed_role_1 and allowed_role_2 and not
+ # disallowed_role_4) or allowed_role_3"
+ <api_action_a>:
+ - <allowed_role_1>, <allowed_role_2>, !<disallowed_role_4>
+ - <allowed_role_3>
-allowed_role = the ``oslo.policy`` role that is allowed to perform the API.
+This example must be read as ``<allowed_role_1> and <allowed_role_2> and not
+<disallowed_role_4> or <allowed_role_3>``.
+
Implementation
--------------
diff --git a/patrole_tempest_plugin/requirements_authority.py b/patrole_tempest_plugin/requirements_authority.py
index 57caf79..4697c3b 100644
--- a/patrole_tempest_plugin/requirements_authority.py
+++ b/patrole_tempest_plugin/requirements_authority.py
@@ -12,6 +12,7 @@
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
+import copy
import yaml
from oslo_log import log as logging
@@ -50,7 +51,7 @@
<service_foo>:
<api_action_a>:
- <allowed_role_1>
- - <allowed_role_2>
+ - <allowed_role_2>,<allowed_role_3>
- <allowed_role_3>
<api_action_b>:
- <allowed_role_2>
@@ -67,7 +68,16 @@
try:
for section in RequirementsParser.Inner._rbac_map:
if component in section:
- return section[component]
+ rules = copy.copy(section[component])
+
+ for rule in rules:
+ rules[rule] = [
+ roles.split(',') for roles in rules[rule]]
+
+ for i, role_pack in enumerate(rules[rule]):
+ rules[rule][i] = [r.strip() for r in role_pack]
+
+ return rules
except yaml.parser.ParserError:
LOG.error("Error while parsing the requirements YAML file. Did "
"you pass a valid component name from the test case?")
@@ -115,8 +125,24 @@
"empty. Ensure the requirements YAML file is correctly "
"formatted.")
try:
- _api = self.roles_dict[rule_name]
- return all(role in _api for role in roles)
+ requirement_roles = self.roles_dict[rule_name]
+
+ for role_reqs in requirement_roles:
+ required_roles = [
+ role for role in role_reqs if not role.startswith("!")]
+ forbidden_roles = [
+ role[1:] for role in role_reqs if role.startswith("!")]
+
+ # User must have all required roles
+ required_passed = all([r in roles for r in required_roles])
+ # User must not have any forbidden roles
+ forbidden_passed = all([r not in forbidden_roles
+ for r in roles])
+
+ if required_passed and forbidden_passed:
+ return True
+
+ return False
except KeyError:
raise KeyError("'%s' API is not defined in the requirements YAML "
"file" % rule_name)
diff --git a/patrole_tempest_plugin/tests/unit/resources/rbac_roles.yaml b/patrole_tempest_plugin/tests/unit/resources/rbac_roles.yaml
index c5436d0..357e0e6 100644
--- a/patrole_tempest_plugin/tests/unit/resources/rbac_roles.yaml
+++ b/patrole_tempest_plugin/tests/unit/resources/rbac_roles.yaml
@@ -4,3 +4,7 @@
- _member_
test:create2:
- test_member
+ test:create3:
+ - test_member, _member_
+ test:create4:
+ - test_member, !_member_
\ No newline at end of file
diff --git a/patrole_tempest_plugin/tests/unit/test_requirements_authority.py b/patrole_tempest_plugin/tests/unit/test_requirements_authority.py
index 4b75ef5..0f7310e 100644
--- a/patrole_tempest_plugin/tests/unit/test_requirements_authority.py
+++ b/patrole_tempest_plugin/tests/unit/test_requirements_authority.py
@@ -20,16 +20,25 @@
from patrole_tempest_plugin import requirements_authority as req_auth
-class RequirementsAuthorityTest(base.TestCase):
+class BaseRequirementsAuthorityTest(base.TestCase):
def setUp(self):
- super(RequirementsAuthorityTest, self).setUp()
+ super(BaseRequirementsAuthorityTest, self).setUp()
self.rbac_auth = req_auth.RequirementsAuthority()
self.current_directory = os.path.dirname(os.path.realpath(__file__))
self.yaml_test_file = os.path.join(self.current_directory,
'resources',
'rbac_roles.yaml')
- self.expected_result = {'test:create': ['test_member', '_member_'],
- 'test:create2': ['test_member']}
+ self.expected_result = {'test:create': [['test_member'], ['_member_']],
+ 'test:create2': [['test_member']],
+ 'test:create3': [['test_member', '_member_']],
+ 'test:create4': [['test_member', '!_member_']]}
+ self.expected_rbac_map = {'test:create': ['test_member', '_member_'],
+ 'test:create2': ['test_member'],
+ 'test:create3': ['test_member, _member_'],
+ 'test:create4': ['test_member, !_member_']}
+
+
+class RequirementsAuthorityTest(BaseRequirementsAuthorityTest):
def test_requirements_auth_init(self):
rbac_auth = req_auth.RequirementsAuthority(self.yaml_test_file, 'Test')
@@ -41,11 +50,11 @@
self.rbac_auth.allowed, "", [""])
def test_auth_allowed_role_in_api(self):
- self.rbac_auth.roles_dict = {'api': ['_member_']}
+ self.rbac_auth.roles_dict = {'api': [['_member_']]}
self.assertTrue(self.rbac_auth.allowed("api", ["_member_"]))
def test_auth_allowed_role_not_in_api(self):
- self.rbac_auth.roles_dict = {'api': ['_member_']}
+ self.rbac_auth.roles_dict = {'api': [['_member_']]}
self.assertFalse(self.rbac_auth.allowed("api", "support_member"))
def test_parser_get_allowed_except_keyerror(self):
@@ -55,12 +64,12 @@
def test_parser_init(self):
req_auth.RequirementsParser(self.yaml_test_file)
- self.assertEqual([{'Test': self.expected_result}],
+ self.assertEqual([{'Test': self.expected_rbac_map}],
req_auth.RequirementsParser.Inner._rbac_map)
def test_parser_role_in_api(self):
req_auth.RequirementsParser.Inner._rbac_map = \
- [{'Test': self.expected_result}]
+ [{'Test': self.expected_rbac_map}]
self.rbac_auth.roles_dict = req_auth.RequirementsParser.parse("Test")
self.assertEqual(self.expected_result, self.rbac_auth.roles_dict)
@@ -69,7 +78,7 @@
def test_parser_role_not_in_api(self):
req_auth.RequirementsParser.Inner._rbac_map = \
- [{'Test': self.expected_result}]
+ [{'Test': self.expected_rbac_map}]
self.rbac_auth.roles_dict = req_auth.RequirementsParser.parse("Test")
self.assertEqual(self.expected_result, self.rbac_auth.roles_dict)
@@ -77,10 +86,102 @@
def test_parser_except_invalid_configuration(self):
req_auth.RequirementsParser.Inner._rbac_map = \
- [{'Test': self.expected_result}]
+ [{'Test': self.expected_rbac_map}]
self.rbac_auth.roles_dict = \
req_auth.RequirementsParser.parse("Failure")
self.assertIsNone(self.rbac_auth.roles_dict)
self.assertRaises(exceptions.InvalidConfiguration,
self.rbac_auth.allowed, "", [""])
+
+ def test_auth_allowed_exclamation_mark_syntax_single_role(self):
+ """Ensure that exclamation mark in front of role is dropped, and not
+ considered as part of role itself.
+ """
+
+ self.rbac_auth.roles_dict = {'api': [['!admin']]}
+ self.assertTrue(self.rbac_auth.allowed("api", ["member"]))
+ self.assertTrue(self.rbac_auth.allowed("api", ["!admin"]))
+ self.assertFalse(self.rbac_auth.allowed("api", ["admin"]))
+
+
+class RequirementsAuthorityMultiRoleTest(BaseRequirementsAuthorityTest):
+
+ def test_auth_allowed_exclamation_mark_syntax_multi_role(self):
+ """Ensure that exclamation mark in front of role is dropped, and not
+ considered as part of role itself.
+ """
+
+ self.rbac_auth.roles_dict = {'api': [['member', '!admin']]}
+ self.assertFalse(self.rbac_auth.allowed("api", ["member", "admin"]))
+ self.assertTrue(self.rbac_auth.allowed("api", ["member", "!admin"]))
+
+ def test_auth_allowed_single_rule_scenario(self):
+ # member and support and not admin and not manager
+ self.rbac_auth.roles_dict = {'api': [['member', 'support',
+ '!admin', '!manager']]}
+
+ # User is member and support and not manager or admin
+ self.assertTrue(self.rbac_auth.allowed("api", ["member",
+ "support"]))
+
+ # User is member and not manager or admin, but not support
+ self.assertFalse(self.rbac_auth.allowed("api", ["member"]))
+
+ # User is support and not manager or admin, but not member
+ self.assertFalse(self.rbac_auth.allowed("api", ["support"]))
+
+ # User is member and support and not manager, but have admin role
+ self.assertFalse(self.rbac_auth.allowed("api", ["member",
+ "support",
+ "admin"]))
+
+ # User is member and not manager, but have admin role and not support
+ self.assertFalse(self.rbac_auth.allowed("api", ["member",
+ "admin"]))
+
+ # User is member and support, but have manager and admin roles
+ self.assertFalse(self.rbac_auth.allowed("api", ["member",
+ "support",
+ "admin",
+ "manager"]))
+
+ def test_auth_allowed_multi_rule_scenario(self):
+ rules = [
+ ['member', 'support', '!admin', '!manager'],
+ ['member', 'admin'],
+ ["manager"]
+ ]
+ self.rbac_auth.roles_dict = {'api': rules}
+
+ # Not a single role allows viewer
+ self.assertFalse(self.rbac_auth.allowed("api", ["viewer"]))
+ # We have no rule that allows support and admin
+ self.assertFalse(self.rbac_auth.allowed("api", ["support",
+ "admin"]))
+ # There is no rule that requires member without additional requirements
+ self.assertFalse(self.rbac_auth.allowed("api", ["member"]))
+
+ # Pass with rules[2]
+ self.assertTrue(self.rbac_auth.allowed("api", ["manager"]))
+ # Pass with rules[0]
+ self.assertTrue(self.rbac_auth.allowed("api", ["member",
+ "support"]))
+ # Pass with rules[1]
+ self.assertTrue(self.rbac_auth.allowed("api", ["member",
+ "admin"]))
+ # Pass with rules[2]
+ self.assertTrue(self.rbac_auth.allowed("api", ["manager",
+ "admin"]))
+ # Pass with rules[1]
+ self.assertTrue(self.rbac_auth.allowed("api", ["member",
+ "support",
+ "admin"]))
+ # Pass with rules[1]
+ self.assertTrue(self.rbac_auth.allowed("api", ["member",
+ "support",
+ "admin",
+ "manager"]))
+ # Pass with rules[2]
+ self.assertTrue(self.rbac_auth.allowed("api", ["admin",
+ "manager"]))
diff --git a/releasenotes/notes/requirements-authority-multi-role-support-0fe53fc49567e595.yaml b/releasenotes/notes/requirements-authority-multi-role-support-0fe53fc49567e595.yaml
new file mode 100644
index 0000000..ffbae0a
--- /dev/null
+++ b/releasenotes/notes/requirements-authority-multi-role-support-0fe53fc49567e595.yaml
@@ -0,0 +1,37 @@
+---
+features:
+ - |
+ The ``requirements_authority`` module now supports the following 3 cases:
+
+ * logical or operation of roles (existing functionality)
+ * logical and operation of roles (new functionality)
+ * logical not operation of roles (new functionality)
+
+ .. code-block:: yaml
+
+ <service_foo>:
+ <logical_or_example>:
+ - <allowed_role_1>
+ - <allowed_role_2>
+ <logical_and_example>:
+ - <allowed_role_3>, <allowed_role_4>
+ <service_bar>:
+ <logical_not_example>:
+ - <!disallowed_role_5>
+
+ Each item under ``logical_or_example`` is "logical OR"-ed together. Each
+ role in the comma-separated string under ``logical_and_example`` is
+ "logical AND"-ed together. And each item prefixed with "!" under
+ ``logical_not_example`` is "logical negated".
+
+ This allows for expressing many more complex cases using the
+ ``requirements_authority`` YAML syntax. For example, the policy rule
+ (i.e. what may exist in a ``policy.yaml`` file)::
+
+ "foo_rule: (role:a and not role:b) or role:c"
+
+ May now be expressed using the YAML syntax as::
+
+ foo_rule:
+ - a, !b
+ - c