From 211577d446616e40de23a92fdeb6c55beb77d3f3 Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Fri, 21 Jun 2019 19:51:35 +0200 Subject: [PATCH 2/2] idp_oidc: add more freedom for matching redirect_uri (#33516) --- src/authentic2_idp_oidc/models.py | 27 +++++++++++++++++++++++++ src/authentic2_idp_oidc/views.py | 2 +- tests/test_idp_oidc.py | 33 +++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) diff --git a/src/authentic2_idp_oidc/models.py b/src/authentic2_idp_oidc/models.py index 412f423b..ebe2d933 100644 --- a/src/authentic2_idp_oidc/models.py +++ b/src/authentic2_idp_oidc/models.py @@ -17,6 +17,7 @@ import uuid from importlib import import_module + from django.db import models from django.core.validators import URLValidator from django.core.exceptions import ValidationError, ImproperlyConfigured @@ -24,6 +25,7 @@ from django.utils.translation import ugettext_lazy as _ from django.conf import settings from django.utils import six from django.utils.timezone import now +from django.utils.six.moves.urllib import parse as urlparse from django.contrib.contenttypes.fields import GenericForeignKey, GenericRelation from authentic2.a2_rbac.models import OrganizationalUnit @@ -171,6 +173,31 @@ class OIDCClient(Service): def get_wanted_attributes(self): return self.oidcclaim_set.filter(name__isnull=False).values_list('value', flat=True) + def is_valid_redirect_uri(self, redirect_uri): + parsed_uri = urlparse.urlparse(redirect_uri) + for valid_redirect_uri in self.redirect_uris.split(): + parsed_valid_uri = urlparse.urlparse(valid_redirect_uri) + if parsed_uri.scheme != parsed_valid_uri.scheme: + continue + if parsed_valid_uri.netloc.startswith('*'): + # globing on the left + netloc = parsed_valid_uri.netloc.lstrip('*') + if (parsed_uri.netloc != netloc + and not parsed_uri.netloc.endswith('.' + netloc)): + continue + elif parsed_uri.netloc != parsed_valid_uri.netloc: + continue + if parsed_valid_uri.path.endswith('*'): + path = parsed_valid_uri.path.rstrip('*').rstrip('/') + if (parsed_uri.path.rstrip('/') != path + and not parsed_uri.path.startswith(path + '/')): + continue + else: + if parsed_uri.path.rstrip('/') != parsed_valid_uri.path.rstrip('/'): + continue + return True + return False + def __repr__(self): return ('' % (self.name, self.client_id, self.get_identifier_policy_display())) diff --git a/src/authentic2_idp_oidc/views.py b/src/authentic2_idp_oidc/views.py index b28973e8..ddd4fc4d 100644 --- a/src/authentic2_idp_oidc/views.py +++ b/src/authentic2_idp_oidc/views.py @@ -115,7 +115,7 @@ def authorize(request, *args, **kwargs): redirect_uri, client_id) return redirect(request, 'auth_homepage') - if redirect_uri not in client.redirect_uris.split(): + if not client.is_valid_redirect_uri(redirect_uri): 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) diff --git a/tests/test_idp_oidc.py b/tests/test_idp_oidc.py index ae725483..07ebfe6c 100644 --- a/tests/test_idp_oidc.py +++ b/tests/test_idp_oidc.py @@ -1100,3 +1100,36 @@ def test_claim_default_value(oidc_settings, normal_oidc_client, simple_user, app assert 'preferred_username' not in user_info assert 'given_name' not in user_info assert 'family_name' not in user_info + + +def test_client_is_valid_redirect_uri(): + client = OIDCClient(redirect_uris='''http://example.com +http://example2.com/ +http://example3.com/toto +http://*example4.com/ +http://example5.com/toto* +''') + assert client.is_valid_redirect_uri('http://example.com') + assert client.is_valid_redirect_uri('http://example.com/') + assert not client.is_valid_redirect_uri('http://coin.example.com/') + assert not client.is_valid_redirect_uri('http://example.com/toto/') + assert not client.is_valid_redirect_uri('http://coin.example.com') + assert client.is_valid_redirect_uri('http://example2.com') + assert client.is_valid_redirect_uri('http://example2.com/') + assert not client.is_valid_redirect_uri('http://example3.com/') + assert not client.is_valid_redirect_uri('http://example3.com') + assert client.is_valid_redirect_uri('http://example3.com/toto') + assert client.is_valid_redirect_uri('http://example3.com/toto/') + assert client.is_valid_redirect_uri('http://example4.com/') + assert client.is_valid_redirect_uri('http://example4.com') + assert client.is_valid_redirect_uri('http://coin.example4.com') + assert client.is_valid_redirect_uri('http://coin.example4.com/') + assert not client.is_valid_redirect_uri('http://coinexample4.com') + assert not client.is_valid_redirect_uri('http://coinexample4.com/') + assert client.is_valid_redirect_uri('http://example5.com/toto') + assert client.is_valid_redirect_uri('http://example5.com/toto/') + assert client.is_valid_redirect_uri('http://example5.com/toto/tata') + assert client.is_valid_redirect_uri('http://example5.com/toto/tata/') + assert not client.is_valid_redirect_uri('http://example5.com/tototata/') + assert not client.is_valid_redirect_uri('http://example5.com/tototata') + -- 2.20.1