From 387415e85c9330454c257020e94b79260017fbd5 Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Fri, 18 Mar 2022 16:25:02 -0400 Subject: [PATCH 1/2] Remove OOB --- CHANGELOG.md | 6 ++ .../oauth2_provider/authorized-oob.html | 23 ----- oauth2_provider/views/base.py | 36 +------- tests/test_authorization_code.py | 90 ------------------- 4 files changed, 8 insertions(+), 147 deletions(-) delete mode 100644 oauth2_provider/templates/oauth2_provider/authorized-oob.html diff --git a/CHANGELOG.md b/CHANGELOG.md index 23035d0b2..d8c2fa20a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed * #1108 OIDC: Fix `validate_bearer_token()` to properly set `request.scopes` to the list of granted scopes. +### Removed +* #TBD (**Breaking**) Removes support for insecure `urn:ietf:wg:oauth:2.0:oob` and `urn:ietf:wg:oauth:2.0:oob:auto` which are replaced + by [RFC 8252](https://datatracker.ietf.org/doc/html/rfc8252) "OAuth 2.0 for Native Apps" BCP. Google has + [deprecated use of oob](https://developers.googleblog.com/2022/02/making-oauth-flows-safer.html?m=1#disallowed-oob) with + a final end date of 2022-10-03. If you still rely on oob support in django-oauth-toolkit, do not upgrade to this release. + ## [1.7.0] 2022-01-23 ### Added diff --git a/oauth2_provider/templates/oauth2_provider/authorized-oob.html b/oauth2_provider/templates/oauth2_provider/authorized-oob.html deleted file mode 100644 index 78399da7c..000000000 --- a/oauth2_provider/templates/oauth2_provider/authorized-oob.html +++ /dev/null @@ -1,23 +0,0 @@ -{% extends "oauth2_provider/base.html" %} - -{% load i18n %} - -{% block title %} -Success code={{code}} -{% endblock %} - -{% 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 e46a49d10..211da45ed 100644 --- a/oauth2_provider/views/base.py +++ b/oauth2_provider/views/base.py @@ -1,11 +1,8 @@ import json import logging -import urllib.parse from django.contrib.auth.mixins import LoginRequiredMixin -from django.http import HttpResponse, JsonResponse -from django.shortcuts import render -from django.urls import reverse +from django.http import HttpResponse from django.utils import timezone from django.utils.decorators import method_decorator from django.views.decorators.csrf import csrf_exempt @@ -207,42 +204,13 @@ def get(self, request, *args, **kwargs): credentials=credentials, allow=True, ) - return self.redirect(uri, application, token) + return self.redirect(uri, application) 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, token=None): - - 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"): - - response = { - "access_token": code, - "token_uri": redirect_to, - "client_id": application.client_id, - "client_secret": application.client_secret, - "revoke_uri": reverse("oauth2_provider:revoke-token"), - } - - return JsonResponse(response) - - 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_authorization_code.py b/tests/test_authorization_code.py index 91fd06bd1..25447b9dd 100644 --- a/tests/test_authorization_code.py +++ b/tests/test_authorization_code.py @@ -2,7 +2,6 @@ import datetime import hashlib import json -import re from urllib.parse import parse_qs, urlparse import pytest @@ -32,8 +31,6 @@ 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" CLEARTEXT_SECRET = "1234567890abcdefghijklmnopqrstuvwxyz" @@ -56,7 +53,6 @@ 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, @@ -1532,92 +1528,6 @@ def test_code_exchange_succeed_when_redirect_uri_match_with_multiple_query_param self.assertEqual(content["scope"], "read write") self.assertEqual(content["expires_in"], self.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".*([^<>]*)", content) - self.assertIsNotNone(matches, msg="OOB response contains code inside tag") - self.assertEqual(len(matches.groups()), 1, msg="OOB response contains multiple 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": CLEARTEXT_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"], self.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": CLEARTEXT_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"], self.oauth2_settings.ACCESS_TOKEN_EXPIRE_SECONDS) - @pytest.mark.oauth2_settings(presets.OIDC_SETTINGS_RW) class TestOIDCAuthorizationCodeTokenView(BaseAuthorizationCodeTokenView): From b5188ee22fcfbd62fdd6611b9f790da75025498c Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Fri, 18 Mar 2022 16:49:32 -0400 Subject: [PATCH 2/2] Indicate that this is a security fix. --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8c2fa20a..c3b10068b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,7 +37,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * #1108 OIDC: Fix `validate_bearer_token()` to properly set `request.scopes` to the list of granted scopes. ### Removed -* #TBD (**Breaking**) Removes support for insecure `urn:ietf:wg:oauth:2.0:oob` and `urn:ietf:wg:oauth:2.0:oob:auto` which are replaced +* #1124 (**Breaking**, **Security**) Removes support for insecure `urn:ietf:wg:oauth:2.0:oob` and `urn:ietf:wg:oauth:2.0:oob:auto` which are replaced by [RFC 8252](https://datatracker.ietf.org/doc/html/rfc8252) "OAuth 2.0 for Native Apps" BCP. Google has [deprecated use of oob](https://developers.googleblog.com/2022/02/making-oauth-flows-safer.html?m=1#disallowed-oob) with a final end date of 2022-10-03. If you still rely on oob support in django-oauth-toolkit, do not upgrade to this release.