From 2b58e263dd9bc2991408ea952ed6898e1d38d32a Mon Sep 17 00:00:00 2001 From: Tobias Reese Date: Fri, 8 Feb 2019 02:55:28 +0100 Subject: [PATCH 1/3] Implemented implicit flow. New fix for username vulnerability. --- oauth2_provider_jwt/urls.py | 4 +-- oauth2_provider_jwt/views.py | 42 +++++++++++++++++++++++-------- tests/test_views.py | 49 +++++++++++++++--------------------- 3 files changed, 54 insertions(+), 41 deletions(-) diff --git a/oauth2_provider_jwt/urls.py b/oauth2_provider_jwt/urls.py index 02be88f..e421c9f 100644 --- a/oauth2_provider_jwt/urls.py +++ b/oauth2_provider_jwt/urls.py @@ -1,12 +1,12 @@ from django.conf.urls import url from oauth2_provider import views -from .views import TokenView +from .views import TokenView, JWTAuthorizationView app_name = "oauth2_provider_jwt" urlpatterns = [ - url(r"^authorize/$", views.AuthorizationView.as_view(), name="authorize"), + url(r"^authorize/$", JWTAuthorizationView.as_view(), name="authorize"), url(r"^token/$", TokenView.as_view(), name="token"), url(r"^revoke_token/$", views.RevokeTokenView.as_view(), name="revoke-token"), diff --git a/oauth2_provider_jwt/views.py b/oauth2_provider_jwt/views.py index a0e464f..d114259 100644 --- a/oauth2_provider_jwt/views.py +++ b/oauth2_provider_jwt/views.py @@ -2,9 +2,16 @@ import json import logging +try: + from urllib.parse import urlencode, urlparse, parse_qs +except ImportError: + from urllib import urlencode # noqa + from urlparse import urlparse, parse_qs + from django.conf import settings from django.utils.module_loading import import_string from oauth2_provider import views +from oauth2_provider.http import OAuth2ResponseRedirect from oauth2_provider.models import get_access_token_model from .utils import generate_payload, encode_jwt @@ -16,6 +23,27 @@ class WrongUsername(Exception): pass +class JWTAuthorizationView(views.AuthorizationView): + + def get(self, request, *args, **kwargs): + response = super(JWTAuthorizationView, self).get(request, *args, + **kwargs) + if request.GET.get('response_type', None) == 'token' \ + and response.status_code == 302: + url = urlparse(response.url) + params = parse_qs(url.fragment) + content = { + 'access_token': params['access_token'][0], + 'expires_in': int(params['expires_in'][0]), + 'scope': params['scope'][0] + } + jwt = TokenView()._get_access_token_jwt(request, content) + response = OAuth2ResponseRedirect( + response.url + '&access_token_jwt=' + jwt, + response.allowed_schemes) + return response + + class TokenView(views.TokenView): def _get_access_token_jwt(self, request, content): extra_data = {} @@ -28,16 +56,10 @@ def _get_access_token_jwt(self, request, content): if 'scope' in content: extra_data['scope'] = content['scope'] - username = request.POST.get('username') - if username: - # HACK: The only way to verify the username is to check the token. - # This means an extra wasted database call - token = get_access_token_model().objects.get( - token=content['access_token'] - ) - if token.user.get_username() != username: - raise WrongUsername() - extra_data['username'] = username + token = get_access_token_model().objects.get( + token=content['access_token'] + ) + extra_data['username'] = token.user.username payload = generate_payload(issuer, content['expires_in'], **extra_data) token = encode_jwt(payload) diff --git a/tests/test_views.py b/tests/test_views.py index 6f53c84..b5f205a 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -4,9 +4,10 @@ import re try: - from urllib.parse import urlencode + from urllib.parse import urlencode, urlparse, parse_qs except ImportError: - from urllib import urlencode # noqa + from urllib import urlencode # noqa + from urlparse import urlparse, parse_qs try: from unittest.mock import patch except ImportError: @@ -170,47 +171,37 @@ def test_get_token_authorization_code(self): self.assertEqual('test_user', payload_content['username']) self.assertEqual('read write', payload_content['scope']) - def test_get_token_authorization_code_wrong_user(self): + def test_get_token_implicit(self): """ - Fix for https://github.com/Humanitec/django-oauth-toolkit-jwt/issues/14 + Request an access token using Resource Owner Password Flow """ Application.objects.create( client_id='user_app_id', client_secret='user_app_secret', client_type=Application.CLIENT_CONFIDENTIAL, - authorization_grant_type=Application.GRANT_AUTHORIZATION_CODE, + authorization_grant_type=Application.GRANT_IMPLICIT, name='user app', skip_authorization=True, redirect_uris='http://localhost:8002/callback', ) - self.client.force_login(self.test_user) - response = self.client.get(reverse("oauth2_provider_jwt:authorize") + - '?response_type=code&client_id=user_app_id') + response = self.client.get( + reverse("oauth2_provider_jwt:authorize") + + '?response_type=token&client_id=user_app_id') self.assertEqual(302, response.status_code) - match = re.match(r'http://localhost:8002/callback\?code=(\w+)', - response.url) - self.assertIsNotNone(match) - code = match.group(1) + url = urlparse(response.url) + params = parse_qs(url.fragment) + self.assertEqual('Bearer', params['token_type'][0]) + self.assertEqual('read write', params['scope'][0]) - # To simulate that the token call is normally made unauthenticated - self.client.logout() - data = { - 'client_id': 'user_app_id', - 'client_secret': 'user_app_secret', - 'code': code, - 'grant_type': 'authorization_code', - 'redirect_uri': 'http://localhost:8002/callback', - 'username': 'some_fake_user', # Pass in wrong user - } - response = self.client.post(reverse("oauth2_provider_jwt:token"), data) - self.assertEqual(400, response.status_code) - self.assertEqual({ - "error": "invalid_request", - "error_description": "Request username doesn't match " - "username in original authorize", - }, response.json()) + self.assertTrue(params['access_token'][0]) + access_token_jwt = params['access_token_jwt'][0] + self.assertTrue(access_token_jwt) + + payload_content = self.decode_jwt(access_token_jwt) + self.assertEqual('test_user', payload_content['username']) + self.assertEqual('read write', payload_content['scope']) @patch('oauth2_provider_jwt.views.TokenView._is_jwt_config_set') def test_do_not_get_token_missing_conf(self, mock_is_jwt_config_set): From ee7345ace3c15b130ea05f1fc71450790f5f8377 Mon Sep 17 00:00:00 2001 From: Tobias Reese Date: Fri, 8 Feb 2019 03:31:17 +0100 Subject: [PATCH 2/3] Fixed Doc Header. --- tests/test_views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_views.py b/tests/test_views.py index b5f205a..dc9eb06 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -173,7 +173,7 @@ def test_get_token_authorization_code(self): def test_get_token_implicit(self): """ - Request an access token using Resource Owner Password Flow + Request an access token using Implicit Flow """ Application.objects.create( client_id='user_app_id', From f0bab462fa6359e2422ebfde547dc34e7dc0754f Mon Sep 17 00:00:00 2001 From: tobias-reese Date: Fri, 8 Feb 2019 13:11:05 +0100 Subject: [PATCH 3/3] Format Query URL composition --- oauth2_provider_jwt/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oauth2_provider_jwt/views.py b/oauth2_provider_jwt/views.py index d114259..b43f046 100644 --- a/oauth2_provider_jwt/views.py +++ b/oauth2_provider_jwt/views.py @@ -39,7 +39,7 @@ def get(self, request, *args, **kwargs): } jwt = TokenView()._get_access_token_jwt(request, content) response = OAuth2ResponseRedirect( - response.url + '&access_token_jwt=' + jwt, + '{}&access_token_jwt={}'.format(response.url, jwt), response.allowed_schemes) return response