Fix RP-initiated Logout with expired Django session (#1270)

* Fix RP-initiated Logout with exired django session * Update tests/test_oidc_views.py Co-authored-by: François Freitag <mail@franek.fr> * Update tests/test_oidc_views.py Co-authored-by: François Freitag <mail@franek.fr> * Update tests/test_oidc_views.py Co-authored-by: François Freitag <mail@franek.fr> * Check post logout redirection * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: François Freitag <mail@franek.fr> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
jazzband · May 20, 2023 · 29da530 · 29da530
1 parent 11294ab
commit 29da530
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 15 deletions.
diff --git a/AUTHORS b/AUTHORS
@@ -18,6 +18,7 @@ Allisson Azevedo
 Andrea Greco
 Andrej Zbín
 Andrew Chen Wang
+Antoine Laurent
 Anvesh Agarwal
 Aristóbulo Meneses
 Aryan Iyappan

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * #1211 documentation improve on 'AUTHORIZATION_CODE_EXPIRE_SECONDS'.
 * #1218 Confim support for Python 3.11.
 * #1222 Remove expired ID tokens alongside access tokens in `cleartokens` management command
+* #1270 Fix RP-initiated Logout with no available Django session
 
 ## [2.2.0] 2022-10-18
 

diff --git a/oauth2_provider/views/oidc.py b/oauth2_provider/views/oidc.py
@@ -210,27 +210,31 @@ def _validate_claims(request, claims):
 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.
+    `(prompt_logout, (post_logout_redirect_uri, application), token_user)` 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.
+    be set to the Application that is requesting the logout. `token_user` is the id_token user, which will
+    used to revoke the tokens if found.
 
     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.
     """
 
     id_token = None
     must_prompt_logout = True
+    token_user = None
     if 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 or not _validate_claims(request, claims):
             raise InvalidIDTokenError()
 
+        token_user = id_token.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.
@@ -268,7 +272,7 @@ def validate_logout_request(request, id_token_hint, client_id, post_logout_redir
         if not application.post_logout_redirect_uri_allowed(post_logout_redirect_uri):
             raise InvalidOIDCRedirectURIError("This client does not have this redirect uri registered.")
 
-    return prompt_logout, (post_logout_redirect_uri, application)
+    return prompt_logout, (post_logout_redirect_uri, application), token_user
 
 
 class RPInitiatedLogoutView(OIDCLogoutOnlyMixin, FormView):
@@ -309,7 +313,7 @@ def get(self, request, *args, **kwargs):
         state = request.GET.get("state")
 
         try:
-            prompt, (redirect_uri, application) = validate_logout_request(
+            prompt, (redirect_uri, application), token_user = validate_logout_request(
                 request=request,
                 id_token_hint=id_token_hint,
                 client_id=client_id,
@@ -319,7 +323,7 @@ def get(self, request, *args, **kwargs):
             return self.error_response(error)
 
         if not prompt:
-            return self.do_logout(application, redirect_uri, state)
+            return self.do_logout(application, redirect_uri, state, token_user)
 
         self.oidc_data = {
             "id_token_hint": id_token_hint,
@@ -341,28 +345,28 @@ def form_valid(self, form):
         state = form.cleaned_data.get("state")
 
         try:
-            prompt, (redirect_uri, application) = validate_logout_request(
+            prompt, (redirect_uri, application), token_user = validate_logout_request(
                 request=self.request,
                 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)
+                return self.do_logout(application, redirect_uri, state, token_user)
             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):
+    def do_logout(self, application=None, post_logout_redirect_uri=None, state=None, token_user=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,
+                user=token_user or self.request.user,
                 application__client_type__in=self.token_deletion_client_types,
                 application__authorization_grant_type__in=self.token_deletion_grant_types,
             )

diff --git a/tests/test_oidc_views.py b/tests/test_oidc_views.py
@@ -4,6 +4,7 @@
 from django.test import RequestFactory, TestCase
 from django.urls import reverse
 from django.utils import timezone
+from pytest_django.asserts import assertRedirects
 
 from oauth2_provider.exceptions import ClientIdMissmatch, InvalidOIDCClientError, InvalidOIDCRedirectURIError
 from oauth2_provider.models import get_access_token_model, get_id_token_model, get_refresh_token_model
@@ -197,37 +198,37 @@ def test_validate_logout_request(oidc_tokens, public_application, other_user, rp
         id_token_hint=None,
         client_id=None,
         post_logout_redirect_uri=None,
-    ) == (True, (None, None))
+    ) == (True, (None, None), None)
     assert validate_logout_request(
         request=mock_request_for(oidc_tokens.user),
         id_token_hint=None,
         client_id=client_id,
         post_logout_redirect_uri=None,
-    ) == (True, (None, application))
+    ) == (True, (None, application), None)
     assert validate_logout_request(
         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))
+    ) == (True, ("http://example.org", application), None)
     assert validate_logout_request(
         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))
+    ) == (ALWAYS_PROMPT, ("http://example.org", application), oidc_tokens.user)
     assert validate_logout_request(
         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))
+    ) == (True, ("http://example.org", application), oidc_tokens.user)
     assert validate_logout_request(
         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))
+    ) == (ALWAYS_PROMPT, ("http://example.org", application), oidc_tokens.user)
     with pytest.raises(ClientIdMissmatch):
         validate_logout_request(
             request=mock_request_for(oidc_tokens.user),
@@ -519,6 +520,47 @@ def test_token_deletion_on_logout(oidc_tokens, loggend_in_client, rp_settings):
     assert all([token.revoked <= timezone.now() for token in RefreshToken.objects.all()])
 
 
+@pytest.mark.django_db
+def test_token_deletion_on_logout_expired_session(oidc_tokens, 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 = 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 == 200
+    assert not is_logged_in(client)
+    # Check that all tokens are active.
+    access_token = AccessToken.objects.get()
+    assert not access_token.is_expired()
+    id_token = IDToken.objects.get()
+    assert not id_token.is_expired()
+    refresh_token = RefreshToken.objects.get()
+    assert refresh_token.revoked is None
+
+    rsp = client.post(
+        reverse("oauth2_provider:rp-initiated-logout"),
+        data={
+            "id_token_hint": oidc_tokens.id_token,
+            "client_id": oidc_tokens.application.client_id,
+            "allow": True,
+        },
+    )
+    assertRedirects(rsp, "http://testserver/", fetch_redirect_response=False)
+    assert not is_logged_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):