From 151dc67b922e4219d7e14f6ea08233e0a48457a3 Mon Sep 17 00:00:00 2001 From: Marnanel Thurman Date: Mon, 30 Dec 2019 23:46:19 +0000 Subject: [PATCH 1/8] First draft of a possible fix for issue #235, adding support for urn:ietf:wg:oauth:2.0:oob and urn:ietf:wg:oauth:2.0:oob:auto redirect URLs. --- .../oauth2_provider/authorized-oob.html | 18 +++ oauth2_provider/views/base.py | 24 +++- tests/test_oob_authorization.py | 108 ++++++++++++++++++ 3 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 oauth2_provider/templates/oauth2_provider/authorized-oob.html create mode 100644 tests/test_oob_authorization.py diff --git a/oauth2_provider/templates/oauth2_provider/authorized-oob.html b/oauth2_provider/templates/oauth2_provider/authorized-oob.html new file mode 100644 index 000000000..b1e11923b --- /dev/null +++ b/oauth2_provider/templates/oauth2_provider/authorized-oob.html @@ -0,0 +1,18 @@ +{% extends "oauth2_provider/base.html" %} + +{% load i18n %} +{% block content %} +
+ {% if not error %} +

{% trans "Success" %}

+ +

{% trans "Please return to your application and enter this code:" %}

+ +

{{ code }}

+ + {% else %} +

Error: {{ error.error }}

+

{{ error.description }}

+ {% endif %} +
+{% endblock %} diff --git a/oauth2_provider/views/base.py b/oauth2_provider/views/base.py index e236f9064..96c817229 100644 --- a/oauth2_provider/views/base.py +++ b/oauth2_provider/views/base.py @@ -1,13 +1,15 @@ import json import logging +import urllib.parse from django.contrib.auth.mixins import LoginRequiredMixin -from django.http import HttpResponse +from django.http import HttpResponse, JsonResponse from django.utils import timezone from django.utils.decorators import method_decorator from django.views.decorators.csrf import csrf_exempt from django.views.decorators.debug import sensitive_post_parameters from django.views.generic import FormView, View +from django.shortcuts import render from ..exceptions import OAuthToolkitError from ..forms import AllowForm @@ -211,6 +213,26 @@ def get(self, request, *args, **kwargs): return self.render_to_response(self.get_context_data(**kwargs)) + def redirect(self, redirect_to, application): + + if not redirect_to.startswith("urn:ietf:wg:oauth:2.0:oob"): + return super().redirect(redirect_to, application) + + parsed_redirect = urllib.parse.urlparse(redirect_to) + code = urllib.parse.parse_qs(parsed_redirect.query)['code'][0] + + if redirect_to.startswith('urn:ietf:wg:oauth:2.0:oob:auto'): + return JsonResponse({ + 'access_token': code, + }) + else: + return render( + request=self.request, + template_name="oauth2_provider/authorized-oob.html", + context={ + 'code': code, + }, + ) @method_decorator(csrf_exempt, name="dispatch") class TokenView(OAuthLibMixin, View): diff --git a/tests/test_oob_authorization.py b/tests/test_oob_authorization.py new file mode 100644 index 000000000..fbfe9f56c --- /dev/null +++ b/tests/test_oob_authorization.py @@ -0,0 +1,108 @@ +import base64 +import datetime +import hashlib +import json +import re +from urllib.parse import parse_qs, urlencode, urlparse + +from django.contrib.auth import get_user_model +from django.test import RequestFactory, TestCase +from django.urls import reverse +from django.utils import timezone +from django.utils.crypto import get_random_string +from oauthlib.oauth2.rfc6749 import errors as oauthlib_errors + +from oauth2_provider.models import ( + get_access_token_model, get_application_model, + get_grant_model, get_refresh_token_model +) +from oauth2_provider.settings import oauth2_settings + +from .utils import get_basic_auth_header + +Application = get_application_model() +AccessToken = get_access_token_model() +Grant = get_grant_model() +RefreshToken = get_refresh_token_model() +UserModel = get_user_model() + +class BaseTest(TestCase): + def setUp(self): + self.factory = RequestFactory() + self.test_user = UserModel.objects.create_user("test_user", "test@example.com", "123456") + self.dev_user = UserModel.objects.create_user("dev_user", "dev@example.com", "123456") + + oauth2_settings.ALLOWED_REDIRECT_URI_SCHEMES = ["http", "custom-scheme", "urn"] + + self.application = Application.objects.create( + name="Test Application", + redirect_uris="urn:ietf:wg:oauth:2.0:oob urn:ietf:wg:oauth:2.0:oob:auto", + user=self.dev_user, + client_type=Application.CLIENT_CONFIDENTIAL, + authorization_grant_type=Application.GRANT_AUTHORIZATION_CODE, + ) + + oauth2_settings._SCOPES = ["read", "write"] + oauth2_settings._DEFAULT_SCOPES = ["read", "write"] + + def tearDown(self): + self.application.delete() + self.test_user.delete() + self.dev_user.delete() + +class TestOobAuthorizationCodeView(BaseTest): + def test_oob_as_html(self): + """ + ... + """ + self.client.login(username="test_user", password="123456") + self.application.skip_authorization = True + self.application.save() + + query_string = urlencode({ + "client_id": self.application.client_id, + "response_type": "code", + "state": "random_state_string", + "scope": "read write", + "redirect_uri": "urn:ietf:wg:oauth:2.0:oob", + }) + url = "{url}?{qs}".format(url=reverse("oauth2_provider:authorize"), qs=query_string) + + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + + content = str(response.content, encoding='UTF-8') + + code_matches = re.search(r'([^<]*)', content) + code = code_matches.groups(0) + self.assertNotEqual(code, '') + + with open('/tmp/fred.html', 'w') as f: + f.write(str(response.content, encoding='UTF-8')) + + def test_oob_as_json(self): + """ + ... + """ + self.client.login(username="test_user", password="123456") + self.application.skip_authorization = True + self.application.save() + + query_string = urlencode({ + "client_id": self.application.client_id, + "response_type": "code", + "state": "random_state_string", + "scope": "read write", + "redirect_uri": "urn:ietf:wg:oauth:2.0:oob:auto", + }) + url = "{url}?{qs}".format(url=reverse("oauth2_provider:authorize"), qs=query_string) + + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + + content = json.loads(str(response.content, encoding='UTF-8')) + + self.assertIn('access_token', content) + + with open('/tmp/fred.json', 'w') as f: + f.write(str(response.content, encoding='UTF-8')) From 8857af44c69a35888661c854bcd72bff79cd1968 Mon Sep 17 00:00:00 2001 From: Marnanel Thurman Date: Tue, 31 Dec 2019 17:45:07 +0000 Subject: [PATCH 2/8] rm debug code --- tests/test_oob_authorization.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/test_oob_authorization.py b/tests/test_oob_authorization.py index fbfe9f56c..0add89836 100644 --- a/tests/test_oob_authorization.py +++ b/tests/test_oob_authorization.py @@ -77,9 +77,6 @@ def test_oob_as_html(self): code = code_matches.groups(0) self.assertNotEqual(code, '') - with open('/tmp/fred.html', 'w') as f: - f.write(str(response.content, encoding='UTF-8')) - def test_oob_as_json(self): """ ... @@ -103,6 +100,3 @@ def test_oob_as_json(self): content = json.loads(str(response.content, encoding='UTF-8')) self.assertIn('access_token', content) - - with open('/tmp/fred.json', 'w') as f: - f.write(str(response.content, encoding='UTF-8')) From d16d3fcda6aaa24bace52fcf615a0bdbeb993a1f Mon Sep 17 00:00:00 2001 From: Marnanel Thurman Date: Mon, 6 Jan 2020 18:49:41 +0000 Subject: [PATCH 3/8] Added extra fields for the out-of-band JSON response. See https://github.com/googleapis/oauth2client/blob/master/oauth2client/client.py#L618 for more information about these fields. --- oauth2_provider/views/base.py | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/oauth2_provider/views/base.py b/oauth2_provider/views/base.py index 96c817229..fc29aa475 100644 --- a/oauth2_provider/views/base.py +++ b/oauth2_provider/views/base.py @@ -10,6 +10,7 @@ from django.views.decorators.debug import sensitive_post_parameters from django.views.generic import FormView, View from django.shortcuts import render +from django.urls import reverse from ..exceptions import OAuthToolkitError from ..forms import AllowForm @@ -20,7 +21,6 @@ from ..signals import app_authorized from .mixins import OAuthLibMixin - log = logging.getLogger("oauth2_provider") @@ -61,6 +61,7 @@ def redirect(self, redirect_to, application): allowed_schemes = application.get_allowed_schemes() return OAuth2ResponseRedirect(redirect_to, allowed_schemes) +RFC3339 = '%Y-%m-%dT%H:%M:%SZ' class AuthorizationView(BaseAuthorizationView, FormView): """ @@ -206,14 +207,15 @@ def get(self, request, *args, **kwargs): request=self.request, scopes=" ".join(scopes), credentials=credentials, allow=True ) - return self.redirect(uri, application) + return self.redirect(uri, application, token) except OAuthToolkitError as error: return self.error_response(error, application) return self.render_to_response(self.get_context_data(**kwargs)) - def redirect(self, redirect_to, application): + def redirect(self, redirect_to, application, + token = None): if not redirect_to.startswith("urn:ietf:wg:oauth:2.0:oob"): return super().redirect(redirect_to, application) @@ -222,9 +224,25 @@ def redirect(self, redirect_to, application): code = urllib.parse.parse_qs(parsed_redirect.query)['code'][0] if redirect_to.startswith('urn:ietf:wg:oauth:2.0:oob:auto'): - return JsonResponse({ - 'access_token': code, - }) + + response = { + 'access_token': code, + 'token_uri': redirect_to, + 'refresh_token': None, + 'token_expiry': None, + 'token_uri': reverse('oauth2_provider:token'), + 'user_agent': '', + 'client_id': application.client_id, + 'client_secret': application.client_secret, + 'revoke_uri': reverse('oauth2_provider:revoke-token'), + } + + if token is not None: + response['refresh_token'] = token.token + response['token_expiry'] = token.expires.format(RFC3339) + + return JsonResponse(response) + else: return render( request=self.request, From fe47a4b9901f5f58d4efff64f578819e927b63ab Mon Sep 17 00:00:00 2001 From: Marnanel Thurman Date: Mon, 6 Jan 2020 19:04:23 +0000 Subject: [PATCH 4/8] basic tests for the new JSON oob response fields --- tests/test_oob_authorization.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/test_oob_authorization.py b/tests/test_oob_authorization.py index 0add89836..c916332e1 100644 --- a/tests/test_oob_authorization.py +++ b/tests/test_oob_authorization.py @@ -99,4 +99,11 @@ def test_oob_as_json(self): content = json.loads(str(response.content, encoding='UTF-8')) - self.assertIn('access_token', content) + for field in [ + 'access_token', 'token_uri', + 'refresh_token', 'token_expiry', + 'token_uri', 'user_agent', + 'client_id', 'client_secret', + 'revoke_uri', + ]: + self.assertIn(field, content) From cc82c972eca04f8695baff5131598dffb12210a7 Mon Sep 17 00:00:00 2001 From: Marnanel Thurman Date: Thu, 16 Jan 2020 10:28:28 +0000 Subject: [PATCH 5/8] Add brief docstrings to OOB tests --- tests/test_oob_authorization.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_oob_authorization.py b/tests/test_oob_authorization.py index c916332e1..2289d244b 100644 --- a/tests/test_oob_authorization.py +++ b/tests/test_oob_authorization.py @@ -53,7 +53,7 @@ def tearDown(self): class TestOobAuthorizationCodeView(BaseTest): def test_oob_as_html(self): """ - ... + Test out-of-band authentication, with a legacy HTML response. """ self.client.login(username="test_user", password="123456") self.application.skip_authorization = True @@ -79,7 +79,7 @@ def test_oob_as_html(self): def test_oob_as_json(self): """ - ... + Test out-of-band authentication, with a JSON response. """ self.client.login(username="test_user", password="123456") self.application.skip_authorization = True From f571dd686be177566f0bbdfd51ce855b28c85a19 Mon Sep 17 00:00:00 2001 From: Marnanel Thurman Date: Tue, 21 Jan 2020 21:26:39 +0000 Subject: [PATCH 6/8] Add auth code to the tag for OOB requests --- .../templates/oauth2_provider/authorized-oob.html | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/oauth2_provider/templates/oauth2_provider/authorized-oob.html b/oauth2_provider/templates/oauth2_provider/authorized-oob.html index b1e11923b..78399da7c 100644 --- a/oauth2_provider/templates/oauth2_provider/authorized-oob.html +++ b/oauth2_provider/templates/oauth2_provider/authorized-oob.html @@ -1,6 +1,11 @@ {% extends "oauth2_provider/base.html" %} {% load i18n %} + +{% block title %} +Success code={{code}} +{% endblock %} + {% block content %} <div class="block-center"> {% if not error %} From 3d1488b0a6aeac0a8f7bbbbc9f2cbc2af27315c2 Mon Sep 17 00:00:00 2001 From: Marnanel Thurman <marnanel@thurman.org.uk> Date: Tue, 21 Jan 2020 21:30:21 +0000 Subject: [PATCH 7/8] Remove token fields from the oob:auto JSON response: it's not used when we're returning a token --- oauth2_provider/views/base.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/oauth2_provider/views/base.py b/oauth2_provider/views/base.py index fc29aa475..41c2a6c67 100644 --- a/oauth2_provider/views/base.py +++ b/oauth2_provider/views/base.py @@ -228,19 +228,11 @@ def redirect(self, redirect_to, application, response = { 'access_token': code, 'token_uri': redirect_to, - 'refresh_token': None, - 'token_expiry': None, - 'token_uri': reverse('oauth2_provider:token'), - 'user_agent': '', 'client_id': application.client_id, 'client_secret': application.client_secret, 'revoke_uri': reverse('oauth2_provider:revoke-token'), } - if token is not None: - response['refresh_token'] = token.token - response['token_expiry'] = token.expires.format(RFC3339) - return JsonResponse(response) else: From 24ca06c753de30ffe8cf7c2b13256eeed78242c0 Mon Sep 17 00:00:00 2001 From: Marnanel Thurman <marnanel@thurman.org.uk> Date: Tue, 21 Jan 2020 21:58:43 +0000 Subject: [PATCH 8/8] test_oob_as_html() and test_oob_as_json() tests added to test_authorization_code.py. test_oob_authorization.py, which contained earlier versions of these, deleted. --- tests/test_authorization_code.py | 92 ++++++++++++++++++++++++++ tests/test_oob_authorization.py | 109 ------------------------------- 2 files changed, 92 insertions(+), 109 deletions(-) delete mode 100644 tests/test_oob_authorization.py diff --git a/tests/test_authorization_code.py b/tests/test_authorization_code.py index 45116dad6..69dcfd93a 100644 --- a/tests/test_authorization_code.py +++ b/tests/test_authorization_code.py @@ -2,6 +2,7 @@ import datetime import hashlib import json +import re from urllib.parse import parse_qs, urlencode, urlparse from django.contrib.auth import get_user_model @@ -27,6 +28,8 @@ RefreshToken = get_refresh_token_model() UserModel = get_user_model() +URI_OOB = "urn:ietf:wg:oauth:2.0:oob" +URI_OOB_AUTO = "urn:ietf:wg:oauth:2.0:oob:auto" # mocking a protected resource view class ResourceView(ProtectedResourceView): @@ -46,6 +49,7 @@ def setUp(self): name="Test Application", redirect_uris=( "http://localhost http://example.com http://example.org custom-scheme://example.com" + " " + URI_OOB + " " + URI_OOB_AUTO ), user=self.dev_user, client_type=Application.CLIENT_CONFIDENTIAL, @@ -1456,6 +1460,94 @@ def test_code_exchange_succeed_when_redirect_uri_match_with_multiple_query_param self.assertEqual(content["scope"], "read write") self.assertEqual(content["expires_in"], oauth2_settings.ACCESS_TOKEN_EXPIRE_SECONDS) + def test_oob_as_html(self): + """ + Test out-of-band authentication. + """ + self.client.login(username="test_user", password="123456") + + authcode_data = { + "client_id": self.application.client_id, + "state": "random_state_string", + "scope": "read write", + "redirect_uri": URI_OOB, + "response_type": "code", + "allow": True, + } + + response = self.client.post(reverse("oauth2_provider:authorize"), data=authcode_data) + self.assertEqual(response.status_code, 200) + self.assertRegex(response['Content-Type'], r'^text/html') + + content = response.content.decode("utf-8") + + # "A lot of applications, for legacy reasons, use this and regex + # to extract the token, risking summoning zalgo in the process." + # -- https://github.com/jazzband/django-oauth-toolkit/issues/235 + + matches = re.search(r'.*<code>([^<>]*)</code>', + content) + self.assertIsNotNone(matches, + msg="OOB response contains code inside <code> tag") + self.assertEqual(len(matches.groups()), 1, + msg="OOB response contains multiple <code> tags") + authorization_code = matches.groups()[0] + + token_request_data = { + "grant_type": "authorization_code", + "code": authorization_code, + "redirect_uri": URI_OOB, + "client_id": self.application.client_id, + "client_secret": self.application.client_secret, + } + + response = self.client.post(reverse("oauth2_provider:token"), data=token_request_data) + self.assertEqual(response.status_code, 200) + + content = json.loads(response.content.decode("utf-8")) + self.assertEqual(content["token_type"], "Bearer") + self.assertEqual(content["scope"], "read write") + self.assertEqual(content["expires_in"], oauth2_settings.ACCESS_TOKEN_EXPIRE_SECONDS) + + def test_oob_as_json(self): + """ + Test out-of-band authentication, with a JSON response. + """ + self.client.login(username="test_user", password="123456") + + authcode_data = { + "client_id": self.application.client_id, + "state": "random_state_string", + "scope": "read write", + "redirect_uri": URI_OOB_AUTO, + "response_type": "code", + "allow": True, + } + + response = self.client.post(reverse("oauth2_provider:authorize"), data=authcode_data) + self.assertEqual(response.status_code, 200) + self.assertRegex(response['Content-Type'], '^application/json') + + parsed_response = json.loads(response.content.decode("utf-8")) + + self.assertIn('access_token', parsed_response) + authorization_code = parsed_response['access_token'] + + token_request_data = { + "grant_type": "authorization_code", + "code": authorization_code, + "redirect_uri": URI_OOB_AUTO, + "client_id": self.application.client_id, + "client_secret": self.application.client_secret, + } + + response = self.client.post(reverse("oauth2_provider:token"), data=token_request_data) + self.assertEqual(response.status_code, 200) + + content = json.loads(response.content.decode("utf-8")) + self.assertEqual(content["token_type"], "Bearer") + self.assertEqual(content["scope"], "read write") + self.assertEqual(content["expires_in"], oauth2_settings.ACCESS_TOKEN_EXPIRE_SECONDS) class TestAuthorizationCodeProtectedResource(BaseTest): def test_resource_access_allowed(self): diff --git a/tests/test_oob_authorization.py b/tests/test_oob_authorization.py deleted file mode 100644 index 2289d244b..000000000 --- a/tests/test_oob_authorization.py +++ /dev/null @@ -1,109 +0,0 @@ -import base64 -import datetime -import hashlib -import json -import re -from urllib.parse import parse_qs, urlencode, urlparse - -from django.contrib.auth import get_user_model -from django.test import RequestFactory, TestCase -from django.urls import reverse -from django.utils import timezone -from django.utils.crypto import get_random_string -from oauthlib.oauth2.rfc6749 import errors as oauthlib_errors - -from oauth2_provider.models import ( - get_access_token_model, get_application_model, - get_grant_model, get_refresh_token_model -) -from oauth2_provider.settings import oauth2_settings - -from .utils import get_basic_auth_header - -Application = get_application_model() -AccessToken = get_access_token_model() -Grant = get_grant_model() -RefreshToken = get_refresh_token_model() -UserModel = get_user_model() - -class BaseTest(TestCase): - def setUp(self): - self.factory = RequestFactory() - self.test_user = UserModel.objects.create_user("test_user", "test@example.com", "123456") - self.dev_user = UserModel.objects.create_user("dev_user", "dev@example.com", "123456") - - oauth2_settings.ALLOWED_REDIRECT_URI_SCHEMES = ["http", "custom-scheme", "urn"] - - self.application = Application.objects.create( - name="Test Application", - redirect_uris="urn:ietf:wg:oauth:2.0:oob urn:ietf:wg:oauth:2.0:oob:auto", - user=self.dev_user, - client_type=Application.CLIENT_CONFIDENTIAL, - authorization_grant_type=Application.GRANT_AUTHORIZATION_CODE, - ) - - oauth2_settings._SCOPES = ["read", "write"] - oauth2_settings._DEFAULT_SCOPES = ["read", "write"] - - def tearDown(self): - self.application.delete() - self.test_user.delete() - self.dev_user.delete() - -class TestOobAuthorizationCodeView(BaseTest): - def test_oob_as_html(self): - """ - Test out-of-band authentication, with a legacy HTML response. - """ - self.client.login(username="test_user", password="123456") - self.application.skip_authorization = True - self.application.save() - - query_string = urlencode({ - "client_id": self.application.client_id, - "response_type": "code", - "state": "random_state_string", - "scope": "read write", - "redirect_uri": "urn:ietf:wg:oauth:2.0:oob", - }) - url = "{url}?{qs}".format(url=reverse("oauth2_provider:authorize"), qs=query_string) - - response = self.client.get(url) - self.assertEqual(response.status_code, 200) - - content = str(response.content, encoding='UTF-8') - - code_matches = re.search(r'<code>([^<]*)</code>', content) - code = code_matches.groups(0) - self.assertNotEqual(code, '') - - def test_oob_as_json(self): - """ - Test out-of-band authentication, with a JSON response. - """ - self.client.login(username="test_user", password="123456") - self.application.skip_authorization = True - self.application.save() - - query_string = urlencode({ - "client_id": self.application.client_id, - "response_type": "code", - "state": "random_state_string", - "scope": "read write", - "redirect_uri": "urn:ietf:wg:oauth:2.0:oob:auto", - }) - url = "{url}?{qs}".format(url=reverse("oauth2_provider:authorize"), qs=query_string) - - response = self.client.get(url) - self.assertEqual(response.status_code, 200) - - content = json.loads(str(response.content, encoding='UTF-8')) - - for field in [ - 'access_token', 'token_uri', - 'refresh_token', 'token_expiry', - 'token_uri', 'user_agent', - 'client_id', 'client_secret', - 'revoke_uri', - ]: - self.assertIn(field, content)