Merge "Add docstrings for requirements_authority module"
diff --git a/etc/patrole.conf.sample b/etc/patrole.conf.sample
index 5816ea9..518d38a 100644
--- a/etc/patrole.conf.sample
+++ b/etc/patrole.conf.sample
@@ -28,15 +28,15 @@
#
# This option determines whether Patrole should run against a
-# `custom_requirements_file` which defines RBAC requirements. The
+# ``custom_requirements_file`` which defines RBAC requirements. The
# purpose of setting this flag to True is to verify that RBAC policy
# is in accordance to requirements. The idea is that the
-# `custom_requirements_file` perfectly defines what the RBAC
+# ``custom_requirements_file`` perfectly defines what the RBAC
# requirements are.
#
# Here are the possible outcomes when running the Patrole tests
# against
-# a `custom_requirements_file`:
+# a ``custom_requirements_file``:
#
# YAML definition: allowed
# test run: allowed
@@ -44,7 +44,7 @@
#
# YAML definition: allowed
# test run: not allowed
-# test result: fail (under-permission)
+# test result: fail (under-permission, e.g. Forbidden exception)
#
# YAML definition: not allowed
# test run: allowed
@@ -53,30 +53,32 @@
#test_custom_requirements = false
#
-# File path of the yaml file that defines your RBAC requirements. This
-# file must be located on the same host that Patrole runs on. The yaml
+# File path of the YAML file that defines your RBAC requirements. This
+# file must be located on the same host that Patrole runs on. The YAML
# file should be written as follows:
#
# ```
-# <service>:
-# <api_action>:
-# - <allowed_role>
-# - <allowed_role>
-# - <allowed_role>
-# <api_action>:
-# - <allowed_role>
-# - <allowed_role>
-# <service>
-# <api_action>:
-# - <allowed_role>
+# <service_foo>:
+# <api_action_x>:
+# - <allowed_role_a>
+# - <allowed_role_b>
+# - <allowed_role_c>
+# <api_action_y>:
+# - <allowed_role_d>
+# - <allowed_role_e>
+# <service_bar>:
+# <api_action_z>:
+# - <allowed_role_b>
# ```
+#
# 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:
# - volume:create
# - os_compute_api:servers:start
# - add_image
-# allowed_role = the Keystone role that is allowed to perform the API
+# allowed_role = the Keystone role that is allowed to perform the API.
# (string value)
#custom_requirements_file = <None>
diff --git a/patrole_tempest_plugin/config.py b/patrole_tempest_plugin/config.py
index 5103888..f379859 100644
--- a/patrole_tempest_plugin/config.py
+++ b/patrole_tempest_plugin/config.py
@@ -41,13 +41,13 @@
default=False,
help="""
This option determines whether Patrole should run against a
-`custom_requirements_file` which defines RBAC requirements. The
+``custom_requirements_file`` which defines RBAC requirements. The
purpose of setting this flag to True is to verify that RBAC policy
is in accordance to requirements. The idea is that the
-`custom_requirements_file` perfectly defines what the RBAC requirements are.
+``custom_requirements_file`` perfectly defines what the RBAC requirements are.
Here are the possible outcomes when running the Patrole tests against
-a `custom_requirements_file`:
+a ``custom_requirements_file``:
YAML definition: allowed
test run: allowed
@@ -55,7 +55,7 @@
YAML definition: allowed
test run: not allowed
-test result: fail (under-permission)
+test result: fail (under-permission, e.g. Forbidden exception)
YAML definition: not allowed
test run: allowed
@@ -63,30 +63,32 @@
"""),
cfg.StrOpt('custom_requirements_file',
help="""
-File path of the yaml file that defines your RBAC requirements. This
-file must be located on the same host that Patrole runs on. The yaml
+File path of the YAML file that defines your RBAC requirements. This
+file must be located on the same host that Patrole runs on. The YAML
file should be written as follows:
```
-<service>:
- <api_action>:
- - <allowed_role>
- - <allowed_role>
- - <allowed_role>
- <api_action>:
- - <allowed_role>
- - <allowed_role>
-<service>
- <api_action>:
- - <allowed_role>
+<service_foo>:
+ <api_action_x>:
+ - <allowed_role_a>
+ - <allowed_role_b>
+ - <allowed_role_c>
+ <api_action_y>:
+ - <allowed_role_d>
+ - <allowed_role_e>
+<service_bar>:
+ <api_action_z>:
+ - <allowed_role_b>
```
+
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:
- volume:create
- os_compute_api:servers:start
- add_image
-allowed_role = the Keystone role that is allowed to perform the API
+allowed_role = the Keystone role that is allowed to perform the API.
""")
]
diff --git a/patrole_tempest_plugin/policy_authority.py b/patrole_tempest_plugin/policy_authority.py
index a22e00a..b813f88 100644
--- a/patrole_tempest_plugin/policy_authority.py
+++ b/patrole_tempest_plugin/policy_authority.py
@@ -24,8 +24,8 @@
from tempest.common import credentials_factory as credentials
from tempest import config
+from patrole_tempest_plugin.rbac_authority import RbacAuthority
from patrole_tempest_plugin import rbac_exceptions
-from patrole_tempest_plugin.rbac_utils import RbacAuthority
CONF = config.CONF
LOG = logging.getLogger(__name__)
@@ -274,9 +274,9 @@
access_data['is_admin'] = is_admin
# TODO(felipemonteiro): Dynamically calculate is_admin_project rather
# than hard-coding it to True. is_admin_project cannot be determined
- # from the role, but rather from project and domain names. See
- # _populate_is_admin_project in keystone.token.providers.common
- # for more information.
+ # from the role, but rather from project and domain names. For more
+ # information, see:
+ # https://github.com/openstack/keystone/blob/37ce5417418f8acbd27f3dacb70c605b0fe48301/keystone/token/providers/common.py#L150
access_data['is_admin_project'] = True
class Object(object):
diff --git a/patrole_tempest_plugin/rbac_authority.py b/patrole_tempest_plugin/rbac_authority.py
new file mode 100644
index 0000000..294ecc5
--- /dev/null
+++ b/patrole_tempest_plugin/rbac_authority.py
@@ -0,0 +1,38 @@
+# Copyright 2017 AT&T Corporation.
+# All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# 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 abc
+
+import six
+
+
+@six.add_metaclass(abc.ABCMeta)
+class RbacAuthority(object):
+ """Class for validating whether a given role can perform a policy action.
+
+ Any class that extends ``RbacAuthority`` provides the logic for determining
+ whether a role has permissions to execute a policy action.
+ """
+
+ @abc.abstractmethod
+ def allowed(self, rule, role):
+ """Determine whether the role should be able to perform the API.
+
+ :param rule: The name of the policy enforced by the API.
+ :param role: The role used to determine whether ``rule`` can be
+ executed.
+ :returns: True if the ``role`` has permissions to execute
+ ``rule``, else False.
+ """
diff --git a/patrole_tempest_plugin/rbac_exceptions.py b/patrole_tempest_plugin/rbac_exceptions.py
index 5ee65ae..e75b8ec 100644
--- a/patrole_tempest_plugin/rbac_exceptions.py
+++ b/patrole_tempest_plugin/rbac_exceptions.py
@@ -38,7 +38,7 @@
class RbacResourceSetupFailed(exceptions.TempestException):
- message = "Rbac resource setup failed"
+ message = "RBAC resource setup failed"
class RbacOverPermission(exceptions.TempestException):
diff --git a/patrole_tempest_plugin/rbac_rule_validation.py b/patrole_tempest_plugin/rbac_rule_validation.py
index d3213cf..69a43ea 100644
--- a/patrole_tempest_plugin/rbac_rule_validation.py
+++ b/patrole_tempest_plugin/rbac_rule_validation.py
@@ -17,6 +17,7 @@
import logging
import sys
+from oslo_log import versionutils
from oslo_utils import excutils
import six
@@ -36,7 +37,8 @@
RBACLOG = logging.getLogger('rbac_reporting')
-def action(service, rule='', expected_error_code=403, extra_target_data=None):
+def action(service, rule='', rules=None, expected_error_code=403,
+ extra_target_data=None):
"""A decorator for verifying OpenStack policy enforcement.
A decorator which allows for positive and negative RBAC testing. Given:
@@ -67,15 +69,18 @@
As such, negative and positive testing can be applied using this decorator.
- :param service: An OpenStack service. Examples: "nova" or "neutron".
- :param rule: A policy action defined in a policy.json file (or in
- code).
+ :param str service: An OpenStack service. Examples: "nova" or "neutron".
+ :param str rule: (DEPRECATED) A policy action defined in a policy.json file
+ or in code.
+ :param list rules: A list of policy actions defined in a policy.json file
+ or in code. The rules are logical-ANDed together to derive the expected
+ result.
.. note::
Patrole currently only supports custom JSON policy files.
- :param expected_error_code: Overrides default value of 403 (Forbidden)
+ :param int expected_error_code: Overrides default value of 403 (Forbidden)
with endpoint-specific error code. Currently only supports 403 and 404.
Support for 404 is needed because some services, like Neutron,
intentionally throw a 404 for security reasons.
@@ -85,11 +90,11 @@
A 404 should not be provided *unless* the endpoint masks a
``Forbidden`` exception as a ``NotFound`` exception.
- :param extra_target_data: Dictionary, keyed with ``oslo.policy`` generic
- check names, whose values are string literals that reference nested
- ``tempest.test.BaseTestCase`` attributes. Used by ``oslo.policy`` for
- performing matching against attributes that are sent along with the API
- calls. Example::
+ :param dict extra_target_data: Dictionary, keyed with ``oslo.policy``
+ generic check names, whose values are string literals that reference
+ nested ``tempest.test.BaseTestCase`` attributes. Used by
+ ``oslo.policy`` for performing matching against attributes that are
+ sent along with the API calls. Example::
extra_target_data={
"target.token.user_id":
@@ -113,6 +118,8 @@
if extra_target_data is None:
extra_target_data = {}
+ rules = _prepare_rules(rule, rules)
+
def decorator(test_func):
role = CONF.patrole.rbac_test_role
@@ -125,8 +132,14 @@
'`rbac_rule_validation` decorator can only be applied to '
'an instance of `tempest.test.BaseTestCase`.')
- allowed = _is_authorized(test_obj, service, rule,
- extra_target_data)
+ allowed = True
+ disallowed_rules = []
+ for rule in rules:
+ _allowed = _is_authorized(
+ test_obj, service, rule, extra_target_data)
+ if not _allowed:
+ disallowed_rules.append(rule)
+ allowed = allowed and _allowed
expected_exception, irregular_msg = _get_exception_type(
expected_error_code)
@@ -148,8 +161,12 @@
if irregular_msg:
LOG.warning(irregular_msg.format(rule, service))
if allowed:
- msg = ("Role %s was not allowed to perform %s." %
- (role, rule))
+ msg = ("Role %s was not allowed to perform the following "
+ "actions: %s. Expected allowed actions: %s. "
+ "Expected disallowed actions: %s." % (
+ role, sorted(rules),
+ sorted(set(rules) - set(disallowed_rules)),
+ sorted(disallowed_rules)))
LOG.error(msg)
raise exceptions.Forbidden(
"%s Exception was: %s" % (msg, e))
@@ -164,10 +181,14 @@
LOG.error(msg)
else:
if not allowed:
- LOG.error("Role %s was allowed to perform %s", role, rule)
- raise rbac_exceptions.RbacOverPermission(
- "OverPermission: Role %s was allowed to perform %s" %
- (role, rule))
+ msg = (
+ "OverPermission: Role %s was allowed to perform the "
+ "following disallowed actions: %s" % (
+ role, sorted(disallowed_rules)
+ )
+ )
+ LOG.error(msg)
+ raise rbac_exceptions.RbacOverPermission(msg)
finally:
if CONF.patrole_log.enable_reporting:
RBACLOG.info(
@@ -181,6 +202,23 @@
return decorator
+def _prepare_rules(rule, rules):
+ if rules is None:
+ rules = []
+ elif not isinstance(rules, (tuple, list)):
+ rules = [rules]
+ if rule:
+ deprecation_msg = (
+ "The `rule` argument has been deprecated in favor of `rules` "
+ "and will be removed in a future version.")
+ versionutils.report_deprecated_feature(LOG, deprecation_msg)
+ if rules:
+ LOG.debug("The `rules` argument will be used instead of `rule`.")
+ else:
+ rules.append(rule)
+ return rules
+
+
def _is_authorized(test_obj, service, rule, extra_target_data):
"""Validates whether current RBAC role has permission to do policy action.
diff --git a/patrole_tempest_plugin/rbac_utils.py b/patrole_tempest_plugin/rbac_utils.py
index 347f77f..2ef88ca 100644
--- a/patrole_tempest_plugin/rbac_utils.py
+++ b/patrole_tempest_plugin/rbac_utils.py
@@ -13,9 +13,7 @@
# License for the specific language governing permissions and limitations
# under the License.
-import abc
from contextlib import contextmanager
-import six
import time
from oslo_log import log as logging
@@ -229,23 +227,3 @@
:returns: True if ``rbac_test_role`` is the admin role.
"""
return CONF.patrole.rbac_test_role == CONF.identity.admin_role
-
-
-@six.add_metaclass(abc.ABCMeta)
-class RbacAuthority(object):
- """Class for validating whether a given role can perform a policy action.
-
- Any class that extends ``RbacAuthority`` provides the logic for determining
- whether a role has permissions to execute a policy action.
- """
-
- @abc.abstractmethod
- def allowed(self, rule, role):
- """Determine whether the role should be able to perform the API.
-
- :param rule: The name of the policy enforced by the API.
- :param role: The role used to determine whether ``rule`` can be
- executed.
- :returns: True if the ``role`` has permissions to execute
- ``rule``, else False.
- """
diff --git a/patrole_tempest_plugin/requirements_authority.py b/patrole_tempest_plugin/requirements_authority.py
index 6dd75eb..75df9f4 100644
--- a/patrole_tempest_plugin/requirements_authority.py
+++ b/patrole_tempest_plugin/requirements_authority.py
@@ -19,7 +19,7 @@
from tempest import config
from tempest.lib import exceptions
-from patrole_tempest_plugin.rbac_utils import RbacAuthority
+from patrole_tempest_plugin.rbac_authority import RbacAuthority
CONF = config.CONF
LOG = logging.getLogger(__name__)
diff --git a/patrole_tempest_plugin/tests/api/compute/test_server_misc_policy_actions_rbac.py b/patrole_tempest_plugin/tests/api/compute/test_server_misc_policy_actions_rbac.py
index 63dee85..f6dfcca 100644
--- a/patrole_tempest_plugin/tests/api/compute/test_server_misc_policy_actions_rbac.py
+++ b/patrole_tempest_plugin/tests/api/compute/test_server_misc_policy_actions_rbac.py
@@ -395,7 +395,8 @@
@rbac_rule_validation.action(
service="nova",
- rule="os_compute_api:os-lock-server:unlock:unlock_override")
+ rules=["os_compute_api:os-lock-server:unlock",
+ "os_compute_api:os-lock-server:unlock:unlock_override"])
@decorators.idempotent_id('40dfeef9-73ee-48a9-be19-a219875de457')
def test_unlock_server_override(self):
"""Test force unlock server, part of os-lock-server.
@@ -728,41 +729,3 @@
with self.rbac_utils.override_role(self):
self.servers_client.add_fixed_ip(self.server['id'],
networkId=network_id)
-
-
-class VirtualInterfacesRbacTest(rbac_base.BaseV2ComputeRbacTest):
- # The compute os-virtual-interfaces API is deprecated from the Microversion
- # 2.44 onward. For more information, see:
- # https://developer.openstack.org/api-ref/compute/#servers-virtual-interfaces-servers-os-virtual-interfaces-deprecated
- max_microversion = '2.43'
-
- @classmethod
- def setup_credentials(cls):
- # This test needs a network and a subnet
- cls.set_network_resources(network=True, subnet=True)
- super(VirtualInterfacesRbacTest, cls).setup_credentials()
-
- @classmethod
- def resource_setup(cls):
- super(VirtualInterfacesRbacTest, cls).resource_setup()
- cls.server = cls.create_test_server(wait_until='ACTIVE')
-
- @rbac_rule_validation.action(
- service="nova",
- rule="os_compute_api:os-virtual-interfaces")
- @decorators.idempotent_id('fc719ae3-0f73-4689-8378-1b841f0f2818')
- def test_list_virtual_interfaces(self):
- """Test list virtual interfaces, part of os-virtual-interfaces.
-
- If Neutron is available, then call the API and expect it to fail
- with a 400 BadRequest (policy enforcement is done before that happens).
- """
- with self.rbac_utils.override_role(self):
- if CONF.service_available.neutron:
- msg = ("Listing virtual interfaces is not supported by this "
- "cloud.")
- with self.assertRaisesRegex(lib_exc.BadRequest, msg):
- self.servers_client.list_virtual_interfaces(
- self.server['id'])
- else:
- self.servers_client.list_virtual_interfaces(self.server['id'])
diff --git a/patrole_tempest_plugin/tests/api/compute/test_virtual_interfaces_rbac.py b/patrole_tempest_plugin/tests/api/compute/test_virtual_interfaces_rbac.py
new file mode 100644
index 0000000..6edb8d9
--- /dev/null
+++ b/patrole_tempest_plugin/tests/api/compute/test_virtual_interfaces_rbac.py
@@ -0,0 +1,61 @@
+# Copyright 2017 AT&T Corporation.
+# All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+from tempest import config
+from tempest.lib import decorators
+from tempest.lib import exceptions as lib_exc
+
+from patrole_tempest_plugin import rbac_rule_validation
+from patrole_tempest_plugin.tests.api.compute import rbac_base
+
+CONF = config.CONF
+
+
+class VirtualInterfacesRbacTest(rbac_base.BaseV2ComputeRbacTest):
+ # The compute os-virtual-interfaces API is deprecated from the Microversion
+ # 2.44 onward. For more information, see:
+ # https://developer.openstack.org/api-ref/compute/#servers-virtual-interfaces-servers-os-virtual-interfaces-deprecated
+ max_microversion = '2.43'
+
+ @classmethod
+ def setup_credentials(cls):
+ # This test needs a network and a subnet
+ cls.set_network_resources(network=True, subnet=True)
+ super(VirtualInterfacesRbacTest, cls).setup_credentials()
+
+ @classmethod
+ def resource_setup(cls):
+ super(VirtualInterfacesRbacTest, cls).resource_setup()
+ cls.server = cls.create_test_server(wait_until='ACTIVE')
+
+ @rbac_rule_validation.action(
+ service="nova",
+ rule="os_compute_api:os-virtual-interfaces")
+ @decorators.idempotent_id('fc719ae3-0f73-4689-8378-1b841f0f2818')
+ def test_list_virtual_interfaces(self):
+ """Test list virtual interfaces, part of os-virtual-interfaces.
+
+ If Neutron is available, then call the API and expect it to fail
+ with a 400 BadRequest (policy enforcement is done before that happens).
+ """
+ with self.rbac_utils.override_role(self):
+ if CONF.service_available.neutron:
+ msg = ("Listing virtual interfaces is not supported by this "
+ "cloud.")
+ with self.assertRaisesRegex(lib_exc.BadRequest, msg):
+ self.servers_client.list_virtual_interfaces(
+ self.server['id'])
+ else:
+ self.servers_client.list_virtual_interfaces(self.server['id'])
diff --git a/patrole_tempest_plugin/tests/api/volume/test_volume_basic_crud_rbac.py b/patrole_tempest_plugin/tests/api/volume/test_volume_basic_crud_rbac.py
index 1bd87d2..61532c6 100644
--- a/patrole_tempest_plugin/tests/api/volume/test_volume_basic_crud_rbac.py
+++ b/patrole_tempest_plugin/tests/api/volume/test_volume_basic_crud_rbac.py
@@ -62,11 +62,3 @@
with self.rbac_utils.override_role(self):
self.volumes_client.update_volume(self.volume['id'],
name=update_name)
-
- @rbac_rule_validation.action(
- service="cinder",
- rule="volume_extension:volume_image_metadata")
- @decorators.idempotent_id('3d48ca91-f02b-4616-a69d-4a8b296c8529')
- def test_volume_list_image_metadata(self):
- with self.rbac_utils.override_role(self):
- self.volumes_client.list_volumes(detail=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 85547e1..0a4c44b 100644
--- a/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
+++ b/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
@@ -13,6 +13,7 @@
# under the License.
import mock
+from oslo_config import cfg
from tempest.lib import exceptions
from tempest import manager
@@ -24,11 +25,13 @@
from patrole_tempest_plugin import rbac_utils
from patrole_tempest_plugin.tests.unit import fixtures
+CONF = cfg.CONF
-class RBACRuleValidationTest(base.TestCase):
+
+class BaseRBACRuleValidationTest(base.TestCase):
def setUp(self):
- super(RBACRuleValidationTest, self).setUp()
+ super(BaseRBACRuleValidationTest, self).setUp()
self.mock_test_args = mock.Mock(spec=test.BaseTestCase)
self.mock_test_args.os_primary = mock.Mock(spec=manager.Manager)
self.mock_test_args.rbac_utils = mock.Mock(
@@ -45,6 +48,12 @@
self.useFixture(
fixtures.ConfPatcher(enable_reporting=False, group='patrole_log'))
+
+class RBACRuleValidationTest(BaseRBACRuleValidationTest):
+ """Test suite for validating fundamental functionality for the
+ ``rbac_rule_validation`` decorator.
+ """
+
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
@mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
def test_rule_validation_have_permission_no_exc(self, mock_authority,
@@ -98,11 +107,11 @@
def test_policy(*args):
raise exceptions.Forbidden()
- test_re = "Role Member was not allowed to perform sentinel.action."
+ test_re = ("Role Member was not allowed to perform the following "
+ "actions: \[%s\].*" % (mock.sentinel.action))
self.assertRaisesRegex(exceptions.Forbidden, test_re, test_policy,
self.mock_test_args)
- mock_log.error.assert_called_once_with("Role Member was not allowed to"
- " perform sentinel.action.")
+ self.assertRegex(mock_log.error.mock_calls[0][1][0], test_re)
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
@mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
@@ -138,11 +147,11 @@
def test_policy(*args):
raise rbac_exceptions.RbacMalformedResponse()
- test_re = "Role Member was not allowed to perform sentinel.action."
+ test_re = ("Role Member was not allowed to perform the following "
+ "actions: \[%s\].*" % (mock.sentinel.action))
self.assertRaisesRegex(exceptions.Forbidden, test_re, test_policy,
self.mock_test_args)
- mock_log.error.assert_called_once_with("Role Member was not allowed to"
- " perform sentinel.action.")
+ self.assertRegex(mock_log.error.mock_calls[0][1][0], test_re)
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
@mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
@@ -179,11 +188,11 @@
def test_policy(*args):
raise rbac_exceptions.RbacConflictingPolicies()
- test_re = "Role Member was not allowed to perform sentinel.action."
+ test_re = ("Role Member was not allowed to perform the following "
+ "actions: \[%s\].*" % (mock.sentinel.action))
self.assertRaisesRegex(exceptions.Forbidden, test_re, test_policy,
self.mock_test_args)
- mock_log.error.assert_called_once_with("Role Member was not allowed to"
- " perform sentinel.action.")
+ self.assertRegex(mock_log.error.mock_calls[0][1][0], test_re)
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
@mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
@@ -233,25 +242,26 @@
raise exceptions.NotFound()
expected_errors = [
- "Role Member was not allowed to perform sentinel.action.", None
+ ("Role Member was not allowed to perform the following "
+ "actions: \[%s\].*" % (mock.sentinel.action)),
+ None
]
for pos, allowed in enumerate([True, False]):
mock_authority.PolicyAuthority.return_value.allowed\
.return_value = allowed
- expected_error = expected_errors[pos]
+ error_re = expected_errors[pos]
- if expected_error:
- self.assertRaisesRegex(
- exceptions.Forbidden, '.* ' + expected_error, test_policy,
- self.mock_test_args)
- mock_log.error.assert_called_once_with(expected_error)
+ if error_re:
+ self.assertRaisesRegex(exceptions.Forbidden, error_re,
+ test_policy, self.mock_test_args)
+ self.assertRegex(mock_log.error.mock_calls[0][1][0], error_re)
else:
test_policy(self.mock_test_args)
mock_log.error.assert_not_called()
- mock_log.warning.assert_called_once_with(
+ mock_log.warning.assert_called_with(
"NotFound exception was caught for policy action {0}. The "
"service {1} throws a 404 instead of a 403, which is "
"irregular.".format(mock.sentinel.action,
@@ -284,13 +294,10 @@
for test_policy in (
test_policy_expect_forbidden, test_policy_expect_not_found):
- error_re = (".* OverPermission: Role Member was allowed to perform"
- " sentinel.action")
+ error_re = ".*OverPermission: .* \[%s\]$" % mock.sentinel.action
self.assertRaisesRegex(rbac_exceptions.RbacOverPermission,
error_re, test_policy, self.mock_test_args)
- mock_log.error.assert_called_once_with(
- 'Role %s was allowed to perform %s', 'Member',
- mock.sentinel.action)
+ self.assertRegex(mock_log.error.mock_calls[0][1][0], error_re)
mock_log.error.reset_mock()
@mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
@@ -405,3 +412,119 @@
mock.sentinel.action,
"Allowed",
"Allowed")
+
+
+class RBACRuleValidationTestMultiPolicy(BaseRBACRuleValidationTest):
+ """Test suite for validating multi-policy support for the
+ ``rbac_rule_validation`` decorator.
+ """
+
+ def _assert_policy_authority_called_with(self, rules, mock_authority):
+ m_authority = mock_authority.PolicyAuthority.return_value
+ m_authority.allowed.assert_has_calls([
+ mock.call(rule, CONF.patrole.rbac_test_role) for rule in rules
+ ])
+
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+ def test_rule_validation_multi_policy_have_permission_success(
+ self, mock_authority):
+ """Test that when expected result is authorized and test passes that
+ the overall evaluation succeeds.
+ """
+ mock_authority.PolicyAuthority.return_value.allowed.\
+ return_value = True
+
+ rules = [mock.sentinel.action1, mock.sentinel.action2]
+
+ @rbac_rv.action(mock.sentinel.service, rules=rules)
+ def test_policy(*args):
+ pass
+
+ test_policy(self.mock_test_args)
+ self._assert_policy_authority_called_with(rules, mock_authority)
+
+ @mock.patch.object(rbac_rv, 'LOG', autospec=True)
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+ def test_rule_validation_multi_policy_overpermission_failure(
+ self, mock_authority, mock_log):
+ """Test that when expected result is unauthorized and test passes that
+ the overall evaluation results in an OverPermission getting raised.
+ """
+
+ rules = [
+ mock.sentinel.action1, mock.sentinel.action2, mock.sentinel.action3
+ ]
+
+ @rbac_rv.action(mock.sentinel.service, rules=rules)
+ def test_policy(*args):
+ pass
+
+ def _do_test(allowed_list, fail_on_action):
+ mock_authority.PolicyAuthority.return_value.allowed.side_effect = (
+ allowed_list)
+
+ error_re = ".*OverPermission: .* \[%s\]$" % fail_on_action
+ self.assertRaisesRegex(rbac_exceptions.RbacOverPermission,
+ error_re, test_policy, self.mock_test_args)
+ self.assertRegex(mock_log.error.mock_calls[0][1][0], error_re)
+ mock_log.error.reset_mock()
+ self._assert_policy_authority_called_with(rules, mock_authority)
+
+ _do_test([True, True, False], mock.sentinel.action3)
+ _do_test([False, True, True], mock.sentinel.action1)
+ _do_test([True, False, True], mock.sentinel.action2)
+
+ @mock.patch.object(rbac_rv, 'LOG', autospec=True)
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+ def test_rule_validation_multi_policy_forbidden_success(
+ self, mock_authority, mock_log):
+ """Test that when the expected result is unauthorized and the test
+ fails that the overall evaluation results in success.
+ """
+
+ rules = [
+ mock.sentinel.action1, mock.sentinel.action2, mock.sentinel.action3
+ ]
+
+ @rbac_rv.action(mock.sentinel.service, rules=rules)
+ def test_policy(*args):
+ raise exceptions.Forbidden()
+
+ def _do_test(allowed_list):
+ mock_authority.PolicyAuthority.return_value.allowed.\
+ side_effect = allowed_list
+ test_policy(self.mock_test_args)
+ mock_log.error.assert_not_called()
+ self._assert_policy_authority_called_with(rules, mock_authority)
+
+ _do_test([True, True, False])
+ _do_test([False, True, True])
+ _do_test([True, False, True])
+
+ @mock.patch.object(rbac_rv, 'LOG', autospec=True)
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+ def test_rule_validation_multi_policy_forbidden_failure(
+ self, mock_authority, mock_log):
+ """Test that when the expected result is authorized and the test
+ fails (with a Forbidden error code) that the overall evaluation
+ results a Forbidden getting raised.
+ """
+
+ # NOTE: Avoid mock.sentinel here due to weird sorting with them.
+ rules = ['action1', 'action2', 'action3']
+
+ @rbac_rv.action(mock.sentinel.service, rules=rules)
+ def test_policy(*args):
+ raise exceptions.Forbidden()
+
+ mock_authority.PolicyAuthority.return_value.allowed\
+ .return_value = True
+
+ error_re = ("Role Member was not allowed to perform the following "
+ "actions: %s. Expected allowed actions: %s. Expected "
+ "disallowed actions: []." % (rules, rules)).replace(
+ '[', '\[').replace(']', '\]')
+ self.assertRaisesRegex(exceptions.Forbidden, error_re, test_policy,
+ self.mock_test_args)
+ self.assertRegex(mock_log.error.mock_calls[0][1][0], error_re)
+ self._assert_policy_authority_called_with(rules, mock_authority)
diff --git a/releasenotes/notes/multi-policy-support-4e5c8b4e9e25ad9d.yaml b/releasenotes/notes/multi-policy-support-4e5c8b4e9e25ad9d.yaml
new file mode 100644
index 0000000..3d192d9
--- /dev/null
+++ b/releasenotes/notes/multi-policy-support-4e5c8b4e9e25ad9d.yaml
@@ -0,0 +1,13 @@
+---
+features:
+ - |
+ Patrole now offers support for multiple policies. The ``rules`` argument
+ has been added to the ``rbac_rule_validation.action`` decorator, which
+ takes a list of policy names which Patrole will use to determine the
+ expected test result. This allows Patrole to more accurately determine
+ whether RBAC is configured correctly, since some API endpoints enforce
+ multiple policies.
+deprecations:
+ - |
+ The ``rule`` argument in the ``rbac_rule_validation.action`` decorator has
+ been deprecated in favor of ``rules``.