From 01a6c7db3e13c21362a39bbba4543ef020906387 Mon Sep 17 00:00:00 2001 From: Brant Knudson Date: Tue, 31 Dec 2013 18:30:17 -0600 Subject: [PATCH] Fix using non-default default_domain_id Changing the default_domain_id to something other than the default would cause several operations to not work correctly. This is because the value of default_domain_id was read and cached in several parts on import, which is before the call to CONF() so the cached value is the original default ('default') rather than the value that the operator has in the config file. This change removes the cached default_domain_id and instead reads the value when it's needed at runtime so that the user-configured value is used instead of the original default. Change-Id: I9543e2108866ed66000d75f868bbd2b1ed978b97 Closes-Bug: #1265108 --- keystone/assignment/backends/ldap.py | 4 ++-- keystone/assignment/controllers.py | 7 +++---- keystone/assignment/core.py | 14 ++++++++------ keystone/common/controller.py | 7 +++---- .../versions/008_create_default_domain.py | 6 +++--- .../versions/016_normalize_domain_ids.py | 9 ++++----- keystone/identity/controllers.py | 4 ++-- keystone/tests/default_fixtures.py | 2 +- keystone/tests/test_auth.py | 13 ++----------- keystone/tests/test_backend_ldap.py | 15 ++++----------- keystone/tests/test_v3_auth.py | 9 +-------- keystone/token/controllers.py | 7 +++---- keystone/token/providers/common.py | 14 ++++++++------ 13 files changed, 44 insertions(+), 67 deletions(-) diff --git a/keystone/assignment/backends/ldap.py b/keystone/assignment/backends/ldap.py index 5985cb45f1..2c71fb21f6 100644 --- a/keystone/assignment/backends/ldap.py +++ b/keystone/assignment/backends/ldap.py @@ -202,7 +202,7 @@ class Assignment(assignment.Driver): def get_domain(self, domain_id): self._validate_default_domain_id(domain_id) - return assignment.DEFAULT_DOMAIN + return assignment.calc_default_domain() def update_domain(self, domain_id, domain): self._validate_default_domain_id(domain_id) @@ -213,7 +213,7 @@ class Assignment(assignment.Driver): raise exception.Forbidden('Domains are read-only against LDAP') def list_domains(self): - return [assignment.DEFAULT_DOMAIN] + return [assignment.calc_default_domain()] #Bulk actions on User From identity def delete_user(self, user_id): diff --git a/keystone/assignment/controllers.py b/keystone/assignment/controllers.py index 5683e06f20..d52aea2f99 100644 --- a/keystone/assignment/controllers.py +++ b/keystone/assignment/controllers.py @@ -30,7 +30,6 @@ from keystone.openstack.common import log CONF = config.CONF -DEFAULT_DOMAIN_ID = CONF.identity.default_domain_id LOG = log.getLogger(__name__) @@ -74,7 +73,7 @@ class Tenant(controller.V2Controller): tenant_refs = ( self.assignment_api.list_projects_for_user(user_ref['id'])) tenant_refs = [self.filter_domain_id(ref) for ref in tenant_refs - if ref['domain_id'] == DEFAULT_DOMAIN_ID] + if ref['domain_id'] == CONF.identity.default_domain_id] params = { 'limit': context['query_string'].get('limit'), 'marker': context['query_string'].get('marker'), @@ -92,7 +91,7 @@ class Tenant(controller.V2Controller): def get_project_by_name(self, context, tenant_name): self.assert_admin(context) ref = self.assignment_api.get_project_by_name( - tenant_name, DEFAULT_DOMAIN_ID) + tenant_name, CONF.identity.default_domain_id) return {'tenant': self.filter_domain_id(ref)} # CRUD Extension @@ -281,7 +280,7 @@ class Role(controller.V2Controller): for tenant in tenants: # As a v2 call, we should limit the response to those projects in # the default domain. - if tenant['domain_id'] != DEFAULT_DOMAIN_ID: + if tenant['domain_id'] != CONF.identity.default_domain_id: continue role_ids = self.assignment_api.get_roles_for_user_and_project( user_id, tenant['id']) diff --git a/keystone/assignment/core.py b/keystone/assignment/core.py index c3275b1f51..b3351b84e6 100644 --- a/keystone/assignment/core.py +++ b/keystone/assignment/core.py @@ -34,12 +34,14 @@ CONF = config.CONF LOG = log.getLogger(__name__) SHOULD_CACHE = cache.should_cache_fn('assignment') -DEFAULT_DOMAIN = {'description': - (u'Owns users and tenants (i.e. projects)' - ' available on Identity API v2.'), - 'enabled': True, - 'id': CONF.identity.default_domain_id, - 'name': u'Default'} + +def calc_default_domain(): + return {'description': + (u'Owns users and tenants (i.e. projects)' + ' available on Identity API v2.'), + 'enabled': True, + 'id': CONF.identity.default_domain_id, + 'name': u'Default'} @dependency.provider('assignment_api') diff --git a/keystone/common/controller.py b/keystone/common/controller.py index 88c3afd1a8..e5bfef11ab 100644 --- a/keystone/common/controller.py +++ b/keystone/common/controller.py @@ -28,7 +28,6 @@ from keystone.openstack.common import versionutils LOG = log.getLogger(__name__) CONF = config.CONF -DEFAULT_DOMAIN_ID = CONF.identity.default_domain_id v2_deprecated = versionutils.deprecated(what='v2 API', as_of=versionutils.deprecated.ICEHOUSE, @@ -227,7 +226,7 @@ class V2Controller(wsgi.Application): specified in the v2 call. """ - ref['domain_id'] = DEFAULT_DOMAIN_ID + ref['domain_id'] = CONF.identity.default_domain_id return ref @staticmethod @@ -347,7 +346,7 @@ class V3Controller(wsgi.Application): """Get the domain_id for a v3 call.""" if context['is_admin']: - return DEFAULT_DOMAIN_ID + return CONF.identity.default_domain_id # Fish the domain_id out of the token # @@ -365,7 +364,7 @@ class V3Controller(wsgi.Application): if 'domain' in token_ref: return token_ref['domain']['id'] else: - return DEFAULT_DOMAIN_ID + return CONF.identity.default_domain_id def _normalize_domain_id(self, context, ref): """Fill in domain_id if not specified in a v3 call.""" diff --git a/keystone/common/sql/migrate_repo/versions/008_create_default_domain.py b/keystone/common/sql/migrate_repo/versions/008_create_default_domain.py index 23153225d9..f28d2ee38a 100644 --- a/keystone/common/sql/migrate_repo/versions/008_create_default_domain.py +++ b/keystone/common/sql/migrate_repo/versions/008_create_default_domain.py @@ -23,7 +23,6 @@ from keystone import config CONF = config.CONF -DEFAULT_DOMAIN_ID = CONF.identity.default_domain_id def upgrade(migrate_engine): @@ -34,7 +33,7 @@ def upgrade(migrate_engine): domain_table = sql.Table('domain', meta, autoload=True) domain = { - 'id': DEFAULT_DOMAIN_ID, + 'id': CONF.identity.default_domain_id, 'name': 'Default', 'enabled': True, 'extra': json.dumps({ @@ -55,5 +54,6 @@ def downgrade(migrate_engine): sql.Table('domain', meta, autoload=True) session = orm.sessionmaker(bind=migrate_engine)() session.execute( - 'DELETE FROM domain WHERE id=:id', {'id': DEFAULT_DOMAIN_ID}) + 'DELETE FROM domain WHERE id=:id', + {'id': CONF.identity.default_domain_id}) session.commit() diff --git a/keystone/common/sql/migrate_repo/versions/016_normalize_domain_ids.py b/keystone/common/sql/migrate_repo/versions/016_normalize_domain_ids.py index bf1ed6524e..5c0586e623 100644 --- a/keystone/common/sql/migrate_repo/versions/016_normalize_domain_ids.py +++ b/keystone/common/sql/migrate_repo/versions/016_normalize_domain_ids.py @@ -44,7 +44,6 @@ from keystone import config CONF = config.CONF -DEFAULT_DOMAIN_ID = CONF.identity.default_domain_id def _disable_foreign_constraints(session, migrate_engine): @@ -121,7 +120,7 @@ def upgrade_user_table_with_copy(meta, migrate_engine, session): 'extra': user.extra, 'password': user.password, 'enabled': user.enabled, - 'domain_id': DEFAULT_DOMAIN_ID}) + 'domain_id': CONF.identity.default_domain_id}) _enable_foreign_constraints(session, migrate_engine) session.execute('drop table temp_user;') @@ -190,7 +189,7 @@ def upgrade_project_table_with_copy(meta, migrate_engine, session): 'extra': project.extra, 'description': project.description, 'enabled': project.enabled, - 'domain_id': DEFAULT_DOMAIN_ID}) + 'domain_id': CONF.identity.default_domain_id}) _enable_foreign_constraints(session, migrate_engine) session.execute('drop table temp_project;') @@ -333,7 +332,7 @@ def upgrade_user_table_with_col_create(meta, migrate_engine, session): sql.Column('domain_id', sql.String(64), sql.ForeignKey('domain.id'), nullable=True)) for user in session.query(user_table).all(): - values = {'domain_id': DEFAULT_DOMAIN_ID} + values = {'domain_id': CONF.identity.default_domain_id} update = user_table.update().\ where(user_table.c.id == user.id).\ values(values) @@ -362,7 +361,7 @@ def upgrade_project_table_with_col_create(meta, migrate_engine, session): sql.Column('domain_id', sql.String(64), sql.ForeignKey('domain.id'), nullable=True)) for project in session.query(project_table).all(): - values = {'domain_id': DEFAULT_DOMAIN_ID} + values = {'domain_id': CONF.identity.default_domain_id} update = project_table.update().\ where(project_table.c.id == project.id).\ values(values) diff --git a/keystone/identity/controllers.py b/keystone/identity/controllers.py index f67716b404..1d35c2ec28 100644 --- a/keystone/identity/controllers.py +++ b/keystone/identity/controllers.py @@ -30,7 +30,6 @@ from keystone.openstack.common import versionutils CONF = config.CONF -DEFAULT_DOMAIN_ID = CONF.identity.default_domain_id LOG = log.getLogger(__name__) @@ -116,7 +115,8 @@ class User(controller.V2Controller): @controller.v2_deprecated def get_user_by_name(self, context, user_name): self.assert_admin(context) - ref = self.identity_api.get_user_by_name(user_name, DEFAULT_DOMAIN_ID) + ref = self.identity_api.get_user_by_name( + user_name, CONF.identity.default_domain_id) return {'user': self.identity_api.v3_to_v2_user(ref)} # CRUD extension diff --git a/keystone/tests/default_fixtures.py b/keystone/tests/default_fixtures.py index b28463fd02..98875bd75b 100644 --- a/keystone/tests/default_fixtures.py +++ b/keystone/tests/default_fixtures.py @@ -121,4 +121,4 @@ ROLES = [ } ] -DOMAINS = [assignment.DEFAULT_DOMAIN] +DOMAINS = [assignment.calc_default_domain()] diff --git a/keystone/tests/test_auth.py b/keystone/tests/test_auth.py index 7ceb4c7807..0cca2aae27 100644 --- a/keystone/tests/test_auth.py +++ b/keystone/tests/test_auth.py @@ -545,17 +545,8 @@ class AuthWithPasswordCredentials(AuthTest): username=self.user_foo['name'], password=new_user_password) - # TODO(blk-u): The following should be - # self.controller.authenticate({}, body_dict) - # We should not expect Unauthorized because the authorizor code should - # be looking up the user in the new default domain, but it's using the - # old domain because it's storing the domain_id statically. - # See bug 1265108 - - self.assertRaises( - exception.Unauthorized, - self.controller.authenticate, - {}, body_dict) + # The test is successful if this doesn't raise, so no need to assert. + self.controller.authenticate({}, body_dict) class AuthWithRemoteUser(AuthTest): diff --git a/keystone/tests/test_backend_ldap.py b/keystone/tests/test_backend_ldap.py index 3b2d203469..4975394822 100644 --- a/keystone/tests/test_backend_ldap.py +++ b/keystone/tests/test_backend_ldap.py @@ -284,25 +284,18 @@ class BaseLDAPIdentity(test_backend.IdentityTests): domains = self.assignment_api.list_domains() self.assertEqual( domains, - [assignment.DEFAULT_DOMAIN]) + [assignment.calc_default_domain()]) def test_list_domains_non_default_domain_id(self): # If change the default_domain_id, the ID of the default domain # returned by list_domains changes is the new default_domain_id. - orig_default_domain_id = CONF.identity.default_domain_id - new_domain_id = uuid.uuid4().hex self.opt_in_group('identity', default_domain_id=new_domain_id) domains = self.assignment_api.list_domains() - # TODO(blk-u): The following should be - # self.assertEqual(domains[0]['id'], new_domain_id) - # but the domain ID doesn't change because some parts are keeping - # references to the old config value. See bug 1265108. - - self.assertEqual(domains[0]['id'], orig_default_domain_id) + self.assertEqual(domains[0]['id'], new_domain_id) def test_authenticate_requires_simple_bind(self): user = { @@ -1052,7 +1045,7 @@ class LdapIdentitySqlAssignment(sql.Base, tests.TestCase, BaseLDAPIdentity): def test_list_domains(self): domains = self.assignment_api.list_domains() - self.assertEqual(domains, [assignment.DEFAULT_DOMAIN]) + self.assertEqual(domains, [assignment.calc_default_domain()]) def test_list_domains_non_default_domain_id(self): # If change the default_domain_id, the ID of the default domain @@ -1149,7 +1142,7 @@ class MultiLDAPandSQLIdentity(sql.Base, tests.TestCase, BaseLDAPIdentity): self.assignment_api.get_domain_by_name(domain['name'])) return ref - self.domain_default = create_domain(assignment.DEFAULT_DOMAIN) + self.domain_default = create_domain(assignment.calc_default_domain()) self.domain1 = create_domain( {'id': uuid.uuid4().hex, 'name': 'domain1'}) self.domain2 = create_domain( diff --git a/keystone/tests/test_v3_auth.py b/keystone/tests/test_v3_auth.py index 3be9c7a615..e9c3946475 100644 --- a/keystone/tests/test_v3_auth.py +++ b/keystone/tests/test_v3_auth.py @@ -188,17 +188,10 @@ class TestPKITokenAPIs(test_v3.RestfulTestCase): # 5) Authenticate token using v2 api. - # TODO(blk-u): The following should work (remove expected_status=401). - # We should not expect Unauthorized because the authorizer code should - # be looking up the user in the new default domain, but it's using the - # old domain because it's storing the domain_id statically. - # See bug 1265108 - path = '/v2.0/tokens/%s' % (token) resp = self.admin_request(path=path, token='ADMIN', - method='GET', - expected_status=401) + method='GET') def test_v3_v2_intermix_domain_scoped_token_failed(self): # grant the domain role to user diff --git a/keystone/token/controllers.py b/keystone/token/controllers.py index 2704775317..ff9b88e016 100644 --- a/keystone/token/controllers.py +++ b/keystone/token/controllers.py @@ -29,7 +29,6 @@ from keystone.token import core CONF = config.CONF LOG = log.getLogger(__name__) -DEFAULT_DOMAIN_ID = CONF.identity.default_domain_id class ExternalAuthNotApplicable(Exception): @@ -259,7 +258,7 @@ class Auth(controller.V2Controller): if username: try: user_ref = self.identity_api.get_user_by_name( - username, DEFAULT_DOMAIN_ID) + username, CONF.identity.default_domain_id) user_id = user_ref['id'] except exception.UserNotFound as e: raise exception.Unauthorized(e) @@ -295,7 +294,7 @@ class Auth(controller.V2Controller): username = context['environment']['REMOTE_USER'] try: user_ref = self.identity_api.get_user_by_name( - username, DEFAULT_DOMAIN_ID) + username, CONF.identity.default_domain_id) user_id = user_ref['id'] except exception.UserNotFound as e: raise exception.Unauthorized(e) @@ -338,7 +337,7 @@ class Auth(controller.V2Controller): if tenant_name: try: tenant_ref = self.assignment_api.get_project_by_name( - tenant_name, DEFAULT_DOMAIN_ID) + tenant_name, CONF.identity.default_domain_id) tenant_id = tenant_ref['id'] except exception.ProjectNotFound as e: raise exception.Unauthorized(e) diff --git a/keystone/token/providers/common.py b/keystone/token/providers/common.py index 233c613e91..d78184d219 100644 --- a/keystone/token/providers/common.py +++ b/keystone/token/providers/common.py @@ -31,7 +31,6 @@ from keystone.openstack.common import timeutils LOG = log.getLogger(__name__) CONF = config.CONF -DEFAULT_DOMAIN_ID = CONF.identity.default_domain_id class V2TokenDataHelper(object): @@ -475,7 +474,7 @@ class BaseProvider(provider.Provider): msg = _('Non-default domain is not supported') # user in a non-default is prohibited if (token_ref['token_data']['token']['user']['domain']['id'] != - DEFAULT_DOMAIN_ID): + CONF.identity.default_domain_id): raise exception.Unauthorized(msg) # domain scoping is prohibited if token_ref['token_data']['token'].get('domain'): @@ -486,7 +485,7 @@ class BaseProvider(provider.Provider): project = token_ref['token_data']['token']['project'] project_domain_id = project['domain']['id'] # scoped to project in non-default domain is prohibited - if project_domain_id != DEFAULT_DOMAIN_ID: + if project_domain_id != CONF.identity.default_domain_id: raise exception.Unauthorized(msg) # if token is scoped to trust, both trustor and trustee must # be in the default domain. Furthermore, the delegated project @@ -496,15 +495,18 @@ class BaseProvider(provider.Provider): trust_ref = self.trust_api.get_trust(metadata_ref['trust_id']) trustee_user_ref = self.identity_api.get_user( trust_ref['trustee_user_id']) - if trustee_user_ref['domain_id'] != DEFAULT_DOMAIN_ID: + if (trustee_user_ref['domain_id'] != + CONF.identity.default_domain_id): raise exception.Unauthorized(msg) trustor_user_ref = self.identity_api.get_user( trust_ref['trustor_user_id']) - if trustor_user_ref['domain_id'] != DEFAULT_DOMAIN_ID: + if (trustor_user_ref['domain_id'] != + CONF.identity.default_domain_id): raise exception.Unauthorized(msg) project_ref = self.assignment_api.get_project( trust_ref['project_id']) - if project_ref['domain_id'] != DEFAULT_DOMAIN_ID: + if (project_ref['domain_id'] != + CONF.identity.default_domain_id): raise exception.Unauthorized(msg) def validate_v2_token(self, token_id):