From a57e118b23b109dffb7df01df752e7180e8908f3 Mon Sep 17 00:00:00 2001 From: John Wood Date: Thu, 1 Nov 2018 11:09:18 +0000 Subject: [PATCH 1/3] Potential fix for username faking --- oauth2_provider_jwt/views.py | 39 +++++++++++---- tests/test_views.py | 93 ++++++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 8 deletions(-) diff --git a/oauth2_provider_jwt/views.py b/oauth2_provider_jwt/views.py index 5c7d669..1d6b395 100644 --- a/oauth2_provider_jwt/views.py +++ b/oauth2_provider_jwt/views.py @@ -5,12 +5,17 @@ from django.conf import settings from django.utils.module_loading import import_string from oauth2_provider import views +from oauth2_provider.models import get_access_token_model from .utils import generate_payload, encode_jwt logger = logging.getLogger(__name__) +class WrongUsername(Exception): + pass + + class TokenView(views.TokenView): def _get_access_token_jwt(self, request, content): extra_data = {} @@ -23,8 +28,17 @@ def _get_access_token_jwt(self, request, content): if 'scope' in content: extra_data['scope'] = content['scope'] - if request.POST.get('username'): - extra_data['username'] = request.POST.get('username') + 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 + payload = generate_payload(issuer, content['expires_in'], **extra_data) token = encode_jwt(payload) return token @@ -41,17 +55,26 @@ def _is_jwt_config_set(): def post(self, request, *args, **kwargs): response = super(TokenView, self).post(request, *args, **kwargs) + content = ast.literal_eval(response.content.decode("utf-8")) if response.status_code == 200 and 'access_token' in content: if not TokenView._is_jwt_config_set(): logger.warning( 'Missing JWT configuration, skipping token build') else: - content['access_token_jwt'] = self._get_access_token_jwt( - request, content) try: - content = bytes(json.dumps(content), 'utf-8') - except TypeError: - content = bytes(json.dumps(content).encode("utf-8")) - response.content = content + content['access_token_jwt'] = self._get_access_token_jwt( + request, content) + try: + content = bytes(json.dumps(content), 'utf-8') + except TypeError: + content = bytes(json.dumps(content).encode("utf-8")) + response.content = content + except WrongUsername: + response.status_code = 400 + response.content = json.dumps({ + "error": "invalid_request", + "error_description": "Request username doesn't match" + "username in original authorize", + }) return response diff --git a/tests/test_views.py b/tests/test_views.py index 212a7ec..b531d24 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -1,6 +1,7 @@ import base64 import datetime import json +import re try: from urllib.parse import urlencode @@ -119,6 +120,98 @@ def test_get_token(self): self.assertDictContainsSubset({'scope': 'read write'}, self.decode_jwt(jwt_token)) + def test_get_token_authorization_code(self): + """ + Request an access token using Authorization Code 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, + 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') + + 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) + + # 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': 'test_user', + } + response = self.client.post(reverse("oauth2_provider_jwt:token"), data) + self.assertEqual(200, response.status_code) + json_obj = response.json() + self.assertEqual('Bearer', json_obj['token_type']) + self.assertEqual('read write', json_obj['scope']) + + access_token = json_obj['access_token'] + self.assertTrue(access_token) + access_token_jwt = json_obj['access_token_jwt'] + 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']) + + def test_get_token_authorization_code_wrong_user(self): + """ + Fix for https://github.com/Humanitec/django-oauth-toolkit-jwt/issues/14 + """ + 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, + 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') + + 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) + + # 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()) + @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 bf7f40b3cdf7dcec50bbf3caa9516afd3af6b703 Mon Sep 17 00:00:00 2001 From: John Wood Date: Thu, 1 Nov 2018 11:13:59 +0000 Subject: [PATCH 2/3] Fix extra line --- oauth2_provider_jwt/views.py | 1 - 1 file changed, 1 deletion(-) diff --git a/oauth2_provider_jwt/views.py b/oauth2_provider_jwt/views.py index 1d6b395..b5fad47 100644 --- a/oauth2_provider_jwt/views.py +++ b/oauth2_provider_jwt/views.py @@ -55,7 +55,6 @@ def _is_jwt_config_set(): def post(self, request, *args, **kwargs): response = super(TokenView, self).post(request, *args, **kwargs) - content = ast.literal_eval(response.content.decode("utf-8")) if response.status_code == 200 and 'access_token' in content: if not TokenView._is_jwt_config_set(): From 13ec9b958b400cb487dc8b07d47a67de1772a53a Mon Sep 17 00:00:00 2001 From: John Wood Date: Thu, 1 Nov 2018 11:15:37 +0000 Subject: [PATCH 3/3] Fix typo --- oauth2_provider_jwt/views.py | 2 +- tests/test_views.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/oauth2_provider_jwt/views.py b/oauth2_provider_jwt/views.py index b5fad47..a0e464f 100644 --- a/oauth2_provider_jwt/views.py +++ b/oauth2_provider_jwt/views.py @@ -73,7 +73,7 @@ def post(self, request, *args, **kwargs): response.status_code = 400 response.content = json.dumps({ "error": "invalid_request", - "error_description": "Request username doesn't match" + "error_description": "Request username doesn't match " "username in original authorize", }) return response diff --git a/tests/test_views.py b/tests/test_views.py index b531d24..6f53c84 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -208,7 +208,7 @@ def test_get_token_authorization_code_wrong_user(self): self.assertEqual(400, response.status_code) self.assertEqual({ "error": "invalid_request", - "error_description": "Request username doesn't match" + "error_description": "Request username doesn't match " "username in original authorize", }, response.json())