From cd0bbbdad37a31248d479ef78df948da0a1e850e Mon Sep 17 00:00:00 2001 From: Colleen Murphy Date: Tue, 1 Oct 2019 16:18:36 -0700 Subject: [PATCH] Add system scope for admin auth Keystone is moving away from using either project-scope or domain-scope for the main cloud administrator user, and instead moving toward the admin user having a role assignment on the "system" scope[1]. This will mean that no particular project or domain is special, and instead the cloud administrator scopes to the system in order to make deployment-wide changes. Keystone has now migrated all of its policies to understand system scope[2], and if a deployment sets [oslo_policy]/enforce_scope=true in keystone.conf and uses the new policies, an admin user scoped to the admin project will not be able to create dynamic credentials for tempest. This patch adds a new parameter ``[auth]/admin_system`` to indicate that neither the ``admin_project`` or ``admin_domain`` parameters apply to the admin user and that the user should instead authenticate with the system scope. This also adds ``admin_user_domain_name`` so that the admin user can be found in its domain (namespace) without setting ``domain_name``, and for completeness also adds ``admin_project_domain_name`` so that ``domain_name`` could be omitted even if using project scope. [1] http://specs.openstack.org/openstack/keystone-specs/specs/keystone/queens/system-scope.html [2] https://bugs.launchpad.net/keystone/+bugs?field.status%3Alist=FIXRELEASED&field.tag=system-scope Depends-on: https://review.opendev.org/739262 Change-Id: I840b273c37ca7cc4592c43813abfb424337e2836 --- tempest/common/credentials_factory.py | 6 ++- tempest/config.py | 19 +++++++- tempest/lib/auth.py | 20 ++++++--- tempest/lib/common/cred_client.py | 32 +++++++++++--- tempest/lib/common/dynamic_creds.py | 9 +++- .../lib/services/identity/v3/token_client.py | 6 ++- .../tests/common/test_credentials_factory.py | 43 +++++++++++++++---- tempest/tests/lib/common/test_cred_client.py | 38 +++++++++++++++- tempest/tests/lib/test_auth.py | 13 ++++++ 9 files changed, 160 insertions(+), 26 deletions(-) diff --git a/tempest/common/credentials_factory.py b/tempest/common/credentials_factory.py index c6e5dcb85e..2d486a7ac9 100644 --- a/tempest/common/credentials_factory.py +++ b/tempest/common/credentials_factory.py @@ -245,6 +245,9 @@ def get_configured_admin_credentials(fill_in=True, identity_version=None): if identity_version == 'v3': conf_attributes.append('domain_name') + conf_attributes.append('user_domain_name') + conf_attributes.append('project_domain_name') + conf_attributes.append('system') # Read the parts of credentials from config params = config.service_client_config() for attr in conf_attributes: @@ -284,7 +287,8 @@ def get_credentials(fill_in=True, identity_version=None, **kwargs): if identity_version == 'v3': domain_fields = set(x for x in auth.KeystoneV3Credentials.ATTRIBUTES if 'domain' in x) - if not domain_fields.intersection(kwargs.keys()): + if (not params.get('system') and + not domain_fields.intersection(kwargs.keys())): domain_name = CONF.auth.default_credentials_domain_name # NOTE(andreaf) Setting domain_name implicitly sets user and # project domain names, if they are None diff --git a/tempest/config.py b/tempest/config.py index 382b80f11e..7c4539cf5e 100644 --- a/tempest/config.py +++ b/tempest/config.py @@ -92,7 +92,24 @@ AuthGroup = [ cfg.StrOpt('admin_domain_name', default='Default', help="Admin domain name for authentication (Keystone V3). " - "The same domain applies to user and project"), + "The same domain applies to user and project if " + "admin_user_domain_name and admin_project_domain_name " + "are not specified"), + cfg.StrOpt('admin_user_domain_name', + help="Domain name that contains the admin user (Keystone V3). " + "May be different from admin_project_domain_name and " + "admin_domain_name"), + cfg.StrOpt('admin_project_domain_name', + help="Domain name that contains the project given by " + "admin_project_name (Keystone V3). May be different from " + "admin_user_domain_name and admin_domain_name"), + cfg.StrOpt('admin_system', + default=None, + help="The system scope on which an admin user has an admin " + "role assignment, if any. Valid values are 'all' or None. " + "This must be set to 'all' if using the " + "[oslo_policy]/enforce_scope=true option for the " + "identity service."), ] identity_group = cfg.OptGroup(name='identity', diff --git a/tempest/lib/auth.py b/tempest/lib/auth.py index 7c279ab289..9f8c7c5eca 100644 --- a/tempest/lib/auth.py +++ b/tempest/lib/auth.py @@ -428,7 +428,7 @@ class KeystoneV2AuthProvider(KeystoneAuthProvider): class KeystoneV3AuthProvider(KeystoneAuthProvider): """Provides authentication based on the Identity V3 API""" - SCOPES = set(['project', 'domain', 'unscoped', None]) + SCOPES = set(['system', 'project', 'domain', 'unscoped', None]) def _auth_client(self, auth_url): return json_v3id.V3TokenClient( @@ -441,8 +441,8 @@ class KeystoneV3AuthProvider(KeystoneAuthProvider): Fields available in Credentials are passed to the token request, depending on the value of scope. Valid values for scope are: "project", - "domain". Any other string (e.g. "unscoped") or None will lead to an - unscoped token request. + "domain", or "system". Any other string (e.g. "unscoped") or None will + lead to an unscoped token request. """ auth_params = dict( @@ -465,12 +465,16 @@ class KeystoneV3AuthProvider(KeystoneAuthProvider): domain_id=self.credentials.domain_id, domain_name=self.credentials.domain_name) + if self.scope == 'system': + auth_params.update(system='all') + return auth_params def _fill_credentials(self, auth_data_body): - # project or domain, depending on the scope + # project, domain, or system depending on the scope project = auth_data_body.get('project', None) domain = auth_data_body.get('domain', None) + system = auth_data_body.get('system', None) # user is always there user = auth_data_body['user'] # Set project fields @@ -490,6 +494,9 @@ class KeystoneV3AuthProvider(KeystoneAuthProvider): self.credentials.domain_id = domain['id'] if self.credentials.domain_name is None: self.credentials.domain_name = domain['name'] + # Set system scope + if system is not None: + self.credentials.system = 'all' # Set user fields if self.credentials.username is None: self.credentials.username = user['name'] @@ -677,7 +684,8 @@ class Credentials(object): raise exceptions.InvalidCredentials(msg) for key in attr: if key in self.ATTRIBUTES: - setattr(self, key, attr[key]) + if attr[key] is not None: + setattr(self, key, attr[key]) else: msg = '%s is not a valid attr for %s' % (key, self.__class__) raise exceptions.InvalidCredentials(msg) @@ -779,7 +787,7 @@ class KeystoneV3Credentials(Credentials): ATTRIBUTES = ['domain_id', 'domain_name', 'password', 'username', 'project_domain_id', 'project_domain_name', 'project_id', 'project_name', 'tenant_id', 'tenant_name', 'user_domain_id', - 'user_domain_name', 'user_id'] + 'user_domain_name', 'user_id', 'system'] COLLISIONS = [('project_name', 'tenant_name'), ('project_id', 'tenant_id')] def __setattr__(self, key, value): diff --git a/tempest/lib/common/cred_client.py b/tempest/lib/common/cred_client.py index a81f53c9cd..3af09a00fe 100644 --- a/tempest/lib/common/cred_client.py +++ b/tempest/lib/common/cred_client.py @@ -83,12 +83,15 @@ class CredsClient(object): role['id'], project['id'], user['id']) @abc.abstractmethod - def get_credentials(self, user, project, password): + def get_credentials( + self, user, project, password, domain=None, system=None): """Produces a Credentials object from the details provided :param user: a user dict - :param project: a project dict + :param project: a project dict or None if using domain or system scope :param password: the password as a string + :param domain: a domain dict + :param system: a system dict :return: a Credentials object with all the available credential details """ pass @@ -116,7 +119,8 @@ class V2CredsClient(CredsClient): def delete_project(self, project_id): self.projects_client.delete_tenant(project_id) - def get_credentials(self, user, project, password): + def get_credentials( + self, user, project, password, domain=None, system=None): # User and project already include both ID and name here, # so there's no need to use the fill_in mode return auth.get_credentials( @@ -156,23 +160,37 @@ class V3CredsClient(CredsClient): def delete_project(self, project_id): self.projects_client.delete_project(project_id) - def get_credentials(self, user, project, password): + def get_credentials( + self, user, project, password, domain=None, system=None): # User, project and domain already include both ID and name here, # so there's no need to use the fill_in mode. # NOTE(andreaf) We need to set all fields in the returned credentials. # Scope is then used to pick only those relevant for the type of # token needed by each service client. + if project: + project_name = project['name'] + project_id = project['id'] + else: + project_name = None + project_id = None + if domain: + domain_name = domain['name'] + domain_id = domain['id'] + else: + domain_name = self.creds_domain['name'] + domain_id = self.creds_domain['id'] return auth.get_credentials( auth_url=None, fill_in=False, identity_version='v3', username=user['name'], user_id=user['id'], - project_name=project['name'], project_id=project['id'], + project_name=project_name, project_id=project_id, password=password, project_domain_id=self.creds_domain['id'], project_domain_name=self.creds_domain['name'], - domain_id=self.creds_domain['id'], - domain_name=self.creds_domain['name']) + domain_id=domain_id, + domain_name=domain_name, + system=system) def assign_user_role_on_domain(self, user, role_name, domain=None): """Assign the specified role on a domain diff --git a/tempest/lib/common/dynamic_creds.py b/tempest/lib/common/dynamic_creds.py index 8b82391653..0a2817b203 100644 --- a/tempest/lib/common/dynamic_creds.py +++ b/tempest/lib/common/dynamic_creds.py @@ -142,7 +142,14 @@ class DynamicCredentialProvider(cred_provider.CredentialProvider): else: # We use a dedicated client manager for identity client in case we # need a different token scope for them. - scope = 'domain' if self.identity_admin_domain_scope else 'project' + if self.default_admin_creds.system: + scope = 'system' + elif (self.default_admin_creds.domain_id or + self.default_admin_creds.domain_name or + self.identity_admin_domain_scope): + scope = 'domain' + else: + scope = 'project' identity_os = clients.ServiceClients(self.default_admin_creds, self.identity_uri, scope=scope) diff --git a/tempest/lib/services/identity/v3/token_client.py b/tempest/lib/services/identity/v3/token_client.py index 69562977ce..08a8f46fb9 100644 --- a/tempest/lib/services/identity/v3/token_client.py +++ b/tempest/lib/services/identity/v3/token_client.py @@ -51,7 +51,7 @@ class V3TokenClient(rest_client.RestClient): def auth(self, user_id=None, username=None, password=None, project_id=None, project_name=None, user_domain_id=None, user_domain_name=None, project_domain_id=None, project_domain_name=None, domain_id=None, - domain_name=None, token=None, app_cred_id=None, + domain_name=None, system=None, token=None, app_cred_id=None, app_cred_secret=None): """Obtains a token from the authentication service @@ -65,6 +65,7 @@ class V3TokenClient(rest_client.RestClient): :param domain_name: a domain name to scope to :param project_id: a project id to scope to :param project_name: a project name to scope to + :param system: whether the token should be scoped to the system :param token: a token to re-scope. Accepts different combinations of credentials. @@ -74,6 +75,7 @@ class V3TokenClient(rest_client.RestClient): - user_id, password - username, password, user_domain_id - username, password, project_name, user_domain_id, project_domain_id + - username, password, user_domain_id, system Validation is left to the server side. """ creds = { @@ -135,6 +137,8 @@ class V3TokenClient(rest_client.RestClient): creds['auth']['scope'] = dict(domain={'id': domain_id}) elif domain_name: creds['auth']['scope'] = dict(domain={'name': domain_name}) + elif system: + creds['auth']['scope'] = dict(system={system: True}) body = json.dumps(creds, sort_keys=True) resp, body = self.post(self.auth_url, body=body) diff --git a/tempest/tests/common/test_credentials_factory.py b/tempest/tests/common/test_credentials_factory.py index 0ef374292d..374474d898 100644 --- a/tempest/tests/common/test_credentials_factory.py +++ b/tempest/tests/common/test_credentials_factory.py @@ -173,10 +173,15 @@ class TestCredentialsFactory(base.TestCase): @mock.patch.object(cf, 'get_credentials') def test_get_configured_admin_credentials(self, mock_get_credentials): cfg.CONF.set_default('auth_version', 'v3', 'identity') - all_params = [('admin_username', 'username', 'my_name'), - ('admin_password', 'password', 'secret'), - ('admin_project_name', 'project_name', 'my_pname'), - ('admin_domain_name', 'domain_name', 'my_dname')] + all_params = [ + ('admin_username', 'username', 'my_name'), + ('admin_user_domain_name', 'user_domain_name', 'my_dname'), + ('admin_password', 'password', 'secret'), + ('admin_project_name', 'project_name', 'my_pname'), + ('admin_project_domain_name', 'project_domain_name', 'my_dname'), + ('admin_domain_name', 'domain_name', 'my_dname'), + ('admin_system', 'system', None), + ] expected_result = 'my_admin_credentials' mock_get_credentials.return_value = expected_result for config_item, _, value in all_params: @@ -194,10 +199,15 @@ class TestCredentialsFactory(base.TestCase): def test_get_configured_admin_credentials_not_fill_valid( self, mock_get_credentials): cfg.CONF.set_default('auth_version', 'v2', 'identity') - all_params = [('admin_username', 'username', 'my_name'), - ('admin_password', 'password', 'secret'), - ('admin_project_name', 'project_name', 'my_pname'), - ('admin_domain_name', 'domain_name', 'my_dname')] + all_params = [ + ('admin_username', 'username', 'my_name'), + ('admin_user_domain_name', 'user_domain_name', 'my_dname'), + ('admin_password', 'password', 'secret'), + ('admin_project_domain_name', 'project_domain_name', 'my_dname'), + ('admin_project_name', 'project_name', 'my_pname'), + ('admin_domain_name', 'domain_name', 'my_dname'), + ('admin_system', 'system', None), + ] expected_result = mock.Mock() expected_result.is_valid.return_value = True mock_get_credentials.return_value = expected_result @@ -278,3 +288,20 @@ class TestCredentialsFactory(base.TestCase): mock_auth_get_credentials.assert_called_once_with( expected_uri, fill_in=False, identity_version='v3', **expected_params) + + @mock.patch('tempest.lib.auth.get_credentials') + def test_get_credentials_v3_system(self, mock_auth_get_credentials): + expected_uri = 'V3_URI' + expected_result = 'my_creds' + mock_auth_get_credentials.return_value = expected_result + cfg.CONF.set_default('uri_v3', expected_uri, 'identity') + cfg.CONF.set_default('admin_system', 'all', 'auth') + params = {'system': 'all'} + expected_params = params.copy() + expected_params.update(config.service_client_config()) + result = cf.get_credentials(fill_in=False, identity_version='v3', + **params) + self.assertEqual(expected_result, result) + mock_auth_get_credentials.assert_called_once_with( + expected_uri, fill_in=False, identity_version='v3', + **expected_params) diff --git a/tempest/tests/lib/common/test_cred_client.py b/tempest/tests/lib/common/test_cred_client.py index 860a4654ce..b99311cf3d 100644 --- a/tempest/tests/lib/common/test_cred_client.py +++ b/tempest/tests/lib/common/test_cred_client.py @@ -43,6 +43,14 @@ class TestCredClientV2(base.TestCase): self.projects_client.delete_tenant.assert_called_once_with( 'fake_id') + def test_get_credentials(self): + ret = self.creds_client.get_credentials( + {'name': 'some_user', 'id': 'fake_id'}, + {'name': 'some_project', 'id': 'fake_id'}, + 'password123') + self.assertEqual(ret.username, 'some_user') + self.assertEqual(ret.project_name, 'some_project') + class TestCredClientV3(base.TestCase): def setUp(self): @@ -53,7 +61,7 @@ class TestCredClientV3(base.TestCase): self.roles_client = mock.MagicMock() self.domains_client = mock.MagicMock() self.domains_client.list_domains.return_value = { - 'domains': [{'id': 'fake_domain_id'}] + 'domains': [{'id': 'fake_domain_id', 'name': 'some_domain'}] } self.creds_client = cred_client.V3CredsClient(self.identity_client, self.projects_client, @@ -75,3 +83,31 @@ class TestCredClientV3(base.TestCase): self.creds_client.delete_project('fake_id') self.projects_client.delete_project.assert_called_once_with( 'fake_id') + + def test_get_credentials(self): + ret = self.creds_client.get_credentials( + {'name': 'some_user', 'id': 'fake_id'}, + {'name': 'some_project', 'id': 'fake_id'}, + 'password123') + self.assertEqual(ret.username, 'some_user') + self.assertEqual(ret.project_name, 'some_project') + self.assertIsNone(ret.system) + self.assertEqual(ret.domain_name, 'some_domain') + ret = self.creds_client.get_credentials( + {'name': 'some_user', 'id': 'fake_id'}, + None, + 'password123', + domain={'name': 'another_domain', 'id': 'another_id'}) + self.assertEqual(ret.username, 'some_user') + self.assertIsNone(ret.project_name) + self.assertIsNone(ret.system) + self.assertEqual(ret.domain_name, 'another_domain') + ret = self.creds_client.get_credentials( + {'name': 'some_user', 'id': 'fake_id'}, + None, + 'password123', + system={'system': 'all'}) + self.assertEqual(ret.username, 'some_user') + self.assertIsNone(ret.project_name) + self.assertEqual(ret.system, {'system': 'all'}) + self.assertEqual(ret.domain_name, 'some_domain') diff --git a/tempest/tests/lib/test_auth.py b/tempest/tests/lib/test_auth.py index c3a792f41c..3edb122f02 100644 --- a/tempest/tests/lib/test_auth.py +++ b/tempest/tests/lib/test_auth.py @@ -786,6 +786,19 @@ class TestKeystoneV3AuthProvider(TestKeystoneV2AuthProvider): self.assertIn(attr, auth_params.keys()) self.assertEqual(getattr(all_creds, attr), auth_params[attr]) + def test_auth_parameters_with_system_scope(self): + all_creds = fake_credentials.FakeKeystoneV3AllCredentials() + self.auth_provider.credentials = all_creds + self.auth_provider.scope = 'system' + auth_params = self.auth_provider._auth_params() + self.assertNotIn('scope', auth_params.keys()) + for attr in all_creds.get_init_attributes(): + if attr.startswith('project_') or attr.startswith('domain_'): + self.assertNotIn(attr, auth_params.keys()) + else: + self.assertIn(attr, auth_params.keys()) + self.assertEqual(getattr(all_creds, attr), auth_params[attr]) + class TestKeystoneV3Credentials(base.TestCase): def testSetAttrUserDomain(self):