diff --git a/etc/keystone.conf.sample b/etc/keystone.conf.sample index 943f7eeafb..45512c77f7 100644 --- a/etc/keystone.conf.sample +++ b/etc/keystone.conf.sample @@ -349,7 +349,8 @@ # user_enabled_attribute = enabled # user_enabled_mask = 0 # user_enabled_default = True -# user_attribute_ignore = tenant_id,tenants +# user_attribute_ignore = default_project_id,tenants +# user_default_project_id_attribute = # user_allow_create = True # user_allow_update = True # user_allow_delete = True diff --git a/keystone/assignment/backends/kvs.py b/keystone/assignment/backends/kvs.py index 63f8f34b97..b50ba49770 100644 --- a/keystone/assignment/backends/kvs.py +++ b/keystone/assignment/backends/kvs.py @@ -19,7 +19,6 @@ from keystone import clean from keystone.common import dependency from keystone.common import kvs from keystone import exception -from keystone import identity @dependency.requires('identity_api') @@ -52,12 +51,12 @@ class Assignment(kvs.Base, assignment.Driver): except exception.NotFound: raise exception.ProjectNotFound(project_id=tenant_name) - def get_project_users(self, tenant_id): + def list_user_ids_for_project(self, tenant_id): self.get_project(tenant_id) user_keys = filter(lambda x: x.startswith("user-"), self.db.keys()) user_refs = [self.db.get(key) for key in user_keys] user_refs = filter(lambda x: tenant_id in x['tenants'], user_refs) - return [identity.filter_user(user_ref) for user_ref in user_refs] + return [user_ref['id'] for user_ref in user_refs] def _get_user(self, user_id): try: diff --git a/keystone/assignment/backends/ldap.py b/keystone/assignment/backends/ldap.py index 69609a31e4..851f9ec7bd 100644 --- a/keystone/assignment/backends/ldap.py +++ b/keystone/assignment/backends/ldap.py @@ -129,14 +129,12 @@ class Assignment(assignment.Driver): return [self._set_default_domain(x) for x in self.project.get_user_projects(user_dn, associations)] - def get_project_users(self, tenant_id): + def list_user_ids_for_project(self, tenant_id): self.get_project(tenant_id) tenant_dn = self.project._id_to_dn(tenant_id) rolegrants = self.role.get_role_assignments(tenant_dn) - users = [self.user.get_filtered(self.user._dn_to_id(user_id)) - for user_id in - self.project.get_user_dns(tenant_id, rolegrants)] - return self._set_default_domain(users) + return [self.user._dn_to_id(user_dn) for user_dn in + self.project.get_user_dns(tenant_id, rolegrants)] def _subrole_id_to_dn(self, role_id, tenant_id): if tenant_id is None: diff --git a/keystone/assignment/backends/sql.py b/keystone/assignment/backends/sql.py index 70583cc29e..5ea641226c 100644 --- a/keystone/assignment/backends/sql.py +++ b/keystone/assignment/backends/sql.py @@ -54,7 +54,7 @@ class Assignment(sql.Base, assignment.Driver): raise exception.ProjectNotFound(project_id=tenant_name) return project_ref.to_dict() - def get_project_user_ids(self, tenant_id): + def list_user_ids_for_project(self, tenant_id): session = self.get_session() self.get_project(tenant_id) query = session.query(UserProjectGrant) @@ -63,16 +63,6 @@ class Assignment(sql.Base, assignment.Driver): project_refs = query.all() return [project_ref.user_id for project_ref in project_refs] - def get_project_users(self, tenant_id): - self.get_session() - self.get_project(tenant_id) - user_refs = [] - #TODO(ayoung): Move to controller or manager - for user_id in self.get_project_user_ids(tenant_id): - user_ref = self.identity_api.get_user(user_id) - user_refs.append(user_ref) - return user_refs - def _get_metadata(self, user_id=None, tenant_id=None, domain_id=None, group_id=None): session = self.get_session() diff --git a/keystone/assignment/core.py b/keystone/assignment/core.py index 7fe52670bb..4f12d7a05f 100644 --- a/keystone/assignment/core.py +++ b/keystone/assignment/core.py @@ -241,7 +241,8 @@ class Manager(manager.Manager): if not roles: raise exception.NotFound(tenant_id) for role_id in roles: - self.remove_role_from_user_and_project(user_id, tenant_id, role_id) + self.driver.remove_role_from_user_and_project(user_id, tenant_id, + role_id) def list_projects_for_user(self, user_id): # NOTE(henry-nash): In order to get a complete list of user projects, @@ -360,10 +361,10 @@ class Driver(object): """ raise exception.NotImplemented() - def get_project_users(self, tenant_id): - """Lists all users with a relationship to the specified project. + def list_user_ids_for_project(self, tenant_id): + """Lists all user IDs with a role assignment in the specified project. - :returns: a list of user_refs or an empty set. + :returns: a list of user_ids or an empty set. :raises: keystone.exception.ProjectNotFound """ diff --git a/keystone/common/config.py b/keystone/common/config.py index 9325e29c43..45e1b6b780 100644 --- a/keystone/common/config.py +++ b/keystone/common/config.py @@ -183,7 +183,8 @@ FILE_OPTIONS = { cfg.IntOpt('user_enabled_mask', default=0), cfg.StrOpt('user_enabled_default', default='True'), cfg.ListOpt('user_attribute_ignore', - default='tenant_id,tenants'), + default='default_project_id,tenants'), + cfg.StrOpt('user_default_project_id_attribute', default=None), cfg.BoolOpt('user_allow_create', default=True), cfg.BoolOpt('user_allow_update', default=True), cfg.BoolOpt('user_allow_delete', default=True), diff --git a/keystone/common/controller.py b/keystone/common/controller.py index 320477bee5..4b547c9dd0 100644 --- a/keystone/common/controller.py +++ b/keystone/common/controller.py @@ -214,8 +214,9 @@ class V2Controller(wsgi.Application): trust['id']) def _delete_tokens_for_project(self, project_id): - for user_ref in self.identity_api.get_project_users(project_id): - self._delete_tokens_for_user(user_ref['id'], project_id=project_id) + user_ids = self.assignment_api.list_user_ids_for_project(project_id) + for user_id in user_ids: + self._delete_tokens_for_user(user_id, project_id=project_id) def _require_attribute(self, ref, attr): """Ensures the reference contains the specified attribute.""" @@ -233,7 +234,8 @@ class V2Controller(wsgi.Application): ref['domain_id'] = DEFAULT_DOMAIN_ID return ref - def _filter_domain_id(self, ref): + @staticmethod + def filter_domain_id(ref): """Remove domain_id since v2 calls are not domain-aware.""" ref.pop('domain_id', None) return ref @@ -379,7 +381,8 @@ class V3Controller(V2Controller): ref['domain_id'] = self._get_domain_id_for_request(context) return ref - def _filter_domain_id(self, ref): + @staticmethod + def filter_domain_id(ref): """Override v2 filter to let domain_id out for v3 calls.""" return ref diff --git a/keystone/common/ldap/core.py b/keystone/common/ldap/core.py index 8418a3c34d..6291b255fc 100644 --- a/keystone/common/ldap/core.py +++ b/keystone/common/ldap/core.py @@ -542,6 +542,11 @@ class LdapWrapper(object): return self.conn.add_s(dn, ldap_attrs) def search_s(self, dn, scope, query, attrlist=None): + # NOTE(morganfainberg): Remove "None" singletons from this list, which + # allows us to set mapped attributes to "None" as defaults in config. + # Without this filtering, the ldap query would raise a TypeError since + # attrlist is expected to be an iterable of strings. + attrlist = [attr for attr in attrlist if attr is not None] LOG.debug(_( 'LDAP search: dn=%(dn)s, scope=%(scope)s, query=%(query)s, ' 'attrs=%(attrlist)s') % { diff --git a/keystone/common/models.py b/keystone/common/models.py index d5c4983bab..b76bcac1cd 100644 --- a/keystone/common/models.py +++ b/keystone/common/models.py @@ -95,10 +95,12 @@ class User(Model): description email enabled (bool, default True) + default_project_id """ required_keys = ('id', 'name', 'domain_id') - optional_keys = ('password', 'description', 'email', 'enabled') + optional_keys = ('password', 'description', 'email', 'enabled', + 'default_project_id') class Group(Model): diff --git a/keystone/common/sql/migrate_repo/versions/034_add_default_project_id_column_to_user.py b/keystone/common/sql/migrate_repo/versions/034_add_default_project_id_column_to_user.py new file mode 100644 index 0000000000..face5d6052 --- /dev/null +++ b/keystone/common/sql/migrate_repo/versions/034_add_default_project_id_column_to_user.py @@ -0,0 +1,104 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2013 OpenStack Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import json + +import sqlalchemy as sql +from sqlalchemy.orm import sessionmaker + + +def migrate_default_project_from_extra_json(meta, migrate_engine): + user_table = sql.Table('user', meta, autoload=True) + + user_list = user_table.select().execute() + session = sessionmaker(bind=migrate_engine)() + for user in user_list: + try: + data = json.loads(user.extra) + default_project_id = data.pop('default_project_id', None) + v2_tenant_id = data.pop('tenantId', None) + alt_v2_tenant_id = data.pop('tenant_id', None) + except (ValueError, TypeError): + # NOTE(morganfainberg): Somehow we have non-json data here. This + # is a broken user, but it was broken beforehand. Cleaning it up + # is not in the scope of this migration. + continue + + values = {} + if default_project_id is not None: + values['default_project_id'] = default_project_id + elif v2_tenant_id is not None: + values['default_project_id'] = v2_tenant_id + elif alt_v2_tenant_id is not None: + values['default_project_id'] = alt_v2_tenant_id + + if 'default_project_id' in values: + values['extra'] = json.dumps(data) + update = user_table.update().where( + user_table.c.id == user['id']).values(values) + migrate_engine.execute(update) + + session.commit() + session.close() + + +def migrate_default_project_to_extra_json(meta, migrate_engine): + user_table = sql.Table('user', meta, autoload=True) + + user_list = user_table.select().execute() + session = sessionmaker(bind=migrate_engine)() + for user in user_list: + try: + data = json.loads(user.extra) + except (ValueError, TypeError): + # NOTE(morganfainberg): Somehow we have non-json data here. This + # is a broken user, but it was broken beforehand. Cleaning it up + # is not in the scope of this migration. + continue + + # NOTE(morganfainberg): We don't really know what the original 'extra' + # property was here. Populate all of the possible variants we may have + # originally used. + if user.default_project_id is not None: + data['default_project_id'] = user.default_project_id + data['tenantId'] = user.default_project_id + data['tenant_id'] = user.default_project_id + + values = {'extra': json.dumps(data)} + update = user_table.update().where( + user_table.c.id == user.id).values(values) + migrate_engine.execute(update) + session.commit() + session.close() + + +def upgrade(migrate_engine): + meta = sql.MetaData() + meta.bind = migrate_engine + + user_table = sql.Table('user', meta, autoload=True) + default_project_id = sql.Column('default_project_id', sql.String(64)) + user_table.create_column(default_project_id) + migrate_default_project_from_extra_json(meta, migrate_engine) + + +def downgrade(migrate_engine): + meta = sql.MetaData() + meta.bind = migrate_engine + + migrate_default_project_to_extra_json(meta, migrate_engine) + user_table = sql.Table('user', meta, autoload=True) + user_table.drop_column('default_project_id') diff --git a/keystone/contrib/admin_crud/core.py b/keystone/contrib/admin_crud/core.py index 8f6bf9cde7..3a6856d858 100644 --- a/keystone/contrib/admin_crud/core.py +++ b/keystone/contrib/admin_crud/core.py @@ -114,12 +114,12 @@ class CrudExtension(wsgi.ExtensionRouter): mapper.connect( '/users/{user_id}/tenant', controller=user_controller, - action='update_user_project', + action='update_user', conditions=dict(method=['PUT'])) mapper.connect( '/users/{user_id}/OS-KSADM/tenant', controller=user_controller, - action='update_user_project', + action='update_user', conditions=dict(method=['PUT'])) # COMPAT(diablo): the copy with no OS-KSADM is from diablo diff --git a/keystone/contrib/ec2/controllers.py b/keystone/contrib/ec2/controllers.py index 928f3227e2..94b7430ec9 100644 --- a/keystone/contrib/ec2/controllers.py +++ b/keystone/contrib/ec2/controllers.py @@ -118,6 +118,10 @@ class Ec2Controller(controller.V2Controller): catalog_ref = self.catalog_api.get_catalog( user_ref['id'], tenant_ref['id'], metadata_ref) + # NOTE(morganfainberg): Make sure the data is in correct form since it + # might be consumed external to Keystone and this is a v2.0 controller. + # The token provider doesn't actually expect either v2 or v3 user data. + user_ref = self.identity_api.v3_to_v2_user(user_ref) auth_token_data = dict(user=user_ref, tenant=tenant_ref, metadata=metadata_ref, diff --git a/keystone/identity/backends/ldap.py b/keystone/identity/backends/ldap.py index 8177a748c8..f7d44d2b90 100644 --- a/keystone/identity/backends/ldap.py +++ b/keystone/identity/backends/ldap.py @@ -93,9 +93,6 @@ class Identity(identity.Driver): # CRUD def create_user(self, user_id, user): user_ref = self.user.create(user) - tenant_id = user.get('tenant_id') - if tenant_id is not None: - self.assignment_api.add_user_to_project(tenant_id, user_id) return identity.filter_user(user_ref) def update_user(self, user_id, user): @@ -105,17 +102,6 @@ class Identity(identity.Driver): if 'name' in user and old_obj.get('name') != user['name']: raise exception.Conflict('Cannot change user name') - if 'tenant_id' in user and \ - old_obj.get('tenant_id') != user['tenant_id']: - if old_obj['tenant_id']: - self.project.remove_user(old_obj['tenant_id'], - self.user._id_to_dn(user_id), - user_id) - if user['tenant_id']: - self.project.add_user(user['tenant_id'], - self.user._id_to_dn(user_id), - user_id) - user = utils.hash_ldap_user_password(user) if self.user.enabled_mask: self.user.mask_enabled_attribute(user) @@ -208,7 +194,8 @@ class UserApi(common_ldap.EnabledEmuMixIn, common_ldap.BaseLdap): 'email': 'mail', 'name': 'name', 'enabled': 'enabled', - 'domain_id': 'domain_id'} + 'domain_id': 'domain_id', + 'default_project_id': 'default_project_id'} immutable_attrs = ['id'] model = models.User diff --git a/keystone/identity/backends/sql.py b/keystone/identity/backends/sql.py index 7717edbdca..d481c25696 100644 --- a/keystone/identity/backends/sql.py +++ b/keystone/identity/backends/sql.py @@ -24,7 +24,8 @@ from keystone import identity class User(sql.ModelBase, sql.DictBase): __tablename__ = 'user' - attributes = ['id', 'name', 'domain_id', 'password', 'enabled'] + attributes = ['id', 'name', 'domain_id', 'password', 'enabled', + 'default_project_id'] id = sql.Column(sql.String(64), primary_key=True) name = sql.Column(sql.String(255), nullable=False) domain_id = sql.Column(sql.String(64), sql.ForeignKey('domain.id'), @@ -32,10 +33,17 @@ class User(sql.ModelBase, sql.DictBase): password = sql.Column(sql.String(128)) enabled = sql.Column(sql.Boolean) extra = sql.Column(sql.JsonBlob()) + default_project_id = sql.Column(sql.String(64)) # Unique constraint across two columns to create the separation # rather than just only 'name' being unique __table_args__ = (sql.UniqueConstraint('domain_id', 'name'), {}) + def to_dict(self, include_extra_dict=False): + d = super(User, self).to_dict(include_extra_dict=include_extra_dict) + if 'default_project_id' in d and d['default_project_id'] is None: + del d['default_project_id'] + return d + class Group(sql.ModelBase, sql.DictBase): __tablename__ = 'group' diff --git a/keystone/identity/controllers.py b/keystone/identity/controllers.py index 5ab1737c7f..73c5f02019 100644 --- a/keystone/identity/controllers.py +++ b/keystone/identity/controllers.py @@ -41,7 +41,7 @@ class Tenant(controller.V2Controller): self.assert_admin(context) tenant_refs = self.identity_api.list_projects() for tenant_ref in tenant_refs: - tenant_ref = self._filter_domain_id(tenant_ref) + tenant_ref = self.filter_domain_id(tenant_ref) params = { 'limit': context['query_string'].get('limit'), 'marker': context['query_string'].get('marker'), @@ -66,7 +66,7 @@ class Tenant(controller.V2Controller): user_ref = token_ref['user'] tenant_refs = ( self.assignment_api.list_projects_for_user(user_ref['id'])) - tenant_refs = [self._filter_domain_id(ref) for ref in tenant_refs + tenant_refs = [self.filter_domain_id(ref) for ref in tenant_refs if ref['domain_id'] == DEFAULT_DOMAIN_ID] params = { 'limit': context['query_string'].get('limit'), @@ -78,13 +78,13 @@ class Tenant(controller.V2Controller): # TODO(termie): this stuff should probably be moved to middleware self.assert_admin(context) ref = self.identity_api.get_project(tenant_id) - return {'tenant': self._filter_domain_id(ref)} + return {'tenant': self.filter_domain_id(ref)} def get_project_by_name(self, context, tenant_name): self.assert_admin(context) ref = self.identity_api.get_project_by_name( tenant_name, DEFAULT_DOMAIN_ID) - return {'tenant': self._filter_domain_id(ref)} + return {'tenant': self.filter_domain_id(ref)} # CRUD Extension def create_project(self, context, tenant): @@ -99,7 +99,7 @@ class Tenant(controller.V2Controller): tenant = self.assignment_api.create_project( tenant_ref['id'], self._normalize_domain_id(context, tenant_ref)) - return {'tenant': self._filter_domain_id(tenant)} + return {'tenant': self.filter_domain_id(tenant)} def update_project(self, context, tenant_id, tenant): self.assert_admin(context) @@ -125,9 +125,11 @@ class Tenant(controller.V2Controller): def get_project_users(self, context, tenant_id, **kw): self.assert_admin(context) - user_refs = self.identity_api.get_project_users(tenant_id) - for user_ref in user_refs: - self._filter_domain_id(user_ref) + user_refs = [] + user_ids = self.assignment_api.list_user_ids_for_project(tenant_id) + for user_id in user_ids: + user_ref = self.identity_api.get_user(user_id) + user_refs.append(self.identity_api.v3_to_v2_user(user_ref)) return {'users': user_refs} def _format_project_list(self, tenant_refs, **kwargs): @@ -169,7 +171,7 @@ class User(controller.V2Controller): def get_user(self, context, user_id): self.assert_admin(context) ref = self.identity_api.get_user(user_id) - return {'user': self._filter_domain_id(ref)} + return {'user': self.identity_api.v3_to_v2_user(ref)} def get_users(self, context): # NOTE(termie): i can't imagine that this really wants all the data @@ -180,15 +182,12 @@ class User(controller.V2Controller): self.assert_admin(context) user_list = self.identity_api.list_users() - for x in user_list: - self._filter_domain_id(x) - return {'users': user_list} + return {'users': self.identity_api.v3_to_v2_user(user_list)} 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) - return {'user': self._filter_domain_id(ref)} + ref = self.identity_api.get_user_by_name(user_name, DEFAULT_DOMAIN_ID) + return {'user': self.identity_api.v3_to_v2_user(ref)} # CRUD extension def create_user(self, context, user): @@ -202,17 +201,21 @@ class User(controller.V2Controller): msg = 'Enabled field must be a boolean' raise exception.ValidationError(message=msg) - default_tenant_id = user.get('tenantId', None) - if (default_tenant_id is not None - and self.identity_api.get_project(default_tenant_id) is None): - raise exception.ProjectNotFound(project_id=default_tenant_id) + default_project_id = user.pop('tenantId', None) + if default_project_id is not None: + # Check to see if the project is valid before moving on. + self.assignment_api.get_project(default_project_id) + user['default_project_id'] = default_project_id + user_id = uuid.uuid4().hex user_ref = self._normalize_domain_id(context, user.copy()) user_ref['id'] = user_id - new_user_ref = self.identity_api.create_user(user_id, user_ref) - if default_tenant_id: - self.identity_api.add_user_to_project(default_tenant_id, user_id) - return {'user': self._filter_domain_id(new_user_ref)} + new_user_ref = self.identity_api.v3_to_v2_user( + self.identity_api.create_user(user_id, user_ref)) + + if default_project_id is not None: + self.identity_api.add_user_to_project(default_project_id, user_id) + return {'user': new_user_ref} def update_user(self, context, user_id, user): # NOTE(termie): this is really more of a patch than a put @@ -222,12 +225,65 @@ class User(controller.V2Controller): msg = 'Enabled field should be a boolean' raise exception.ValidationError(message=msg) - user_ref = self.identity_api.update_user(user_id, user) + default_project_id = user.pop('tenantId', None) + if default_project_id is not None: + user['default_project_id'] = default_project_id + + old_user_ref = self.identity_api.v3_to_v2_user( + self.identity_api.get_user(user_id)) + + if ('tenantId' in old_user_ref and + old_user_ref['tenantId'] != default_project_id and + default_project_id is not None): + # Make sure the new project actually exists before we perform the + # user update. + self.assignment_api.get_project(default_project_id) + + user_ref = self.identity_api.v3_to_v2_user( + self.identity_api.update_user(user_id, user)) if user.get('password') or not user.get('enabled', True): # If the password was changed or the user was disabled we clear tokens self._delete_tokens_for_user(user_id) - return {'user': self._filter_domain_id(user_ref)} + + # If 'tenantId' is in either ref, we might need to add or remove the + # user from a project. + if 'tenantId' in user_ref or 'tenantId' in old_user_ref: + if user_ref['tenantId'] != old_user_ref.get('tenantId'): + if old_user_ref.get('tenantId'): + try: + self.assignment_api.remove_user_from_project( + old_user_ref['tenantId'], user_id) + except exception.NotFound: + # NOTE(morganfainberg): This is not a critical error it + # just means that the user cannot be removed from the + # old tenant. This could occur if roles aren't found + # or if the project is invalid or if there are no roles + # for the user on that project. + msg = _('Unable to remove user %(user)s from ' + '%(tenant)s.') + LOG.warning(msg, {'user': user_id, + 'tenant': old_user_ref['tenantId']}) + + if user_ref['tenantId']: + try: + self.assignment_api.add_user_to_project( + user_ref['tenantId'], user_id) + except exception.Conflict: + # We are already a member of that tenant + pass + except exception.NotFound: + # NOTE(morganfainberg): Log this and move on. This is + # not the end of the world if we can't add the user to + # the appropriate tenant. Most of the time this means + # that the project is invalid or roles are some how + # incorrect. This shouldn't prevent the return of the + # new ref. + msg = _('Unable to add user %(user)s to %(tenant)s.') + LOG.warning(msg, {'user': user_id, + 'tenant': user_ref['tenantId']}) + + return {'user': user_ref} def delete_user(self, context, user_id): self.assert_admin(context) @@ -240,20 +296,6 @@ class User(controller.V2Controller): def set_user_password(self, context, user_id, user): return self.update_user(context, user_id, user) - def update_user_project(self, context, user_id, user): - """Update the default tenant.""" - self.assert_admin(context) - - try: - # ensure that we're a member of that tenant - self.identity_api.add_user_to_project( - user.get('tenantId'), user_id) - except exception.Conflict: - # we're already a member of that tenant - pass - - return self.update_user(context, user_id, user) - class Role(controller.V2Controller): # COMPAT(essex-3) diff --git a/keystone/identity/core.py b/keystone/identity/core.py index f63dc399e6..a0109457cd 100644 --- a/keystone/identity/core.py +++ b/keystone/identity/core.py @@ -22,6 +22,7 @@ import os from oslo.config import cfg from keystone import clean +from keystone.common import controller from keystone.common import dependency from keystone.common import manager from keystone import config @@ -203,6 +204,45 @@ class Manager(manager.Manager): super(Manager, self).__init__(CONF.identity.driver) self.domain_configs = DomainConfigs() + @staticmethod + def v3_to_v2_user(ref): + """Convert a user_ref from v3 to v2 compatible. + + * v2.0 users are not domain aware, and should have domain_id removed + * v2.0 users expect the use of tenantId instead of default_project_id + + This method should only be applied to user_refs being returned from the + v2.0 controller(s). + + If ref is a list type, we will iterate through each element and do the + conversion. + """ + + def _format_default_project_id(ref): + """Convert default_project_id to tenantId for v2 calls.""" + default_project_id = ref.pop('default_project_id', None) + if default_project_id is not None: + ref['tenantId'] = default_project_id + elif 'tenantId' in ref: + # NOTE(morganfainberg): To avoid v2.0 confusion if somehow a + # tenantId property sneaks its way into the extra blob on the + # user, we remove it here. If default_project_id is set, we + # would override it in either case. + del ref['tenantId'] + + def _normalize_and_filter_user_properties(ref): + """Run through the various filter/normalization methods.""" + _format_default_project_id(ref) + controller.V2Controller.filter_domain_id(ref) + return ref + + if isinstance(ref, dict): + return _normalize_and_filter_user_properties(ref) + elif isinstance(ref, list): + return [_normalize_and_filter_user_properties(x) for x in ref] + else: + raise ValueError(_('Expected dict or list: %s') % type(ref)) + # Domain ID normalization methods def _set_domain_id(self, ref, domain_id): diff --git a/keystone/tests/default_fixtures.py b/keystone/tests/default_fixtures.py index bff9c374af..b28463fd02 100644 --- a/keystone/tests/default_fixtures.py +++ b/keystone/tests/default_fixtures.py @@ -72,7 +72,7 @@ USERS = [ 'password': 'two2', 'email': 'two@example.com', 'enabled': True, - 'tenant_id': 'baz', + 'default_project_id': 'baz', 'tenants': ['baz'], 'email': 'two@three.com', }, { @@ -82,7 +82,7 @@ USERS = [ 'password': 'bad', 'email': 'bad@guy.com', 'enabled': False, - 'tenant_id': 'baz', + 'default_project_id': 'baz', 'tenants': ['baz'], 'email': 'badguy@goodguy.com', }, { diff --git a/keystone/tests/test_backend.py b/keystone/tests/test_backend.py index d5d2cd655b..04a1944886 100644 --- a/keystone/tests/test_backend.py +++ b/keystone/tests/test_backend.py @@ -40,23 +40,26 @@ class IdentityTests(object): return domain def test_project_add_and_remove_user_role(self): - user_refs = self.identity_api.get_project_users(self.tenant_bar['id']) - self.assertNotIn(self.user_two['id'], [x['id'] for x in user_refs]) + user_ids = self.assignment_api.list_user_ids_for_project( + self.tenant_bar['id']) + self.assertNotIn(self.user_two['id'], user_ids) self.identity_api.add_role_to_user_and_project( tenant_id=self.tenant_bar['id'], user_id=self.user_two['id'], role_id=self.role_other['id']) - user_refs = self.identity_api.get_project_users(self.tenant_bar['id']) - self.assertIn(self.user_two['id'], [x['id'] for x in user_refs]) + user_ids = self.assignment_api.list_user_ids_for_project( + self.tenant_bar['id']) + self.assertIn(self.user_two['id'], user_ids) self.identity_api.remove_role_from_user_and_project( tenant_id=self.tenant_bar['id'], user_id=self.user_two['id'], role_id=self.role_other['id']) - user_refs = self.identity_api.get_project_users(self.tenant_bar['id']) - self.assertNotIn(self.user_two['id'], [x['id'] for x in user_refs]) + user_ids = self.assignment_api.list_user_ids_for_project( + self.tenant_bar['id']) + self.assertNotIn(self.user_two['id'], user_ids) def test_authenticate_bad_user(self): self.assertRaises(AssertionError, @@ -74,7 +77,7 @@ class IdentityTests(object): user_ref = self.identity_api.authenticate( user_id=self.user_sna['id'], password=self.user_sna['password']) - # NOTE(termie): the password field is left in user_foo to make + # NOTE(termie): the password field is left in user_sna to make # it easier to authenticate in tests, but should # not be returned by the api self.user_sna.pop('password') @@ -94,7 +97,8 @@ class IdentityTests(object): user_ref = self.identity_api.authenticate( user_id=user['id'], password=user['password']) - # NOTE(termie): the password field is left in user_foo to make + self.assertNotIn('password', user_ref) + # NOTE(termie): the password field is left in user_sna to make # it easier to authenticate in tests, but should # not be returned by the api user.pop('password') @@ -140,19 +144,16 @@ class IdentityTests(object): uuid.uuid4().hex, DEFAULT_DOMAIN_ID) - def test_get_project_users(self): - tenant_ref = self.identity_api.get_project_users(self.tenant_baz['id']) - user_ids = [] - for user in tenant_ref: - self.assertNotIn('password', user) - user_ids.append(user.get('id')) + def test_list_user_ids_for_project(self): + user_ids = self.assignment_api.list_user_ids_for_project( + self.tenant_baz['id']) self.assertEquals(len(user_ids), 2) self.assertIn(self.user_two['id'], user_ids) self.assertIn(self.user_badguy['id'], user_ids) - def test_get_project_users_404(self): + def test_get_project_user_ids_404(self): self.assertRaises(exception.ProjectNotFound, - self.identity_api.get_project_users, + self.assignment_api.list_user_ids_for_project, uuid.uuid4().hex) def test_get_user(self): @@ -171,7 +172,6 @@ class IdentityTests(object): def test_get_user_by_name(self): user_ref = self.identity_api.get_user_by_name( self.user_foo['name'], DEFAULT_DOMAIN_ID) - # NOTE(termie): the password field is left in user_foo to make # it easier to authenticate in tests, but should # not be returned by the api @@ -1744,6 +1744,8 @@ class IdentityTests(object): self.assertEqual(len(default_fixtures.USERS), len(users)) user_ids = set(user['id'] for user in users) expected_user_ids = set(user['id'] for user in default_fixtures.USERS) + for user_ref in users: + self.assertNotIn('password', user_ref) self.assertEqual(expected_user_ids, user_ids) def test_list_groups(self): @@ -2020,6 +2022,7 @@ class IdentityTests(object): for x in user_refs: if (x['id'] == new_user['id']): found = True + self.assertNotIn('password', x) self.assertTrue(found) def test_list_groups_for_user(self): diff --git a/keystone/tests/test_backend_sql.py b/keystone/tests/test_backend_sql.py index a8e9f2411a..3e61c49e13 100644 --- a/keystone/tests/test_backend_sql.py +++ b/keystone/tests/test_backend_sql.py @@ -21,6 +21,7 @@ import sqlalchemy from keystone.common import sql from keystone import config from keystone import exception +from keystone.identity.backends import sql as identity_sql from keystone import tests from keystone.tests import default_fixtures from keystone.tests import test_backend @@ -327,6 +328,24 @@ class SqlIdentity(SqlTests, test_backend.IdentityTests): self.assertEqual(arbitrary_value, ref[arbitrary_key]) self.assertEqual(arbitrary_value, ref['extra'][arbitrary_key]) + def test_sql_user_to_dict_null_default_project_id(self): + user_id = uuid.uuid4().hex + user = { + 'id': user_id, + 'name': uuid.uuid4().hex, + 'domain_id': DEFAULT_DOMAIN_ID, + 'password': uuid.uuid4().hex} + + self.identity_api.create_user(user_id, user) + session = self.get_session() + query = session.query(identity_sql.User) + query = query.filter_by(id=user_id) + raw_user_ref = query.one() + self.assertIsNone(raw_user_ref.default_project_id) + user_ref = raw_user_ref.to_dict() + self.assertNotIn('default_project_id', user_ref) + session.close() + class SqlTrust(SqlTests, test_backend.TrustTests): pass diff --git a/keystone/tests/test_content_types.py b/keystone/tests/test_content_types.py index b2811c5e57..989597ceaa 100644 --- a/keystone/tests/test_content_types.py +++ b/keystone/tests/test_content_types.py @@ -631,6 +631,15 @@ class JsonTestCase(RestfulTestCase, CoreApiTests): def assertValidExtensionResponse(self, r, expected): self.assertValidExtension(r.result.get('extension'), expected) + def assertValidUser(self, user): + super(JsonTestCase, self).assertValidUser(user) + self.assertNotIn('default_project_id', user) + if 'tenantId' in user: + # NOTE(morganfainberg): tenantId should never be "None", it gets + # filtered out of the object if it is there. This is suspenders + # and a belt check to avoid unintended regressions. + self.assertIsNotNone(user.get('tenantId')) + def assertValidAuthenticationResponse(self, r, require_service_catalog=False): self.assertIsNotNone(r.result.get('access')) diff --git a/keystone/tests/test_sql_upgrade.py b/keystone/tests/test_sql_upgrade.py index ffa0efbff8..3b763715cd 100644 --- a/keystone/tests/test_sql_upgrade.py +++ b/keystone/tests/test_sql_upgrade.py @@ -1535,6 +1535,127 @@ class SqlUpgradeTests(SqlMigrateBase): self.insert_dict(session, 'credential', v3_cred_invalid_blob) self.assertRaises(exception.ValidationError, self.upgrade, 33) + def test_migrate_add_default_project_id_column_upgrade(self): + user1 = { + 'id': 'foo1', + 'name': 'FOO1', + 'password': 'foo2', + 'enabled': True, + 'email': 'foo@bar.com', + 'extra': json.dumps({'tenantId': 'bar'}), + 'domain_id': DEFAULT_DOMAIN_ID + } + user2 = { + 'id': 'foo2', + 'name': 'FOO2', + 'password': 'foo2', + 'enabled': True, + 'email': 'foo@bar.com', + 'extra': json.dumps({'tenant_id': 'bar'}), + 'domain_id': DEFAULT_DOMAIN_ID + } + user3 = { + 'id': 'foo3', + 'name': 'FOO3', + 'password': 'foo2', + 'enabled': True, + 'email': 'foo@bar.com', + 'extra': json.dumps({'default_project_id': 'bar'}), + 'domain_id': DEFAULT_DOMAIN_ID + } + user4 = { + 'id': 'foo4', + 'name': 'FOO4', + 'password': 'foo2', + 'enabled': True, + 'email': 'foo@bar.com', + 'extra': json.dumps({'tenantId': 'baz', + 'default_project_id': 'bar'}), + 'domain_id': DEFAULT_DOMAIN_ID + } + + session = self.Session() + self.upgrade(33) + self.insert_dict(session, 'user', user1) + self.insert_dict(session, 'user', user2) + self.insert_dict(session, 'user', user3) + self.insert_dict(session, 'user', user4) + self.assertTableColumns('user', + ['id', 'name', 'extra', 'password', + 'enabled', 'domain_id']) + session.commit() + session.close() + self.upgrade(34) + session = self.Session() + self.assertTableColumns('user', + ['id', 'name', 'extra', 'password', + 'enabled', 'domain_id', 'default_project_id']) + + user_table = sqlalchemy.Table('user', self.metadata, autoload=True) + updated_user1 = session.query(user_table).filter_by(id='foo1').one() + old_json_data = json.loads(user1['extra']) + new_json_data = json.loads(updated_user1.extra) + self.assertNotIn('tenantId', new_json_data) + self.assertEqual(old_json_data['tenantId'], + updated_user1.default_project_id) + updated_user2 = session.query(user_table).filter_by(id='foo2').one() + old_json_data = json.loads(user2['extra']) + new_json_data = json.loads(updated_user2.extra) + self.assertNotIn('tenant_id', new_json_data) + self.assertEqual(old_json_data['tenant_id'], + updated_user2.default_project_id) + updated_user3 = session.query(user_table).filter_by(id='foo3').one() + old_json_data = json.loads(user3['extra']) + new_json_data = json.loads(updated_user3.extra) + self.assertNotIn('default_project_id', new_json_data) + self.assertEqual(old_json_data['default_project_id'], + updated_user3.default_project_id) + updated_user4 = session.query(user_table).filter_by(id='foo4').one() + old_json_data = json.loads(user4['extra']) + new_json_data = json.loads(updated_user4.extra) + self.assertNotIn('default_project_id', new_json_data) + self.assertNotIn('tenantId', new_json_data) + self.assertEqual(old_json_data['default_project_id'], + updated_user4.default_project_id) + + def test_migrate_add_default_project_id_column_downgrade(self): + user1 = { + 'id': 'foo1', + 'name': 'FOO1', + 'password': 'foo2', + 'enabled': True, + 'email': 'foo@bar.com', + 'extra': json.dumps({}), + 'default_project_id': 'bar', + 'domain_id': DEFAULT_DOMAIN_ID + } + + self.upgrade(34) + session = self.Session() + self.insert_dict(session, 'user', user1) + self.assertTableColumns('user', + ['id', 'name', 'extra', 'password', + 'enabled', 'domain_id', 'default_project_id']) + session.commit() + session.close() + self.downgrade(33) + session = self.Session() + self.assertTableColumns('user', + ['id', 'name', 'extra', 'password', + 'enabled', 'domain_id']) + + user_table = sqlalchemy.Table('user', self.metadata, autoload=True) + updated_user1 = session.query(user_table).filter_by(id='foo1').one() + new_json_data = json.loads(updated_user1.extra) + self.assertIn('tenantId', new_json_data) + self.assertIn('default_project_id', new_json_data) + self.assertEqual(user1['default_project_id'], + new_json_data['tenantId']) + self.assertEqual(user1['default_project_id'], + new_json_data['default_project_id']) + self.assertEqual(user1['default_project_id'], + new_json_data['tenant_id']) + def populate_user_table(self, with_pass_enab=False, with_pass_enab_domain=False): # Populate the appropriate fields in the user diff --git a/keystone/tests/test_v3.py b/keystone/tests/test_v3.py index 87f2bcd16a..d4602203e5 100644 --- a/keystone/tests/test_v3.py +++ b/keystone/tests/test_v3.py @@ -109,9 +109,7 @@ class RestfulTestCase(test_content_types.RestfulTestCase): self.assignment_api.create_project(self.project_id, self.project) self.user_id = uuid.uuid4().hex - self.user = self.new_user_ref( - domain_id=self.domain_id, - project_id=self.project_id) + self.user = self.new_user_ref(domain_id=self.domain_id) self.user['id'] = self.user_id self.identity_api.create_user(self.user_id, self.user) @@ -124,8 +122,7 @@ class RestfulTestCase(test_content_types.RestfulTestCase): self.default_domain_user_id = uuid.uuid4().hex self.default_domain_user = self.new_user_ref( - domain_id=DEFAULT_DOMAIN_ID, - project_id=self.default_domain_project_id) + domain_id=DEFAULT_DOMAIN_ID) self.default_domain_user['id'] = self.default_domain_user_id self.identity_api.create_user(self.default_domain_user_id, self.default_domain_user) @@ -212,7 +209,7 @@ class RestfulTestCase(test_content_types.RestfulTestCase): ref['email'] = uuid.uuid4().hex ref['password'] = uuid.uuid4().hex if project_id: - ref['project_id'] = project_id + ref['default_project_id'] = project_id return ref def new_group_ref(self, domain_id): @@ -717,9 +714,14 @@ class RestfulTestCase(test_content_types.RestfulTestCase): self.assertIsNotNone(entity.get('domain_id')) self.assertIsNotNone(entity.get('email')) self.assertIsNone(entity.get('password')) + self.assertNotIn('tenantId', entity) if ref: self.assertEqual(ref['domain_id'], entity['domain_id']) self.assertEqual(ref['email'], entity['email']) + if 'default_project_id' in ref: + self.assertIsNotNone(ref['default_project_id']) + self.assertEqual(ref['default_project_id'], + entity['default_project_id']) return entity # group validation diff --git a/keystone/tests/test_v3_identity.py b/keystone/tests/test_v3_identity.py index 4d4a23ca15..ad90ea1f2e 100644 --- a/keystone/tests/test_v3_identity.py +++ b/keystone/tests/test_v3_identity.py @@ -16,7 +16,9 @@ import uuid +from keystone.common import controller from keystone import exception +from keystone import tests from keystone.tests import test_v3 @@ -391,6 +393,13 @@ class IdentityTestCase(test_v3.RestfulTestCase): r = self.get('/users') self.assertValidUserListResponse(r, ref=self.user) + def test_list_users_no_default_project(self): + """Call ``GET /users`` making sure no default_project_id.""" + user = self.new_user_ref(self.domain_id) + self.identity_api.create_user(self.user_id, user) + r = self.get('/users') + self.assertValidUserListResponse(r, ref=user) + def test_list_users_xml(self): """Call ``GET /users`` (xml data).""" r = self.get('/users', content_type='xml') @@ -402,6 +411,14 @@ class IdentityTestCase(test_v3.RestfulTestCase): 'user_id': self.user['id']}) self.assertValidUserResponse(r, self.user) + def test_get_user_with_default_project(self): + """Call ``GET /users/{user_id}`` making sure of default_project_id.""" + user = self.new_user_ref(domain_id=self.domain_id, + project_id=self.project_id) + self.identity_api.create_user(self.user_id, user) + r = self.get('/users/%(user_id)s' % {'user_id': user['id']}) + self.assertValidUserResponse(r, user) + def test_add_user_to_group(self): """Call ``PUT /groups/{group_id}/users/{user_id}``.""" self.put('/groups/%(group_id)s/users/%(user_id)s' % { @@ -1552,3 +1569,104 @@ class IdentityInheritanceDisabledTestCase(test_v3.RestfulTestCase): self.head(member_url, expected_status=404) self.get(collection_url, expected_status=404) self.delete(member_url, expected_status=404) + + +class TestV3toV2Methods(tests.TestCase): + """Test V3 to V2 conversion methods.""" + + def setUp(self): + super(TestV3toV2Methods, self).setUp() + self.load_backends() + self.user_id = uuid.uuid4().hex + self.default_project_id = uuid.uuid4().hex + self.tenant_id = uuid.uuid4().hex + self.domain_id = uuid.uuid4().hex + # User with only default_project_id in ref + self.user1 = {'id': self.user_id, + 'name': self.user_id, + 'default_project_id': self.default_project_id, + 'domain_id': self.domain_id} + # User without default_project_id or tenantId in ref + self.user2 = {'id': self.user_id, + 'name': self.user_id, + 'domain_id': self.domain_id} + # User with both tenantId and default_project_id in ref + self.user3 = {'id': self.user_id, + 'name': self.user_id, + 'default_project_id': self.default_project_id, + 'tenantId': self.tenant_id, + 'domain_id': self.domain_id} + # User with only tenantId in ref + self.user4 = {'id': self.user_id, + 'name': self.user_id, + 'tenantId': self.tenant_id, + 'domain_id': self.domain_id} + + # Expected result if the user is meant to have a tenantId element + self.expected_user = {'id': self.user_id, + 'name': self.user_id, + 'tenantId': self.default_project_id} + + # Expected result if the user is not meant ot have a tenantId element + self.expected_user_no_tenant_id = {'id': self.user_id, + 'name': self.user_id} + + def test_v3_to_v2_user_method(self): + + updated_user1 = self.identity_api.v3_to_v2_user(self.user1) + self.assertIs(self.user1, updated_user1) + self.assertDictEqual(self.user1, self.expected_user) + updated_user2 = self.identity_api.v3_to_v2_user(self.user2) + self.assertIs(self.user2, updated_user2) + self.assertDictEqual(self.user2, self.expected_user_no_tenant_id) + updated_user3 = self.identity_api.v3_to_v2_user(self.user3) + self.assertIs(self.user3, updated_user3) + self.assertDictEqual(self.user3, self.expected_user) + updated_user4 = self.identity_api.v3_to_v2_user(self.user4) + self.assertIs(self.user4, updated_user4) + self.assertDictEqual(self.user4, self.expected_user_no_tenant_id) + + def test_v3_to_v2_user_method_list(self): + user_list = [self.user1, self.user2, self.user3, self.user4] + updated_list = self.identity_api.v3_to_v2_user(user_list) + + self.assertEquals(len(updated_list), len(user_list)) + + for i, ref in enumerate(updated_list): + # Order should not change. + self.assertIs(ref, user_list[i]) + + self.assertDictEqual(self.user1, self.expected_user) + self.assertDictEqual(self.user2, self.expected_user_no_tenant_id) + self.assertDictEqual(self.user3, self.expected_user) + self.assertDictEqual(self.user4, self.expected_user_no_tenant_id) + + def test_v2controller_filter_domain_id(self): + # V2.0 is not domain aware, ensure domain_id is popped off the ref. + other_data = uuid.uuid4().hex + domain_id = uuid.uuid4().hex + ref = {'domain_id': domain_id, + 'other_data': other_data} + + ref_no_domain = {'other_data': other_data} + expected_ref = ref_no_domain.copy() + + updated_ref = controller.V2Controller.filter_domain_id(ref) + self.assertIs(ref, updated_ref) + self.assertDictEqual(ref, expected_ref) + # Make sure we don't error/muck up data if domain_id isn't present + updated_ref = controller.V2Controller.filter_domain_id(ref_no_domain) + self.assertIs(ref_no_domain, updated_ref) + self.assertDictEqual(ref_no_domain, expected_ref) + + def test_v3controller_filter_domain_id(self): + # No data should be filtered out in this case. + other_data = uuid.uuid4().hex + domain_id = uuid.uuid4().hex + ref = {'domain_id': domain_id, + 'other_data': other_data} + + expected_ref = ref.copy() + updated_ref = controller.V3Controller.filter_domain_id(ref) + self.assertIs(ref, updated_ref) + self.assertDictEqual(ref, expected_ref) diff --git a/keystone/token/controllers.py b/keystone/token/controllers.py index bc0d073d88..8d2ce878d1 100644 --- a/keystone/token/controllers.py +++ b/keystone/token/controllers.py @@ -95,9 +95,14 @@ class Auth(controller.V2Controller): user_ref, tenant_ref, metadata_ref, expiry, bind = auth_info core.validate_auth_info(self, user_ref, tenant_ref) - user_ref = self._filter_domain_id(user_ref) + # NOTE(morganfainberg): Make sure the data is in correct form since it + # might be consumed external to Keystone and this is a v2.0 controller. + # The user_ref is encoded into the auth_token_data which is returned as + # part of the token data. The token provider doesn't care about the + # format. + user_ref = self.identity_api.v3_to_v2_user(user_ref) if tenant_ref: - tenant_ref = self._filter_domain_id(tenant_ref) + tenant_ref = self.filter_domain_id(tenant_ref) auth_token_data = self._get_auth_token_data(user_ref, tenant_ref, metadata_ref,