Merge "Fix Neutron-related return values on some list APIs"
diff --git a/.zuul.yaml b/.zuul.yaml
index 1eab464..60f0d05 100644
--- a/.zuul.yaml
+++ b/.zuul.yaml
@@ -123,7 +123,3 @@
jobs:
- patrole-admin
- patrole-member
- - patrole-member-queens
- - patrole-member-pike
- - patrole-py35-member
- - openstack-tox-lower-constraints
diff --git a/README.rst b/README.rst
index 0c786b9..fb8976f 100644
--- a/README.rst
+++ b/README.rst
@@ -33,10 +33,22 @@
* *Atomicity*. Patrole tests should be atomic: they should test policies in
isolation. Unlike Tempest, a Patrole test strives to only call a single
endpoint at a time.
-* *Holistic coverage*. Patrole strives for complete coverage of the OpenStack
- API. Additionally, Patrole strives to test the API-to-policy mapping
- contained in each project's policy in code documentation.
-* *Self-contained*. Patrole should attempt to clean up after itself; whenever
+* *Complete coverage*. Patrole should validate all policy in code defaults. For
+ testing, Patrole uses the API-to-policy mapping contained in each project's
+ `policy in code`_ documentation where applicable.
+
+ For example, Nova's policy in code documentation is located in the
+ `Nova repository`_ under ``nova/policies``. Likewise, Keystone's policy in
+ code documentation is located in the `Keystone repository`_ under
+ ``keystone/common/policies``. The other OpenStack services follow the same
+ directory layout pattern with respect to policy in code.
+
+ .. note::
+
+ Realistically this is not always possible because some services have
+ not yet moved to policy in code.
+
+* *Self-cleaning*. Patrole should attempt to clean up after itself; whenever
possible we should tear down resources when done.
.. note::
@@ -45,7 +57,11 @@
pre-provisioned credentials. Work is currently underway to clean up
modifications made to pre-provisioned credentials.
-* *Self-tested*. Patrole should be self-tested.
+* *Self-testing*. Patrole should be self-testing.
+
+.. _policy in code: https://specs.openstack.org/openstack/oslo-specs/specs/newton/policy-in-code.html
+.. _Nova repository: https://github.com/openstack/nova/tree/master/nova/policies
+.. _Keystone repository: https://github.com/openstack/keystone/tree/master/keystone/common/policies
Features
--------
diff --git a/doc/source/HACKING.rst b/doc/source/HACKING.rst
index 1847447..8777875 100644
--- a/doc/source/HACKING.rst
+++ b/doc/source/HACKING.rst
@@ -1,4 +1,5 @@
-=======
-Hacking
-=======
+====================
+Patrole Coding Guide
+====================
+
.. include:: ../../HACKING.rst
diff --git a/doc/source/framework/rbac_utils.rst b/doc/source/framework/rbac_utils.rst
index 69ba045..7143928 100644
--- a/doc/source/framework/rbac_utils.rst
+++ b/doc/source/framework/rbac_utils.rst
@@ -23,8 +23,19 @@
and test execution, respectively. This is especially true when considering
custom policy rule definitions, which can be arbitrarily complex.
-Patrole, therefore, implicitly splits up each test into 3 stages: set up,
-test execution, and teardown.
+.. _role-overriding:
+
+Role Overriding
+^^^^^^^^^^^^^^^
+
+Role overriding is the way Patrole is able to create resources and delete
+resources -- including those that require admin credentials -- while still
+being able to exercise the same set of Tempest credentials to perform the API
+action that authorizes the policy under test, by manipulating the role of
+the Tempest credentials.
+
+Patrole implicitly splits up each test into 3 stages: set up, test execution,
+and teardown.
The role workflow is as follows:
@@ -43,7 +54,7 @@
Test Setup
----------
-Automatic role switch in background.
+Automatic role override in background.
Resources can be set up inside the ``resource_setup`` class method that Tempest
provides. These resources are typically reserved for "expensive" resources
@@ -59,7 +70,7 @@
Test Execution
--------------
-Manual role switch required.
+Manual role override required.
"Test execution" here means calling the API endpoint that enforces the policy
action expected by the ``rbac_rule_validation`` decorator. Test execution
@@ -152,7 +163,7 @@
Test Cleanup
------------
-Automatic role switch in background.
+Automatic role override in background.
After the test -- no matter whether it ended successfully or in failure --
the credentials are overridden with the admin role by the Patrole framework,
diff --git a/doc/source/index.rst b/doc/source/index.rst
index 8368262..255fd9a 100644
--- a/doc/source/index.rst
+++ b/doc/source/index.rst
@@ -2,6 +2,14 @@
Patrole: Tempest Plugin for RBAC Testing
========================================
+Overview
+========
+
+.. toctree::
+ :maxdepth: 2
+
+ overview
+
User's Guide
============
@@ -12,7 +20,6 @@
:maxdepth: 2
configuration
- usage
Patrole Installation Guide
--------------------------
diff --git a/doc/source/overview.rst b/doc/source/overview.rst
new file mode 100644
index 0000000..795359e
--- /dev/null
+++ b/doc/source/overview.rst
@@ -0,0 +1,178 @@
+========================
+Team and repository tags
+========================
+
+.. image:: https://governance.openstack.org/tc/badges/patrole.svg
+ :target: https://governance.openstack.org/tc/reference/tags/index.html
+
+Patrole - The OpenStack RBAC Validation Test Suite
+==================================================
+
+The documentation for Patrole is officially hosted at:
+https://docs.openstack.org/patrole/latest/
+
+This is a set of integration tests to be run against a live OpenStack
+cluster. Patrole has a battery of tests dedicated to validating the correctness
+and security of the cloud's RBAC implementation.
+
+Design Principles
+-----------------
+
+As a `Tempest plugin`_, Patrole borrows some `design principles`_ from Tempest,
+but not all, as its testing scope is confined to policies.
+
+* Patrole uses OpenStack public interfaces. Tests in Patrole should only touch
+ public OpenStack APIs.
+* Patrole tests should be atomic: they should test policies in isolation.
+ Unlike Tempest, a Patrole test strives to only call a single endpoint at a
+ time. This is because it is important to validate each policy is authorized
+ correctly and the best way to do that is to validate the policy alone.
+* Patrole should validate all policy in code defaults. For testing, Patrole
+ uses the API-to-policy mapping contained in each project's `policy in code`_
+ documentation where applicable.
+
+ For example, Nova's policy in code documentation is located in the
+ `Nova repository`_ under ``nova/policies``. Likewise, Keystone's policy in
+ code documentation is located in the `Keystone repository`_ under
+ ``keystone/common/policies``. The other OpenStack services follow the same
+ directory layout pattern with respect to policy in code.
+
+ .. note::
+
+ Realistically this is not always possible because some services have
+ not yet moved to policy in code.
+
+* Patrole should attempt to clean up after itself; whenever possible it should
+ tear down resources when done.
+
+ .. note::
+
+ Patrole modifies roles dynamically in the background, which affects
+ pre-provisioned credentials. Work is currently underway to clean up
+ modifications made to pre-provisioned credentials.
+
+* Patrole should be self-testing.
+
+.. _Tempest plugin: https://docs.openstack.org/tempest/latest/plugin.html
+.. _design principles: https://docs.openstack.org/tempest/latest/overview.html#design-principles
+.. _policy in code: https://specs.openstack.org/openstack/oslo-specs/specs/newton/policy-in-code.html
+.. _Nova repository: https://github.com/openstack/nova/tree/master/nova/policies
+.. _Keystone repository: https://github.com/openstack/keystone/tree/master/keystone/common/policies
+
+Quickstart
+----------
+
+To run Patrole, you must first have `Tempest`_ installed and configured
+properly. Please reference Tempest's `Quickstart`_ guide to do so. Follow all
+the steps outlined therein. Afterward, proceed with the steps below.
+
+#. You first need to install Patrole. This is done with pip after you check out
+ the Patrole repo::
+
+ $ git clone https://git.openstack.org/openstack/patrole
+ $ pip install patrole/
+
+ This can be done within a venv.
+
+ .. note::
+
+ You may also install Patrole from source code by running::
+
+ pip install -e patrole/
+
+#. Next you must properly configure Patrole, which is relatively
+ straightforward. For details on configuring Patrole refer to the
+ :ref:`patrole-configuration`.
+
+#. Once the configuration is done you're now ready to run Patrole. This can
+ be done using the `tempest_run`_ command. This can be done by running::
+
+ $ tempest run --regex '^patrole_tempest_plugin\.tests\.api'
+
+ There is also the option to use testr directly, or any `testr`_ based test
+ runner, like `ostestr`_. For example, from the workspace dir run::
+
+ $ stestr --regex '(?!.*\[.*\bslow\b.*\])(^patrole_tempest_plugin\.tests\.api))'
+
+ will run the same set of tests as the default gate jobs.
+
+ You can also run Patrole tests using `tox`_. To do so, ``cd`` into the
+ **Tempest** directory and run::
+
+ $ tox -eall-plugin -- patrole_tempest_plugin.tests.api
+
+ .. note::
+
+ It is possible to run Patrole via ``tox -eall`` in order to run Patrole
+ isolated from other plugins. This can be accomplished by including the
+ installation of services that currently use policy in code -- for example,
+ Nova and Keystone. For example::
+
+ $ tox -evenv-tempest -- pip install /opt/stack/patrole /opt/stack/keystone /opt/stack/nova
+ $ tox -eall -- patrole_tempest_plugin.tests.api
+
+#. Log information from tests is captured in ``tempest.log`` under the Tempest
+ repository. Some Patrole debugging information is captured in that log
+ related to expected test results and :ref:`role-overriding`.
+
+ More detailed RBAC testing log output is emitted to ``patrole.log`` under
+ the Patrole repository. To configure Patrole's logging, see the
+ :ref:`patrole-configuration` guide.
+
+.. _Tempest: https://github.com/openstack/tempest
+.. _Quickstart: https://docs.openstack.org/tempest/latest/overview.html#quickstart
+.. _tempest_run: https://docs.openstack.org/tempest/latest/run.html
+.. _testr: https://testrepository.readthedocs.org/en/latest/MANUAL.html
+.. _ostestr: https://docs.openstack.org/os-testr/latest/
+.. _tox: https://tox.readthedocs.io/en/latest/
+
+RBAC Tests
+----------
+
+To change the role that the patrole tests are being run as, edit
+``rbac_test_role`` in the ``patrole`` section of tempest.conf: ::
+
+ [patrole]
+ rbac_test_role = member
+ ...
+
+.. note::
+
+ The ``rbac_test_role`` is service-specific. member, for example,
+ is an arbitrary role, but by convention is used to designate the default
+ non-admin role in the system. Most Patrole tests should be run with
+ **admin** and **member** roles. However, other services may use entirely
+ different roles.
+
+For more information about the member role and its nomenclature,
+please see: `<https://ask.openstack.org/en/question/4759/member-vs-_member_/>`__.
+
+Unit Tests
+----------
+
+Patrole also has a set of unit tests which test the Patrole code itself. These
+tests can be run by specifying the test discovery path::
+
+ $ stestr --test-path ./patrole_tempest_plugin/tests/unit run
+
+By setting ``--test-path`` option to ``./patrole_tempest_plugin/tests/unit``
+it specifies that test discovery should only be run on the unit test directory.
+
+Alternatively, there are the py27 and py35 tox jobs which will run the unit
+tests with the corresponding version of Python.
+
+One common activity is to just run a single test; you can do this with tox
+simply by specifying to just run py27 or py35 tests against a single test::
+
+ $ tox -e py27 -- -n patrole_tempest_plugin.tests.unit.test_rbac_utils.RBACUtilsTest.test_override_role_with_missing_admin_role
+
+Or all tests in the test_rbac_utils.py file::
+
+ $ tox -e py27 -- -n patrole_tempest_plugin.tests.unit.test_rbac_utils
+
+You may also use regular expressions to run any matching tests::
+
+ $ tox -e py27 -- test_rbac_utils
+
+For more information on these options and details about stestr, please see the
+`stestr documentation <http://stestr.readthedocs.io/en/latest/MANUAL.html>`_.
diff --git a/doc/source/usage.rst b/doc/source/usage.rst
deleted file mode 100644
index 14c2cc7..0000000
--- a/doc/source/usage.rst
+++ /dev/null
@@ -1,61 +0,0 @@
-.. _patrole-usage:
-
-========
-Usage
-========
-
-Patrole (API) Tests
-===================
-
-If Patrole is installed correctly, then the RBAC tests can be executed
-from inside the tempest root directory as follows::
-
- $ tox -eall-plugin -- patrole_tempest_plugin.tests.api
-
-To execute patrole tests for a specific module, run::
-
- $ tox -eall-plugin -- patrole_tempest_plugin.tests.api.compute
-
-.. note::
-
- It is possible to run Patrole via ``tox -eall`` in order to run Patrole
- isolated from other plugins. This can be accomplished by including the
- installation of services that currently use policy in code -- for example,
- Nova and Keystone. For example::
-
- $ tox -evenv-tempest -- pip install /opt/stack/patrole /opt/stack/keystone /opt/stack/nova
- $ tox -eall -- patrole_tempest_plugin.tests.api
-..
-
-To change the role that the patrole tests are being run as, edit
-``rbac_test_role`` in the ``patrole`` section of tempest.conf: ::
-
- [patrole]
- rbac_test_role = Member
- ...
-
-.. note::
-
- The ``rbac_test_role`` is service-specific. Member, for example,
- is an arbitrary role, but by convention is used to designate the default
- non-admin role in the system. Most patrole tests should be run with
- **admin** and **Member** roles. However, some services, like Heat, take
- advantage of a role called **heat_stack_user**, as it appears frequently
- in Heat's policy.json.
-
-For more information about the Member role,
-please see: `<https://ask.openstack.org/en/question/4759/member-vs-_member_/>`__.
-
-Unit Tests
-==========
-
-Patrole includes unit tests for its RBAC framework. They can be run by
-executing::
-
- $ tox -e py27
-
-or::
-
- $ tox -e py35
-
-against the Python 3.5 interpreter.
diff --git a/patrole_tempest_plugin/rbac_rule_validation.py b/patrole_tempest_plugin/rbac_rule_validation.py
index 69a43ea..7d48870 100644
--- a/patrole_tempest_plugin/rbac_rule_validation.py
+++ b/patrole_tempest_plugin/rbac_rule_validation.py
@@ -33,11 +33,13 @@
LOG = logging.getLogger(__name__)
_SUPPORTED_ERROR_CODES = [403, 404]
+_DEFAULT_ERROR_CODE = 403
RBACLOG = logging.getLogger('rbac_reporting')
-def action(service, rule='', rules=None, expected_error_code=403,
+def action(service, rule='', rules=None,
+ expected_error_code=_DEFAULT_ERROR_CODE, expected_error_codes=None,
extra_target_data=None):
"""A decorator for verifying OpenStack policy enforcement.
@@ -90,6 +92,25 @@
A 404 should not be provided *unless* the endpoint masks a
``Forbidden`` exception as a ``NotFound`` exception.
+ :param list expected_error_codes: When the ``rules`` list parameter is
+ used, then this list indicates the expected error code to use if one
+ of the rules does not allow the role being tested. This list must
+ coincide with and its elements remain in the same order as the rules
+ in the rules list.
+
+ Example::
+ rules=["api_action1", "api_action2"]
+ expected_error_codes=[404, 403]
+
+ a) If api_action1 fails and api_action2 passes, then the expected
+ error code is 404.
+ b) if api_action2 fails and api_action1 passes, then the expected
+ error code is 403.
+ c) if both api_action1 and api_action2 fail, then the expected error
+ code is the first error seen (404).
+
+ If an error code is missing from the list, it is defaulted to 403.
+
: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
@@ -118,7 +139,9 @@
if extra_target_data is None:
extra_target_data = {}
- rules = _prepare_rules(rule, rules)
+ rules, expected_error_codes = _prepare_multi_policy(rule, rules,
+ expected_error_code,
+ expected_error_codes)
def decorator(test_func):
role = CONF.patrole.rbac_test_role
@@ -141,8 +164,18 @@
disallowed_rules.append(rule)
allowed = allowed and _allowed
+ exp_error_code = expected_error_code
+ if disallowed_rules:
+ # Choose the first disallowed rule and expect the error
+ # code corresponding to it.
+ first_error_index = rules.index(disallowed_rules[0])
+ exp_error_code = expected_error_codes[first_error_index]
+ LOG.debug("%s: Expecting %d to be raised for policy name: %s",
+ test_func.__name__, exp_error_code,
+ disallowed_rules[0])
+
expected_exception, irregular_msg = _get_exception_type(
- expected_error_code)
+ exp_error_code)
test_status = 'Allowed'
@@ -202,7 +235,32 @@
return decorator
-def _prepare_rules(rule, rules):
+def _prepare_multi_policy(rule, rules, exp_error_code, exp_error_codes):
+
+ if exp_error_codes:
+ if not rules:
+ msg = ("The `rules` list must be provided if using the "
+ "`expected_error_codes` list.")
+ raise ValueError(msg)
+ if len(rules) != len(exp_error_codes):
+ msg = ("The `expected_error_codes` list is not the same length "
+ "as the `rules` list.")
+ raise ValueError(msg)
+ if exp_error_code:
+ deprecation_msg = (
+ "The `exp_error_code` argument has been deprecated in favor "
+ "of `exp_error_codes` and will be removed in a future "
+ "version.")
+ versionutils.report_deprecated_feature(LOG, deprecation_msg)
+ LOG.debug("The `exp_error_codes` argument will be used instead of "
+ "`exp_error_code`.")
+ if not isinstance(exp_error_codes, (tuple, list)):
+ exp_error_codes = [exp_error_codes]
+ else:
+ exp_error_codes = []
+ if exp_error_code:
+ exp_error_codes.append(exp_error_code)
+
if rules is None:
rules = []
elif not isinstance(rules, (tuple, list)):
@@ -216,7 +274,18 @@
LOG.debug("The `rules` argument will be used instead of `rule`.")
else:
rules.append(rule)
- return rules
+
+ # Fill in the exp_error_codes if needed. This is needed for the scenarios
+ # where no exp_error_codes array is provided, so the error codes must be
+ # set to the default error code value and there must be the same number
+ # of error codes as rules.
+ num_ecs = len(exp_error_codes)
+ num_rules = len(rules)
+ if (num_ecs < num_rules):
+ for i in range(num_rules - num_ecs):
+ exp_error_codes.append(_DEFAULT_ERROR_CODE)
+
+ return rules, exp_error_codes
def _is_authorized(test_obj, service, rule, extra_target_data):
diff --git a/patrole_tempest_plugin/tests/api/network/test_routers_rbac.py b/patrole_tempest_plugin/tests/api/network/test_routers_rbac.py
index ab85745..812b0c1 100644
--- a/patrole_tempest_plugin/tests/api/network/test_routers_rbac.py
+++ b/patrole_tempest_plugin/tests/api/network/test_routers_rbac.py
@@ -330,7 +330,8 @@
self.routers_client.delete_router(router['id'])
@rbac_rule_validation.action(service="neutron",
- rule="add_router_interface")
+ rules=["get_router", "add_router_interface"],
+ expected_error_codes=[404, 403])
@decorators.idempotent_id('a0627778-d68d-4913-881b-e345360cca19')
def test_add_router_interface(self):
"""Add Router Interface
diff --git a/patrole_tempest_plugin/tests/api/volume/test_volume_actions_rbac.py b/patrole_tempest_plugin/tests/api/volume/test_volume_actions_rbac.py
index a8c1727..a4fc3fd 100644
--- a/patrole_tempest_plugin/tests/api/volume/test_volume_actions_rbac.py
+++ b/patrole_tempest_plugin/tests/api/volume/test_volume_actions_rbac.py
@@ -106,31 +106,6 @@
waiters.wait_for_volume_resource_status(
self.volumes_client, volume_id, 'available')
- @decorators.attr(type=["slow"])
- @utils.services('image')
- @rbac_rule_validation.action(
- service="cinder",
- rule="volume_extension:volume_actions:upload_image")
- @decorators.idempotent_id('b0d0da46-903c-4445-893e-20e680d68b50')
- def test_volume_upload(self):
- # TODO(felipemonteiro): The ``upload_volume`` endpoint also enforces
- # "volume:copy_volume_to_image" but is not currently contained in
- # Cinder's policy.json.
- image_name = data_utils.rand_name(self.__class__.__name__ + '-Image')
-
- with self.rbac_utils.override_role(self):
- body = self.volumes_client.upload_volume(
- self.volume['id'], image_name=image_name, visibility="private",
- disk_format=CONF.volume.disk_format)['os-volume_upload_image']
- image_id = body["image_id"]
- self.addCleanup(test_utils.call_and_ignore_notfound_exc,
- self.image_client.delete_image,
- image_id)
- waiters.wait_for_image_status(self.image_client, image_id,
- 'active')
- waiters.wait_for_volume_resource_status(self.volumes_client,
- self.volume['id'], 'available')
-
@rbac_rule_validation.action(service="cinder",
rule="volume:update_readonly_flag")
@decorators.idempotent_id('2750717a-f250-4e41-9e09-02624aad6ff8')
@@ -243,6 +218,35 @@
super(VolumesActionsV310RbacTest, cls).setup_clients()
cls.image_client = cls.os_primary.image_client_v2
+ @classmethod
+ def resource_setup(cls):
+ super(VolumesActionsV310RbacTest, cls).resource_setup()
+ cls.volume = cls.create_volume()
+
+ @decorators.attr(type=["slow"])
+ @utils.services('image')
+ @rbac_rule_validation.action(
+ service="cinder",
+ rule="volume_extension:volume_actions:upload_image")
+ @decorators.idempotent_id('b0d0da46-903c-4445-893e-20e680d68b50')
+ def test_volume_upload_image(self):
+ # TODO(felipemonteiro): The ``upload_volume`` endpoint also enforces
+ # "volume:copy_volume_to_image".
+ image_name = data_utils.rand_name(self.__class__.__name__ + '-Image')
+
+ with self.rbac_utils.override_role(self):
+ body = self.volumes_client.upload_volume(
+ self.volume['id'], image_name=image_name, visibility="private",
+ disk_format=CONF.volume.disk_format)['os-volume_upload_image']
+ image_id = body["image_id"]
+ self.addCleanup(test_utils.call_and_ignore_notfound_exc,
+ self.image_client.delete_image,
+ image_id)
+ waiters.wait_for_image_status(self.image_client, image_id,
+ 'active')
+ waiters.wait_for_volume_resource_status(self.volumes_client,
+ self.volume['id'], 'available')
+
@decorators.attr(type=["slow"])
@utils.services('image')
@rbac_rule_validation.action(
@@ -251,12 +255,11 @@
@decorators.idempotent_id('578a84dd-a6bd-4f97-a418-4a0c3c272c08')
def test_volume_upload_public(self):
# This also enforces "volume_extension:volume_actions:upload_image".
- volume = self.create_volume()
image_name = data_utils.rand_name(self.__class__.__name__ + '-Image')
with self.rbac_utils.override_role(self):
body = self.volumes_client.upload_volume(
- volume['id'], image_name=image_name, visibility="public",
+ self.volume['id'], image_name=image_name, visibility="public",
disk_format=CONF.volume.disk_format)['os-volume_upload_image']
image_id = body["image_id"]
self.addCleanup(test_utils.call_and_ignore_notfound_exc,
@@ -265,7 +268,7 @@
waiters.wait_for_image_status(self.image_client, image_id,
'active')
waiters.wait_for_volume_resource_status(self.volumes_client,
- volume['id'], 'available')
+ self.volume['id'], 'available')
class VolumesActionsV312RbacTest(rbac_base.BaseVolumeRbacTest):
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 0a4c44b..2ae860c 100644
--- a/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
+++ b/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
@@ -436,7 +436,8 @@
rules = [mock.sentinel.action1, mock.sentinel.action2]
- @rbac_rv.action(mock.sentinel.service, rules=rules)
+ @rbac_rv.action(mock.sentinel.service, rules=rules,
+ expected_error_codes=[403, 403])
def test_policy(*args):
pass
@@ -454,8 +455,10 @@
rules = [
mock.sentinel.action1, mock.sentinel.action2, mock.sentinel.action3
]
+ exp_ecodes = [403, 403, 403]
- @rbac_rv.action(mock.sentinel.service, rules=rules)
+ @rbac_rv.action(mock.sentinel.service, rules=rules,
+ expected_error_codes=exp_ecodes)
def test_policy(*args):
pass
@@ -466,6 +469,9 @@
error_re = ".*OverPermission: .* \[%s\]$" % fail_on_action
self.assertRaisesRegex(rbac_exceptions.RbacOverPermission,
error_re, test_policy, self.mock_test_args)
+ mock_log.debug.assert_any_call(
+ "%s: Expecting %d to be raised for policy name: %s",
+ 'test_policy', 403, fail_on_action)
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)
@@ -485,21 +491,26 @@
rules = [
mock.sentinel.action1, mock.sentinel.action2, mock.sentinel.action3
]
+ exp_ecodes = [403, 403, 403]
- @rbac_rv.action(mock.sentinel.service, rules=rules)
+ @rbac_rv.action(mock.sentinel.service, rules=rules,
+ expected_error_codes=exp_ecodes)
def test_policy(*args):
raise exceptions.Forbidden()
- def _do_test(allowed_list):
+ def _do_test(allowed_list, fail_on_action):
mock_authority.PolicyAuthority.return_value.allowed.\
side_effect = allowed_list
test_policy(self.mock_test_args)
+ mock_log.debug.assert_called_with(
+ "%s: Expecting %d to be raised for policy name: %s",
+ 'test_policy', 403, fail_on_action)
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])
+ _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)
@@ -513,7 +524,8 @@
# NOTE: Avoid mock.sentinel here due to weird sorting with them.
rules = ['action1', 'action2', 'action3']
- @rbac_rv.action(mock.sentinel.service, rules=rules)
+ @rbac_rv.action(mock.sentinel.service, rules=rules,
+ expected_error_codes=[403, 403, 403])
def test_policy(*args):
raise exceptions.Forbidden()
@@ -528,3 +540,136 @@
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)
+
+ @mock.patch.object(rbac_rv, 'LOG', autospec=True)
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+ def test_rule_validation_multi_actions_forbidden(
+ self, mock_authority, mock_log):
+ """Test that when the expected result is forbidden because
+ two of the actions fail and the first action specifies 403,
+ verify that the overall evaluation results in success.
+ """
+
+ rules = [
+ mock.sentinel.action1, mock.sentinel.action2, mock.sentinel.action3
+ ]
+ exp_ecodes = [403, 403, 404]
+
+ @rbac_rv.action(mock.sentinel.service, rules=rules,
+ expected_error_codes=exp_ecodes)
+ def test_policy(*args):
+ raise exceptions.Forbidden()
+
+ def _do_test(allowed_list, fail_on_action):
+ mock_authority.PolicyAuthority.return_value.allowed.\
+ side_effect = allowed_list
+ test_policy(self.mock_test_args)
+ mock_log.debug.assert_called_with(
+ "%s: Expecting %d to be raised for policy name: %s",
+ 'test_policy', 403, fail_on_action)
+ mock_log.error.assert_not_called()
+ self._assert_policy_authority_called_with(rules, mock_authority)
+
+ _do_test([False, True, False], mock.sentinel.action1)
+ _do_test([False, False, True], mock.sentinel.action1)
+
+ @mock.patch.object(rbac_rv, 'LOG', autospec=True)
+ @mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
+ def test_rule_validation_multi_actions_notfound(
+ self, mock_authority, mock_log):
+ """Test that when the expected result is not found because
+ two of the actions fail and the first action specifies 404,
+ verify that the overall evaluation results in success.
+ """
+
+ rules = [
+ mock.sentinel.action1, mock.sentinel.action2,
+ mock.sentinel.action3, mock.sentinel.action4
+ ]
+ exp_ecodes = [403, 404, 403, 403]
+
+ @rbac_rv.action(mock.sentinel.service, rules=rules,
+ expected_error_codes=exp_ecodes)
+ def test_policy(*args):
+ raise exceptions.NotFound()
+
+ def _do_test(allowed_list, fail_on_action):
+ mock_authority.PolicyAuthority.return_value.allowed.\
+ side_effect = allowed_list
+ test_policy(self.mock_test_args)
+ mock_log.debug.assert_called_with(
+ "%s: Expecting %d to be raised for policy name: %s",
+ 'test_policy', 404, fail_on_action)
+ mock_log.error.assert_not_called()
+ self._assert_policy_authority_called_with(rules, mock_authority)
+
+ _do_test([True, False, False, True], mock.sentinel.action2)
+ _do_test([True, False, True, False], mock.sentinel.action2)
+
+ @mock.patch.object(rbac_rv, 'LOG', autospec=True)
+ def test_prepare_multi_policy_allowed_usages(self, mock_log):
+
+ def _do_test(rule, rules, ecode, ecodes, exp_rules, exp_ecodes):
+ rule_list, ec_list = rbac_rv._prepare_multi_policy(rule, rules,
+ ecode, ecodes)
+ self.assertEqual(rule_list, exp_rules)
+ self.assertEqual(ec_list, exp_ecodes)
+
+ # Validate that using deprecated values: rule and expected_error_code
+ # are converted into rules = [rule] and expected_error_codes =
+ # [expected_error_code]
+ _do_test("rule1", None, 403, None, ["rule1"], [403])
+
+ # Validate that rules = [rule] and expected_error_codes defaults to
+ # 403 when no values are provided.
+ _do_test("rule1", None, None, None, ["rule1"], [403])
+
+ # Validate that `len(rules) == len(expected_error_codes)` works when
+ # both == 1.
+ _do_test(None, ["rule1"], None, [403], ["rule1"], [403])
+
+ # Validate that `len(rules) == len(expected_error_codes)` works when
+ # both are > 1.
+ _do_test(None, ["rule1", "rule2"], None, [403, 404],
+ ["rule1", "rule2"], [403, 404])
+
+ # Validate that when only a default expected_error_code argument is
+ # provided, that default value and other default values (403) are
+ # filled into the expected_error_codes list.
+ # Example:
+ # @rbac_rv.action(service, rules=[<rule>, <rule>])
+ # def test_policy(*args):
+ # ...
+ _do_test(None, ["rule1", "rule2"], 403, None,
+ ["rule1", "rule2"], [403, 403])
+
+ # Validate that the deprecated values are ignored when new values are
+ # provided.
+ _do_test("rule3", ["rule1", "rule2"], 404, [403, 403],
+ ["rule1", "rule2"], [403, 403])
+ mock_log.debug.assert_any_call(
+ "The `rules` argument will be used instead of `rule`.")
+ mock_log.debug.assert_any_call(
+ "The `exp_error_codes` argument will be used instead of "
+ "`exp_error_code`.")
+
+ @mock.patch.object(rbac_rv, 'LOG', autospec=True)
+ def test_prepare_multi_policy_disallowed_usages(self, mock_log):
+
+ def _do_test(rule, rules, ecode, ecodes):
+ rule_list, ec_list = rbac_rv._prepare_multi_policy(rule, rules,
+ ecode, ecodes)
+
+ error_re = ("The `expected_error_codes` list is not the same length"
+ " as the `rules` list.")
+ # When len(rules) > 1 then len(expected_error_codes) must be same len.
+ self.assertRaisesRegex(ValueError, error_re, _do_test,
+ None, ["rule1", "rule2"], None, [403])
+ # When len(expected_error_codes) > 1 len(rules) must be same len.
+ self.assertRaisesRegex(ValueError, error_re, _do_test,
+ None, ["rule1"], None, [403, 404])
+ error_re = ("The `rules` list must be provided if using the "
+ "`expected_error_codes` list.")
+ # When expected_error_codes is provided rules must be as well.
+ self.assertRaisesRegex(ValueError, error_re, _do_test,
+ None, None, None, [404])
diff --git a/releasenotes/notes/multi-policy-support-4e5c8b4e9e25ad9d.yaml b/releasenotes/notes/multi-policy-support-4e5c8b4e9e25ad9d.yaml
index 3d192d9..1f33d8f 100644
--- a/releasenotes/notes/multi-policy-support-4e5c8b4e9e25ad9d.yaml
+++ b/releasenotes/notes/multi-policy-support-4e5c8b4e9e25ad9d.yaml
@@ -7,7 +7,25 @@
expected test result. This allows Patrole to more accurately determine
whether RBAC is configured correctly, since some API endpoints enforce
multiple policies.
+
+ Multiple policy support includes the capability to specify multiple
+ expected error codes, as some components may return different error codes
+ for different roles due to checking multiple policy rules. The
+ ``expected_error_codes`` argument has been added to the
+ ``rbac_rule_validation.action`` decorator, which is a list of error codes
+ expected when the corresponding rule in the ``rules`` list is disallowed
+ to perform the API action. For this reason, the error codes in the
+ ``expected_error_codes`` list must appear in the same order as their
+ corresponding rules in the ``rules`` list. For example:
+
+ expected_error_codes[0] is the error code for the rules[0] rule.
+ expected_error_codes[1] is the error code for the rules[1] rule.
+ ...
+
deprecations:
- |
The ``rule`` argument in the ``rbac_rule_validation.action`` decorator has
been deprecated in favor of ``rules``.
+
+ The ``expected_error_code`` argument in the ``rbac_rule_validation.action``
+ decorator has been deprecated in favor of ``expected_error_codes``.
diff --git a/tox.ini b/tox.ini
index cca09d0..a09822f 100644
--- a/tox.ini
+++ b/tox.ini
@@ -21,16 +21,20 @@
stestr --test-path ./patrole_tempest_plugin/tests/unit run {posargs}
[testenv:pep8]
+basepython = python3
commands = flake8 {posargs}
check-uuid --package patrole_tempest_plugin.tests.api
[testenv:uuidgen]
+basepython = python3
commands = check-uuid --package patrole_tempest_plugin.tests.api --fix
[testenv:venv]
+basepython = python3
commands = {posargs}
[testenv:cover]
+basepython = python3
commands = rm -rf *.pyc
rm -rf cover
rm -f .coverage
@@ -46,6 +50,7 @@
rm
[testenv:docs]
+basepython = python3
deps =
-c{env:UPPER_CONSTRAINTS_FILE:https://git.openstack.org/cgit/openstack/requirements/plain/upper-constraints.txt}
-r{toxinidir}/requirements.txt
@@ -56,6 +61,7 @@
whitelist_externals = rm
[testenv:releasenotes]
+basepython = python3
deps =
-c{env:UPPER_CONSTRAINTS_FILE:https://git.openstack.org/cgit/openstack/requirements/plain/upper-constraints.txt}
-r{toxinidir}/requirements.txt
@@ -66,9 +72,11 @@
whitelist_externals = rm
[testenv:debug]
+basepython = python3
commands = oslo_debug_helper -t patrole_tempest_plugin/tests {posargs}
[testenv:genconfig]
+basepython = python3
commands = oslo-config-generator --config-file etc/config-generator.patrole.conf
[flake8]