From 659529a4ade95ca335684117e1541cc673084f44 Mon Sep 17 00:00:00 2001 From: Steve Martinelli Date: Thu, 19 Mar 2015 22:22:58 -0400 Subject: [PATCH] Add relay_state_prefix to Service Provider To correctly create an ECP assertion, a keystone acting as an identity provider needs to know the relay state's prefix used by the service provider. This is usually 'ss:mem', but is configurable with mod_shib. Co-Authored-By: Rodrigo Duarte Sousa Change-Id: I55fe059df8b73e8c6d1f195a958e73ee0a321015 Partial-Bug: 1426128 bp: ecp-wrapped-saml-assertions --- keystone/common/config.py | 5 +++ keystone/contrib/federation/backends/sql.py | 6 ++- keystone/contrib/federation/controllers.py | 6 ++- .../versions/008_add_relay_state_to_sp.py | 39 +++++++++++++++++ keystone/contrib/federation/schema.py | 3 +- .../tests/unit/test_backend_federation_sql.py | 1 + .../tests/unit/test_sql_migrate_extensions.py | 6 +++ keystone/tests/unit/test_v3_federation.py | 42 ++++++++++++++++++- 8 files changed, 102 insertions(+), 6 deletions(-) create mode 100644 keystone/contrib/federation/migrate_repo/versions/008_add_relay_state_to_sp.py diff --git a/keystone/common/config.py b/keystone/common/config.py index 3d5596d6c2..6b2573f82a 100644 --- a/keystone/common/config.py +++ b/keystone/common/config.py @@ -978,6 +978,11 @@ FILE_OPTIONS = { help='Path to the Identity Provider Metadata file. ' 'This file should be generated with the ' 'keystone-manage saml_idp_metadata command.'), + cfg.StrOpt('relay_state_prefix', + default='ss:mem:', + help='The prefix to use for the RelayState SAML ' + 'attribute, used when generating ECP wrapped ' + 'assertions.'), ], 'eventlet_server': [ cfg.IntOpt('public_workers', diff --git a/keystone/contrib/federation/backends/sql.py b/keystone/contrib/federation/backends/sql.py index e5c60918c1..324cd1d581 100644 --- a/keystone/contrib/federation/backends/sql.py +++ b/keystone/contrib/federation/backends/sql.py @@ -126,15 +126,17 @@ class MappingModel(sql.ModelBase, sql.DictBase): class ServiceProviderModel(sql.ModelBase, sql.DictBase): __tablename__ = 'service_provider' - attributes = ['auth_url', 'id', 'enabled', 'description', 'sp_url'] + attributes = ['auth_url', 'id', 'enabled', 'description', + 'relay_state_prefix', 'sp_url'] mutable_attributes = frozenset(['auth_url', 'description', 'enabled', - 'sp_url']) + 'relay_state_prefix', 'sp_url']) id = sql.Column(sql.String(64), primary_key=True) enabled = sql.Column(sql.Boolean, nullable=False) description = sql.Column(sql.Text(), nullable=True) auth_url = sql.Column(sql.String(256), nullable=False) sp_url = sql.Column(sql.String(256), nullable=False) + relay_state_prefix = sql.Column(sql.String(256), nullable=False) @classmethod def from_dict(cls, dictionary): diff --git a/keystone/contrib/federation/controllers.py b/keystone/contrib/federation/controllers.py index f63d707da5..cc6a1ec32c 100644 --- a/keystone/contrib/federation/controllers.py +++ b/keystone/contrib/federation/controllers.py @@ -406,15 +406,17 @@ class ServiceProvider(_ControllerBase): member_name = 'service_provider' _mutable_parameters = frozenset(['auth_url', 'description', 'enabled', - 'sp_url']) + 'relay_state_prefix', 'sp_url']) _public_parameters = frozenset(['auth_url', 'id', 'enabled', 'description', - 'links', 'sp_url']) + 'links', 'relay_state_prefix', 'sp_url']) @controller.protected() @validation.validated(schema.service_provider_create, 'service_provider') def create_service_provider(self, context, sp_id, service_provider): service_provider = self._normalize_dict(service_provider) service_provider.setdefault('enabled', False) + service_provider.setdefault('relay_state_prefix', + CONF.saml.relay_state_prefix) ServiceProvider.check_immutable_params(service_provider) sp_ref = self.federation_api.create_sp(sp_id, service_provider) response = ServiceProvider.wrap_member(context, sp_ref) diff --git a/keystone/contrib/federation/migrate_repo/versions/008_add_relay_state_to_sp.py b/keystone/contrib/federation/migrate_repo/versions/008_add_relay_state_to_sp.py new file mode 100644 index 0000000000..150dcfedb6 --- /dev/null +++ b/keystone/contrib/federation/migrate_repo/versions/008_add_relay_state_to_sp.py @@ -0,0 +1,39 @@ +# 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. + +from oslo_config import cfg +from oslo_db.sqlalchemy import utils +import sqlalchemy as sql + + +CONF = cfg.CONF +_SP_TABLE_NAME = 'service_provider' +_RELAY_STATE_PREFIX = 'relay_state_prefix' + + +def upgrade(migrate_engine): + meta = sql.MetaData() + meta.bind = migrate_engine + + idp_table = utils.get_table(migrate_engine, _SP_TABLE_NAME) + relay_state_prefix_default = CONF.saml.relay_state_prefix + relay_state_prefix = sql.Column(_RELAY_STATE_PREFIX, sql.String(256), + nullable=False, + server_default=relay_state_prefix_default) + idp_table.create_column(relay_state_prefix) + + +def downgrade(migrate_engine): + meta = sql.MetaData() + meta.bind = migrate_engine + idp_table = utils.get_table(migrate_engine, _SP_TABLE_NAME) + idp_table.drop_column(_RELAY_STATE_PREFIX) diff --git a/keystone/contrib/federation/schema.py b/keystone/contrib/federation/schema.py index 645e11294c..17818a9893 100644 --- a/keystone/contrib/federation/schema.py +++ b/keystone/contrib/federation/schema.py @@ -58,7 +58,8 @@ _service_provider_properties = { 'auth_url': parameter_types.url, 'sp_url': parameter_types.url, 'description': validation.nullable(parameter_types.description), - 'enabled': parameter_types.boolean + 'enabled': parameter_types.boolean, + 'relay_state_prefix': validation.nullable(parameter_types.description) } service_provider_create = { diff --git a/keystone/tests/unit/test_backend_federation_sql.py b/keystone/tests/unit/test_backend_federation_sql.py index e5c7fb3513..995c564db3 100644 --- a/keystone/tests/unit/test_backend_federation_sql.py +++ b/keystone/tests/unit/test_backend_federation_sql.py @@ -46,5 +46,6 @@ class SqlFederation(test_backend_sql.SqlModels): ('id', sql.String, 64), ('enabled', sql.Boolean, None), ('description', sql.Text, None), + ('relay_state_prefix', sql.String, 256), ('sp_url', sql.String, 256)) self.assertExpectedSchema('service_provider', cols) diff --git a/keystone/tests/unit/test_sql_migrate_extensions.py b/keystone/tests/unit/test_sql_migrate_extensions.py index c70b77ad4e..87b3d48d9e 100644 --- a/keystone/tests/unit/test_sql_migrate_extensions.py +++ b/keystone/tests/unit/test_sql_migrate_extensions.py @@ -310,6 +310,12 @@ class FederationExtension(test_sql_upgrade.SqlMigrateBase): self.assertEqual('', sp.auth_url) self.assertEqual('', sp.sp_url) + def test_add_relay_state_column(self): + self.upgrade(8, repository=self.repo_path) + self.assertTableColumns(self.service_provider, + ['id', 'description', 'enabled', 'auth_url', + 'relay_state_prefix', 'sp_url']) + class RevokeExtension(test_sql_upgrade.SqlMigrateBase): diff --git a/keystone/tests/unit/test_v3_federation.py b/keystone/tests/unit/test_v3_federation.py index 8112443d76..0786880602 100644 --- a/keystone/tests/unit/test_v3_federation.py +++ b/keystone/tests/unit/test_v3_federation.py @@ -2947,6 +2947,7 @@ class SAMLGenerationTests(FederationTests): 'enabled': True, 'description': uuid.uuid4().hex, 'sp_url': self.RECIPIENT, + 'relay_state_prefix': CONF.saml.relay_state_prefix, } return ref @@ -3388,7 +3389,8 @@ class ServiceProviderTests(FederationTests): MEMBER_NAME = 'service_provider' COLLECTION_NAME = 'service_providers' SERVICE_PROVIDER_ID = 'ACME' - SP_KEYS = ['auth_url', 'id', 'enabled', 'description', 'sp_url'] + SP_KEYS = ['auth_url', 'id', 'enabled', 'description', + 'relay_state_prefix', 'sp_url'] def setUp(self): super(FederationTests, self).setUp() @@ -3405,6 +3407,7 @@ class ServiceProviderTests(FederationTests): 'enabled': True, 'description': uuid.uuid4().hex, 'sp_url': 'https://' + uuid.uuid4().hex + '.com', + 'relay_state_prefix': CONF.saml.relay_state_prefix } return ref @@ -3431,6 +3434,29 @@ class ServiceProviderTests(FederationTests): self.assertValidEntity(resp.result['service_provider'], keys_to_check=self.SP_KEYS) + def test_create_sp_relay_state_default(self): + """Create an SP without relay state, should default to `ss:mem`.""" + url = self.base_url(suffix=uuid.uuid4().hex) + sp = self.sp_ref() + del sp['relay_state_prefix'] + resp = self.put(url, body={'service_provider': sp}, + expected_status=201) + sp_result = resp.result['service_provider'] + self.assertEqual(CONF.saml.relay_state_prefix, + sp_result['relay_state_prefix']) + + def test_create_sp_relay_state_non_default(self): + """Create an SP with custom relay state.""" + url = self.base_url(suffix=uuid.uuid4().hex) + sp = self.sp_ref() + non_default_prefix = uuid.uuid4().hex + sp['relay_state_prefix'] = non_default_prefix + resp = self.put(url, body={'service_provider': sp}, + expected_status=201) + sp_result = resp.result['service_provider'] + self.assertEqual(non_default_prefix, + sp_result['relay_state_prefix']) + def test_create_service_provider_fail(self): """Try adding SP object with unallowed attribute.""" url = self.base_url(suffix=uuid.uuid4().hex) @@ -3520,6 +3546,18 @@ class ServiceProviderTests(FederationTests): self.patch(url, body={'service_provider': new_sp_ref}, expected_status=404) + def test_update_sp_relay_state(self): + """Update an SP with custome relay state.""" + new_sp_ref = self.sp_ref() + non_default_prefix = uuid.uuid4().hex + new_sp_ref['relay_state_prefix'] = non_default_prefix + url = self.base_url(suffix=self.SERVICE_PROVIDER_ID) + resp = self.patch(url, body={'service_provider': new_sp_ref}, + expected_status=200) + sp_result = resp.result['service_provider'] + self.assertEqual(non_default_prefix, + sp_result['relay_state_prefix']) + def test_delete_service_provider(self): url = self.base_url(suffix=self.SERVICE_PROVIDER_ID) self.delete(url, expected_status=204) @@ -3641,6 +3679,7 @@ class K2KServiceCatalogTests(FederationTests): def sp_response(self, id, ref): ref.pop('enabled') ref.pop('description') + ref.pop('relay_state_prefix') ref['id'] = id return ref @@ -3650,6 +3689,7 @@ class K2KServiceCatalogTests(FederationTests): 'enabled': True, 'description': uuid.uuid4().hex, 'sp_url': uuid.uuid4().hex, + 'relay_state_prefix': CONF.saml.relay_state_prefix, } return ref