project membership to role conversion

Changes the relationship between users and projects.
There is no more direct membership in projects.  Instead,
all membership is now done via roles.
A default role has been created called _member_ with a uuid (both
configurable) that will be added in place of the group membership
for databse upgrades.

DocImpact:  https://bugs.launchpad.net/openstack-manuals/+bug/1087483

Change-Id: I2482f9ef7b838e5dade5096d6d00e81db71604d1

16 files changed, 427 insertions, 185 deletions

diff --git a/etc/keystone.conf.sample b/etc/keystone.conf.sample
index 8eddfe1a..3143f3fe 100644
--- a/etc/keystone.conf.sample
+++ b/etc/keystone.conf.sample
@@ -27,6 +27,14 @@
 # FIXME(dolph): This should really be defined as [policy] default_rule
 # policy_default_rule = admin_required
 
+# Role for migrating membership relationships
+# During a SQL upgrade, the following values will be used to create a new role
+# that will replace records in the user_tenant_membership table with explicit
+# role grants.  After migration, the member_role_id will be used in the API
+# add_user_to_project, and member_role_name will be ignored.
+# member_role_id = 9fe2ff9ee4384b1894a90878d3e92bab
+# member_role_name = _member_
+
 # === Logging Options ===
 # Print debugging output
 # (includes plaintext request logging, potentially including passwords)
diff --git a/keystone/common/sql/migrate_repo/versions/015_tenant_to_project.py b/keystone/common/sql/migrate_repo/versions/015_tenant_to_project.py
index 4ac0d612..5b3c2ac0 100644
--- a/keystone/common/sql/migrate_repo/versions/015_tenant_to_project.py
+++ b/keystone/common/sql/migrate_repo/versions/015_tenant_to_project.py
@@ -1,19 +1,133 @@
 import sqlalchemy as sql
+from sqlalchemy.orm import sessionmaker
 
 
-def upgrade(migrate_engine):
-    meta = sql.MetaData()
-    meta.bind = migrate_engine
+def upgrade_with_rename(meta, migrate_engine):
     legacy_table = sql.Table('tenant', meta, autoload=True)
     legacy_table.rename('project')
     legacy_table = sql.Table('user_tenant_membership', meta, autoload=True)
     legacy_table.rename('user_project_membership')
 
 
-def downgrade(migrate_engine):
-    meta = sql.MetaData()
-    meta.bind = migrate_engine
+def downgrade_with_rename(meta, migrate_engine):
     upgrade_table = sql.Table('project', meta, autoload=True)
     upgrade_table.rename('tenant')
     upgrade_table = sql.Table('user_project_membership', meta, autoload=True)
     upgrade_table.rename('user_tenant_membership')
+
+
+def upgrade_with_copy(meta, migrate_engine):
+    legacy_table = sql.Table('user', meta, autoload=True)
+    project_table = sql.Table(
+        'project',
+        meta,
+        sql.Column('id', sql.String(64), primary_key=True),
+        sql.Column('name', sql.String(64), unique=True, nullable=False),
+        sql.Column('extra', sql.Text()),
+        sql.Column('description', sql.Text(), nullable=True),
+        sql.Column('enabled', sql.types.Boolean, default=True))
+    project_table.create(migrate_engine, checkfirst=True)
+
+    user_project_membership_table = sql.Table(
+        'user_project_membership',
+        meta,
+        sql.Column(
+            'user_id',
+            sql.String(64),
+            sql.ForeignKey('user.id'),
+            primary_key=True),
+        sql.Column(
+            'tenant_id',
+            sql.String(64),
+            sql.ForeignKey('project.id'),
+            primary_key=True))
+    user_project_membership_table.create(migrate_engine, checkfirst=True)
+
+    session = sessionmaker(bind=migrate_engine)()
+
+    tenant_table = sql.Table('tenant', meta, autoload=True)
+    insert = project_table.insert()
+    for tenant in session.query(tenant_table):
+        insert.execute({'id': tenant.id,
+                        'name': tenant.name,
+                        'extra': tenant.extra,
+                        'description': tenant.description,
+                        'enabled': tenant.enabled})
+
+    user_tenant_membership_table = sql.Table('user_tenant_membership',
+                                             meta,
+                                             autoload=True)
+    insert = user_project_membership_table.insert()
+    for membership in session.query(user_tenant_membership_table):
+        insert.execute(membership)
+
+    session.commit()
+    session.close()
+
+    user_tenant_membership_table.drop()
+    tenant_table.drop()
+
+
+def downgrade_with_copy(meta, migrate_engine):
+    legacy_table = sql.Table('user', meta, autoload=True)
+    tenant_table = sql.Table(
+        'tenant',
+        meta,
+        sql.Column('id', sql.String(64), primary_key=True),
+        sql.Column('name', sql.String(64), unique=True, nullable=False),
+        sql.Column('extra', sql.Text()),
+        sql.Column('description', sql.Text(), nullable=True),
+        sql.Column('enabled', sql.types.Boolean))
+    tenant_table.create(migrate_engine, checkfirst=True)
+
+    user_tenant_membership_table = sql.Table(
+        'user_tenant_membership',
+        meta,
+        sql.Column(
+            'user_id',
+            sql.String(64),
+            sql.ForeignKey('user.id'),
+            primary_key=True),
+        sql.Column(
+            'tenant_id',
+            sql.String(64),
+            sql.ForeignKey('tenant.id'),
+            primary_key=True))
+    user_tenant_membership_table.create(migrate_engine, checkfirst=True)
+
+    session = sessionmaker(bind=migrate_engine)()
+
+    project_table = sql.Table('project', meta, autoload=True)
+    insert = tenant_table.insert()
+    for project in session.query(project_table):
+        insert.values(project).execute()
+    project_table.drop()
+
+    user_project_membership_table = sql.Table('user_project_membership',
+                                              meta,
+                                              autoload=True)
+    insert = user_tenant_membership_table.insert()
+    for membership in session.query(user_project_membership_table):
+        insert.execute(membership)
+    user_project_membership_table.drop()
+
+    session.commit()
+    session.close()
+
+
+def upgrade(migrate_engine):
+    meta = sql.MetaData()
+    meta.bind = migrate_engine
+    if migrate_engine.name == "sqlite":
+        upgrade_with_copy(meta, migrate_engine)
+    else:
+        upgrade_with_rename(meta, migrate_engine)
+
+
+def downgrade(migrate_engine):
+    meta = sql.MetaData()
+    meta.bind = migrate_engine
+    if migrate_engine.name == "sqlite":
+        downgrade_with_copy(meta, migrate_engine)
+    else:
+        downgrade_with_rename(meta, migrate_engine)
diff --git a/keystone/common/sql/migrate_repo/versions/017_membership_role.py b/keystone/common/sql/migrate_repo/versions/017_membership_role.py
new file mode 100644
index 00000000..ed5b482f
--- /dev/null
+++ b/keystone/common/sql/migrate_repo/versions/017_membership_role.py
@@ -0,0 +1,102 @@
+import json
+import uuid
+
+import sqlalchemy as sql
+from sqlalchemy import orm
+
+from keystone import config
+from keystone import exception
+
+
+CONF = config.CONF
+
+
+def upgrade(migrate_engine):
+    meta = sql.MetaData()
+    meta.bind = migrate_engine
+
+    user_table = sql.Table('user', meta, autoload=True)
+    role_table = sql.Table('role', meta, autoload=True)
+    project_table = sql.Table('project', meta, autoload=True)
+
+    user_project_role_table = sql.Table(
+        'user_project_metadata',
+        meta,
+        sql.Column('user_id',
+                   sql.String(64),
+                   sql.ForeignKey('user.id'),
+                   primary_key=True),
+        sql.Column('project_id',
+                   sql.String(64),
+                   sql.ForeignKey('project.id'),
+                   primary_key=True),
+        sql.Column('data', sql.Text()))
+    user_project_role_table.create(migrate_engine, checkfirst=True)
+
+    conn = migrate_engine.connect()
+    conn.execute(role_table.insert(),
+                 id=CONF.member_role_id,
+                 name=CONF.member_role_name,
+                 extra=json.dumps({'description':
+                                   'Default role for project membership',
+                                   'enabled': 'True'}))
+
+    user_project_membership_table = sql.Table('user_project_membership',
+                                              meta, autoload=True)
+    session = sql.orm.sessionmaker(bind=migrate_engine)()
+    for membership in session.query(user_project_membership_table):
+        data = {'roles': [config.CONF.member_role_id]}
+        ins = user_project_role_table.insert().values(
+            user_id=membership.user_id,
+            project_id=membership.project_id,
+            data=json.dumps(data))
+        conn.execute(ins)
+    session.close()
+    user_project_membership_table.drop()
+
+
+def downgrade(migrate_engine):
+    meta = sql.MetaData()
+    meta.bind = migrate_engine
+
+    user_table = sql.Table('user', meta, autoload=True)
+    project_table = sql.Table('project', meta, autoload=True)
+
+    user_project_membership_table = sql.Table(
+        'user_project_membership',
+        meta,
+        sql.Column(
+            'user_id',
+            sql.String(64),
+            sql.ForeignKey('user.id'),
+            primary_key=True),
+        sql.Column(
+            'project_id',
+            sql.String(64),
+            sql.ForeignKey('project.id'),
+            primary_key=True))
+    user_project_membership_table.create(migrate_engine, checkfirst=True)
+
+    user_project_metadata_table = sql.Table(
+        'user_project_metadata',
+        meta,
+        autoload=True)
+
+    session = sql.orm.sessionmaker(bind=migrate_engine)()
+    for membership in session.query(user_project_metadata_table):
+        if 'roles' in membership:
+            roles = membership['roles']
+            if config.CONF.member_role_id in roles:
+                ins = (user_project_membership_table.insert()
+                       .values(user_id=membership.user_id,
+                               project_id=membership.project_id))
+    session.close()
+    role_table = sql.Table('role', meta, autoload=True)
+    conn = migrate_engine.connect()
+    user_project_membership_table = sql.Table(
+        'user_project_membership', meta, autoload=True)
+
+    role_table = sql.Table('role', meta, autoload=True)
+    conn.execute(role_table.delete().where(role_table.c.id ==
+                                           config.CONF.member_role_id))
+    user_project_metadata_table.drop()
diff --git a/keystone/config.py b/keystone/config.py
index b604db43..f2d8f546 100644
--- a/keystone/config.py
+++ b/keystone/config.py
@@ -193,6 +193,10 @@ register_int('max_request_body_size', default=114688)
 register_int('max_param_size', default=64)
 # we allow tokens to be a bit larger to accommodate PKI
 register_int('max_token_size', default=8192)
+register_str('member_role_id',
+             default='9fe2ff9ee4384b1894a90878d3e92bab')
+register_str('member_role_name', default='_member_')
+
 
 # identity
 register_str('default_domain_id', group='identity', default='default')
diff --git a/keystone/identity/backends/kvs.py b/keystone/identity/backends/kvs.py
index 1d8ce608..6c59fa03 100644
--- a/keystone/identity/backends/kvs.py
+++ b/keystone/identity/backends/kvs.py
@@ -134,24 +134,6 @@ class Identity(kvs.Base, identity.Driver):
         role_ids = self.db.get('role_list', [])
         return [self.get_role(x) for x in role_ids]
 
-    # These should probably be part of the high-level API
-    def add_user_to_project(self, tenant_id, user_id):
-        self.get_project(tenant_id)
-        user_ref = self._get_user(user_id)
-        tenants = set(user_ref.get('tenants', []))
-        tenants.add(tenant_id)
-        self.update_user(user_id, {'tenants': list(tenants)})
-
-    def remove_user_from_project(self, tenant_id, user_id):
-        self.get_project(tenant_id)
-        user_ref = self._get_user(user_id)
-        tenants = set(user_ref.get('tenants', []))
-        try:
-            tenants.remove(tenant_id)
-        except KeyError:
-            raise exception.NotFound('User not found in tenant')
-        self.update_user(user_id, {'tenants': list(tenants)})
-
     def get_projects_for_user(self, user_id):
         user_ref = self._get_user(user_id)
         return user_ref.get('tenants', [])
@@ -194,7 +176,16 @@ class Identity(kvs.Base, identity.Driver):
 
         roles.remove(role_id)
         metadata_ref['roles'] = list(roles)
-        self.update_metadata(user_id, tenant_id, metadata_ref)
+
+        if not len(roles):
+            self.db.delete('metadata-%s-%s' % (tenant_id, user_id))
+            user_ref = self._get_user(user_id)
+            tenants = set(user_ref.get('tenants', []))
+            tenants.remove(tenant_id)
+            user_ref['tenants'] = list(tenants)
+            self.update_user(user_id, user_ref)
+        else:
+            self.update_metadata(user_id, tenant_id, metadata_ref)
 
     # CRUD
     def create_user(self, user_id, user):
@@ -360,6 +351,12 @@ class Identity(kvs.Base, identity.Driver):
         if user_id:
             if tenant_id:
                 self.db.set('metadata-%s-%s' % (tenant_id, user_id), metadata)
+                user_ref = self._get_user(user_id)
+                tenants = set(user_ref.get('tenants', []))
+                if tenant_id not in tenants:
+                    tenants.add(tenant_id)
+                    user_ref['tenants'] = list(tenants)
+                    self.update_user(user_id, user_ref)
             else:
                 self.db.set('metadata-%s-%s' % (domain_id, user_id), metadata)
         else:
diff --git a/keystone/identity/backends/ldap/core.py b/keystone/identity/backends/ldap/core.py
index db95e246..22a7fd0c 100644
--- a/keystone/identity/backends/ldap/core.py
+++ b/keystone/identity/backends/ldap/core.py
@@ -152,12 +152,6 @@ class Identity(identity.Driver):
     def list_roles(self):
         return self.role.get_all()
 
-    # These should probably be part of the high-level API
-    def add_user_to_project(self, tenant_id, user_id):
-        self.get_project(tenant_id)
-        self.get_user(user_id)
-        return self.project.add_user(tenant_id, user_id)
-
     def get_projects_for_user(self, user_id):
         self.get_user(user_id)
         return [p['id'] for p in self.project.get_user_projects(user_id)]
@@ -243,11 +237,6 @@ class Identity(identity.Driver):
     def remove_role_from_user_and_project(self, user_id, tenant_id, role_id):
         return self.role.delete_user(role_id, user_id, tenant_id)
 
-    def remove_user_from_project(self, tenant_id, user_id):
-        self.get_user(user_id)
-        self.get_project(tenant_id)
-        return self.project.remove_user(tenant_id, user_id)
-
     def update_role(self, role_id, role):
         self.get_role(role_id)
         self.role.update(role_id, role)
@@ -551,13 +540,17 @@ class ProjectApi(common_ldap.BaseLdap, ApiShimMixin):
 
     def get_user_projects(self, user_id):
         """Returns list of tenants a user has access to
-
-        Always includes default tenants.
         """
-        user_dn = self.user_api._id_to_dn(user_id)
-        query = '(%s=%s)' % (self.member_attribute, user_dn)
-        memberships = self.get_all(query)
-        return memberships
+        associations = self.role_api.list_project_roles_for_user(user_id)
+        project_ids = set()
+        for assoc in associations:
+            project_ids.add(assoc.project_id)
+        projects = []
+        for project_id in project_ids:
+            #slower to get them one at a time, but a huge list could blow out
+            #the connection.  This is the safer way
+            projects.append(self.get(project_id))
+        return projects
 
     def list_for_user_get_page(self, user, marker, limit):
         return self._get_page(marker,
diff --git a/keystone/identity/backends/sql.py b/keystone/identity/backends/sql.py
index 1d821287..bda2bf93 100644
--- a/keystone/identity/backends/sql.py
+++ b/keystone/identity/backends/sql.py
@@ -17,6 +17,7 @@
 import functools
 
 from keystone import clean
+from keystone import config
 from keystone.common import sql
 from keystone.common.sql import migration
 from keystone.common import utils
@@ -82,7 +83,6 @@ class Domain(sql.ModelBase, sql.DictBase):
     extra = sql.Column(sql.JsonBlob())
 
 
-# TODO(dolph): rename to Project
 class Project(sql.ModelBase, sql.DictBase):
     __tablename__ = 'project'
     attributes = ['id', 'name', 'domain_id']
@@ -114,11 +114,13 @@ class BaseGrant(sql.DictBase):
 
 
 class UserProjectGrant(sql.ModelBase, BaseGrant):
-    # TODO(dolph): rename to user_project_metadata (needs a migration)
-    __tablename__ = 'metadata'
-    user_id = sql.Column(sql.String(64), primary_key=True)
-    # TODO(dolph): rename to project_id (needs a migration)
-    tenant_id = sql.Column(sql.String(64), primary_key=True)
+    __tablename__ = 'user_project_metadata'
+    user_id = sql.Column(sql.String(64),
+                         sql.ForeignKey('user.id'),
+                         primary_key=True)
+    project_id = sql.Column(sql.String(64),
+                            sql.ForeignKey('project.id'),
+                            primary_key=True)
     data = sql.Column(sql.JsonBlob())
 
 
@@ -143,18 +145,6 @@ class GroupDomainGrant(sql.ModelBase, BaseGrant):
     data = sql.Column(sql.JsonBlob())
 
 
-# TODO(dolph): ... do we need this table?
-class UserProjectMembership(sql.ModelBase, sql.DictBase):
-    """Project membership join table."""
-    __tablename__ = 'user_project_membership'
-    user_id = sql.Column(sql.String(64),
-                         sql.ForeignKey('user.id'),
-                         primary_key=True)
-    tenant_id = sql.Column(sql.String(64),
-                           sql.ForeignKey('project.id'),
-                           primary_key=True)
-
-
 class UserGroupMembership(sql.ModelBase, sql.DictBase):
     """Group membership join table."""
     __tablename__ = 'user_group_membership'
@@ -242,8 +232,8 @@ class Identity(sql.Base, identity.Driver):
         session = self.get_session()
         self.get_project(tenant_id)
         query = session.query(User)
-        query = query.join(UserProjectMembership)
-        query = query.filter(UserProjectMembership.tenant_id == tenant_id)
+        query = query.join(UserProjectGrant)
+        query = query.filter(UserProjectGrant.project_id == tenant_id)
         user_refs = query.all()
         return [identity.filter_user(user_ref.to_dict())
                 for user_ref in user_refs]
@@ -255,7 +245,7 @@ class Identity(sql.Base, identity.Driver):
         if user_id:
             if tenant_id:
                 q = session.query(UserProjectGrant)
-                q = q.filter_by(tenant_id=tenant_id)
+                q = q.filter_by(project_id=tenant_id)
             elif domain_id:
                 q = session.query(UserDomainGrant)
                 q = q.filter_by(domain_id=domain_id)
@@ -375,37 +365,6 @@ class Identity(sql.Base, identity.Driver):
             self.update_metadata(user_id, project_id, metadata_ref,
                                  domain_id, group_id)
 
-    # These should probably be part of the high-level API
-    def add_user_to_project(self, tenant_id, user_id):
-        session = self.get_session()
-        self.get_project(tenant_id)
-        self.get_user(user_id)
-        query = session.query(UserProjectMembership)
-        query = query.filter_by(user_id=user_id)
-        query = query.filter_by(tenant_id=tenant_id)
-        rv = query.first()
-        if rv:
-            return
-
-        with session.begin():
-            session.add(UserProjectMembership(user_id=user_id,
-                                              tenant_id=tenant_id))
-            session.flush()
-
-    def remove_user_from_project(self, tenant_id, user_id):
-        session = self.get_session()
-        self.get_project(tenant_id)
-        self.get_user(user_id)
-        query = session.query(UserProjectMembership)
-        query = query.filter_by(user_id=user_id)
-        query = query.filter_by(tenant_id=tenant_id)
-        membership_ref = query.first()
-        if membership_ref is None:
-            raise exception.NotFound('User not found in tenant')
-        with session.begin():
-            session.delete(membership_ref)
-            session.flush()
-
     def list_projects(self):
         session = self.get_session()
         tenant_refs = session.query(Project).all()
@@ -414,10 +373,10 @@ class Identity(sql.Base, identity.Driver):
     def get_projects_for_user(self, user_id):
         session = self.get_session()
         self.get_user(user_id)
-        query = session.query(UserProjectMembership)
+        query = session.query(UserProjectGrant)
         query = query.filter_by(user_id=user_id)
         membership_refs = query.all()
-        return [x.tenant_id for x in membership_refs]
+        return [x.project_id for x in membership_refs]
 
     def get_roles_for_user_and_project(self, user_id, tenant_id):
         self.get_user(user_id)
@@ -453,22 +412,24 @@ class Identity(sql.Base, identity.Driver):
     def remove_role_from_user_and_project(self, user_id, tenant_id, role_id):
         try:
             metadata_ref = self.get_metadata(user_id, tenant_id)
-            is_new = False
+            roles = set(metadata_ref.get('roles', []))
+            if role_id not in roles:
+                msg = _('Cannot remove role that has not been granted, %s' %
+                        role_id)
+                raise exception.RoleNotFound(message=msg)
+            roles.remove(role_id)
+            metadata_ref['roles'] = list(roles)
+            if len(roles):
+                self.update_metadata(user_id, tenant_id, metadata_ref)
+            else:
+                session = self.get_session()
+                q = session.query(UserProjectGrant)
+                q = q.filter_by(project_id=tenant_id)
+                q.delete()
         except exception.MetadataNotFound:
-            metadata_ref = {}
-            is_new = True
-        roles = set(metadata_ref.get('roles', []))
-        if role_id not in roles:
             msg = 'Cannot remove role that has not been granted, %s' % role_id
             raise exception.RoleNotFound(message=msg)
 
-        roles.remove(role_id)
-        metadata_ref['roles'] = list(roles)
-        if is_new:
-            self.create_metadata(user_id, tenant_id, metadata_ref)
-        else:
-            self.update_metadata(user_id, tenant_id, metadata_ref)
-
     # CRUD
     @handle_conflicts(type='project')
     def create_project(self, tenant_id, tenant):
@@ -512,12 +473,12 @@ class Identity(sql.Base, identity.Driver):
             raise exception.ProjectNotFound(project_id=tenant_id)
 
         with session.begin():
-            q = session.query(UserProjectMembership)
-            q = q.filter_by(tenant_id=tenant_id)
+            q = session.query(UserProjectGrant)
+            q = q.filter_by(project_id=tenant_id)
             q.delete(False)
 
             q = session.query(UserProjectGrant)
-            q = q.filter_by(tenant_id=tenant_id)
+            q = q.filter_by(project_id=tenant_id)
             q.delete(False)
 
             q = session.query(GroupProjectGrant)
@@ -539,7 +500,7 @@ class Identity(sql.Base, identity.Driver):
             if user_id:
                 if tenant_id:
                     session.add(UserProjectGrant(user_id=user_id,
-                                                 tenant_id=tenant_id,
+                                                 project_id=tenant_id,
                                                  data=metadata))
                 elif domain_id:
                     session.add(UserDomainGrant(user_id=user_id,
@@ -566,7 +527,7 @@ class Identity(sql.Base, identity.Driver):
                 if tenant_id:
                     q = session.query(UserProjectGrant)
                     q = q.filter_by(user_id=user_id)
-                    q = q.filter_by(tenant_id=tenant_id)
+                    q = q.filter_by(project_id=tenant_id)
                 elif domain_id:
                     q = session.query(UserDomainGrant)
                     q = q.filter_by(user_id=user_id)
@@ -651,7 +612,7 @@ class Identity(sql.Base, identity.Driver):
         metadata_refs = session\
             .query(UserProjectGrant)\
             .filter_by(user_id=user_id)
-        project_ids = set([x.tenant_id for x in metadata_refs
+        project_ids = set([x.project_id for x in metadata_refs
                            if x.data.get('roles')])
         if user.get('project_id'):
             project_ids.add(user['project_id'])
@@ -797,9 +758,6 @@ class Identity(sql.Base, identity.Driver):
             raise exception.UserNotFound(user_id=user_id)
 
         with session.begin():
-            q = session.query(UserProjectMembership)
-            q = q.filter_by(user_id=user_id)
-            q.delete(False)
 
             q = session.query(UserProjectGrant)
             q = q.filter_by(user_id=user_id)
@@ -999,7 +957,7 @@ class Identity(sql.Base, identity.Driver):
                 metadata = metadata_ref.to_dict()
                 try:
                     self.remove_role_from_user_and_project(
-                        metadata['user_id'], metadata['tenant_id'], role_id)
+                        metadata['user_id'], metadata['project_id'], role_id)
                 except exception.RoleNotFound:
                     pass
 
diff --git a/keystone/identity/controllers.py b/keystone/identity/controllers.py
index 8060e28f..31125267 100644
--- a/keystone/identity/controllers.py
+++ b/keystone/identity/controllers.py
@@ -304,9 +304,6 @@ class Role(controller.V2Controller):
             raise exception.NotImplemented(message='User roles not supported: '
                                                    'tenant_id required')
 
-        # This still has the weird legacy semantics that adding a role to
-        # a user also adds them to a tenant
-        self.identity_api.add_user_to_project(context, tenant_id, user_id)
         self.identity_api.add_role_to_user_and_project(
             context, user_id, tenant_id, role_id)
         self.token_api.revoke_tokens(context, user_id, tenant_id)
@@ -332,9 +329,6 @@ class Role(controller.V2Controller):
             context, user_id, tenant_id, role_id)
         roles = self.identity_api.get_roles_for_user_and_project(
             context, user_id, tenant_id)
-        if not roles:
-            self.identity_api.remove_user_from_project(
-                context, tenant_id, user_id)
         self.token_api.revoke_tokens(context, user_id, tenant_id)
 
     # COMPAT(diablo): CRUD extension
@@ -375,7 +369,6 @@ class Role(controller.V2Controller):
         # TODO(termie): for now we're ignoring the actual role
         tenant_id = role.get('tenantId')
         role_id = role.get('roleId')
-        self.identity_api.add_user_to_project(context, tenant_id, user_id)
         self.identity_api.add_role_to_user_and_project(
             context, user_id, tenant_id, role_id)
         self.token_api.revoke_tokens(context, user_id, tenant_id)
@@ -404,9 +397,6 @@ class Role(controller.V2Controller):
             context, user_id, tenant_id, role_id)
         roles = self.identity_api.get_roles_for_user_and_project(
             context, user_id, tenant_id)
-        if not roles:
-            self.identity_api.remove_user_from_project(
-                context, tenant_id, user_id)
         self.token_api.revoke_tokens(context, user_id, tenant_id)
 
 
diff --git a/keystone/identity/core.py b/keystone/identity/core.py
index 7810d3f4..7d8c991f 100644
--- a/keystone/identity/core.py
+++ b/keystone/identity/core.py
@@ -102,22 +102,28 @@ class Driver(object):
         raise exception.NotImplemented()
 
     def add_user_to_project(self, tenant_id, user_id):
-        """Add user to a tenant without an explicit role relationship.
+            """Add user to a tenant by creating a default role relationship.
 
-        :raises: keystone.exception.ProjectNotFound,
-                 keystone.exception.UserNotFound
+            :raises: keystone.exception.ProjectNotFound,
+                     keystone.exception.UserNotFound
 
-        """
-        raise exception.NotImplemented()
+            """
+            self.add_role_to_user_and_project(user_id,
+                                              tenant_id,
+                                              config.CONF.member_role_id)
 
     def remove_user_from_project(self, tenant_id, user_id):
-        """Remove user from a tenant without an explicit role relationship.
+        """Remove user from a tenant
 
         :raises: keystone.exception.ProjectNotFound,
                  keystone.exception.UserNotFound
 
         """
-        raise exception.NotImplemented()
+        roles = self.get_roles_for_user_and_project(user_id, tenant_id)
+        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)
 
     def get_project_users(self, tenant_id):
         """Lists all users with a relationship to the specified project.
diff --git a/keystone/test.py b/keystone/test.py
index 01f5da8f..5b69313e 100644
--- a/keystone/test.py
+++ b/keystone/test.py
@@ -237,6 +237,13 @@ class TestCase(NoModule, unittest.TestCase):
                 rv = self.identity_api.create_project(tenant['id'], tenant)
                 setattr(self, 'tenant_%s' % tenant['id'], rv)
 
+            for role in fixtures.ROLES:
+                try:
+                    rv = self.identity_api.create_role(role['id'], role)
+                except exception.Conflict:
+                    pass
+                setattr(self, 'role_%s' % role['id'], rv)
+
             for user in fixtures.USERS:
                 user_copy = user.copy()
                 tenants = user_copy.pop('tenants')
@@ -247,10 +254,6 @@ class TestCase(NoModule, unittest.TestCase):
                                                           user['id'])
                 setattr(self, 'user_%s' % user['id'], user_copy)
 
-            for role in fixtures.ROLES:
-                rv = self.identity_api.create_role(role['id'], role)
-                setattr(self, 'role_%s' % role['id'], rv)
-
             for metadata in fixtures.METADATA:
                 metadata_ref = metadata.copy()
                 # TODO(termie): these will probably end up in the model anyway,
diff --git a/tests/default_fixtures.py b/tests/default_fixtures.py
index e20f9d7a..44fb64fb 100644
--- a/tests/default_fixtures.py
+++ b/tests/default_fixtures.py
@@ -20,6 +20,9 @@
 from keystone import config
 
 
+CONF = config.CONF
+
+
 DEFAULT_DOMAIN_ID = config.CONF.identity.default_domain_id
 
 
@@ -34,6 +37,12 @@ TENANTS = [
         'domain_id': DEFAULT_DOMAIN_ID,
         'description': 'description',
         'enabled': True,
+    }, {
+        'id': 'mtu',
+        'name': 'MTU',
+        'description': 'description',
+        'enabled': True,
+        'domain_id': DEFAULT_DOMAIN_ID
     }
 ]
 
@@ -63,14 +72,20 @@ USERS = [
         'enabled': False,
         'tenant_id': 'baz',
         'tenants': ['baz'],
+    }, {
+        'id': 'sna',
+        'name': 'SNA',
+        'domain_id': DEFAULT_DOMAIN_ID,
+        'password': 'snafu',
+        'enabled': True,
+        'tenants': ['bar']
     }
 ]
 
 METADATA = [
     {
-        'user_id': 'foo',
-        'tenant_id': 'bar',
-        'extra': 'extra',
+        'user_id': 'sna',
+        'tenant_id': 'mtu',
     }
 ]
 
@@ -81,5 +96,11 @@ ROLES = [
     }, {
         'id': 'member',
         'name': 'Member',
+    }, {
+        'id': CONF.member_role_id,
+        'name': CONF.member_role_name,
+    }, {
+        'id': 'other',
+        'name': 'Other',
     }
 ]
diff --git a/tests/test_backend.py b/tests/test_backend.py
index 172afbcc..12e99b6c 100644
--- a/tests/test_backend.py
+++ b/tests/test_backend.py
@@ -20,6 +20,7 @@ import uuid
 import nose.exc
 
 from keystone.catalog import core
+from keystone import config
 from keystone import exception
 from keystone.openstack.common import timeutils
 from keystone import config
@@ -31,6 +32,25 @@ DEFAULT_DOMAIN_ID = CONF.identity.default_domain_id
 
 
 class IdentityTests(object):
+    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])
+
+        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])
+
+        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])
+
     def test_authenticate_bad_user(self):
         self.assertRaises(AssertionError,
                           self.identity_api.authenticate,
@@ -66,23 +86,25 @@ class IdentityTests(object):
 
     def test_authenticate(self):
         user_ref, tenant_ref, metadata_ref = self.identity_api.authenticate(
-            user_id=self.user_foo['id'],
+            user_id=self.user_sna['id'],
             tenant_id=self.tenant_bar['id'],
-            password=self.user_foo['password'])
+            password=self.user_sna['password'])
         # 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
-        self.user_foo.pop('password')
-        self.assertDictEqual(user_ref, self.user_foo)
+        self.user_sna.pop('password')
+        self.user_sna['enabled'] = True
+        self.assertDictEqual(user_ref, self.user_sna)
         self.assertDictEqual(tenant_ref, self.tenant_bar)
-        self.assertDictEqual(metadata_ref, self.metadata_foobar)
+        metadata_ref.pop('roles')
+        self.assertDictEqual(metadata_ref, self.metadata_snamtu)
 
     def test_authenticate_role_return(self):
         self.identity_api.add_role_to_user_and_project(
-            self.user_foo['id'], self.tenant_bar['id'], 'keystone_admin')
+            self.user_foo['id'], self.tenant_baz['id'], 'keystone_admin')
         user_ref, tenant_ref, metadata_ref = self.identity_api.authenticate(
             user_id=self.user_foo['id'],
-            tenant_id=self.tenant_bar['id'],
+            tenant_id=self.tenant_baz['id'],
             password=self.user_foo['password'])
         self.assertIn('roles', metadata_ref)
         self.assertIn('keystone_admin', metadata_ref['roles'])
@@ -105,7 +127,8 @@ class IdentityTests(object):
         #               it easier to authenticate in tests, but should
         #               not be returned by the api
         user.pop('password')
-        self.assertEquals(metadata_ref, {})
+        self.assertEquals(metadata_ref, {"roles":
+                                         [CONF.member_role_id]})
         self.assertDictEqual(user_ref, user)
         self.assertDictEqual(tenant_ref, self.tenant_baz)
 
@@ -181,9 +204,10 @@ class IdentityTests(object):
 
     def test_get_metadata(self):
         metadata_ref = self.identity_api.get_metadata(
-            user_id=self.user_foo['id'],
+            user_id=self.user_sna['id'],
             tenant_id=self.tenant_bar['id'])
-        self.assertDictEqual(metadata_ref, self.metadata_foobar)
+        metadata_ref.pop('roles')
+        self.assertDictEqual(metadata_ref, self.metadata_snamtu)
 
     def test_get_metadata_404(self):
         # FIXME(dolph): these exceptions could be more specific
@@ -421,14 +445,14 @@ class IdentityTests(object):
         roles_ref = self.identity_api.list_grants(
             user_id=self.user_foo['id'],
             project_id=self.tenant_bar['id'])
-        self.assertEquals(len(roles_ref), 0)
+        self.assertEquals(len(roles_ref), 1)
         self.identity_api.create_grant(user_id=self.user_foo['id'],
                                        project_id=self.tenant_bar['id'],
                                        role_id='keystone_admin')
         roles_ref = self.identity_api.list_grants(
             user_id=self.user_foo['id'],
             project_id=self.tenant_bar['id'])
-        self.assertDictEqual(roles_ref[0], self.role_keystone_admin)
+        self.assertDictEqual(roles_ref[1], self.role_keystone_admin)
 
         self.identity_api.create_grant(user_id=self.user_foo['id'],
                                        project_id=self.tenant_bar['id'],
@@ -475,24 +499,24 @@ class IdentityTests(object):
 
     def test_remove_role_grant_from_user_and_project(self):
         self.identity_api.create_grant(user_id=self.user_foo['id'],
-                                       project_id=self.tenant_bar['id'],
+                                       project_id=self.tenant_baz['id'],
                                        role_id='member')
         roles_ref = self.identity_api.list_grants(
             user_id=self.user_foo['id'],
-            project_id=self.tenant_bar['id'])
+            project_id=self.tenant_baz['id'])
         self.assertDictEqual(roles_ref[0], self.role_member)
 
         self.identity_api.delete_grant(user_id=self.user_foo['id'],
-                                       project_id=self.tenant_bar['id'],
+                                       project_id=self.tenant_baz['id'],
                                        role_id='member')
         roles_ref = self.identity_api.list_grants(
             user_id=self.user_foo['id'],
-            project_id=self.tenant_bar['id'])
+            project_id=self.tenant_baz['id'])
         self.assertEquals(len(roles_ref), 0)
         self.assertRaises(exception.NotFound,
                           self.identity_api.delete_grant,
                           user_id=self.user_foo['id'],
-                          project_id=self.tenant_bar['id'],
+                          project_id=self.tenant_baz['id'],
                           role_id='member')
 
     def test_get_and_remove_role_grant_by_group_and_project(self):
@@ -1115,10 +1139,10 @@ class IdentityTests(object):
                           role)
 
     def test_add_user_to_project(self):
-        self.identity_api.add_user_to_project(self.tenant_bar['id'],
+        self.identity_api.add_user_to_project(self.tenant_baz['id'],
                                               self.user_foo['id'])
         tenants = self.identity_api.get_projects_for_user(self.user_foo['id'])
-        self.assertIn(self.tenant_bar['id'], tenants)
+        self.assertIn(self.tenant_baz['id'], tenants)
 
     def test_add_user_to_project_404(self):
         self.assertRaises(exception.ProjectNotFound,
@@ -1132,12 +1156,12 @@ class IdentityTests(object):
                           uuid.uuid4().hex)
 
     def test_remove_user_from_project(self):
-        self.identity_api.add_user_to_project(self.tenant_bar['id'],
+        self.identity_api.add_user_to_project(self.tenant_baz['id'],
                                               self.user_foo['id'])
-        self.identity_api.remove_user_from_project(self.tenant_bar['id'],
+        self.identity_api.remove_user_from_project(self.tenant_baz['id'],
                                                    self.user_foo['id'])
         tenants = self.identity_api.get_projects_for_user(self.user_foo['id'])
-        self.assertNotIn(self.tenant_bar['id'], tenants)
+        self.assertNotIn(self.tenant_baz['id'], tenants)
 
     def test_remove_user_from_project_404(self):
         self.assertRaises(exception.ProjectNotFound,
@@ -1385,7 +1409,7 @@ class IdentityTests(object):
 
     def test_list_projects(self):
         projects = self.identity_api.list_projects()
-        self.assertEquals(len(projects), 2)
+        self.assertEquals(len(projects), 3)
         project_ids = []
         for project in projects:
             project_ids.append(project.get('id'))
diff --git a/tests/test_keystoneclient.py b/tests/test_keystoneclient.py
index 3863ff54..14089759 100644
--- a/tests/test_keystoneclient.py
+++ b/tests/test_keystoneclient.py
@@ -804,16 +804,19 @@ class KcMasterTestCase(CompatTestCase, KeystoneClientTests):
 
     def test_tenant_add_and_remove_user(self):
         client = self.get_client(admin=True)
-        client.roles.add_user_role(tenant=self.tenant_baz['id'],
+        client.roles.add_user_role(tenant=self.tenant_bar['id'],
                                    user=self.user_two['id'],
-                                   role=self.role_member['id'])
-        user_refs = client.tenants.list_users(tenant=self.tenant_baz['id'])
+                                   role=self.role_other['id'])
+        user_refs = client.tenants.list_users(tenant=self.tenant_bar['id'])
         self.assert_(self.user_two['id'] in [x.id for x in user_refs])
-        client.roles.remove_user_role(tenant=self.tenant_baz['id'],
+        client.roles.remove_user_role(tenant=self.tenant_bar['id'],
                                       user=self.user_two['id'],
-                                      role=self.role_member['id'])
-        user_refs = client.tenants.list_users(tenant=self.tenant_baz['id'])
-        self.assert_(self.user_two['id'] not in [x.id for x in user_refs])
+                                      role=self.role_other['id'])
+        roles = client.roles.roles_for_user(user=self.user_foo['id'],
+                                            tenant=self.tenant_bar['id'])
+        self.assertNotIn(self.role_other['id'], roles)
+        user_refs = client.tenants.list_users(tenant=self.tenant_bar['id'])
+        self.assertNotIn(self.user_two['id'], [x.id for x in user_refs])
 
     def test_user_role_add_404(self):
         from keystoneclient import exceptions as client_exceptions
@@ -1013,7 +1016,7 @@ class KcEssex3TestCase(CompatTestCase, KeystoneClientTests):
 
     def test_tenant_add_and_remove_user(self):
         client = self.get_client(admin=True)
-        client.roles.add_user_to_tenant(tenant_id=self.tenant_baz['id'],
+        client.roles.add_user_to_tenant(tenant_id=self.tenant_bar['id'],
                                         user_id=self.user_two['id'],
                                         role_id=self.role_member['id'])
         role_refs = client.roles.get_user_role_refs(
@@ -1030,7 +1033,7 @@ class KcEssex3TestCase(CompatTestCase, KeystoneClientTests):
                 # use python's scope fall through to leave roleref_ref set
                 break
 
-        client.roles.remove_user_from_tenant(tenant_id=self.tenant_baz['id'],
+        client.roles.remove_user_from_tenant(tenant_id=self.tenant_bar['id'],
                                              user_id=self.user_two['id'],
                                              role_id=roleref_ref.id)
 
diff --git a/tests/test_migrate_nova_auth.py b/tests/test_migrate_nova_auth.py
index 3a257dff..4e3e37b8 100644
--- a/tests/test_migrate_nova_auth.py
+++ b/tests/test_migrate_nova_auth.py
@@ -131,7 +131,8 @@ class MigrateNovaAuth(test.TestCase):
 
         roles = self.identity_api.list_roles()
         role_names = set([role['name'] for role in roles])
-        self.assertEqual(role_names, set(['role2', 'role1', 'role3']))
+        self.assertEqual(role_names, set(['role2', 'role1', 'role3',
+                                          CONF.member_role_name]))
 
         assignment_map = {
             'user1': {'proj1': ['role1', 'role2']},
@@ -149,5 +150,7 @@ class MigrateNovaAuth(test.TestCase):
                     user['id'], tenant['id'])
                 actual = [self.identity_api.get_role(role_id)['name']
                           for role_id in roles]
+                if CONF.member_role_name in actual:
+                    actual.remove(CONF.member_role_name)
                 expected = old_project_map.get(tenant_name, [])
                 self.assertEqual(set(actual), set(expected))
diff --git a/tests/test_sql_upgrade.py b/tests/test_sql_upgrade.py
index d204492c..7dcce7fe 100644
--- a/tests/test_sql_upgrade.py
+++ b/tests/test_sql_upgrade.py
@@ -56,9 +56,9 @@ class SqlUpgradeTests(test.TestCase):
         self.config([test.etcdir('keystone.conf.sample'),
                      test.testsdir('test_overrides.conf'),
                      test.testsdir('backend_sql.conf')])
+        self.base = sql.Base()
 
         # create and share a single sqlalchemy engine for testing
-        self.base = sql.Base()
         self.engine = self.base.get_engine(allow_global_engine=False)
         self.Session = self.base.get_sessionmaker(engine=self.engine,
                                                   autocommit=False)
@@ -107,7 +107,7 @@ class SqlUpgradeTests(test.TestCase):
         actual_cols = [col.name for col in table.columns]
         self.assertEqual(expected_cols, actual_cols, '%s table' % table_name)
 
-    def test_upgrade_0_to_1(self):
+    def test_upgrade_add_initial_tables(self):
         self.upgrade(1)
         self.assertTableColumns("user", ["id", "name", "extra"])
         self.assertTableColumns("tenant", ["id", "name", "extra"])
@@ -117,7 +117,7 @@ class SqlUpgradeTests(test.TestCase):
         self.assertTableColumns("metadata", ["user_id", "tenant_id", "data"])
         self.populate_user_table()
 
-    def test_upgrade_5_to_6(self):
+    def test_upgrade_add_policy(self):
         self.upgrade(5)
         self.assertTableDoesNotExist('policy')
 
@@ -125,7 +125,7 @@ class SqlUpgradeTests(test.TestCase):
         self.assertTableExists('policy')
         self.assertTableColumns('policy', ['id', 'type', 'blob', 'extra'])
 
-    def test_upgrade_8_to_10(self):
+    def test_upgrade_normalize_identity(self):
         self.upgrade(8)
         self.populate_user_table()
         self.populate_tenant_table()
@@ -179,7 +179,7 @@ class SqlUpgradeTests(test.TestCase):
         session.commit()
         session.close()
 
-    def test_upgrade_10_to_13(self):
+    def test_upgrade_endpoints(self):
         self.upgrade(10)
         service_extra = {
             'name': uuid.uuid4().hex,
@@ -266,7 +266,7 @@ class SqlUpgradeTests(test.TestCase):
         self.downgrade(14)
         self.assertTenantTables()
 
-    def test_upgrade_13_to_14(self):
+    def test_upgrade_add_group_tables(self):
         self.upgrade(13)
         self.upgrade(14)
         self.assertTableExists('group')
@@ -327,7 +327,7 @@ class SqlUpgradeTests(test.TestCase):
         session.commit()
         session.close()
 
-    def test_downgrade_14_to_13(self):
+    def test_downgrade_remove_group_tables(self):
         self.upgrade(14)
         self.downgrade(13)
         self.assertTableDoesNotExist('group')
@@ -335,7 +335,7 @@ class SqlUpgradeTests(test.TestCase):
         self.assertTableDoesNotExist('group_domain_metadata')
         self.assertTableDoesNotExist('user_group_membership')
 
-    def test_downgrade_13_to_10(self):
+    def test_downgrade_endpoints(self):
         self.upgrade(13)
 
         service_extra = {
@@ -420,7 +420,7 @@ class SqlUpgradeTests(test.TestCase):
                            "metadata"]:
             self.assertTableDoesNotExist(table_name)
 
-    def test_upgrade_6_to_7(self):
+    def test_upgrade_add_domain_tables(self):
         self.upgrade(6)
         self.assertTableDoesNotExist('credential')
         self.assertTableDoesNotExist('domain')
@@ -436,6 +436,22 @@ class SqlUpgradeTests(test.TestCase):
         self.assertTableColumns('user_domain_metadata',
                                 ['user_id', 'domain_id', 'data'])
 
+    def test_upgrade_default_roles(self):
+        def count_member_roles():
+            session = self.Session()
+            query_string = ("select count(*) as c from role "
+                            "where name='%s'" % config.CONF.member_role_name)
+            role_count = session.execute(query_string).fetchone()['c']
+            session.close()
+            return role_count
+
+        self.upgrade(16)
+        self.assertEquals(0, count_member_roles())
+        self.upgrade(17)
+        self.assertEquals(1, count_member_roles())
+        self.downgrade(16)
+        self.assertEquals(0, count_member_roles())
+
     def populate_user_table(self, with_pass_enab=False,
                             with_pass_enab_domain=False):
         # Populate the appropriate fields in the user
diff --git a/tests/test_v3.py b/tests/test_v3.py
index 28475e14..f5602035 100644
--- a/tests/test_v3.py
+++ b/tests/test_v3.py
@@ -185,7 +185,7 @@ class RestfulTestCase(test_content_types.RestfulTestCase):
         keys = ['name', 'description', 'enabled']
 
         for k in ['id'] + keys:
-            msg = '%s unnexpectedly None in %s' % (k, entity)
+            msg = '%s unexpectedly None in %s' % (k, entity)
             self.assertIsNotNone(entity.get(k), msg)
 
         self.assertIsNotNone(entity.get('links'))