diff --git a/oidc_provider/lib/endpoints/authorize.py b/oidc_provider/lib/endpoints/authorize.py index 1844eb6..58be181 100644 --- a/oidc_provider/lib/endpoints/authorize.py +++ b/oidc_provider/lib/endpoints/authorize.py @@ -25,6 +25,7 @@ from oidc_provider.models import ( UserConsent, ) from oidc_provider import settings +from oidc_provider.lib.utils.common import cleanup_url_from_query_string logger = logging.getLogger(__name__) @@ -85,8 +86,7 @@ class AuthorizeEndpoint(object): if self.is_authentication and not self.params['redirect_uri']: logger.debug('[Authorize] Missing redirect uri.') raise RedirectUriError() - clean_redirect_uri = urlsplit(self.params['redirect_uri']) - clean_redirect_uri = urlunsplit(clean_redirect_uri._replace(query='')) + clean_redirect_uri = cleanup_url_from_query_string(self.params['redirect_uri']) if not (clean_redirect_uri in self.client.redirect_uris): logger.debug('[Authorize] Invalid redirect uri: %s', self.params['redirect_uri']) raise RedirectUriError() diff --git a/oidc_provider/lib/endpoints/token.py b/oidc_provider/lib/endpoints/token.py index fd5d761..0bf52da 100644 --- a/oidc_provider/lib/endpoints/token.py +++ b/oidc_provider/lib/endpoints/token.py @@ -2,6 +2,9 @@ from base64 import b64decode, urlsafe_b64encode import hashlib import logging import re + +from oidc_provider.lib.utils.common import cleanup_url_from_query_string + try: from urllib.parse import unquote except ImportError: @@ -87,7 +90,8 @@ class TokenEndpoint(object): raise TokenError('invalid_client') if self.params['grant_type'] == 'authorization_code': - if not (self.params['redirect_uri'] in self.client.redirect_uris): + clean_redirect_uri = cleanup_url_from_query_string(self.params['redirect_uri']) + if not (clean_redirect_uri in self.client.redirect_uris): logger.debug('[Token] Invalid redirect uri: %s', self.params['redirect_uri']) raise TokenError('invalid_client') diff --git a/oidc_provider/lib/utils/common.py b/oidc_provider/lib/utils/common.py index 236c12a..2c4971c 100644 --- a/oidc_provider/lib/utils/common.py +++ b/oidc_provider/lib/utils/common.py @@ -3,6 +3,26 @@ from django.http import HttpResponse from oidc_provider import settings +try: + from urlparse import urlsplit, urlunsplit +except ImportError: + from urllib.parse import urlsplit, urlunsplit + + +def cleanup_url_from_query_string(uri): + """ + Function used to clean up the uri from any query string, used i.e. by endpoints to validate redirect_uri + + :param uri: URI to clean from query string + :type uri: str + :return: cleaned URI without query string + """ + + clean_uri = urlsplit(uri) + # noinspection PyProtectedMember + clean_uri = urlunsplit(clean_uri._replace(query='')) + return clean_uri + def redirect(uri): """ diff --git a/oidc_provider/migrations/0019_auto_20161005_1552.py b/oidc_provider/migrations/0019_auto_20161005_1552.py new file mode 100644 index 0000000..f2afff9 --- /dev/null +++ b/oidc_provider/migrations/0019_auto_20161005_1552.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.10.2 on 2016-10-05 15:52 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('oidc_provider', '0018_hybridflow_and_clientattrs'), + ] + + operations = [ + migrations.AlterField( + model_name='client', + name='client_secret', + field=models.CharField(blank=True, max_length=255, verbose_name='Client SECRET'), + ), + ] diff --git a/oidc_provider/models.py b/oidc_provider/models.py index 4167b96..19cc068 100644 --- a/oidc_provider/models.py +++ b/oidc_provider/models.py @@ -35,7 +35,7 @@ class Client(models.Model): name = models.CharField(max_length=100, default='', verbose_name=_(u'Name')) client_type = models.CharField(max_length=30, choices=CLIENT_TYPE_CHOICES, default='confidential', verbose_name=_(u'Client Type'), help_text=_(u'Confidential clients are capable of maintaining the confidentiality of their credentials. Public clients are incapable.')) client_id = models.CharField(max_length=255, unique=True, verbose_name=_(u'Client ID')) - client_secret = models.CharField(max_length=255, blank=True, default='', verbose_name=_(u'Client SECRET')) + client_secret = models.CharField(max_length=255, blank=True, verbose_name=_(u'Client SECRET')) response_type = models.CharField(max_length=30, choices=RESPONSE_TYPE_CHOICES, verbose_name=_(u'Response Type')) jwt_alg = models.CharField(max_length=10, choices=JWT_ALGS, default='RS256', verbose_name=_(u'JWT Algorithm'), help_text=_(u'Algorithm used to encode ID Tokens.')) date_created = models.DateField(auto_now_add=True, verbose_name=_(u'Date Created')) diff --git a/oidc_provider/tests/test_token_endpoint.py b/oidc_provider/tests/test_token_endpoint.py index c0c37b7..de7972e 100644 --- a/oidc_provider/tests/test_token_endpoint.py +++ b/oidc_provider/tests/test_token_endpoint.py @@ -213,6 +213,31 @@ class TokenTestCase(TestCase): response = self._post_request(post_data) self.assertIn('invalid_grant', response.content.decode('utf-8')) + def test_client_redirect_url(self): + """ + Validate that client redirect URIs with query strings match registered + URIs, and that unregistered URIs are rejected. + + source: https://github.com/jerrykan/django-oidc-provider/blob/2f54e537666c689dd8448f8bbc6a3a0244b01a97/oidc_provider/tests/test_token_endpoint.py + """ + SIGKEYS = self._get_keys() + code = self._create_code() + post_data = self._auth_code_post_data(code=code.code) + + # Unregistered URI + post_data['redirect_uri'] = 'http://invalid.example.org' + + response = self._post_request(post_data) + + self.assertIn('invalid_client', response.content.decode('utf-8')), + + # Registered URI contained a query string + post_data['redirect_uri'] = 'http://example.com/?client=OidcClient' + + response = self._post_request(post_data) + + self.assertNotIn('invalid_client', response.content.decode('utf-8')), + def test_request_methods(self): """ Client sends an HTTP POST request to the Token Endpoint. Other request