openstack
/
keystone
Code Issues Proposed changes

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
This commit is contained in:
Brant Knudson 2013-12-31 18:30:17 -06:00
parent af4c2040ec
commit 01a6c7db3e
13 changed files with 44 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
keystone/assignment/backends/ldap.py
View File

@ -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):

7
keystone/assignment/controllers.py
View File

@ -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'])

4
keystone/assignment/core.py
View File

@ -34,7 +34,9 @@ CONF = config.CONF
LOG = log.getLogger(__name__)
SHOULD_CACHE = cache.should_cache_fn('assignment')
DEFAULT_DOMAIN = {'description':
def calc_default_domain():
return {'description':
(u'Owns users and tenants (i.e. projects)'
' available on Identity API v2.'),
'enabled': True,

7
keystone/common/controller.py
View File

@ -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."""

6
keystone/common/sql/migrate_repo/versions/008_create_default_domain.py
View File

@ -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()

9
keystone/common/sql/migrate_repo/versions/016_normalize_domain_ids.py
View File

@ -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)

4
keystone/identity/controllers.py
View File

@ -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

2
keystone/tests/default_fixtures.py
View File

@ -121,4 +121,4 @@ ROLES = [
}
]
DOMAINS = [assignment.DEFAULT_DOMAIN]
DOMAINS = [assignment.calc_default_domain()]

13
keystone/tests/test_auth.py
View File

@ -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):

15
keystone/tests/test_backend_ldap.py
View File

@ -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(

9
keystone/tests/test_v3_auth.py
View File

@ -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

7
keystone/token/controllers.py
View File

@ -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)

14
keystone/token/providers/common.py
View File

@ -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):