Update overpermission/underpermission rbac exceptions
This patchset updates rbac_exceptions by bringing the concept
of under-permission and over-permission together. An over-permission
occurs when an unauthorized role is allowed to perform an action
and an under-permission occurs when an authorized role is not
allowed to perform an action. Both of these are important failure
scenarios.
Current Patrole has an RbacOverPermission Exception but uses
a "Forbidden" as a pseudonym for the under-permission version
but this is not ideal for the following reasons:
* Patrole can expect a 404 Not Found due to Neutron policy enforcement [0]
* The naming is inconsistent with RbacOverPermission
* It should have a Patrole wrapper exception (NotFound is used directly
from Tempest)
So, this patchset:
* renames RbacOverPermission to RbacOverPermissionException
* replaces Forbidden exception with RbacUnderPermissionException
* updates documentation, docstrings and unit tests
In addition, this patchset introduces a new exception called
RbacExpectedWrongException which is raised when the expected
exception does not match the actual exception and both are instances
of 403 and 404, which means that the RBAC test uses the wrong
expected_error_codes.
Change-Id: I681610448cbe0269f02c34ea6afaaaf29c306121
diff --git a/patrole_tempest_plugin/rbac_rule_validation.py b/patrole_tempest_plugin/rbac_rule_validation.py
index 7d48870..115e23e 100644
--- a/patrole_tempest_plugin/rbac_rule_validation.py
+++ b/patrole_tempest_plugin/rbac_rule_validation.py
@@ -64,9 +64,9 @@
1) If *expected* is True and the test passes (*actual*), this is a success.
2) If *expected* is True and the test fails (*actual*), this results in a
- `Forbidden` exception failure.
+ ``RbacUnderPermissionException`` exception failure.
3) If *expected* is False and the test passes (*actual*), this results in
- an `OverPermission` exception failure.
+ an ``RbacOverPermissionException`` exception failure.
4) If *expected* is False and the test fails (*actual*), this is a success.
As such, negative and positive testing can be applied using this decorator.
@@ -123,8 +123,10 @@
})
:raises NotFound: If ``service`` is invalid.
- :raises Forbidden: For item (2) above.
- :raises RbacOverPermission: For item (3) above.
+ :raises RbacUnderPermissionException: For item (2) above.
+ :raises RbacOverPermissionException: For item (3) above.
+ :raises RbacExpectedWrongException: When a 403 is expected but a 404
+ is raised instead or vice versa.
Examples::
@@ -201,17 +203,28 @@
sorted(set(rules) - set(disallowed_rules)),
sorted(disallowed_rules)))
LOG.error(msg)
- raise exceptions.Forbidden(
+ raise rbac_exceptions.RbacUnderPermissionException(
"%s Exception was: %s" % (msg, e))
- except Exception as e:
- with excutils.save_and_reraise_exception():
- exc_info = sys.exc_info()
- error_details = six.text_type(exc_info[1])
- msg = ("An unexpected exception has occurred during test: "
- "%s. Exception was: %s" % (test_func.__name__,
- error_details))
- test_status = 'Error, %s' % (error_details)
- LOG.error(msg)
+ except Exception as actual_exception:
+ if _check_for_expected_mismatch_exception(expected_exception,
+ actual_exception):
+ LOG.error('Expected and actual exceptions do not match. '
+ 'Expected: %s. Actual: %s.',
+ expected_exception,
+ actual_exception.__class__)
+ raise rbac_exceptions.RbacExpectedWrongException(
+ expected=expected_exception,
+ actual=actual_exception.__class__,
+ exception=actual_exception)
+ else:
+ with excutils.save_and_reraise_exception():
+ exc_info = sys.exc_info()
+ error_details = six.text_type(exc_info[1])
+ msg = ("An unexpected exception has occurred during "
+ "test: %s. Exception was: %s" % (
+ test_func.__name__, error_details))
+ test_status = 'Error, %s' % (error_details)
+ LOG.error(msg)
else:
if not allowed:
msg = (
@@ -221,7 +234,7 @@
)
)
LOG.error(msg)
- raise rbac_exceptions.RbacOverPermission(msg)
+ raise rbac_exceptions.RbacOverPermissionException(msg)
finally:
if CONF.patrole_log.enable_reporting:
RBACLOG.info(
@@ -236,7 +249,6 @@
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 "
@@ -410,3 +422,12 @@
formatted_target_data[user_attribute] = attr_value
return formatted_target_data
+
+
+def _check_for_expected_mismatch_exception(expected_exception,
+ actual_exception):
+ permission_exceptions = (exceptions.Forbidden, exceptions.NotFound)
+ if isinstance(actual_exception, permission_exceptions):
+ if not isinstance(actual_exception, expected_exception.__class__):
+ return True
+ return False