Work on end_session_endpoint

juanifioren · juanifioren · commit a981a3d95a80 · 2024-12-02T17:56:40.000-03:00
diff --git a/docs/sections/changelog.rst b/docs/sections/changelog.rst
@@ -8,6 +8,7 @@ All notable changes to this project will be documented in this file.
 Unreleased
 ==========
 
+* Changed: Improved "OpenID Connect RP-Initiated Logout" implementation.
 * Fixed: RSA server keys random ordering.
 * Fixed: Example app working with Django 4.
 
diff --git a/docs/sections/sessionmanagement.rst b/docs/sections/sessionmanagement.rst
@@ -22,6 +22,39 @@ Somewhere in your Django ``settings.py``::
 If you're in a multi-server setup, you might also want to add ``OIDC_UNAUTHENTICATED_SESSION_MANAGEMENT_KEY`` to your settings and set it to some random but fixed string. While authenticated clients have a session that can be used to calculate the browser state, there is no such thing for unauthenticated clients. Hence this value. By default a value is generated randomly on startup, so this will be different on each server. To get a consistent value across all servers you should set this yourself.
 
 
+RP-Initiated Logout
+===================
+
+An RP can notify the OP that the End-User has logged out of the site, and might want to log out of the OP as well. In this case, the RP, after having logged the End-User out of the RP, redirects the End-User's User Agent to the OP's logout endpoint URL.
+
+This URL is normally obtained via the ``end_session_endpoint`` element of the OP's Discovery response.
+
+Parameters that are passed as query parameters in the logout request:
+
+* ``id_token_hint``
+    RECOMMENDED. Previously issued ID Token passed to the logout endpoint as a hint about the End-User's current authenticated session with the Client.
+* ``post_logout_redirect_uri``
+    OPTIONAL. URL to which the RP is requesting that the End-User's User Agent be redirected after a logout has been performed.
+    
+    The value must be a valid, encoded URL that has been registered in the list of "Post Logout Redirect URIs" in your Client (RP) page.
+* ``state``
+    OPTIONAL. Opaque value used by the RP to maintain state between the logout request and the callback to the endpoint specified by the ``post_logout_redirect_uri`` query parameter.
+
+Example redirect::
+
+    http://localhost:8000/end-session/?id_token_hint=eyJhbGciOiJSUzI1NiIsImtpZCI6ImQwM...&post_logout_redirect_uri=http://rp.example.com/logged-out/&state=c91c03ea6c46a86
+
+**Logout consent prompt**
+
+The standard defines that the logout flow should be interrupted to prompt the user for consent if the OpenID provider cannot verify that the request was made by the user.
+
+We enforce this behavior by displaying a logout consent prompt if it detects any of the following conditions:
+
+* If ``id_token_hint`` is not present or is invalid (we could not validate the client from it).
+* If ``post_logout_redirect_uri`` is not registered in the list of "Post Logout Redirect URIs".
+
+If the user confirms the logout request, we continue the logout flow. To modify the logout consent template create your own ``oidc_provider/end_session_prompt.html``.
+
 Example RP iframe
 =================
 
@@ -70,22 +103,4 @@ Example RP iframe
     </script>
     </html>
 
-RP-Initiated Logout
-===================
-
-An RP can notify the OP that the End-User has logged out of the site, and might want to log out of the OP as well. In this case, the RP, after having logged the End-User out of the RP, redirects the End-User's User Agent to the OP's logout endpoint URL.
 
-This URL is normally obtained via the ``end_session_endpoint`` element of the OP's Discovery response.
-
-Parameters that are passed as query parameters in the logout request:
-
-* ``id_token_hint``
-    Previously issued ID Token passed to the logout endpoint as a hint about the End-User's current authenticated session with the Client.
-* ``post_logout_redirect_uri``
-    URL to which the RP is requesting that the End-User's User Agent be redirected after a logout has been performed.
-* ``state``
-    OPTIONAL. Opaque value used by the RP to maintain state between the logout request and the callback to the endpoint specified by the ``post_logout_redirect_uri`` query parameter.
-
-Example redirect::
-
-    http://localhost:8000/end-session/?id_token_hint=eyJhbGciOiJSUzI1NiIsImtpZCI6ImQwM...&post_logout_redirect_uri=http://rp.example.com/logged-out/&state=c91c03ea6c46a86
diff --git a/oidc_provider/templates/oidc_provider/end_session_prompt.html b/oidc_provider/templates/oidc_provider/end_session_prompt.html
@@ -0,0 +1,12 @@
+<h1>End Session</h1>
+
+<p>Hi <strong>{{ user.email }}</strong>, are you sure you want to log out{% if client %} from <strong>{{ client.name }}</strong> app{% endif %}?</p>
+
+<form method="post" action="{{ end_session_prompt_url }}">
+
+    {% csrf_token %}
+
+    <input type="submit" value="Cancel" />
+    <input type="submit" name="allow" value="Yes" />
+
+</form>
diff --git a/oidc_provider/tests/cases/test_end_session_endpoint.py b/oidc_provider/tests/cases/test_end_session_endpoint.py
@@ -1,3 +1,8 @@
+try:
+    from urllib import urlencode
+except ImportError:
+    from urllib.parse import urlencode
+
 from django.core.management import call_command
 try:
     from django.urls import reverse
@@ -26,55 +31,139 @@ class EndSessionTestCase(TestCase):
     def setUp(self):
         call_command('creatersakey')
         self.user = create_fake_user()
+        self.client.force_login(self.user)
 
+        # Create a client with a custom logout URL.
         self.oidc_client = create_fake_client('id_token')
-        self.LOGOUT_URL = 'http://example.com/logged-out/'
-        self.oidc_client.post_logout_redirect_uris = [self.LOGOUT_URL]
+        self.url_logout = 'http://example.com/logged-out/'
+        self.oidc_client.post_logout_redirect_uris = [self.url_logout]
         self.oidc_client.save()
 
-        self.url = reverse('oidc_provider:end-session')
-
-    def test_redirects_when_aud_is_str(self):
-        query_params = {
-            'post_logout_redirect_uri': self.LOGOUT_URL,
-        }
-        response = self.client.get(self.url, query_params)
-        # With no id_token the OP MUST NOT redirect to the requested
-        # redirect_uri.
-        self.assertRedirects(
-            response, settings.get('OIDC_LOGIN_URL'),
-            fetch_redirect_response=False)
-
+        # Create a valid ID Token for the user.
         token = create_token(self.user, self.oidc_client, [])
         id_token_dic = create_id_token(
             token=token, user=self.user, aud=self.oidc_client.client_id)
-        id_token = encode_id_token(id_token_dic, self.oidc_client)
+        self.id_token = encode_id_token(id_token_dic, self.oidc_client)
 
-        query_params['id_token_hint'] = id_token
+        self.url = reverse('oidc_provider:end-session')
+        self.url_prompt = reverse('oidc_provider:end-session-prompt')
 
+    def test_id_token_hint_not_present_user_prompted(self):
+        response = self.client.get(self.url)
+        # We should display a logout consent prompt if id_token_hint parameter is not present.
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.headers['Location'], self.url_prompt)
+    
+    def test_id_token_hint_is_present_user_redirected_to_client_logout_url(self):
+        query_params = {
+            'id_token_hint': self.id_token,
+        }
         response = self.client.get(self.url, query_params)
-        self.assertRedirects(
-            response, self.LOGOUT_URL, fetch_redirect_response=False)
+        # ID Token is valid so user was
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.headers['Location'], self.url_logout)
+    
+    def test_id_token_hint_is_present_user_redirected_to_client_logout_url_with_post(self):
+        data = {
+            'id_token_hint': self.id_token,
+        }
+        response = self.client.post(self.url, data)
+        # ID Token is valid so user was
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.headers['Location'], self.url_logout)
 
-    def test_redirects_when_aud_is_list(self):
-        """Check with 'aud' containing a list of str."""
+    def test_state_is_present_and_being_passed_to_logout_url(self):
         query_params = {
-            'post_logout_redirect_uri': self.LOGOUT_URL,
+            'id_token_hint': self.id_token,
+            'state': 'ABCDE',
         }
-        token = create_token(self.user, self.oidc_client, [])
-        id_token_dic = create_id_token(
-            token=token, user=self.user, aud=self.oidc_client.client_id)
-        id_token_dic['aud'] = [id_token_dic['aud']]
-        id_token = encode_id_token(id_token_dic, self.oidc_client)
-        query_params['id_token_hint'] = id_token
         response = self.client.get(self.url, query_params)
-        self.assertRedirects(
-            response, self.LOGOUT_URL, fetch_redirect_response=False)
+        # Let's ensure state is being passed to the logout url.
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.headers['Location'], '{0}?state={1}'.format(self.url_logout, 'ABCDE'))
+    
+    def test_post_logout_uri_not_in_client_urls(self):
+        query_params = {
+            'id_token_hint': self.id_token,
+            'post_logout_redirect_uri': 'http://other.com/bye/',
+        }
+        response = self.client.get(self.url, query_params)
+        # We prompt the user since the post logout url is not from client urls.
+        # Also ensure client_id is present since we could validate id_token_hint.
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.headers['Location'], '{0}?client_id={1}'.format(self.url_prompt, self.oidc_client.client_id))
+
+    def test_prompt_view_redirecting_to_client_post_logout_since_user_unauthenticated(self):
+        self.client.logout()
+        query_params = {
+            'client_id': self.oidc_client.client_id,
+        }
+        response = self.client.get(self.url_prompt, query_params)
+        # Since user is unauthenticated on the backend, we send it back to client post logout
+        # registered uri.
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.headers['Location'], self.url_logout)
+    
+    def test_prompt_view_raising_404_since_user_unauthenticated_and_no_client(self):
+        self.client.logout()
+        response = self.client.get(self.url_prompt)
+        # Since user is unauthenticated and no client information is present, we just show
+        # not found page.
+        self.assertEqual(response.status_code, 404)
+
+    def test_prompt_view_displaying_logout_decision_form_to_user(self):
+        query_params = {
+            'client_id': self.oidc_client.client_id,
+        }
+        response = self.client.get(self.url_prompt, query_params)
+        # User is prompted to logout with client information displayed.
+        self.assertContains(response, '<p>Hi <strong>johndoe@example.com</strong>, are you sure you want to log out from <strong>Some Client</strong> app?</p>', status_code=200, html=True)
 
+    def test_prompt_view_displaying_logout_decision_form_to_user_no_client(self):
+        response = self.client.get(self.url_prompt)
+        # User is prompted to logout without client information displayed.
+        self.assertContains(response, '<p>Hi <strong>johndoe@example.com</strong>, are you sure you want to log out?</p>', status_code=200, html=True)
+    
+    @mock.patch(settings.get('OIDC_AFTER_END_SESSION_HOOK'))
+    def test_prompt_view_user_logged_out_after_form_allowed(self, end_session_hook):
+        self.assertIn('_auth_user_id', self.client.session)
+        # We want to POST to /end-session-prompt/?client_id=ABC endpoint.
+        url_prompt_with_client = self.url_prompt + '?' + urlencode({
+            'client_id': self.oidc_client.client_id,
+        })
+        data = {
+            'allow': 'Anything',  # This means user allowed being logged out.
+        }
+        response = self.client.post(url_prompt_with_client, data)
+        # Ensure user is now logged out and redirected to client post logout uri.
+        self.assertNotIn('_auth_user_id', self.client.session)
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.headers['Location'], self.url_logout)
+        # End session hook should be called.
+        self.assertTrue(end_session_hook.called)
+        self.assertTrue(end_session_hook.call_count == 1)
+    
+    @mock.patch(settings.get('OIDC_AFTER_END_SESSION_HOOK'))
+    def test_prompt_view_user_logged_out_after_form_not_allowed(self, end_session_hook):
+        self.assertIn('_auth_user_id', self.client.session)
+        # We want to POST to /end-session-prompt/?client_id=ABC endpoint.
+        url_prompt_with_client = self.url_prompt + '?' + urlencode({
+            'client_id': self.oidc_client.client_id,
+        })
+        response = self.client.post(url_prompt_with_client)  # No data.
+        # Ensure user is still logged in and redirected to client post logout uri.
+        self.assertIn('_auth_user_id', self.client.session)
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.headers['Location'], self.url_logout)
+        # End session hook should not be called.
+        self.assertFalse(end_session_hook.called)
+    
     @mock.patch(settings.get('OIDC_AFTER_END_SESSION_HOOK'))
-    def test_call_post_end_session_hook(self, hook_function):
-        self.client.get(self.url)
-        self.assertTrue(hook_function.called, 'OIDC_AFTER_END_SESSION_HOOK should be called')
-        self.assertTrue(
-            hook_function.call_count == 1,
-            'OIDC_AFTER_END_SESSION_HOOK should be called once')
+    def test_prompt_view_user_not_logged_out_after_form_not_allowed_no_client(self, end_session_hook):
+        self.assertIn('_auth_user_id', self.client.session)
+        response = self.client.post(self.url_prompt)  # No data.
+        # Ensure user is still logged in and 404 NOT FOUND was raised.
+        self.assertIn('_auth_user_id', self.client.session)
+        self.assertEqual(response.status_code, 404)
+        # End session hook should not be called.
+        self.assertFalse(end_session_hook.called)
diff --git a/oidc_provider/urls.py b/oidc_provider/urls.py
@@ -12,6 +12,7 @@
     re_path(r'^token/?$', csrf_exempt(views.TokenView.as_view()), name='token'),
     re_path(r'^userinfo/?$', csrf_exempt(views.userinfo), name='userinfo'),
     re_path(r'^end-session/?$', views.EndSessionView.as_view(), name='end-session'),
+    re_path(r'^end-session-prompt/?$', views.EndSessionPromptView.as_view(), name='end-session-prompt'),
     re_path(r'^\.well-known/openid-configuration/?$', views.ProviderInfoView.as_view(),
             name='provider-info'),
     re_path(r'^introspect/?$', views.TokenIntrospectionView.as_view(), name='token-introspection'),
diff --git a/oidc_provider/views.py b/oidc_provider/views.py
