From de8fbcf9a0072c84adf4f3630088bc34f9e9782e Mon Sep 17 00:00:00 2001 From: Ronald De Rose Date: Mon, 29 Aug 2016 20:13:35 +0000 Subject: [PATCH] Validate mapping exists when creating/updating a protocol This patch validates that a mapping exists when adding or updating a federation protocol. Change-Id: I996f94d26eb0f2c679542ba13a03bbaa4442486a Closes-Bug: #1571878 --- .../federation/identity-provider/idp.inc | 2 + keystone/federation/core.py | 17 ++++ keystone/tests/unit/federation/__init__.py | 0 keystone/tests/unit/federation/test_core.py | 92 +++++++++++++++++++ keystone/tests/unit/test_v3_federation.py | 12 ++- .../identity/v3/test_identity_providers.py | 20 ++-- ...-federation-protocol-1bcaea5337905af0.yaml | 7 ++ 7 files changed, 142 insertions(+), 8 deletions(-) create mode 100644 keystone/tests/unit/federation/__init__.py create mode 100644 keystone/tests/unit/federation/test_core.py create mode 100644 releasenotes/notes/validate-mapping-exists-for-federation-protocol-1bcaea5337905af0.yaml diff --git a/api-ref/source/v3-ext/federation/identity-provider/idp.inc b/api-ref/source/v3-ext/federation/identity-provider/idp.inc index 1e1e065ce2..57ffef740b 100644 --- a/api-ref/source/v3-ext/federation/identity-provider/idp.inc +++ b/api-ref/source/v3-ext/federation/identity-provider/idp.inc @@ -182,6 +182,7 @@ Add a protocol and attribute mapping to an identity provider Relationship: ``http://docs.openstack.org/api/openstack-identity/3/ext/OS-FEDERATION/1.0/rel/identity_provider_protocol`` Normal response codes: 201 +Error response codes: 400 Request ------- @@ -283,6 +284,7 @@ Update the attribute mapping for an identity provider and protocol Relationship: ``http://docs.openstack.org/api/openstack-identity/3/ext/OS-FEDERATION/1.0/rel/identity_provider_protocol`` Normal response codes: 200 +Error response codes: 400 Request ------- diff --git a/keystone/federation/core.py b/keystone/federation/core.py index 3a5560f9cb..eb2ea997a3 100644 --- a/keystone/federation/core.py +++ b/keystone/federation/core.py @@ -17,7 +17,9 @@ from keystone.common import dependency from keystone.common import extension from keystone.common import manager import keystone.conf +from keystone import exception from keystone.federation import utils +from keystone.i18n import _ # This is a general cache region for service providers. @@ -101,3 +103,18 @@ class Manager(manager.Manager): rule_processor = utils.RuleProcessor(mapping['id'], rules) mapped_properties = rule_processor.process(assertion_data) return mapped_properties, mapping['id'] + + def create_protocol(self, idp_id, protocol_id, protocol): + self._validate_mapping_exists(protocol['mapping_id']) + return self.driver.create_protocol(idp_id, protocol_id, protocol) + + def update_protocol(self, idp_id, protocol_id, protocol): + self._validate_mapping_exists(protocol['mapping_id']) + return self.driver.update_protocol(idp_id, protocol_id, protocol) + + def _validate_mapping_exists(self, mapping_id): + try: + self.driver.get_mapping(mapping_id) + except exception.MappingNotFound: + msg = _('Invalid mapping id: %s') + raise exception.ValidationError(message=(msg % mapping_id)) diff --git a/keystone/tests/unit/federation/__init__.py b/keystone/tests/unit/federation/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/keystone/tests/unit/federation/test_core.py b/keystone/tests/unit/federation/test_core.py new file mode 100644 index 0000000000..40a3d15046 --- /dev/null +++ b/keystone/tests/unit/federation/test_core.py @@ -0,0 +1,92 @@ +# 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 uuid + +from keystone import exception +from keystone.tests import unit +from keystone.tests.unit.ksfixtures import database +from keystone.tests.unit import mapping_fixtures + + +class TestFederationProtocol(unit.TestCase): + + def setUp(self): + super(TestFederationProtocol, self).setUp() + self.useFixture(database.Database()) + self.load_backends() + self.idp = { + 'id': uuid.uuid4().hex, + 'enabled': True, + 'description': uuid.uuid4().hex + } + self.federation_api.create_idp(self.idp['id'], self.idp) + self.mapping = mapping_fixtures.MAPPING_EPHEMERAL_USER + self.mapping['id'] = uuid.uuid4().hex + self.federation_api.create_mapping(self.mapping['id'], + self.mapping) + + def test_create_protocol(self): + protocol = { + 'id': uuid.uuid4().hex, + 'mapping_id': self.mapping['id'] + } + protocol_ret = self.federation_api.create_protocol(self.idp['id'], + protocol['id'], + protocol) + self.assertEqual(protocol['id'], protocol_ret['id']) + + def test_create_protocol_with_invalid_mapping_id(self): + protocol = { + 'id': uuid.uuid4().hex, + 'mapping_id': uuid.uuid4().hex + } + self.assertRaises(exception.ValidationError, + self.federation_api.create_protocol, + self.idp['id'], + protocol['id'], + protocol) + + def test_update_protocol(self): + protocol = { + 'id': uuid.uuid4().hex, + 'mapping_id': self.mapping['id'] + } + protocol_ret = self.federation_api.create_protocol(self.idp['id'], + protocol['id'], + protocol) + self.assertEqual(protocol['id'], protocol_ret['id']) + new_mapping = mapping_fixtures.MAPPING_EPHEMERAL_USER + new_mapping['id'] = uuid.uuid4().hex + self.federation_api.create_mapping(new_mapping['id'], new_mapping) + protocol['mapping_id'] = new_mapping['id'] + protocol_ret = self.federation_api.update_protocol(self.idp['id'], + protocol['id'], + protocol) + self.assertEqual(protocol['id'], protocol_ret['id']) + self.assertEqual(new_mapping['id'], protocol_ret['mapping_id']) + + def test_update_protocol_with_invalid_mapping_id(self): + protocol = { + 'id': uuid.uuid4().hex, + 'mapping_id': self.mapping['id'] + } + protocol_ret = self.federation_api.create_protocol(self.idp['id'], + protocol['id'], + protocol) + self.assertEqual(protocol['id'], protocol_ret['id']) + protocol['mapping_id'] = uuid.uuid4().hex + self.assertRaises(exception.ValidationError, + self.federation_api.update_protocol, + self.idp['id'], + protocol['id'], + protocol) diff --git a/keystone/tests/unit/test_v3_federation.py b/keystone/tests/unit/test_v3_federation.py index 2ef8a216cd..1a6c3384ba 100644 --- a/keystone/tests/unit/test_v3_federation.py +++ b/keystone/tests/unit/test_v3_federation.py @@ -852,6 +852,7 @@ class FederatedIdentityProviderTests(test_v3.RestfulTestCase): proto = uuid.uuid4().hex if mapping_id is None: mapping_id = uuid.uuid4().hex + self._create_mapping(mapping_id) body = {'mapping_id': mapping_id} url = url % {'idp_id': idp_id, 'protocol_id': proto} resp = self.put(url, body={'protocol': body}, **kwargs) @@ -863,11 +864,19 @@ class FederatedIdentityProviderTests(test_v3.RestfulTestCase): return (resp, idp_id, proto) def _get_protocol(self, idp_id, protocol_id): - url = "%s/protocols/%s" % (idp_id, protocol_id) + url = '%s/protocols/%s' % (idp_id, protocol_id) url = self.base_url(suffix=url) r = self.get(url) return r + def _create_mapping(self, mapping_id): + mapping = mapping_fixtures.MAPPING_EPHEMERAL_USER + mapping['id'] = mapping_id + url = '/OS-FEDERATION/mappings/%s' % mapping_id + self.put(url, + body={'mapping': mapping}, + expected_status=http_client.CREATED) + def test_create_idp(self): """Create the IdentityProvider entity associated to remote_ids.""" keys_to_check = list(self.idp_keys) @@ -1386,6 +1395,7 @@ class FederatedIdentityProviderTests(test_v3.RestfulTestCase): resp, idp_id, proto = self._assign_protocol_to_idp( expected_status=http_client.CREATED) new_mapping_id = uuid.uuid4().hex + self._create_mapping(mapping_id=new_mapping_id) url = "%s/protocols/%s" % (idp_id, proto) url = self.base_url(suffix=url) diff --git a/keystone_tempest_plugin/tests/api/identity/v3/test_identity_providers.py b/keystone_tempest_plugin/tests/api/identity/v3/test_identity_providers.py index ebd00d46d1..619fc65d66 100644 --- a/keystone_tempest_plugin/tests/api/identity/v3/test_identity_providers.py +++ b/keystone_tempest_plugin/tests/api/identity/v3/test_identity_providers.py @@ -14,6 +14,7 @@ from tempest.lib.common.utils import data_utils from tempest.lib import decorators +from tempest.lib import exceptions as lib_exc from keystone_tempest_plugin.tests.api.identity import base from keystone_tempest_plugin.tests.api.identity.v3 import fixtures @@ -217,9 +218,12 @@ class IndentityProvidersTest(base.BaseIdentityTest): # a non existent mapping ID mapping_id = data_utils.rand_uuid_hex() protocol_id = data_utils.rand_uuid_hex() - protocol = self._create_protocol(idp_id, protocol_id, mapping_id) - - self._assert_protocol_attributes(protocol, protocol_id, mapping_id) + self.assertRaises( + lib_exc.BadRequest, + self._create_protocol, + idp_id, + protocol_id, + mapping_id) @decorators.idempotent_id('c73311e7-c207-4c11-998f-532a91f1b0d1') def test_update_protocol_from_identity_provider_unknown_mapping(self): @@ -232,7 +236,9 @@ class IndentityProvidersTest(base.BaseIdentityTest): # Update the identity provider protocol using a non existent # mapping_id new_mapping_id = data_utils.rand_uuid_hex() - protocol = self.idps_client.update_protocol_mapping( - idp_id, protocol_id, new_mapping_id)['protocol'] - - self._assert_protocol_attributes(protocol, protocol_id, new_mapping_id) + self.assertRaises( + lib_exc.BadRequest, + self.idps_client.update_protocol_mapping, + idp_id, + protocol_id, + new_mapping_id) diff --git a/releasenotes/notes/validate-mapping-exists-for-federation-protocol-1bcaea5337905af0.yaml b/releasenotes/notes/validate-mapping-exists-for-federation-protocol-1bcaea5337905af0.yaml new file mode 100644 index 0000000000..15ce8c2378 --- /dev/null +++ b/releasenotes/notes/validate-mapping-exists-for-federation-protocol-1bcaea5337905af0.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - > + [`bug 1571878 `_] + A valid ``mapping_id`` is now required when creating or updating a + federation protocol. If the ``mapping_id`` does not exist, a + ``400 - Bad Request`` will be returned.