openstack
/
patrole
Code Issues Proposed changes

Merge "Update overpermission/underpermission rbac exceptions"

This commit is contained in:
Zuul 2018-07-10 19:54:19 +00:00 committed by Gerrit Code Review
parent f800a1c5ca f16b6b3a1d
commit 94986d7e3c
6 changed files with 98 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
doc/source/framework/overview.rst
View File

@ -26,9 +26,11 @@ RBAC testing validation is broken up into 3 stages:
"Inconsistent" (or failing) cases include:
* Expected result is ``False`` and the test passes. This results in an
``RbacOverPermission`` exception getting thrown.
:class:`~rbac_exceptions.RbacOverPermissionException` exception
getting thrown.
* Expected result is ``True`` and the test fails. This results in a
``Forbidden`` exception getting thrown.
:class:`~rbac_exceptions.RbacOverPermissionException` exception
getting thrown.
For example, a 200 from the API call and a ``False`` result from
``oslo.policy`` or a 403 from the API call and a ``True`` result from

2
etc/patrole.conf.sample
View File

@ -45,7 +45,7 @@
#
# YAML definition: allowed
# test run: not allowed
# test result: fail (under-permission, e.g. Forbidden exception)
# test result: fail (under-permission)
#
# YAML definition: not allowed
# test run: allowed

2
patrole_tempest_plugin/config.py
View File

@ -55,7 +55,7 @@ test result: pass
YAML definition: allowed
test run: not allowed
test result: fail (under-permission, e.g. Forbidden exception)
test result: fail (under-permission)
YAML definition: not allowed
test run: allowed

23
patrole_tempest_plugin/rbac_exceptions.py
View File

@ -41,8 +41,27 @@ class RbacResourceSetupFailed(exceptions.TempestException):
message = "RBAC resource setup failed"
class RbacOverPermission(exceptions.TempestException):
message = "Action performed that should not be permitted"
class RbacOverPermissionException(exceptions.TempestException):
"""Raised when the expected result is failure but the actual result is
pass.
"""
message = "Unauthorized action was allowed to be performed"
class RbacUnderPermissionException(exceptions.TempestException):
"""Raised when the expected result is pass but the actual result is
failure.
"""
message = "Authorized action was not allowed to be performed"
class RbacExpectedWrongException(exceptions.TempestException):
"""Raised when the expected exception does not match the actual exception
raised, when both are instances of Forbidden or NotFound, indicating
the test provides a wrong argument to `expected_error_codes`.
"""
message = ("Expected %(expected)s to be raised but %(actual)s was raised "
"instead. Actual exception: %(exception)s")
class RbacInvalidService(exceptions.TempestException):

53
patrole_tempest_plugin/rbac_rule_validation.py
View File

@ -64,9 +64,9 @@ def action(service, rule='', rules=None,
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.
@ -124,8 +124,10 @@ def action(service, rule='', rules=None,
})
: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::
@ -202,17 +204,28 @@ def action(service, rule='', rules=None,
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 = (
@ -222,7 +235,7 @@ def action(service, rule='', rules=None,
)
)
LOG.error(msg)
raise rbac_exceptions.RbacOverPermission(msg)
raise rbac_exceptions.RbacOverPermissionException(msg)
finally:
if CONF.patrole_log.enable_reporting:
RBACLOG.info(
@ -237,7 +250,6 @@ def action(service, rule='', rules=None,
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 "
@ -411,3 +423,12 @@ def _format_extra_target_data(test_obj, extra_target_data):
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

62
patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
View File

@ -95,11 +95,12 @@ class RBACRuleValidationTest(BaseRBACRuleValidationTest):
@mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
def test_rule_validation_forbidden_negative(self, mock_authority,
mock_log):
"""Test Forbidden error is thrown and have permission fails.
"""Test RbacUnderPermissionException error is thrown and have
permission fails.
Negative test case: if Forbidden is thrown and the user should be
allowed to perform the action, then the Forbidden exception should be
raised.
allowed to perform the action, then the RbacUnderPermissionException
exception should be raised.
"""
mock_authority.PolicyAuthority.return_value.allowed.return_value = True
@ -109,8 +110,9 @@ class RBACRuleValidationTest(BaseRBACRuleValidationTest):
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)
self.assertRaisesRegex(
rbac_exceptions.RbacUnderPermissionException, test_re, test_policy,
self.mock_test_args)
self.assertRegex(mock_log.error.mock_calls[0][1][0], test_re)
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
@ -149,8 +151,9 @@ class RBACRuleValidationTest(BaseRBACRuleValidationTest):
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)
self.assertRaisesRegex(
rbac_exceptions.RbacUnderPermissionException, test_re, test_policy,
self.mock_test_args)
self.assertRegex(mock_log.error.mock_calls[0][1][0], test_re)
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
@ -190,8 +193,9 @@ class RBACRuleValidationTest(BaseRBACRuleValidationTest):
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)
self.assertRaisesRegex(
rbac_exceptions.RbacUnderPermissionException, test_re, test_policy,
self.mock_test_args)
self.assertRegex(mock_log.error.mock_calls[0][1][0], test_re)
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
@ -211,17 +215,15 @@ class RBACRuleValidationTest(BaseRBACRuleValidationTest):
def test_policy(*args):
raise exceptions.Forbidden('Test message')
error_msg = ("An unexpected exception has occurred during test: "
"test_policy. Exception was: Forbidden\nDetails: Test "
"message")
error_re = r'Expected .* to be raised but .* was raised instead'
for allowed in [True, False]:
mock_authority.PolicyAuthority.return_value.allowed.\
return_value = allowed
self.assertRaisesRegex(exceptions.Forbidden, 'Test message',
test_policy, self.mock_test_args)
self.assertIn(error_msg, mock_log.error.mock_calls[0][1][0])
mock_log.error.reset_mock()
self.assertRaisesRegex(
rbac_exceptions.RbacExpectedWrongException, error_re,
test_policy, self.mock_test_args)
self.assertTrue(mock_log.error.called)
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
@mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
@ -254,8 +256,9 @@ class RBACRuleValidationTest(BaseRBACRuleValidationTest):
error_re = expected_errors[pos]
if error_re:
self.assertRaisesRegex(exceptions.Forbidden, error_re,
test_policy, self.mock_test_args)
self.assertRaisesRegex(
rbac_exceptions.RbacUnderPermissionException, 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)
@ -274,7 +277,7 @@ class RBACRuleValidationTest(BaseRBACRuleValidationTest):
@mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
def test_rule_validation_overpermission_negative(self, mock_authority,
mock_log):
"""Test that OverPermission is correctly handled.
"""Test that RbacOverPermissionException is correctly handled.
Tests that case where no exception is thrown but the Patrole framework
says that the role should not be allowed to perform the policy action.
@ -295,7 +298,7 @@ class RBACRuleValidationTest(BaseRBACRuleValidationTest):
test_policy_expect_forbidden, test_policy_expect_not_found):
error_re = ".*OverPermission: .* \[%s\]$" % mock.sentinel.action
self.assertRaisesRegex(rbac_exceptions.RbacOverPermission,
self.assertRaisesRegex(rbac_exceptions.RbacOverPermissionException,
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()
@ -336,7 +339,7 @@ class RBACRuleValidationTest(BaseRBACRuleValidationTest):
@mock.patch.object(rbac_rv, 'policy_authority', autospec=True)
def test_get_exception_type_403(self, _):
"""Test that getting a 404 exception type returns Forbidden."""
"""Test that getting a 403 exception type returns Forbidden."""
expected_exception = exceptions.Forbidden
expected_irregular_msg = None
@ -449,7 +452,8 @@ class RBACRuleValidationTestMultiPolicy(BaseRBACRuleValidationTest):
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.
the overall evaluation results in an RbacOverPermissionException
getting raised.
"""
rules = [
@ -467,8 +471,9 @@ class RBACRuleValidationTestMultiPolicy(BaseRBACRuleValidationTest):
allowed_list)
error_re = ".*OverPermission: .* \[%s\]$" % fail_on_action
self.assertRaisesRegex(rbac_exceptions.RbacOverPermission,
error_re, test_policy, self.mock_test_args)
self.assertRaisesRegex(
rbac_exceptions.RbacOverPermissionException, 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)
@ -518,7 +523,7 @@ class RBACRuleValidationTestMultiPolicy(BaseRBACRuleValidationTest):
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.
results in a RbacUnderPermissionException getting raised.
"""
# NOTE: Avoid mock.sentinel here due to weird sorting with them.
@ -536,8 +541,9 @@ class RBACRuleValidationTestMultiPolicy(BaseRBACRuleValidationTest):
"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.assertRaisesRegex(
rbac_exceptions.RbacUnderPermissionException, 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)
@ -545,7 +551,7 @@ class RBACRuleValidationTestMultiPolicy(BaseRBACRuleValidationTest):
@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
"""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.
"""