diff options
14 files changed, 223 insertions, 27 deletions
diff --git a/api-ref/source/v3-ext/federation.inc b/api-ref/source/v3-ext/federation.inc index 5fa86bb44..bdeb8ddf2 100644 --- a/api-ref/source/v3-ext/federation.inc +++ b/api-ref/source/v3-ext/federation.inc @@ -8,6 +8,13 @@ Provide the ability for users to manage Identity Providers (IdPs) and establish a set of rules to map federation protocol attributes to Identity API attributes. Requires v3.0+ of the Identity API. +What's New in Version 1.4 +========================= + +Corresponding to Identity API v3.12 release. + +- Added `remote_id_attribute` as an attribute of a Protocol. + What's New in Version 1.3 ========================= @@ -117,6 +124,16 @@ Required attributes: Indicates which mapping should be used to process federated authentication requests. +Optional attributes: + +- ``remote_id_attribute`` (string) + + Key to obtain the entity ID of the Identity Provider from the HTTPD + environment. For `mod_shib`, this would be `Shib-Identity-Provider`. For + `mod_auth_openidc`, this could be `HTTP_OIDC_ISS`. For `mod_auth_mellon`, + this could be `MELLON_IDP`. This overrides the default value provided in + keystone.conf. + Mappings -------- diff --git a/keystone/api/auth.py b/keystone/api/auth.py index da63ed9f8..00382d013 100644 --- a/keystone/api/auth.py +++ b/keystone/api/auth.py @@ -334,11 +334,14 @@ class AuthTokenResource(_AuthFederationWebSSOBase): class AuthFederationWebSSOResource(_AuthFederationWebSSOBase): @classmethod def _perform_auth(cls, protocol_id): - try: + idps = PROVIDERS.federation_api.list_idps() + for idp in idps: remote_id_name = federation_utils.get_remote_id_parameter( - protocol_id) - remote_id = flask.request.environ[remote_id_name] - except KeyError: + idp, protocol_id) + remote_id = flask.request.environ.get(remote_id_name) + if remote_id: + break + if not remote_id: msg = 'Missing entity ID from environment' tr_msg = _('Missing entity ID from environment') LOG.error(msg) diff --git a/keystone/common/sql/contract_repo/versions/064_contract_add_remote_id_attribute_to_federation_protocol_table.py b/keystone/common/sql/contract_repo/versions/064_contract_add_remote_id_attribute_to_federation_protocol_table.py new file mode 100644 index 000000000..8aa15c1ef --- /dev/null +++ b/keystone/common/sql/contract_repo/versions/064_contract_add_remote_id_attribute_to_federation_protocol_table.py @@ -0,0 +1,15 @@ +# 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. + + +def upgrade(migrate_engine): + pass diff --git a/keystone/common/sql/data_migration_repo/versions/064_migrate_add_remote_id_attribute_to_federation_protocol_table.py b/keystone/common/sql/data_migration_repo/versions/064_migrate_add_remote_id_attribute_to_federation_protocol_table.py new file mode 100644 index 000000000..8aa15c1ef --- /dev/null +++ b/keystone/common/sql/data_migration_repo/versions/064_migrate_add_remote_id_attribute_to_federation_protocol_table.py @@ -0,0 +1,15 @@ +# 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. + + +def upgrade(migrate_engine): + pass diff --git a/keystone/common/sql/expand_repo/versions/064_expand_add_remote_id_attribute_to_federation_protocol_table.py b/keystone/common/sql/expand_repo/versions/064_expand_add_remote_id_attribute_to_federation_protocol_table.py new file mode 100644 index 000000000..44f20f05c --- /dev/null +++ b/keystone/common/sql/expand_repo/versions/064_expand_add_remote_id_attribute_to_federation_protocol_table.py @@ -0,0 +1,22 @@ +# 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 + + +def upgrade(migrate_engine): + meta = sql.MetaData() + meta.bind = migrate_engine + + federation_protocol_table = sql.Table('federation_protocol', meta, autoload=True) + remote_id_attribute = sql.Column('remote_id_attribute', sql.String(64)) + federation_protocol_table.create_column(remote_id_attribute) diff --git a/keystone/conf/federation.py b/keystone/conf/federation.py index fa3d57df5..ac0838545 100644 --- a/keystone/conf/federation.py +++ b/keystone/conf/federation.py @@ -35,10 +35,12 @@ assertions. Matched variables are passed into the federated mapping engine. remote_id_attribute = cfg.StrOpt( 'remote_id_attribute', help=utils.fmt(""" -Value to be used to obtain the entity ID of the Identity Provider from the -environment. For `mod_shib`, this would be `Shib-Identity-Provider`. For -`mod_auth_openidc`, this could be `HTTP_OIDC_ISS`. For `mod_auth_mellon`, this -could be `MELLON_IDP`. +Default value for all protocols to be used to obtain the entity ID of the +Identity Provider from the environment. For `mod_shib`, this would be +`Shib-Identity-Provider`. For `mod_auth_openidc`, this could be +`HTTP_OIDC_ISS`. For `mod_auth_mellon`, this could be `MELLON_IDP`. This can be +overridden on a per-protocol basis by providing a `remote_id_attribute` to the +federation protocol using the API. """)) federated_domain_name = cfg.StrOpt( diff --git a/keystone/federation/backends/sql.py b/keystone/federation/backends/sql.py index ba26e5560..9451e1a4b 100644 --- a/keystone/federation/backends/sql.py +++ b/keystone/federation/backends/sql.py @@ -28,13 +28,14 @@ LOG = log.getLogger(__name__) class FederationProtocolModel(sql.ModelBase, sql.ModelDictMixin): __tablename__ = 'federation_protocol' - attributes = ['id', 'idp_id', 'mapping_id'] - mutable_attributes = frozenset(['mapping_id']) + attributes = ['id', 'idp_id', 'mapping_id', 'remote_id_attribute'] + mutable_attributes = frozenset(['mapping_id', 'remote_id_attribute']) id = sql.Column(sql.String(64), primary_key=True) idp_id = sql.Column(sql.String(64), sql.ForeignKey('identity_provider.id', ondelete='CASCADE'), primary_key=True) mapping_id = sql.Column(sql.String(64), nullable=False) + remote_id_attribute = sql.Column(sql.String(64)) @classmethod def from_dict(cls, dictionary): diff --git a/keystone/federation/schema.py b/keystone/federation/schema.py index ca2951592..77d4e63ff 100644 --- a/keystone/federation/schema.py +++ b/keystone/federation/schema.py @@ -117,8 +117,14 @@ identity_provider_update = { 'additionalProperties': False } +_remote_id_attribute_properties = { + 'type': 'string', + 'maxLength': 64, +} + _protocol_properties = { - 'mapping_id': parameter_types.mapping_id_string + 'mapping_id': parameter_types.mapping_id_string, + 'remote_id_attribute': _remote_id_attribute_properties } protocol_create = { diff --git a/keystone/federation/utils.py b/keystone/federation/utils.py index 78deeb41b..92028fa7d 100644 --- a/keystone/federation/utils.py +++ b/keystone/federation/utils.py @@ -22,6 +22,7 @@ from oslo_log import log from oslo_utils import timeutils import six +from keystone.common import provider_api import keystone.conf from keystone import exception from keystone.i18n import _ @@ -29,6 +30,7 @@ from keystone.i18n import _ CONF = keystone.conf.CONF LOG = log.getLogger(__name__) +PROVIDERS = provider_api.ProviderAPIs class UserType(object): @@ -277,23 +279,28 @@ def validate_expiration(token): raise exception.Unauthorized(_('Federation token is expired')) -def get_remote_id_parameter(protocol): +def get_remote_id_parameter(idp, protocol): # NOTE(marco-fargetta): Since we support any protocol ID, we attempt to - # retrieve the remote_id_attribute of the protocol ID. If it's not - # registered in the config, then register the option and try again. - # This allows the user to register protocols other than oidc and saml2. - remote_id_parameter = None - try: - remote_id_parameter = CONF[protocol]['remote_id_attribute'] - except AttributeError: - # TODO(dolph): Move configuration registration to keystone.conf - CONF.register_opt(cfg.StrOpt('remote_id_attribute'), - group=protocol) + # retrieve the remote_id_attribute of the protocol ID. It will look up first + # if the remote_id_attribute exists. + protocol_ref = PROVIDERS.federation_api.get_protocol(idp['id'], protocol) + remote_id_parameter = protocol_ref.get('remote_id_attribute') + if remote_id_parameter: + return remote_id_parameter + else: + # If it's not registered in the config, then register the option and try again. + # This allows the user to register protocols other than oidc and saml2. try: remote_id_parameter = CONF[protocol]['remote_id_attribute'] - except AttributeError: # nosec - # No remote ID attr, will be logged and use the default instead. - pass + except AttributeError: + # TODO(dolph): Move configuration registration to keystone.conf + CONF.register_opt(cfg.StrOpt('remote_id_attribute'), + group=protocol) + try: + remote_id_parameter = CONF[protocol]['remote_id_attribute'] + except AttributeError: # nosec + # No remote ID attr, will be logged and use the default instead. + pass if not remote_id_parameter: LOG.debug('Cannot find "remote_id_attribute" in configuration ' 'group %s. Trying default location in ' @@ -305,7 +312,7 @@ def get_remote_id_parameter(protocol): def validate_idp(idp, protocol, assertion): """The IdP providing the assertion should be registered for the mapping.""" - remote_id_parameter = get_remote_id_parameter(protocol) + remote_id_parameter = get_remote_id_parameter(idp, protocol) if not remote_id_parameter or not idp['remote_ids']: LOG.debug('Impossible to identify the IdP %s ', idp['id']) # If nothing is defined, the administrator may want to diff --git a/keystone/tests/unit/federation/test_core.py b/keystone/tests/unit/federation/test_core.py index 48da21ff5..3a65b69c6 100644 --- a/keystone/tests/unit/federation/test_core.py +++ b/keystone/tests/unit/federation/test_core.py @@ -63,6 +63,18 @@ class TestFederationProtocol(unit.TestCase): protocol['id'], protocol) + def test_create_protocol_with_remote_id_attribute(self): + protocol = { + 'id': uuid.uuid4().hex, + 'mapping_id': self.mapping['id'], + 'remote_id_attribute': uuid.uuid4().hex + } + protocol_ret = PROVIDERS.federation_api.create_protocol( + self.idp['id'], protocol['id'], protocol + ) + self.assertEqual(protocol['remote_id_attribute'], + protocol_ret['remote_id_attribute']) + def test_update_protocol(self): protocol = { 'id': uuid.uuid4().hex, @@ -97,3 +109,19 @@ class TestFederationProtocol(unit.TestCase): self.idp['id'], protocol['id'], protocol) + + def test_update_protocol_with_remote_id_attribute(self): + protocol = { + 'id': uuid.uuid4().hex, + 'mapping_id': self.mapping['id'] + } + protocol_ret = PROVIDERS.federation_api.create_protocol( + self.idp['id'], protocol['id'], protocol + ) + new_remote_id_attribute = uuid.uuid4().hex + protocol['remote_id_attribute'] = new_remote_id_attribute + protocol_ret = PROVIDERS.federation_api.update_protocol( + self.idp['id'], protocol['id'], protocol + ) + self.assertEqual(protocol['remote_id_attribute'], + protocol_ret['remote_id_attribute']) diff --git a/keystone/tests/unit/test_backend_federation_sql.py b/keystone/tests/unit/test_backend_federation_sql.py index ca8043b49..d20e076cf 100644 --- a/keystone/tests/unit/test_backend_federation_sql.py +++ b/keystone/tests/unit/test_backend_federation_sql.py @@ -34,7 +34,8 @@ class SqlFederation(test_backend_sql.SqlModels): def test_federated_protocol(self): cols = (('id', sql.String, 64), ('idp_id', sql.String, 64), - ('mapping_id', sql.String, 64)) + ('mapping_id', sql.String, 64), + ('remote_id_attribute', sql.String, 64)) self.assertExpectedSchema('federation_protocol', cols) def test_mapping(self): diff --git a/keystone/tests/unit/test_sql_upgrade.py b/keystone/tests/unit/test_sql_upgrade.py index cfa65cf06..acd884737 100644 --- a/keystone/tests/unit/test_sql_upgrade.py +++ b/keystone/tests/unit/test_sql_upgrade.py @@ -3387,6 +3387,26 @@ class FullMigration(SqlMigrateBase, unit.TestCase): ['id', 'project_id', 'resource_limit', 'description', 'internal_id', 'registered_limit_id', 'domain_id']) + def test_migration_064_add_remote_id_attribute_to_federation_protocol(self): + self.expand(63) + self.migrate(63) + self.contract(63) + + federation_protocol_table_name = 'federation_protocol' + self.assertTableColumns( + federation_protocol_table_name, + ['id', 'idp_id', 'mapping_id'] + ) + + self.expand(64) + self.migrate(64) + self.contract(64) + + self.assertTableColumns( + federation_protocol_table_name, + ['id', 'idp_id', 'mapping_id', 'remote_id_attribute'] + ) + class MySQLOpportunisticFullMigration(FullMigration): FIXTURE = db_fixtures.MySQLOpportunisticFixture diff --git a/keystone/tests/unit/test_v3_federation.py b/keystone/tests/unit/test_v3_federation.py index 3de15d50f..e2134cdd6 100644 --- a/keystone/tests/unit/test_v3_federation.py +++ b/keystone/tests/unit/test_v3_federation.py @@ -2968,6 +2968,27 @@ class FederatedTokenTests(test_v3.RestfulTestCase, FederatedSetupMixin): token['user']['name']) self.assertNotEqual(token['user']['name'], token['user']['id']) + def test_issue_unscoped_token_with_remote_different_from_protocol(self): + protocol = PROVIDERS.federation_api.get_protocol( + self.IDP_WITH_REMOTE, self.PROTOCOL + ) + protocol['remote_id_attribute'] = uuid.uuid4().hex + PROVIDERS.federation_api.update_protocol( + self.IDP_WITH_REMOTE, protocol['id'], protocol + ) + self._issue_unscoped_token( + idp=self.IDP_WITH_REMOTE, + environment={ + protocol['remote_id_attribute']: self.REMOTE_IDS[0] + } + ) + self.assertRaises( + exception.Unauthorized, + self._issue_unscoped_token, + idp=self.IDP_WITH_REMOTE, + environment={uuid.uuid4().hex: self.REMOTE_IDS[0]} + ) + class FernetFederatedTokenTests(test_v3.RestfulTestCase, FederatedSetupMixin): AUTH_METHOD = 'token' @@ -4896,6 +4917,28 @@ class WebSSOTests(FederatedTokenTests): # needs to be encoded self.assertIn(self.TRUSTED_DASHBOARD.encode('utf-8'), resp.data) + def test_issue_unscoped_token_with_remote_from_protocol(self): + self.config_fixture.config( + group='federation', remote_id_attribute=None + ) + self.config_fixture.config( + group=self.PROTOCOL, remote_id_attribute=None + ) + protocol = PROVIDERS.federation_api.get_protocol( + self.IDP_WITH_REMOTE, self.PROTOCOL + ) + protocol['remote_id_attribute'] = self.PROTOCOL_REMOTE_ID_ATTR + PROVIDERS.federation_api.update_protocol( + self.IDP_WITH_REMOTE, protocol['id'], protocol + ) + environment = {self.PROTOCOL_REMOTE_ID_ATTR: self.REMOTE_IDS[0], + 'QUERY_STRING': 'origin=%s' % self.ORIGIN} + environment.update(mapping_fixtures.EMPLOYEE_ASSERTION) + with self.make_request(environ=environment): + resp = auth_api.AuthFederationWebSSOResource._perform_auth( + self.PROTOCOL) + self.assertIn(self.TRUSTED_DASHBOARD.encode('utf-8'), resp.data) + class K2KServiceCatalogTests(test_v3.RestfulTestCase): SP1 = 'SP1' diff --git a/releasenotes/notes/bug-1724645-a94659dfd0f45b9a.yaml b/releasenotes/notes/bug-1724645-a94659dfd0f45b9a.yaml new file mode 100644 index 000000000..726595b61 --- /dev/null +++ b/releasenotes/notes/bug-1724645-a94659dfd0f45b9a.yaml @@ -0,0 +1,16 @@ +--- +features: + - | + [`bug 1724645 <https://bugs.launchpad.net/keystone/+bug/1724645>`_] + Adds a new attribute, ``remote_id_attribute``, to the federation protocol + object, which allows WebSSO authentication to forward authentication + requests through the right implementation for a federated protocol based on + the remote ID attribute in the authentication headers. +fixes: + - | + [`bug 1724645 <https://bugs.launchpad.net/keystone/+bug/1724645>`_] + Fixes an issue where multiple implementations of a federation protocol, + such as Shibboleth and Mellon for the SAML2.0 protocol, could not be + differentiated from one another because they had to share the same globally + configured remote ID attribute. Now the remote ID attribute can be set on + the protocol object itself. |