Merge "Fixes v3 identity tests with policy actions with rule admin_or_owner."
diff --git a/patrole_tempest_plugin/rbac_auth.py b/patrole_tempest_plugin/rbac_auth.py
index e4e35b1..687c0a8 100644
--- a/patrole_tempest_plugin/rbac_auth.py
+++ b/patrole_tempest_plugin/rbac_auth.py
@@ -13,8 +13,11 @@
# License for the specific language governing permissions and limitations
# under the License.
+import testtools
+
from oslo_log import log as logging
+from patrole_tempest_plugin import rbac_exceptions
from patrole_tempest_plugin import rbac_policy_parser
LOG = logging.getLogger(__name__)
@@ -22,20 +25,19 @@
class RbacAuthority(object):
def __init__(self, tenant_id, user_id, service=None):
- self.converter = rbac_policy_parser.RbacPolicyParser(
+ self.policy_parser = rbac_policy_parser.RbacPolicyParser(
tenant_id, user_id, service)
def get_permission(self, rule_name, role):
try:
- is_allowed = self.converter.allowed(rule_name, role)
+ is_allowed = self.policy_parser.allowed(rule_name, role)
if is_allowed:
- LOG.debug("[API]: %s, [Role]: %s is allowed!", rule_name, role)
+ LOG.debug("[Action]: %s, [Role]: %s is allowed!", rule_name,
+ role)
else:
- LOG.debug("[API]: %s, [Role]: %s is NOT allowed!",
+ LOG.debug("[Action]: %s, [Role]: %s is NOT allowed!",
rule_name, role)
return is_allowed
- except KeyError:
- LOG.debug("[API]: %s, [Role]: %s is NOT allowed!",
- rule_name, role)
- return False
+ except rbac_exceptions.RbacParsingException as e:
+ raise testtools.TestCase.skipException(str(e))
return False
diff --git a/patrole_tempest_plugin/rbac_exceptions.py b/patrole_tempest_plugin/rbac_exceptions.py
index ee42e19..967342b 100644
--- a/patrole_tempest_plugin/rbac_exceptions.py
+++ b/patrole_tempest_plugin/rbac_exceptions.py
@@ -30,3 +30,7 @@
class RbacInvalidService (exceptions.TempestException):
message = "Attempted to test an invalid service"
+
+
+class RbacParsingException (exceptions.TempestException):
+ message = "Attempted to test an invalid policy file or action"
diff --git a/patrole_tempest_plugin/rbac_policy_parser.py b/patrole_tempest_plugin/rbac_policy_parser.py
index 9926613..62113ac 100644
--- a/patrole_tempest_plugin/rbac_policy_parser.py
+++ b/patrole_tempest_plugin/rbac_policy_parser.py
@@ -90,7 +90,7 @@
policy_data = policy_data[:-2] + "\n}"
# Otherwise raise an exception.
else:
- raise rbac_exceptions.RbacResourceSetupFailed(
+ raise rbac_exceptions.RbacParsingException(
'Policy file for service: {0}, {1} not found.'
.format(service, self.path))
@@ -172,8 +172,12 @@
rule = self.rules[apply_rule]
return rule(target, access_data, o)
except KeyError as e:
- LOG.debug("{0} not found in policy file.".format(apply_rule))
- return False
+ message = "Policy action: {0} not found in policy file: {1}."\
+ .format(apply_rule, self.path)
+ LOG.debug(message)
+ raise rbac_exceptions.RbacParsingException(message)
except Exception as e:
- LOG.debug("Exception: {0} for rule: {1}.".format(e, apply_rule))
- return False
+ message = "Unknown exception: {0} for policy action: {1} in "\
+ "policy file: {2}.".format(e, apply_rule, self.path)
+ LOG.debug(message)
+ raise rbac_exceptions.RbacParsingException(message)
diff --git a/patrole_tempest_plugin/rbac_rule_validation.py b/patrole_tempest_plugin/rbac_rule_validation.py
index 463adce..a6490e7 100644
--- a/patrole_tempest_plugin/rbac_rule_validation.py
+++ b/patrole_tempest_plugin/rbac_rule_validation.py
@@ -47,11 +47,11 @@
try:
func(*args)
except rbac_exceptions.RbacInvalidService as e:
- msg = ("%s is not a valid service." % service)
- LOG.error(msg)
- raise exceptions.NotFound(
- "%s RbacInvalidService was: %s" %
- (msg, e))
+ msg = ("%s is not a valid service." % service)
+ LOG.error(msg)
+ raise exceptions.NotFound(
+ "%s RbacInvalidService was: %s" %
+ (msg, e))
except exceptions.Forbidden as e:
if allowed:
msg = ("Role %s was not allowed to perform %s." %
diff --git a/patrole_tempest_plugin/rbac_utils.py b/patrole_tempest_plugin/rbac_utils.py
index abbb435..18f132e 100644
--- a/patrole_tempest_plugin/rbac_utils.py
+++ b/patrole_tempest_plugin/rbac_utils.py
@@ -87,12 +87,18 @@
raise
finally:
- if BaseTestCase.get_identity_version() != 'v3':
- test_obj.auth_provider.clear_auth()
- # Sleep to avoid 401 errors caused by rounding in timing of
- # fernet token creation.
- time.sleep(1)
- test_obj.auth_provider.set_auth()
+ # NOTE(felipemonteiro): These two comments below are copied from
+ # tempest.api.identity.v2/v3.test_users.
+ #
+ # Reset auth again to verify the password restore does work.
+ # Clear auth restores the original credentials and deletes
+ # cached auth data.
+ test_obj.auth_provider.clear_auth()
+ # Fernet tokens are not subsecond aware and Keystone should only be
+ # precise to the second. Sleep to ensure we are passing the second
+ # boundary before attempting to authenticate.
+ time.sleep(1)
+ test_obj.auth_provider.set_auth()
def _clear_user_roles(cls, user_id, tenant_id):
roles = cls.creds_client.roles_client.list_user_roles_on_project(
diff --git a/patrole_tempest_plugin/tests/api/compute/test_flavor_access_rbac.py b/patrole_tempest_plugin/tests/api/compute/test_flavor_access_rbac.py
index 356a74a..1704644 100644
--- a/patrole_tempest_plugin/tests/api/compute/test_flavor_access_rbac.py
+++ b/patrole_tempest_plugin/tests/api/compute/test_flavor_access_rbac.py
@@ -13,18 +13,17 @@
# License for the specific language governing permissions and limitations
# under the License.
+from oslo_config import cfg
from oslo_log import log
-from tempest import config
from tempest.lib.common.utils import test_utils
from tempest.lib import decorators
-from tempest.lib import exceptions
+from tempest import test
-from patrole_tempest_plugin import rbac_exceptions
from patrole_tempest_plugin import rbac_rule_validation
from patrole_tempest_plugin.tests.api.compute import rbac_base
-CONF = config.CONF
+CONF = cfg.CONF
LOG = log.getLogger(__name__)
@@ -38,29 +37,27 @@
@classmethod
def skip_checks(cls):
super(FlavorAccessAdminRbacTest, cls).skip_checks()
- if not CONF.compute_feature_enabled.api_extensions:
- raise cls.skipException(
- '%s skipped as no compute extensions enabled' % cls.__name__)
+ if not test.is_extension_enabled('OS-FLV-EXT-DATA', 'compute'):
+ msg = "%s skipped as OS-FLV-EXT-DATA extension not enabled."\
+ % cls.__name__
+ raise cls.skipException(msg)
@classmethod
def resource_setup(cls):
super(FlavorAccessAdminRbacTest, cls).resource_setup()
cls.flavor_id = cls._create_flavor(is_public=False)['id']
+ cls.public_flavor_id = CONF.compute.flavor_ref
cls.tenant_id = cls.auth_provider.credentials.tenant_id
@decorators.idempotent_id('a2bd3740-765d-4c95-ac98-9e027378c75e')
@rbac_rule_validation.action(
service="nova",
rule="os_compute_api:os-flavor-access")
- def test_list_flavor_access(self):
+ def test_show_flavor(self):
+ # NOTE(felipemonteiro): show_flavor enforces the specified policy
+ # action, but only works if a public flavor is passed.
self.rbac_utils.switch_role(self, switchToRbacRole=True)
- try:
- self.client.list_flavor_access(self.flavor_id)
- except exceptions.NotFound as e:
- LOG.info("NotFound exception caught. Exception is thrown when "
- "role doesn't have access to the endpoint."
- "This is irregular and should be fixed.")
- raise rbac_exceptions.RbacActionFailed(e)
+ self.client.show_flavor(self.public_flavor_id)['flavor']
@decorators.idempotent_id('39cb5c8f-9990-436f-9282-fc76a41d9bac')
@rbac_rule_validation.action(
@@ -69,7 +66,8 @@
def test_add_flavor_access(self):
self.rbac_utils.switch_role(self, switchToRbacRole=True)
self.client.add_flavor_access(
- flavor_id=self.flavor_id, tenant_id=self.tenant_id)
+ flavor_id=self.flavor_id, tenant_id=self.tenant_id)[
+ 'flavor_access']
self.addCleanup(self.client.remove_flavor_access,
flavor_id=self.flavor_id, tenant_id=self.tenant_id)
diff --git a/patrole_tempest_plugin/tests/api/compute/test_instance_actions_rbac.py b/patrole_tempest_plugin/tests/api/compute/test_instance_actions_rbac.py
index 2903342..dcf3c90 100644
--- a/patrole_tempest_plugin/tests/api/compute/test_instance_actions_rbac.py
+++ b/patrole_tempest_plugin/tests/api/compute/test_instance_actions_rbac.py
@@ -16,6 +16,7 @@
from tempest.lib import decorators
from tempest import test
+from patrole_tempest_plugin import rbac_exceptions
from patrole_tempest_plugin import rbac_rule_validation
from patrole_tempest_plugin.tests.api.compute import rbac_base
@@ -54,5 +55,7 @@
rule="os_compute_api:os-instance-actions:events")
def test_get_instance_action(self):
self.rbac_utils.switch_role(self, switchToRbacRole=True)
- self.client.show_instance_action(
+ instance_action = self.client.show_instance_action(
self.server['id'], self.request_id)['instanceAction']
+ if 'events' not in instance_action:
+ raise rbac_exceptions.RbacActionFailed
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 a227e5c..e66da21 100644
--- a/patrole_tempest_plugin/tests/api/network/test_routers_rbac.py
+++ b/patrole_tempest_plugin/tests/api/network/test_routers_rbac.py
@@ -96,6 +96,7 @@
create_router:external_gateway_info:external_fixed_ips policy
"""
name = data_utils.rand_name('snat-router')
+
# Pick an ip address within the allocation_pools range
ip_address = random.choice(list(self.admin_ip_range))
external_fixed_ips = {'subnet_id': self.admin_subnet['id'],
@@ -167,6 +168,10 @@
self.routers_client.update_router(
self.admin_router['id'],
external_gateway_info={'network_id': self.admin_network['id']})
+ self.addCleanup(
+ self.routers_client.update_router,
+ self.admin_router['id'],
+ external_gateway_info=None)
@rbac_rule_validation.action(
service="neutron",
@@ -183,6 +188,10 @@
self.admin_router['id'],
external_gateway_info={'network_id': self.admin_network['id'],
'enable_snat': True})
+ self.addCleanup(
+ self.routers_client.update_router,
+ self.admin_router['id'],
+ external_gateway_info=None)
@rbac_rule_validation.action(
service="neutron",
diff --git a/patrole_tempest_plugin/tests/unit/test_rbac_policy_parser.py b/patrole_tempest_plugin/tests/unit/test_rbac_policy_parser.py
index edc63b8..8583eb5 100644
--- a/patrole_tempest_plugin/tests/unit/test_rbac_policy_parser.py
+++ b/patrole_tempest_plugin/tests/unit/test_rbac_policy_parser.py
@@ -82,7 +82,7 @@
test_tenant_id = mock.sentinel.tenant_id
test_user_id = mock.sentinel.user_id
- converter = rbac_policy_parser.RbacPolicyParser(
+ parser = rbac_policy_parser.RbacPolicyParser(
test_tenant_id, test_user_id, "test", self.custom_policy_file)
expected = {
@@ -95,24 +95,16 @@
'policy_action_6': ['eight'],
}
- fake_rule = 'fake_rule'
-
- for role in default_roles:
- self.assertFalse(converter.allowed(fake_rule, role))
- m_log.debug.assert_called_once_with(
- "{0} not found in policy file.".format('fake_rule'))
- m_log.debug.reset_mock()
-
for rule, role_list in expected.items():
for role in role_list:
- self.assertTrue(converter.allowed(rule, role))
+ self.assertTrue(parser.allowed(rule, role))
for role in set(default_roles) - set(role_list):
- self.assertFalse(converter.allowed(rule, role))
+ self.assertFalse(parser.allowed(rule, role))
def test_admin_policy_file_with_admin_role(self):
test_tenant_id = mock.sentinel.tenant_id
test_user_id = mock.sentinel.user_id
- converter = rbac_policy_parser.RbacPolicyParser(
+ parser = rbac_policy_parser.RbacPolicyParser(
test_tenant_id, test_user_id, "test", self.admin_policy_file)
role = 'admin'
@@ -122,17 +114,17 @@
disallowed_rules = ['non_admin_rule']
for rule in allowed_rules:
- allowed = converter.allowed(rule, role)
+ allowed = parser.allowed(rule, role)
self.assertTrue(allowed)
for rule in disallowed_rules:
- allowed = converter.allowed(rule, role)
+ allowed = parser.allowed(rule, role)
self.assertFalse(allowed)
def test_admin_policy_file_with_member_role(self):
test_tenant_id = mock.sentinel.tenant_id
test_user_id = mock.sentinel.user_id
- converter = rbac_policy_parser.RbacPolicyParser(
+ parser = rbac_policy_parser.RbacPolicyParser(
test_tenant_id, test_user_id, "test", self.admin_policy_file)
role = 'Member'
@@ -143,17 +135,17 @@
'admin_rule', 'is_admin_rule', 'alt_admin_rule']
for rule in allowed_rules:
- allowed = converter.allowed(rule, role)
+ allowed = parser.allowed(rule, role)
self.assertTrue(allowed)
for rule in disallowed_rules:
- allowed = converter.allowed(rule, role)
+ allowed = parser.allowed(rule, role)
self.assertFalse(allowed)
def test_admin_policy_file_with_context_is_admin(self):
test_tenant_id = mock.sentinel.tenant_id
test_user_id = mock.sentinel.user_id
- converter = rbac_policy_parser.RbacPolicyParser(
+ parser = rbac_policy_parser.RbacPolicyParser(
test_tenant_id, test_user_id, "test", self.alt_admin_policy_file)
role = 'fake_admin'
@@ -161,11 +153,11 @@
disallowed_rules = ['admin_rule']
for rule in allowed_rules:
- allowed = converter.allowed(rule, role)
+ allowed = parser.allowed(rule, role)
self.assertTrue(allowed)
for rule in disallowed_rules:
- allowed = converter.allowed(rule, role)
+ allowed = parser.allowed(rule, role)
self.assertFalse(allowed)
role = 'super_admin'
@@ -173,11 +165,11 @@
disallowed_rules = ['non_admin_rule']
for rule in allowed_rules:
- allowed = converter.allowed(rule, role)
+ allowed = parser.allowed(rule, role)
self.assertTrue(allowed)
for rule in disallowed_rules:
- allowed = converter.allowed(rule, role)
+ allowed = parser.allowed(rule, role)
self.assertFalse(allowed)
def test_tenant_user_policy(self):
@@ -189,28 +181,28 @@
"""
test_tenant_id = mock.sentinel.tenant_id
test_user_id = mock.sentinel.user_id
- converter = rbac_policy_parser.RbacPolicyParser(
+ parser = rbac_policy_parser.RbacPolicyParser(
test_tenant_id, test_user_id, "test", self.tenant_policy_file)
# Check whether Member role can perform expected actions.
allowed_rules = ['rule1', 'rule2', 'rule3', 'rule4']
for rule in allowed_rules:
- allowed = converter.allowed(rule, 'Member')
+ allowed = parser.allowed(rule, 'Member')
self.assertTrue(allowed)
disallowed_rules = ['admin_tenant_rule', 'admin_user_rule']
for disallowed_rule in disallowed_rules:
- self.assertFalse(converter.allowed(disallowed_rule, 'Member'))
+ self.assertFalse(parser.allowed(disallowed_rule, 'Member'))
# Check whether admin role can perform expected actions.
allowed_rules.extend(disallowed_rules)
for rule in allowed_rules:
- allowed = converter.allowed(rule, 'admin')
+ allowed = parser.allowed(rule, 'admin')
self.assertTrue(allowed)
# Check whether _try_rule is called with the correct target dictionary.
with mock.patch.object(
- converter, '_try_rule', return_value=True, autospec=True) \
+ parser, '_try_rule', return_value=True, autospec=True) \
as mock_try_rule:
expected_target = {
@@ -230,7 +222,7 @@
}
for rule in allowed_rules:
- allowed = converter.allowed(rule, 'Member')
+ allowed = parser.allowed(rule, 'Member')
self.assertTrue(allowed)
mock_try_rule.assert_called_once_with(
rule, expected_target, expected_access_data, mock.ANY)
@@ -265,3 +257,41 @@
m_log.debug.assert_called_once_with(
"{0} is NOT a valid service.".format(str(service)))
+
+ @mock.patch.object(rbac_policy_parser, 'LOG', autospec=True)
+ def test_invalid_policy_rule_throws_rbac_parsing_exception(self, m_log):
+ test_tenant_id = mock.sentinel.tenant_id
+ test_user_id = mock.sentinel.user_id
+
+ parser = rbac_policy_parser.RbacPolicyParser(
+ test_tenant_id, test_user_id, "test", self.custom_policy_file)
+
+ fake_rule = 'fake_rule'
+ expected_message = "Policy action: {0} not found in policy file: {1}."\
+ .format(fake_rule, self.custom_policy_file)
+
+ e = self.assertRaises(rbac_exceptions.RbacParsingException,
+ parser.allowed, fake_rule, None)
+ self.assertIn(expected_message, str(e))
+ m_log.debug.assert_called_once_with(expected_message)
+
+ @mock.patch.object(rbac_policy_parser, 'LOG', autospec=True)
+ def test_unknown_exception_throws_rbac_parsing_exception(self, m_log):
+ test_tenant_id = mock.sentinel.tenant_id
+ test_user_id = mock.sentinel.user_id
+
+ parser = rbac_policy_parser.RbacPolicyParser(
+ test_tenant_id, test_user_id, "test", self.custom_policy_file)
+ parser.rules = mock.MagicMock(
+ **{'__getitem__.return_value.side_effect': Exception(
+ mock.sentinel.error)})
+
+ expected_message = "Unknown exception: {0} for policy action: {1} in "\
+ "policy file: {2}.".format(mock.sentinel.error,
+ mock.sentinel.rule,
+ self.custom_policy_file)
+
+ e = self.assertRaises(rbac_exceptions.RbacParsingException,
+ parser.allowed, mock.sentinel.rule, None)
+ self.assertIn(expected_message, str(e))
+ m_log.debug.assert_called_once_with(expected_message)
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 1e78a7d..eeb06cc 100644
--- a/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
+++ b/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
@@ -13,7 +13,9 @@
# under the License.
import mock
+import testtools
+from patrole_tempest_plugin import rbac_auth
from patrole_tempest_plugin import rbac_exceptions
from patrole_tempest_plugin import rbac_rule_validation as rbac_rv
@@ -29,7 +31,10 @@
self.mock_args = mock.Mock(spec=test.BaseTestCase)
self.mock_args.auth_provider = mock.Mock()
self.mock_args.rbac_utils = mock.Mock()
- self.mock_args.auth_provider.credentials.tenant_id = 'tenant_id'
+ self.mock_args.auth_provider.credentials.tenant_id = \
+ mock.sentinel.tenant_id
+ self.mock_args.auth_provider.credentials.user_id = \
+ mock.sentinel.user_id
@mock.patch('patrole_tempest_plugin.rbac_auth.RbacAuthority')
def test_RBAC_rv_happy_path(self, mock_auth):
@@ -98,3 +103,22 @@
mock_auth.return_value = mock_permission
self.assertIsNone(wrapper(self.mock_args))
+
+ @mock.patch.object(rbac_auth, 'rbac_policy_parser', autospec=True)
+ def test_invalid_policy_rule_throws_skip_exception(
+ self, mock_rbac_policy_parser):
+ mock_rbac_policy_parser.RbacPolicyParser.return_value.allowed.\
+ side_effect = rbac_exceptions.RbacParsingException
+
+ decorator = rbac_rv.action(mock.sentinel.service,
+ mock.sentinel.policy_rule)
+ wrapper = decorator(mock.Mock())
+
+ e = self.assertRaises(testtools.TestCase.skipException, wrapper,
+ self.mock_args)
+ self.assertEqual('Attempted to test an invalid policy file or action',
+ str(e))
+
+ mock_rbac_policy_parser.RbacPolicyParser.assert_called_once_with(
+ mock.sentinel.tenant_id, mock.sentinel.user_id,
+ mock.sentinel.service)