From dc9ec6bf3d4dc1e0cc1d2312bba04e7bad5642b5 Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Mon, 13 Mar 2017 12:47:28 +0100 Subject: [PATCH] idp_oidc: make OIDCCode.{nonce,state} nullable (fixes #15612) Those fields are optional so we need to differentiate their presence or their absence. --- .../migrations/0003_auto_20170324_1426.py | 24 +++++++++++++++++++ src/authentic2_idp_oidc/models.py | 4 ++-- src/authentic2_idp_oidc/views.py | 28 +++++++++++++--------- 3 files changed, 43 insertions(+), 13 deletions(-) create mode 100644 src/authentic2_idp_oidc/migrations/0003_auto_20170324_1426.py diff --git a/src/authentic2_idp_oidc/migrations/0003_auto_20170324_1426.py b/src/authentic2_idp_oidc/migrations/0003_auto_20170324_1426.py new file mode 100644 index 0000000..1d5e27f --- /dev/null +++ b/src/authentic2_idp_oidc/migrations/0003_auto_20170324_1426.py @@ -0,0 +1,24 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('authentic2_idp_oidc', '0002_auto_20170121_2346'), + ] + + operations = [ + migrations.AlterField( + model_name='oidccode', + name='nonce', + field=models.TextField(null=True, verbose_name='nonce'), + ), + migrations.AlterField( + model_name='oidccode', + name='state', + field=models.TextField(null=True, verbose_name='state'), + ), + ] diff --git a/src/authentic2_idp_oidc/models.py b/src/authentic2_idp_oidc/models.py index 0f0226b..ec299a8 100644 --- a/src/authentic2_idp_oidc/models.py +++ b/src/authentic2_idp_oidc/models.py @@ -152,10 +152,10 @@ class OIDCCode(models.Model): scopes = models.TextField( verbose_name=_('scopes')) state = models.TextField( - default='', + null=True, verbose_name=_('state')) nonce = models.TextField( - default='', + null=True, verbose_name=_('nonce')) redirect_uri = models.URLField( verbose_name=_('redirect URI')) diff --git a/src/authentic2_idp_oidc/views.py b/src/authentic2_idp_oidc/views.py index 66933fc..172a09b 100644 --- a/src/authentic2_idp_oidc/views.py +++ b/src/authentic2_idp_oidc/views.py @@ -59,7 +59,7 @@ def authorization_error(request, redirect_uri, error, error_description=None, er params['error_description'] = error_description if error_uri: params['error_uri'] = error_uri - if state: + if state is not None: params['state'] = state if fragment: return redirect(request, redirect_uri + '#%s' % urlencode(params), resolve=False) @@ -83,7 +83,7 @@ def authorize(request, *args, **kwargs): return HttpResponseBadRequest('invalid request: unknown client_id') fragment = client.authorization_flow == client.FLOW_IMPLICIT - state = request.GET.get('state', '') + state = request.GET.get('state') try: response_type = request.GET['response_type'] @@ -95,7 +95,7 @@ def authorize(request, *args, **kwargs): fragment=fragment) prompt = set(filter(None, request.GET.get('prompt', '').split())) - nonce = request.GET.get('nonce', '') + nonce = request.GET.get('nonce') scopes = utils.scope_set(scope) max_age = request.GET.get('max_age') @@ -158,7 +158,10 @@ def authorize(request, *args, **kwargs): error_description='login is required but prompt is none', state=state, fragment=fragment) - return login_require(request, params={'nonce': nonce}) + params = {} + if nonce is not None: + params['nonce'] = nonce + return login_require(request, params=params) last_auth = last_authentication_event(request.session) if max_age is not None and time.time() - last_auth['when'] >= max_age: @@ -167,7 +170,10 @@ def authorize(request, *args, **kwargs): error_description='login is required but prompt is none', state=state, fragment=fragment) - return login_require(request, params={'nonce': nonce}) + params = {} + if nonce is not None: + params['nonce'] = nonce + return login_require(request, params=params) qs = models.OIDCAuthorization.objects.filter(client=client, user=request.user) if 'consent' in prompt: @@ -226,7 +232,7 @@ def authorize(request, *args, **kwargs): params = { 'code': unicode(code.uuid), } - if state: + if state is not None: params['state'] = state return redirect(request, redirect_uri, params=params, resolve=False) else: @@ -241,7 +247,7 @@ def authorize(request, *args, **kwargs): session_key=request.session.session_key, expired=start + datetime.timedelta(seconds=expires_in)) acr = 0 - if nonce and last_auth.get('nonce') == nonce: + if nonce is not None and last_auth.get('nonce') == nonce: acr = 1 id_token = { 'iss': request.build_absolute_uri('/'), @@ -253,12 +259,12 @@ def authorize(request, *args, **kwargs): 'auth_time': last_auth['when'], 'acr': acr, } - if nonce: + if nonce is not None: id_token['nonce'] = nonce params = { 'id_token': utils.make_idtoken(client, id_token), } - if state: + if state is not None: params['state'] = state if need_access_token: params.update({ @@ -343,7 +349,7 @@ def token(request, *args, **kwargs): expired=oidc_code.created + datetime.timedelta(seconds=expires_in)) start = now() acr = 0 - if (oidc_code.nonce and last_authentication_event(oidc_code.session).get('nonce') == + if (oidc_code.nonce is not None and last_authentication_event(oidc_code.session).get('nonce') == oidc_code.nonce): acr = 1 id_token = { @@ -356,7 +362,7 @@ def token(request, *args, **kwargs): 'auth_time': timestamp_from_datetime(oidc_code.auth_time), 'acr': acr, } - if oidc_code.nonce: + if oidc_code.nonce is not None: id_token['nonce'] = oidc_code.nonce response = HttpResponse(json.dumps({ 'access_token': unicode(access_token.uuid), -- 2.1.4