From 1de68138a9afe1b368ef959a0aefc1fd3a69f107 Mon Sep 17 00:00:00 2001 From: Dylan Tack Date: Thu, 27 May 2021 19:06:22 -0700 Subject: [PATCH 1/2] Require redirect_uri if multiple uris are registered --- AUTHORS | 1 + oauth2_provider/models.py | 9 ++++++--- tests/test_authorization_code.py | 18 ++++++++++++++++++ tests/test_hybrid.py | 3 +++ tests/test_implicit.py | 2 ++ 5 files changed, 30 insertions(+), 3 deletions(-) diff --git a/AUTHORS b/AUTHORS index 50589287a..88491adea 100644 --- a/AUTHORS +++ b/AUTHORS @@ -25,6 +25,7 @@ David Smith Diego Garcia Dulmandakh Sukhbaatar Dylan Giesler +Dylan Tack Emanuele Palazzetti Federico Dolce Frederico Vieira diff --git a/oauth2_provider/models.py b/oauth2_provider/models.py index aa10eca16..d5d01374a 100644 --- a/oauth2_provider/models.py +++ b/oauth2_provider/models.py @@ -12,6 +12,7 @@ from django.utils.translation import gettext_lazy as _ from jwcrypto import jwk from jwcrypto.common import base64url_encode +from oauthlib.oauth2.rfc6749 import errors from .generators import generate_client_id, generate_client_secret from .scopes import get_scopes_backend @@ -107,11 +108,13 @@ def __str__(self): @property def default_redirect_uri(self): """ - Returns the default redirect_uri extracting the first item from - the :attr:`redirect_uris` string + Returns the default redirect_uri, *if* only one is registered. """ if self.redirect_uris: - return self.redirect_uris.split().pop(0) + uris = self.redirect_uris.split() + if len(uris) == 1: + return self.redirect_uris.split().pop(0) + raise errors.MissingRedirectURIError() assert False, ( "If you are using implicit, authorization_code" diff --git a/tests/test_authorization_code.py b/tests/test_authorization_code.py index ea1bee86d..c9bef0f5c 100644 --- a/tests/test_authorization_code.py +++ b/tests/test_authorization_code.py @@ -257,6 +257,8 @@ def test_pre_auth_default_redirect(self): Test for default redirect uri if omitted from query string with response_type: code """ self.client.login(username="test_user", password="123456") + self.application.redirect_uris = "http://localhost" + self.application.save() query_data = { "client_id": self.application.client_id, @@ -269,6 +271,21 @@ def test_pre_auth_default_redirect(self): form = response.context["form"] self.assertEqual(form["redirect_uri"].value(), "http://localhost") + def test_pre_auth_missing_redirect(self): + """ + Test response if redirect_uri is missing and multiple URIs are registered. + @see https://datatracker.ietf.org/doc/html/rfc6749#section-3.1.2.3 + """ + self.client.login(username="test_user", password="123456") + + query_data = { + "client_id": self.application.client_id, + "response_type": "code", + } + + response = self.client.get(reverse("oauth2_provider:authorize"), data=query_data) + self.assertEqual(response.status_code, 400) + def test_pre_auth_forbibben_redirect(self): """ Test error when passing a forbidden redirect_uri in query string with response_type: code @@ -293,6 +310,7 @@ def test_pre_auth_wrong_response_type(self): query_data = { "client_id": self.application.client_id, "response_type": "WRONG", + "redirect_uri": "http://example.org", } response = self.client.get(reverse("oauth2_provider:authorize"), data=query_data) diff --git a/tests/test_hybrid.py b/tests/test_hybrid.py index d198988f6..4f9753979 100644 --- a/tests/test_hybrid.py +++ b/tests/test_hybrid.py @@ -370,6 +370,8 @@ def test_pre_auth_default_redirect(self): Test for default redirect uri if omitted from query string with response_type: code """ self.client.login(username="hy_test_user", password="123456") + self.application.redirect_uris = "http://localhost" + self.application.save() query_string = urlencode( { @@ -413,6 +415,7 @@ def test_pre_auth_wrong_response_type(self): { "client_id": self.application.client_id, "response_type": "WRONG", + "redirect_uri": "http://example.org", } ) url = "{url}?{qs}".format(url=reverse("oauth2_provider:authorize"), qs=query_string) diff --git a/tests/test_implicit.py b/tests/test_implicit.py index a5863401c..5fcad62b0 100644 --- a/tests/test_implicit.py +++ b/tests/test_implicit.py @@ -110,6 +110,8 @@ def test_pre_auth_default_redirect(self): Test for default redirect uri if omitted from query string with response_type: token """ self.client.login(username="test_user", password="123456") + self.application.redirect_uris = "http://localhost" + self.application.save() query_data = { "client_id": self.application.client_id, From bc92f2fd7af950a8fc3b9635a84e5017db4e9e15 Mon Sep 17 00:00:00 2001 From: Dylan Tack Date: Thu, 27 May 2021 19:11:40 -0700 Subject: [PATCH 2/2] update changelog for #981 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b2deb05d..b82cfb218 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [unreleased] * Remove support for Django 3.0 * Add support for Django 3.2 +* #981 redirect_uri is now required in authorization requests when multiple URIs are registered. ### Added * #712, #636, #808. Calls to `django.contrib.auth.authenticate()` now pass a `request`