Cleanup of tenantId, tenant_id, and default_project_id

This patchset normalizes the use of tenantId, tenant_id, and
default_project_id across the Identity backend.  This includes
making default_project_id no longer part of the "extra" json blob
on the user object and migrating all "tenantId" "tenant_id" and
"default_project_id" into the new column (SQL).

In the LDAP driver, None is set as the mapping for
default_project_id.  This means that use of default_project_id with
LDAP Identity will require an explicit mapping to be defined by the
cloud operator.

"default_project_id" remains (by default) configured to be in the
"ignore" attributes for the LDAP driver, so 'tenantId' and
'default_project_id' will not be saved on the user_object during
update or create unless Keystone is explicitly configured to do so.

closes-bug: 1219739
closes-bug: 1226475
related-bug: 1201251
Change-Id: I07f9dfe111646884ac5efd42fc8c2974188b3b94

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

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:

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))
+        return [self.user._dn_to_id(user_dn) for user_dn in
-                 for user_id in
                 self.project.get_user_dns(tenant_id, rolegrants)]
-        return self._set_default_domain(users)
 
     def _subrole_id_to_dn(self, role_id, tenant_id):
         if tenant_id is None:

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

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):
+    def list_user_ids_for_project(self, tenant_id):
-        """Lists all users with a relationship to the specified project.
+        """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
 
         """

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

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):
+        user_ids = self.assignment_api.list_user_ids_for_project(project_id)
-            self._delete_tokens_for_user(user_ref['id'], project_id=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
 

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') % {

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

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

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

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,

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

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'

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)
+        user_refs = []
-        for user_ref in user_refs:
+        user_ids = self.assignment_api.list_user_ids_for_project(tenant_id)
-            self._filter_domain_id(user_ref)
+        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:
+        return {'users': self.identity_api.v3_to_v2_user(user_list)}
-            self._filter_domain_id(x)
-        return {'users': user_list}
 
     def get_user_by_name(self, context, user_name):
         self.assert_admin(context)
-        ref = self.identity_api.get_user_by_name(
+        ref = self.identity_api.get_user_by_name(user_name, DEFAULT_DOMAIN_ID)
-            user_name, DEFAULT_DOMAIN_ID)
+        return {'user': self.identity_api.v3_to_v2_user(ref)}
-        return {'user': self._filter_domain_id(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)
+        default_project_id = user.pop('tenantId', None)
-        if (default_tenant_id is not None
+        if default_project_id is not None:
-                and self.identity_api.get_project(default_tenant_id) is None):
+            # Check to see if the project is valid before moving on.
-            raise exception.ProjectNotFound(project_id=default_tenant_id)
+            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)
+        new_user_ref = self.identity_api.v3_to_v2_user(
-        if default_tenant_id:
+            self.identity_api.create_user(user_id, user_ref))
-            self.identity_api.add_user_to_project(default_tenant_id, user_id)
+
-        return {'user': self._filter_domain_id(new_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)

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

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',
     }, {

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'])
+        user_ids = self.assignment_api.list_user_ids_for_project(
-        self.assertNotIn(self.user_two['id'], [x['id'] for x in user_refs])
+            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'])
+        user_ids = self.assignment_api.list_user_ids_for_project(
-        self.assertIn(self.user_two['id'], [x['id'] for x in user_refs])
+            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'])
+        user_ids = self.assignment_api.list_user_ids_for_project(
-        self.assertNotIn(self.user_two['id'], [x['id'] for x in user_refs])
+            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):
+    def test_list_user_ids_for_project(self):
-        tenant_ref = self.identity_api.get_project_users(self.tenant_baz['id'])
+        user_ids = self.assignment_api.list_user_ids_for_project(
-        user_ids = []
+            self.tenant_baz['id'])
-        for user in tenant_ref:
-            self.assertNotIn('password', user)
-            user_ids.append(user.get('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):

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

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

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

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(
+            self.user = self.new_user_ref(domain_id=self.domain_id)
-                domain_id=self.domain_id,
-                project_id=self.project_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,
+                domain_id=DEFAULT_DOMAIN_ID)
-                project_id=self.default_domain_project_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

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)

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,