openstack/keystone
Code Issues Proposed changes

Add domain_id to the user table

Browse Source 
All users (including federated) should belong to a domain. Currently,
the domain_id is being populated in the local_user and nonlocal_user
(ldap) tables. However, it isn't being set for federated users. This
patch moves the domain_id up to the user table, and creates composite
foreign key (fk) relationships to the local_user and nonlocal_user
tables, as the domain_id is still needed in those tables to enforce
user name uniqueness:

+-----------+
| user      |
| (pk) id   |
| domain_id |
+-----------+
1:1
+----------------+
| local_user     | (and nonlocal_user)
| (pk) id        |
| (fk) user_id   |
| (fk) domain_id |
+----------------+

Likewise, creating a unique constraint on user (id, domain_id) to
support the new composite fk.

This will allow us to set the domain_id in the user table and ensure
that it is in sync with the local_user and nonlocal_user tables, such
that a user belongs to a domain.

Partial-Bug: #1642687
Partially-Implements: bp support-federated-attr
Change-Id: I08a8f3cb59150c8e9a2f90c5ea6b0aa197a03572
This commit is contained in:
Ronald De Rose 2016-12-12 17:49:13 +00:00 committed by Ron De Rose
parent a6adf059f7
commit 2bd88d30e1
7 changed files with 436 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

95
keystone/common/sql/contract_repo/versions/014_contract_add_domain_id_to_user_table.py Normal file
View File

@ -0,0 +1,95 @@
# 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 migrate
import sqlalchemy as sql
from sqlalchemy.engine import reflection
from keystone.common.sql import upgrades
def upgrade(migrate_engine):
meta = sql.MetaData()
meta.bind = migrate_engine
inspector = reflection.Inspector.from_engine(migrate_engine)
user = sql.Table('user', meta, autoload=True)
local_user = sql.Table('local_user', meta, autoload=True)
nonlocal_user = sql.Table('nonlocal_user', meta, autoload=True)
# drop previous fk constraints
fk_name = _get_fk_name(inspector, 'local_user', 'user_id')
if fk_name:
migrate.ForeignKeyConstraint(columns=[local_user.c.user_id],
refcolumns=[user.c.id],
name=fk_name).drop()
fk_name = _get_fk_name(inspector, 'nonlocal_user', 'user_id')
if fk_name:
migrate.ForeignKeyConstraint(columns=[nonlocal_user.c.user_id],
refcolumns=[user.c.id],
name=fk_name).drop()
# create user unique constraint needed for the new composite fk constraint
migrate.UniqueConstraint(user.c.id, user.c.domain_id,
name='ixu_user_id_domain_id').create()
# create new composite fk constraints
migrate.ForeignKeyConstraint(
columns=[local_user.c.user_id, local_user.c.domain_id],
refcolumns=[user.c.id, user.c.domain_id],
onupdate='CASCADE', ondelete='CASCADE').create()
migrate.ForeignKeyConstraint(
columns=[nonlocal_user.c.user_id, nonlocal_user.c.domain_id],
refcolumns=[user.c.id, user.c.domain_id],
onupdate='CASCADE', ondelete='CASCADE').create()
# drop triggers
if upgrades.USE_TRIGGERS:
if migrate_engine.name == 'postgresql':
drop_local_user_insert_trigger = (
'DROP TRIGGER local_user_after_insert_trigger on local_user;')
drop_local_user_update_trigger = (
'DROP TRIGGER local_user_after_update_trigger on local_user;')
drop_nonlocal_user_insert_trigger = (
'DROP TRIGGER nonlocal_user_after_insert_trigger '
'on nonlocal_user;')
drop_nonlocal_user_update_trigger = (
'DROP TRIGGER nonlocal_user_after_update_trigger '
'on nonlocal_user;')
elif migrate_engine.name == 'mysql':
drop_local_user_insert_trigger = (
'DROP TRIGGER local_user_after_insert_trigger;')
drop_local_user_update_trigger = (
'DROP TRIGGER local_user_after_update_trigger;')
drop_nonlocal_user_insert_trigger = (
'DROP TRIGGER nonlocal_user_after_insert_trigger;')
drop_nonlocal_user_update_trigger = (
'DROP TRIGGER nonlocal_user_after_update_trigger;')
else:
drop_local_user_insert_trigger = (
'DROP TRIGGER IF EXISTS local_user_after_insert_trigger;')
drop_local_user_update_trigger = (
'DROP TRIGGER IF EXISTS local_user_after_update_trigger;')
drop_nonlocal_user_insert_trigger = (
'DROP TRIGGER IF EXISTS nonlocal_user_after_insert_trigger;')
drop_nonlocal_user_update_trigger = (
'DROP TRIGGER IF EXISTS nonlocal_user_after_update_trigger;')
migrate_engine.execute(drop_local_user_insert_trigger)
migrate_engine.execute(drop_local_user_update_trigger)
migrate_engine.execute(drop_nonlocal_user_insert_trigger)
migrate_engine.execute(drop_nonlocal_user_update_trigger)
def _get_fk_name(inspector, table, fk_column):
for fk in inspector.get_foreign_keys(table):
if fk_column in fk['constrained_columns']:
return fk['name']

45
keystone/common/sql/data_migration_repo/versions/014_migrate_add_domain_id_to_user_table.py Normal file
View File

@ -0,0 +1,45 @@
# 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 sqlalchemy as sql
import sqlalchemy.sql.expression as expression
def upgrade(migrate_engine):
meta = sql.MetaData()
meta.bind = migrate_engine
user_table = sql.Table('user', meta, autoload=True)
# update user domain_id from local_user
local_table = sql.Table('local_user', meta, autoload=True)
_update_user_domain_id(migrate_engine, user_table, local_table)
# update user domain_id from nonlocal_user
nonlocal_table = sql.Table('nonlocal_user', meta, autoload=True)
_update_user_domain_id(migrate_engine, user_table, nonlocal_table)
def _update_user_domain_id(migrate_engine, user_table, child_user_table):
join = sql.join(user_table, child_user_table,
user_table.c.id == child_user_table.c.user_id)
where = user_table.c.domain_id == expression.null()
sel = (
sql.select([user_table.c.id, child_user_table.c.domain_id])
.select_from(join).where(where)
)
with migrate_engine.begin() as conn:
for user in conn.execute(sel):
values = {'domain_id': user['domain_id']}
stmt = user_table.update().where(
user_table.c.id == user['id']).values(values)
conn.execute(stmt)

165
keystone/common/sql/expand_repo/versions/014_expand_add_domain_id_to_user_table.py Normal file
View File

@ -0,0 +1,165 @@
# 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 sqlalchemy as sql
from keystone.common.sql import upgrades
# define the local_user triggers for insert and update
MYSQL_LOCAL_USER_INSERT_TRIGGER = """
CREATE TRIGGER local_user_after_insert_trigger
AFTER INSERT
ON local_user FOR EACH ROW
BEGIN
UPDATE user SET domain_id = NEW.domain_id
WHERE id = NEW.user_id and domain_id IS NULL;
END;
"""
MYSQL_LOCAL_USER_UPDATE_TRIGGER = """
CREATE TRIGGER local_user_after_update_trigger
AFTER UPDATE
ON local_user FOR EACH ROW
BEGIN
UPDATE user SET domain_id = NEW.domain_id
WHERE id = NEW.user_id and domain_id <> NEW.domain_id;
END;
"""
SQLITE_LOCAL_USER_INSERT_TRIGGER = """
CREATE TRIGGER local_user_after_insert_trigger
AFTER INSERT
ON local_user
BEGIN
UPDATE user SET domain_id = NEW.domain_id
WHERE id = NEW.user_id and domain_id IS NULL;
END;
"""
SQLITE_LOCAL_USER_UPDATE_TRIGGER = """
CREATE TRIGGER local_user_after_update_trigger
AFTER UPDATE
ON local_user
BEGIN
UPDATE user SET domain_id = NEW.domain_id
WHERE id = NEW.user_id and domain_id <> NEW.domain_id;
END;
"""
POSTGRESQL_LOCAL_USER_INSERT_TRIGGER = """
CREATE OR REPLACE FUNCTION update_user_domain_id()
RETURNS trigger AS
$BODY$
BEGIN
UPDATE "user" SET domain_id = NEW.domain_id
WHERE id = NEW.user_id;
RETURN NULL;
END
$BODY$ LANGUAGE plpgsql;
CREATE TRIGGER local_user_after_insert_trigger AFTER INSERT ON local_user
FOR EACH ROW
EXECUTE PROCEDURE update_user_domain_id();
"""
POSTGRESQL_LOCAL_USER_UPDATE_TRIGGER = """
CREATE TRIGGER local_user_after_update_trigger AFTER UPDATE ON local_user
FOR EACH ROW
EXECUTE PROCEDURE update_user_domain_id();
"""
MYSQL_NONLOCAL_USER_INSERT_TRIGGER = """
CREATE TRIGGER nonlocal_user_after_insert_trigger
AFTER INSERT
ON nonlocal_user FOR EACH ROW
BEGIN
UPDATE user SET domain_id = NEW.domain_id
WHERE id = NEW.user_id and domain_id IS NULL;
END;
"""
# define the nonlocal_user triggers for insert and update
MYSQL_NONLOCAL_USER_UPDATE_TRIGGER = """
CREATE TRIGGER nonlocal_user_after_update_trigger
AFTER UPDATE
ON nonlocal_user FOR EACH ROW
BEGIN
UPDATE user SET domain_id = NEW.domain_id
WHERE id = NEW.user_id and domain_id <> NEW.domain_id;
END;
"""
SQLITE_NONLOCAL_USER_INSERT_TRIGGER = """
CREATE TRIGGER nonlocal_user_after_insert_trigger
AFTER INSERT
ON nonlocal_user
BEGIN
UPDATE user SET domain_id = NEW.domain_id
WHERE id = NEW.user_id and domain_id IS NULL;
END;
"""
SQLITE_NONLOCAL_USER_UPDATE_TRIGGER = """
CREATE TRIGGER nonlocal_user_after_update_trigger
AFTER UPDATE
ON nonlocal_user
BEGIN
UPDATE user SET domain_id = NEW.domain_id
WHERE id = NEW.user_id and domain_id <> NEW.domain_id;
END;
"""
POSTGRESQL_NONLOCAL_USER_INSERT_TRIGGER = """
CREATE TRIGGER nonlocal_user_after_insert_trigger AFTER INSERT ON nonlocal_user
FOR EACH ROW
EXECUTE PROCEDURE update_user_domain_id();
"""
POSTGRESQL_NONLOCAL_USER_UPDATE_TRIGGER = """
CREATE TRIGGER nonlocal_user_after_update_trigger AFTER UPDATE ON nonlocal_user
FOR EACH ROW
EXECUTE PROCEDURE update_user_domain_id();
"""
def upgrade(migrate_engine):
meta = sql.MetaData()
meta.bind = migrate_engine
user = sql.Table('user', meta, autoload=True)
project = sql.Table('project', meta, autoload=True)
domain_id = sql.Column('domain_id', sql.String(64),
sql.ForeignKey(project.c.id), nullable=True)
user.create_column(domain_id)
if upgrades.USE_TRIGGERS:
if migrate_engine.name == 'postgresql':
local_user_insert_trigger = POSTGRESQL_LOCAL_USER_INSERT_TRIGGER
local_user_update_trigger = POSTGRESQL_LOCAL_USER_UPDATE_TRIGGER
nonlocal_user_insert_trigger = (
POSTGRESQL_NONLOCAL_USER_INSERT_TRIGGER)
nonlocal_user_update_trigger = (
POSTGRESQL_NONLOCAL_USER_UPDATE_TRIGGER)
elif migrate_engine.name == 'sqlite':
local_user_insert_trigger = SQLITE_LOCAL_USER_INSERT_TRIGGER
local_user_update_trigger = SQLITE_LOCAL_USER_UPDATE_TRIGGER
nonlocal_user_insert_trigger = SQLITE_NONLOCAL_USER_INSERT_TRIGGER
nonlocal_user_update_trigger = SQLITE_NONLOCAL_USER_UPDATE_TRIGGER
else:
local_user_insert_trigger = MYSQL_LOCAL_USER_INSERT_TRIGGER
local_user_update_trigger = MYSQL_LOCAL_USER_UPDATE_TRIGGER
nonlocal_user_insert_trigger = MYSQL_NONLOCAL_USER_INSERT_TRIGGER
nonlocal_user_update_trigger = MYSQL_NONLOCAL_USER_UPDATE_TRIGGER
migrate_engine.execute(local_user_insert_trigger)
migrate_engine.execute(local_user_update_trigger)
migrate_engine.execute(nonlocal_user_insert_trigger)
migrate_engine.execute(nonlocal_user_update_trigger)

45
keystone/identity/backends/sql_model.py
View File

@ -31,6 +31,7 @@ class User(sql.ModelBase, sql.DictBase):
'default_project_id', 'password_expires_at']
readonly_attributes = ['id', 'password_expires_at']
id = sql.Column(sql.String(64), primary_key=True)
domain_id = sql.Column(sql.String(64), nullable=True)
_enabled = sql.Column('enabled', sql.Boolean)
extra = sql.Column(sql.JsonBlob())
default_project_id = sql.Column(sql.String(64))
@ -50,6 +51,8 @@ class User(sql.ModelBase, sql.DictBase):
backref='user')
created_at = sql.Column(sql.DateTime, nullable=True)
last_active_at = sql.Column(sql.Date, nullable=True)
# unique constraint needed here to support composite fk constraints
__table_args__ = (sql.UniqueConstraint('id', 'domain_id'), {})
# NOTE(stevemar): we use a hybrid property here because we leverage the
# expression method, see `@name.expression` and `LocalUser.name` below.
@ -145,29 +148,6 @@ class User(sql.ModelBase, sql.DictBase):
def password(cls):
return Password.password
# NOTE(stevemar): we use a hybrid property here because we leverage the
# expression method, see `@domain_id.expression` and `LocalUser.domain_id`
# below.
@hybrid_property
def domain_id(self):
"""Return user's domain id."""
if self.local_user:
return self.local_user.domain_id
elif self.nonlocal_user:
return self.nonlocal_user.domain_id
else:
return None
@domain_id.setter
def domain_id(self, value):
if not self.local_user:
self.local_user = LocalUser()
self.local_user.domain_id = value
@domain_id.expression
def domain_id(cls):
return LocalUser.domain_id
# NOTE(stevemar): we use a hybrid property here because we leverage the
# expression method, see `@enabled.expression` and `User._enabled` below.
@hybrid_property
@ -229,8 +209,7 @@ class LocalUser(sql.ModelBase, sql.DictBase):
__tablename__ = 'local_user'
attributes = ['id', 'user_id', 'domain_id', 'name']
id = sql.Column(sql.Integer, primary_key=True)
user_id = sql.Column(sql.String(64), sql.ForeignKey('user.id',
ondelete='CASCADE'), unique=True)
user_id = sql.Column(sql.String(64))
domain_id = sql.Column(sql.String(64), nullable=False)
name = sql.Column(sql.String(255), nullable=False)
passwords = orm.relationship('Password',
@ -241,7 +220,13 @@ class LocalUser(sql.ModelBase, sql.DictBase):
order_by='Password.created_at')
failed_auth_count = sql.Column(sql.Integer, nullable=True)
failed_auth_at = sql.Column(sql.DateTime, nullable=True)
__table_args__ = (sql.UniqueConstraint('domain_id', 'name'), {})
__table_args__ = (
sql.UniqueConstraint('user_id'),
sql.UniqueConstraint('domain_id', 'name'),
sqlalchemy.ForeignKeyConstraint(['user_id', 'domain_id'],
['user.id', 'user.domain_id'],
onupdate='CASCADE', ondelete='CASCADE')
)
class Password(sql.ModelBase, sql.DictBase):
@ -287,8 +272,12 @@ class NonLocalUser(sql.ModelBase, sql.ModelDictMixin):
attributes = ['domain_id', 'name', 'user_id']
domain_id = sql.Column(sql.String(64), primary_key=True)
name = sql.Column(sql.String(255), primary_key=True)
user_id = sql.Column(sql.String(64), sql.ForeignKey('user.id',
ondelete='CASCADE'), unique=True)
user_id = sql.Column(sql.String(64))
__table_args__ = (
sql.UniqueConstraint('user_id'),
sqlalchemy.ForeignKeyConstraint(
['user_id', 'domain_id'], ['user.id', 'user.domain_id'],
onupdate='CASCADE', ondelete='CASCADE'),)
class Group(sql.ModelBase, sql.DictBase):

3
keystone/identity/shadow_backends/sql.py
View File

@ -129,13 +129,12 @@ class ShadowUsers(base.ShadowUsersDriverBase):
def create_nonlocal_user(self, user_dict):
new_user_dict = copy.deepcopy(user_dict)
# remove local_user attributes from new_user_dict
keys_to_delete = ['domain_id', 'name', 'password']
keys_to_delete = ['name', 'password']
for key in keys_to_delete:
if key in new_user_dict:
del new_user_dict[key]
# create nonlocal_user dict
new_nonlocal_user_dict = {
'domain_id': user_dict['domain_id'],
'name': user_dict['name']
}
with sql.session_for_write() as session:

1
keystone/tests/unit/test_backend_sql.py
View File

@ -133,6 +133,7 @@ class SqlModels(SqlTests):
def test_user_model(self):
cols = (('id', sql.String, 64),
('domain_id', sql.String, 64),
('default_project_id', sql.String, 64),
('enabled', sql.Boolean, None),
('extra', sql.JsonBlob, None),

112
keystone/tests/unit/test_sql_upgrade.py
View File

@ -1993,6 +1993,118 @@ class FullMigration(SqlMigrateBase, unit.TestCase):
protocol_id=federated_user['protocol_id']).all()
self.assertThat(federated_users, matchers.HasLength(0))
def test_migration_014_add_domain_id_to_user_table(self):
def create_domain():
table = sqlalchemy.Table('project', self.metadata, autoload=True)
domain_id = uuid.uuid4().hex
domain = {
'id': domain_id,
'name': domain_id,
'enabled': True,
'description': uuid.uuid4().hex,
'domain_id': resource_base.NULL_DOMAIN_ID,
'is_domain': True,
'parent_id': None,
'extra': '{}'
}
table.insert().values(domain).execute()
return domain_id
def create_user(table):
user_id = uuid.uuid4().hex
user = {'id': user_id, 'enabled': True}
table.insert().values(user).execute()
return user_id
# insert local_user or nonlocal_user
def create_child_user(table, user_id, domain_id):
child_user = {
'user_id': user_id,
'domain_id': domain_id,
'name': uuid.uuid4().hex
}
table.insert().values(child_user).execute()
# update local_user or nonlocal_user
def update_child_user(table, user_id, new_domain_id):
table.update().where(table.c.user_id == user_id).values(
domain_id=new_domain_id).execute()
def assertUserDomain(user_id, domain_id):
user = sqlalchemy.Table('user', self.metadata, autoload=True)
cols = [user.c.domain_id]
filter = user.c.id == user_id
sel = sqlalchemy.select(cols).where(filter)
domains = sel.execute().fetchone()
self.assertEqual(domain_id, domains[0])
user_table_name = 'user'
self.expand(13)
self.migrate(13)
self.contract(13)
self.assertTableColumns(
user_table_name, ['id', 'extra', 'enabled', 'default_project_id',
'created_at', 'last_active_at'])
self.expand(14)
self.assertTableColumns(
user_table_name, ['id', 'extra', 'enabled', 'default_project_id',
'created_at', 'last_active_at', 'domain_id'])
user_table = sqlalchemy.Table(user_table_name, self.metadata,
autoload=True)
local_user_table = sqlalchemy.Table('local_user', self.metadata,
autoload=True)
nonlocal_user_table = sqlalchemy.Table('nonlocal_user', self.metadata,
autoload=True)
# add users before migrate to test that the user.domain_id gets updated
# after migrate
user_ids = []
expected_domain_id = create_domain()
user_id = create_user(user_table)
create_child_user(local_user_table, user_id, expected_domain_id)
user_ids.append(user_id)
user_id = create_user(user_table)
create_child_user(nonlocal_user_table, user_id, expected_domain_id)
user_ids.append(user_id)
self.migrate(14)
# test local_user insert trigger updates user.domain_id
user_id = create_user(user_table)
domain_id = create_domain()
create_child_user(local_user_table, user_id, domain_id)
assertUserDomain(user_id, domain_id)
# test local_user update trigger updates user.domain_id
new_domain_id = create_domain()
update_child_user(local_user_table, user_id, new_domain_id)
assertUserDomain(user_id, new_domain_id)
# test nonlocal_user insert trigger updates user.domain_id
user_id = create_user(user_table)
create_child_user(nonlocal_user_table, user_id, domain_id)
assertUserDomain(user_id, domain_id)
# test nonlocal_user update trigger updates user.domain_id
update_child_user(nonlocal_user_table, user_id, new_domain_id)
assertUserDomain(user_id, new_domain_id)
self.contract(14)
# test migrate updated the user.domain_id
for user_id in user_ids:
assertUserDomain(user_id, expected_domain_id)
# test unique and fk constraints
if self.engine.name == 'mysql':
self.assertTrue(
self.does_index_exist('user', 'ixu_user_id_domain_id'))
else:
self.assertTrue(
self.does_constraint_exist('user', 'ixu_user_id_domain_id'))
self.assertTrue(self.does_fk_exist('local_user', 'user_id'))
self.assertTrue(self.does_fk_exist('local_user', 'domain_id'))
self.assertTrue(self.does_fk_exist('nonlocal_user', 'user_id'))
self.assertTrue(self.does_fk_exist('nonlocal_user', 'domain_id'))
class MySQLOpportunisticFullMigration(FullMigration):
FIXTURE = test_base.MySQLOpportunisticFixture