idp_oidc: never use an invalid redirect_uri (fixes #28029)
Check of "redirect_uri" move earlier during authorization request processing. For any redirect_uri check failure errors are only shown to the end user and redirect_uri is never used to redirect to the requesting RP.
This commit is contained in:
parent
e176dec736
commit
4c45876e0a
|
@ -87,12 +87,23 @@ def authorize(request, *args, **kwargs):
|
|||
client_id = request.GET['client_id']
|
||||
redirect_uri = request.GET['redirect_uri']
|
||||
except KeyError as k:
|
||||
return HttpResponseBadRequest('invalid request: missing parameter %s' % k.args[0],
|
||||
content_type='text/plain')
|
||||
messages.warning(request, _('Authorization request is invalid'))
|
||||
logger.warning(u'idp_oidc: authorization request error, missing %s', k.args[0])
|
||||
return redirect(request, 'auth_homepage')
|
||||
try:
|
||||
client = models.OIDCClient.objects.get(client_id=client_id)
|
||||
except models.OIDCClient.DoesNotExist:
|
||||
return HttpResponseBadRequest('invalid request: unknown client_id', content_type='text/plain')
|
||||
messages.warning(request, _('Authorization request is invalid'))
|
||||
logger.warning(u'idp_oidc: authorization request error, unknown client_id redirect_uri=%r client_id=%r',
|
||||
redirect_uri, client_id)
|
||||
return redirect(request, 'auth_homepage')
|
||||
|
||||
if redirect_uri not in client.redirect_uris.split():
|
||||
messages.warning(request, _('Authorization request is invalid'))
|
||||
logger.warning(u'idp_oidc: authorization request error, unknown redirect_uri redirect_uri=%r client_id=%r',
|
||||
redirect_uri, client_id)
|
||||
return redirect(request, 'auth_homepage')
|
||||
|
||||
fragment = client.authorization_flow == client.FLOW_IMPLICIT
|
||||
|
||||
state = request.GET.get('state')
|
||||
|
@ -122,11 +133,6 @@ def authorize(request, *args, **kwargs):
|
|||
state=state,
|
||||
fragment=fragment)
|
||||
|
||||
if redirect_uri not in client.redirect_uris.split():
|
||||
return authorization_error(request, redirect_uri, 'invalid_request',
|
||||
error_description='unauthorized redirect_uri',
|
||||
state=state,
|
||||
fragment=fragment)
|
||||
if client.authorization_flow == client.FLOW_AUTHORIZATION_CODE:
|
||||
if response_type != 'code':
|
||||
return authorization_error(request, redirect_uri, 'unsupported_response_type',
|
||||
|
|
|
@ -344,19 +344,23 @@ def test_invalid_request(caplog, oidc_settings, oidc_client, simple_user, app):
|
|||
else:
|
||||
raise NotImplementedError
|
||||
|
||||
# client_id
|
||||
# missing client_id
|
||||
authorize_url = make_url('oidc-authorize', params={})
|
||||
|
||||
response = app.get(authorize_url, status=400)
|
||||
assert 'missing parameter \'client_id\'' in response.content
|
||||
response = app.get(authorize_url, status=302)
|
||||
assert urlparse.urlparse(response['Location']).path == '/'
|
||||
response = response.maybe_follow()
|
||||
assert 'Authorization request is invalid' in response
|
||||
|
||||
# redirect_uri
|
||||
# missing redirect_uri
|
||||
authorize_url = make_url('oidc-authorize', params={
|
||||
'client_id': oidc_client.client_id,
|
||||
})
|
||||
|
||||
response = app.get(authorize_url, status=400)
|
||||
assert 'missing parameter \'redirect_uri\'' in response.content
|
||||
response = app.get(authorize_url, status=302)
|
||||
assert urlparse.urlparse(response['Location']).path == '/'
|
||||
response = response.maybe_follow()
|
||||
assert 'Authorization request is invalid' in response
|
||||
|
||||
# invalid client_id
|
||||
authorize_url = make_url('oidc-authorize', params={
|
||||
|
@ -364,8 +368,23 @@ def test_invalid_request(caplog, oidc_settings, oidc_client, simple_user, app):
|
|||
'redirect_uri': redirect_uri,
|
||||
})
|
||||
|
||||
response = app.get(authorize_url, status=400)
|
||||
assert 'unknown client_id' in response.content
|
||||
response = app.get(authorize_url, status=302)
|
||||
assert urlparse.urlparse(response['Location']).path == '/'
|
||||
response = response.maybe_follow()
|
||||
assert 'Authorization request is invalid' in response
|
||||
|
||||
# invalid redirect_uri
|
||||
authorize_url = make_url('oidc-authorize', params={
|
||||
'client_id': oidc_client.client_id,
|
||||
'redirect_uri': 'xxx',
|
||||
'response_type': 'code',
|
||||
'scope': 'openid',
|
||||
})
|
||||
|
||||
response = app.get(authorize_url, status=302)
|
||||
assert urlparse.urlparse(response['Location']).path == '/'
|
||||
response = response.maybe_follow()
|
||||
assert 'Authorization request is invalid' in response
|
||||
|
||||
# missing response_type
|
||||
authorize_url = make_url('oidc-authorize', params={
|
||||
|
@ -411,17 +430,6 @@ def test_invalid_request(caplog, oidc_settings, oidc_client, simple_user, app):
|
|||
response = app.get(authorize_url)
|
||||
assert_oidc_error(response, 'invalid_request', 'max_age is not', fragment=fragment)
|
||||
|
||||
# invalid redirect_uri
|
||||
authorize_url = make_url('oidc-authorize', params={
|
||||
'client_id': oidc_client.client_id,
|
||||
'redirect_uri': 'xxx',
|
||||
'response_type': 'code',
|
||||
'scope': 'openid',
|
||||
})
|
||||
|
||||
response = app.get(authorize_url)
|
||||
assert_oidc_error(response, 'invalid_request', 'unauthorized redirect_uri', fragment=fragment)
|
||||
|
||||
# unsupported response_type
|
||||
authorize_url = make_url('oidc-authorize', params={
|
||||
'client_id': oidc_client.client_id,
|
||||
|
|
Loading…
Reference in New Issue