Merge "docs: Add multi-policy validation documentation"
diff --git a/REVIEWING.rst b/REVIEWING.rst
index 7e3c71f..4ee847f 100644
--- a/REVIEWING.rst
+++ b/REVIEWING.rst
@@ -109,6 +109,58 @@
whether to skip or not.
+Multi-Policy Guidelines
+-----------------------
+
+Care should be taken when using multiple policies in an RBAC test. The
+following guidelines should be followed before deciding to add multiple
+policies to a Patrole test.
+
+.. _general-multi-policy-guidelines:
+
+General Multi-policy API Code Guidelines
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The list below enumerates guidelines beginning with those with the highest
+priority and ending with those with the lowest priority. A higher priority
+item takes precedence over lower priority items.
+
+#. Order the policies in the ``rules`` parameter chronologically with respect
+ to the order they're called by the API endpoint under test.
+#. Only use policies that map to the API by referencing the appropriate policy
+ in code documentation.
+#. Only include the minimum number of policies needed to test the API
+ correctly: don't add extraneous policies.
+#. If possible, only use policies that directly relate to the API. If the
+ policies are used across multiple APIs, try to omit it. If a "generic"
+ policy needs to be added to get the test to pass, then this is fair game.
+#. Limit the number of policies to a reasonable number, such as 3.
+
+Neutron Multi-policy API Code Guidelines
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Because Neutron can raise a 403 or 404 following failed authorization, Patrole
+uses the ``expected_error_codes`` parameter to accommodate this behavior.
+Each policy action enumerated in ``rules`` must have a corresponding entry
+in ``expected_error_codes``. Each expected error code must be either a 403 or a
+404, which indicates that, when policy enforcement fails for the corresponding
+policy action, that error code is expected by Patrole. For more information
+about these parameters, see :ref:`rbac-validation`.
+
+The list below enumerates additional multi-policy guidelines that apply in
+particular to Neutron. A higher priority item takes precedence over lower
+priority items.
+
+#. Order the expected error codes in the ``expected_error_codes`` parameter
+ chronologically with respect to the order each corresponding policy in
+ ``rules`` is authorized by the API under test.
+#. Ensure the :ref:`neutron-multi-policy-validation` is followed when
+ determining the expected error code for each corresponding policy.
+
+The same guidelines under :ref:`general-multi-policy-guidelines` should be
+applied afterward.
+
+
Release Notes
-------------
Release notes are how we indicate to users and other consumers of Patrole what
diff --git a/doc/source/index.rst b/doc/source/index.rst
index c03aac6..a9dcdc0 100644
--- a/doc/source/index.rst
+++ b/doc/source/index.rst
@@ -17,6 +17,7 @@
:maxdepth: 2
rbac-overview
+ multi-policy-validation
User's Guide
============
diff --git a/doc/source/multi-policy-validation.rst b/doc/source/multi-policy-validation.rst
new file mode 100644
index 0000000..d38b31e
--- /dev/null
+++ b/doc/source/multi-policy-validation.rst
@@ -0,0 +1,187 @@
+.. _multi-policy-validation:
+
+=======================
+Multi-policy Validation
+=======================
+
+Introduction
+------------
+
+Multi-policy validation exists in Patrole because if one policy were assumed,
+then tests could fail because they would not consider all the policies actually
+being enforced. The reasoning can be found in `this spec`_. Basically,
+since Patrole derives the expected test result dynamically in order to test any
+role, each policy enforced by the API under test must be considered to derive
+an accurate expected test result, or else the expected and actual test
+results will not always match, resulting in overall test failure. For more
+information about Patrole's RBAC validation work flow, reference
+:ref:`rbac-validation`.
+
+Multi-policy support allows Patrole to more accurately offer RBAC tests for API
+endpoints that enforce multiple policy actions.
+
+.. _this spec: https://github.com/openstack/qa-specs/blob/master/specs/patrole/rbac-testing-multiple-policies.rst
+
+Scope
+-----
+
+Multiple policies should be applied only to tests that require them. Not all
+API endpoints enforce multiple policies. Some services consistently enforce
+1 policy per API, while on the other side of the spectrum, services like
+Neutron have much more involved policy enforcement work flows. See
+:ref:`neutron-multi-policy-validation` for more information.
+
+.. _neutron-multi-policy-validation:
+
+Neutron Multi-policy Validation
+-------------------------------
+
+Neutron can raise different :ref:`policy-error-codes` following failed policy
+authorization. Many endpoints in Neutron enforce multiple policies, which
+complicates matters when trying to determine whether the endpoint raises a
+403 or a 404 following unauthorized access.
+
+Multi-policy Examples
+---------------------
+
+General Examples
+^^^^^^^^^^^^^^^^
+
+Below is an example of multi-policy validation for a carefully chosen Nova API:
+
+.. code-block:: python
+
+ @rbac_rule_validation.action(
+ service="nova",
+ 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.
+
+ In order to trigger the unlock:unlock_override policy instead
+ of the unlock policy, the server must be locked by a different
+ user than the one who is attempting to unlock it.
+ """
+ self.os_admin.servers_client.lock_server(self.server['id'])
+ self.addCleanup(self.servers_client.unlock_server, self.server['id'])
+
+ with self.rbac_utils.override_role(self):
+ self.servers_client.unlock_server(self.server['id'])
+
+While the ``expected_error_codes`` parameter is omitted in the example above,
+Patrole automatically populates it with a 403 for each policy in ``rules``.
+Therefore, in the example above, the following expected error codes/rules
+relationship is observed:
+
+* "os_compute_api:os-lock-server:unlock" => 403
+* "os_compute_api:os-lock-server:unlock:unlock_override" => 403
+
+Below is an example that uses ``expected_error_codes`` to account for the
+fact that Neutron is expected to raise a ``404`` on the first policy that
+is enforced server-side ("get_port"). Also, in this example, soft authorization
+is performed, meaning that it is necessary to check the response body for an
+attribute that is added only following successful policy authorization.
+
+.. code-block:: python
+
+ @utils.requires_ext(extension='binding', service='network')
+ @rbac_rule_validation.action(service="neutron",
+ rules=["get_port",
+ "get_port:binding:vif_type"],
+ expected_error_codes=[404, 403])
+ @decorators.idempotent_id('125aff0b-8fed-4f8e-8410-338616594b06')
+ def test_show_port_binding_vif_type(self):
+
+ # Verify specific fields of a port
+ fields = ['binding:vif_type']
+
+ with self.rbac_utils.override_role(self):
+ retrieved_port = self.ports_client.show_port(
+ self.port['id'], fields=fields)['port']
+
+ # Rather than throwing a 403, the field is not present, so raise exc.
+ if fields[0] not in retrieved_port:
+ raise rbac_exceptions.RbacMalformedResponse(
+ attribute='binding:vif_type')
+
+Note that in the example above, failure to authorize
+"get_port:binding:vif_type" results in the response body getting successfully
+returned by the server, but without additional dictionary keys. If Patrole
+fails to find those expected keys, it *acts as though* a 403 was thrown (by
+raising an exception itself, the ``rbac_rule_validation`` decorator handles
+the rest).
+
+Neutron Examples
+^^^^^^^^^^^^^^^^
+
+A basic Neutron example that only expects 403's to be raised:
+
+.. code-block:: python
+
+ @utils.requires_ext(extension='external-net', service='network')
+ @rbac_rule_validation.action(service="neutron",
+ rules=["create_network",
+ "create_network:router:external"],
+ expected_error_codes=[403, 403])
+ @decorators.idempotent_id('51adf2a7-739c-41e0-8857-3b4c460cbd24')
+ def test_create_network_router_external(self):
+
+ """Create External Router Network Test
+
+ RBAC test for the neutron create_network:router:external policy
+ """
+ with self.rbac_utils.override_role(self):
+ self._create_network(router_external=True)
+
+Note that above the following expected error codes/rules relationship is
+observed:
+
+* "create_network" => 403
+* "create_network:router:external" => 403
+
+A more involved example that expects a 404 to be raised, should the first
+policy under ``rules`` fail authorization, and a 403 to be raised for any
+subsequent policy authorization failure:
+
+.. code-block:: python
+
+ @rbac_rule_validation.action(service="neutron",
+ rules=["get_network",
+ "update_network",
+ "update_network:shared"],
+ expected_error_codes=[404, 403, 403])
+ @decorators.idempotent_id('37ea3e33-47d9-49fc-9bba-1af98fbd46d6')
+ def test_update_network_shared(self):
+
+ """Update Shared Network Test
+
+ RBAC test for the neutron update_network:shared policy
+ """
+ with self.rbac_utils.override_role(self):
+ self._update_network(shared_network=True)
+ self.addCleanup(self._update_network, shared_network=False)
+
+Note that above the following expected error codes/rules relationship is
+observed:
+
+* "get_network" => 404
+* "update_network" => 403
+* "update_network:shared" => 403
+
+Limitations
+-----------
+
+Multi-policy validation in RBAC tests comes with limitations, due to technical
+and practical challenges.
+
+Technically, there are challenges associated with multiple policies across
+cross-service API communication in OpenStack, such as between Nova and Cinder
+or Nova and Neutron. The current framework does not account for these
+cross-service policy enforcement workflows, and it is still up for debate
+whether it should.
+
+Practically, it is not possible to enumerate every policy enforced by every API
+in Patrole, as the maintenance overhead would be huge.
+
+.. _Neutron policy documentation: https://docs.openstack.org/neutron/pike/contributor/internals/policy.html
diff --git a/doc/source/rbac-overview.rst b/doc/source/rbac-overview.rst
index 09ab17d..acfd66f 100644
--- a/doc/source/rbac-overview.rst
+++ b/doc/source/rbac-overview.rst
@@ -124,12 +124,9 @@
degree of log tracing is required by developers to confirm that the expected
policies are getting enforced, prior to the tests getting merged.
-.. todo::
+For more information, see :ref:`multi-policy-validation`.
- Link to multi-policy validation documentation section once it has been
- written.
-
-.. _error-codes:
+.. _policy-error-codes:
Error Codes
-----------
@@ -196,7 +193,7 @@
in an exception getting raised or a boolean value getting returned.
Hard authorization results in an exception getting raised. Usually, this
results in a ``403 Forbidden`` getting returned for unauthorized requests.
- (See :ref:`error-codes` for further details.)
+ (See :ref:`policy-error-codes` for further details.)
Related term: :term:`soft authorization`.