From 9998e7bb839e312414777e20c62b17bdd81e9ade Mon Sep 17 00:00:00 2001 From: marius-mather Date: Fri, 10 Apr 2026 12:44:34 +1000 Subject: [PATCH 01/24] feat: add 'oidc_require_refresh' config variable --- lib/galaxy/config/__init__.py | 1 + lib/galaxy/config/schemas/config_schema.yml | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/lib/galaxy/config/__init__.py b/lib/galaxy/config/__init__.py index a0c5aa09611..dea274271d2 100644 --- a/lib/galaxy/config/__init__.py +++ b/lib/galaxy/config/__init__.py @@ -757,6 +757,7 @@ class GalaxyAppConfiguration(BaseAppConfiguration, CommonConfigurationMixin): mulled_channels: list[str] new_file_path: str nginx_upload_store: str + oidc_require_refresh: bool password_expiration_period: timedelta preserve_python_environment: str pretty_datetime_format: str diff --git a/lib/galaxy/config/schemas/config_schema.yml b/lib/galaxy/config/schemas/config_schema.yml index 731b35c7840..7b817e93102 100644 --- a/lib/galaxy/config/schemas/config_schema.yml +++ b/lib/galaxy/config/schemas/config_schema.yml @@ -3085,6 +3085,13 @@ mapping: desc: | Enables and disables OpenID Connect (OIDC) support. + oidc_require_refresh: + type: bool + default: false + required: false + desc: | + Require a current OIDC session. Users will be redirected to OIDC login if refresh fails. + oidc_config_file: type: str default: oidc_config.xml From e3ebc6839a377a04774b9cd901d540158515ad33 Mon Sep 17 00:00:00 2001 From: marius-mather Date: Fri, 10 Apr 2026 13:04:10 +1000 Subject: [PATCH 02/24] feat: check if reauthentication is required when refreshing with OIDC provider --- lib/galaxy/authnz/managers.py | 55 +++++++++++++++++++++++++++++------ 1 file changed, 46 insertions(+), 9 deletions(-) diff --git a/lib/galaxy/authnz/managers.py b/lib/galaxy/authnz/managers.py index b6ed0197675..624e0bc2c64 100644 --- a/lib/galaxy/authnz/managers.py +++ b/lib/galaxy/authnz/managers.py @@ -1,6 +1,6 @@ import builtins import logging -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, TypedDict import jwt as pyjwt from social_core.exceptions import ( @@ -45,6 +45,11 @@ DEFAULT_OIDC_IDP_ICONS = { } +class RefreshResult(TypedDict): + refreshed: bool + reauthentication_required: bool + + class AuthnzManager: def __init__(self, app, oidc_config_file, oidc_backends_config_file): """ @@ -279,27 +284,59 @@ class AuthnzManager: log.warning(msg) raise exceptions.ItemAccessibilityException(msg) - def refresh_expiring_oidc_tokens_for_provider(self, trans, auth): + @staticmethod + def _reauth_required_from_refresh_exception(exc): + """ + Check if the exception is due to a failed refresh attempt. + """ + response = getattr(exc, "response", None) + if response is None: + return False + # Auth failures on token refresh should force re-authentication. + if response.status_code in (400, 401, 403): + return True + return False + + def refresh_expiring_oidc_tokens_for_provider(self, trans, auth) -> RefreshResult: + """ + Refresh expiring OIDC tokens for a specific provider. + + Returns: + RefreshResult: A dictionary containing a boolean indicating success, and a boolean + indicating if reauthentication is required + """ try: success, message, backend = self._get_authnz_backend(auth.provider) if success is False: msg = f"An error occurred when refreshing user token on `{auth.provider}` identity provider: {message}" log.error(msg) - return False + return {"refreshed": False, "reauthentication_required": False} refreshed = backend.refresh(trans, auth) if refreshed: log.debug(f"Refreshed user token via `{auth.provider}` identity provider") - return True - except Exception: + return {"refreshed": refreshed, "reauthentication_required": False} + except Exception as e: log.exception("An error occurred when refreshing user token") - return False + if self._reauth_required_from_refresh_exception(e): + return {"refreshed": False, "reauthentication_required": True} + return {"refreshed": False, "reauthentication_required": False} - def refresh_expiring_oidc_tokens(self, trans, user=None): + def refresh_expiring_oidc_tokens(self, trans, user=None) -> str | None: + """ + Refresh expiring OIDC tokens for all providers associated with a user. + + Returns: + str | None: The provider name if refresh fails and require_refresh is enabled, otherwise None + """ user = trans.user or user if not isinstance(user, model.User): - return + return None for auth in user.social_auth or []: - self.refresh_expiring_oidc_tokens_for_provider(trans, auth) + result = self.refresh_expiring_oidc_tokens_for_provider(trans, auth) + # Redirect to OIDC login if refresh fails and require_refresh is enabled + if trans.app.config.oidc_require_refresh and result["reauthentication_required"]: + return auth.provider + return None def authenticate(self, provider, trans, idphint=None): """ From e36260c934f91956e3d743df1c3b702cf1dc90ce Mon Sep 17 00:00:00 2001 From: marius-mather Date: Fri, 10 Apr 2026 13:36:53 +1000 Subject: [PATCH 03/24] feat: logout and redirect to login on a failed ODIC refresh --- lib/galaxy/webapps/base/webapp.py | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/webapps/base/webapp.py b/lib/galaxy/webapps/base/webapp.py index aea09627b0e..27248853a37 100644 --- a/lib/galaxy/webapps/base/webapp.py +++ b/lib/galaxy/webapps/base/webapp.py @@ -358,7 +358,12 @@ class GalaxyWebTransaction(base.DefaultWebTransaction, context.ProvidesHistoryCo self._ensure_valid_session(session_cookie) if hasattr(self.app, "authnz_manager") and self.app.authnz_manager: - self.app.authnz_manager.refresh_expiring_oidc_tokens(self) + # Check for expiring tokens and refresh them. If configured, require a reauthentication + # on failed refresh. + reauth_provider = self.app.authnz_manager.refresh_expiring_oidc_tokens(self) + if self.app.config.oidc_require_refresh and reauth_provider: + self.handle_user_reauthentication(reauth_provider) + return if self.galaxy_session: # When we've authenticated by session, we have to check the @@ -893,6 +898,22 @@ class GalaxyWebTransaction(base.DefaultWebTransaction, context.ProvidesHistoryCo elif self.webapp.name == "tool_shed": self.__update_session_cookie(name="galaxycommunitysession") + def handle_user_reauthentication(self, reauth_provider: str): + """ + Handle user being required to log in again after failed OIDC refresh + """ + log.info("OIDC refresh failed terminally for provider `%s`, forcing re-login", reauth_provider) + if self.galaxy_session: + self.handle_user_logout() + if self.environ.get("is_api_request", False): + self.response.status = 401 + self.error_message = "Authentication session expired. Please log in again." + self.user = None + self.galaxy_session = None + else: + self.response.send_redirect(url_for(f"/authnz/{reauth_provider}/login", redirect="true", next="/")) + return + def get_galaxy_session(self): """ Return the current galaxy session From c3e15803d7dfab728a3c3ba0c8073a64e62eb7ad Mon Sep 17 00:00:00 2001 From: marius-mather Date: Fri, 10 Apr 2026 13:40:40 +1000 Subject: [PATCH 04/24] test: add tests of OIDC refresh behaviour --- test/unit/authnz/test_authnz.py | 71 ++++++++++++++++++++++++++++++++- 1 file changed, 69 insertions(+), 2 deletions(-) diff --git a/test/unit/authnz/test_authnz.py b/test/unit/authnz/test_authnz.py index 1b72b4756b9..df8330d28b8 100644 --- a/test/unit/authnz/test_authnz.py +++ b/test/unit/authnz/test_authnz.py @@ -1,12 +1,19 @@ import tempfile -from typing import Optional -from unittest.mock import MagicMock +from types import SimpleNamespace +from typing import Optional, cast +from unittest.mock import MagicMock, patch import pytest from social_core.utils import setting_name +from galaxy import model +from galaxy.app_unittest_utils import galaxy_mock from galaxy.authnz.managers import AuthnzManager +from galaxy.structured_app import BasicSharedApp from galaxy.util import asbool +from ..webapps.test_webapp_base import CORSParsingMockConfig, StubGalaxyWebTransaction + +from galaxy.webapps.base.webapp import WebApplication @pytest.fixture @@ -199,3 +206,63 @@ def test_missing_idphint_is_none(mock_app): app_config=mock_app.config, ) assert psa.config.get("IDPHINT") is None, "IDPHINT must be None when is absent from XML" + + +def test_refresh_expiring_oidc_tokens_returns_none_after_successful_refresh(mock_app): + _, oidc_path = create_oidc_config() + _, backend_path = create_backend_config(provider_name="oidc") + mock_app.config.oidc = {} + manager = AuthnzManager(app=mock_app, oidc_config_file=oidc_path, oidc_backends_config_file=backend_path) + backend = MagicMock() + backend.refresh.return_value = True + manager._get_authnz_backend = MagicMock(return_value=(True, None, backend)) + + user = model.User(email="user@example.com", password="password") + auth = model.UserAuthnzToken(provider="oidc", uid="user-1", extra_data={"refresh_token": "refresh"}, user=user) + user.social_auth.append(auth) + trans = SimpleNamespace(user=user, app=SimpleNamespace(config=SimpleNamespace(oidc_require_refresh=True))) + + reauth_provider = manager.refresh_expiring_oidc_tokens(trans) + + assert reauth_provider is None + backend.refresh.assert_called_once_with(trans, auth) + + +def test_refresh_expiring_oidc_tokens_returns_provider_on_terminal_refresh_failure(mock_app): + _, oidc_path = create_oidc_config() + _, backend_path = create_backend_config(provider_name="oidc") + mock_app.config.oidc = {} + manager = AuthnzManager(app=mock_app, oidc_config_file=oidc_path, oidc_backends_config_file=backend_path) + + class RefreshFailure(Exception): + def __init__(self): + self.response = SimpleNamespace(status_code=400) + + backend = MagicMock() + backend.refresh.side_effect = RefreshFailure() + manager._get_authnz_backend = MagicMock(return_value=(True, None, backend)) + + user = model.User(email="user@example.com", password="password") + auth = model.UserAuthnzToken(provider="oidc", uid="user-1", extra_data={"refresh_token": "refresh"}, user=user) + user.social_auth.append(auth) + trans = SimpleNamespace(user=user, app=SimpleNamespace(config=SimpleNamespace(oidc_require_refresh=True))) + + reauth_provider = manager.refresh_expiring_oidc_tokens(trans) + + assert reauth_provider == "oidc" + backend.refresh.assert_called_once_with(trans, auth) + + +def test_redirects_to_oidc_login_on_terminal_refresh_failure() -> None: + app = cast(BasicSharedApp, galaxy_mock.MockApp()) + app.config = CORSParsingMockConfig(oidc_require_refresh=True) + app.authnz_manager = MagicMock() + app.authnz_manager.refresh_expiring_oidc_tokens.return_value = "oidc" + webapp = cast(WebApplication, galaxy_mock.MockWebapp(app.security)) + environ = galaxy_mock.buildMockEnviron() + + with patch("galaxy.webapps.base.webapp.url_for", return_value="/authnz/oidc/login?redirect=true&next=%2F"): + with pytest.raises(Exception) as exc_info: + StubGalaxyWebTransaction(environ, app, webapp, "session_cookie") + + assert exc_info.value.location == "/authnz/oidc/login?redirect=true&next=%2F" From 0f2b2f55034a43dda0786fb7cc6155ed4c324f4d Mon Sep 17 00:00:00 2001 From: marius-mather Date: Fri, 10 Apr 2026 14:20:55 +1000 Subject: [PATCH 05/24] chore: linting --- lib/galaxy/authnz/managers.py | 5 ++++- test/unit/authnz/test_authnz.py | 16 ++++++++++++---- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/authnz/managers.py b/lib/galaxy/authnz/managers.py index 624e0bc2c64..e1c888228c1 100644 --- a/lib/galaxy/authnz/managers.py +++ b/lib/galaxy/authnz/managers.py @@ -1,6 +1,9 @@ import builtins import logging -from typing import TYPE_CHECKING, TypedDict +from typing import ( + TYPE_CHECKING, + TypedDict, +) import jwt as pyjwt from social_core.exceptions import ( diff --git a/test/unit/authnz/test_authnz.py b/test/unit/authnz/test_authnz.py index df8330d28b8..6886c06941e 100644 --- a/test/unit/authnz/test_authnz.py +++ b/test/unit/authnz/test_authnz.py @@ -1,7 +1,13 @@ import tempfile from types import SimpleNamespace -from typing import Optional, cast -from unittest.mock import MagicMock, patch +from typing import ( + cast, + Optional, +) +from unittest.mock import ( + MagicMock, + patch, +) import pytest from social_core.utils import setting_name @@ -11,9 +17,11 @@ from galaxy.app_unittest_utils import galaxy_mock from galaxy.authnz.managers import AuthnzManager from galaxy.structured_app import BasicSharedApp from galaxy.util import asbool -from ..webapps.test_webapp_base import CORSParsingMockConfig, StubGalaxyWebTransaction - from galaxy.webapps.base.webapp import WebApplication +from ..webapps.test_webapp_base import ( + CORSParsingMockConfig, + StubGalaxyWebTransaction, +) @pytest.fixture From ce69d1b57c839375ff98c7f0fd4a897ba4b60f59 Mon Sep 17 00:00:00 2001 From: marius-mather Date: Fri, 10 Apr 2026 14:32:08 +1000 Subject: [PATCH 06/24] chore: fix mypy errors in tests --- test/unit/authnz/test_authnz.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/test/unit/authnz/test_authnz.py b/test/unit/authnz/test_authnz.py index 6886c06941e..1472e98b2fa 100644 --- a/test/unit/authnz/test_authnz.py +++ b/test/unit/authnz/test_authnz.py @@ -1,6 +1,7 @@ import tempfile from types import SimpleNamespace from typing import ( + Any, cast, Optional, ) @@ -11,6 +12,7 @@ from unittest.mock import ( import pytest from social_core.utils import setting_name +from webob.exc import HTTPFound from galaxy import model from galaxy.app_unittest_utils import galaxy_mock @@ -223,14 +225,14 @@ def test_refresh_expiring_oidc_tokens_returns_none_after_successful_refresh(mock manager = AuthnzManager(app=mock_app, oidc_config_file=oidc_path, oidc_backends_config_file=backend_path) backend = MagicMock() backend.refresh.return_value = True - manager._get_authnz_backend = MagicMock(return_value=(True, None, backend)) user = model.User(email="user@example.com", password="password") auth = model.UserAuthnzToken(provider="oidc", uid="user-1", extra_data={"refresh_token": "refresh"}, user=user) user.social_auth.append(auth) trans = SimpleNamespace(user=user, app=SimpleNamespace(config=SimpleNamespace(oidc_require_refresh=True))) - reauth_provider = manager.refresh_expiring_oidc_tokens(trans) + with patch.object(manager, "_get_authnz_backend", return_value=(True, None, backend)): + reauth_provider = manager.refresh_expiring_oidc_tokens(trans) assert reauth_provider is None backend.refresh.assert_called_once_with(trans, auth) @@ -248,21 +250,21 @@ def test_refresh_expiring_oidc_tokens_returns_provider_on_terminal_refresh_failu backend = MagicMock() backend.refresh.side_effect = RefreshFailure() - manager._get_authnz_backend = MagicMock(return_value=(True, None, backend)) user = model.User(email="user@example.com", password="password") auth = model.UserAuthnzToken(provider="oidc", uid="user-1", extra_data={"refresh_token": "refresh"}, user=user) user.social_auth.append(auth) trans = SimpleNamespace(user=user, app=SimpleNamespace(config=SimpleNamespace(oidc_require_refresh=True))) - reauth_provider = manager.refresh_expiring_oidc_tokens(trans) + with patch.object(manager, "_get_authnz_backend", return_value=(True, None, backend)): + reauth_provider = manager.refresh_expiring_oidc_tokens(trans) assert reauth_provider == "oidc" backend.refresh.assert_called_once_with(trans, auth) def test_redirects_to_oidc_login_on_terminal_refresh_failure() -> None: - app = cast(BasicSharedApp, galaxy_mock.MockApp()) + app = cast(Any, galaxy_mock.MockApp()) app.config = CORSParsingMockConfig(oidc_require_refresh=True) app.authnz_manager = MagicMock() app.authnz_manager.refresh_expiring_oidc_tokens.return_value = "oidc" @@ -270,7 +272,7 @@ def test_redirects_to_oidc_login_on_terminal_refresh_failure() -> None: environ = galaxy_mock.buildMockEnviron() with patch("galaxy.webapps.base.webapp.url_for", return_value="/authnz/oidc/login?redirect=true&next=%2F"): - with pytest.raises(Exception) as exc_info: + with pytest.raises(HTTPFound) as exc_info: StubGalaxyWebTransaction(environ, app, webapp, "session_cookie") assert exc_info.value.location == "/authnz/oidc/login?redirect=true&next=%2F" From 23cb47f264730799e6f6fa921b05bf7b48a4f4eb Mon Sep 17 00:00:00 2001 From: marius-mather Date: Fri, 10 Apr 2026 14:43:04 +1000 Subject: [PATCH 07/24] chore: linting again --- test/unit/authnz/test_authnz.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit/authnz/test_authnz.py b/test/unit/authnz/test_authnz.py index 1472e98b2fa..c098e4a5d83 100644 --- a/test/unit/authnz/test_authnz.py +++ b/test/unit/authnz/test_authnz.py @@ -17,7 +17,6 @@ from webob.exc import HTTPFound from galaxy import model from galaxy.app_unittest_utils import galaxy_mock from galaxy.authnz.managers import AuthnzManager -from galaxy.structured_app import BasicSharedApp from galaxy.util import asbool from galaxy.webapps.base.webapp import WebApplication from ..webapps.test_webapp_base import ( From 7564e70f04c35fdf92ee86ee8a2a56c887a14366 Mon Sep 17 00:00:00 2001 From: marius-mather Date: Wed, 15 Apr 2026 13:49:41 +1000 Subject: [PATCH 08/24] test: add test of expired token for API request --- test/unit/authnz/test_authnz.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/unit/authnz/test_authnz.py b/test/unit/authnz/test_authnz.py index c098e4a5d83..b88d0704355 100644 --- a/test/unit/authnz/test_authnz.py +++ b/test/unit/authnz/test_authnz.py @@ -275,3 +275,19 @@ def test_redirects_to_oidc_login_on_terminal_refresh_failure() -> None: StubGalaxyWebTransaction(environ, app, webapp, "session_cookie") assert exc_info.value.location == "/authnz/oidc/login?redirect=true&next=%2F" + + +def test_returns_401_for_api_request_on_terminal_refresh_failure() -> None: + app = cast(Any, galaxy_mock.MockApp()) + app.config = CORSParsingMockConfig(oidc_require_refresh=True) + app.authnz_manager = MagicMock() + app.authnz_manager.refresh_expiring_oidc_tokens.return_value = "oidc" + webapp = cast(WebApplication, galaxy_mock.MockWebapp(app.security)) + environ = galaxy_mock.buildMockEnviron(PATH_INFO="/api/users/current", is_api_request=True) + + trans = StubGalaxyWebTransaction(environ, app, webapp, "session_cookie") + + assert trans.response.status == 401 + assert trans.error_message == "Authentication session expired. Please log in again." + assert trans.user is None + assert trans.galaxy_session is None From 8cbebf3e4648ef00528cfaf4a06f4e0faa42098e Mon Sep 17 00:00:00 2001 From: marius-mather Date: Wed, 15 Apr 2026 13:52:12 +1000 Subject: [PATCH 09/24] test: check API requests are unaffected on successful refresh --- test/unit/authnz/test_authnz.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/unit/authnz/test_authnz.py b/test/unit/authnz/test_authnz.py index b88d0704355..53ff67c6d3b 100644 --- a/test/unit/authnz/test_authnz.py +++ b/test/unit/authnz/test_authnz.py @@ -291,3 +291,21 @@ def test_returns_401_for_api_request_on_terminal_refresh_failure() -> None: assert trans.error_message == "Authentication session expired. Please log in again." assert trans.user is None assert trans.galaxy_session is None + + +def test_allows_api_request_on_successful_oidc_refresh() -> None: + """ + Test that API requests proceed when the refresh succeeds. + """ + app = cast(Any, galaxy_mock.MockApp()) + app.config = CORSParsingMockConfig(oidc_require_refresh=True) + app.authnz_manager = MagicMock() + app.authnz_manager.refresh_expiring_oidc_tokens.return_value = None + webapp = cast(WebApplication, galaxy_mock.MockWebapp(app.security)) + environ = galaxy_mock.buildMockEnviron(PATH_INFO="/api/users/current", is_api_request=True) + + trans = StubGalaxyWebTransaction(environ, app, webapp, "session_cookie") + + assert trans.response.status == 200 + assert trans.error_message is None + assert trans.galaxy_session is None From d550fee715dbde8efe0dd3f95d1c94e3bad11fef Mon Sep 17 00:00:00 2001 From: marius-mather Date: Fri, 17 Apr 2026 10:29:41 +1000 Subject: [PATCH 10/24] test: update tests of token refresh --- lib/galaxy/authnz/managers.py | 22 ++++------------- test/unit/authnz/test_authnz.py | 42 ++++++++++++++++++++++++++++----- 2 files changed, 41 insertions(+), 23 deletions(-) diff --git a/lib/galaxy/authnz/managers.py b/lib/galaxy/authnz/managers.py index e1c888228c1..2574acd7f3e 100644 --- a/lib/galaxy/authnz/managers.py +++ b/lib/galaxy/authnz/managers.py @@ -9,7 +9,7 @@ import jwt as pyjwt from social_core.exceptions import ( AuthAlreadyAssociated, AuthCanceled, - AuthTokenError, + AuthForbidden, AuthTokenError, ) from galaxy import ( @@ -287,19 +287,6 @@ class AuthnzManager: log.warning(msg) raise exceptions.ItemAccessibilityException(msg) - @staticmethod - def _reauth_required_from_refresh_exception(exc): - """ - Check if the exception is due to a failed refresh attempt. - """ - response = getattr(exc, "response", None) - if response is None: - return False - # Auth failures on token refresh should force re-authentication. - if response.status_code in (400, 401, 403): - return True - return False - def refresh_expiring_oidc_tokens_for_provider(self, trans, auth) -> RefreshResult: """ Refresh expiring OIDC tokens for a specific provider. @@ -318,10 +305,11 @@ class AuthnzManager: if refreshed: log.debug(f"Refreshed user token via `{auth.provider}` identity provider") return {"refreshed": refreshed, "reauthentication_required": False} + except (AuthTokenError, AuthCanceled, AuthForbidden): + log.warning("Authentication session has expired or is invalid, reauth required.") + return {"refreshed": False, "reauthentication_required": True} except Exception as e: - log.exception("An error occurred when refreshing user token") - if self._reauth_required_from_refresh_exception(e): - return {"refreshed": False, "reauthentication_required": True} + log.warning(f"An error occurred when refreshing user token: {e}") return {"refreshed": False, "reauthentication_required": False} def refresh_expiring_oidc_tokens(self, trans, user=None) -> str | None: diff --git a/test/unit/authnz/test_authnz.py b/test/unit/authnz/test_authnz.py index 53ff67c6d3b..fe705e81a48 100644 --- a/test/unit/authnz/test_authnz.py +++ b/test/unit/authnz/test_authnz.py @@ -11,6 +11,11 @@ from unittest.mock import ( ) import pytest +from social_core.exceptions import ( + AuthCanceled, + AuthForbidden, + AuthTokenError, +) from social_core.utils import setting_name from webob.exc import HTTPFound @@ -237,18 +242,22 @@ def test_refresh_expiring_oidc_tokens_returns_none_after_successful_refresh(mock backend.refresh.assert_called_once_with(trans, auth) -def test_refresh_expiring_oidc_tokens_returns_provider_on_terminal_refresh_failure(mock_app): +@pytest.mark.parametrize( + "refresh_exception", + [ + AuthTokenError(backend=None), + AuthCanceled(backend=None), + AuthForbidden(backend=None), + ], +) +def test_refresh_expiring_oidc_tokens_returns_provider_on_terminal_refresh_failure(mock_app, refresh_exception): _, oidc_path = create_oidc_config() _, backend_path = create_backend_config(provider_name="oidc") mock_app.config.oidc = {} manager = AuthnzManager(app=mock_app, oidc_config_file=oidc_path, oidc_backends_config_file=backend_path) - class RefreshFailure(Exception): - def __init__(self): - self.response = SimpleNamespace(status_code=400) - backend = MagicMock() - backend.refresh.side_effect = RefreshFailure() + backend.refresh.side_effect = refresh_exception user = model.User(email="user@example.com", password="password") auth = model.UserAuthnzToken(provider="oidc", uid="user-1", extra_data={"refresh_token": "refresh"}, user=user) @@ -262,6 +271,27 @@ def test_refresh_expiring_oidc_tokens_returns_provider_on_terminal_refresh_failu backend.refresh.assert_called_once_with(trans, auth) +def test_refresh_expiring_oidc_tokens_returns_none_on_unexpected_refresh_failure(mock_app): + _, oidc_path = create_oidc_config() + _, backend_path = create_backend_config(provider_name="oidc") + mock_app.config.oidc = {} + manager = AuthnzManager(app=mock_app, oidc_config_file=oidc_path, oidc_backends_config_file=backend_path) + + backend = MagicMock() + backend.refresh.side_effect = RuntimeError("unexpected refresh failure") + + user = model.User(email="user@example.com", password="password") + auth = model.UserAuthnzToken(provider="oidc", uid="user-1", extra_data={"refresh_token": "refresh"}, user=user) + user.social_auth.append(auth) + trans = SimpleNamespace(user=user, app=SimpleNamespace(config=SimpleNamespace(oidc_require_refresh=True))) + + with patch.object(manager, "_get_authnz_backend", return_value=(True, None, backend)): + reauth_provider = manager.refresh_expiring_oidc_tokens(trans) + + assert reauth_provider is None + backend.refresh.assert_called_once_with(trans, auth) + + def test_redirects_to_oidc_login_on_terminal_refresh_failure() -> None: app = cast(Any, galaxy_mock.MockApp()) app.config = CORSParsingMockConfig(oidc_require_refresh=True) From 038a75e9c92be5745bab77c2021433519e67a636 Mon Sep 17 00:00:00 2001 From: marius-mather Date: Fri, 17 Apr 2026 10:37:42 +1000 Subject: [PATCH 11/24] chore: better type annotations for refresh/reauth methods --- lib/galaxy/authnz/managers.py | 8 +++++--- lib/galaxy/webapps/base/webapp.py | 3 +-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/authnz/managers.py b/lib/galaxy/authnz/managers.py index 2574acd7f3e..84a616f7367 100644 --- a/lib/galaxy/authnz/managers.py +++ b/lib/galaxy/authnz/managers.py @@ -1,7 +1,7 @@ import builtins import logging from typing import ( - TYPE_CHECKING, + Optional, TYPE_CHECKING, TypedDict, ) @@ -32,6 +32,8 @@ from .psa_authnz import ( BACKENDS_NAME, PSAAuthnz, ) +from ..model import UserAuthnzToken +from ..webapps.base.webapp import GalaxyWebTransaction if TYPE_CHECKING: from galaxy.managers.context import ProvidesAppContext @@ -287,7 +289,7 @@ class AuthnzManager: log.warning(msg) raise exceptions.ItemAccessibilityException(msg) - def refresh_expiring_oidc_tokens_for_provider(self, trans, auth) -> RefreshResult: + def refresh_expiring_oidc_tokens_for_provider(self, trans: GalaxyWebTransaction, auth: UserAuthnzToken) -> RefreshResult: """ Refresh expiring OIDC tokens for a specific provider. @@ -312,7 +314,7 @@ class AuthnzManager: log.warning(f"An error occurred when refreshing user token: {e}") return {"refreshed": False, "reauthentication_required": False} - def refresh_expiring_oidc_tokens(self, trans, user=None) -> str | None: + def refresh_expiring_oidc_tokens(self, trans: GalaxyWebTransaction, user: Optional[model.User] = None) -> str | None: """ Refresh expiring OIDC tokens for all providers associated with a user. diff --git a/lib/galaxy/webapps/base/webapp.py b/lib/galaxy/webapps/base/webapp.py index 27248853a37..93ab1e6d5e2 100644 --- a/lib/galaxy/webapps/base/webapp.py +++ b/lib/galaxy/webapps/base/webapp.py @@ -898,7 +898,7 @@ class GalaxyWebTransaction(base.DefaultWebTransaction, context.ProvidesHistoryCo elif self.webapp.name == "tool_shed": self.__update_session_cookie(name="galaxycommunitysession") - def handle_user_reauthentication(self, reauth_provider: str): + def handle_user_reauthentication(self, reauth_provider: str) -> None: """ Handle user being required to log in again after failed OIDC refresh """ @@ -912,7 +912,6 @@ class GalaxyWebTransaction(base.DefaultWebTransaction, context.ProvidesHistoryCo self.galaxy_session = None else: self.response.send_redirect(url_for(f"/authnz/{reauth_provider}/login", redirect="true", next="/")) - return def get_galaxy_session(self): """ From 2e973249381315bf3bb34a98638700dff4aaf2da Mon Sep 17 00:00:00 2001 From: marius-mather Date: Fri, 17 Apr 2026 11:00:56 +1000 Subject: [PATCH 12/24] chore: more type hints for auth functions --- lib/galaxy/authnz/managers.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/authnz/managers.py b/lib/galaxy/authnz/managers.py index 84a616f7367..295b52cb4b4 100644 --- a/lib/galaxy/authnz/managers.py +++ b/lib/galaxy/authnz/managers.py @@ -1,7 +1,8 @@ import builtins import logging from typing import ( - Optional, TYPE_CHECKING, + Optional, + TYPE_CHECKING, TypedDict, ) @@ -9,7 +10,8 @@ import jwt as pyjwt from social_core.exceptions import ( AuthAlreadyAssociated, AuthCanceled, - AuthForbidden, AuthTokenError, + AuthForbidden, + AuthTokenError, ) from galaxy import ( @@ -234,7 +236,7 @@ class AuthnzManager: # None, if no allowed idp list is set, and a list of EntityIDs if configured (in oidc_backend) return self.allowed_idps - def _unify_provider_name(self, provider): + def _unify_provider_name(self, provider: str) -> str | None: if provider.lower() in self.oidc_backends_config: return provider.lower() for k, v in BACKENDS_NAME.items(): @@ -242,7 +244,7 @@ class AuthnzManager: return k.lower() return None - def _get_authnz_backend(self, provider: str, idphint=None): + def _get_authnz_backend(self, provider: str, idphint: str | None = None) -> tuple[bool, str, PSAAuthnz | None]: unified_provider_name = self._unify_provider_name(provider) if unified_provider_name in self.oidc_backends_config: provider = unified_provider_name @@ -331,7 +333,9 @@ class AuthnzManager: return auth.provider return None - def authenticate(self, provider, trans, idphint=None): + def authenticate( + self, provider: str, trans: GalaxyWebTransaction, idphint: str | None = None + ) -> tuple[bool, str, str | None]: """ :type provider: string :param provider: set the name of the identity provider to be From 96dc07d7eafdfcc5b1bc3229504a2b77819ac340 Mon Sep 17 00:00:00 2001 From: marius-mather Date: Fri, 17 Apr 2026 11:04:18 +1000 Subject: [PATCH 13/24] test: update tests to use actual AuthnzManager where possible --- test/unit/authnz/test_authnz.py | 157 +++++++++++++++++++++----------- 1 file changed, 103 insertions(+), 54 deletions(-) diff --git a/test/unit/authnz/test_authnz.py b/test/unit/authnz/test_authnz.py index fe705e81a48..5aabe1d845d 100644 --- a/test/unit/authnz/test_authnz.py +++ b/test/unit/authnz/test_authnz.py @@ -1,5 +1,4 @@ import tempfile -from types import SimpleNamespace from typing import ( Any, cast, @@ -9,6 +8,7 @@ from unittest.mock import ( MagicMock, patch, ) +from urllib.parse import urlencode import pytest from social_core.exceptions import ( @@ -23,6 +23,8 @@ from galaxy import model from galaxy.app_unittest_utils import galaxy_mock from galaxy.authnz.managers import AuthnzManager from galaxy.util import asbool +from galaxy.web.framework import base as web_framework_base +from galaxy.web.framework.base import Response from galaxy.webapps.base.webapp import WebApplication from ..webapps.test_webapp_base import ( CORSParsingMockConfig, @@ -91,6 +93,56 @@ def create_backend_config( return contents, file.name +class FakeRefreshBackend: + refresh_result: bool = False + refresh_exception: Exception | None = None + + def __init__(self, provider, oidc_config, oidc_backend_config, app_config): + self.provider = provider + self.oidc_config = oidc_config + self.oidc_backend_config = oidc_backend_config + self.app_config = app_config + + def refresh(self, trans, auth): + if self.refresh_exception is not None: + raise self.refresh_exception + return self.refresh_result + + +class AuthenticatedStubGalaxyWebTransaction(StubGalaxyWebTransaction): + auth_user: model.User | None = None + + def _ensure_valid_session(self, session_cookie: str, create: bool = True) -> None: + self.user = self.auth_user + self.galaxy_session = None + + def _authenticate_api(self, session_cookie: str) -> Optional[str]: + self.user = self.auth_user + self.galaxy_session = None + return None + + +def _make_user_with_social_auth(provider: str = "oidc") -> model.User: + user = model.User(email="user@example.com", password="password") + auth = model.UserAuthnzToken(provider=provider, uid="user-1", extra_data={"refresh_token": "refresh"}, user=user) + user.social_auth.append(auth) + return user + + +def _make_authnz_manager(app: Any, provider_name: str = "oidc") -> AuthnzManager: + _, oidc_path = create_oidc_config() + _, backend_path = create_backend_config(provider_name=provider_name) + app.config.oidc = {} + return AuthnzManager(app=app, oidc_config_file=oidc_path, oidc_backends_config_file=backend_path) + + +def _make_mock_trans_with_user(user: model.User) -> galaxy_mock.MockTrans: + app = galaxy_mock.MockApp() + app.config.oidc_require_refresh = True + trans = galaxy_mock.MockTrans(app=app, user=user) + return trans + + def test_parse_backend_config(mock_app): config_values = { "url": "https://example.com", @@ -223,23 +275,16 @@ def test_missing_idphint_is_none(mock_app): def test_refresh_expiring_oidc_tokens_returns_none_after_successful_refresh(mock_app): - _, oidc_path = create_oidc_config() - _, backend_path = create_backend_config(provider_name="oidc") - mock_app.config.oidc = {} - manager = AuthnzManager(app=mock_app, oidc_config_file=oidc_path, oidc_backends_config_file=backend_path) - backend = MagicMock() - backend.refresh.return_value = True + user = _make_user_with_social_auth() + trans = _make_mock_trans_with_user(user) + manager = _make_authnz_manager(trans.app) + FakeRefreshBackend.refresh_result = True + FakeRefreshBackend.refresh_exception = None - user = model.User(email="user@example.com", password="password") - auth = model.UserAuthnzToken(provider="oidc", uid="user-1", extra_data={"refresh_token": "refresh"}, user=user) - user.social_auth.append(auth) - trans = SimpleNamespace(user=user, app=SimpleNamespace(config=SimpleNamespace(oidc_require_refresh=True))) - - with patch.object(manager, "_get_authnz_backend", return_value=(True, None, backend)): + with patch.object(AuthnzManager, "_get_identity_provider_factory", return_value=FakeRefreshBackend): reauth_provider = manager.refresh_expiring_oidc_tokens(trans) assert reauth_provider is None - backend.refresh.assert_called_once_with(trans, auth) @pytest.mark.parametrize( @@ -251,71 +296,72 @@ def test_refresh_expiring_oidc_tokens_returns_none_after_successful_refresh(mock ], ) def test_refresh_expiring_oidc_tokens_returns_provider_on_terminal_refresh_failure(mock_app, refresh_exception): - _, oidc_path = create_oidc_config() - _, backend_path = create_backend_config(provider_name="oidc") - mock_app.config.oidc = {} - manager = AuthnzManager(app=mock_app, oidc_config_file=oidc_path, oidc_backends_config_file=backend_path) + user = _make_user_with_social_auth() + trans = _make_mock_trans_with_user(user) + manager = _make_authnz_manager(trans.app) + FakeRefreshBackend.refresh_result = False + FakeRefreshBackend.refresh_exception = refresh_exception - backend = MagicMock() - backend.refresh.side_effect = refresh_exception - - user = model.User(email="user@example.com", password="password") - auth = model.UserAuthnzToken(provider="oidc", uid="user-1", extra_data={"refresh_token": "refresh"}, user=user) - user.social_auth.append(auth) - trans = SimpleNamespace(user=user, app=SimpleNamespace(config=SimpleNamespace(oidc_require_refresh=True))) - - with patch.object(manager, "_get_authnz_backend", return_value=(True, None, backend)): + with patch.object(AuthnzManager, "_get_identity_provider_factory", return_value=FakeRefreshBackend): reauth_provider = manager.refresh_expiring_oidc_tokens(trans) assert reauth_provider == "oidc" - backend.refresh.assert_called_once_with(trans, auth) def test_refresh_expiring_oidc_tokens_returns_none_on_unexpected_refresh_failure(mock_app): - _, oidc_path = create_oidc_config() - _, backend_path = create_backend_config(provider_name="oidc") - mock_app.config.oidc = {} - manager = AuthnzManager(app=mock_app, oidc_config_file=oidc_path, oidc_backends_config_file=backend_path) + user = _make_user_with_social_auth() + trans = _make_mock_trans_with_user(user) + manager = _make_authnz_manager(trans.app) + FakeRefreshBackend.refresh_result = False + FakeRefreshBackend.refresh_exception = RuntimeError("unexpected refresh failure") - backend = MagicMock() - backend.refresh.side_effect = RuntimeError("unexpected refresh failure") - - user = model.User(email="user@example.com", password="password") - auth = model.UserAuthnzToken(provider="oidc", uid="user-1", extra_data={"refresh_token": "refresh"}, user=user) - user.social_auth.append(auth) - trans = SimpleNamespace(user=user, app=SimpleNamespace(config=SimpleNamespace(oidc_require_refresh=True))) - - with patch.object(manager, "_get_authnz_backend", return_value=(True, None, backend)): + with patch.object(AuthnzManager, "_get_identity_provider_factory", return_value=FakeRefreshBackend): reauth_provider = manager.refresh_expiring_oidc_tokens(trans) assert reauth_provider is None - backend.refresh.assert_called_once_with(trans, auth) def test_redirects_to_oidc_login_on_terminal_refresh_failure() -> None: app = cast(Any, galaxy_mock.MockApp()) app.config = CORSParsingMockConfig(oidc_require_refresh=True) - app.authnz_manager = MagicMock() - app.authnz_manager.refresh_expiring_oidc_tokens.return_value = "oidc" + app.authnz_manager = _make_authnz_manager(app) webapp = cast(WebApplication, galaxy_mock.MockWebapp(app.security)) environ = galaxy_mock.buildMockEnviron() + AuthenticatedStubGalaxyWebTransaction.auth_user = _make_user_with_social_auth() + FakeRefreshBackend.refresh_result = False + FakeRefreshBackend.refresh_exception = AuthTokenError(backend=None) + original_send_redirect = Response.send_redirect + redirect_urls: list[str] = [] - with patch("galaxy.webapps.base.webapp.url_for", return_value="/authnz/oidc/login?redirect=true&next=%2F"): - with pytest.raises(HTTPFound) as exc_info: - StubGalaxyWebTransaction(environ, app, webapp, "session_cookie") + def capture_send_redirect(self, url: str): + redirect_urls.append(url) + return original_send_redirect(self, url) - assert exc_info.value.location == "/authnz/oidc/login?redirect=true&next=%2F" + def build_test_url(path: str, **query_params): + return f"{path}?{urlencode(query_params)}" + + with patch.object(AuthnzManager, "_get_identity_provider_factory", return_value=FakeRefreshBackend): + with patch.object(web_framework_base.routes, "url_for", side_effect=build_test_url): + with patch.object(Response, "send_redirect", autospec=True, side_effect=capture_send_redirect): + with pytest.raises(HTTPFound): + AuthenticatedStubGalaxyWebTransaction(environ, app, webapp, "session_cookie") + + assert redirect_urls + assert "/authnz/oidc/login" in redirect_urls[0] def test_returns_401_for_api_request_on_terminal_refresh_failure() -> None: app = cast(Any, galaxy_mock.MockApp()) app.config = CORSParsingMockConfig(oidc_require_refresh=True) - app.authnz_manager = MagicMock() - app.authnz_manager.refresh_expiring_oidc_tokens.return_value = "oidc" + app.authnz_manager = _make_authnz_manager(app) webapp = cast(WebApplication, galaxy_mock.MockWebapp(app.security)) environ = galaxy_mock.buildMockEnviron(PATH_INFO="/api/users/current", is_api_request=True) + AuthenticatedStubGalaxyWebTransaction.auth_user = _make_user_with_social_auth() + FakeRefreshBackend.refresh_result = False + FakeRefreshBackend.refresh_exception = AuthTokenError(backend=None) - trans = StubGalaxyWebTransaction(environ, app, webapp, "session_cookie") + with patch.object(AuthnzManager, "_get_identity_provider_factory", return_value=FakeRefreshBackend): + trans = AuthenticatedStubGalaxyWebTransaction(environ, app, webapp, "session_cookie") assert trans.response.status == 401 assert trans.error_message == "Authentication session expired. Please log in again." @@ -329,12 +375,15 @@ def test_allows_api_request_on_successful_oidc_refresh() -> None: """ app = cast(Any, galaxy_mock.MockApp()) app.config = CORSParsingMockConfig(oidc_require_refresh=True) - app.authnz_manager = MagicMock() - app.authnz_manager.refresh_expiring_oidc_tokens.return_value = None + app.authnz_manager = _make_authnz_manager(app) webapp = cast(WebApplication, galaxy_mock.MockWebapp(app.security)) environ = galaxy_mock.buildMockEnviron(PATH_INFO="/api/users/current", is_api_request=True) + AuthenticatedStubGalaxyWebTransaction.auth_user = _make_user_with_social_auth() + FakeRefreshBackend.refresh_result = True + FakeRefreshBackend.refresh_exception = None - trans = StubGalaxyWebTransaction(environ, app, webapp, "session_cookie") + with patch.object(AuthnzManager, "_get_identity_provider_factory", return_value=FakeRefreshBackend): + trans = AuthenticatedStubGalaxyWebTransaction(environ, app, webapp, "session_cookie") assert trans.response.status == 200 assert trans.error_message is None From ea3e65ee70ba39a140d5ec960a6febf48f8c6e08 Mon Sep 17 00:00:00 2001 From: marius-mather Date: Fri, 17 Apr 2026 11:06:22 +1000 Subject: [PATCH 14/24] chore: linting --- lib/galaxy/authnz/managers.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/authnz/managers.py b/lib/galaxy/authnz/managers.py index 295b52cb4b4..625f58bc702 100644 --- a/lib/galaxy/authnz/managers.py +++ b/lib/galaxy/authnz/managers.py @@ -291,7 +291,9 @@ class AuthnzManager: log.warning(msg) raise exceptions.ItemAccessibilityException(msg) - def refresh_expiring_oidc_tokens_for_provider(self, trans: GalaxyWebTransaction, auth: UserAuthnzToken) -> RefreshResult: + def refresh_expiring_oidc_tokens_for_provider( + self, trans: GalaxyWebTransaction, auth: UserAuthnzToken + ) -> RefreshResult: """ Refresh expiring OIDC tokens for a specific provider. @@ -316,7 +318,9 @@ class AuthnzManager: log.warning(f"An error occurred when refreshing user token: {e}") return {"refreshed": False, "reauthentication_required": False} - def refresh_expiring_oidc_tokens(self, trans: GalaxyWebTransaction, user: Optional[model.User] = None) -> str | None: + def refresh_expiring_oidc_tokens( + self, trans: GalaxyWebTransaction, user: Optional[model.User] = None + ) -> str | None: """ Refresh expiring OIDC tokens for all providers associated with a user. From 80fa406c3f4091f5fd2664b4e571ce564e9cb5cf Mon Sep 17 00:00:00 2001 From: marius-mather Date: Fri, 17 Apr 2026 11:41:16 +1000 Subject: [PATCH 15/24] chore: fix mypy errors - check when backend/provider is None --- lib/galaxy/authnz/managers.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/authnz/managers.py b/lib/galaxy/authnz/managers.py index 625f58bc702..42d038b6efc 100644 --- a/lib/galaxy/authnz/managers.py +++ b/lib/galaxy/authnz/managers.py @@ -246,7 +246,7 @@ class AuthnzManager: def _get_authnz_backend(self, provider: str, idphint: str | None = None) -> tuple[bool, str, PSAAuthnz | None]: unified_provider_name = self._unify_provider_name(provider) - if unified_provider_name in self.oidc_backends_config: + if unified_provider_name is not None and unified_provider_name in self.oidc_backends_config: provider = unified_provider_name identity_provider_class = self._get_identity_provider_factory(self.oidc_backends_implementation[provider]) try: @@ -302,7 +302,13 @@ class AuthnzManager: indicating if reauthentication is required """ try: + if auth.provider is None: + raise exceptions.AuthenticationFailed("Provider is not set") success, message, backend = self._get_authnz_backend(auth.provider) + if backend is None: + msg = f"Provider `{auth.provider}` not found" + log.error(msg) + return {"refreshed": False, "reauthentication_required": False} if success is False: msg = f"An error occurred when refreshing user token on `{auth.provider}` identity provider: {message}" log.error(msg) @@ -350,6 +356,8 @@ class AuthnzManager: """ try: success, message, backend = self._get_authnz_backend(provider, idphint=idphint) + if backend is None: + return False, f"Provider `{provider}` not found", None if success is False: return False, message, None # Check allowed IDPs for providers that support idphint (keycloak, cilogon) @@ -404,6 +412,8 @@ class AuthnzManager: def create_user(self, provider: str, token: str, trans: "ProvidesAppContext", login_redirect_url: str): try: success, message, backend = self._get_authnz_backend(provider) + if backend is None: + raise ValueError(f"Provider `{provider}` not found") if success is False: return False, message, (None, None) return success, message, backend.create_user(token, trans, login_redirect_url) From 2bdf872e1dd9d47cc0d5deeb5858be36d845e9f8 Mon Sep 17 00:00:00 2001 From: marius-mather Date: Fri, 17 Apr 2026 11:48:07 +1000 Subject: [PATCH 16/24] test: fix type errors in tests --- test/unit/authnz/test_authnz.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/test/unit/authnz/test_authnz.py b/test/unit/authnz/test_authnz.py index 5aabe1d845d..a90626f9aa9 100644 --- a/test/unit/authnz/test_authnz.py +++ b/test/unit/authnz/test_authnz.py @@ -11,6 +11,7 @@ from unittest.mock import ( from urllib.parse import urlencode import pytest +from social_core.backends.base import BaseAuth from social_core.exceptions import ( AuthCanceled, AuthForbidden, @@ -109,6 +110,9 @@ class FakeRefreshBackend: return self.refresh_result +FAKE_SOCIAL_AUTH_BACKEND = cast(BaseAuth, MagicMock()) + + class AuthenticatedStubGalaxyWebTransaction(StubGalaxyWebTransaction): auth_user: model.User | None = None @@ -138,7 +142,7 @@ def _make_authnz_manager(app: Any, provider_name: str = "oidc") -> AuthnzManager def _make_mock_trans_with_user(user: model.User) -> galaxy_mock.MockTrans: app = galaxy_mock.MockApp() - app.config.oidc_require_refresh = True + cast(Any, app.config).oidc_require_refresh = True trans = galaxy_mock.MockTrans(app=app, user=user) return trans @@ -282,7 +286,7 @@ def test_refresh_expiring_oidc_tokens_returns_none_after_successful_refresh(mock FakeRefreshBackend.refresh_exception = None with patch.object(AuthnzManager, "_get_identity_provider_factory", return_value=FakeRefreshBackend): - reauth_provider = manager.refresh_expiring_oidc_tokens(trans) + reauth_provider = manager.refresh_expiring_oidc_tokens(cast(Any, trans)) assert reauth_provider is None @@ -290,9 +294,9 @@ def test_refresh_expiring_oidc_tokens_returns_none_after_successful_refresh(mock @pytest.mark.parametrize( "refresh_exception", [ - AuthTokenError(backend=None), - AuthCanceled(backend=None), - AuthForbidden(backend=None), + AuthTokenError(backend=FAKE_SOCIAL_AUTH_BACKEND), + AuthCanceled(backend=FAKE_SOCIAL_AUTH_BACKEND), + AuthForbidden(backend=FAKE_SOCIAL_AUTH_BACKEND), ], ) def test_refresh_expiring_oidc_tokens_returns_provider_on_terminal_refresh_failure(mock_app, refresh_exception): @@ -303,7 +307,7 @@ def test_refresh_expiring_oidc_tokens_returns_provider_on_terminal_refresh_failu FakeRefreshBackend.refresh_exception = refresh_exception with patch.object(AuthnzManager, "_get_identity_provider_factory", return_value=FakeRefreshBackend): - reauth_provider = manager.refresh_expiring_oidc_tokens(trans) + reauth_provider = manager.refresh_expiring_oidc_tokens(cast(Any, trans)) assert reauth_provider == "oidc" @@ -316,7 +320,7 @@ def test_refresh_expiring_oidc_tokens_returns_none_on_unexpected_refresh_failure FakeRefreshBackend.refresh_exception = RuntimeError("unexpected refresh failure") with patch.object(AuthnzManager, "_get_identity_provider_factory", return_value=FakeRefreshBackend): - reauth_provider = manager.refresh_expiring_oidc_tokens(trans) + reauth_provider = manager.refresh_expiring_oidc_tokens(cast(Any, trans)) assert reauth_provider is None @@ -329,7 +333,7 @@ def test_redirects_to_oidc_login_on_terminal_refresh_failure() -> None: environ = galaxy_mock.buildMockEnviron() AuthenticatedStubGalaxyWebTransaction.auth_user = _make_user_with_social_auth() FakeRefreshBackend.refresh_result = False - FakeRefreshBackend.refresh_exception = AuthTokenError(backend=None) + FakeRefreshBackend.refresh_exception = AuthTokenError(backend=FAKE_SOCIAL_AUTH_BACKEND) original_send_redirect = Response.send_redirect redirect_urls: list[str] = [] @@ -358,7 +362,7 @@ def test_returns_401_for_api_request_on_terminal_refresh_failure() -> None: environ = galaxy_mock.buildMockEnviron(PATH_INFO="/api/users/current", is_api_request=True) AuthenticatedStubGalaxyWebTransaction.auth_user = _make_user_with_social_auth() FakeRefreshBackend.refresh_result = False - FakeRefreshBackend.refresh_exception = AuthTokenError(backend=None) + FakeRefreshBackend.refresh_exception = AuthTokenError(backend=FAKE_SOCIAL_AUTH_BACKEND) with patch.object(AuthnzManager, "_get_identity_provider_factory", return_value=FakeRefreshBackend): trans = AuthenticatedStubGalaxyWebTransaction(environ, app, webapp, "session_cookie") From 96988a86737db831bdf2e853e12b63bd607c713f Mon Sep 17 00:00:00 2001 From: marius-mather Date: Wed, 22 Apr 2026 10:07:49 +1000 Subject: [PATCH 17/24] fix: fix imports in authnz/managers.py --- lib/galaxy/authnz/managers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/authnz/managers.py b/lib/galaxy/authnz/managers.py index 42d038b6efc..fc393e2bf5d 100644 --- a/lib/galaxy/authnz/managers.py +++ b/lib/galaxy/authnz/managers.py @@ -18,6 +18,7 @@ from galaxy import ( exceptions, model, ) +from galaxy.model import UserAuthnzToken from galaxy.util import ( asbool, etree, @@ -30,12 +31,11 @@ from galaxy.util.resources import ( as_file, resource_path, ) +from galaxy.webapps.base.webapp import GalaxyWebTransaction from .psa_authnz import ( BACKENDS_NAME, PSAAuthnz, ) -from ..model import UserAuthnzToken -from ..webapps.base.webapp import GalaxyWebTransaction if TYPE_CHECKING: from galaxy.managers.context import ProvidesAppContext From 0e9e6f84997631127be6fe61acd10f7ac4dc4591 Mon Sep 17 00:00:00 2001 From: marius-mather Date: Thu, 23 Apr 2026 14:29:57 +1000 Subject: [PATCH 18/24] fix: only import GalaxyWebTransaction for type-checking --- lib/galaxy/authnz/managers.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/authnz/managers.py b/lib/galaxy/authnz/managers.py index fc393e2bf5d..0049d6277fe 100644 --- a/lib/galaxy/authnz/managers.py +++ b/lib/galaxy/authnz/managers.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import builtins import logging from typing import ( @@ -31,7 +33,6 @@ from galaxy.util.resources import ( as_file, resource_path, ) -from galaxy.webapps.base.webapp import GalaxyWebTransaction from .psa_authnz import ( BACKENDS_NAME, PSAAuthnz, @@ -39,6 +40,7 @@ from .psa_authnz import ( if TYPE_CHECKING: from galaxy.managers.context import ProvidesAppContext + from galaxy.webapps.base.webapp import GalaxyWebTransaction OIDC_BACKEND_SCHEMA = resource_path(__name__, "xsd/oidc_backends_config.xsd") From 552d3a5ce58560efaa06ead4318a374750c87aaf Mon Sep 17 00:00:00 2001 From: marius-mather Date: Thu, 23 Apr 2026 14:33:35 +1000 Subject: [PATCH 19/24] chore: style fix --- lib/galaxy/authnz/managers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/authnz/managers.py b/lib/galaxy/authnz/managers.py index 0049d6277fe..6360b059bd7 100644 --- a/lib/galaxy/authnz/managers.py +++ b/lib/galaxy/authnz/managers.py @@ -411,7 +411,7 @@ class AuthnzManager: log.exception(msg) return False, msg, (None, None) - def create_user(self, provider: str, token: str, trans: "ProvidesAppContext", login_redirect_url: str): + def create_user(self, provider: str, token: str, trans: ProvidesAppContext, login_redirect_url: str): try: success, message, backend = self._get_authnz_backend(provider) if backend is None: From 58632d14de820a0d2aeb68fe28960b4bda88cc94 Mon Sep 17 00:00:00 2001 From: marius-mather Date: Wed, 29 Apr 2026 13:28:08 +1000 Subject: [PATCH 20/24] refactor: move require refresh config down to the individual provider level --- lib/galaxy/authnz/managers.py | 2 ++ lib/galaxy/authnz/xsd/oidc_backends_config.xsd | 9 +++++++++ lib/galaxy/config/__init__.py | 1 - lib/galaxy/config/schemas/config_schema.yml | 7 ------- test/unit/authnz/test_authnz.py | 2 ++ 5 files changed, 13 insertions(+), 8 deletions(-) diff --git a/lib/galaxy/authnz/managers.py b/lib/galaxy/authnz/managers.py index 6360b059bd7..c16194b5708 100644 --- a/lib/galaxy/authnz/managers.py +++ b/lib/galaxy/authnz/managers.py @@ -177,6 +177,8 @@ class AuthnzManager: rtv["label"] = config_xml.find("label").text if config_xml.find("require_create_confirmation") is not None: rtv["require_create_confirmation"] = asbool(config_xml.find("require_create_confirmation").text) + if config_xml.find("require_session_refresh") is not None: + rtv["require_session_refresh"] = asbool(config_xml.find("require_session_refresh").text) if config_xml.find("prompt") is not None: rtv["prompt"] = config_xml.find("prompt").text if config_xml.find("api_url") is not None: diff --git a/lib/galaxy/authnz/xsd/oidc_backends_config.xsd b/lib/galaxy/authnz/xsd/oidc_backends_config.xsd index 610380f21fd..19d22b2a8b1 100644 --- a/lib/galaxy/authnz/xsd/oidc_backends_config.xsd +++ b/lib/galaxy/authnz/xsd/oidc_backends_config.xsd @@ -65,6 +65,15 @@ + + + + Require the user to refresh their session (via refresh token) + when the access token expires. Users will be required to reauthenticate + if refreshing fails. + + + diff --git a/lib/galaxy/config/__init__.py b/lib/galaxy/config/__init__.py index dea274271d2..a0c5aa09611 100644 --- a/lib/galaxy/config/__init__.py +++ b/lib/galaxy/config/__init__.py @@ -757,7 +757,6 @@ class GalaxyAppConfiguration(BaseAppConfiguration, CommonConfigurationMixin): mulled_channels: list[str] new_file_path: str nginx_upload_store: str - oidc_require_refresh: bool password_expiration_period: timedelta preserve_python_environment: str pretty_datetime_format: str diff --git a/lib/galaxy/config/schemas/config_schema.yml b/lib/galaxy/config/schemas/config_schema.yml index 7b817e93102..731b35c7840 100644 --- a/lib/galaxy/config/schemas/config_schema.yml +++ b/lib/galaxy/config/schemas/config_schema.yml @@ -3085,13 +3085,6 @@ mapping: desc: | Enables and disables OpenID Connect (OIDC) support. - oidc_require_refresh: - type: bool - default: false - required: false - desc: | - Require a current OIDC session. Users will be redirected to OIDC login if refresh fails. - oidc_config_file: type: str default: oidc_config.xml diff --git a/test/unit/authnz/test_authnz.py b/test/unit/authnz/test_authnz.py index a90626f9aa9..3fc4da8f02e 100644 --- a/test/unit/authnz/test_authnz.py +++ b/test/unit/authnz/test_authnz.py @@ -154,6 +154,7 @@ def test_parse_backend_config(mock_app): "client_secret": "abcd1234", "enable_idp_logout": "true", "require_create_confirmation": "false", + "require_session_refresh": "true", "accepted_audiences": "https://audience.example.com", "username_key": "custom_username", } @@ -170,6 +171,7 @@ def test_parse_backend_config(mock_app): # Boolean values should be parsed into bools assert parsed["enable_idp_logout"] == asbool(config_values["enable_idp_logout"]) assert parsed["require_create_confirmation"] == asbool(config_values["require_create_confirmation"]) + assert parsed["require_session_refresh"] == asbool(config_values["require_session_refresh"]) def test_psa_authnz_config(mock_app): From acdd9c12638dd1e8bf9443e1abb6dea236eb3da8 Mon Sep 17 00:00:00 2001 From: marius-mather Date: Wed, 29 Apr 2026 13:31:00 +1000 Subject: [PATCH 21/24] refactor: update logic for refreshing tokens --- lib/galaxy/authnz/managers.py | 5 ++++- lib/galaxy/webapps/base/webapp.py | 6 +++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/authnz/managers.py b/lib/galaxy/authnz/managers.py index c16194b5708..2578db9ebf3 100644 --- a/lib/galaxy/authnz/managers.py +++ b/lib/galaxy/authnz/managers.py @@ -342,8 +342,11 @@ class AuthnzManager: return None for auth in user.social_auth or []: result = self.refresh_expiring_oidc_tokens_for_provider(trans, auth) + config = self.oidc_backends_config.get(auth.provider, None) + if config is None: + continue # Redirect to OIDC login if refresh fails and require_refresh is enabled - if trans.app.config.oidc_require_refresh and result["reauthentication_required"]: + if config.get("require_session_refresh") and result["reauthentication_required"]: return auth.provider return None diff --git a/lib/galaxy/webapps/base/webapp.py b/lib/galaxy/webapps/base/webapp.py index 93ab1e6d5e2..76980af1c34 100644 --- a/lib/galaxy/webapps/base/webapp.py +++ b/lib/galaxy/webapps/base/webapp.py @@ -358,10 +358,10 @@ class GalaxyWebTransaction(base.DefaultWebTransaction, context.ProvidesHistoryCo self._ensure_valid_session(session_cookie) if hasattr(self.app, "authnz_manager") and self.app.authnz_manager: - # Check for expiring tokens and refresh them. If configured, require a reauthentication - # on failed refresh. + # Check for expiring tokens and refresh them. If configured (at the individual provider + # level), require a reauthentication on failed refresh. reauth_provider = self.app.authnz_manager.refresh_expiring_oidc_tokens(self) - if self.app.config.oidc_require_refresh and reauth_provider: + if reauth_provider: self.handle_user_reauthentication(reauth_provider) return From b2ac569eb212c5988ac09ffd79e07ecd2d2b28dc Mon Sep 17 00:00:00 2001 From: marius-mather Date: Wed, 29 Apr 2026 13:59:55 +1000 Subject: [PATCH 22/24] test: update tests of token refresh --- test/unit/authnz/test_authnz.py | 71 +++++++++++++++++++++++++++------ 1 file changed, 59 insertions(+), 12 deletions(-) diff --git a/test/unit/authnz/test_authnz.py b/test/unit/authnz/test_authnz.py index 3fc4da8f02e..5e972b0596e 100644 --- a/test/unit/authnz/test_authnz.py +++ b/test/unit/authnz/test_authnz.py @@ -47,6 +47,7 @@ OIDC_BACKEND_CONFIG_TEMPLATE = """ $galaxy_url/authnz/keycloak/callback {enable_idp_logout} {require_create_confirmation} + {require_session_refresh} {accepted_audiences} {username_key} @@ -76,6 +77,7 @@ def create_backend_config( client_secret="client_secret", enable_idp_logout="true", require_create_confirmation="false", + require_session_refresh="false", accepted_audiences="https://audience.example.com", username_key="custom_username", ) -> tuple[str, str]: @@ -86,6 +88,7 @@ def create_backend_config( client_secret=client_secret, enable_idp_logout=enable_idp_logout, require_create_confirmation=require_create_confirmation, + require_session_refresh=require_session_refresh, accepted_audiences=accepted_audiences, username_key=username_key, ) @@ -133,16 +136,19 @@ def _make_user_with_social_auth(provider: str = "oidc") -> model.User: return user -def _make_authnz_manager(app: Any, provider_name: str = "oidc") -> AuthnzManager: +def _make_authnz_manager( + app: Any, provider_name: str = "oidc", require_session_refresh: str = "false" +) -> AuthnzManager: _, oidc_path = create_oidc_config() - _, backend_path = create_backend_config(provider_name=provider_name) + _, backend_path = create_backend_config( + provider_name=provider_name, require_session_refresh=require_session_refresh + ) app.config.oidc = {} return AuthnzManager(app=app, oidc_config_file=oidc_path, oidc_backends_config_file=backend_path) def _make_mock_trans_with_user(user: model.User) -> galaxy_mock.MockTrans: app = galaxy_mock.MockApp() - cast(Any, app.config).oidc_require_refresh = True trans = galaxy_mock.MockTrans(app=app, user=user) return trans @@ -174,6 +180,32 @@ def test_parse_backend_config(mock_app): assert parsed["require_session_refresh"] == asbool(config_values["require_session_refresh"]) +def test_parse_backend_config_bool_defaults(mock_app): + # XML config without boolean fields + config = """ + + + https://example.com + abcd1234 + abcdef99999 + $galaxy_url/authnz/oidc/callback + + + """ + config_file = tempfile.NamedTemporaryFile(mode="w", delete=False) + config_file.write(config) + config_file.flush() + config_file.close() + oidc_contents, oidc_path = create_oidc_config() + manager = AuthnzManager(app=mock_app, oidc_config_file=oidc_path, oidc_backends_config_file=config_file.name) + assert isinstance(manager.oidc_backends_config["oidc"], dict) + parsed = manager.oidc_backends_config["oidc"] + # Boolean values should be False by default + assert parsed["enable_idp_logout"] is False + assert parsed.get("require_create_confirmation", False) is False + assert parsed.get("require_session_refresh", False) is False + + def test_psa_authnz_config(mock_app): """ Test config values are set correctly in PSAAuthnz @@ -301,10 +333,12 @@ def test_refresh_expiring_oidc_tokens_returns_none_after_successful_refresh(mock AuthForbidden(backend=FAKE_SOCIAL_AUTH_BACKEND), ], ) -def test_refresh_expiring_oidc_tokens_returns_provider_on_terminal_refresh_failure(mock_app, refresh_exception): +def test_refresh_expiring_oidc_tokens_returns_provider_on_required_terminal_refresh_failure( + mock_app, refresh_exception +): user = _make_user_with_social_auth() trans = _make_mock_trans_with_user(user) - manager = _make_authnz_manager(trans.app) + manager = _make_authnz_manager(trans.app, require_session_refresh="true") FakeRefreshBackend.refresh_result = False FakeRefreshBackend.refresh_exception = refresh_exception @@ -314,10 +348,23 @@ def test_refresh_expiring_oidc_tokens_returns_provider_on_terminal_refresh_failu assert reauth_provider == "oidc" +def test_refresh_expiring_oidc_tokens_returns_none_on_optional_terminal_refresh_failure(mock_app): + user = _make_user_with_social_auth() + trans = _make_mock_trans_with_user(user) + manager = _make_authnz_manager(trans.app, require_session_refresh="false") + FakeRefreshBackend.refresh_result = False + FakeRefreshBackend.refresh_exception = AuthTokenError(backend=FAKE_SOCIAL_AUTH_BACKEND) + + with patch.object(AuthnzManager, "_get_identity_provider_factory", return_value=FakeRefreshBackend): + reauth_provider = manager.refresh_expiring_oidc_tokens(cast(Any, trans)) + + assert reauth_provider is None + + def test_refresh_expiring_oidc_tokens_returns_none_on_unexpected_refresh_failure(mock_app): user = _make_user_with_social_auth() trans = _make_mock_trans_with_user(user) - manager = _make_authnz_manager(trans.app) + manager = _make_authnz_manager(trans.app, require_session_refresh="true") FakeRefreshBackend.refresh_result = False FakeRefreshBackend.refresh_exception = RuntimeError("unexpected refresh failure") @@ -329,8 +376,8 @@ def test_refresh_expiring_oidc_tokens_returns_none_on_unexpected_refresh_failure def test_redirects_to_oidc_login_on_terminal_refresh_failure() -> None: app = cast(Any, galaxy_mock.MockApp()) - app.config = CORSParsingMockConfig(oidc_require_refresh=True) - app.authnz_manager = _make_authnz_manager(app) + app.config = CORSParsingMockConfig() + app.authnz_manager = _make_authnz_manager(app, require_session_refresh="true") webapp = cast(WebApplication, galaxy_mock.MockWebapp(app.security)) environ = galaxy_mock.buildMockEnviron() AuthenticatedStubGalaxyWebTransaction.auth_user = _make_user_with_social_auth() @@ -358,8 +405,8 @@ def test_redirects_to_oidc_login_on_terminal_refresh_failure() -> None: def test_returns_401_for_api_request_on_terminal_refresh_failure() -> None: app = cast(Any, galaxy_mock.MockApp()) - app.config = CORSParsingMockConfig(oidc_require_refresh=True) - app.authnz_manager = _make_authnz_manager(app) + app.config = CORSParsingMockConfig() + app.authnz_manager = _make_authnz_manager(app, require_session_refresh="true") webapp = cast(WebApplication, galaxy_mock.MockWebapp(app.security)) environ = galaxy_mock.buildMockEnviron(PATH_INFO="/api/users/current", is_api_request=True) AuthenticatedStubGalaxyWebTransaction.auth_user = _make_user_with_social_auth() @@ -380,8 +427,8 @@ def test_allows_api_request_on_successful_oidc_refresh() -> None: Test that API requests proceed when the refresh succeeds. """ app = cast(Any, galaxy_mock.MockApp()) - app.config = CORSParsingMockConfig(oidc_require_refresh=True) - app.authnz_manager = _make_authnz_manager(app) + app.config = CORSParsingMockConfig() + app.authnz_manager = _make_authnz_manager(app, require_session_refresh="true") webapp = cast(WebApplication, galaxy_mock.MockWebapp(app.security)) environ = galaxy_mock.buildMockEnviron(PATH_INFO="/api/users/current", is_api_request=True) AuthenticatedStubGalaxyWebTransaction.auth_user = _make_user_with_social_auth() From 48f8bdc8f3b3c3a33b752cdc90242900ee1c0214 Mon Sep 17 00:00:00 2001 From: marius-mather Date: Wed, 29 Apr 2026 14:07:26 +1000 Subject: [PATCH 23/24] fix: make sure refresh logic works with unified provider name --- lib/galaxy/authnz/managers.py | 13 +++++++++---- test/unit/authnz/test_authnz.py | 13 +++++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/authnz/managers.py b/lib/galaxy/authnz/managers.py index 2578db9ebf3..35c0227ba6e 100644 --- a/lib/galaxy/authnz/managers.py +++ b/lib/galaxy/authnz/managers.py @@ -335,19 +335,24 @@ class AuthnzManager: Refresh expiring OIDC tokens for all providers associated with a user. Returns: - str | None: The provider name if refresh fails and require_refresh is enabled, otherwise None + str | None: The provider name if refresh fails and require_session_refresh is enabled, otherwise None """ user = trans.user or user if not isinstance(user, model.User): return None for auth in user.social_auth or []: result = self.refresh_expiring_oidc_tokens_for_provider(trans, auth) - config = self.oidc_backends_config.get(auth.provider, None) + if auth.provider is None: + continue + provider = self._unify_provider_name(auth.provider) + if provider is None: + continue + config = self.oidc_backends_config.get(provider, None) if config is None: continue - # Redirect to OIDC login if refresh fails and require_refresh is enabled + # Redirect to OIDC login if refresh fails and require_session_refresh is enabled if config.get("require_session_refresh") and result["reauthentication_required"]: - return auth.provider + return provider return None def authenticate( diff --git a/test/unit/authnz/test_authnz.py b/test/unit/authnz/test_authnz.py index 5e972b0596e..9d17188df0f 100644 --- a/test/unit/authnz/test_authnz.py +++ b/test/unit/authnz/test_authnz.py @@ -361,6 +361,19 @@ def test_refresh_expiring_oidc_tokens_returns_none_on_optional_terminal_refresh_ assert reauth_provider is None +def test_refresh_expiring_oidc_tokens_uses_unified_provider_for_refresh_config(mock_app): + user = _make_user_with_social_auth(provider="google-openidconnect") + trans = _make_mock_trans_with_user(user) + manager = _make_authnz_manager(trans.app, provider_name="google", require_session_refresh="true") + FakeRefreshBackend.refresh_result = False + FakeRefreshBackend.refresh_exception = AuthTokenError(backend=FAKE_SOCIAL_AUTH_BACKEND) + + with patch.object(AuthnzManager, "_get_identity_provider_factory", return_value=FakeRefreshBackend): + reauth_provider = manager.refresh_expiring_oidc_tokens(cast(Any, trans)) + + assert reauth_provider == "google" + + def test_refresh_expiring_oidc_tokens_returns_none_on_unexpected_refresh_failure(mock_app): user = _make_user_with_social_auth() trans = _make_mock_trans_with_user(user) From 0dbca9b7d8c19c5b3072139e751ea4138a731cbd Mon Sep 17 00:00:00 2001 From: marius-mather Date: Wed, 29 Apr 2026 14:11:09 +1000 Subject: [PATCH 24/24] docs: document require_session_refresh in sample OIDC config --- lib/galaxy/config/sample/oidc_backends_config.xml.sample | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/galaxy/config/sample/oidc_backends_config.xml.sample b/lib/galaxy/config/sample/oidc_backends_config.xml.sample index 0e8ea9cf557..1038b41b050 100644 --- a/lib/galaxy/config/sample/oidc_backends_config.xml.sample +++ b/lib/galaxy/config/sample/oidc_backends_config.xml.sample @@ -28,6 +28,9 @@ _______________ - require_create_confirmation: A boolean value that decides whether a NewUserConfirmation page shows up. +- require_session_refresh: A boolean value that decides whether failed token refresh requires the + user to reauthenticate with this provider. + IMPORTANT NOTES _______________