From 59312bf811e9b86156a8e94a83057a16212e9090 Mon Sep 17 00:00:00 2001 From: Wojciech Bartosiak Date: Tue, 4 Oct 2016 19:32:54 +0200 Subject: [PATCH 1/3] redirect URI clean up moved to utils module --- oidc_provider/lib/endpoints/authorize.py | 4 ++-- oidc_provider/lib/endpoints/token.py | 6 +++++- oidc_provider/lib/utils/common.py | 20 +++++++++++++++++ oidc_provider/tests/test_token_endpoint.py | 25 ++++++++++++++++++++++ 4 files changed, 52 insertions(+), 3 deletions(-) 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 707f403..6272912 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: @@ -86,7 +89,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/tests/test_token_endpoint.py b/oidc_provider/tests/test_token_endpoint.py index 7f705d6..572f781 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 From d174d4e629f62a25af22c17e3b6eafb9392d06b8 Mon Sep 17 00:00:00 2001 From: Wojciech Bartosiak Date: Wed, 5 Oct 2016 17:37:49 +0200 Subject: [PATCH 2/3] fix for generating client secret --- oidc_provider/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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')) From 2bf2ffa2756bf44b9a01a7933652ce3d53aaeb3c Mon Sep 17 00:00:00 2001 From: Wojciech Bartosiak Date: Wed, 5 Oct 2016 17:58:39 +0200 Subject: [PATCH 3/3] added migrations for client secret --- .../migrations/0019_auto_20161005_1552.py | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 oidc_provider/migrations/0019_auto_20161005_1552.py 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'), + ), + ]