Merge "Set the domain for federated users"
This commit is contained in:
commit
161fbeb294
@ -0,0 +1,34 @@
|
||||
# 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
|
||||
|
||||
|
||||
def upgrade(migrate_engine):
|
||||
meta = sql.MetaData()
|
||||
meta.bind = migrate_engine
|
||||
|
||||
user_table = sql.Table('user', meta, autoload=True)
|
||||
user_table.c.domain_id.alter(nullable=False)
|
||||
|
||||
if upgrades.USE_TRIGGERS:
|
||||
if migrate_engine.name == 'postgresql':
|
||||
drop_trigger_stmt = 'DROP TRIGGER federated_user_insert_trigger '
|
||||
drop_trigger_stmt += 'on federated_user;'
|
||||
elif migrate_engine.name == 'mysql':
|
||||
drop_trigger_stmt = 'DROP TRIGGER federated_user_insert_trigger;'
|
||||
else:
|
||||
drop_trigger_stmt = (
|
||||
'DROP TRIGGER IF EXISTS federated_user_insert_trigger;')
|
||||
migrate_engine.execute(drop_trigger_stmt)
|
@ -0,0 +1,36 @@
|
||||
# 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)
|
||||
federated_table = sql.Table('federated_user', meta, autoload=True)
|
||||
idp_table = sql.Table('identity_provider', meta, autoload=True)
|
||||
|
||||
join = sql.join(federated_table, idp_table,
|
||||
federated_table.c.idp_id == idp_table.c.id)
|
||||
sel = sql.select(
|
||||
[federated_table.c.user_id, idp_table.c.domain_id]).select_from(join)
|
||||
with migrate_engine.begin() as conn:
|
||||
for user in conn.execute(sel):
|
||||
values = {'domain_id': user['domain_id']}
|
||||
stmt = user_table.update().where(
|
||||
sql.and_(
|
||||
user_table.c.domain_id == expression.null(),
|
||||
user_table.c.id == user['user_id'])).values(values)
|
||||
conn.execute(stmt)
|
@ -0,0 +1,69 @@
|
||||
# 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
|
||||
|
||||
|
||||
MYSQL_INSERT_TRIGGER = """
|
||||
CREATE TRIGGER federated_user_insert_trigger
|
||||
AFTER INSERT
|
||||
ON federated_user FOR EACH ROW
|
||||
BEGIN
|
||||
UPDATE user SET domain_id = (
|
||||
SELECT domain_id FROM identity_provider WHERE id = NEW.idp_id)
|
||||
WHERE id = NEW.user_id and domain_id IS NULL;
|
||||
END;
|
||||
"""
|
||||
|
||||
SQLITE_INSERT_TRIGGER = """
|
||||
CREATE TRIGGER federated_user_insert_trigger
|
||||
AFTER INSERT
|
||||
ON federated_user
|
||||
BEGIN
|
||||
UPDATE user SET domain_id = (
|
||||
SELECT domain_id FROM identity_provider WHERE id = NEW.idp_id)
|
||||
WHERE id = NEW.user_id and domain_id IS NULL;
|
||||
END;
|
||||
"""
|
||||
|
||||
POSTGRESQL_INSERT_TRIGGER = """
|
||||
CREATE OR REPLACE FUNCTION update_federated_user_domain_id()
|
||||
RETURNS trigger AS
|
||||
$BODY$
|
||||
BEGIN
|
||||
UPDATE "user" SET domain_id = (
|
||||
SELECT domain_id FROM identity_provider WHERE id = NEW.idp_id)
|
||||
WHERE id = NEW.user_id and domain_id IS NULL;
|
||||
RETURN NULL;
|
||||
END
|
||||
$BODY$ LANGUAGE plpgsql;
|
||||
|
||||
CREATE TRIGGER federated_user_insert_trigger AFTER INSERT ON federated_user
|
||||
FOR EACH ROW
|
||||
EXECUTE PROCEDURE update_federated_user_domain_id();
|
||||
"""
|
||||
|
||||
|
||||
def upgrade(migrate_engine):
|
||||
meta = sql.MetaData()
|
||||
meta.bind = migrate_engine
|
||||
|
||||
if upgrades.USE_TRIGGERS:
|
||||
if migrate_engine.name == 'postgresql':
|
||||
insert_trigger = POSTGRESQL_INSERT_TRIGGER
|
||||
elif migrate_engine.name == 'sqlite':
|
||||
insert_trigger = SQLITE_INSERT_TRIGGER
|
||||
else:
|
||||
insert_trigger = MYSQL_INSERT_TRIGGER
|
||||
migrate_engine.execute(insert_trigger)
|
@ -31,7 +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)
|
||||
domain_id = sql.Column(sql.String(64), nullable=False)
|
||||
_enabled = sql.Column('enabled', sql.Boolean)
|
||||
extra = sql.Column(sql.JsonBlob())
|
||||
default_project_id = sql.Column(sql.String(64))
|
||||
|
@ -436,7 +436,8 @@ def exception_translated(exception_type):
|
||||
@notifications.listener
|
||||
@dependency.provider('identity_api')
|
||||
@dependency.requires('assignment_api', 'credential_api', 'id_mapping_api',
|
||||
'resource_api', 'revoke_api', 'shadow_users_api')
|
||||
'resource_api', 'revoke_api', 'shadow_users_api',
|
||||
'federation_api')
|
||||
class Manager(manager.Manager):
|
||||
"""Default pivot point for the Identity backend.
|
||||
|
||||
@ -1383,14 +1384,16 @@ class Manager(manager.Manager):
|
||||
user_dict = self.shadow_users_api.get_federated_user(
|
||||
idp_id, protocol_id, unique_id)
|
||||
except exception.UserNotFound:
|
||||
idp = self.federation_api.get_idp(idp_id)
|
||||
federated_dict = {
|
||||
'idp_id': idp_id,
|
||||
'protocol_id': protocol_id,
|
||||
'unique_id': unique_id,
|
||||
'display_name': display_name
|
||||
}
|
||||
user_dict = self.shadow_users_api.create_federated_user(
|
||||
federated_dict)
|
||||
user_dict = (
|
||||
self.shadow_users_api.create_federated_user(idp['domain_id'],
|
||||
federated_dict))
|
||||
self.shadow_users_api.set_last_active_at(user_dict['id'])
|
||||
return user_dict
|
||||
|
||||
|
@ -24,11 +24,11 @@ class ShadowUsersDriverBase(object):
|
||||
"""Interface description for an Shadow Users driver."""
|
||||
|
||||
@abc.abstractmethod
|
||||
def create_federated_user(self, federated_dict):
|
||||
def create_federated_user(self, domain_id, federated_dict):
|
||||
"""Create a new user with the federated identity.
|
||||
|
||||
:param domain_id: The domain ID of the IdP used for the federated user
|
||||
:param dict federated_dict: Reference to the federated user
|
||||
:param user_id: user ID for linking to the federated identity
|
||||
:returns dict: Containing the user reference
|
||||
|
||||
"""
|
||||
|
@ -29,9 +29,10 @@ CONF = cfg.CONF
|
||||
|
||||
class ShadowUsers(base.ShadowUsersDriverBase):
|
||||
@sql.handle_conflicts(conflict_type='federated_user')
|
||||
def create_federated_user(self, federated_dict):
|
||||
def create_federated_user(self, domain_id, federated_dict):
|
||||
user = {
|
||||
'id': uuid.uuid4().hex,
|
||||
'domain_id': domain_id,
|
||||
'enabled': True
|
||||
}
|
||||
with sql.session_for_write() as session:
|
||||
|
@ -66,16 +66,22 @@ class ShadowUsersBackendTests(object):
|
||||
|
||||
def test_create_federated_user_unique_constraint(self):
|
||||
user_dict = self.shadow_users_api.create_federated_user(
|
||||
self.federated_user)
|
||||
self.domain_id, self.federated_user)
|
||||
user_dict = self.shadow_users_api.get_user(user_dict["id"])
|
||||
self.assertIsNotNone(user_dict["id"])
|
||||
self.assertRaises(exception.Conflict,
|
||||
self.shadow_users_api.create_federated_user,
|
||||
self.domain_id,
|
||||
self.federated_user)
|
||||
|
||||
def test_create_federated_user_domain(self):
|
||||
user = self.shadow_users_api.create_federated_user(
|
||||
self.domain_id, self.federated_user)
|
||||
self.assertEqual(user['domain_id'], self.domain_id)
|
||||
|
||||
def test_get_federated_user(self):
|
||||
user_dict_create = self.shadow_users_api.create_federated_user(
|
||||
self.federated_user)
|
||||
self.domain_id, self.federated_user)
|
||||
user_dict_get = self.shadow_users_api.get_federated_user(
|
||||
self.federated_user["idp_id"],
|
||||
self.federated_user["protocol_id"],
|
||||
@ -85,7 +91,7 @@ class ShadowUsersBackendTests(object):
|
||||
|
||||
def test_update_federated_user_display_name(self):
|
||||
user_dict_create = self.shadow_users_api.create_federated_user(
|
||||
self.federated_user)
|
||||
self.domain_id, self.federated_user)
|
||||
new_display_name = uuid.uuid4().hex
|
||||
self.shadow_users_api.update_federated_user_display_name(
|
||||
self.federated_user["idp_id"],
|
||||
|
@ -24,7 +24,7 @@ class ShadowUsersCoreTests(object):
|
||||
self.assertEqual(5, len(user.keys()))
|
||||
self.assertIsNotNone(user['name'])
|
||||
self.assertIsNone(user['password_expires_at'])
|
||||
self.assertIsNone(user['domain_id'])
|
||||
self.assertIsNotNone(user['domain_id'])
|
||||
# NOTE(breton): below, attribute `enabled` is explicitly tested to be
|
||||
# equal True. assertTrue should not be used, because it converts
|
||||
# the passed value to bool().
|
||||
|
@ -443,6 +443,7 @@ class IdentityTests(object):
|
||||
return hints
|
||||
|
||||
def _test_list_users_with_attribute(self, filters, fed_dict):
|
||||
domain = self._get_domain_fixture()
|
||||
# Call list_users while no match exists for the federated user
|
||||
hints = driver_hints.Hints()
|
||||
hints = self._build_hints(hints, filters, fed_dict)
|
||||
@ -451,7 +452,7 @@ class IdentityTests(object):
|
||||
|
||||
# list_users with a new relational user and federated user
|
||||
hints = self._build_hints(hints, filters, fed_dict)
|
||||
self.shadow_users_api.create_federated_user(fed_dict)
|
||||
self.shadow_users_api.create_federated_user(domain['id'], fed_dict)
|
||||
users = self.identity_api.list_users(hints=hints)
|
||||
self.assertEqual(1, len(users))
|
||||
|
||||
@ -461,7 +462,7 @@ class IdentityTests(object):
|
||||
fed_dict2 = unit.new_federated_user_ref()
|
||||
fed_dict2['idp_id'] = 'myidp'
|
||||
fed_dict2['protocol_id'] = 'mapped'
|
||||
self.shadow_users_api.create_federated_user(fed_dict2)
|
||||
self.shadow_users_api.create_federated_user(domain['id'], fed_dict2)
|
||||
users = self.identity_api.list_users(hints=hints)
|
||||
self.assertEqual(1, len(users))
|
||||
|
||||
@ -479,7 +480,8 @@ class IdentityTests(object):
|
||||
fed_dict3['idp_id'] = fed_dict['idp_id']
|
||||
elif filters_['name'] == 'protocol_id':
|
||||
fed_dict3['protocol_id'] = fed_dict['protocol_id']
|
||||
self.shadow_users_api.create_federated_user(fed_dict3)
|
||||
self.shadow_users_api.create_federated_user(domain['id'],
|
||||
fed_dict3)
|
||||
users = self.identity_api.list_users(hints=hints)
|
||||
self.assertEqual(2, len(users))
|
||||
|
||||
|
@ -2105,6 +2105,114 @@ class FullMigration(SqlMigrateBase, unit.TestCase):
|
||||
self.assertTrue(self.does_fk_exist('nonlocal_user', 'user_id'))
|
||||
self.assertTrue(self.does_fk_exist('nonlocal_user', 'domain_id'))
|
||||
|
||||
def test_migration_015_update_federated_user_domain(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_idp(domain_id):
|
||||
table = sqlalchemy.Table('identity_provider', self.metadata,
|
||||
autoload=True)
|
||||
idp_id = uuid.uuid4().hex
|
||||
idp = {
|
||||
'id': idp_id,
|
||||
'domain_id': domain_id,
|
||||
'enabled': True,
|
||||
'description': uuid.uuid4().hex
|
||||
}
|
||||
table.insert().values(idp).execute()
|
||||
return idp_id
|
||||
|
||||
def create_protocol(idp_id):
|
||||
table = sqlalchemy.Table('federation_protocol', self.metadata,
|
||||
autoload=True)
|
||||
protocol_id = uuid.uuid4().hex
|
||||
protocol = {
|
||||
'id': protocol_id,
|
||||
'idp_id': idp_id,
|
||||
'mapping_id': uuid.uuid4().hex
|
||||
}
|
||||
table.insert().values(protocol).execute()
|
||||
return protocol_id
|
||||
|
||||
def create_user():
|
||||
table = sqlalchemy.Table('user', self.metadata, autoload=True)
|
||||
user_id = uuid.uuid4().hex
|
||||
user = {'id': user_id, 'enabled': True}
|
||||
table.insert().values(user).execute()
|
||||
return user_id
|
||||
|
||||
def create_federated_user(user_id, idp_id, protocol_id):
|
||||
table = sqlalchemy.Table('federated_user', self.metadata,
|
||||
autoload=True)
|
||||
federated_user = {
|
||||
'user_id': user_id,
|
||||
'idp_id': idp_id,
|
||||
'protocol_id': protocol_id,
|
||||
'unique_id': uuid.uuid4().hex,
|
||||
'display_name': uuid.uuid4().hex
|
||||
}
|
||||
table.insert().values(federated_user).execute()
|
||||
|
||||
def assertUserDomain(user_id, domain_id):
|
||||
table = sqlalchemy.Table('user', self.metadata, autoload=True)
|
||||
where = table.c.id == user_id
|
||||
stmt = sqlalchemy.select([table.c.domain_id]).where(where)
|
||||
domains = stmt.execute().fetchone()
|
||||
self.assertEqual(domain_id, domains[0])
|
||||
|
||||
def assertUserDomainIsNone(user_id):
|
||||
table = sqlalchemy.Table('user', self.metadata, autoload=True)
|
||||
where = table.c.id == user_id
|
||||
stmt = sqlalchemy.select([table.c.domain_id]).where(where)
|
||||
domains = stmt.execute().fetchone()
|
||||
self.assertIsNone(domains[0])
|
||||
|
||||
self.expand(14)
|
||||
self.migrate(14)
|
||||
self.contract(14)
|
||||
|
||||
domain_id = create_domain()
|
||||
idp_id = create_idp(domain_id)
|
||||
protocol_id = create_protocol(idp_id)
|
||||
|
||||
# create user before expand to test data migration
|
||||
user_id_before_expand = create_user()
|
||||
create_federated_user(user_id_before_expand, idp_id, protocol_id)
|
||||
assertUserDomainIsNone(user_id_before_expand)
|
||||
|
||||
self.expand(15)
|
||||
# create user before migrate to test insert trigger
|
||||
user_id_before_migrate = create_user()
|
||||
create_federated_user(user_id_before_migrate, idp_id, protocol_id)
|
||||
assertUserDomain(user_id_before_migrate, domain_id)
|
||||
|
||||
self.migrate(15)
|
||||
# test insert trigger after migrate
|
||||
user_id = create_user()
|
||||
create_federated_user(user_id, idp_id, protocol_id)
|
||||
assertUserDomain(user_id, domain_id)
|
||||
|
||||
self.contract(15)
|
||||
# test migrate updated the user.domain_id
|
||||
assertUserDomain(user_id_before_expand, domain_id)
|
||||
|
||||
# verify that the user.domain_id is now not nullable
|
||||
user_table = sqlalchemy.Table('user', self.metadata, autoload=True)
|
||||
self.assertFalse(user_table.c.domain_id.nullable)
|
||||
|
||||
|
||||
class MySQLOpportunisticFullMigration(FullMigration):
|
||||
FIXTURE = test_base.MySQLOpportunisticFixture
|
||||
|
@ -0,0 +1,16 @@
|
||||
---
|
||||
upgrade:
|
||||
- |
|
||||
The abstract base class for the shadow users backend has changed. We've
|
||||
added a ``domain_id`` parameter to the ``create_federated_user`` method.
|
||||
This is so that the domain ID of the Identity Provider gets set for the
|
||||
federated user. If you have a custom implementation for the shadow users
|
||||
backend, you will need to add the new parameter to your method
|
||||
implementation.
|
||||
fixes:
|
||||
- |
|
||||
[`bug 1642687 <https://bugs.launchpad.net/keystone/+bug/1642687>`_]
|
||||
Prior to this release federated users did not belong to a real domain. Now
|
||||
when federated users are created, as part of shadowing users, federated
|
||||
users will belong to the domain Id of the Identity Provider.
|
||||
|
Loading…
Reference in New Issue
Block a user