Merge "RequirementsAuthority multi role support enhancement"

doc/source/framework/requirements_authority.rst

@@ -7,8 +7,9 @@ Overview
 --------
 
 Requirements-driven approach to declaring the expected RBAC test results
-referenced by Patrole. Uses a high-level YAML syntax to crystallize policy
+referenced by Patrole. These requirements express the *intention* behind the
-requirements concisely and unambiguously.
+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 @@ expected test results by performing lookups against a
 :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
@@ -74,28 +71,84 @@ file should be written as follows:
   .. code-block:: yaml
 
       <service_foo>:
-      <api_action_a>:
+        <logical_or_example>:
           - <allowed_role_1>
           - <allowed_role_2>
-        - <allowed_role_3>
+        <logical_and_example>:
-      <api_action_b>:
+          - <allowed_role_3>, <allowed_role_4>
-        - <allowed_role_2>
-        - <allowed_role_4>
       <service_bar>:
-      <api_action_c>:
+        <logical_not_example>:
-        - <allowed_role_3>
+          - <!disallowed_role_5>
 
 Where:
 
-service = the service that is being tested (Cinder, Nova, etc.).
+* ``service`` - the service that is being tested (Cinder, Nova, etc.).
-
+* ``api_action`` - the policy action that is being tested. Examples:
-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.
+* ``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>
+
+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>
+
+as ``<allowed_role_1> and <allowed_role_2> or <allowed_role_3>``.
+
+Also negative roles may be defined with an exclamation mark ahead of role:
+
+.. code-block:: yaml
+
+    <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>
+
+This example must be read as ``<allowed_role_1> and <allowed_role_2> and not
+<disallowed_role_4> or <allowed_role_3>``.
+
 
 Implementation
 --------------

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 @@ class RequirementsParser(object):
             <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 @@ class RequirementsParser(object):
         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 @@ class RequirementsAuthority(RbacAuthority):
                 "empty. Ensure the requirements YAML file is correctly "
                 "formatted.")
         try:
-            _api = self.roles_dict[rule_name]
+            requirement_roles = self.roles_dict[rule_name]
-            return all(role in _api for role in roles)
+
+            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)

patrole_tempest_plugin/tests/unit/resources/rbac_roles.yaml

@@ -4,3 +4,7 @@ Test:
     - _member_
   test:create2:
     - test_member
+  test:create3:
+    - test_member, _member_
+  test:create4:
+    - test_member, !_member_

patrole_tempest_plugin/tests/unit/test_requirements_authority.py

@@ -20,16 +20,25 @@ from tempest.tests import base
 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_'],
+        self.expected_result = {'test:create': [['test_member'], ['_member_']],
-                                'test:create2': ['test_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 @@ class RequirementsAuthorityTest(base.TestCase):
                           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 @@ class RequirementsAuthorityTest(base.TestCase):
 
     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 @@ class RequirementsAuthorityTest(base.TestCase):
 
     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 @@ class RequirementsAuthorityTest(base.TestCase):
 
     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"]))

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