diff --git a/keystone/api/os_federation.py b/keystone/api/os_federation.py index 83c4ea8613..840e13e059 100644 --- a/keystone/api/os_federation.py +++ b/keystone/api/os_federation.py @@ -154,11 +154,10 @@ class IdentityProvidersResource(_ResourceBase): return None, http_client.NO_CONTENT -class IdentityProvidersProtocolsResource(_ResourceBase): +class _IdentityProvidersProtocolsResourceBase(_ResourceBase): collection_key = 'protocols' member_key = 'protocol' _public_parameters = frozenset(['id', 'mapping_id', 'links']) - api_prefix = '/OS-FEDERATION/identity_providers/' json_home_additional_parameters = { 'idp_id': IDP_ID_PARAMETER_RELATION} json_home_collection_resource_name_override = 'identity_provider_protocols' @@ -178,22 +177,10 @@ class IdentityProvidersProtocolsResource(_ResourceBase): ref['links']['identity_provider'] = ks_flask.base_url( path=ref['idp_id']) - def get(self, idp_id, protocol_id=None): - if protocol_id is not None: - return self._get_protocol(idp_id, protocol_id) - return self._list_protocols(idp_id) - def _get_protocol(self, idp_id, protocol_id): - """Get protocols for an IDP. +class IDPProtocolsListResource(_IdentityProvidersProtocolsResourceBase): - HEAD/GET /OS-FEDERATION/identity_providers/ - {idp_id}/protocols/{protocol_id} - """ - ENFORCER.enforce_call(action='identity:get_protocol') - ref = PROVIDERS.federation_api.get_protocol(idp_id, protocol_id) - return self.wrap_member(ref) - - def _list_protocols(self, idp_id): + def get(self, idp_id): """List protocols for an IDP. HEAD/GET /OS-FEDERATION/identity_providers/{idp_id}/protocols @@ -207,6 +194,19 @@ class IdentityProvidersProtocolsResource(_ResourceBase): self._add_related_links(r) return collection + +class IDPProtocolsCRUDResource(_IdentityProvidersProtocolsResourceBase): + + def get(self, idp_id, protocol_id): + """Get protocols for an IDP. + + HEAD/GET /OS-FEDERATION/identity_providers/ + {idp_id}/protocols/{protocol_id} + """ + ENFORCER.enforce_call(action='identity:get_protocol') + ref = PROVIDERS.federation_api.get_protocol(idp_id, protocol_id) + return self.wrap_member(ref) + def put(self, idp_id, protocol_id): """Create protocol for an IDP. @@ -478,9 +478,31 @@ class OSFederationIdentityProvidersAPI(ks_flask.APIBase): class OSFederationIdentityProvidersProtocolsAPI(ks_flask.APIBase): _name = 'protocols' _import_name = __name__ - _api_url_prefix = '/OS-FEDERATION/identity_providers/' - resources = [IdentityProvidersProtocolsResource] - resource_mapping = [] + resources = [] + resource_mapping = [ + ks_flask.construct_resource_map( + resource=IDPProtocolsCRUDResource, + url=('/OS-FEDERATION/identity_providers//protocols/' + ''), + resource_kwargs={}, + rel='identity_provider_protocol', + resource_relation_func=_build_resource_relation, + path_vars={ + 'idp_id': IDP_ID_PARAMETER_RELATION, + 'protocol_id': PROTOCOL_ID_PARAMETER_RELATION + } + ), + ks_flask.construct_resource_map( + resource=IDPProtocolsListResource, + url='/OS-FEDERATION/identity_providers//protocols', + resource_kwargs={}, + rel='identity_provider_protocols', + resource_relation_func=_build_resource_relation, + path_vars={ + 'idp_id': IDP_ID_PARAMETER_RELATION + } + ), + ] class OSFederationMappingsAPI(ks_flask.APIBase): diff --git a/keystone/tests/unit/test_v3_federation.py b/keystone/tests/unit/test_v3_federation.py index 00a5ace2b7..b02b9a8ab0 100644 --- a/keystone/tests/unit/test_v3_federation.py +++ b/keystone/tests/unit/test_v3_federation.py @@ -1527,6 +1527,54 @@ class FederatedIdentityProviderTests(test_v3.RestfulTestCase): validate=False, **kwargs) + def test_crud_protocol_without_protocol_id_in_url(self): + # NOTE(morgan): This test is redundant but is added to ensure + # the url routing error in bug 1817313 is explicitly covered. + # create a protocol, but do not put the ID in the URL + idp_id, _ = self._create_and_decapsulate_response() + mapping_id = uuid.uuid4().hex + self._create_mapping(mapping_id=mapping_id) + protocol = { + 'id': uuid.uuid4().hex, + 'mapping_id': mapping_id + } + with self.test_client() as c: + token = self.get_scoped_token() + # DELETE/PATCH/PUT on non-trailing `/` results in + # METHOD_NOT_ALLOWED + c.delete('/v3/OS-FEDERATION/identity_providers/%(idp_id)s' + '/protocols' % {'idp_id': idp_id}, + headers={'X-Auth-Token': token}, + expected_status_code=http_client.METHOD_NOT_ALLOWED) + c.patch('/v3/OS-FEDERATION/identity_providers/%(idp_id)s' + '/protocols/' % {'idp_id': idp_id}, + json={'protocol': protocol}, + headers={'X-Auth-Token': token}, + expected_status_code=http_client.METHOD_NOT_ALLOWED) + c.put('/v3/OS-FEDERATION/identity_providers/%(idp_id)s' + '/protocols' % {'idp_id': idp_id}, + json={'protocol': protocol}, + headers={'X-Auth-Token': token}, + expected_status_code=http_client.METHOD_NOT_ALLOWED) + + # DELETE/PATCH/PUT should raise 405 with trailing '/', it is + # remapped to without the trailing '/' by the normalization + # middleware. + c.delete('/v3/OS-FEDERATION/identity_providers/%(idp_id)s' + '/protocols/' % {'idp_id': idp_id}, + headers={'X-Auth-Token': token}, + expected_status_code=http_client.METHOD_NOT_ALLOWED) + c.patch('/v3/OS-FEDERATION/identity_providers/%(idp_id)s' + '/protocols/' % {'idp_id': idp_id}, + json={'protocol': protocol}, + headers={'X-Auth-Token': token}, + expected_status_code=http_client.METHOD_NOT_ALLOWED) + c.put('/v3/OS-FEDERATION/identity_providers/%(idp_id)s' + '/protocols/' % {'idp_id': idp_id}, + json={'protocol': protocol}, + headers={'X-Auth-Token': token}, + expected_status_code=http_client.METHOD_NOT_ALLOWED) + def test_get_head_protocol(self): """Create and later fetch protocol tied to IdP.""" resp, idp_id, proto = self._assign_protocol_to_idp( diff --git a/releasenotes/notes/bug-1817313-c11481e6eed29ec2.yaml b/releasenotes/notes/bug-1817313-c11481e6eed29ec2.yaml new file mode 100644 index 0000000000..81f7b3366c --- /dev/null +++ b/releasenotes/notes/bug-1817313-c11481e6eed29ec2.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + [`bug 1817313 `_] + Raise METHOD NOT ALLOWED for OS-Federation protocols creation + if the protocol_id is not in the URL. The corrective action was to split + the LIST from CRUD resources so that the routing regexes can work as + expected. \ No newline at end of file