From 8c626cd297195fa4186b5df023f90fb17fbcd7cb Mon Sep 17 00:00:00 2001
From: Ray Luo <rayluo.mba@gmail.com>
Date: Sun, 23 Jun 2024 12:34:56 -0700
Subject: [PATCH] Logout regression in 0.9.0 for Flask

---
 identity/pallet.py  |  3 ++-
 identity/web.py     | 19 +++++++++++++------
 tests/test_flask.py | 13 +++++++++++++
 tests/test_quart.py |  8 ++++++++
 4 files changed, 36 insertions(+), 7 deletions(-)
 create mode 100644 tests/test_flask.py

diff --git a/identity/pallet.py b/identity/pallet.py
index 98c6aca..d809bb3 100644
--- a/identity/pallet.py
+++ b/identity/pallet.py
@@ -63,7 +63,8 @@ def __getattribute__(self, name):
         return super(PalletAuth, self).__getattribute__(name)
 
     def logout(self):
-        return self._redirect(self._auth.log_out(self._request.host_url))
+        return self.__class__._redirect(  # self._redirect(...) won't work
+            self._auth.log_out(self._request.host_url))
 
     def login_required(  # Named after Django's login_required
         self,
diff --git a/identity/web.py b/identity/web.py
index 4196de9..3b9ac20 100644
--- a/identity/web.py
+++ b/identity/web.py
@@ -19,6 +19,8 @@ class Auth(object):  # This a low level helper which is web framework agnostic
     _EXPLICITLY_REQUESTED_SCOPES = f"{__name__}.explicitly_requested_scopes"
     _STATE_NO_OP = f"{__name__}.no_op"  # A special state to indicate an auth response shall be ignored
     __NEXT_LINK = f"{__name__}.next_link"  # The next page after a successful auth
+    _END_SESSION_ENDPOINT = "end_session_endpoint"
+
     def __init__(
             self,
             *,
@@ -285,7 +287,11 @@ def _get_oidc_config(self):
         # The self._authority is usually the V1 endpoint of Microsoft Entra ID,
         # which is still good enough for log_out()
         a = self._oidc_authority or self._authority
-        return requests.get(f"{a}/.well-known/openid-configuration").json()
+        conf = requests.get(f"{a}/.well-known/openid-configuration").json()
+        if not conf.get(self._END_SESSION_ENDPOINT):
+            logger.warning(
+                "%s not found from OIDC config: %s", self._END_SESSION_ENDPOINT, conf)
+        return conf
 
     def log_out(self, homepage):
         # The vocabulary is "log out" (rather than "sign out") in the specs
@@ -305,12 +311,13 @@ def log_out(self, homepage):
         try:
             # Empirically, Microsoft Entra ID's /v2.0 endpoint shows an account picker
             # but its default (i.e. v1.0) endpoint will sign out the (only?) account
-            e = self._get_oidc_config().get("end_session_endpoint")
-        except requests.exceptions.RequestException as e:
+            endpoint = self._get_oidc_config().get(self._END_SESSION_ENDPOINT)
+            if endpoint:
+                return f"{endpoint}?post_logout_redirect_uri={homepage}"
+        except requests.exceptions.RequestException:
             logger.exception("Failed to get OIDC config")
-            return homepage
-        else:
-            return f"{e}?post_logout_redirect_uri={homepage}" if e else homepage
+        logger.warning("No end_session_endpoint found. Fallback to %s", homepage)
+        return homepage
 
     def get_token_for_client(self, scopes):
         """Get access token for the current app, with specified scopes.
diff --git a/tests/test_flask.py b/tests/test_flask.py
new file mode 100644
index 0000000..89c6453
--- /dev/null
+++ b/tests/test_flask.py
@@ -0,0 +1,13 @@
+from flask import Flask
+from identity.flask import Auth
+
+
+def test_logout():
+    app = Flask(__name__)
+    app.config["SESSION_TYPE"] = "filesystem"  # Required for Flask-session,
+        # see also https://stackoverflow.com/questions/26080872
+    auth = Auth(app, client_id="fake")
+    with app.test_request_context("/", method="GET"):
+        assert auth._request.host_url in auth.logout().get_data(as_text=True), (
+            "The host_url should be in the logout URL. There was a bug in 0.9.0.")
+
diff --git a/tests/test_quart.py b/tests/test_quart.py
index dcdbb5d..f76ef5a 100644
--- a/tests/test_quart.py
+++ b/tests/test_quart.py
@@ -29,3 +29,11 @@ async def test_login(monkeypatch):
         rendered_template = await auth.login()
 
         assert "https://login.microsoftonline.com/123/oauth2/v2.0/authorize" in rendered_template
+
+
+def test_logout():
+    """Intended to add a test case similar to test_flask.py,
+    but skipped for now because Quart's session requires a backend such as Redis.
+    In the future, we might remove the session dependency anyway and revisit this.
+    """
+