logout refactor : split prompt checks from logout request validation

Author: tonial

oauth2_provider/views/oidc.py

@@ -212,44 +212,28 @@ 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, application, token_user)` is returned.
+    `(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`.
-    If it is set `application` will also
-    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.
+    If it is set, `application` is 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.
-            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:
@@ -273,7 +257,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, application, token_user
+    return application, id_token.user if id_token else None
 
 
 class RPInitiatedLogoutView(OIDCLogoutOnlyMixin, FormView):
@@ -314,7 +298,7 @@ def get(self, request, *args, **kwargs):
         state = request.GET.get("state")
 
         try:
-            prompt, application, token_user = validate_logout_request(
+            application, token_user = validate_logout_request(
                 request=request,
                 id_token_hint=id_token_hint,
                 client_id=client_id,
@@ -323,7 +307,7 @@ def get(self, request, *args, **kwargs):
         except OIDCError as error:
             return self.error_response(error)
 
-        if not prompt:
+        if not self.must_prompt(token_user):
             return self.do_logout(application, post_logout_redirect_uri, state, token_user)
 
         self.oidc_data = {
@@ -346,21 +330,34 @@ def form_valid(self, form):
         state = form.cleaned_data.get("state")
 
         try:
-            prompt, application, token_user = validate_logout_request(
+            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"):
+            if not self.must_prompt(token_user) or form.cleaned_data.get("allow"):
                 return self.do_logout(application, post_logout_redirect_uri, state, token_user)
             else:
                 raise LogoutDenied()
 
         except OIDCError as error:
             return self.error_response(error)
 
+    def must_prompt(self, token_user):
+        """Indicate 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`.
+
+        A logout without user interaction (i.e. no prompt) is only allowed
+        if an ID Token is provided that matches the current user.
+        """
+        return (
+            oauth2_settings.OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT
+            or token_user is None
+            or token_user != self.request.user
+        )
+
     def do_logout(self, application=None, post_logout_redirect_uri=None, state=None, token_user=None):
         user = token_user or self.request.user
         # Delete Access Tokens if a user was found

tests/test_oidc_views.py

@@ -10,7 +10,12 @@
 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
+from oauth2_provider.views.oidc import (
+    RPInitiatedLogoutView,
+    _load_id_token,
+    _validate_claims,
+    validate_logout_request,
+)
 
 from . import presets
 
@@ -186,9 +191,7 @@ def mock_request_for(user):
 
 
 @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):
-    rp_settings.OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT = ALWAYS_PROMPT
+def test_validate_logout_request(oidc_tokens, public_application):
     oidc_tokens = oidc_tokens
     application = oidc_tokens.application
     client_id = application.client_id
@@ -198,37 +201,31 @@ 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)
+    ) == (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, application, 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, application, None)
+    ) == (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, 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, application, oidc_tokens.user)
+    ) == (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, application, oidc_tokens.user)
+    ) == (application, oidc_tokens.user)
     with pytest.raises(ClientIdMissmatch):
         validate_logout_request(
             request=mock_request_for(oidc_tokens.user),
@@ -266,6 +263,19 @@ def test_validate_logout_request(oidc_tokens, public_application, other_user, rp
         )
 
 
+@pytest.mark.django_db
+@pytest.mark.parametrize("ALWAYS_PROMPT", [True, False])
+def test_must_prompt(oidc_tokens, other_user, rp_settings, ALWAYS_PROMPT):
+    rp_settings.OIDC_RP_INITIATED_LOGOUT_ALWAYS_PROMPT = ALWAYS_PROMPT
+    oidc_tokens = oidc_tokens
+    assert RPInitiatedLogoutView(request=mock_request_for(oidc_tokens.user)).must_prompt(None) is True
+    assert (
+        RPInitiatedLogoutView(request=mock_request_for(oidc_tokens.user)).must_prompt(oidc_tokens.user)
+        == ALWAYS_PROMPT
+    )
+    assert RPInitiatedLogoutView(request=mock_request_for(other_user)).must_prompt(oidc_tokens.user) is True
+
+
 def test__load_id_token():
     assert _load_id_token("Not a Valid ID Token.") == (None, None)