From b1709fdd511431ee53155c3114fa523d0bcc6344 Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Fri, 15 Jul 2022 11:27:11 +0200 Subject: [PATCH] idp_oidc: adapt error message for expired codes (#67277) --- src/authentic2_idp_oidc/views.py | 7 ++++--- tests/idp_oidc/test_misc.py | 36 +++++++++++++++++++++----------- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/src/authentic2_idp_oidc/views.py b/src/authentic2_idp_oidc/views.py index 056bb72e..95a28215 100644 --- a/src/authentic2_idp_oidc/views.py +++ b/src/authentic2_idp_oidc/views.py @@ -732,14 +732,15 @@ def tokens_from_authz_code(request): code = request.POST.get('code') if not code: raise MissingParameter('code', client=client) + oidc_code_qs = models.OIDCCode.objects.filter(expired__gte=now()).select_related() try: - oidc_code = models.OIDCCode.objects.select_related().get(uuid=code) + oidc_code = oidc_code_qs.get(uuid=code) except models.OIDCCode.DoesNotExist: - raise InvalidGrant(_('Code is unknown.'), client=client) + raise InvalidGrant(_('Code is unknown or has expired.'), client=client) if oidc_code.client != client: raise InvalidGrant(_('Code was issued to a different client.'), client=client) if not oidc_code.is_valid(): - raise InvalidGrant(_('Code has expired, user is disconnected or session was lost.'), client=client) + raise InvalidGrant(_('User is disconnected or session was lost.'), client=client) redirect_uri = request.POST.get('redirect_uri') if oidc_code.redirect_uri != redirect_uri: raise InvalidGrant(_('Redirect_uri does not match the code.'), client=client) diff --git a/tests/idp_oidc/test_misc.py b/tests/idp_oidc/test_misc.py index 4e566b0a..c2a998ea 100644 --- a/tests/idp_oidc/test_misc.py +++ b/tests/idp_oidc/test_misc.py @@ -470,7 +470,7 @@ def assert_authorization_response(response, fragment=False, **kwargs): @pytest.mark.parametrize('oidc_client', OIDC_CLIENT_PARAMS, indirect=True) -def test_invalid_request(oidc_client, caplog, oidc_settings, simple_user, app, make_client): +def test_invalid_request(oidc_client, caplog, oidc_settings, simple_user, app, make_client, app_factory): redirect_uri = oidc_client.redirect_uris.split()[0] if oidc_client.authorization_flow == oidc_client.FLOW_AUTHORIZATION_CODE: fragment = False @@ -864,6 +864,7 @@ def test_invalid_request(oidc_client, caplog, oidc_settings, simple_user, app, m # check expired codes if oidc_client.authorization_flow == oidc_client.FLOW_AUTHORIZATION_CODE: + rp_app = app_factory() assert OIDCCode.objects.count() == 1 oidc_code = OIDCCode.objects.get() assert oidc_code.is_valid() @@ -880,7 +881,7 @@ def test_invalid_request(oidc_client, caplog, oidc_settings, simple_user, app, m 'grant_type': 'authorization_code', 'redirect_uri': oidc_client.redirect_uris.split()[0], } - response = app.post( + response = rp_app.post( token_url, params=params, headers=client_authentication_headers(oidc_client), status=400 ) assert response.json['error'] == 'invalid_request' @@ -889,7 +890,7 @@ def test_invalid_request(oidc_client, caplog, oidc_settings, simple_user, app, m params['code'] = code # wrong redirect_uri - response = app.post( + response = rp_app.post( token_url, params=dict(params, redirect_uri='https://example.com/'), headers=client_authentication_headers(oidc_client), @@ -901,7 +902,7 @@ def test_invalid_request(oidc_client, caplog, oidc_settings, simple_user, app, m assert response.json['client_id'] == '1234' # unknown code - response = app.post( + response = rp_app.post( token_url, params=dict(params, code='xyz'), headers=client_authentication_headers(oidc_client), @@ -909,11 +910,11 @@ def test_invalid_request(oidc_client, caplog, oidc_settings, simple_user, app, m ) assert 'error' in response.json assert response.json['error'] == 'invalid_grant' - assert response.json['error_description'] == 'Code is unknown.' + assert response.json['error_description'] == 'Code is unknown or has expired.' assert response.json['client_id'] == '1234' # code from another client - other_client = make_client(app, params={'slug': 'other', 'name': 'other', 'client_id': 'abcd'}) + other_client = make_client(rp_app, params={'slug': 'other', 'name': 'other', 'client_id': 'abcd'}) other_oidc_code = OIDCCode.objects.create( client=other_client, user=oidc_code.user, @@ -926,7 +927,7 @@ def test_invalid_request(oidc_client, caplog, oidc_settings, simple_user, app, m auth_time=now(), session_key=oidc_code.session_key, ) - response = app.post( + response = rp_app.post( token_url, params=dict(params, code=other_oidc_code.uuid), headers=client_authentication_headers(oidc_client), @@ -939,21 +940,32 @@ def test_invalid_request(oidc_client, caplog, oidc_settings, simple_user, app, m other_oidc_code.delete() other_client.delete() + # simulate expired session + from django.contrib.sessions.models import Session + + session = Session.objects.get(session_key=oidc_code.session_key) + Session.objects.filter(pk=session.pk).delete() + response = rp_app.post( + token_url, params=params, headers=client_authentication_headers(oidc_client), status=400 + ) + assert 'error' in response.json + assert response.json['error'] == 'invalid_grant' + assert response.json['error_description'] == 'User is disconnected or session was lost.' + assert response.json['client_id'] == '1234' + session.save() + # make code expire oidc_code.expired = now() - datetime.timedelta(seconds=120) assert not oidc_code.is_valid() oidc_code.save() # expired code - response = app.post( + response = rp_app.post( token_url, params=params, headers=client_authentication_headers(oidc_client), status=400 ) assert 'error' in response.json assert response.json['error'] == 'invalid_grant' - assert ( - response.json['error_description'] - == 'Code has expired, user is disconnected or session was lost.' - ) + assert response.json['error_description'] == 'Code is unknown or has expired.' assert response.json['client_id'] == '1234' # invalid logout -- 2.36.1