From 1c11d56c58d1a9b294927dc152a5ae74d8824fa6 Mon Sep 17 00:00:00 2001 From: Julian Mundhahs Date: Sat, 14 Jan 2023 18:19:34 +0100 Subject: [PATCH 01/18] Implement OIDC RP-Initiated Logout --- AUTHORS | 1 + CHANGELOG.md | 1 + docs/oidc.rst | 26 +++ docs/settings.rst | 15 ++ oauth2_provider/exceptions.py | 44 ++++ oauth2_provider/forms.py | 14 ++ ...7_application_post_logout_redirect_uris.py | 18 ++ oauth2_provider/models.py | 12 + oauth2_provider/settings.py | 2 + .../oauth2_provider/logout_confirm.html | 37 ++++ oauth2_provider/urls.py | 1 + oauth2_provider/views/__init__.py | 2 +- oauth2_provider/views/oidc.py | 179 ++++++++++++++- tests/conftest.py | 34 +++ ...tion_post_logout_redirect_uris_and_more.py | 26 +++ tests/presets.py | 3 + tests/test_oidc_views.py | 206 ++++++++++++++++++ 17 files changed, 619 insertions(+), 2 deletions(-) create mode 100644 oauth2_provider/migrations/0007_application_post_logout_redirect_uris.py create mode 100644 oauth2_provider/templates/oauth2_provider/logout_confirm.html create mode 100644 tests/migrations/0003_basetestapplication_post_logout_redirect_uris_and_more.py diff --git a/AUTHORS b/AUTHORS index 8887b9919..ac206edce 100644 --- a/AUTHORS +++ b/AUTHORS @@ -59,6 +59,7 @@ Jordi Sanchez Joseph Abrahams Josh Thomas Jozef Knaperek +Julian Mundhahs Julien Palard Julian Mundhahs Jun Zhou diff --git a/CHANGELOG.md b/CHANGELOG.md index edc5f8abc..8c92aa849 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added * Add Japanese(日本語) Language Support +* [OIDC RP-Initiated Logout](https://openid.net/specs/openid-connect-rpinitiated-1_0.html) ### Changed * #1211 documentation improve on 'AUTHORIZATION_CODE_EXPIRE_SECONDS'. diff --git a/docs/oidc.rst b/docs/oidc.rst index 2770722f0..2a69ceccc 100644 --- a/docs/oidc.rst +++ b/docs/oidc.rst @@ -23,6 +23,8 @@ We support: * OpenID Connect Implicit Flow * OpenID Connect Hybrid Flow +Furthermore ``django-oauth-toolkit`` also supports `OpenID Connect RP-Initiated Logout `_. + Configuration ============= @@ -147,6 +149,23 @@ scopes in your ``settings.py``:: If you want to enable ``RS256`` at a later date, you can do so - just add the private key as described above. + +RP-Initiated Logout +~~~~~~~~~~~~~~~~~~~ +This feature has to be enabled separately as it is an extension to the core standard. + +.. code-block:: python + + OAUTH2_PROVIDER = { + # OIDC has to be enabled to use RP-Initiated Logout + "OIDC_ENABLED": True, + # Enable and configure RP-Initiated Logout + "OIDC_RP_INITIATED_LOGOUT": True, + "OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT": True, + # ... any other settings you want + } + + Setting up OIDC enabled clients =============================== @@ -403,3 +422,10 @@ UserInfoView Available at ``/o/userinfo/``, this view provides extra user details. You can customize the details included in the response as described above. + + +RPInitiatedLogoutView +~~~~~~~~~~~~~~~~~~~~~ + +Available at ``/o/rp-initiated-logout/``, this view allows a :term:`Client` (Relying Party) to request that a :term:`Resource Owner` +is logged out at the :term:`Authorization Server` (OpenID Provider). diff --git a/docs/settings.rst b/docs/settings.rst index 6b6939c9a..d2bf6ce7e 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -313,6 +313,21 @@ this you must also provide the service at that endpoint. If unset, the default location is used, eg if ``django-oauth-toolkit`` is mounted at ``/o/``, it will be ``/o/userinfo/``. +OIDC_RP_INITIATED_LOGOUT_ENABLED +~~~~~~~~~~~~~~~~~~~~~~~~ +Default: ``False`` + +When is set to `False` (default) the `OpenID Connect RP-Initiated Logout `_ +endpoint is not enabled. OpenID Connect RP-Initiated Logout enables an :term:`Client` (Relying Party) +to request that a :term:`Resource Owner` (End User) is logged out at the :term:`Authorization Server` (OpenID Provider). + +OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Default: ``True`` + +Whether to always prompt the :term:`Resource Owner` (End User) to confirm a logout requested by a +:term:`Client` (Relying Party). If it is disabled the :term:`Resource Owner` (End User) will only be prompted if required by the standard. + OIDC_ISS_ENDPOINT ~~~~~~~~~~~~~~~~~ Default: ``""`` diff --git a/oauth2_provider/exceptions.py b/oauth2_provider/exceptions.py index c4208488d..b15665d8d 100644 --- a/oauth2_provider/exceptions.py +++ b/oauth2_provider/exceptions.py @@ -17,3 +17,47 @@ class FatalClientError(OAuthToolkitError): """ pass + + +# TODO: Cleanup +class OIDCError(Exception): + status_code = 400 + error = None + + def __init__(self, description=None): + if description is not None: + self.description = description + + message = "({}) {}".format(self.error, self.description) + super().__init__(message) + + +class InvalidRequestFatalError(OIDCError): + """ + For fatal errors, the request is missing a required parameter, includes + an invalid parameter value, includes a parameter more than once, or is + otherwise malformed. + """ + + error = "invalid_request" + + +class ClientIdMissmatch(InvalidRequestFatalError): + description = "Missmatch between Client ID of the ID Token and provided the Client ID." + + +class InvalidOIDCClientError(InvalidRequestFatalError): + description = "The Client is unknown or no client was included." + + +class MismatchingOIDCRedirectURIError(InvalidRequestFatalError): + description = "Mismatching post logout redirect URI." + + +class InvalidIDTokenError(InvalidRequestFatalError): + description = "The ID Token is expired, revoked, malformed, or invalid for other reasons." + + +class LogoutDenied(OIDCError): + error = "logout_denied" + description = "Logout was denied by the user." diff --git a/oauth2_provider/forms.py b/oauth2_provider/forms.py index 876213626..113ab3f53 100644 --- a/oauth2_provider/forms.py +++ b/oauth2_provider/forms.py @@ -12,3 +12,17 @@ class AllowForm(forms.Form): code_challenge = forms.CharField(required=False, widget=forms.HiddenInput()) code_challenge_method = forms.CharField(required=False, widget=forms.HiddenInput()) claims = forms.CharField(required=False, widget=forms.HiddenInput()) + + +class ConfirmLogoutForm(forms.Form): + allow = forms.BooleanField(required=False) + id_token_hint = forms.CharField(required=False, widget=forms.HiddenInput()) + logout_hint = forms.CharField(required=False, widget=forms.HiddenInput()) + client_id = forms.CharField(required=False, widget=forms.HiddenInput()) + post_logout_redirect_uri = forms.CharField(required=False, widget=forms.HiddenInput()) + state = forms.CharField(required=False, widget=forms.HiddenInput()) + ui_locales = forms.CharField(required=False, widget=forms.HiddenInput()) + + def __init__(self, *args, **kwargs): + self.request = kwargs.pop("request", None) + super(ConfirmLogoutForm, self).__init__(*args, **kwargs) diff --git a/oauth2_provider/migrations/0007_application_post_logout_redirect_uris.py b/oauth2_provider/migrations/0007_application_post_logout_redirect_uris.py new file mode 100644 index 000000000..6eba65118 --- /dev/null +++ b/oauth2_provider/migrations/0007_application_post_logout_redirect_uris.py @@ -0,0 +1,18 @@ +# Generated by Django 4.1.5 on 2023-01-14 12:32 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("oauth2_provider", "0006_alter_application_client_secret"), + ] + + operations = [ + migrations.AddField( + model_name="application", + name="post_logout_redirect_uris", + field=models.TextField(blank=True, help_text="Allowed Post Logout URIs list, space separated"), + ), + ] diff --git a/oauth2_provider/models.py b/oauth2_provider/models.py index 723328549..7acbe4113 100644 --- a/oauth2_provider/models.py +++ b/oauth2_provider/models.py @@ -103,6 +103,10 @@ class AbstractApplication(models.Model): blank=True, help_text=_("Allowed URIs list, space separated"), ) + post_logout_redirect_uris = models.TextField( + blank=True, + help_text=_("Allowed Post Logout URIs list, space separated"), + ) client_type = models.CharField(max_length=32, choices=CLIENT_TYPES) authorization_grant_type = models.CharField(max_length=32, choices=GRANT_TYPES) client_secret = ClientSecretField( @@ -150,6 +154,14 @@ def redirect_uri_allowed(self, uri): """ return redirect_to_uri_allowed(uri, self.redirect_uris.split()) + def post_logout_redirect_uri_allowed(self, uri): + """ + Checks if given URI is one of the items in :attr:`post_logout_redirect_uris` string + + :param uri: URI to check + """ + return redirect_to_uri_allowed(uri, self.post_logout_redirect_uris.split()) + def clean(self): from django.core.exceptions import ValidationError diff --git a/oauth2_provider/settings.py b/oauth2_provider/settings.py index 00a4e631c..9390ed953 100644 --- a/oauth2_provider/settings.py +++ b/oauth2_provider/settings.py @@ -88,6 +88,8 @@ "client_secret_post", "client_secret_basic", ], + "OIDC_RP_INITIATED_LOGOUT_ENABLED": False, + "OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT": True, # Special settings that will be evaluated at runtime "_SCOPES": [], "_DEFAULT_SCOPES": [], diff --git a/oauth2_provider/templates/oauth2_provider/logout_confirm.html b/oauth2_provider/templates/oauth2_provider/logout_confirm.html new file mode 100644 index 000000000..8b64f8314 --- /dev/null +++ b/oauth2_provider/templates/oauth2_provider/logout_confirm.html @@ -0,0 +1,37 @@ +{% extends "oauth2_provider/base.html" %} + +{% load i18n %} +{% block content %} +
+ {% if not error %} +
+ {% if application %} +

Confirm Logout requested by {{ application.name }}

+ {% else %} +

Confirm Logout

+ {% endif %} + {% csrf_token %} + + {% for field in form %} + {% if field.is_hidden %} + {{ field }} + {% endif %} + {% endfor %} + + {{ form.errors }} + {{ form.non_field_errors }} + +
+
+ + +
+
+
+ + {% else %} +

Error: {{ error.error }}

+

{{ error.description }}

+ {% endif %} +
+{% endblock %} diff --git a/oauth2_provider/urls.py b/oauth2_provider/urls.py index 508f97c96..ba76b7453 100644 --- a/oauth2_provider/urls.py +++ b/oauth2_provider/urls.py @@ -38,6 +38,7 @@ ), re_path(r"^\.well-known/jwks.json$", views.JwksInfoView.as_view(), name="jwks-info"), re_path(r"^userinfo/$", views.UserInfoView.as_view(), name="user-info"), + re_path(r"^rp-initiated-logout/$", views.RPInitiatedLogoutView.as_view(), name="rp-initiated-logout"), ] diff --git a/oauth2_provider/views/__init__.py b/oauth2_provider/views/__init__.py index 0720c1aa2..70b0431b7 100644 --- a/oauth2_provider/views/__init__.py +++ b/oauth2_provider/views/__init__.py @@ -15,5 +15,5 @@ ScopedProtectedResourceView, ) from .introspect import IntrospectTokenView -from .oidc import ConnectDiscoveryInfoView, JwksInfoView, UserInfoView +from .oidc import ConnectDiscoveryInfoView, JwksInfoView, UserInfoView, RPInitiatedLogoutView from .token import AuthorizedTokenDeleteView, AuthorizedTokensListView diff --git a/oauth2_provider/views/oidc.py b/oauth2_provider/views/oidc.py index 38560aea1..9d8693746 100644 --- a/oauth2_provider/views/oidc.py +++ b/oauth2_provider/views/oidc.py @@ -1,13 +1,25 @@ import json from urllib.parse import urlparse +from django.contrib.auth import logout from django.http import HttpResponse, JsonResponse from django.urls import reverse from django.utils.decorators import method_decorator from django.views.decorators.csrf import csrf_exempt -from django.views.generic import View +from django.views.generic import FormView, View from jwcrypto import jwk +from oauthlib.common import add_params_to_uri +from ..exceptions import ( + ClientIdMissmatch, + InvalidIDTokenError, + InvalidOIDCClientError, + LogoutDenied, + MismatchingOIDCRedirectURIError, + OIDCError, +) +from ..forms import ConfirmLogoutForm +from ..http import OAuth2ResponseRedirect from ..models import get_application_model from ..settings import oauth2_settings from .mixins import OAuthLibMixin, OIDCOnlyMixin @@ -33,6 +45,10 @@ def get(self, request, *args, **kwargs): reverse("oauth2_provider:user-info") ) jwks_uri = request.build_absolute_uri(reverse("oauth2_provider:jwks-info")) + if oauth2_settings.OIDC_RP_INITIATED_LOGOUT_ENABLED: + end_session_endpoint = request.build_absolute_uri( + reverse("oauth2_provider:rp-initiated-logout") + ) else: parsed_url = urlparse(oauth2_settings.OIDC_ISS_ENDPOINT) host = parsed_url.scheme + "://" + parsed_url.netloc @@ -42,6 +58,8 @@ def get(self, request, *args, **kwargs): host, reverse("oauth2_provider:user-info") ) jwks_uri = "{}{}".format(host, reverse("oauth2_provider:jwks-info")) + if oauth2_settings.OIDC_RP_INITIATED_LOGOUT_ENABLED: + end_session_endpoint = "{}{}".format(host, reverse("oauth2_provider:rp-initiated-logout")) signing_algorithms = [Application.HS256_ALGORITHM] if oauth2_settings.OIDC_RSA_PRIVATE_KEY: @@ -69,6 +87,8 @@ def get(self, request, *args, **kwargs): ), "claims_supported": oidc_claims, } + if oauth2_settings.OIDC_RP_INITIATED_LOGOUT_ENABLED: + data["end_session_endpoint"] = end_session_endpoint response = JsonResponse(data) response["Access-Control-Allow-Origin"] = "*" return response @@ -120,3 +140,160 @@ def _create_userinfo_response(self, request): for k, v in headers.items(): response[k] = v return response + + +def validate_logout_request(user, id_token_hint, client_id, post_logout_redirect_uri): + """ + Validate an OIDC RP-Initiated Logout Request. + `(prompt_logout, (post_logout_redirect_uri, application))` is returned. + + `prompt_logout` indicates whether the logout has to be confirmed by the user. This happens if the + specifications force a confirmation, or it is enabled by `OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT`. + `post_logout_redirect_uri` is the validated URI where the User should be redirected to after the + logout. Can be None. None will redirect to "/" of this app. If it is set `application` will also + be set to the Application that is requesting the logout. + + The `id_token_hint` will be validated if given. If both `client_id` and `id_token_hint` are given they + will be validated against each other. + """ + validator = oauth2_settings.OAUTH2_VALIDATOR_CLASS() + + id_token = None + must_prompt_logout = True + if id_token_hint: + # Note: The standard states that expired tokens should still be accepted. + # This implementation only accepts tokens that are still valid. + id_token = validator._load_id_token(id_token_hint) + + if not id_token: + raise InvalidIDTokenError() + + if id_token.user == user: + # A logout without user interaction (i.e. no prompt) is only allowed + # if an ID Token is provided that matches the current user. + must_prompt_logout = False + + # If both id_token_hint and client_id are given it must be verified that they match. + if client_id: + if id_token.application.client_id != client_id: + raise ClientIdMissmatch() + + # The standard states that a prompt should always be shown. + # This behaviour can be configured with OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT. + prompt_logout = must_prompt_logout or oauth2_settings.OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT + + application = None + # Determine the application that is requesting the logout. + if client_id: + application = get_application_model().objects.get(client_id=client_id) + elif id_token: + application = id_token.application + + # Validate `post_logout_redirect_uri` + if post_logout_redirect_uri: + if not application: + raise InvalidOIDCClientError() + scheme = urlparse(post_logout_redirect_uri)[0] + if not scheme: + raise MismatchingOIDCRedirectURIError("A Scheme is required for the redirect URI.") + if scheme == "http" and application.client_type != "confidential": + raise MismatchingOIDCRedirectURIError("http is only allowed with confidential clients.") + if scheme not in application.get_allowed_schemes(): + raise MismatchingOIDCRedirectURIError(f'Redirect to scheme "{scheme}" is not permitted.') + if not application.post_logout_redirect_uri_allowed(post_logout_redirect_uri): + raise MismatchingOIDCRedirectURIError("This client does not have this redirect uri registered.") + + return prompt_logout, (post_logout_redirect_uri, application) + + +class RPInitiatedLogoutView(OIDCOnlyMixin, FormView): + template_name = "oauth2_provider/logout_confirm.html" + form_class = ConfirmLogoutForm + + def get_initial(self): + return { + "id_token_hint": self.oidc_data.get("id_token_hint", None), + "logout_hint": self.oidc_data.get("logout_hint", None), + "client_id": self.oidc_data.get("client_id", None), + "post_logout_redirect_uri": self.oidc_data.get("post_logout_redirect_uri", None), + "state": self.oidc_data.get("state", None), + "ui_locales": self.oidc_data.get("ui_locales", None), + } + + def dispatch(self, request, *args, **kwargs): + self.oidc_data = {} + return super().dispatch(request, *args, **kwargs) + + def get(self, request, *args, **kwargs): + id_token_hint = request.GET.get("id_token_hint") + client_id = request.GET.get("client_id") + post_logout_redirect_uri = request.GET.get("post_logout_redirect_uri") + state = request.GET.get("state") + + try: + prompt, (redirect_uri, application) = validate_logout_request( + user=request.user, + id_token_hint=id_token_hint, + client_id=client_id, + post_logout_redirect_uri=post_logout_redirect_uri, + ) + except OIDCError as error: + return self.error_response(error) + + if not prompt: + return self.do_logout(application, redirect_uri, state) + + self.oidc_data = { + "id_token_hint": id_token_hint, + "client_id": client_id, + "post_logout_redirect_uri": post_logout_redirect_uri, + "state": state, + } + form = self.get_form(self.get_form_class()) + kwargs["form"] = form + if application: + kwargs["application"] = application + + return self.render_to_response(self.get_context_data(**kwargs)) + + def form_valid(self, form): + id_token_hint = form.cleaned_data.get("id_token_hint") + client_id = form.cleaned_data.get("client_id") + post_logout_redirect_uri = form.cleaned_data.get("post_logout_redirect_uri") + state = form.cleaned_data.get("state") + + try: + prompt, (redirect_uri, application) = validate_logout_request( + user=self.request.user, + id_token_hint=id_token_hint, + client_id=client_id, + post_logout_redirect_uri=post_logout_redirect_uri, + ) + + if not prompt or form.cleaned_data.get("allow"): + return self.do_logout(application, redirect_uri, state) + else: + raise LogoutDenied() + + except OIDCError as error: + return self.error_response(error) + + def do_logout(self, application=None, post_logout_redirect_uri=None, state=None): + logout(self.request) + if post_logout_redirect_uri: + if state: + return OAuth2ResponseRedirect( + add_params_to_uri(post_logout_redirect_uri, [("state", state)]), + application.get_allowed_schemes(), + ) + else: + return OAuth2ResponseRedirect(post_logout_redirect_uri, application.get_allowed_schemes()) + else: + return OAuth2ResponseRedirect( + self.request.build_absolute_uri("/"), + oauth2_settings.ALLOWED_REDIRECT_URI_SCHEMES, + ) + + def error_response(self, error): + error_response = {"error": error} + return self.render_to_response(error_response, status=error.status_code) diff --git a/tests/conftest.py b/tests/conftest.py index 14db54aa5..f8574a804 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -100,6 +100,7 @@ def application(): return Application.objects.create( name="Test Application", redirect_uris="http://example.org", + post_logout_redirect_uris="http://example.org", client_type=Application.CLIENT_CONFIDENTIAL, authorization_grant_type=Application.GRANT_AUTHORIZATION_CODE, algorithm=Application.RS256_ALGORITHM, @@ -107,6 +108,28 @@ def application(): ) +@pytest.fixture +def other_application(): + return Application.objects.create( + name="Other Application", + redirect_uris="http://other.org", + post_logout_redirect_uris="http://other.org", + client_type=Application.CLIENT_CONFIDENTIAL, + authorization_grant_type=Application.GRANT_AUTHORIZATION_CODE, + algorithm=Application.RS256_ALGORITHM, + client_secret=CLEARTEXT_SECRET, + ) + + +@pytest.fixture +def loggend_in_client(test_user): + from django.test.client import Client + + client = Client() + client.force_login(test_user) + return client + + @pytest.fixture def hybrid_application(application): application.authorization_grant_type = application.GRANT_OPENID_HYBRID @@ -120,6 +143,17 @@ def test_user(): return UserModel.objects.create_user("test_user", "test@example.com", "123456") +@pytest.fixture +def other_user(): + return UserModel.objects.create_user("other_user", "other@example.com", "123456") + + +@pytest.fixture +def rp_settings(oauth2_settings): + oauth2_settings.update(presets.OIDC_SETTINGS_RP_LOGOUT) + return oauth2_settings + + @pytest.fixture def oidc_tokens(oauth2_settings, application, test_user, client): oauth2_settings.update(presets.OIDC_SETTINGS_RW) diff --git a/tests/migrations/0003_basetestapplication_post_logout_redirect_uris_and_more.py b/tests/migrations/0003_basetestapplication_post_logout_redirect_uris_and_more.py new file mode 100644 index 000000000..8ca59c84b --- /dev/null +++ b/tests/migrations/0003_basetestapplication_post_logout_redirect_uris_and_more.py @@ -0,0 +1,26 @@ +# Generated by Django 4.1.5 on 2023-01-14 20:07 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.OAUTH2_PROVIDER_ID_TOKEN_MODEL), + ("tests", "0002_swapped_models"), + ] + + operations = [ + migrations.AddField( + model_name="basetestapplication", + name="post_logout_redirect_uris", + field=models.TextField(blank=True, help_text="Allowed Post Logout URIs list, space separated"), + ), + migrations.AddField( + model_name="sampleapplication", + name="post_logout_redirect_uris", + field=models.TextField(blank=True, help_text="Allowed Post Logout URIs list, space separated"), + ), + ] diff --git a/tests/presets.py b/tests/presets.py index 4b207f25c..26f2fee1f 100644 --- a/tests/presets.py +++ b/tests/presets.py @@ -28,6 +28,9 @@ OIDC_SETTINGS_EMAIL_SCOPE["SCOPES"].update({"email": "return email address"}) OIDC_SETTINGS_HS256_ONLY = deepcopy(OIDC_SETTINGS_RW) del OIDC_SETTINGS_HS256_ONLY["OIDC_RSA_PRIVATE_KEY"] +OIDC_SETTINGS_RP_LOGOUT = deepcopy(OIDC_SETTINGS_RW) +OIDC_SETTINGS_RP_LOGOUT["OIDC_RP_INITIATED_LOGOUT_ENABLED"] = True +OIDC_SETTINGS_RP_LOGOUT["OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT"] = False REST_FRAMEWORK_SCOPES = { "SCOPES": { "read": "Read scope", diff --git a/tests/test_oidc_views.py b/tests/test_oidc_views.py index 7b379d1b3..c5f7c4e73 100644 --- a/tests/test_oidc_views.py +++ b/tests/test_oidc_views.py @@ -1,8 +1,15 @@ import pytest +from django.contrib.auth import get_user from django.test import TestCase from django.urls import reverse +from oauth2_provider.exceptions import ( + ClientIdMissmatch, + InvalidOIDCClientError, + MismatchingOIDCRedirectURIError, +) from oauth2_provider.oauth2_validators import OAuth2Validator +from oauth2_provider.views.oidc import validate_logout_request from . import presets @@ -36,6 +43,37 @@ def test_get_connect_discovery_info(self): self.assertEqual(response.status_code, 200) assert response.json() == expected_response + def expect_json_response_with_rp(self, base): + expected_response = { + "issuer": f"{base}", + "authorization_endpoint": f"{base}/authorize/", + "token_endpoint": f"{base}/token/", + "userinfo_endpoint": f"{base}/userinfo/", + "jwks_uri": f"{base}/.well-known/jwks.json", + "scopes_supported": ["read", "write", "openid"], + "response_types_supported": [ + "code", + "token", + "id_token", + "id_token token", + "code token", + "code id_token", + "code id_token token", + ], + "subject_types_supported": ["public"], + "id_token_signing_alg_values_supported": ["RS256", "HS256"], + "token_endpoint_auth_methods_supported": ["client_secret_post", "client_secret_basic"], + "claims_supported": ["sub"], + "end_session_endpoint": f"{base}/rp-initiated-logout/", + } + response = self.client.get(reverse("oauth2_provider:oidc-connect-discovery-info")) + self.assertEqual(response.status_code, 200) + assert response.json() == expected_response + + def test_get_connect_discovery_info_with_rp_logout(self): + self.oauth2_settings.OIDC_RP_INITIATED_LOGOUT_ENABLED = True + self.expect_json_response_with_rp(self.oauth2_settings.OIDC_ISS_ENDPOINT) + def test_get_connect_discovery_info_without_issuer_url(self): self.oauth2_settings.OIDC_ISS_ENDPOINT = None self.oauth2_settings.OIDC_USERINFO_ENDPOINT = None @@ -64,6 +102,12 @@ def test_get_connect_discovery_info_without_issuer_url(self): self.assertEqual(response.status_code, 200) assert response.json() == expected_response + def test_get_connect_discovery_info_without_issuer_url_with_rp_logout(self): + self.oauth2_settings.OIDC_RP_INITIATED_LOGOUT_ENABLED = True + self.oauth2_settings.OIDC_ISS_ENDPOINT = None + self.oauth2_settings.OIDC_USERINFO_ENDPOINT = None + self.expect_json_response_with_rp("http://testserver/o") + def test_get_connect_discovery_info_without_rsa_key(self): self.oauth2_settings.OIDC_RSA_PRIVATE_KEY = None response = self.client.get(reverse("oauth2_provider:oidc-connect-discovery-info")) @@ -124,6 +168,168 @@ def test_get_jwks_info_multiple_rsa_keys(self): assert response.json() == expected_response +@pytest.mark.django_db +@pytest.mark.parametrize("ALWAYS_PROMPT", [True, False]) +def test_validate_logout_request(oidc_tokens, other_application, other_user, rp_settings, ALWAYS_PROMPT): + rp_settings.OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT = ALWAYS_PROMPT + oidc_tokens = oidc_tokens + application = oidc_tokens.application + client_id = application.client_id + id_token = oidc_tokens.id_token + assert validate_logout_request( + user=oidc_tokens.user, id_token_hint=None, client_id=None, post_logout_redirect_uri=None + ) == (True, (None, None)) + assert validate_logout_request( + user=oidc_tokens.user, id_token_hint=None, client_id=client_id, post_logout_redirect_uri=None + ) == (True, (None, application)) + assert validate_logout_request( + user=oidc_tokens.user, + id_token_hint=None, + client_id=client_id, + post_logout_redirect_uri="http://example.org", + ) == (True, ("http://example.org", application)) + assert validate_logout_request( + user=oidc_tokens.user, + id_token_hint=id_token, + client_id=None, + post_logout_redirect_uri="http://example.org", + ) == (ALWAYS_PROMPT, ("http://example.org", application)) + assert validate_logout_request( + user=other_user, + id_token_hint=id_token, + client_id=None, + post_logout_redirect_uri="http://example.org", + ) == (True, ("http://example.org", application)) + assert validate_logout_request( + user=oidc_tokens.user, + id_token_hint=id_token, + client_id=client_id, + post_logout_redirect_uri="http://example.org", + ) == (ALWAYS_PROMPT, ("http://example.org", application)) + with pytest.raises(ClientIdMissmatch): + validate_logout_request( + user=oidc_tokens.user, + id_token_hint=id_token, + client_id=other_application.client_id, + post_logout_redirect_uri="http://other.org", + ) + with pytest.raises(InvalidOIDCClientError): + validate_logout_request( + user=oidc_tokens.user, + id_token_hint=None, + client_id=None, + post_logout_redirect_uri="http://example.org", + ) + with pytest.raises(MismatchingOIDCRedirectURIError): + validate_logout_request( + user=oidc_tokens.user, + id_token_hint=None, + client_id=client_id, + post_logout_redirect_uri="example.org", + ) + with pytest.raises(MismatchingOIDCRedirectURIError): + validate_logout_request( + user=oidc_tokens.user, + id_token_hint=None, + client_id=client_id, + post_logout_redirect_uri="imap://example.org", + ) + with pytest.raises(MismatchingOIDCRedirectURIError): + validate_logout_request( + user=oidc_tokens.user, + id_token_hint=None, + client_id=client_id, + post_logout_redirect_uri="http://other.org", + ) + + +def is_logged_in(client): + return get_user(client).is_authenticated + + +@pytest.mark.django_db +def test_rp_initiated_logout_get(loggend_in_client, rp_settings): + rsp = loggend_in_client.get(reverse("oauth2_provider:rp-initiated-logout"), data={}) + assert rsp.status_code == 200 + assert is_logged_in(loggend_in_client) + + +@pytest.mark.django_db +def test_rp_initiated_logout_get_id_token(loggend_in_client, oidc_tokens, rp_settings): + rsp = loggend_in_client.get( + reverse("oauth2_provider:rp-initiated-logout"), data={"id_token_hint": oidc_tokens.id_token} + ) + assert rsp.status_code == 302 + assert rsp.headers["Location"] == "http://testserver/" + assert not is_logged_in(loggend_in_client) + + +@pytest.mark.django_db +def test_rp_initiated_logout_get_id_token_redirect(loggend_in_client, oidc_tokens, rp_settings): + rsp = loggend_in_client.get( + reverse("oauth2_provider:rp-initiated-logout"), + data={"id_token_hint": oidc_tokens.id_token, "post_logout_redirect_uri": "http://example.org"}, + ) + assert rsp.status_code == 302 + assert rsp.headers["Location"] == "http://example.org" + assert not is_logged_in(loggend_in_client) + + +@pytest.mark.django_db +def test_rp_initiated_logout_get_id_token_redirect_with_state(loggend_in_client, oidc_tokens, rp_settings): + rsp = loggend_in_client.get( + reverse("oauth2_provider:rp-initiated-logout"), + data={ + "id_token_hint": oidc_tokens.id_token, + "post_logout_redirect_uri": "http://example.org", + "state": "987654321", + }, + ) + assert rsp.status_code == 302 + assert rsp.headers["Location"] == "http://example.org?state=987654321" + assert not is_logged_in(loggend_in_client) + + +@pytest.mark.django_db +def test_rp_initiated_logout_get_id_token_missmatch_client_id( + loggend_in_client, oidc_tokens, other_application, rp_settings +): + rsp = loggend_in_client.get( + reverse("oauth2_provider:rp-initiated-logout"), + data={"id_token_hint": oidc_tokens.id_token, "client_id": other_application.client_id}, + ) + assert rsp.status_code == 400 + assert is_logged_in(loggend_in_client) + + +@pytest.mark.django_db +def test_rp_initiated_logout_get_id_token_client_id(loggend_in_client, oidc_tokens, rp_settings): + rsp = loggend_in_client.get( + reverse("oauth2_provider:rp-initiated-logout"), data={"client_id": oidc_tokens.application.client_id} + ) + assert rsp.status_code == 200 + assert is_logged_in(loggend_in_client) + + +@pytest.mark.django_db +def test_rp_initiated_logout_post(loggend_in_client, oidc_tokens, rp_settings): + form_data = { + "client_id": oidc_tokens.application.client_id, + } + rsp = loggend_in_client.post(reverse("oauth2_provider:rp-initiated-logout"), form_data) + assert rsp.status_code == 400 + assert is_logged_in(loggend_in_client) + + +@pytest.mark.django_db +def test_rp_initiated_logout_post_allowed(loggend_in_client, oidc_tokens, rp_settings): + form_data = {"client_id": oidc_tokens.application.client_id, "allow": True} + rsp = loggend_in_client.post(reverse("oauth2_provider:rp-initiated-logout"), form_data) + assert rsp.status_code == 302 + assert rsp.headers["Location"] == "http://testserver/" + assert not is_logged_in(loggend_in_client) + + @pytest.mark.django_db @pytest.mark.parametrize("method", ["get", "post"]) def test_userinfo_endpoint(oidc_tokens, client, method): From c9159c5e9891f0559c072f00a76c0923593006a9 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 19 Jan 2023 13:39:34 +0000 Subject: [PATCH 02/18] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- oauth2_provider/views/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oauth2_provider/views/__init__.py b/oauth2_provider/views/__init__.py index 70b0431b7..9e32e17d8 100644 --- a/oauth2_provider/views/__init__.py +++ b/oauth2_provider/views/__init__.py @@ -15,5 +15,5 @@ ScopedProtectedResourceView, ) from .introspect import IntrospectTokenView -from .oidc import ConnectDiscoveryInfoView, JwksInfoView, UserInfoView, RPInitiatedLogoutView +from .oidc import ConnectDiscoveryInfoView, JwksInfoView, RPInitiatedLogoutView, UserInfoView from .token import AuthorizedTokenDeleteView, AuthorizedTokensListView From b1f431d0ae0d57c7443f75b204ade95b64574977 Mon Sep 17 00:00:00 2001 From: Julian Mundhahs Date: Sat, 21 Jan 2023 00:52:23 +0100 Subject: [PATCH 03/18] Fix double entry in `AUTHORS` --- AUTHORS | 1 - 1 file changed, 1 deletion(-) diff --git a/AUTHORS b/AUTHORS index ac206edce..47a2aeaf2 100644 --- a/AUTHORS +++ b/AUTHORS @@ -61,7 +61,6 @@ Josh Thomas Jozef Knaperek Julian Mundhahs Julien Palard -Julian Mundhahs Jun Zhou Kaleb Porter Kristian Rune Larsen From da14c1fee4482cbe192f342b2cd27b3ce1ab2337 Mon Sep 17 00:00:00 2001 From: Julian Mundhahs Date: Sat, 21 Jan 2023 00:52:56 +0100 Subject: [PATCH 04/18] Make tests compatible with Django 2.2 --- tests/test_oidc_views.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_oidc_views.py b/tests/test_oidc_views.py index c5f7c4e73..eac2e9f83 100644 --- a/tests/test_oidc_views.py +++ b/tests/test_oidc_views.py @@ -260,7 +260,7 @@ def test_rp_initiated_logout_get_id_token(loggend_in_client, oidc_tokens, rp_set reverse("oauth2_provider:rp-initiated-logout"), data={"id_token_hint": oidc_tokens.id_token} ) assert rsp.status_code == 302 - assert rsp.headers["Location"] == "http://testserver/" + assert rsp["Location"] == "http://testserver/" assert not is_logged_in(loggend_in_client) @@ -271,7 +271,7 @@ def test_rp_initiated_logout_get_id_token_redirect(loggend_in_client, oidc_token data={"id_token_hint": oidc_tokens.id_token, "post_logout_redirect_uri": "http://example.org"}, ) assert rsp.status_code == 302 - assert rsp.headers["Location"] == "http://example.org" + assert rsp["Location"] == "http://example.org" assert not is_logged_in(loggend_in_client) @@ -286,7 +286,7 @@ def test_rp_initiated_logout_get_id_token_redirect_with_state(loggend_in_client, }, ) assert rsp.status_code == 302 - assert rsp.headers["Location"] == "http://example.org?state=987654321" + assert rsp["Location"] == "http://example.org?state=987654321" assert not is_logged_in(loggend_in_client) @@ -326,7 +326,7 @@ def test_rp_initiated_logout_post_allowed(loggend_in_client, oidc_tokens, rp_set form_data = {"client_id": oidc_tokens.application.client_id, "allow": True} rsp = loggend_in_client.post(reverse("oauth2_provider:rp-initiated-logout"), form_data) assert rsp.status_code == 302 - assert rsp.headers["Location"] == "http://testserver/" + assert rsp["Location"] == "http://testserver/" assert not is_logged_in(loggend_in_client) From d7c5bc986dafeb91facb9ea01d04712f9ef38c14 Mon Sep 17 00:00:00 2001 From: Julian Mundhahs Date: Sat, 21 Jan 2023 18:28:49 +0100 Subject: [PATCH 05/18] Add a test for a revoked token --- tests/test_oidc_views.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/test_oidc_views.py b/tests/test_oidc_views.py index eac2e9f83..961600f6d 100644 --- a/tests/test_oidc_views.py +++ b/tests/test_oidc_views.py @@ -9,6 +9,7 @@ MismatchingOIDCRedirectURIError, ) from oauth2_provider.oauth2_validators import OAuth2Validator +from oauth2_provider.settings import oauth2_settings from oauth2_provider.views.oidc import validate_logout_request from . import presets @@ -264,6 +265,17 @@ def test_rp_initiated_logout_get_id_token(loggend_in_client, oidc_tokens, rp_set assert not is_logged_in(loggend_in_client) +@pytest.mark.django_db +def test_rp_initiated_logout_get_revoked_id_token(loggend_in_client, oidc_tokens, rp_settings): + validator = oauth2_settings.OAUTH2_VALIDATOR_CLASS() + validator._load_id_token(oidc_tokens.id_token).revoke() + rsp = loggend_in_client.get( + reverse("oauth2_provider:rp-initiated-logout"), data={"id_token_hint": oidc_tokens.id_token} + ) + assert rsp.status_code == 400 + assert is_logged_in(loggend_in_client) + + @pytest.mark.django_db def test_rp_initiated_logout_get_id_token_redirect(loggend_in_client, oidc_tokens, rp_settings): rsp = loggend_in_client.get( From 6014550aaabc0cc8ae92f8b93f368283c54d8711 Mon Sep 17 00:00:00 2001 From: Julian Mundhahs Date: Sun, 22 Jan 2023 16:05:21 +0100 Subject: [PATCH 06/18] Extract Test acces token generation to a helper function --- tests/conftest.py | 72 ++++++++++++++++++++--------------------------- 1 file changed, 30 insertions(+), 42 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index f8574a804..d5e9a0f7e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -154,17 +154,19 @@ def rp_settings(oauth2_settings): return oauth2_settings -@pytest.fixture -def oidc_tokens(oauth2_settings, application, test_user, client): - oauth2_settings.update(presets.OIDC_SETTINGS_RW) +def generate_access_token(oauth2_settings, application, test_user, client, settings, scope, redirect_uri): + """ + A helper function that generates an access_token and ID Token for a given Application and User. + """ + oauth2_settings.update(settings) client.force_login(test_user) auth_rsp = client.post( reverse("oauth2_provider:authorize"), data={ "client_id": application.client_id, "state": "random_state_string", - "scope": "openid", - "redirect_uri": "http://example.org", + "scope": scope, + "redirect_uri": redirect_uri, "response_type": "code", "allow": True, }, @@ -177,10 +179,10 @@ def oidc_tokens(oauth2_settings, application, test_user, client): data={ "grant_type": "authorization_code", "code": code, - "redirect_uri": "http://example.org", + "redirect_uri": redirect_uri, "client_id": application.client_id, "client_secret": CLEARTEXT_SECRET, - "scope": "openid", + "scope": scope, }, ) assert token_rsp.status_code == 200 @@ -195,40 +197,26 @@ def oidc_tokens(oauth2_settings, application, test_user, client): @pytest.fixture -def oidc_email_scope_tokens(oauth2_settings, application, test_user, client): - oauth2_settings.update(presets.OIDC_SETTINGS_EMAIL_SCOPE) - client.force_login(test_user) - auth_rsp = client.post( - reverse("oauth2_provider:authorize"), - data={ - "client_id": application.client_id, - "state": "random_state_string", - "scope": "openid email", - "redirect_uri": "http://example.org", - "response_type": "code", - "allow": True, - }, - ) - assert auth_rsp.status_code == 302 - code = parse_qs(urlparse(auth_rsp["Location"]).query)["code"] - client.logout() - token_rsp = client.post( - reverse("oauth2_provider:token"), - data={ - "grant_type": "authorization_code", - "code": code, - "redirect_uri": "http://example.org", - "client_id": application.client_id, - "client_secret": CLEARTEXT_SECRET, - "scope": "openid email", - }, +def oidc_tokens(oauth2_settings, application, test_user, client): + return generate_access_token( + oauth2_settings, + application, + test_user, + client, + presets.OIDC_SETTINGS_RW, + "openid", + "http://example.org", ) - assert token_rsp.status_code == 200 - token_data = token_rsp.json() - return SimpleNamespace( - user=test_user, - application=application, - access_token=token_data["access_token"], - id_token=token_data["id_token"], - oauth2_settings=oauth2_settings, + + +@pytest.fixture +def oidc_email_scope_tokens(oauth2_settings, application, test_user, client): + return generate_access_token( + oauth2_settings, + application, + test_user, + client, + presets.OIDC_SETTINGS_EMAIL_SCOPE, + "openid email", + "http://example.org", ) From 764351b408b981109b91976a19afef779433f1e9 Mon Sep 17 00:00:00 2001 From: Julian Mundhahs Date: Sun, 22 Jan 2023 16:07:27 +0100 Subject: [PATCH 07/18] Add a test for http redirect of public Application --- tests/conftest.py | 17 +++++++++++++++-- tests/test_oidc_views.py | 24 ++++++++++++++++++++---- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index d5e9a0f7e..8e602cce7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -109,12 +109,12 @@ def application(): @pytest.fixture -def other_application(): +def public_application(): return Application.objects.create( name="Other Application", redirect_uris="http://other.org", post_logout_redirect_uris="http://other.org", - client_type=Application.CLIENT_CONFIDENTIAL, + client_type=Application.CLIENT_PUBLIC, authorization_grant_type=Application.GRANT_AUTHORIZATION_CODE, algorithm=Application.RS256_ALGORITHM, client_secret=CLEARTEXT_SECRET, @@ -220,3 +220,16 @@ def oidc_email_scope_tokens(oauth2_settings, application, test_user, client): "openid email", "http://example.org", ) + + +@pytest.fixture +def oidc_non_confidential_tokens(oauth2_settings, public_application, test_user, client): + return generate_access_token( + oauth2_settings, + public_application, + test_user, + client, + presets.OIDC_SETTINGS_EMAIL_SCOPE, + "openid", + "http://other.org", + ) diff --git a/tests/test_oidc_views.py b/tests/test_oidc_views.py index 961600f6d..649996d17 100644 --- a/tests/test_oidc_views.py +++ b/tests/test_oidc_views.py @@ -171,7 +171,7 @@ def test_get_jwks_info_multiple_rsa_keys(self): @pytest.mark.django_db @pytest.mark.parametrize("ALWAYS_PROMPT", [True, False]) -def test_validate_logout_request(oidc_tokens, other_application, other_user, rp_settings, ALWAYS_PROMPT): +def test_validate_logout_request(oidc_tokens, public_application, other_user, rp_settings, ALWAYS_PROMPT): rp_settings.OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT = ALWAYS_PROMPT oidc_tokens = oidc_tokens application = oidc_tokens.application @@ -211,7 +211,7 @@ def test_validate_logout_request(oidc_tokens, other_application, other_user, rp_ validate_logout_request( user=oidc_tokens.user, id_token_hint=id_token, - client_id=other_application.client_id, + client_id=public_application.client_id, post_logout_redirect_uri="http://other.org", ) with pytest.raises(InvalidOIDCClientError): @@ -304,11 +304,27 @@ def test_rp_initiated_logout_get_id_token_redirect_with_state(loggend_in_client, @pytest.mark.django_db def test_rp_initiated_logout_get_id_token_missmatch_client_id( - loggend_in_client, oidc_tokens, other_application, rp_settings + loggend_in_client, oidc_tokens, public_application, rp_settings ): rsp = loggend_in_client.get( reverse("oauth2_provider:rp-initiated-logout"), - data={"id_token_hint": oidc_tokens.id_token, "client_id": other_application.client_id}, + data={"id_token_hint": oidc_tokens.id_token, "client_id": public_application.client_id}, + ) + assert rsp.status_code == 400 + assert is_logged_in(loggend_in_client) + + +@pytest.mark.django_db +def test_rp_initiated_logout_public_client_redirect_client_id( + loggend_in_client, oidc_non_confidential_tokens, public_application, rp_settings +): + rsp = loggend_in_client.get( + reverse("oauth2_provider:rp-initiated-logout"), + data={ + "id_token_hint": oidc_non_confidential_tokens.id_token, + "client_id": public_application.client_id, + "post_logout_redirect_uri": "http://other.org", + }, ) assert rsp.status_code == 400 assert is_logged_in(loggend_in_client) From 502d18e6e840553740011f8c51652ad3431cc493 Mon Sep 17 00:00:00 2001 From: Julian Mundhahs Date: Fri, 3 Feb 2023 15:03:01 +0100 Subject: [PATCH 08/18] Small rewording of exception messages --- oauth2_provider/exceptions.py | 22 ++++++++++++---------- oauth2_provider/views/oidc.py | 10 +++++----- tests/test_oidc_views.py | 12 ++++-------- 3 files changed, 21 insertions(+), 23 deletions(-) diff --git a/oauth2_provider/exceptions.py b/oauth2_provider/exceptions.py index b15665d8d..f68a651b6 100644 --- a/oauth2_provider/exceptions.py +++ b/oauth2_provider/exceptions.py @@ -19,8 +19,11 @@ class FatalClientError(OAuthToolkitError): pass -# TODO: Cleanup class OIDCError(Exception): + """ + General class to derive from for all OIDC related errors. + """ + status_code = 400 error = None @@ -34,30 +37,29 @@ def __init__(self, description=None): class InvalidRequestFatalError(OIDCError): """ - For fatal errors, the request is missing a required parameter, includes - an invalid parameter value, includes a parameter more than once, or is - otherwise malformed. + For fatal errors. These are requests with invalid parameter values, missing parameters or otherwise + incorrect requests. """ error = "invalid_request" class ClientIdMissmatch(InvalidRequestFatalError): - description = "Missmatch between Client ID of the ID Token and provided the Client ID." + description = "Mismatch between the Client ID of the ID Token and the Client ID that was provided." class InvalidOIDCClientError(InvalidRequestFatalError): - description = "The Client is unknown or no client was included." + description = "The client is unknown or no client has been included." -class MismatchingOIDCRedirectURIError(InvalidRequestFatalError): - description = "Mismatching post logout redirect URI." +class InvalidOIDCRedirectURIError(InvalidRequestFatalError): + description = "Invalid post logout redirect URI." class InvalidIDTokenError(InvalidRequestFatalError): - description = "The ID Token is expired, revoked, malformed, or invalid for other reasons." + description = "The ID Token is expired, revoked, malformed, or otherwise invalid." class LogoutDenied(OIDCError): error = "logout_denied" - description = "Logout was denied by the user." + description = "Logout has been refused by the user." diff --git a/oauth2_provider/views/oidc.py b/oauth2_provider/views/oidc.py index 9d8693746..14a24ede6 100644 --- a/oauth2_provider/views/oidc.py +++ b/oauth2_provider/views/oidc.py @@ -14,8 +14,8 @@ ClientIdMissmatch, InvalidIDTokenError, InvalidOIDCClientError, + InvalidOIDCRedirectURIError, LogoutDenied, - MismatchingOIDCRedirectURIError, OIDCError, ) from ..forms import ConfirmLogoutForm @@ -195,13 +195,13 @@ def validate_logout_request(user, id_token_hint, client_id, post_logout_redirect raise InvalidOIDCClientError() scheme = urlparse(post_logout_redirect_uri)[0] if not scheme: - raise MismatchingOIDCRedirectURIError("A Scheme is required for the redirect URI.") + raise InvalidOIDCRedirectURIError("A Scheme is required for the redirect URI.") if scheme == "http" and application.client_type != "confidential": - raise MismatchingOIDCRedirectURIError("http is only allowed with confidential clients.") + raise InvalidOIDCRedirectURIError("http is only allowed with confidential clients.") if scheme not in application.get_allowed_schemes(): - raise MismatchingOIDCRedirectURIError(f'Redirect to scheme "{scheme}" is not permitted.') + raise InvalidOIDCRedirectURIError(f'Redirect to scheme "{scheme}" is not permitted.') if not application.post_logout_redirect_uri_allowed(post_logout_redirect_uri): - raise MismatchingOIDCRedirectURIError("This client does not have this redirect uri registered.") + raise InvalidOIDCRedirectURIError("This client does not have this redirect uri registered.") return prompt_logout, (post_logout_redirect_uri, application) diff --git a/tests/test_oidc_views.py b/tests/test_oidc_views.py index 649996d17..78bfe249d 100644 --- a/tests/test_oidc_views.py +++ b/tests/test_oidc_views.py @@ -3,11 +3,7 @@ from django.test import TestCase from django.urls import reverse -from oauth2_provider.exceptions import ( - ClientIdMissmatch, - InvalidOIDCClientError, - MismatchingOIDCRedirectURIError, -) +from oauth2_provider.exceptions import ClientIdMissmatch, InvalidOIDCClientError, InvalidOIDCRedirectURIError from oauth2_provider.oauth2_validators import OAuth2Validator from oauth2_provider.settings import oauth2_settings from oauth2_provider.views.oidc import validate_logout_request @@ -221,21 +217,21 @@ def test_validate_logout_request(oidc_tokens, public_application, other_user, rp client_id=None, post_logout_redirect_uri="http://example.org", ) - with pytest.raises(MismatchingOIDCRedirectURIError): + with pytest.raises(InvalidOIDCRedirectURIError): validate_logout_request( user=oidc_tokens.user, id_token_hint=None, client_id=client_id, post_logout_redirect_uri="example.org", ) - with pytest.raises(MismatchingOIDCRedirectURIError): + with pytest.raises(InvalidOIDCRedirectURIError): validate_logout_request( user=oidc_tokens.user, id_token_hint=None, client_id=client_id, post_logout_redirect_uri="imap://example.org", ) - with pytest.raises(MismatchingOIDCRedirectURIError): + with pytest.raises(InvalidOIDCRedirectURIError): validate_logout_request( user=oidc_tokens.user, id_token_hint=None, From 930fdc176374ad9bd3378f45d396b692a8cf941a Mon Sep 17 00:00:00 2001 From: Julian Mundhahs Date: Mon, 6 Feb 2023 14:41:23 +0100 Subject: [PATCH 09/18] Shorten Logout URL --- oauth2_provider/urls.py | 2 +- tests/test_oidc_views.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/oauth2_provider/urls.py b/oauth2_provider/urls.py index ba76b7453..4d23a3a5f 100644 --- a/oauth2_provider/urls.py +++ b/oauth2_provider/urls.py @@ -38,7 +38,7 @@ ), re_path(r"^\.well-known/jwks.json$", views.JwksInfoView.as_view(), name="jwks-info"), re_path(r"^userinfo/$", views.UserInfoView.as_view(), name="user-info"), - re_path(r"^rp-initiated-logout/$", views.RPInitiatedLogoutView.as_view(), name="rp-initiated-logout"), + re_path(r"^logout/$", views.RPInitiatedLogoutView.as_view(), name="rp-initiated-logout"), ] diff --git a/tests/test_oidc_views.py b/tests/test_oidc_views.py index 78bfe249d..56e4167a8 100644 --- a/tests/test_oidc_views.py +++ b/tests/test_oidc_views.py @@ -61,7 +61,7 @@ def expect_json_response_with_rp(self, base): "id_token_signing_alg_values_supported": ["RS256", "HS256"], "token_endpoint_auth_methods_supported": ["client_secret_post", "client_secret_basic"], "claims_supported": ["sub"], - "end_session_endpoint": f"{base}/rp-initiated-logout/", + "end_session_endpoint": f"{base}/logout/", } response = self.client.get(reverse("oauth2_provider:oidc-connect-discovery-info")) self.assertEqual(response.status_code, 200) From e2236bc85a75d18380a4e75b70cc096752dfd914 Mon Sep 17 00:00:00 2001 From: Julian Mundhahs Date: Mon, 6 Feb 2023 15:40:43 +0100 Subject: [PATCH 10/18] Add `OIDCLogoutOnlyMixin` to `RPInitiatedLogoutView` --- oauth2_provider/views/mixins.py | 24 +++++++++++++++++++++ oauth2_provider/views/oidc.py | 4 ++-- tests/test_mixins.py | 38 +++++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 2 deletions(-) diff --git a/oauth2_provider/views/mixins.py b/oauth2_provider/views/mixins.py index ebb654216..b3d9ab2f2 100644 --- a/oauth2_provider/views/mixins.py +++ b/oauth2_provider/views/mixins.py @@ -326,3 +326,27 @@ def dispatch(self, *args, **kwargs): log.warning(self.debug_error_message) return HttpResponseNotFound() return super().dispatch(*args, **kwargs) + + +class OIDCLogoutOnlyMixin(OIDCOnlyMixin): + """ + Mixin for views that should only be accessible when OIDC and OIDC RP-Initiated Logout are enabled. + + If either is not enabled: + + * if DEBUG is True, raises an ImproperlyConfigured exception explaining why + * otherwise, returns a 404 response, logging the same warning + """ + + debug_error_message = ( + "The django-oauth-toolkit OIDC RP-Initiated Logout view is not enabled unless you " + "have configured OIDC_RP_INITIATED_LOGOUT_ENABLED in the settings" + ) + + def dispatch(self, *args, **kwargs): + if not oauth2_settings.OIDC_RP_INITIATED_LOGOUT_ENABLED: + if settings.DEBUG: + raise ImproperlyConfigured(self.debug_error_message) + log.warning(self.debug_error_message) + return HttpResponseNotFound() + return super().dispatch(*args, **kwargs) diff --git a/oauth2_provider/views/oidc.py b/oauth2_provider/views/oidc.py index 14a24ede6..68652ba29 100644 --- a/oauth2_provider/views/oidc.py +++ b/oauth2_provider/views/oidc.py @@ -22,7 +22,7 @@ from ..http import OAuth2ResponseRedirect from ..models import get_application_model from ..settings import oauth2_settings -from .mixins import OAuthLibMixin, OIDCOnlyMixin +from .mixins import OAuthLibMixin, OIDCLogoutOnlyMixin, OIDCOnlyMixin Application = get_application_model() @@ -206,7 +206,7 @@ def validate_logout_request(user, id_token_hint, client_id, post_logout_redirect return prompt_logout, (post_logout_redirect_uri, application) -class RPInitiatedLogoutView(OIDCOnlyMixin, FormView): +class RPInitiatedLogoutView(OIDCLogoutOnlyMixin, FormView): template_name = "oauth2_provider/logout_confirm.html" form_class = ConfirmLogoutForm diff --git a/tests/test_mixins.py b/tests/test_mixins.py index 1294b75cb..327a99194 100644 --- a/tests/test_mixins.py +++ b/tests/test_mixins.py @@ -11,6 +11,7 @@ from oauth2_provider.oauth2_validators import OAuth2Validator from oauth2_provider.views.mixins import ( OAuthLibMixin, + OIDCLogoutOnlyMixin, OIDCOnlyMixin, ProtectedResourceMixin, ScopedResourceMixin, @@ -145,6 +146,15 @@ def get(self, *args, **kwargs): return TView.as_view() +@pytest.fixture +def oidc_logout_only_view(): + class TView(OIDCLogoutOnlyMixin, View): + def get(self, *args, **kwargs): + return HttpResponse("OK") + + return TView.as_view() + + @pytest.mark.oauth2_settings(presets.OIDC_SETTINGS_RW) def test_oidc_only_mixin_oidc_enabled(oauth2_settings, rf, oidc_only_view): assert oauth2_settings.OIDC_ENABLED @@ -153,6 +163,14 @@ def test_oidc_only_mixin_oidc_enabled(oauth2_settings, rf, oidc_only_view): assert rsp.content.decode("utf-8") == "OK" +@pytest.mark.oauth2_settings(presets.OIDC_SETTINGS_RP_LOGOUT) +def test_oidc_logout_only_mixin_oidc_enabled(oauth2_settings, rf, oidc_only_view): + assert oauth2_settings.OIDC_RP_INITIATED_LOGOUT_ENABLED + rsp = oidc_only_view(rf.get("/")) + assert rsp.status_code == 200 + assert rsp.content.decode("utf-8") == "OK" + + def test_oidc_only_mixin_oidc_disabled_debug(oauth2_settings, rf, settings, oidc_only_view): assert oauth2_settings.OIDC_ENABLED is False settings.DEBUG = True @@ -161,6 +179,14 @@ def test_oidc_only_mixin_oidc_disabled_debug(oauth2_settings, rf, settings, oidc assert "OIDC views are not enabled" in str(exc.value) +def test_oidc_logout_only_mixin_oidc_disabled_debug(oauth2_settings, rf, settings, oidc_logout_only_view): + assert oauth2_settings.OIDC_RP_INITIATED_LOGOUT_ENABLED is False + settings.DEBUG = True + with pytest.raises(ImproperlyConfigured) as exc: + oidc_logout_only_view(rf.get("/")) + assert str(exc.value) == OIDCLogoutOnlyMixin.debug_error_message + + def test_oidc_only_mixin_oidc_disabled_no_debug(oauth2_settings, rf, settings, oidc_only_view, caplog): assert oauth2_settings.OIDC_ENABLED is False settings.DEBUG = False @@ -169,3 +195,15 @@ def test_oidc_only_mixin_oidc_disabled_no_debug(oauth2_settings, rf, settings, o assert rsp.status_code == 404 assert len(caplog.records) == 1 assert "OIDC views are not enabled" in caplog.records[0].message + + +def test_oidc_logout_only_mixin_oidc_disabled_no_debug( + oauth2_settings, rf, settings, oidc_logout_only_view, caplog +): + assert oauth2_settings.OIDC_RP_INITIATED_LOGOUT_ENABLED is False + settings.DEBUG = False + with caplog.at_level(logging.WARNING, logger="oauth2_provider"): + rsp = oidc_logout_only_view(rf.get("/")) + assert rsp.status_code == 404 + assert len(caplog.records) == 1 + assert caplog.records[0].message == OIDCLogoutOnlyMixin.debug_error_message From 05a8124180ddb89a3c89c6ab0ae1efc7115df148 Mon Sep 17 00:00:00 2001 From: Julian Mundhahs Date: Tue, 7 Mar 2023 10:39:49 +0100 Subject: [PATCH 11/18] Add configuration option to accept expired tokens --- docs/settings.rst | 6 ++ oauth2_provider/settings.py | 1 + oauth2_provider/views/oidc.py | 60 +++++++++++++++++--- tests/conftest.py | 53 ++++++++++++++++- tests/presets.py | 2 + tests/test_oidc_views.py | 104 +++++++++++++++++++++++++++++----- 6 files changed, 203 insertions(+), 23 deletions(-) diff --git a/docs/settings.rst b/docs/settings.rst index d2bf6ce7e..11608b949 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -328,6 +328,12 @@ Default: ``True`` Whether to always prompt the :term:`Resource Owner` (End User) to confirm a logout requested by a :term:`Client` (Relying Party). If it is disabled the :term:`Resource Owner` (End User) will only be prompted if required by the standard. +OIDC_RP_INITIATED_LOGOUT_ACCEPT_EXPIRED_TOKENS +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Default: ``True`` + +Whether expired ID tokens are accepted for RP-Initiated Logout. The Tokens must still be signed by the OP and otherwise valid. + OIDC_ISS_ENDPOINT ~~~~~~~~~~~~~~~~~ Default: ``""`` diff --git a/oauth2_provider/settings.py b/oauth2_provider/settings.py index 9390ed953..6d818316f 100644 --- a/oauth2_provider/settings.py +++ b/oauth2_provider/settings.py @@ -90,6 +90,7 @@ ], "OIDC_RP_INITIATED_LOGOUT_ENABLED": False, "OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT": True, + "OIDC_RP_INITIATED_LOGOUT_ACCEPT_EXPIRED_TOKENS": True, # Special settings that will be evaluated at runtime "_SCOPES": [], "_DEFAULT_SCOPES": [], diff --git a/oauth2_provider/views/oidc.py b/oauth2_provider/views/oidc.py index 68652ba29..200698f8a 100644 --- a/oauth2_provider/views/oidc.py +++ b/oauth2_provider/views/oidc.py @@ -7,7 +7,9 @@ from django.utils.decorators import method_decorator from django.views.decorators.csrf import csrf_exempt from django.views.generic import FormView, View -from jwcrypto import jwk +from jwcrypto import jwk, jwt +from jwcrypto.common import JWException +from jwcrypto.jwt import JWTExpired from oauthlib.common import add_params_to_uri from ..exceptions import ( @@ -20,7 +22,7 @@ ) from ..forms import ConfirmLogoutForm from ..http import OAuth2ResponseRedirect -from ..models import get_application_model +from ..models import get_application_model, get_id_token_model from ..settings import oauth2_settings from .mixins import OAuthLibMixin, OIDCLogoutOnlyMixin, OIDCOnlyMixin @@ -142,7 +144,50 @@ def _create_userinfo_response(self, request): return response -def validate_logout_request(user, id_token_hint, client_id, post_logout_redirect_uri): +def _load_id_token(request, token): + """ + Loads an IDToken given its string representation for use with RP-Initiated Logout. + This method differs from `oauth2_validators._load_id_token` in two ways. + This method validates the `iss` claim and might accept expired but otherwise valid tokens + depending on the configuration. + """ + IDToken = get_id_token_model() + validator = oauth2_settings.OAUTH2_VALIDATOR_CLASS() + + key = validator._get_key_for_token(token) + if not key: + return None + + try: + if oauth2_settings.OIDC_RP_INITIATED_LOGOUT_ACCEPT_EXPIRED_TOKENS: + # Only check the following while loading the JWT + # - claims are dict + # - the Claims defined in RFC7519 if present have the correct type (string, integer, etc.) + # The claim contents are not validated. `exp` and `nbf` in particular are not validated. + check_claims = {} + else: + # Also validate the `exp` (expiration time) and `nbf` (not before) claims. + check_claims = None + jwt_token = jwt.JWT(key=key, jwt=token, check_claims=check_claims) + claims = json.loads(jwt_token.claims) + + # Verification of `iss` claim is mandated by OIDC RP-Initiated Logout specs. + if claims["iss"] != validator.get_oidc_issuer_endpoint(request): + # IDToken was not issued by this OP. + return None + + # Assumption: the `sub` claim and `user` property of the corresponding IDToken Object point to the + # same user. + # To verify that the IDToken was intended for the user it is therefore sufficient to check the `user` + # attribute on the IDToken Object later on. + + return IDToken.objects.get(jti=claims["jti"]) + + except (JWException, JWTExpired, IDToken.DoesNotExist): + return None + + +def validate_logout_request(request, id_token_hint, client_id, post_logout_redirect_uri): """ Validate an OIDC RP-Initiated Logout Request. `(prompt_logout, (post_logout_redirect_uri, application))` is returned. @@ -156,19 +201,18 @@ def validate_logout_request(user, id_token_hint, client_id, post_logout_redirect The `id_token_hint` will be validated if given. If both `client_id` and `id_token_hint` are given they will be validated against each other. """ - validator = oauth2_settings.OAUTH2_VALIDATOR_CLASS() id_token = None must_prompt_logout = True if id_token_hint: # Note: The standard states that expired tokens should still be accepted. # This implementation only accepts tokens that are still valid. - id_token = validator._load_id_token(id_token_hint) + id_token = _load_id_token(request, id_token_hint) if not id_token: raise InvalidIDTokenError() - if id_token.user == user: + if id_token.user == request.user: # A logout without user interaction (i.e. no prompt) is only allowed # if an ID Token is provided that matches the current user. must_prompt_logout = False @@ -232,7 +276,7 @@ def get(self, request, *args, **kwargs): try: prompt, (redirect_uri, application) = validate_logout_request( - user=request.user, + request=request, id_token_hint=id_token_hint, client_id=client_id, post_logout_redirect_uri=post_logout_redirect_uri, @@ -264,7 +308,7 @@ def form_valid(self, form): try: prompt, (redirect_uri, application) = validate_logout_request( - user=self.request.user, + request=self.request, id_token_hint=id_token_hint, client_id=client_id, post_logout_redirect_uri=post_logout_redirect_uri, diff --git a/tests/conftest.py b/tests/conftest.py index 8e602cce7..3a88c5261 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,3 +1,5 @@ +import uuid +from datetime import timedelta from types import SimpleNamespace from urllib.parse import parse_qs, urlparse @@ -5,9 +7,10 @@ from django.conf import settings as test_settings from django.contrib.auth import get_user_model from django.urls import reverse -from jwcrypto import jwk +from django.utils import dateformat, timezone +from jwcrypto import jwk, jwt -from oauth2_provider.models import get_application_model +from oauth2_provider.models import get_application_model, get_id_token_model from oauth2_provider.settings import oauth2_settings as _oauth2_settings from . import presets @@ -196,6 +199,52 @@ def generate_access_token(oauth2_settings, application, test_user, client, setti ) +@pytest.fixture +def expired_id_token(oauth2_settings, oidc_key, test_user, application): + payload = generate_id_token_payload(oauth2_settings, application, oidc_key) + return generate_id_token(test_user, payload, oidc_key, application) + + +@pytest.fixture +def id_token_wrong_aud(oauth2_settings, oidc_key, test_user, application): + payload = generate_id_token_payload(oauth2_settings, application, oidc_key) + payload[1]["aud"] = "" + return generate_id_token(test_user, payload, oidc_key, application) + + +@pytest.fixture +def id_token_wrong_iss(oauth2_settings, oidc_key, test_user, application): + payload = generate_id_token_payload(oauth2_settings, application, oidc_key) + payload[1]["iss"] = "" + return generate_id_token(test_user, payload, oidc_key, application) + + +def generate_id_token_payload(oauth2_settings, application, oidc_key): + # Default leeway of JWT in jwcrypto is 60 seconds. This means that tokens that expired up to 60 seconds + # ago are still accepted. + expiration_time = timezone.now() - timedelta(seconds=61) + # Calculate values for the IDToken + exp = int(dateformat.format(expiration_time, "U")) + jti = str(uuid.uuid4()) + aud = application.client_id + iss = oauth2_settings.OIDC_ISS_ENDPOINT + # Construct and sign the IDToken + header = {"typ": "JWT", "alg": "RS256", "kid": oidc_key.thumbprint()} + id_token = {"exp": exp, "jti": jti, "aud": aud, "iss": iss} + return header, id_token, jti, expiration_time + + +def generate_id_token(user, payload, oidc_key, application): + header, id_token, jti, expiration_time = payload + jwt_token = jwt.JWT(header=header, claims=id_token) + jwt_token.make_signed_token(oidc_key) + # Save the IDToken in the DB. Required for later lookups from e.g. RP-Initiated Logout. + IDToken = get_id_token_model() + IDToken.objects.create(user=user, scope="", expires=expiration_time, jti=jti, application=application) + # Return the token as a string. + return jwt_token.token.serialize(compact=True) + + @pytest.fixture def oidc_tokens(oauth2_settings, application, test_user, client): return generate_access_token( diff --git a/tests/presets.py b/tests/presets.py index 26f2fee1f..39e0aaf7c 100644 --- a/tests/presets.py +++ b/tests/presets.py @@ -31,6 +31,8 @@ OIDC_SETTINGS_RP_LOGOUT = deepcopy(OIDC_SETTINGS_RW) OIDC_SETTINGS_RP_LOGOUT["OIDC_RP_INITIATED_LOGOUT_ENABLED"] = True OIDC_SETTINGS_RP_LOGOUT["OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT"] = False +OIDC_SETTINGS_RP_LOGOUT_DENY_EXPIRED = deepcopy(OIDC_SETTINGS_RP_LOGOUT) +OIDC_SETTINGS_RP_LOGOUT_DENY_EXPIRED["OIDC_RP_INITIATED_LOGOUT_ACCEPT_EXPIRED_TOKENS"] = False REST_FRAMEWORK_SCOPES = { "SCOPES": { "read": "Read scope", diff --git a/tests/test_oidc_views.py b/tests/test_oidc_views.py index 56e4167a8..cc353bb1d 100644 --- a/tests/test_oidc_views.py +++ b/tests/test_oidc_views.py @@ -1,12 +1,14 @@ import pytest from django.contrib.auth import get_user -from django.test import TestCase +from django.contrib.auth.models import AnonymousUser +from django.test import RequestFactory, TestCase from django.urls import reverse from oauth2_provider.exceptions import ClientIdMissmatch, InvalidOIDCClientError, InvalidOIDCRedirectURIError +from oauth2_provider.models import get_id_token_model from oauth2_provider.oauth2_validators import OAuth2Validator from oauth2_provider.settings import oauth2_settings -from oauth2_provider.views.oidc import validate_logout_request +from oauth2_provider.views.oidc import _load_id_token, validate_logout_request from . import presets @@ -165,6 +167,22 @@ def test_get_jwks_info_multiple_rsa_keys(self): assert response.json() == expected_response +def mock_request(): + """ + Dummy request with an AnonymousUser attached. + """ + return mock_request_for(AnonymousUser()) + + +def mock_request_for(user): + """ + Dummy request with the `user` attached. + """ + request = RequestFactory().get("") + request.user = user + return request + + @pytest.mark.django_db @pytest.mark.parametrize("ALWAYS_PROMPT", [True, False]) def test_validate_logout_request(oidc_tokens, public_application, other_user, rp_settings, ALWAYS_PROMPT): @@ -174,66 +192,72 @@ def test_validate_logout_request(oidc_tokens, public_application, other_user, rp client_id = application.client_id id_token = oidc_tokens.id_token assert validate_logout_request( - user=oidc_tokens.user, id_token_hint=None, client_id=None, post_logout_redirect_uri=None + request=mock_request_for(oidc_tokens.user), + id_token_hint=None, + client_id=None, + post_logout_redirect_uri=None, ) == (True, (None, None)) assert validate_logout_request( - user=oidc_tokens.user, id_token_hint=None, client_id=client_id, post_logout_redirect_uri=None + request=mock_request_for(oidc_tokens.user), + id_token_hint=None, + client_id=client_id, + post_logout_redirect_uri=None, ) == (True, (None, application)) assert validate_logout_request( - user=oidc_tokens.user, + request=mock_request_for(oidc_tokens.user), id_token_hint=None, client_id=client_id, post_logout_redirect_uri="http://example.org", ) == (True, ("http://example.org", application)) assert validate_logout_request( - user=oidc_tokens.user, + request=mock_request_for(oidc_tokens.user), id_token_hint=id_token, client_id=None, post_logout_redirect_uri="http://example.org", ) == (ALWAYS_PROMPT, ("http://example.org", application)) assert validate_logout_request( - user=other_user, + request=mock_request_for(other_user), id_token_hint=id_token, client_id=None, post_logout_redirect_uri="http://example.org", ) == (True, ("http://example.org", application)) assert validate_logout_request( - user=oidc_tokens.user, + request=mock_request_for(oidc_tokens.user), id_token_hint=id_token, client_id=client_id, post_logout_redirect_uri="http://example.org", ) == (ALWAYS_PROMPT, ("http://example.org", application)) with pytest.raises(ClientIdMissmatch): validate_logout_request( - user=oidc_tokens.user, + request=mock_request_for(oidc_tokens.user), id_token_hint=id_token, client_id=public_application.client_id, post_logout_redirect_uri="http://other.org", ) with pytest.raises(InvalidOIDCClientError): validate_logout_request( - user=oidc_tokens.user, + request=mock_request_for(oidc_tokens.user), id_token_hint=None, client_id=None, post_logout_redirect_uri="http://example.org", ) with pytest.raises(InvalidOIDCRedirectURIError): validate_logout_request( - user=oidc_tokens.user, + request=mock_request_for(oidc_tokens.user), id_token_hint=None, client_id=client_id, post_logout_redirect_uri="example.org", ) with pytest.raises(InvalidOIDCRedirectURIError): validate_logout_request( - user=oidc_tokens.user, + request=mock_request_for(oidc_tokens.user), id_token_hint=None, client_id=client_id, post_logout_redirect_uri="imap://example.org", ) with pytest.raises(InvalidOIDCRedirectURIError): validate_logout_request( - user=oidc_tokens.user, + request=mock_request_for(oidc_tokens.user), id_token_hint=None, client_id=client_id, post_logout_redirect_uri="http://other.org", @@ -354,6 +378,60 @@ def test_rp_initiated_logout_post_allowed(loggend_in_client, oidc_tokens, rp_set assert not is_logged_in(loggend_in_client) +@pytest.mark.django_db +@pytest.mark.oauth2_settings(presets.OIDC_SETTINGS_RP_LOGOUT) +def test_rp_initiated_logout_expired_tokens_accept(loggend_in_client, application, expired_id_token): + # Accepting expired (but otherwise valid and signed by us) tokens is enabled. Logout should go through. + rsp = loggend_in_client.get( + reverse("oauth2_provider:rp-initiated-logout"), + data={ + "id_token_hint": expired_id_token, + "client_id": application.client_id, + }, + ) + assert rsp.status_code == 302 + assert not is_logged_in(loggend_in_client) + + +@pytest.mark.django_db +@pytest.mark.oauth2_settings(presets.OIDC_SETTINGS_RP_LOGOUT_DENY_EXPIRED) +def test_rp_initiated_logout_expired_tokens_deny(loggend_in_client, application, expired_id_token): + # Expired tokens should not be accepted by default. + rsp = loggend_in_client.get( + reverse("oauth2_provider:rp-initiated-logout"), + data={ + "id_token_hint": expired_id_token, + "client_id": application.client_id, + }, + ) + assert rsp.status_code == 400 + assert is_logged_in(loggend_in_client) + + +@pytest.mark.django_db +@pytest.mark.oauth2_settings(presets.OIDC_SETTINGS_RP_LOGOUT) +def test_load_id_token_accept_expired(expired_id_token): + assert isinstance(_load_id_token(mock_request(), expired_id_token), get_id_token_model()) + + +@pytest.mark.django_db +@pytest.mark.oauth2_settings(presets.OIDC_SETTINGS_RP_LOGOUT) +def test_load_id_token_wrong_aud(id_token_wrong_aud): + assert _load_id_token(mock_request(), id_token_wrong_aud) is None + + +@pytest.mark.django_db +@pytest.mark.oauth2_settings(presets.OIDC_SETTINGS_RP_LOGOUT) +def test_load_id_token_wrong_iss(id_token_wrong_iss): + assert _load_id_token(mock_request(), id_token_wrong_iss) is None + + +@pytest.mark.django_db +@pytest.mark.oauth2_settings(presets.OIDC_SETTINGS_RP_LOGOUT_DENY_EXPIRED) +def test_load_id_token_deny_expired(expired_id_token): + assert _load_id_token(mock_request(), expired_id_token) is None + + @pytest.mark.django_db @pytest.mark.parametrize("method", ["get", "post"]) def test_userinfo_endpoint(oidc_tokens, client, method): From 67a5208274323579cbf7ca9d38f7a7a056b0fc67 Mon Sep 17 00:00:00 2001 From: Julian Mundhahs Date: Fri, 10 Mar 2023 11:48:10 +0100 Subject: [PATCH 12/18] Split IDToken loading and claim validation into two functions --- oauth2_provider/views/oidc.py | 39 +++++++++++++++++++++-------------- tests/test_oidc_views.py | 32 +++++++++++++++++++++------- 2 files changed, 47 insertions(+), 24 deletions(-) diff --git a/oauth2_provider/views/oidc.py b/oauth2_provider/views/oidc.py index 200698f8a..239cc23eb 100644 --- a/oauth2_provider/views/oidc.py +++ b/oauth2_provider/views/oidc.py @@ -144,19 +144,18 @@ def _create_userinfo_response(self, request): return response -def _load_id_token(request, token): +def _load_id_token(token): """ Loads an IDToken given its string representation for use with RP-Initiated Logout. - This method differs from `oauth2_validators._load_id_token` in two ways. - This method validates the `iss` claim and might accept expired but otherwise valid tokens - depending on the configuration. + A tuple (IDToken, claims) is returned. Depending on the configuration expired tokens may be loaded. + If loading failed (None, None) is returned. """ IDToken = get_id_token_model() validator = oauth2_settings.OAUTH2_VALIDATOR_CLASS() key = validator._get_key_for_token(token) if not key: - return None + return None, None try: if oauth2_settings.OIDC_RP_INITIATED_LOGOUT_ACCEPT_EXPIRED_TOKENS: @@ -171,20 +170,29 @@ def _load_id_token(request, token): jwt_token = jwt.JWT(key=key, jwt=token, check_claims=check_claims) claims = json.loads(jwt_token.claims) - # Verification of `iss` claim is mandated by OIDC RP-Initiated Logout specs. - if claims["iss"] != validator.get_oidc_issuer_endpoint(request): - # IDToken was not issued by this OP. - return None - # Assumption: the `sub` claim and `user` property of the corresponding IDToken Object point to the # same user. # To verify that the IDToken was intended for the user it is therefore sufficient to check the `user` # attribute on the IDToken Object later on. - return IDToken.objects.get(jti=claims["jti"]) + return IDToken.objects.get(jti=claims["jti"]), claims except (JWException, JWTExpired, IDToken.DoesNotExist): - return None + return None, None + + +def _validate_claims(request, claims): + """ + Validates the claims of an IDToken for use with OIDC RP-Initiated Logout. + """ + validator = oauth2_settings.OAUTH2_VALIDATOR_CLASS() + + # Verification of `iss` claim is mandated by OIDC RP-Initiated Logout specs. + if "iss" not in claims or claims["iss"] != validator.get_oidc_issuer_endpoint(request): + # IDToken was not issued by this OP, or it can not be verified. + return False + + return True def validate_logout_request(request, id_token_hint, client_id, post_logout_redirect_uri): @@ -205,11 +213,10 @@ def validate_logout_request(request, id_token_hint, client_id, post_logout_redir id_token = None must_prompt_logout = True if id_token_hint: - # Note: The standard states that expired tokens should still be accepted. - # This implementation only accepts tokens that are still valid. - id_token = _load_id_token(request, id_token_hint) + # Only basic validation has been done on the IDToken at this point. + id_token, claims = _load_id_token(id_token_hint) - if not id_token: + if not id_token or not _validate_claims(request, claims): raise InvalidIDTokenError() if id_token.user == request.user: diff --git a/tests/test_oidc_views.py b/tests/test_oidc_views.py index cc353bb1d..3ea6e3dff 100644 --- a/tests/test_oidc_views.py +++ b/tests/test_oidc_views.py @@ -8,7 +8,7 @@ from oauth2_provider.models import get_id_token_model from oauth2_provider.oauth2_validators import OAuth2Validator from oauth2_provider.settings import oauth2_settings -from oauth2_provider.views.oidc import _load_id_token, validate_logout_request +from oauth2_provider.views.oidc import _load_id_token, _validate_claims, validate_logout_request from . import presets @@ -411,25 +411,41 @@ def test_rp_initiated_logout_expired_tokens_deny(loggend_in_client, application, @pytest.mark.django_db @pytest.mark.oauth2_settings(presets.OIDC_SETTINGS_RP_LOGOUT) def test_load_id_token_accept_expired(expired_id_token): - assert isinstance(_load_id_token(mock_request(), expired_id_token), get_id_token_model()) + id_token, _ = _load_id_token(expired_id_token) + assert isinstance(id_token, get_id_token_model()) @pytest.mark.django_db @pytest.mark.oauth2_settings(presets.OIDC_SETTINGS_RP_LOGOUT) def test_load_id_token_wrong_aud(id_token_wrong_aud): - assert _load_id_token(mock_request(), id_token_wrong_aud) is None + id_token, claims = _load_id_token(id_token_wrong_aud) + assert id_token is None + assert claims is None + + +@pytest.mark.django_db +@pytest.mark.oauth2_settings(presets.OIDC_SETTINGS_RP_LOGOUT_DENY_EXPIRED) +def test_load_id_token_deny_expired(expired_id_token): + id_token, claims = _load_id_token(expired_id_token) + assert id_token is None + assert claims is None @pytest.mark.django_db @pytest.mark.oauth2_settings(presets.OIDC_SETTINGS_RP_LOGOUT) -def test_load_id_token_wrong_iss(id_token_wrong_iss): - assert _load_id_token(mock_request(), id_token_wrong_iss) is None +def test_validate_claims_wrong_iss(id_token_wrong_iss): + id_token, claims = _load_id_token(id_token_wrong_iss) + assert id_token is not None + assert claims is not None + assert not _validate_claims(mock_request(), claims) @pytest.mark.django_db -@pytest.mark.oauth2_settings(presets.OIDC_SETTINGS_RP_LOGOUT_DENY_EXPIRED) -def test_load_id_token_deny_expired(expired_id_token): - assert _load_id_token(mock_request(), expired_id_token) is None +@pytest.mark.oauth2_settings(presets.OIDC_SETTINGS_RP_LOGOUT) +def test_validate_claims(oidc_tokens): + id_token, claims = _load_id_token(oidc_tokens.id_token) + assert claims is not None + assert _validate_claims(mock_request_for(oidc_tokens.user), claims) @pytest.mark.django_db From 371f091ed29358f8d498559c1bec11534e4b4046 Mon Sep 17 00:00:00 2001 From: Julian Mundhahs Date: Fri, 10 Mar 2023 12:04:33 +0100 Subject: [PATCH 13/18] Add configuration to delete AccessTokens on Logout --- docs/settings.rst | 8 +++++ oauth2_provider/settings.py | 1 + oauth2_provider/views/oidc.py | 33 ++++++++++++++++++++- tests/presets.py | 2 ++ tests/test_oidc_views.py | 55 ++++++++++++++++++++++++++++++++++- 5 files changed, 97 insertions(+), 2 deletions(-) diff --git a/docs/settings.rst b/docs/settings.rst index 11608b949..9baa031bd 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -334,6 +334,14 @@ Default: ``True`` Whether expired ID tokens are accepted for RP-Initiated Logout. The Tokens must still be signed by the OP and otherwise valid. +OIDC_RP_INITIATED_LOGOUT_DELETE_TOKENS +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Default: ``True`` + +Whether to delete the access, refresh and ID tokens of the user that is being logged out. +The types of applications for which tokens are deleted can be customized with `RPInitiatedLogoutView.token_types_to_delete`. +The default is to delete the tokens of all applications if this flag is enabled. + OIDC_ISS_ENDPOINT ~~~~~~~~~~~~~~~~~ Default: ``""`` diff --git a/oauth2_provider/settings.py b/oauth2_provider/settings.py index 6d818316f..fa821d922 100644 --- a/oauth2_provider/settings.py +++ b/oauth2_provider/settings.py @@ -91,6 +91,7 @@ "OIDC_RP_INITIATED_LOGOUT_ENABLED": False, "OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT": True, "OIDC_RP_INITIATED_LOGOUT_ACCEPT_EXPIRED_TOKENS": True, + "OIDC_RP_INITIATED_LOGOUT_DELETE_TOKENS": True, # Special settings that will be evaluated at runtime "_SCOPES": [], "_DEFAULT_SCOPES": [], diff --git a/oauth2_provider/views/oidc.py b/oauth2_provider/views/oidc.py index 239cc23eb..7c4be2760 100644 --- a/oauth2_provider/views/oidc.py +++ b/oauth2_provider/views/oidc.py @@ -22,7 +22,12 @@ ) from ..forms import ConfirmLogoutForm from ..http import OAuth2ResponseRedirect -from ..models import get_application_model, get_id_token_model +from ..models import ( + get_access_token_model, + get_application_model, + get_id_token_model, + get_refresh_token_model, +) from ..settings import oauth2_settings from .mixins import OAuthLibMixin, OIDCLogoutOnlyMixin, OIDCOnlyMixin @@ -260,6 +265,10 @@ def validate_logout_request(request, id_token_hint, client_id, post_logout_redir class RPInitiatedLogoutView(OIDCLogoutOnlyMixin, FormView): template_name = "oauth2_provider/logout_confirm.html" form_class = ConfirmLogoutForm + token_types_to_delete = [ + Application.CLIENT_PUBLIC, + Application.CLIENT_CONFIDENTIAL, + ] def get_initial(self): return { @@ -330,7 +339,29 @@ def form_valid(self, form): return self.error_response(error) def do_logout(self, application=None, post_logout_redirect_uri=None, state=None): + # Delete Access Tokens + if oauth2_settings.OIDC_RP_INITIATED_LOGOUT_DELETE_TOKENS: + AccessToken = get_access_token_model() + RefreshToken = get_refresh_token_model() + access_tokens_to_delete = AccessToken.objects.filter( + user=self.request.user, application__client_type__in=self.token_types_to_delete + ) + # This queryset has to be evaluated eagerly. The queryset would be empty with lazy evaluation + # because `access_tokens_to_delete` represents an empty queryset once `refresh_tokens_to_delete` + # is evaluated as all AccessTokens have been deleted. + refresh_tokens_to_delete = list( + RefreshToken.objects.filter(access_token__in=access_tokens_to_delete) + ) + for token in access_tokens_to_delete: + # Delete the token and its corresponding refresh and IDTokens. + if token.id_token: + token.id_token.revoke() + token.revoke() + for refresh_token in refresh_tokens_to_delete: + refresh_token.revoke() + # Logout in Django logout(self.request) + # Redirect if post_logout_redirect_uri: if state: return OAuth2ResponseRedirect( diff --git a/tests/presets.py b/tests/presets.py index 39e0aaf7c..9a19f372c 100644 --- a/tests/presets.py +++ b/tests/presets.py @@ -33,6 +33,8 @@ OIDC_SETTINGS_RP_LOGOUT["OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT"] = False OIDC_SETTINGS_RP_LOGOUT_DENY_EXPIRED = deepcopy(OIDC_SETTINGS_RP_LOGOUT) OIDC_SETTINGS_RP_LOGOUT_DENY_EXPIRED["OIDC_RP_INITIATED_LOGOUT_ACCEPT_EXPIRED_TOKENS"] = False +OIDC_SETTINGS_RP_LOGOUT_KEEP_TOKENS = deepcopy(OIDC_SETTINGS_RP_LOGOUT) +OIDC_SETTINGS_RP_LOGOUT_KEEP_TOKENS["OIDC_RP_INITIATED_LOGOUT_DELETE_TOKENS"] = False REST_FRAMEWORK_SCOPES = { "SCOPES": { "read": "Read scope", diff --git a/tests/test_oidc_views.py b/tests/test_oidc_views.py index 3ea6e3dff..28f4232b1 100644 --- a/tests/test_oidc_views.py +++ b/tests/test_oidc_views.py @@ -3,9 +3,10 @@ from django.contrib.auth.models import AnonymousUser from django.test import RequestFactory, TestCase from django.urls import reverse +from django.utils import timezone from oauth2_provider.exceptions import ClientIdMissmatch, InvalidOIDCClientError, InvalidOIDCRedirectURIError -from oauth2_provider.models import get_id_token_model +from oauth2_provider.models import get_access_token_model, get_id_token_model, get_refresh_token_model from oauth2_provider.oauth2_validators import OAuth2Validator from oauth2_provider.settings import oauth2_settings from oauth2_provider.views.oidc import _load_id_token, _validate_claims, validate_logout_request @@ -474,6 +475,58 @@ def test_userinfo_endpoint_bad_token(oidc_tokens, client): assert rsp.status_code == 401 +@pytest.mark.django_db +def test_token_deletion_on_logout(oidc_tokens, loggend_in_client, rp_settings): + AccessToken = get_access_token_model() + IDToken = get_id_token_model() + RefreshToken = get_refresh_token_model() + assert AccessToken.objects.count() == 1 + assert IDToken.objects.count() == 1 + assert RefreshToken.objects.count() == 1 + rsp = loggend_in_client.get( + reverse("oauth2_provider:rp-initiated-logout"), + data={ + "id_token_hint": oidc_tokens.id_token, + "client_id": oidc_tokens.application.client_id, + }, + ) + assert rsp.status_code == 302 + assert not is_logged_in(loggend_in_client) + # Check that all tokens have either been deleted or expired. + assert all([token.is_expired() for token in AccessToken.objects.all()]) + assert all([token.is_expired() for token in IDToken.objects.all()]) + assert all([token.revoked <= timezone.now() for token in RefreshToken.objects.all()]) + + +@pytest.mark.django_db +@pytest.mark.oauth2_settings(presets.OIDC_SETTINGS_RP_LOGOUT_KEEP_TOKENS) +def test_token_deletion_on_logout_disabled(oidc_tokens, loggend_in_client, rp_settings): + rp_settings.OIDC_RP_INITIATED_LOGOUT_DELETE_TOKENS = False + + AccessToken = get_access_token_model() + IDToken = get_id_token_model() + RefreshToken = get_refresh_token_model() + assert AccessToken.objects.count() == 1 + assert IDToken.objects.count() == 1 + assert RefreshToken.objects.count() == 1 + rsp = loggend_in_client.get( + reverse("oauth2_provider:rp-initiated-logout"), + data={ + "id_token_hint": oidc_tokens.id_token, + "client_id": oidc_tokens.application.client_id, + }, + ) + assert rsp.status_code == 302 + assert not is_logged_in(loggend_in_client) + # Check that the tokens have not been expired or deleted. + assert AccessToken.objects.count() == 1 + assert not any([token.is_expired() for token in AccessToken.objects.all()]) + assert IDToken.objects.count() == 1 + assert not any([token.is_expired() for token in IDToken.objects.all()]) + assert RefreshToken.objects.count() == 1 + assert not any([token.revoked is not None for token in RefreshToken.objects.all()]) + + EXAMPLE_EMAIL = "example.email@example.com" From ba4d7655ecc4086b8dbe1ac5d69496a8bcd31b9b Mon Sep 17 00:00:00 2001 From: Julian Mundhahs Date: Tue, 21 Mar 2023 22:34:35 +0100 Subject: [PATCH 14/18] Add configuration by authorization grant type for OIDC-RP Logout token deletion --- oauth2_provider/views/oidc.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/oauth2_provider/views/oidc.py b/oauth2_provider/views/oidc.py index 7c4be2760..0d9ae535d 100644 --- a/oauth2_provider/views/oidc.py +++ b/oauth2_provider/views/oidc.py @@ -265,10 +265,19 @@ def validate_logout_request(request, id_token_hint, client_id, post_logout_redir class RPInitiatedLogoutView(OIDCLogoutOnlyMixin, FormView): template_name = "oauth2_provider/logout_confirm.html" form_class = ConfirmLogoutForm - token_types_to_delete = [ + # Only delete tokens for Application whose client type and authorization + # grant type are in the respective lists. + token_deletion_client_types = [ Application.CLIENT_PUBLIC, Application.CLIENT_CONFIDENTIAL, ] + token_deletion_grant_types = [ + Application.GRANT_AUTHORIZATION_CODE, + Application.GRANT_IMPLICIT, + Application.GRANT_PASSWORD, + Application.GRANT_CLIENT_CREDENTIALS, + Application.GRANT_OPENID_HYBRID, + ] def get_initial(self): return { @@ -344,7 +353,9 @@ def do_logout(self, application=None, post_logout_redirect_uri=None, state=None) AccessToken = get_access_token_model() RefreshToken = get_refresh_token_model() access_tokens_to_delete = AccessToken.objects.filter( - user=self.request.user, application__client_type__in=self.token_types_to_delete + user=self.request.user, + application__client_type__in=self.token_deletion_client_types, + application__authorization_grant_type__in=self.token_deletion_grant_types, ) # This queryset has to be evaluated eagerly. The queryset would be empty with lazy evaluation # because `access_tokens_to_delete` represents an empty queryset once `refresh_tokens_to_delete` From 686530137264a4cc2690a11c4e82ae35430c9cf1 Mon Sep 17 00:00:00 2001 From: Julian Mundhahs Date: Thu, 11 May 2023 22:02:57 +0200 Subject: [PATCH 15/18] Fix Errors --- docs/oidc.rst | 2 +- oauth2_provider/views/oidc.py | 9 ++++++++- tests/test_oidc_views.py | 4 ++++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/docs/oidc.rst b/docs/oidc.rst index 2a69ceccc..c06af5c1a 100644 --- a/docs/oidc.rst +++ b/docs/oidc.rst @@ -160,7 +160,7 @@ This feature has to be enabled separately as it is an extension to the core stan # OIDC has to be enabled to use RP-Initiated Logout "OIDC_ENABLED": True, # Enable and configure RP-Initiated Logout - "OIDC_RP_INITIATED_LOGOUT": True, + "OIDC_RP_INITIATED_LOGOUT_ENABLED": True, "OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT": True, # ... any other settings you want } diff --git a/oauth2_provider/views/oidc.py b/oauth2_provider/views/oidc.py index 0d9ae535d..aafb461c5 100644 --- a/oauth2_provider/views/oidc.py +++ b/oauth2_provider/views/oidc.py @@ -9,6 +9,7 @@ from django.views.generic import FormView, View from jwcrypto import jwk, jwt from jwcrypto.common import JWException +from jwcrypto.jws import InvalidJWSObject from jwcrypto.jwt import JWTExpired from oauthlib.common import add_params_to_uri @@ -158,7 +159,13 @@ def _load_id_token(token): IDToken = get_id_token_model() validator = oauth2_settings.OAUTH2_VALIDATOR_CLASS() - key = validator._get_key_for_token(token) + try: + key = validator._get_key_for_token(token) + except InvalidJWSObject: + # Failed to deserialize the key. + return None, None + + # Could not identify key from the ID Token. if not key: return None, None diff --git a/tests/test_oidc_views.py b/tests/test_oidc_views.py index 28f4232b1..2ff5359af 100644 --- a/tests/test_oidc_views.py +++ b/tests/test_oidc_views.py @@ -265,6 +265,10 @@ def test_validate_logout_request(oidc_tokens, public_application, other_user, rp ) +def test__load_id_token(): + assert _load_id_token("Not a Valid ID Token.") == (None, None) + + def is_logged_in(client): return get_user(client).is_authenticated From 30cd41d601b124e166692b420721856757373ad7 Mon Sep 17 00:00:00 2001 From: Julian Mundhahs Date: Thu, 11 May 2023 22:56:40 +0200 Subject: [PATCH 16/18] Allow `http` scheme in post logout redirect URIs --- docs/settings.rst | 6 ++++++ oauth2_provider/settings.py | 1 + oauth2_provider/views/oidc.py | 4 +++- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/docs/settings.rst b/docs/settings.rst index 9baa031bd..249b026e5 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -328,6 +328,12 @@ Default: ``True`` Whether to always prompt the :term:`Resource Owner` (End User) to confirm a logout requested by a :term:`Client` (Relying Party). If it is disabled the :term:`Resource Owner` (End User) will only be prompted if required by the standard. +OIDC_RP_INITIATED_LOGOUT_STRICT_REDIRECT_URIS +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Default: ``False`` + +Enable to only allow the `http` scheme in post logout redirect URIs when a :term:`Client` is `confidential`. + OIDC_RP_INITIATED_LOGOUT_ACCEPT_EXPIRED_TOKENS ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Default: ``True`` diff --git a/oauth2_provider/settings.py b/oauth2_provider/settings.py index fa821d922..aa7de7351 100644 --- a/oauth2_provider/settings.py +++ b/oauth2_provider/settings.py @@ -90,6 +90,7 @@ ], "OIDC_RP_INITIATED_LOGOUT_ENABLED": False, "OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT": True, + "OIDC_RP_INITIATED_LOGOUT_STRICT_REDIRECT_URIS": False, "OIDC_RP_INITIATED_LOGOUT_ACCEPT_EXPIRED_TOKENS": True, "OIDC_RP_INITIATED_LOGOUT_DELETE_TOKENS": True, # Special settings that will be evaluated at runtime diff --git a/oauth2_provider/views/oidc.py b/oauth2_provider/views/oidc.py index aafb461c5..f819388b9 100644 --- a/oauth2_provider/views/oidc.py +++ b/oauth2_provider/views/oidc.py @@ -259,7 +259,9 @@ def validate_logout_request(request, id_token_hint, client_id, post_logout_redir scheme = urlparse(post_logout_redirect_uri)[0] if not scheme: raise InvalidOIDCRedirectURIError("A Scheme is required for the redirect URI.") - if scheme == "http" and application.client_type != "confidential": + if oauth2_settings.OIDC_RP_INITIATED_LOGOUT_STRICT_REDIRECT_URIS and ( + scheme == "http" and application.client_type != "confidential" + ): raise InvalidOIDCRedirectURIError("http is only allowed with confidential clients.") if scheme not in application.get_allowed_schemes(): raise InvalidOIDCRedirectURIError(f'Redirect to scheme "{scheme}" is not permitted.') From 3f7ecd7dc8a1decdf9e2ca0fff97bb0d2a257033 Mon Sep 17 00:00:00 2001 From: Julian Mundhahs Date: Fri, 12 May 2023 15:11:43 +0200 Subject: [PATCH 17/18] Add Tests and Documentation for option to disable `http` post logout URIs --- docs/advanced_topics.rst | 1 + docs/management_commands.rst | 4 ++++ .../management/commands/createapplication.py | 6 ++++++ oauth2_provider/models.py | 3 +++ tests/presets.py | 2 ++ tests/test_oidc_views.py | 17 +++++++++++++++++ 6 files changed, 33 insertions(+) diff --git a/docs/advanced_topics.rst b/docs/advanced_topics.rst index ecba6bcdd..12fd7c04a 100644 --- a/docs/advanced_topics.rst +++ b/docs/advanced_topics.rst @@ -20,6 +20,7 @@ logo, acceptance of some user agreement and so on. * :attr:`client_id` The client identifier issued to the client during the registration process as described in :rfc:`2.2` * :attr:`user` ref to a Django user * :attr:`redirect_uris` The list of allowed redirect uri. The string consists of valid URLs separated by space + * :attr:`post_logout_redirect_uris` The list of allowed redirect uris after an RP initiated logout. The string consists of valid URLs separated by space * :attr:`client_type` Client type as described in :rfc:`2.1` * :attr:`authorization_grant_type` Authorization flows available to the Application * :attr:`client_secret` Confidential secret issued to the client during the registration process as described in :rfc:`2.2` diff --git a/docs/management_commands.rst b/docs/management_commands.rst index 770543375..aa36e2ebf 100644 --- a/docs/management_commands.rst +++ b/docs/management_commands.rst @@ -38,6 +38,7 @@ The ``createapplication`` management command provides a shortcut to create a new usage: manage.py createapplication [-h] [--client-id CLIENT_ID] [--user USER] [--redirect-uris REDIRECT_URIS] + [--post-logout-redirect-uris POST_LOGOUT_REDIRECT_URIS] [--client-secret CLIENT_SECRET] [--name NAME] [--skip-authorization] [--algorithm ALGORITHM] [--version] @@ -64,6 +65,9 @@ The ``createapplication`` management command provides a shortcut to create a new --redirect-uris REDIRECT_URIS The redirect URIs, this must be a space separated string e.g 'URI1 URI2' + --post-logout-redirect-uris POST_LOGOUT_REDIRECT_URIS + The post logout redirect URIs, this must be a space + separated string e.g 'URI1 URI2' --client-secret CLIENT_SECRET The secret for this application --name NAME The name this application diff --git a/oauth2_provider/management/commands/createapplication.py b/oauth2_provider/management/commands/createapplication.py index 01a72377e..dcc46e765 100644 --- a/oauth2_provider/management/commands/createapplication.py +++ b/oauth2_provider/management/commands/createapplication.py @@ -37,6 +37,12 @@ def add_arguments(self, parser): type=str, help="The redirect URIs, this must be a space separated string e.g 'URI1 URI2'", ) + parser.add_argument( + "--post-logout-redirect-uris", + type=str, + help="The post logout redirect URIs, this must be a space separated string e.g 'URI1 URI2'", + default="", + ) parser.add_argument( "--client-secret", type=str, diff --git a/oauth2_provider/models.py b/oauth2_provider/models.py index 7acbe4113..3779ed491 100644 --- a/oauth2_provider/models.py +++ b/oauth2_provider/models.py @@ -52,6 +52,9 @@ class AbstractApplication(models.Model): * :attr:`user` ref to a Django user * :attr:`redirect_uris` The list of allowed redirect uri. The string consists of valid URLs separated by space + * :attr:`post_logout_redirect_uris` The list of allowed redirect uris after + an RP initiated logout. The string + consists of valid URLs separated by space * :attr:`client_type` Client type as described in :rfc:`2.1` * :attr:`authorization_grant_type` Authorization flows available to the Application diff --git a/tests/presets.py b/tests/presets.py index 9a19f372c..1ac8d3279 100644 --- a/tests/presets.py +++ b/tests/presets.py @@ -31,6 +31,8 @@ OIDC_SETTINGS_RP_LOGOUT = deepcopy(OIDC_SETTINGS_RW) OIDC_SETTINGS_RP_LOGOUT["OIDC_RP_INITIATED_LOGOUT_ENABLED"] = True OIDC_SETTINGS_RP_LOGOUT["OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT"] = False +OIDC_SETTINGS_RP_LOGOUT_STRICT_REDIRECT_URI = deepcopy(OIDC_SETTINGS_RP_LOGOUT) +OIDC_SETTINGS_RP_LOGOUT_STRICT_REDIRECT_URI["OIDC_RP_INITIATED_LOGOUT_STRICT_REDIRECT_URIS"] = True OIDC_SETTINGS_RP_LOGOUT_DENY_EXPIRED = deepcopy(OIDC_SETTINGS_RP_LOGOUT) OIDC_SETTINGS_RP_LOGOUT_DENY_EXPIRED["OIDC_RP_INITIATED_LOGOUT_ACCEPT_EXPIRED_TOKENS"] = False OIDC_SETTINGS_RP_LOGOUT_KEEP_TOKENS = deepcopy(OIDC_SETTINGS_RP_LOGOUT) diff --git a/tests/test_oidc_views.py b/tests/test_oidc_views.py index 2ff5359af..6ba100d89 100644 --- a/tests/test_oidc_views.py +++ b/tests/test_oidc_views.py @@ -343,6 +343,23 @@ def test_rp_initiated_logout_get_id_token_missmatch_client_id( def test_rp_initiated_logout_public_client_redirect_client_id( loggend_in_client, oidc_non_confidential_tokens, public_application, rp_settings ): + rsp = loggend_in_client.get( + reverse("oauth2_provider:rp-initiated-logout"), + data={ + "id_token_hint": oidc_non_confidential_tokens.id_token, + "client_id": public_application.client_id, + "post_logout_redirect_uri": "http://other.org", + }, + ) + assert rsp.status_code == 302 + assert not is_logged_in(loggend_in_client) + + +@pytest.mark.django_db +def test_rp_initiated_logout_public_client_strict_redirect_client_id( + loggend_in_client, oidc_non_confidential_tokens, public_application, oauth2_settings +): + oauth2_settings.update(presets.OIDC_SETTINGS_RP_LOGOUT_STRICT_REDIRECT_URI) rsp = loggend_in_client.get( reverse("oauth2_provider:rp-initiated-logout"), data={ From 84130520c7e1627b4a0eea17234ef75404d74f2d Mon Sep 17 00:00:00 2001 From: Julian Mundhahs Date: Fri, 12 May 2023 18:03:34 +0200 Subject: [PATCH 18/18] Fix mistake in settings documentation --- docs/settings.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/settings.rst b/docs/settings.rst index 249b026e5..f31aff533 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -332,7 +332,7 @@ OIDC_RP_INITIATED_LOGOUT_STRICT_REDIRECT_URIS ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Default: ``False`` -Enable to only allow the `http` scheme in post logout redirect URIs when a :term:`Client` is `confidential`. +Enable this setting to require `https` in post logout redirect URIs. `http` is only allowed when a :term:`Client` is `confidential`. OIDC_RP_INITIATED_LOGOUT_ACCEPT_EXPIRED_TOKENS ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~