From 36121c58baa3febfb642e3946dbfd96d72256020 Mon Sep 17 00:00:00 2001 From: Andy Zickler Date: Thu, 9 Nov 2023 17:42:37 -0500 Subject: [PATCH] fix: prompt=none redirects to login screen fixes #1268 --- AUTHORS | 1 + CHANGELOG.md | 5 +-- oauth2_provider/views/base.py | 27 +++++++++++++++ tests/app/idp/idp/oauth.py | 40 +++++++++++++++++++++++ tests/app/idp/idp/settings.py | 1 + tests/test_authorization_code.py | 56 ++++++++++++++++++++++++++++++++ 6 files changed, 128 insertions(+), 2 deletions(-) create mode 100644 tests/app/idp/idp/oauth.py diff --git a/AUTHORS b/AUTHORS index 689ab48de..8596063b9 100644 --- a/AUTHORS +++ b/AUTHORS @@ -23,6 +23,7 @@ Allisson Azevedo Andrea Greco Andrej Zbín Andrew Chen Wang +Andrew Zickler Antoine Laurent Anvesh Agarwal Aristóbulo Meneses diff --git a/CHANGELOG.md b/CHANGELOG.md index 28125afa3..d1e9704d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,8 +26,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * #1311 Add option to disable client_secret hashing to allow verifying JWTs' signatures. * #1337 Gracefully handle expired or deleted refresh tokens, in `validate_user`. * #1350 Support Python 3.12 and Django 5.0 -* #1249 Add code_challenge_methods_supported property to auto discovery informations - per [RFC 8414 section 2](https://www.rfc-editor.org/rfc/rfc8414.html#page-7) +* #1249 Add code_challenge_methods_supported property to auto discovery informations, per [RFC 8414 section 2](https://www.rfc-editor.org/rfc/rfc8414.html#page-7) + ### Fixed * #1322 Instructions in documentation on how to create a code challenge and code verifier @@ -35,6 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * #1296 Added reverse function in migration 0006_alter_application_client_secret * #1336 Fix encapsulation for Redirect URI scheme validation * #1357 Move import of setting_changed signal from test to django core modules +* #1268 fix prompt=none redirects to login screen ### Removed * #1350 Remove support for Python 3.7 and Django 2.2 diff --git a/oauth2_provider/views/base.py b/oauth2_provider/views/base.py index abaa81f59..846be3e73 100644 --- a/oauth2_provider/views/base.py +++ b/oauth2_provider/views/base.py @@ -244,6 +244,33 @@ def handle_prompt_login(self): self.get_redirect_field_name(), ) + def handle_no_permission(self): + """ + Generate response for unauthorized users. + + If prompt is set to none, then we redirect with an error code + as defined by OIDC 3.1.2.6 + + Some code copied from OAuthLibMixin.error_response, but that is designed + to operated on OAuth1Error from oauthlib wrapped in a OAuthToolkitError + """ + prompt = self.request.GET.get("prompt") + redirect_uri = self.request.GET.get("redirect_uri") + if prompt == "none" and redirect_uri: + response_parameters = {"error": "login_required"} + + # REQUIRED if the Authorization Request included the state parameter. + # Set to the value received from the Client + state = self.request.GET.get("state") + if state: + response_parameters["state"] = state + + separator = "&" if "?" in redirect_uri else "?" + redirect_to = redirect_uri + separator + urlencode(response_parameters) + return self.redirect(redirect_to, application=None) + else: + return super().handle_no_permission() + @method_decorator(csrf_exempt, name="dispatch") class TokenView(OAuthLibMixin, View): diff --git a/tests/app/idp/idp/oauth.py b/tests/app/idp/idp/oauth.py new file mode 100644 index 000000000..3e8a4645e --- /dev/null +++ b/tests/app/idp/idp/oauth.py @@ -0,0 +1,40 @@ +from django.conf import settings +from django.contrib.auth.middleware import AuthenticationMiddleware +from django.contrib.sessions.middleware import SessionMiddleware + +from oauth2_provider.oauth2_validators import OAuth2Validator + + +# get_response is required for middlware, it doesn't need to do anything +# the way we're using it, so we just use a lambda that returns None +def get_response(): + None + + +class CustomOAuth2Validator(OAuth2Validator): + def validate_silent_login(self, request) -> None: + # request is an OAuthLib.common.Request and doesn't have the session + # or user of the django request. We will emulate the session and auth + # middleware here, since that is what the idp is using for auth. You + # may need to modify this if you are using a different session + # middleware or auth backend. + + session_cookie_name = settings.SESSION_COOKIE_NAME + HTTP_COOKIE = request.headers.get("HTTP_COOKIE") + COOKIES = HTTP_COOKIE.split("; ") + for cookie in COOKIES: + cookie_name, cookie_value = cookie.split("=") + if cookie.startswith(session_cookie_name): + break + session_middleware = SessionMiddleware(get_response) + session = session_middleware.SessionStore(cookie_value) + # add session to request for compatibility with django.contrib.auth + request.session = session + + # call the auth middleware to set request.user + auth_middleware = AuthenticationMiddleware(get_response) + auth_middleware.process_request(request) + return request.user.is_authenticated + + def validate_silent_authorization(self, request) -> None: + return True diff --git a/tests/app/idp/idp/settings.py b/tests/app/idp/idp/settings.py index 9ef6c15a6..375cdcc9b 100644 --- a/tests/app/idp/idp/settings.py +++ b/tests/app/idp/idp/settings.py @@ -129,6 +129,7 @@ DEFAULT_AUTO_FIELD = "django.db.models.BigAutoField" OAUTH2_PROVIDER = { + "OAUTH2_VALIDATOR_CLASS": "idp.oauth.CustomOAuth2Validator", "OIDC_ENABLED": True, "OIDC_RP_INITIATED_LOGOUT_ENABLED": True, # this key is just for out test app, you should never store a key like this in a production environment. diff --git a/tests/test_authorization_code.py b/tests/test_authorization_code.py index 9d71016d3..b8ebb14f4 100644 --- a/tests/test_authorization_code.py +++ b/tests/test_authorization_code.py @@ -545,6 +545,35 @@ def test_code_post_auth_fails_when_redirect_uri_path_is_invalid(self): @pytest.mark.oauth2_settings(presets.OIDC_SETTINGS_RW) class TestOIDCAuthorizationCodeView(BaseTest): + def test_login(self): + """ + Test login page is rendered if user is not authenticated + """ + self.oauth2_settings.PKCE_REQUIRED = False + + query_data = { + "client_id": self.application.client_id, + "response_type": "code", + "state": "random_state_string", + "scope": "openid", + "redirect_uri": "http://example.org", + } + path = reverse("oauth2_provider:authorize") + response = self.client.get(path, data=query_data) + # The authorization view redirects to the login page with the + self.assertEqual(response.status_code, 302) + scheme, netloc, path, params, query, fragment = urlparse(response["Location"]) + self.assertEqual(path, settings.LOGIN_URL) + parsed_query = parse_qs(query) + next = parsed_query["next"][0] + self.assertIn(f"client_id={self.application.client_id}", next) + self.assertIn("response_type=code", next) + self.assertIn("state=random_state_string", next) + self.assertIn("scope=openid", next) + self.assertIn("redirect_uri=http%3A%2F%2Fexample.org", next) + + + def test_id_token_skip_authorization_completely(self): """ If application.skip_authorization = True, should skip the authorization page. @@ -645,6 +674,33 @@ def test_prompt_login(self): self.assertNotIn("prompt=login", next) + def test_prompt_none_unauthorized(self): + """ + Test response for redirect when supplied with prompt: none + + Should redirect to redirect_uri with an error of login_required + """ + self.oauth2_settings.PKCE_REQUIRED = False + + query_data = { + "client_id": self.application.client_id, + "response_type": "code", + "state": "random_state_string", + "scope": "read write", + "redirect_uri": "http://example.org", + "prompt": "none", + } + + response = self.client.get(reverse("oauth2_provider:authorize"), data=query_data) + + self.assertEqual(response.status_code, 302) + + scheme, netloc, path, params, query, fragment = urlparse(response["Location"]) + parsed_query = parse_qs(query) + + self.assertIn("login_required", parsed_query["error"]) + self.assertIn("random_state_string", parsed_query["state"]) + class BaseAuthorizationCodeTokenView(BaseTest): def get_auth(self, scope="read write"):