Skip to content

Commit

Permalink
Logout regression in 0.9.0 for Flask
Browse files Browse the repository at this point in the history
  • Loading branch information
rayluo committed Aug 26, 2024
1 parent b6c9c50 commit 8c626cd
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 7 deletions.
3 changes: 2 additions & 1 deletion identity/pallet.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
19 changes: 13 additions & 6 deletions identity/web.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
*,
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down
13 changes: 13 additions & 0 deletions tests/test_flask.py
Original file line number Diff line number Diff line change
@@ -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.")

8 changes: 8 additions & 0 deletions tests/test_quart.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""

0 comments on commit 8c626cd

Please sign in to comment.