From e6ec017b9b3defe032dffda2b09922cd67c2a6b9 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Sat, 5 Oct 2024 12:34:56 -0700 Subject: [PATCH 1/2] Pallet logout shall honor APPLICATION_ROOT --- identity/pallet.py | 2 +- tests/test_flask.py | 31 +++++++++++++++++++++++++------ 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/identity/pallet.py b/identity/pallet.py index 8e4f075..033adfe 100644 --- a/identity/pallet.py +++ b/identity/pallet.py @@ -64,7 +64,7 @@ def __getattribute__(self, name): def logout(self): return self.__class__._redirect( # self._redirect(...) won't work - self._auth.log_out(self._request.host_url)) + self._auth.log_out(self._request.url_root)) def login_required( # Named after Django's login_required self, diff --git a/tests/test_flask.py b/tests/test_flask.py index 84752f0..af2dee3 100644 --- a/tests/test_flask.py +++ b/tests/test_flask.py @@ -1,21 +1,40 @@ +import shutil from unittest.mock import patch, Mock +import pytest from flask import Flask from identity.flask import Auth -def test_logout(): +@pytest.fixture() +def app(): # https://flask.palletsprojects.com/en/3.0.x/testing/ 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", authority="https://example.com/foo") + app.config.update({ + "APPLICATION_ROOT": "/app_root", # Mimicking app with explicit root + "SESSION_TYPE": "filesystem", # Required for Flask-session, + # see also https://stackoverflow.com/questions/26080872 + }) + yield app + shutil.rmtree("flask_session") # clean up + +@pytest.fixture() +def auth(app): + return Auth( + app, + client_id="fake", + redirect_uri="http://localhost:5000/redirect", # To use auth code flow + oidc_authority="https://example.com/foo", + ) + +def test_logout(app, auth): with patch.object(auth._auth, "_get_oidc_config", new=Mock(return_value={ "end_session_endpoint": "https://example.com/end_session", })): 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.") + homepage = "http://localhost/app_root" + assert homepage in auth.logout().get_data(as_text=True), ( + "The homepage should be in the logout URL. There was a bug in 0.9.0.") @patch("msal.authority.tenant_discovery", new=Mock(return_value={ "authorization_endpoint": "https://example.com/placeholder", From 67d3cc176f363a0d8519a4d07762d5163835db9e Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Sun, 6 Oct 2024 12:34:56 -0700 Subject: [PATCH 2/2] Pallet login() shall honor APPLICATION_ROOT --- identity/pallet.py | 7 ++----- tests/test_flask.py | 30 +++++++++++++++++------------- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/identity/pallet.py b/identity/pallet.py index 033adfe..5a4d065 100644 --- a/identity/pallet.py +++ b/identity/pallet.py @@ -92,7 +92,7 @@ async def wrapper(*args, **kwargs): if context: return await function(*args, context=context, **kwargs) # Save an http 302 by calling self.login(request) instead of redirect(self.login) - return await self.login(next_link=self._request.path, scopes=scopes) + return await self.login(next_link=self._request.url, scopes=scopes) else: # For Flask @wraps(function) def wrapper(*args, **kwargs): @@ -101,9 +101,6 @@ def wrapper(*args, **kwargs): if context: return function(*args, context=context, **kwargs) # Save an http 302 by calling self.login(request) instead of redirect(self.login) - return self.login( - next_link=self._request.path, # https://flask.palletsprojects.com/en/3.0.x/api/#flask.Request.path - scopes=scopes, - ) + return self.login(next_link=self._request.url, scopes=scopes) return wrapper diff --git a/tests/test_flask.py b/tests/test_flask.py index af2dee3..a63e113 100644 --- a/tests/test_flask.py +++ b/tests/test_flask.py @@ -40,17 +40,21 @@ def test_logout(app, auth): "authorization_endpoint": "https://example.com/placeholder", "token_endpoint": "https://example.com/placeholder", })) -def test_login_should_locate_its_template(): - app = Flask(__name__) - app.config["SESSION_TYPE"] = "filesystem" # Required for Flask-session, - # see also https://stackoverflow.com/questions/26080872 - client_id = str(hash(app)) - auth = Auth( - app, - client_id=client_id, - redirect_uri="http://localhost:5000/redirect", # To use auth code flow - oidc_authority="https://example.com/foo", - ) - with app.test_request_context("/", method="GET"): - assert client_id in auth.login() # Proper template output contains client_id +def test_login(app, auth): + + @app.route("/path") + @auth.login_required + def dummy_view(): + return "content visible after login" + + with app.test_request_context("/path", method="GET"): + should_find_template = "login() should have template to render" + assert auth._client_id in auth.login(), should_find_template + with app.test_client() as client: + result = client.get("/path?foo=bar") + assert auth._client_id in str(result.data), should_find_template + from flask import session # It is different than auth._auth._session + assert session.get("_auth_flow", {}).get("identity.web.next_link") == ( + "http://localhost/app_root/path?foo=bar" # The full url + ), "Next path should honor APPLICATION_ROOT"