diff --git a/keystone/common/sql/core.py b/keystone/common/sql/core.py index 3634c75a3e..cfec58db8d 100644 --- a/keystone/common/sql/core.py +++ b/keystone/common/sql/core.py @@ -49,6 +49,7 @@ IntegrityError = sql.exc.IntegrityError NotFound = sql.orm.exc.NoResultFound Boolean = sql.Boolean Text = sql.Text +UniqueConstraint = sql.UniqueConstraint def initialize_decorator(init): diff --git a/keystone/common/sql/migrate_repo/versions/014_add_group_tables.py b/keystone/common/sql/migrate_repo/versions/014_add_group_tables.py index 668bca2d58..845db1e316 100644 --- a/keystone/common/sql/migrate_repo/versions/014_add_group_tables.py +++ b/keystone/common/sql/migrate_repo/versions/014_add_group_tables.py @@ -28,9 +28,10 @@ def upgrade(migrate_engine): sql.Column('id', sql.String(64), primary_key=True), sql.Column('domain_id', sql.String(64), sql.ForeignKey('domain.id'), nullable=False), - sql.Column('name', sql.String(64), unique=True, nullable=False), + sql.Column('name', sql.String(64), nullable=False), sql.Column('description', sql.Text()), - sql.Column('extra', sql.Text())) + sql.Column('extra', sql.Text()), + sql.UniqueConstraint('domain_id', 'name')) group_table.create(migrate_engine, checkfirst=True) sql.Table('user', meta, autoload=True) diff --git a/keystone/common/sql/migrate_repo/versions/016_normalize_domain_ids.py b/keystone/common/sql/migrate_repo/versions/016_normalize_domain_ids.py index 4705daf0ae..c64e25a3b3 100644 --- a/keystone/common/sql/migrate_repo/versions/016_normalize_domain_ids.py +++ b/keystone/common/sql/migrate_repo/versions/016_normalize_domain_ids.py @@ -70,16 +70,16 @@ def upgrade_user_table_with_copy(meta, migrate_engine, session): 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("password", sql.String(128)), - sql.Column("enabled", sql.Boolean, default=True)) + sql.Column('password', sql.String(128)), + sql.Column('enabled', sql.Boolean, default=True)) temp_user_table.create(migrate_engine, checkfirst=True) user_table = sql.Table('user', meta, autoload=True) for user in session.query(user_table): - session.execute("insert into temp_user (id, name, extra, " - "password, enabled) " - "values ( :id, :name, :extra, " - ":password, :enabled);", + session.execute('insert into temp_user (id, name, extra, ' + 'password, enabled) ' + 'values ( :id, :name, :extra, ' + ':password, :enabled);', {'id': user.id, 'name': user.name, 'extra': user.extra, @@ -99,21 +99,22 @@ def upgrade_user_table_with_copy(meta, migrate_engine, session): 'user', meta2, sql.Column('id', sql.String(64), primary_key=True), - sql.Column('name', sql.String(64), unique=True, nullable=False), + sql.Column('name', sql.String(64), nullable=False), sql.Column('extra', sql.Text()), sql.Column("password", sql.String(128)), sql.Column("enabled", sql.Boolean, default=True), sql.Column('domain_id', sql.String(64), sql.ForeignKey('domain.id'), - nullable=False)) + nullable=False), + sql.UniqueConstraint('domain_id', 'name')) user_table.create(migrate_engine, checkfirst=True) # Finally copy in the data from our temp table and then clean # up by deleting our temp table for user in session.query(temp_user_table): - session.execute("insert into user (id, name, extra, " - "password, enabled, domain_id) " - "values ( :id, :name, :extra, " - ":password, :enabled, :domain_id);", + session.execute('insert into user (id, name, extra, ' + 'password, enabled, domain_id) ' + 'values ( :id, :name, :extra, ' + ':password, :enabled, :domain_id);', {'id': user.id, 'name': user.name, 'extra': user.extra, @@ -121,7 +122,7 @@ def upgrade_user_table_with_copy(meta, migrate_engine, session): 'enabled': user.enabled, 'domain_id': DEFAULT_DOMAIN_ID}) _enable_foreign_constraints(session, migrate_engine) - session.execute("drop table temp_user;") + session.execute('drop table temp_user;') def upgrade_project_table_with_copy(meta, migrate_engine, session): @@ -138,16 +139,16 @@ def upgrade_project_table_with_copy(meta, migrate_engine, session): 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()), - sql.Column("enabled", sql.Boolean, default=True)) + sql.Column('description', sql.Text()), + sql.Column('enabled', sql.Boolean, default=True)) temp_project_table.create(migrate_engine, checkfirst=True) project_table = sql.Table('project', meta, autoload=True) for project in session.query(project_table): - session.execute("insert into temp_project (id, name, extra, " - "description, enabled) " - "values ( :id, :name, :extra, " - ":description, :enabled);", + session.execute('insert into temp_project (id, name, extra, ' + 'description, enabled) ' + 'values ( :id, :name, :extra, ' + ':description, :enabled);', {'id': project.id, 'name': project.name, 'extra': project.extra, @@ -157,7 +158,7 @@ def upgrade_project_table_with_copy(meta, migrate_engine, session): # Now switch off constraints while we drop and then re-create the # project table, with the additional domain_id column _disable_foreign_constraints(session, migrate_engine) - session.execute("drop table project;") + session.execute('drop table project;') # Need to create a new metadata stream since we are going to load a # different version of the project table meta2 = sql.MetaData() @@ -167,21 +168,22 @@ def upgrade_project_table_with_copy(meta, migrate_engine, session): 'project', meta2, sql.Column('id', sql.String(64), primary_key=True), - sql.Column('name', sql.String(64), unique=True, nullable=False), + sql.Column('name', sql.String(64), nullable=False), sql.Column('extra', sql.Text()), sql.Column('description', sql.Text()), - sql.Column("enabled", sql.Boolean, default=True), + sql.Column('enabled', sql.Boolean, default=True), sql.Column('domain_id', sql.String(64), sql.ForeignKey('domain.id'), - nullable=False)) + nullable=False), + sql.UniqueConstraint('domain_id', 'name')) project_table.create(migrate_engine, checkfirst=True) # Finally copy in the data from our temp table and then clean # up by deleting our temp table for project in session.query(temp_project_table): - session.execute("insert into project (id, name, extra, " - "description, enabled, domain_id) " - "values ( :id, :name, :extra, " - ":description, :enabled, :domain_id);", + session.execute('insert into project (id, name, extra, ' + 'description, enabled, domain_id) ' + 'values ( :id, :name, :extra, ' + ':description, :enabled, :domain_id);', {'id': project.id, 'name': project.name, 'extra': project.extra, @@ -189,7 +191,7 @@ def upgrade_project_table_with_copy(meta, migrate_engine, session): 'enabled': project.enabled, 'domain_id': DEFAULT_DOMAIN_ID}) _enable_foreign_constraints(session, migrate_engine) - session.execute("drop table temp_project;") + session.execute('drop table temp_project;') def downgrade_user_table_with_copy(meta, migrate_engine, session): @@ -204,22 +206,19 @@ def downgrade_user_table_with_copy(meta, migrate_engine, session): meta, sql.Column('id', sql.String(64), primary_key=True), sql.Column('name', sql.String(64), unique=True, nullable=False), - # Temporary table, so no need to make it a foreign key - sql.Column('domain_id', sql.String(64), nullable=False), - sql.Column("password", sql.String(128)), - sql.Column("enabled", sql.Boolean, default=True), + sql.Column('password', sql.String(128)), + sql.Column('enabled', sql.Boolean, default=True), sql.Column('extra', sql.Text())) temp_user_table.create(migrate_engine, checkfirst=True) user_table = sql.Table('user', meta, autoload=True) for user in session.query(user_table): - session.execute("insert into temp_user (id, name, domain_id, " - "password, enabled, extra) " - "values ( :id, :name, :domain_id, " - ":password, :enabled, :extra);", + session.execute('insert into temp_user (id, name, ' + 'password, enabled, extra) ' + 'values ( :id, :name, ' + ':password, :enabled, :extra);', {'id': user.id, 'name': user.name, - 'domain_id': user.domain_id, 'password': user.password, 'enabled': user.enabled, 'extra': user.extra}) @@ -227,7 +226,7 @@ def downgrade_user_table_with_copy(meta, migrate_engine, session): # Now switch off constraints while we drop and then re-create the # user table, less the columns we wanted to drop _disable_foreign_constraints(session, migrate_engine) - session.execute("drop table user;") + session.execute('drop table user;') # Need to create a new metadata stream since we are going to load a # different version of the user table meta2 = sql.MetaData() @@ -238,24 +237,24 @@ def downgrade_user_table_with_copy(meta, migrate_engine, session): 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("password", sql.String(128)), - sql.Column("enabled", sql.Boolean, default=True)) + sql.Column('password', sql.String(128)), + sql.Column('enabled', sql.Boolean, default=True)) user_table.create(migrate_engine, checkfirst=True) _enable_foreign_constraints(session, migrate_engine) # Finally copy in the data from our temp table and then clean # up by deleting our temp table for user in session.query(temp_user_table): - session.execute("insert into user (id, name, extra, " - "password, enabled) " - "values ( :id, :name, :extra, " - ":password, :enabled);", + session.execute('insert into user (id, name, extra, ' + 'password, enabled) ' + 'values ( :id, :name, :extra, ' + ':password, :enabled);', {'id': user.id, 'name': user.name, 'extra': user.extra, 'password': user.password, 'enabled': user.enabled}) - session.execute("drop table temp_user;") + session.execute('drop table temp_user;') def downgrade_project_table_with_copy(meta, migrate_engine, session): @@ -270,22 +269,19 @@ def downgrade_project_table_with_copy(meta, migrate_engine, session): meta, sql.Column('id', sql.String(64), primary_key=True), sql.Column('name', sql.String(64), unique=True, nullable=False), - # Temporary table, so no need to make it a foreign key - sql.Column('domain_id', sql.String(64), nullable=False), sql.Column('description', sql.Text()), - sql.Column("enabled", sql.Boolean, default=True), + sql.Column('enabled', sql.Boolean, default=True), sql.Column('extra', sql.Text())) temp_project_table.create(migrate_engine, checkfirst=True) project_table = sql.Table('project', meta, autoload=True) for project in session.query(project_table): - session.execute("insert into temp_project (id, name, domain_id, " - "description, enabled, extra) " - "values ( :id, :name, :domain_id, " - ":description, :enabled, :extra);", + session.execute('insert into temp_project (id, name, ' + 'description, enabled, extra) ' + 'values ( :id, :name, ' + ':description, :enabled, :extra);', {'id': project.id, 'name': project.name, - 'domain_id': project.domain_id, 'description': project.description, 'enabled': project.enabled, 'extra': project.extra}) @@ -293,7 +289,7 @@ def downgrade_project_table_with_copy(meta, migrate_engine, session): # Now switch off constraints while we drop and then re-create the # project table, less the columns we wanted to drop _disable_foreign_constraints(session, migrate_engine) - session.execute("drop table project;") + session.execute('drop table project;') # Need to create a new metadata stream since we are going to load a # different version of the project table meta2 = sql.MetaData() @@ -304,18 +300,18 @@ def downgrade_project_table_with_copy(meta, migrate_engine, session): 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()), - sql.Column("enabled", sql.Boolean, default=True)) + sql.Column('description', sql.Text()), + sql.Column('enabled', sql.Boolean, default=True)) project_table.create(migrate_engine, checkfirst=True) _enable_foreign_constraints(session, migrate_engine) # Finally copy in the data from our temp table and then clean # up by deleting our temp table for project in session.query(temp_project_table): - session.execute("insert into project (id, name, extra, " - "description, enabled) " - "values ( :id, :name, :extra, " - ":description, :enabled);", + session.execute('insert into project (id, name, extra, ' + 'description, enabled) ' + 'values ( :id, :name, :extra, ' + ':description, :enabled);', {'id': project.id, 'name': project.name, 'extra': project.extra, @@ -345,6 +341,11 @@ def upgrade_user_table_with_col_create(meta, migrate_engine, session): session.commit() user_table.columns.domain_id.alter(nullable=False) + # Finally, change the uniqueness settings for the name attribute + session.execute('ALTER TABLE "user" DROP CONSTRAINT user_name_key;') + session.execute('ALTER TABLE "user" ADD CONSTRAINT user_dom_name_unique ' + 'UNIQUE (domain_id, name);') + def upgrade_project_table_with_col_create(meta, migrate_engine, session): # Create the domain_id column. We want this to be not nullable @@ -367,8 +368,19 @@ def upgrade_project_table_with_col_create(meta, migrate_engine, session): session.commit() project_table.columns.domain_id.alter(nullable=False) + # Finally, change the uniqueness settings for the name attribute + session.execute('ALTER TABLE project DROP CONSTRAINT tenant_name_key;') + session.execute('ALTER TABLE project ADD CONSTRAINT proj_dom_name_unique ' + 'UNIQUE (domain_id, name);') -def downgrade_user_table_with_col_drop(meta, migrate_engine): + +def downgrade_user_table_with_col_drop(meta, migrate_engine, session): + # Revert uniqueness settings for the name attribute + session.execute('ALTER TABLE "user" DROP CONSTRAINT ' + 'user_dom_name_unique;') + session.execute('ALTER TABLE "user" ADD UNIQUE (name);') + session.commit() + # And now go ahead an drop the domain_id column domain_table = sql.Table('domain', meta, autoload=True) user_table = sql.Table('user', meta, autoload=True) column = sql.Column('domain_id', sql.String(64), @@ -376,7 +388,14 @@ def downgrade_user_table_with_col_drop(meta, migrate_engine): column.drop(user_table) -def downgrade_project_table_with_col_drop(meta, migrate_engine): +def downgrade_project_table_with_col_drop(meta, migrate_engine, session): + # Revert uniqueness settings for the name attribute + session.execute('ALTER TABLE project DROP CONSTRAINT ' + 'proj_dom_name_unique;') + session.execute('ALTER TABLE project ADD CONSTRAINT tenant_name_key ' + 'UNIQUE (name);') + session.commit() + # And now go ahead an drop the domain_id column domain_table = sql.Table('domain', meta, autoload=True) project_table = sql.Table('project', meta, autoload=True) column = sql.Column('domain_id', sql.String(64), @@ -408,7 +427,7 @@ def downgrade(migrate_engine): else: # MySQL should in theory be able to use this path, but seems to # have problems dropping columns which are foreign keys - downgrade_user_table_with_col_drop(meta, migrate_engine) - downgrade_project_table_with_col_drop(meta, migrate_engine) + downgrade_user_table_with_col_drop(meta, migrate_engine, session) + downgrade_project_table_with_col_drop(meta, migrate_engine, session) session.commit() session.close() diff --git a/keystone/identity/backends/kvs.py b/keystone/identity/backends/kvs.py index 6c59fa0345..65b3f0423a 100644 --- a/keystone/identity/backends/kvs.py +++ b/keystone/identity/backends/kvs.py @@ -573,7 +573,23 @@ class Identity(kvs.Base, identity.Driver): # group crud def create_group(self, group_id, group): + try: + return self.db.get('group-%s' % group_id) + except exception.NotFound: + pass + else: + msg = _('Duplicate ID, %s.') % group_id + raise exception.Conflict(type='group', details=msg) + try: + self.db.get('group_name-%s' % group['name']) + except exception.NotFound: + pass + else: + msg = _('Duplicate name, %s.') % group['name'] + raise exception.Conflict(type='group', details=msg) + self.db.set('group-%s' % group_id, group) + self.db.set('group_name-%s' % group['name'], group) group_list = set(self.db.get('group_list', [])) group_list.add(group_id) self.db.set('group_list', list(group_list)) @@ -590,10 +606,33 @@ class Identity(kvs.Base, identity.Driver): raise exception.GroupNotFound(group_id=group_id) def update_group(self, group_id, group): + # First, make sure we are not trying to change the + # name to one that is already in use + try: + self.db.get('group_name-%s' % group['name']) + except exception.NotFound: + pass + else: + msg = _('Duplicate name, %s.') % group['name'] + raise exception.Conflict(type='group', details=msg) + + # Now, get the old name and delete it + try: + old_group = self.db.get('group-%s' % group_id) + except exception.NotFound: + raise exception.GroupNotFound(group_id=group_id) + self.db.delete('group_name-%s' % old_group['name']) + + # Finally, actually do the update self.db.set('group-%s' % group_id, group) + self.db.set('group_name-%s' % group['name'], group) return group def delete_group(self, group_id): + try: + group = self.db.get('group-%s' % group_id) + except exception.NotFound: + raise exception.GroupNotFound(group_id=group_id) # Delete any entries in the group lists of all users user_keys = filter(lambda x: x.startswith("user-"), self.db.keys()) user_refs = [self.db.get(key) for key in user_keys] @@ -605,6 +644,7 @@ class Identity(kvs.Base, identity.Driver): # Now delete the group itself self.db.delete('group-%s' % group_id) + self.db.delete('group_name-%s' % group['name']) group_list = set(self.db.get('group_list', [])) group_list.remove(group_id) self.db.set('group_list', list(group_list)) diff --git a/keystone/identity/backends/sql.py b/keystone/identity/backends/sql.py index bda2bf932c..100c8902b9 100644 --- a/keystone/identity/backends/sql.py +++ b/keystone/identity/backends/sql.py @@ -42,23 +42,29 @@ class User(sql.ModelBase, sql.DictBase): __tablename__ = 'user' attributes = ['id', 'name', 'domain_id', 'password', 'enabled'] id = sql.Column(sql.String(64), primary_key=True) - name = sql.Column(sql.String(64), unique=True, nullable=False) + name = sql.Column(sql.String(64), nullable=False) domain_id = sql.Column(sql.String(64), sql.ForeignKey('domain.id'), nullable=False) password = sql.Column(sql.String(128)) enabled = sql.Column(sql.Boolean) extra = sql.Column(sql.JsonBlob()) + # Unique constraint across two columns to create the separation + # rather than just only 'name' being unique + __table_args__ = (sql.UniqueConstraint('domain_id', 'name'), {}) class Group(sql.ModelBase, sql.DictBase): __tablename__ = 'group' attributes = ['id', 'name', 'domain_id'] id = sql.Column(sql.String(64), primary_key=True) - name = sql.Column(sql.String(64), unique=True, nullable=False) + name = sql.Column(sql.String(64), nullable=False) domain_id = sql.Column(sql.String(64), sql.ForeignKey('domain.id'), nullable=False) description = sql.Column(sql.Text()) extra = sql.Column(sql.JsonBlob()) + # Unique constraint across two columns to create the separation + # rather than just only 'name' being unique + __table_args__ = (sql.UniqueConstraint('domain_id', 'name'), {}) class Credential(sql.ModelBase, sql.DictBase): @@ -87,12 +93,15 @@ class Project(sql.ModelBase, sql.DictBase): __tablename__ = 'project' attributes = ['id', 'name', 'domain_id'] id = sql.Column(sql.String(64), primary_key=True) - name = sql.Column(sql.String(64), unique=True, nullable=False) + name = sql.Column(sql.String(64), nullable=False) domain_id = sql.Column(sql.String(64), sql.ForeignKey('domain.id'), nullable=False) description = sql.Column(sql.Text()) enabled = sql.Column(sql.Boolean) extra = sql.Column(sql.JsonBlob()) + # Unique constraint across two columns to create the separation + # rather than just only 'name' being unique + __table_args__ = (sql.UniqueConstraint('domain_id', 'name'), {}) class Role(sql.ModelBase, sql.DictBase): @@ -451,14 +460,15 @@ class Identity(sql.Base, identity.Driver): tenant_ref = session.query(Project).filter_by(id=tenant_id).one() except sql.NotFound: raise exception.ProjectNotFound(project_id=tenant_id) - # FIXME(henry-nash) Think about how we detect potential name clash - # when we move domains + with session.begin(): old_project_dict = tenant_ref.to_dict() for k in tenant: old_project_dict[k] = tenant[k] new_project = Project.from_dict(old_project_dict) - tenant_ref.name = new_project.name + for attr in Project.attributes: + if attr != 'id': + setattr(tenant_ref, attr, getattr(new_project, attr)) tenant_ref.extra = new_project.extra session.flush() return tenant_ref.to_dict(include_extra_dict=True) @@ -675,8 +685,7 @@ class Identity(sql.Base, identity.Driver): session = self.get_session() if 'id' in user and user_id != user['id']: raise exception.ValidationError('Cannot change user ID') - # FIXME(henry-nash) Think about how we detect potential name clash - # when we move domains + with session.begin(): user_ref = session.query(User).filter_by(id=user_id).first() if user_ref is None: @@ -806,8 +815,7 @@ class Identity(sql.Base, identity.Driver): @handle_conflicts(type='group') def update_group(self, group_id, group): session = self.get_session() - # FIXME(henry-nash) Think about how we detect potential name clash - # when we move domains + with session.begin(): ref = session.query(Group).filter_by(id=group_id).first() if ref is None: diff --git a/tests/test_backend.py b/tests/test_backend.py index 12e99b6c7e..09bc0b8adf 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -285,6 +285,59 @@ class IdentityTests(object): 'fake2', user) + def test_create_duplicate_user_name_in_different_domains(self): + new_domain = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex} + self.identity_api.create_domain(new_domain['id'], new_domain) + user1 = {'id': uuid.uuid4().hex, + 'name': uuid.uuid4().hex, + 'domain_id': DEFAULT_DOMAIN_ID, + 'password': uuid.uuid4().hex} + user2 = {'id': uuid.uuid4().hex, + 'name': user1['name'], + 'domain_id': new_domain['id'], + 'password': uuid.uuid4().hex} + self.identity_api.create_user(user1['id'], user1) + self.identity_api.create_user(user2['id'], user2) + + def test_move_user_between_domains(self): + domain1 = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex} + self.identity_api.create_domain(domain1['id'], domain1) + domain2 = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex} + self.identity_api.create_domain(domain2['id'], domain2) + user = {'id': uuid.uuid4().hex, + 'name': uuid.uuid4().hex, + 'domain_id': domain1['id'], + 'password': uuid.uuid4().hex} + self.identity_api.create_user(user['id'], user) + user['domain_id'] = domain2['id'] + self.identity_api.update_user(user['id'], user) + + def test_move_user_between_domains_with_clashing_names_fails(self): + domain1 = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex} + self.identity_api.create_domain(domain1['id'], domain1) + domain2 = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex} + self.identity_api.create_domain(domain2['id'], domain2) + # First, create a user in domain1 + user1 = {'id': uuid.uuid4().hex, + 'name': uuid.uuid4().hex, + 'domain_id': domain1['id'], + 'password': uuid.uuid4().hex} + self.identity_api.create_user(user1['id'], user1) + # Now create a user in domain2 with a potentially clashing + # name - which should work since we have domain separation + user2 = {'id': uuid.uuid4().hex, + 'name': user1['name'], + 'domain_id': domain2['id'], + 'password': uuid.uuid4().hex} + self.identity_api.create_user(user2['id'], user2) + # Now try and move user1 into the 2nd domain - which should + # fail since the names clash + user1['domain_id'] = domain2['id'] + self.assertRaises(exception.Conflict, + self.identity_api.update_user, + user1['id'], + user1) + def test_rename_duplicate_user_name_fails(self): user1 = {'id': 'fake1', 'name': 'fake1', @@ -342,6 +395,52 @@ class IdentityTests(object): 'fake1', tenant) + def test_create_duplicate_project_name_in_different_domains(self): + new_domain = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex} + self.identity_api.create_domain(new_domain['id'], new_domain) + tenant1 = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex, + 'domain_id': DEFAULT_DOMAIN_ID} + tenant2 = {'id': uuid.uuid4().hex, 'name': tenant1['name'], + 'domain_id': new_domain['id']} + self.identity_api.create_project(tenant1['id'], tenant1) + self.identity_api.create_project(tenant2['id'], tenant2) + + def test_move_project_between_domains(self): + domain1 = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex} + self.identity_api.create_domain(domain1['id'], domain1) + domain2 = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex} + self.identity_api.create_domain(domain2['id'], domain2) + project = {'id': uuid.uuid4().hex, + 'name': uuid.uuid4().hex, + 'domain_id': domain1['id']} + self.identity_api.create_project(project['id'], project) + project['domain_id'] = domain2['id'] + self.identity_api.update_project(project['id'], project) + + def test_move_project_between_domains_with_clashing_names_fails(self): + domain1 = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex} + self.identity_api.create_domain(domain1['id'], domain1) + domain2 = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex} + self.identity_api.create_domain(domain2['id'], domain2) + # First, create a project in domain1 + project1 = {'id': uuid.uuid4().hex, + 'name': uuid.uuid4().hex, + 'domain_id': domain1['id']} + self.identity_api.create_project(project1['id'], project1) + # Now create a project in domain2 with a potentially clashing + # name - which should work since we have domain separation + project2 = {'id': uuid.uuid4().hex, + 'name': project1['name'], + 'domain_id': domain2['id']} + self.identity_api.create_project(project2['id'], project2) + # Now try and move project1 into the 2nd domain - which should + # fail since the names clash + project1['domain_id'] = domain2['id'] + self.assertRaises(exception.Conflict, + self.identity_api.update_project, + project1['id'], + project1) + def test_rename_duplicate_project_name_fails(self): tenant1 = {'id': 'fake1', 'name': 'fake1', 'domain_id': DEFAULT_DOMAIN_ID} @@ -1639,6 +1738,62 @@ class IdentityTests(object): self.identity_api.get_group, group['id']) + def test_create_duplicate_group_name_fails(self): + group1 = {'id': uuid.uuid4().hex, 'domain_id': DEFAULT_DOMAIN_ID, + 'name': uuid.uuid4().hex} + group2 = {'id': uuid.uuid4().hex, 'domain_id': DEFAULT_DOMAIN_ID, + 'name': group1['name']} + self.identity_api.create_group(group1['id'], group1) + self.assertRaises(exception.Conflict, + self.identity_api.create_group, + group2['id'], group2) + + def test_create_duplicate_group_name_in_different_domains(self): + new_domain = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex} + self.identity_api.create_domain(new_domain['id'], new_domain) + group1 = {'id': uuid.uuid4().hex, 'domain_id': DEFAULT_DOMAIN_ID, + 'name': uuid.uuid4().hex} + group2 = {'id': uuid.uuid4().hex, 'domain_id': new_domain['id'], + 'name': group1['name']} + self.identity_api.create_group(group1['id'], group1) + self.identity_api.create_group(group2['id'], group2) + + def test_move_group_between_domains(self): + domain1 = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex} + self.identity_api.create_domain(domain1['id'], domain1) + domain2 = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex} + self.identity_api.create_domain(domain2['id'], domain2) + group = {'id': uuid.uuid4().hex, + 'name': uuid.uuid4().hex, + 'domain_id': domain1['id']} + self.identity_api.create_group(group['id'], group) + group['domain_id'] = domain2['id'] + self.identity_api.update_group(group['id'], group) + + def test_move_group_between_domains_with_clashing_names_fails(self): + domain1 = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex} + self.identity_api.create_domain(domain1['id'], domain1) + domain2 = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex} + self.identity_api.create_domain(domain2['id'], domain2) + # First, create a group in domain1 + group1 = {'id': uuid.uuid4().hex, + 'name': uuid.uuid4().hex, + 'domain_id': domain1['id']} + self.identity_api.create_group(group1['id'], group1) + # Now create a group in domain2 with a potentially clashing + # name - which should work since we have domain separation + group2 = {'id': uuid.uuid4().hex, + 'name': group1['name'], + 'domain_id': domain2['id']} + self.identity_api.create_group(group2['id'], group2) + # Now try and move group1 into the 2nd domain - which should + # fail since the names clash + group1['domain_id'] = domain2['id'] + self.assertRaises(exception.Conflict, + self.identity_api.update_group, + group1['id'], + group1) + def test_project_crud(self): project = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex, 'domain_id': uuid.uuid4().hex} diff --git a/tests/test_backend_kvs.py b/tests/test_backend_kvs.py index 9a733ae2b3..e1d99d4707 100644 --- a/tests/test_backend_kvs.py +++ b/tests/test_backend_kvs.py @@ -37,6 +37,33 @@ class KvsIdentity(test.TestCase, test_backend.IdentityTests): # NOTE(chungg): not implemented raise nose.exc.SkipTest('Blocked by bug 1119770') + def test_create_duplicate_group_name_in_different_domains(self): + raise nose.exc.SkipTest('Blocked by bug 1119770') + + def test_create_duplicate_user_name_in_different_domains(self): + raise nose.exc.SkipTest('Blocked by bug 1119770') + + def test_create_duplicate_project_name_in_different_domains(self): + raise nose.exc.SkipTest('Blocked by bug 1119770') + + def test_move_user_between_domains(self): + raise nose.exc.SkipTest('Blocked by bug 1119770') + + def test_move_user_between_domains_with_clashing_names_fails(self): + raise nose.exc.SkipTest('Blocked by bug 1119770') + + def test_move_group_between_domains(self): + raise nose.exc.SkipTest('Blocked by bug 1119770') + + def test_move_group_between_domains_with_clashing_names_fails(self): + raise nose.exc.SkipTest('Blocked by bug 1119770') + + def test_move_project_between_domains(self): + raise nose.exc.SkipTest('Blocked by bug 1119770') + + def test_move_project_between_domains_with_clashing_names_fails(self): + raise nose.exc.SkipTest('Blocked by bug 1119770') + class KvsToken(test.TestCase, test_backend.TokenTests): def setUp(self): diff --git a/tests/test_backend_ldap.py b/tests/test_backend_ldap.py index b5ad68b3cb..4cf8d17ee3 100644 --- a/tests/test_backend_ldap.py +++ b/tests/test_backend_ldap.py @@ -438,3 +438,33 @@ class LDAPIdentity(test.TestCase, test_backend.IdentityTests): def test_get_project_users(self): raise nose.exc.SkipTest('Blocked by bug 1101287') + + def test_create_duplicate_user_name_in_different_domains(self): + raise nose.exc.SkipTest('Blocked by bug 1101276') + + def test_create_duplicate_project_name_in_different_domains(self): + raise nose.exc.SkipTest('Blocked by bug 1101276') + + def test_create_duplicate_group_name_fails(self): + raise nose.exc.SkipTest('Blocked by bug 1092187') + + def test_create_duplicate_group_name_in_different_domains(self): + raise nose.exc.SkipTest('Blocked by bug 1101276') + + def test_move_user_between_domains(self): + raise nose.exc.SkipTest('Blocked by bug 1101276') + + def test_move_user_between_domains_with_clashing_names_fails(self): + raise nose.exc.SkipTest('Blocked by bug 1101276') + + def test_move_group_between_domains(self): + raise nose.exc.SkipTest('Blocked by bug 1101276') + + def test_move_group_between_domains_with_clashing_names_fails(self): + raise nose.exc.SkipTest('Blocked by bug 1101276') + + def test_move_project_between_domains(self): + raise nose.exc.SkipTest('Blocked by bug 1101276') + + def test_move_project_between_domains_with_clashing_names_fails(self): + raise nose.exc.SkipTest('Blocked by bug 1101276') diff --git a/tests/test_sql_upgrade.py b/tests/test_sql_upgrade.py index 7dcce7fe84..85ea758040 100644 --- a/tests/test_sql_upgrade.py +++ b/tests/test_sql_upgrade.py @@ -279,6 +279,7 @@ class SqlUpgradeTests(test.TestCase): self.populate_user_table(with_pass_enab=True) self.populate_tenant_table(with_desc_enab=True) self.upgrade(16) + self.assertTableColumns("user", ["id", "name", "extra", "password", "enabled", "domain_id"]) @@ -299,9 +300,12 @@ class SqlUpgradeTests(test.TestCase): self.assertEqual(a_project.description, default_fixtures.TENANTS[1]['description']) self.assertEqual(a_project.domain_id, DEFAULT_DOMAIN_ID) + session.commit() session.close() + self.check_uniqueness_constraints() + def test_downgrade_16_to_14(self): self.upgrade(16) self.populate_user_table(with_pass_enab_domain=True) @@ -452,6 +456,76 @@ class SqlUpgradeTests(test.TestCase): self.downgrade(16) self.assertEquals(0, count_member_roles()) + def check_uniqueness_constraints(self): + # Check uniqueness constraints for User & Project tables are + # correct following schema modification. The Group table's + # schema is never modified, so we don't bother to check that. + domain_table = sqlalchemy.Table('domain', + self.metadata, + autoload=True) + domain1 = {'id': uuid.uuid4().hex, + 'name': uuid.uuid4().hex, + 'enabled': True} + domain2 = {'id': uuid.uuid4().hex, + 'name': uuid.uuid4().hex, + 'enabled': True} + cmd = domain_table.insert().values(domain1) + self.engine.execute(cmd) + cmd = domain_table.insert().values(domain2) + self.engine.execute(cmd) + + # First, the User table. + this_table = sqlalchemy.Table('user', + self.metadata, + autoload=True) + user = {'id': uuid.uuid4().hex, + 'name': uuid.uuid4().hex, + 'domain_id': domain1['id'], + 'password': uuid.uuid4().hex, + 'enabled': True, + 'extra': json.dumps({})} + cmd = this_table.insert().values(user) + self.engine.execute(cmd) + # now insert a user with the same name into a different + # domain - which should work. + user['id'] = uuid.uuid4().hex + user['domain_id'] = domain2['id'] + cmd = this_table.insert().values(user) + self.engine.execute(cmd) + # TODO(henry-nash). For now, as part of clean-up we + # delete one of these users. Although not part of this test, + # unless we do so the downgrade(16->15) that is part of + # teardown with fail due to having two uses with clashing + # name as we try to revert to a single global name space. This + # limitation is raised as Bug #1125046 and the delete + # could be removed depending on how that bug is resolved. + cmd = this_table.delete(id=user['id']) + self.engine.execute(cmd) + + # Now, the Project table. + this_table = sqlalchemy.Table('project', + self.metadata, + autoload=True) + project = {'id': uuid.uuid4().hex, + 'name': uuid.uuid4().hex, + 'domain_id': domain1['id'], + 'description': uuid.uuid4().hex, + 'enabled': True, + 'extra': json.dumps({})} + cmd = this_table.insert().values(project) + self.engine.execute(cmd) + # now insert a project with the same name into a different + # domain - which should work. + project['id'] = uuid.uuid4().hex + project['domain_id'] = domain2['id'] + cmd = this_table.insert().values(project) + self.engine.execute(cmd) + # TODO(henry-nash) For now, we delete one of the projects for + # the same reason as we delete one of the users (Bug #1125046). + # This delete could be removed depending on that bug resolution. + cmd = this_table.delete(id=project['id']) + self.engine.execute(cmd) + def populate_user_table(self, with_pass_enab=False, with_pass_enab_domain=False): # Populate the appropriate fields in the user