Merge branch 'endsession-bugfix' into 'main'

OIDC end_session was not returning the `state` parameter in the `post_logout_redirect_uri`

See merge request yaal/canaille!82

CHANGES.rst

@@ -3,6 +3,12 @@ All notable changes to this project will be documented in this file.
 The format is based on `Keep a Changelog <https://keepachangelog.com/en/1.0.0/>`_,
 and this project adheres to `Semantic Versioning <https://semver.org/spec/v2.0.0.html>`_.
 
+Fixed
+*****
+
+- OIDC end_session was not returning the ``state`` parameter in the
+  ``post_logout_redirect_uri`` :pr:`82`
+
 [0.0.17] - 2022-12-26
 =====================
 

canaille/oidc/endpoints.py

@@ -335,7 +335,7 @@ def end_session():
         url = data["post_logout_redirect_uri"]
         if "state" in data:
             url = set_parameter_in_url_query(url, state=data["state"])
-        return redirect(data["post_logout_redirect_uri"])
+        return redirect(url)
 
     flash(_("You have been disconnected"), "success")
     return redirect(url_for("account.index"))

tests/oidc/test_end_session.py

@@ -19,7 +19,7 @@ def test_end_session(testclient, slapd_connection, logged_user, client, id_token
         status=302,
     )
 
-    assert res.location.startswith(post_logout_redirect_url)
+    assert res.location == f"{post_logout_redirect_url}?state=foobar"
 
     with testclient.session_transaction() as sess:
         assert not sess.get("user_dn")
@@ -44,7 +44,7 @@ def test_end_session_no_client_id(
         status=302,
     )
 
-    assert res.location.startswith(post_logout_redirect_url)
+    assert res.location == f"{post_logout_redirect_url}?state=foobar"
 
     with testclient.session_transaction() as sess:
         assert not sess.get("user_dn")
@@ -207,7 +207,7 @@ def test_no_jwt_logout(testclient, slapd_connection, logged_user, client):
     with testclient.session_transaction() as sess:
         assert not sess.get("user_dn")
 
-    assert res.location.startswith(post_logout_redirect_url)
+    assert res.location == f"{post_logout_redirect_url}?state=foobar"
 
     testclient.get(f"/profile/{logged_user.uid[0]}", status=403)
 
@@ -323,7 +323,7 @@ def test_bad_user_id_token_mismatch(
     with testclient.session_transaction() as sess:
         assert not sess.get("user_dn")
 
-    assert res.location.startswith(post_logout_redirect_url)
+    assert res.location == f"{post_logout_redirect_url}?state=foobar"
 
     testclient.get(f"/profile/{logged_user.uid[0]}", status=403)
 
@@ -352,7 +352,7 @@ def test_bad_user_hint(
     with testclient.session_transaction() as sess:
         assert not sess.get("user_dn")
 
-    assert res.location.startswith(post_logout_redirect_url)
+    assert res.location == f"{post_logout_redirect_url}?state=foobar"
 
     testclient.get(f"/profile/{logged_user.uid[0]}", status=403)
 
@@ -384,7 +384,7 @@ def test_no_jwt_bad_csrf(testclient, slapd_connection, logged_user, client):
     with testclient.session_transaction() as sess:
         assert not sess.get("user_dn")
 
-    assert res.location.startswith(post_logout_redirect_url)
+    assert res.location == f"{post_logout_redirect_url}?state=foobar"
 
     testclient.get(f"/profile/{logged_user.uid[0]}", status=403)
 
@@ -406,3 +406,28 @@ def test_end_session_already_disconnected(
     )
 
     assert res.location == "/"
+
+
+def test_end_session_no_state(
+    testclient, slapd_connection, logged_user, client, id_token
+):
+    testclient.get(f"/profile/{logged_user.uid[0]}", status=200)
+
+    post_logout_redirect_url = "https://mydomain.tld/disconnected"
+    res = testclient.get(
+        "/oauth/end_session",
+        params={
+            "id_token_hint": id_token,
+            "logout_hint": logged_user.uid[0],
+            "client_id": client.client_id,
+            "post_logout_redirect_uri": post_logout_redirect_url,
+        },
+        status=302,
+    )
+
+    assert res.location == post_logout_redirect_url
+
+    with testclient.session_transaction() as sess:
+        assert not sess.get("user_dn")
+
+    testclient.get(f"/profile/{logged_user.uid[0]}", status=403)