From 1c9c2b5a72fe0e8708f9b0acd3a1c824834d2bbd Mon Sep 17 00:00:00 2001 From: Govind Kailas Date: Tue, 17 Mar 2026 19:55:15 +0530 Subject: [PATCH 1/2] fix(authnz): parse from oidc_backends_config.xml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The element was already defined in the XSD schema (oidc_backends_config.xsd, minOccurs=0) but _parse_idp_config() in managers.py had no branch to read it into the oidc_backend_config dict. As a result, PSAAuthnz._setup_idp() always set IDPHINT=None (from oidc_backend_config.get('idphint') returning None), and the Keycloak backend's auth_extra_params() silently skipped kc_idp_hint entirely: # keycloak.py if idphint := self.setting('IDPHINT', 'oidc'): # None → falsy → skipped params['kc_idp_hint'] = idphint This caused Keycloak to show its own login form instead of redirecting users directly to the configured federated Identity Provider (e.g. Switch edu-ID, AAI, SWITCHaai). Fix: add the missing parsing branch after the checkin_env block, exactly following the pattern used for every other optional element. Also adds three regression tests: - test_parse_idphint_from_xml: is in the parsed dict - test_idphint_propagated_to_psa_config: IDPHINT flows through to PSAAuthnz - test_missing_idphint_is_none: absent → IDPHINT=None (no kc_idp_hint sent) --- lib/galaxy/authnz/managers.py | 6 +++ test/unit/authnz/test_authnz.py | 79 +++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/lib/galaxy/authnz/managers.py b/lib/galaxy/authnz/managers.py index 6193334a3b0..add1862ea4b 100644 --- a/lib/galaxy/authnz/managers.py +++ b/lib/galaxy/authnz/managers.py @@ -187,6 +187,12 @@ class AuthnzManager: if config_xml.find("checkin_env") is not None: rtv["checkin_env"] = config_xml.find("checkin_env").text + # Keycloak/CILogon IDP hint: tells Keycloak which federated IdP to redirect + # to directly (kc_idp_hint), bypassing the Keycloak login page. + # Corresponds to in oidc_backends_config.xml (already in XSD). + if config_xml.find("idphint") is not None: + rtv["idphint"] = config_xml.find("idphint").text + return rtv def _parse_custos_config(self, config_xml): diff --git a/test/unit/authnz/test_authnz.py b/test/unit/authnz/test_authnz.py index c7b090a0f41..dbdd354c01d 100644 --- a/test/unit/authnz/test_authnz.py +++ b/test/unit/authnz/test_authnz.py @@ -119,3 +119,82 @@ def test_psa_authnz_config(mock_app): app_config=mock_app.config, ) assert psa_authnz.config[setting_name("USERNAME_KEY")] == config_values["username_key"] + + +def _create_backend_config_with_idphint(idphint_value: str = None) -> tuple[str, str]: + """Create a Keycloak backend config, optionally including an element.""" + idphint_element = f" {idphint_value}" if idphint_value else "" + contents = f""" + + + https://auth.example.org/realms/MyRealm/ + galaxy-oidc + secret + https://galaxy.example.org/authnz/keycloak/callback + true +{idphint_element} + + +""" + file = tempfile.NamedTemporaryFile(mode="w", delete=False, suffix=".xml") + file.write(contents) + file.flush() + return contents, file.name + + +def test_parse_idphint_from_xml(mock_app): + """ + Regression test: in oidc_backends_config.xml must be parsed into + the oidc_backend_config dict so that PSAAuthnz can forward it as IDPHINT to + the Keycloak/CILogon backends (which use it to set kc_idp_hint). + + Previously, _parse_idp_config() had no branch for , so the element + was silently ignored and oidc_backend_config.get("idphint") always returned + None, causing kc_idp_hint to never be sent to Keycloak. + """ + _, oidc_path = create_oidc_config() + _, backend_path = _create_backend_config_with_idphint(idphint_value="my-switch-edu-id") + manager = AuthnzManager(app=mock_app, oidc_config_file=oidc_path, oidc_backends_config_file=backend_path) + parsed = manager.oidc_backends_config["keycloak"] + assert "idphint" in parsed, " element must be parsed into oidc_backend_config dict" + assert parsed["idphint"] == "my-switch-edu-id" + + +def test_idphint_propagated_to_psa_config(mock_app): + """ + When is configured, PSAAuthnz must expose it as IDPHINT in its + config so the Keycloak/CILogon PSA backend can add kc_idp_hint to the + authorization URL. + """ + from galaxy.authnz.psa_authnz import PSAAuthnz + + _, oidc_path = create_oidc_config() + _, backend_path = _create_backend_config_with_idphint(idphint_value="stage") + manager = AuthnzManager(app=mock_app, oidc_config_file=oidc_path, oidc_backends_config_file=backend_path) + psa = PSAAuthnz( + provider="keycloak", + oidc_config=manager.oidc_config, + oidc_backend_config=manager.oidc_backends_config["keycloak"], + app_config=mock_app.config, + ) + assert psa.config.get("IDPHINT") == "stage", "IDPHINT must be 'stage' when stage is in XML" + + +def test_missing_idphint_is_none(mock_app): + """ + When is absent from the XML, IDPHINT must be None (not a hardcoded + default string like 'oidc'), so the Keycloak backend omits kc_idp_hint + entirely rather than sending a wrong value. + """ + from galaxy.authnz.psa_authnz import PSAAuthnz + + _, oidc_path = create_oidc_config() + _, backend_path = _create_backend_config_with_idphint(idphint_value=None) + manager = AuthnzManager(app=mock_app, oidc_config_file=oidc_path, oidc_backends_config_file=backend_path) + psa = PSAAuthnz( + provider="keycloak", + oidc_config=manager.oidc_config, + oidc_backend_config=manager.oidc_backends_config["keycloak"], + app_config=mock_app.config, + ) + assert psa.config.get("IDPHINT") is None, "IDPHINT must be None when is absent from XML" From d0851167c411fc3cb9e4ea0435e71b1329357939 Mon Sep 17 00:00:00 2001 From: Govind Kailas Date: Tue, 17 Mar 2026 19:55:15 +0530 Subject: [PATCH 2/2] fix(test): use Optional[str] for idphint_value to satisfy mypy --- test/unit/authnz/test_authnz.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/unit/authnz/test_authnz.py b/test/unit/authnz/test_authnz.py index dbdd354c01d..1b72b4756b9 100644 --- a/test/unit/authnz/test_authnz.py +++ b/test/unit/authnz/test_authnz.py @@ -1,4 +1,5 @@ import tempfile +from typing import Optional from unittest.mock import MagicMock import pytest @@ -121,7 +122,7 @@ def test_psa_authnz_config(mock_app): assert psa_authnz.config[setting_name("USERNAME_KEY")] == config_values["username_key"] -def _create_backend_config_with_idphint(idphint_value: str = None) -> tuple[str, str]: +def _create_backend_config_with_idphint(idphint_value: Optional[str] = None) -> tuple[str, str]: """Create a Keycloak backend config, optionally including an element.""" idphint_element = f" {idphint_value}" if idphint_value else "" contents = f"""