From 38a6f9e1890c6ffbeb1f2ef849cdcc6cb77227c0 Mon Sep 17 00:00:00 2001 From: Nuwan Goonasekera <2070605+nuwang@users.noreply.github.com> Date: Mon, 16 Mar 2026 14:26:36 +0530 Subject: [PATCH 1/5] Add test for idp logout --- lib/galaxy/authnz/psa_authnz.py | 26 +++++++++++--- test/integration/oidc/test_auth_oidc.py | 44 +++++++++++++++++++++--- test/unit/authnz/test_psa_authnz.py | 45 +++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 9 deletions(-) diff --git a/lib/galaxy/authnz/psa_authnz.py b/lib/galaxy/authnz/psa_authnz.py index 9e5eba673f80..b471c802cbfa 100644 --- a/lib/galaxy/authnz/psa_authnz.py +++ b/lib/galaxy/authnz/psa_authnz.py @@ -2,7 +2,10 @@ import logging import time from typing import TYPE_CHECKING -from urllib.parse import quote +from urllib.parse import ( + quote, + urlencode, +) import jwt from jwt import InvalidTokenError @@ -412,11 +415,24 @@ def logout(self, trans, post_user_logout_href=None): end_session_endpoint = oidc_config.get("end_session_endpoint") if end_session_endpoint: - # Construct logout URL with optional redirect_uri + logout_params = {} + + # Provide the current ID token so providers such as Keycloak + # can complete RP-initiated logout without showing a confirmation page. + if trans.user is not None: + for social_auth in trans.user.social_auth: + if social_auth.provider == BACKENDS_NAME[self.config["provider"]]: + id_token = social_auth.extra_data.get("id_token") + if id_token: + logout_params["id_token_hint"] = id_token + break + if post_user_logout_href: - logout_url = f"{end_session_endpoint}?redirect_uri={quote(post_user_logout_href)}" - else: - logout_url = end_session_endpoint + logout_params["post_logout_redirect_uri"] = post_user_logout_href + + logout_url = end_session_endpoint + if logout_params: + logout_url = f"{logout_url}?{urlencode(logout_params)}" return logout_url else: diff --git a/test/integration/oidc/test_auth_oidc.py b/test/integration/oidc/test_auth_oidc.py index 910a7ac89ce8..2a62af7da1e0 100644 --- a/test/integration/oidc/test_auth_oidc.py +++ b/test/integration/oidc/test_auth_oidc.py @@ -187,14 +187,27 @@ def handle_galaxy_oidc_config_kwds(cls, config): def _get_interactor(self, api_key=None, allow_anonymous=False) -> "ApiTestInteractor": return super()._get_interactor(api_key=None, allow_anonymous=True) - def _login_via_keycloak(self, username, password, expected_codes=None, save_cookies=False, session=None): - - if expected_codes is None: - expected_codes = [200, 404] + def _start_oidc_login(self, session=None): session = session or requests.Session() response = session.get(f"{self.url}authnz/{self.provider_name}/login") provider_url = response.json()["redirect_uri"] + return session, provider_url + + def _visit_keycloak_login_page(self, session=None): + session, provider_url = self._start_oidc_login(session=session) response = session.get(provider_url, verify=False) + return session, response + + def _clear_galaxy_session_cookies(self, session): + for cookie in list(session.cookies): + if cookie.name == "galaxysession": + session.cookies.clear(domain=cookie.domain, path=cookie.path, name=cookie.name) + + def _login_via_keycloak(self, username, password, expected_codes=None, save_cookies=False, session=None): + + if expected_codes is None: + expected_codes = [200, 404] + session, response = self._visit_keycloak_login_page(session=session) matches = self.REGEX_KEYCLOAK_LOGIN_ACTION.search(response.text) assert matches auth_url = html.unescape(str(matches.group(1))) @@ -413,6 +426,29 @@ def test_oidc_logout(self): self._assert_status_code_is(response, 200) assert "email" not in response.json() + def test_oidc_logout_logs_out_of_idp(self): + session, _ = self._login_via_keycloak( + KEYCLOAK_TEST_USERNAME, KEYCLOAK_TEST_PASSWORD, session=requests.Session() + ) + + # Dropping only Galaxy's session should still allow silent re-auth via the existing Keycloak session. + self._clear_galaxy_session_cookies(session) + session, response = self._visit_keycloak_login_page(session=session) + assert self.REGEX_KEYCLOAK_LOGIN_ACTION.search(response.text) is None + + response = session.get(self._api_url("users/current")) + self._assert_status_code_is(response, 200) + assert response.json()["email"] == "gxyuser@galaxy.org" + + # Galaxy logout with enable_idp_logout should invalidate the Keycloak browser session too. + response = session.get(self._api_url("../authnz/logout")) + response = session.get(response.json()["redirect_uri"], verify=False) + self._clear_galaxy_session_cookies(session) + + session, response = self._visit_keycloak_login_page(session=session) + matches = self.REGEX_KEYCLOAK_LOGIN_ACTION.search(response.text) + assert matches, "Expected Keycloak to prompt for credentials after IDP logout" + def test_auth_by_access_token_logged_in_once(self): # login at least once self._login_via_keycloak("gxyuser_logged_in_once", KEYCLOAK_TEST_PASSWORD) diff --git a/test/unit/authnz/test_psa_authnz.py b/test/unit/authnz/test_psa_authnz.py index f14d30fa7317..b3857e922369 100644 --- a/test/unit/authnz/test_psa_authnz.py +++ b/test/unit/authnz/test_psa_authnz.py @@ -13,6 +13,7 @@ MagicMock, patch, ) +from urllib import parse import jwt import pytest @@ -373,6 +374,50 @@ def test_oidc_config_custom_auth_pipeline_and_extra(mock_oidc_config_file, mock_ assert psa_authnz.config["SOCIAL_AUTH_PIPELINE"] == custom_auth_pipeline + tuple(custom_auth_pipeline_extra) +def test_logout_uses_id_token_hint_and_post_logout_redirect_uri(): + app_config = SimpleNamespace( + oidc_auth_pipeline=None, + oidc_auth_pipeline_extra=None, + fixed_delegated_auth=False, + ) + psa_authnz = PSAAuthnz( + provider="keycloak", + oidc_config={}, + oidc_backend_config={ + "client_id": "gxyclient", + "client_secret": "dummyclientsecret", + "redirect_uri": "http://localhost/authnz/keycloak/callback", + }, + app_config=app_config, + ) + backend = MagicMock() + backend.oidc_config.return_value = {"end_session_endpoint": "https://keycloak.example.com/logout"} + trans = SimpleNamespace( + request=SimpleNamespace(host="http://localhost"), + session={}, + sa_session=MagicMock(), + user=SimpleNamespace( + social_auth=[SimpleNamespace(provider="keycloak", extra_data={"id_token": "header.payload.signature"})] + ), + ) + + with ( + patch("galaxy.authnz.psa_authnz.on_the_fly_config"), + patch.object(PSAAuthnz, "_load_backend", return_value=backend), + patch("galaxy.authnz.psa_authnz.is_oidc_backend", return_value=True), + ): + logout_url = psa_authnz.logout(trans, post_user_logout_href="http://galaxy.example.com/root/login") + + parsed_url = parse.urlparse(logout_url) + query_params = parse.parse_qs(parsed_url.query) + assert parsed_url.scheme == "https" + assert parsed_url.netloc == "keycloak.example.com" + assert parsed_url.path == "/logout" + assert query_params["id_token_hint"] == ["header.payload.signature"] + assert query_params["post_logout_redirect_uri"] == ["http://galaxy.example.com/root/login"] + assert "redirect_uri" not in query_params + + def test_sync_user_profile_skips_when_account_interface_enabled(): manager = MagicMock() session = MagicMock() From b07b20e99016c7e6efb25b01585aa2db9eaae67a Mon Sep 17 00:00:00 2001 From: Nuwan Goonasekera <2070605+nuwang@users.noreply.github.com> Date: Mon, 16 Mar 2026 14:33:40 +0530 Subject: [PATCH 2/5] Refactor code for obtaining id token --- lib/galaxy/authnz/psa_authnz.py | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/lib/galaxy/authnz/psa_authnz.py b/lib/galaxy/authnz/psa_authnz.py index b471c802cbfa..66f49d895f92 100644 --- a/lib/galaxy/authnz/psa_authnz.py +++ b/lib/galaxy/authnz/psa_authnz.py @@ -242,6 +242,19 @@ def _load_backend(self, strategy, redirect_uri) -> "BaseOAuth2": backend = get_backend(backends, BACKENDS_NAME[self.config["provider"]]) return backend(strategy, redirect_uri) + def _get_user_id_token(self, user): + if user is None: + return None + + provider_name = BACKENDS_NAME[self.config["provider"]] + for social_auth in user.social_auth: + if social_auth.provider != provider_name: + continue + extra_data = social_auth.extra_data or {} + return extra_data.get("id_token") + + return None + def _login_user(self, backend, user, social_user): self.config["user"] = user @@ -419,13 +432,8 @@ def logout(self, trans, post_user_logout_href=None): # Provide the current ID token so providers such as Keycloak # can complete RP-initiated logout without showing a confirmation page. - if trans.user is not None: - for social_auth in trans.user.social_auth: - if social_auth.provider == BACKENDS_NAME[self.config["provider"]]: - id_token = social_auth.extra_data.get("id_token") - if id_token: - logout_params["id_token_hint"] = id_token - break + if id_token := self._get_user_id_token(trans.user): + logout_params["id_token_hint"] = id_token if post_user_logout_href: logout_params["post_logout_redirect_uri"] = post_user_logout_href From 4533fe4618ef2fc6c2403b77e8b7d04107dc1cab Mon Sep 17 00:00:00 2001 From: Nuwan Goonasekera <2070605+nuwang@users.noreply.github.com> Date: Mon, 16 Mar 2026 16:57:35 +0530 Subject: [PATCH 3/5] Fix mypy errors --- test/unit/authnz/test_psa_authnz.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/test/unit/authnz/test_psa_authnz.py b/test/unit/authnz/test_psa_authnz.py index b3857e922369..de4ee37fa5b5 100644 --- a/test/unit/authnz/test_psa_authnz.py +++ b/test/unit/authnz/test_psa_authnz.py @@ -8,7 +8,10 @@ timedelta, ) from types import SimpleNamespace -from typing import Optional +from typing import ( + cast, + Optional, +) from unittest.mock import ( MagicMock, patch, @@ -42,6 +45,7 @@ PSAAuthnz, sync_user_profile, ) +from galaxy.config import GalaxyAppConfiguration @pytest.fixture(scope="module") @@ -319,7 +323,7 @@ def test_oidc_config_custom_auth_pipeline(mock_oidc_config_file, mock_oidc_backe provider="oidc", oidc_config=manager.oidc_config, oidc_backend_config=manager.oidc_backends_config, - app_config=mock_app.config, + app_config=cast(GalaxyAppConfiguration, mock_app.config), ) assert psa_authnz.config["SOCIAL_AUTH_PIPELINE"] == custom_auth_pipeline @@ -343,7 +347,7 @@ def test_oidc_config_auth_pipeline_extra(mock_oidc_config_file, mock_oidc_backen provider="oidc", oidc_config=manager.oidc_config, oidc_backend_config=manager.oidc_backends_config, - app_config=mock_app.config, + app_config=cast(GalaxyAppConfiguration, mock_app.config), ) assert psa_authnz.config["SOCIAL_AUTH_PIPELINE"] == AUTH_PIPELINE + tuple(custom_auth_pipeline_extra) @@ -369,7 +373,7 @@ def test_oidc_config_custom_auth_pipeline_and_extra(mock_oidc_config_file, mock_ provider="oidc", oidc_config=manager.oidc_config, oidc_backend_config=manager.oidc_backends_config, - app_config=mock_app.config, + app_config=cast(GalaxyAppConfiguration, mock_app.config), ) assert psa_authnz.config["SOCIAL_AUTH_PIPELINE"] == custom_auth_pipeline + tuple(custom_auth_pipeline_extra) @@ -388,7 +392,7 @@ def test_logout_uses_id_token_hint_and_post_logout_redirect_uri(): "client_secret": "dummyclientsecret", "redirect_uri": "http://localhost/authnz/keycloak/callback", }, - app_config=app_config, + app_config=cast(GalaxyAppConfiguration, app_config), ) backend = MagicMock() backend.oidc_config.return_value = {"end_session_endpoint": "https://keycloak.example.com/logout"} From f6f3c142f1c197d10d373bfde91a72479d5744b0 Mon Sep 17 00:00:00 2001 From: Nuwan Goonasekera <2070605+nuwang@users.noreply.github.com> Date: Mon, 16 Mar 2026 18:05:23 +0530 Subject: [PATCH 4/5] Also perform a single-step client side logout --- client/src/utils/logout.js | 65 +++++++++++-------- .../webapps/galaxy/controllers/authnz.py | 9 ++- 2 files changed, 44 insertions(+), 30 deletions(-) diff --git a/client/src/utils/logout.js b/client/src/utils/logout.js index a60fb63d72a8..ec490ae6f86d 100644 --- a/client/src/utils/logout.js +++ b/client/src/utils/logout.js @@ -3,6 +3,22 @@ import axios from "axios"; import { getGalaxyInstance } from "@/app"; import { withPrefix } from "@/utils/redirect"; +function userLogoutUrl(sessionCsrfToken, logoutAll) { + return withPrefix(`/user/logout?session_csrf_token=${sessionCsrfToken}&logout_all=${logoutAll}`); +} + +function authnzLogoutUrl(provider, logoutAll) { + const query = new URLSearchParams({ logout_all: String(logoutAll) }); + if (provider) { + query.set("provider", provider); + } + return withPrefix(`/authnz/logout?${query.toString()}`); +} + +function hasAuthnzLogoutResponse(response) { + return Boolean(response?.data?.redirect_uri || response?.data?.message); +} + /** * Handles user logout. Invalidates the current session, checks to see if we * need to log out of OIDC too, and goes to our POST_LOGOUT_URL (or some other @@ -11,33 +27,28 @@ export function userLogout(logoutAll = false) { const Galaxy = getGalaxyInstance(); const post_user_logout_href = Galaxy.config.post_user_logout_href; const session_csrf_token = Galaxy.session_csrf_token; - const url = `/user/logout?session_csrf_token=${session_csrf_token}&logout_all=${logoutAll}`; - axios - .get(withPrefix(url)) - .then((response) => { - if (Galaxy.user) { - Galaxy.user.clearSessionStorage(); - } - // Check if we need to logout of OIDC IDP - if (Galaxy.config.enable_oidc) { - const provider = localStorage.getItem("galaxy-provider"); - if (provider) { - localStorage.removeItem("galaxy-provider"); - return axios.get(withPrefix(`/authnz/logout?provider=${provider}`)); - } - return axios.get(withPrefix("/authnz/logout")); - } else { - // Otherwise pass through the initial logout response - return response; - } - }) - .then((response) => { - if (response.data?.redirect_uri) { - window.top.location.href = response.data.redirect_uri; - } else { - window.top.location.href = withPrefix(post_user_logout_href); - } - }); + const provider = localStorage.getItem("galaxy-provider"); + const logoutRequest = Galaxy.config.enable_oidc + ? axios.get(authnzLogoutUrl(provider, logoutAll)).then((response) => { + if (hasAuthnzLogoutResponse(response)) { + return response; + } + return axios.get(userLogoutUrl(session_csrf_token, logoutAll)); + }) + : axios.get(userLogoutUrl(session_csrf_token, logoutAll)); + + localStorage.removeItem("galaxy-provider"); + + return logoutRequest.then((response) => { + if (Galaxy.user) { + Galaxy.user.clearSessionStorage(); + } + if (response.data?.redirect_uri) { + window.top.location.href = response.data.redirect_uri; + } else { + window.top.location.href = withPrefix(post_user_logout_href); + } + }); } /** User logout with 'log out all sessions' flag set. This will invalidate all diff --git a/lib/galaxy/webapps/galaxy/controllers/authnz.py b/lib/galaxy/webapps/galaxy/controllers/authnz.py index 01dfa4b05341..e5beea71ebe4 100644 --- a/lib/galaxy/webapps/galaxy/controllers/authnz.py +++ b/lib/galaxy/webapps/galaxy/controllers/authnz.py @@ -13,7 +13,10 @@ exceptions, web, ) -from galaxy.util import url_get +from galaxy.util import ( + asbool, + url_get, +) from galaxy.web import url_for from galaxy.webapps.base.controller import BaseUIController @@ -213,14 +216,14 @@ def disconnect(self, trans, provider, email=None, **kwargs): @web.json @web.expose - def logout(self, trans, provider, **kwargs): + def logout(self, trans, provider, logout_all=False, **kwargs): post_user_logout_href = trans.app.config.post_user_logout_href if post_user_logout_href is not None: post_user_logout_href = trans.request.base + url_for(post_user_logout_href) success, message, redirect_uri = trans.app.authnz_manager.logout( provider, trans, post_user_logout_href=post_user_logout_href ) - trans.handle_user_logout() + trans.handle_user_logout(logout_all=asbool(logout_all)) if success: return {"redirect_uri": redirect_uri} else: From 6e14393a44e483444ea4309012d588cef0078628 Mon Sep 17 00:00:00 2001 From: Ahmed Awan Date: Tue, 17 Mar 2026 13:27:27 -0500 Subject: [PATCH 5/5] use addSearchParams utility for logout URL construction --- client/src/utils/logout.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/client/src/utils/logout.js b/client/src/utils/logout.js index ec490ae6f86d..5e0e75209bf4 100644 --- a/client/src/utils/logout.js +++ b/client/src/utils/logout.js @@ -2,17 +2,21 @@ import axios from "axios"; import { getGalaxyInstance } from "@/app"; import { withPrefix } from "@/utils/redirect"; +import { addSearchParams } from "@/utils/url"; function userLogoutUrl(sessionCsrfToken, logoutAll) { - return withPrefix(`/user/logout?session_csrf_token=${sessionCsrfToken}&logout_all=${logoutAll}`); + return addSearchParams(withPrefix("/user/logout"), { + session_csrf_token: sessionCsrfToken, + logout_all: String(logoutAll), + }); } function authnzLogoutUrl(provider, logoutAll) { - const query = new URLSearchParams({ logout_all: String(logoutAll) }); + const params = { logout_all: String(logoutAll) }; if (provider) { - query.set("provider", provider); + params.provider = provider; } - return withPrefix(`/authnz/logout?${query.toString()}`); + return addSearchParams(withPrefix("/authnz/logout"), params); } function hasAuthnzLogoutResponse(response) {