diff --git a/REVIEWING.rst b/REVIEWING.rst index 7e3c71fd..4ee847f7 100644 --- a/REVIEWING.rst +++ b/REVIEWING.rst @@ -109,6 +109,58 @@ skipped or not. Do not approve changes that depend on an API call to determine 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 c03aac6f..a9dcdc0f 100644 --- a/doc/source/index.rst +++ b/doc/source/index.rst @@ -17,6 +17,7 @@ RBAC Overview :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 00000000..d38b31e5 --- /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 09ab17d4..acfd66f1 100644 --- a/doc/source/rbac-overview.rst +++ b/doc/source/rbac-overview.rst @@ -124,12 +124,9 @@ accurate validation, Patrole handles multiple policies: 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 @@ related to RBAC in Patrole. 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`.