openstack
/
keystone
Code Issues Proposed changes

Set the domain for federated users 

This patch updates the domain for federated users to be the domain of
the Identity Provider (IdP).

Closes-Bug: #1642687
Partially-Implements: bp support-federated-attr
Depends-On: If8c8ad39c4c55a2d800bf4432411db59799e84e6
Change-Id: Iccfad6f39dc339ca054bedf3c6882c3701dcf0ec
This commit is contained in:
Ronald De Rose 2017-01-21 21:10:56 +00:00
parent 6e0faa96ed
commit c19f243152
12 changed files with 289 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
keystone/common/sql/contract_repo/versions/015_contract_update_federated_user_domain.py Normal file
View File

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

36
keystone/common/sql/data_migration_repo/versions/015_migrate_update_federated_user_domain.py Normal file
View File

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

69
keystone/common/sql/expand_repo/versions/015_expand_update_federated_user_domain.py Normal file
View File

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

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

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

9
keystone/identity/core.py
View File

@ -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

4
keystone/identity/shadow_backends/base.py
View File

@ -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
"""

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

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

12
keystone/tests/unit/identity/shadow_users/test_backend.py
View File

@ -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"],

2
keystone/tests/unit/identity/shadow_users/test_core.py
View File

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

8
keystone/tests/unit/identity/test_backends.py
View File

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

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

@ -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

16
releasenotes/notes/shadow-users-backend-driver-has-changed-c7ab1c9be152db20.yaml Normal file
View File

@ -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.