Raise METHOD NOT ALLOWED instead of 500 error on protocol CRUD

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.

Change-Id: I063e3afa1ef8dbf957d62fb4d44dac0f0860ec94
closes-bug: #1817313
This commit is contained in:
Morgan Fainberg 2019-03-27 13:49:39 -07:00
parent f85fca14c5
commit 9717f0c12f
3 changed files with 97 additions and 19 deletions

View File

@ -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/<string:idp_id>'
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/<string:idp_id>'
resources = [IdentityProvidersProtocolsResource]
resource_mapping = []
resources = []
resource_mapping = [
ks_flask.construct_resource_map(
resource=IDPProtocolsCRUDResource,
url=('/OS-FEDERATION/identity_providers/<string:idp_id>/protocols/'
'<string:protocol_id>'),
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/<string:idp_id>/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):

View File

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

View File

@ -0,0 +1,8 @@
---
fixes:
- |
[`bug 1817313 <https://bugs.launchpad.net/keystone/+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.