From 52deb8b15529aec1d62fdc4ea47fa79efd51b805 Mon Sep 17 00:00:00 2001 From: "Andrea Frittoli (andreaf)" Date: Wed, 18 May 2016 19:14:22 +0100 Subject: [PATCH] Hash credentials on user, project/tenant and pwd Preprovision credential provider hashes credentials based on all fields specified in the YAML. The same configured credentials can be used to build both v2 and v3 credential objects, so we need to hash on the fields that are common between v2 and v3 only. Because v2 only understand tenants (and not project) the intersection would be only user and password. Because of that, and because we want to promote project against tenant, accept project in v2 credentials as well, by translating it to tenant at __init__ time. Change-Id: Ib62c26cdffc2db6f6352d9889c689db3ff09aa5d --- tempest/common/credentials_factory.py | 5 - tempest/common/preprov_creds.py | 53 ++++---- tempest/lib/auth.py | 50 +++++-- tempest/tests/common/test_preprov_creds.py | 144 +++++++++++++++------ tempest/tests/lib/test_credentials.py | 4 +- 5 files changed, 172 insertions(+), 84 deletions(-) diff --git a/tempest/common/credentials_factory.py b/tempest/common/credentials_factory.py index 7c73ada430..82db9cc05a 100644 --- a/tempest/common/credentials_factory.py +++ b/tempest/common/credentials_factory.py @@ -186,11 +186,6 @@ def get_configured_credentials(credential_type, fill_in=True, params[attr] = getattr(_section, attr) else: params[attr] = getattr(_section, prefix + "_" + attr) - # NOTE(andreaf) v2 API still uses tenants, so we must translate project - # to tenant before building the Credentials object - if identity_version == 'v2': - params['tenant_name'] = params.get('project_name') - params.pop('project_name', None) # Build and validate credentials. We are reading configured credentials, # so validate them even if fill_in is False credentials = get_credentials(fill_in=fill_in, diff --git a/tempest/common/preprov_creds.py b/tempest/common/preprov_creds.py index 51f723b106..7350222a89 100644 --- a/tempest/common/preprov_creds.py +++ b/tempest/common/preprov_creds.py @@ -43,6 +43,11 @@ def read_accounts_yaml(path): class PreProvisionedCredentialProvider(cred_provider.CredentialProvider): + # Exclude from the hash fields specific to v2 or v3 identity API + # i.e. only include user*, project*, tenant* and password + HASH_CRED_FIELDS = (set(auth.KeystoneV2Credentials.ATTRIBUTES) & + set(auth.KeystoneV3Credentials.ATTRIBUTES)) + def __init__(self, identity_version, test_accounts_file, accounts_lock_dir, name=None, credentials_domain=None, admin_role=None, object_storage_operator_role=None, @@ -104,6 +109,7 @@ class PreProvisionedCredentialProvider(cred_provider.CredentialProvider): object_storage_operator_role=None, object_storage_reseller_admin_role=None): hash_dict = {'roles': {}, 'creds': {}, 'networks': {}} + # Loop over the accounts read from the yaml file for account in accounts: roles = [] @@ -116,7 +122,9 @@ class PreProvisionedCredentialProvider(cred_provider.CredentialProvider): if 'resources' in account: resources = account.pop('resources') temp_hash = hashlib.md5() - temp_hash.update(six.text_type(account).encode('utf-8')) + account_for_hash = dict((k, v) for (k, v) in six.iteritems(account) + if k in cls.HASH_CRED_FIELDS) + temp_hash.update(six.text_type(account_for_hash).encode('utf-8')) temp_hash_key = temp_hash.hexdigest() hash_dict['creds'][temp_hash_key] = account for role in roles: @@ -262,13 +270,13 @@ class PreProvisionedCredentialProvider(cred_provider.CredentialProvider): for _hash in self.hash_dict['creds']: # Comparing on the attributes that are expected in the YAML init_attributes = creds.get_init_attributes() + # Only use the attributes initially used to calculate the hash + init_attributes = [x for x in init_attributes if + x in self.HASH_CRED_FIELDS] hash_attributes = self.hash_dict['creds'][_hash].copy() - if ('user_domain_name' in init_attributes and 'user_domain_name' - not in hash_attributes): - # Allow for the case of domain_name populated from config - domain_name = self.credentials_domain - hash_attributes['user_domain_name'] = domain_name - if all([getattr(creds, k) == hash_attributes[k] for + # NOTE(andreaf) Not all fields may be available on all credentials + # so defaulting to None for that case. + if all([getattr(creds, k, None) == hash_attributes.get(k, None) for k in init_attributes]): return _hash raise AttributeError('Invalid credentials %s' % creds) @@ -351,23 +359,20 @@ class PreProvisionedCredentialProvider(cred_provider.CredentialProvider): return net_creds def _extend_credentials(self, creds_dict): - # In case of v3, adds a user_domain_name field to the creds - # dict if not defined + # Add or remove credential domain fields to fit the identity version + domain_fields = set(x for x in auth.KeystoneV3Credentials.ATTRIBUTES + if 'domain' in x) + msg = 'Assuming they are valid in the default domain.' if self.identity_version == 'v3': - user_domain_fields = set(['user_domain_name', 'user_domain_id']) - if not user_domain_fields.intersection(set(creds_dict.keys())): - creds_dict['user_domain_name'] = self.credentials_domain - # NOTE(andreaf) In case of v2, replace project with tenant if project - # is provided and tenant is not + if not domain_fields.intersection(set(creds_dict.keys())): + msg = 'Using credentials %s for v3 API calls. ' + msg + LOG.warning(msg, self._sanitize_creds(creds_dict)) + creds_dict['domain_name'] = self.credentials_domain if self.identity_version == 'v2': - if ('project_name' in creds_dict and - 'tenant_name' in creds_dict and - creds_dict['project_name'] != creds_dict['tenant_name']): - clean_creds = self._sanitize_creds(creds_dict) - msg = 'Cannot specify project and tenant at the same time %s' - raise exceptions.InvalidCredentials(msg % clean_creds) - if ('project_name' in creds_dict and - 'tenant_name' not in creds_dict): - creds_dict['tenant_name'] = creds_dict['project_name'] - creds_dict.pop('project_name') + if domain_fields.intersection(set(creds_dict.keys())): + msg = 'Using credentials %s for v2 API calls. ' + msg + LOG.warning(msg, self._sanitize_creds(creds_dict)) + # Remove all valid domain attributes + for attr in domain_fields.intersection(set(creds_dict.keys())): + creds_dict.pop(attr) return creds_dict diff --git a/tempest/lib/auth.py b/tempest/lib/auth.py index a6833bede3..c2ccf7757a 100644 --- a/tempest/lib/auth.py +++ b/tempest/lib/auth.py @@ -532,6 +532,7 @@ class Credentials(object): """ ATTRIBUTES = [] + COLLISIONS = [] def __init__(self, **kwargs): """Enforce the available attributes at init time (only). @@ -543,6 +544,13 @@ class Credentials(object): self._apply_credentials(kwargs) def _apply_credentials(self, attr): + for (key1, key2) in self.COLLISIONS: + val1 = attr.get(key1) + val2 = attr.get(key2) + if val1 and val2 and val1 != val2: + msg = ('Cannot have conflicting values for %s and %s' % + (key1, key2)) + raise exceptions.InvalidCredentials(msg) for key in attr.keys(): if key in self.ATTRIBUTES: setattr(self, key, attr[key]) @@ -600,7 +608,33 @@ class Credentials(object): class KeystoneV2Credentials(Credentials): ATTRIBUTES = ['username', 'password', 'tenant_name', 'user_id', - 'tenant_id'] + 'tenant_id', 'project_id', 'project_name'] + COLLISIONS = [('project_name', 'tenant_name'), ('project_id', 'tenant_id')] + + def __str__(self): + """Represent only attributes included in self.ATTRIBUTES""" + attrs = [attr for attr in self.ATTRIBUTES if attr is not 'password'] + _repr = dict((k, getattr(self, k)) for k in attrs) + return str(_repr) + + def __setattr__(self, key, value): + # NOTE(andreaf) In order to ease the migration towards 'project' we + # support v2 credentials configured with 'project' and translate it + # to tenant on the fly. The original kwargs are stored for clients + # that may rely on them. We also set project when tenant is defined + # so clients can rely on project being part of credentials. + parent = super(KeystoneV2Credentials, self) + # for project_* set tenant only + if key == 'project_id': + parent.__setattr__('tenant_id', value) + elif key == 'project_name': + parent.__setattr__('tenant_name', value) + if key == 'tenant_id': + parent.__setattr__('project_id', value) + elif key == 'tenant_name': + parent.__setattr__('project_name', value) + # trigger default behaviour for all attributes + parent.__setattr__(key, value) def is_valid(self): """Check of credentials (no API call) @@ -611,9 +645,6 @@ class KeystoneV2Credentials(Credentials): return None not in (self.username, self.password) -COLLISIONS = [('project_name', 'tenant_name'), ('project_id', 'tenant_id')] - - class KeystoneV3Credentials(Credentials): """Credentials suitable for the Keystone Identity V3 API""" @@ -621,16 +652,7 @@ class KeystoneV3Credentials(Credentials): 'project_domain_id', 'project_domain_name', 'project_id', 'project_name', 'tenant_id', 'tenant_name', 'user_domain_id', 'user_domain_name', 'user_id'] - - def _apply_credentials(self, attr): - for (key1, key2) in COLLISIONS: - val1 = attr.get(key1) - val2 = attr.get(key2) - if val1 and val2 and val1 != val2: - msg = ('Cannot have conflicting values for %s and %s' % - (key1, key2)) - raise exceptions.InvalidCredentials(msg) - super(KeystoneV3Credentials, self)._apply_credentials(attr) + COLLISIONS = [('project_name', 'tenant_name'), ('project_id', 'tenant_id')] def __setattr__(self, key, value): parent = super(KeystoneV3Credentials, self) diff --git a/tempest/tests/common/test_preprov_creds.py b/tempest/tests/common/test_preprov_creds.py index b595c8889b..22bbdd3bfb 100644 --- a/tempest/tests/common/test_preprov_creds.py +++ b/tempest/tests/common/test_preprov_creds.py @@ -27,7 +27,6 @@ from tempest.common import preprov_creds from tempest import config from tempest.lib import auth from tempest.lib import exceptions as lib_exc -from tempest.lib.services.identity.v2 import token_client from tempest.tests import base from tempest.tests import fake_config from tempest.tests.lib import fake_identity @@ -43,40 +42,46 @@ class TestPreProvisionedCredentials(base.TestCase): 'object_storage_operator_role': 'operator', 'object_storage_reseller_admin_role': 'reseller'} + identity_response = fake_identity._fake_v2_response + token_client = ('tempest.lib.services.identity.v2.token_client' + '.TokenClient.raw_request') + + @classmethod + def _fake_accounts(cls, admin_role): + return [ + {'username': 'test_user1', 'tenant_name': 'test_tenant1', + 'password': 'p'}, + {'username': 'test_user2', 'project_name': 'test_tenant2', + 'password': 'p'}, + {'username': 'test_user3', 'tenant_name': 'test_tenant3', + 'password': 'p'}, + {'username': 'test_user4', 'project_name': 'test_tenant4', + 'password': 'p'}, + {'username': 'test_user5', 'tenant_name': 'test_tenant5', + 'password': 'p'}, + {'username': 'test_user6', 'project_name': 'test_tenant6', + 'password': 'p', 'roles': ['role1', 'role2']}, + {'username': 'test_user7', 'tenant_name': 'test_tenant7', + 'password': 'p', 'roles': ['role2', 'role3']}, + {'username': 'test_user8', 'project_name': 'test_tenant8', + 'password': 'p', 'roles': ['role4', 'role1']}, + {'username': 'test_user9', 'tenant_name': 'test_tenant9', + 'password': 'p', 'roles': ['role1', 'role2', 'role3', 'role4']}, + {'username': 'test_user10', 'project_name': 'test_tenant10', + 'password': 'p', 'roles': ['role1', 'role2', 'role3', 'role4']}, + {'username': 'test_user11', 'tenant_name': 'test_tenant11', + 'password': 'p', 'roles': [admin_role]}, + {'username': 'test_user12', 'project_name': 'test_tenant12', + 'password': 'p', 'roles': [admin_role]}] + def setUp(self): super(TestPreProvisionedCredentials, self).setUp() self.useFixture(fake_config.ConfigFixture()) self.patchobject(config, 'TempestConfigPrivate', fake_config.FakePrivate) - self.patchobject(token_client.TokenClient, 'raw_request', - fake_identity._fake_v2_response) + self.patch(self.token_client, side_effect=self.identity_response) self.useFixture(lockutils_fixtures.ExternalLockFixture()) - self.test_accounts = [ - {'username': 'test_user1', 'tenant_name': 'test_tenant1', - 'password': 'p'}, - {'username': 'test_user2', 'tenant_name': 'test_tenant2', - 'password': 'p'}, - {'username': 'test_user3', 'tenant_name': 'test_tenant3', - 'password': 'p'}, - {'username': 'test_user4', 'tenant_name': 'test_tenant4', - 'password': 'p'}, - {'username': 'test_user5', 'tenant_name': 'test_tenant5', - 'password': 'p'}, - {'username': 'test_user6', 'tenant_name': 'test_tenant6', - 'password': 'p', 'roles': ['role1', 'role2']}, - {'username': 'test_user7', 'tenant_name': 'test_tenant7', - 'password': 'p', 'roles': ['role2', 'role3']}, - {'username': 'test_user8', 'tenant_name': 'test_tenant8', - 'password': 'p', 'roles': ['role4', 'role1']}, - {'username': 'test_user9', 'tenant_name': 'test_tenant9', - 'password': 'p', 'roles': ['role1', 'role2', 'role3', 'role4']}, - {'username': 'test_user10', 'tenant_name': 'test_tenant10', - 'password': 'p', 'roles': ['role1', 'role2', 'role3', 'role4']}, - {'username': 'test_user11', 'tenant_name': 'test_tenant11', - 'password': 'p', 'roles': [cfg.CONF.identity.admin_role]}, - {'username': 'test_user12', 'tenant_name': 'test_tenant12', - 'password': 'p', 'roles': [cfg.CONF.identity.admin_role]}, - ] + self.test_accounts = self._fake_accounts(cfg.CONF.identity.admin_role) self.accounts_mock = self.useFixture(mockpatch.Patch( 'tempest.common.preprov_creds.read_accounts_yaml', return_value=self.test_accounts)) @@ -89,24 +94,33 @@ class TestPreProvisionedCredentials(base.TestCase): def _get_hash_list(self, accounts_list): hash_list = [] + hash_fields = ( + preprov_creds.PreProvisionedCredentialProvider.HASH_CRED_FIELDS) for account in accounts_list: hash = hashlib.md5() - hash.update(six.text_type(account).encode('utf-8')) + account_for_hash = dict((k, v) for (k, v) in six.iteritems(account) + if k in hash_fields) + hash.update(six.text_type(account_for_hash).encode('utf-8')) temp_hash = hash.hexdigest() hash_list.append(temp_hash) return hash_list def test_get_hash(self): - self.patchobject(token_client.TokenClient, 'raw_request', - fake_identity._fake_v2_response) - test_account_class = preprov_creds.PreProvisionedCredentialProvider( - **self.fixed_params) - hash_list = self._get_hash_list(self.test_accounts) - test_cred_dict = self.test_accounts[3] - test_creds = auth.get_credentials(fake_identity.FAKE_AUTH_URL, - **test_cred_dict) - results = test_account_class.get_hash(test_creds) - self.assertEqual(hash_list[3], results) + # Test with all accounts to make sure we try all combinations + # and hide no race conditions + hash_index = 0 + for test_cred_dict in self.test_accounts: + test_account_class = ( + preprov_creds.PreProvisionedCredentialProvider( + **self.fixed_params)) + hash_list = self._get_hash_list(self.test_accounts) + test_creds = auth.get_credentials( + fake_identity.FAKE_AUTH_URL, + identity_version=self.fixed_params['identity_version'], + **test_cred_dict) + results = test_account_class.get_hash(test_creds) + self.assertEqual(hash_list[hash_index], results) + hash_index += 1 def test_get_hash_dict(self): test_account_class = preprov_creds.PreProvisionedCredentialProvider( @@ -331,3 +345,53 @@ class TestPreProvisionedCredentials(base.TestCase): self.assertIn('id', network) self.assertEqual('fake-id', network['id']) self.assertEqual('network-2', network['name']) + + +class TestPreProvisionedCredentialsV3(TestPreProvisionedCredentials): + + fixed_params = {'name': 'test class', + 'identity_version': 'v3', + 'test_accounts_file': 'fake_accounts_file', + 'accounts_lock_dir': 'fake_locks_dir', + 'admin_role': 'admin', + 'object_storage_operator_role': 'operator', + 'object_storage_reseller_admin_role': 'reseller'} + + identity_response = fake_identity._fake_v3_response + token_client = ('tempest.lib.services.identity.v3.token_client' + '.V3TokenClient.raw_request') + + @classmethod + def _fake_accounts(cls, admin_role): + return [ + {'username': 'test_user1', 'project_name': 'test_project1', + 'domain_name': 'domain', 'password': 'p'}, + {'username': 'test_user2', 'project_name': 'test_project2', + 'domain_name': 'domain', 'password': 'p'}, + {'username': 'test_user3', 'project_name': 'test_project3', + 'domain_name': 'domain', 'password': 'p'}, + {'username': 'test_user4', 'project_name': 'test_project4', + 'domain_name': 'domain', 'password': 'p'}, + {'username': 'test_user5', 'project_name': 'test_project5', + 'domain_name': 'domain', 'password': 'p'}, + {'username': 'test_user6', 'project_name': 'test_project6', + 'domain_name': 'domain', 'password': 'p', + 'roles': ['role1', 'role2']}, + {'username': 'test_user7', 'project_name': 'test_project7', + 'domain_name': 'domain', 'password': 'p', + 'roles': ['role2', 'role3']}, + {'username': 'test_user8', 'project_name': 'test_project8', + 'domain_name': 'domain', 'password': 'p', + 'roles': ['role4', 'role1']}, + {'username': 'test_user9', 'project_name': 'test_project9', + 'domain_name': 'domain', 'password': 'p', + 'roles': ['role1', 'role2', 'role3', 'role4']}, + {'username': 'test_user10', 'project_name': 'test_project10', + 'domain_name': 'domain', 'password': 'p', + 'roles': ['role1', 'role2', 'role3', 'role4']}, + {'username': 'test_user11', 'project_name': 'test_project11', + 'domain_name': 'domain', 'password': 'p', + 'roles': [admin_role]}, + {'username': 'test_user12', 'project_name': 'test_project12', + 'domain_name': 'domain', 'password': 'p', + 'roles': [admin_role]}] diff --git a/tempest/tests/lib/test_credentials.py b/tempest/tests/lib/test_credentials.py index ca3baa135c..b6f2cf602a 100644 --- a/tempest/tests/lib/test_credentials.py +++ b/tempest/tests/lib/test_credentials.py @@ -36,8 +36,10 @@ class CredentialsTests(base.TestCase): # Check the right version of credentials has been returned self.assertIsInstance(credentials, credentials_class) # Check the id attributes are filled in + # NOTE(andreaf) project_* attributes are accepted as input but + # never set on the credentials object attributes = [x for x in credentials.ATTRIBUTES if ( - '_id' in x and x != 'domain_id')] + '_id' in x and x != 'domain_id' and x != 'project_id')] for attr in attributes: if filled: self.assertIsNotNone(getattr(credentials, attr))