From 4a213421bb89246947a183c2ed80e828963b8647 Mon Sep 17 00:00:00 2001 From: Nuwan Goonasekera <2070605+nuwang@users.noreply.github.com> Date: Fri, 26 Jun 2026 09:35:51 +0530 Subject: [PATCH 1/3] Fix broken Google OIDC cloud-platform scope check without mutating DEFAULT_SCOPE The Google secondary-AuthZ cloud-platform scope was gated on `backend.name is BACKENDS_NAME["google"]`, an identity comparison that is always False for the non-interned "google-openidconnect" string, so the scope was in fact never requested. Correcting that check would re-expose the original accumulation defect: the append mutated the backend's shared class-level DEFAULT_SCOPE, so the scope piled up on every login. Instead, request the cloud-platform scope the same way PR #22997 handles extra_scopes -- add it to the SCOPE setting at construction time and let social-core combine it with DEFAULT_SCOPE non-destructively. authenticate() no longer touches DEFAULT_SCOPE at all. Also drop the now-unused EXTRA_SCOPES config key; its only reader was removed when extra_scopes moved to the SCOPE setting. --- lib/galaxy/authnz/psa_authnz.py | 20 +++++---- test/unit/authnz/test_psa_authnz.py | 69 +++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 9 deletions(-) diff --git a/lib/galaxy/authnz/psa_authnz.py b/lib/galaxy/authnz/psa_authnz.py index 0f4276b68dd..1aa06378eb1 100644 --- a/lib/galaxy/authnz/psa_authnz.py +++ b/lib/galaxy/authnz/psa_authnz.py @@ -200,6 +200,17 @@ class PSAAuthnz(IdentityProvider): del self.config["SOCIAL_AUTH_SECONDARY_AUTH_PROVIDER"] if "SOCIAL_AUTH_SECONDARY_AUTH_ENDPOINT" in self.config: del self.config["SOCIAL_AUTH_SECONDARY_AUTH_ENDPOINT"] + elif ( + "SOCIAL_AUTH_SECONDARY_AUTH_PROVIDER" in self.config + and "SOCIAL_AUTH_SECONDARY_AUTH_ENDPOINT" in self.config + ): + # Google secondary AuthZ needs the cloud-platform scope. Request it + # via the SCOPE setting (which social-core combines with the backend's + # DEFAULT_SCOPE) instead of mutating the shared class-level + # DEFAULT_SCOPE, which would accumulate the scope across logins. + scope = list(self.config.get(setting_name("SCOPE")) or []) + scope.append("https://www.googleapis.com/auth/cloud-platform") + self.config[setting_name("SCOPE")] = scope def _is_oidc_backend(self) -> bool: """ @@ -223,7 +234,6 @@ class PSAAuthnz(IdentityProvider): self.config["SECRET"] = oidc_backend_config.get("client_secret") self.config["TENANT_ID"] = oidc_backend_config.get("tenant_id") # Azure/Tapis self.config["redirect_uri"] = oidc_backend_config.get("redirect_uri") - self.config["EXTRA_SCOPES"] = oidc_backend_config.get("extra_scopes") self.config["LABEL"] = oidc_backend_config.get("label", self.config["provider"].capitalize()) # Galaxy-specific pipeline settings (affect all backends) @@ -310,14 +320,6 @@ class PSAAuthnz(IdentityProvider): on_the_fly_config(trans.sa_session) strategy = Strategy(trans.request, trans.session, Storage, self.config) backend = self._load_backend(strategy, self.config["redirect_uri"]) - backend.DEFAULT_SCOPE = backend.DEFAULT_SCOPE or [] - if ( - backend.name is BACKENDS_NAME["google"] - and "SOCIAL_AUTH_SECONDARY_AUTH_PROVIDER" in self.config - and "SOCIAL_AUTH_SECONDARY_AUTH_ENDPOINT" in self.config - ): - backend.DEFAULT_SCOPE.append("https://www.googleapis.com/auth/cloud-platform") - return do_auth(backend) def callback(self, state_token, authz_code, trans, login_redirect_url): diff --git a/test/unit/authnz/test_psa_authnz.py b/test/unit/authnz/test_psa_authnz.py index dbc41917783..935f429c07f 100644 --- a/test/unit/authnz/test_psa_authnz.py +++ b/test/unit/authnz/test_psa_authnz.py @@ -667,3 +667,72 @@ def test_authenticate_with_real_backend_does_not_accumulate_extra_scopes(psa_aut assert list(backend_class.DEFAULT_SCOPE or []) == expected_default_scope finally: backend_class.DEFAULT_SCOPE = original_default_scope + + +def make_google_secondary_auth_psa_authnz(extra_scopes=None): + """Build a PSAAuthnz configured for the Google provider with secondary AuthZ.""" + oidc_backend_config = { + "client_id": "gxyclient", + "client_secret": "dummyclientsecret", + "redirect_uri": "https://galaxy.example.com/authnz/callback", + } + if extra_scopes is not None: + oidc_backend_config["extra_scopes"] = extra_scopes + return PSAAuthnz( + provider="google", + oidc_config={ + "SECONDARY_AUTH_PROVIDER": "secondary-provider", + "SECONDARY_AUTH_ENDPOINT": "https://secondary.example.com/auth", + }, + oidc_backend_config=oidc_backend_config, + app_config=SimpleNamespace( + oidc_auth_pipeline=None, + oidc_auth_pipeline_extra=None, + fixed_delegated_auth=False, + ), + ) + + +def test_google_secondary_auth_requests_cloud_platform_scope_without_mutating_default(): + """ + When Google secondary AuthZ is configured, the cloud-platform scope is + requested via the SCOPE setting (combined with DEFAULT_SCOPE by social-core), + so it is sent exactly once per login and the shared class-level DEFAULT_SCOPE + is never mutated. + """ + cloud_platform_scope = "https://www.googleapis.com/auth/cloud-platform" + backend_class = module_member(BACKENDS["google"]) + original_default_scope = backend_class.DEFAULT_SCOPE + expected_default_scope = list(original_default_scope or []) + observed_scopes = [] + + psa_authnz = make_google_secondary_auth_psa_authnz(extra_scopes=["offline_access"]) + + def fake_load_backend(strategy, redirect_uri): + # Real _load_backend builds a fresh backend instance on every call. + return backend_class(strategy, redirect_uri) + + def fake_do_auth(backend): + observed_scopes.append(backend.get_scope()) + return MagicMock() + + try: + with ( + patch("galaxy.authnz.psa_authnz.on_the_fly_config"), + patch.object(psa_authnz, "_load_backend", side_effect=fake_load_backend), + patch("galaxy.authnz.psa_authnz.do_auth", side_effect=fake_do_auth), + ): + psa_authnz.authenticate(make_mock_trans()) + psa_authnz.authenticate(make_mock_trans()) + + for scope in observed_scopes: + # cloud-platform requested exactly once, alongside the configured extra scope + assert scope.count(cloud_platform_scope) == 1 + assert "offline_access" in scope + # identical across logins (no accumulation)... + assert observed_scopes[0] == observed_scopes[1] + # ...and the shared class-level default scope is left untouched. + assert backend_class.DEFAULT_SCOPE is original_default_scope + assert list(backend_class.DEFAULT_SCOPE or []) == expected_default_scope + finally: + backend_class.DEFAULT_SCOPE = original_default_scope From ab32e80688b69f83d7a70fbe6ffbcb812e98383b Mon Sep 17 00:00:00 2001 From: Nuwan Goonasekera <2070605+nuwang@users.noreply.github.com> Date: Fri, 26 Jun 2026 11:12:16 +0530 Subject: [PATCH 2/3] Fix mypy arg-type error in Google secondary-auth test helper PSAAuthnz expects app_config: GalaxyAppConfiguration, so passing a SimpleNamespace directly fails mypy. Assign it onto a MagicMock attribute (typed Any) instead, matching the existing make_psa_authnz helper. --- test/unit/authnz/test_psa_authnz.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/test/unit/authnz/test_psa_authnz.py b/test/unit/authnz/test_psa_authnz.py index 935f429c07f..9d1daa19c6a 100644 --- a/test/unit/authnz/test_psa_authnz.py +++ b/test/unit/authnz/test_psa_authnz.py @@ -678,6 +678,12 @@ def make_google_secondary_auth_psa_authnz(extra_scopes=None): } if extra_scopes is not None: oidc_backend_config["extra_scopes"] = extra_scopes + mock_app = MagicMock() + mock_app.config = SimpleNamespace( + oidc_auth_pipeline=None, + oidc_auth_pipeline_extra=None, + fixed_delegated_auth=False, + ) return PSAAuthnz( provider="google", oidc_config={ @@ -685,11 +691,7 @@ def make_google_secondary_auth_psa_authnz(extra_scopes=None): "SECONDARY_AUTH_ENDPOINT": "https://secondary.example.com/auth", }, oidc_backend_config=oidc_backend_config, - app_config=SimpleNamespace( - oidc_auth_pipeline=None, - oidc_auth_pipeline_extra=None, - fixed_delegated_auth=False, - ), + app_config=mock_app.config, ) From 649ec73c86bedd4eb5d1c5506dbe4f4067c52895 Mon Sep 17 00:00:00 2001 From: Nuwan Goonasekera <2070605+nuwang@users.noreply.github.com> Date: Fri, 26 Jun 2026 19:03:24 +0530 Subject: [PATCH 3/3] Address review comments: remove unit test --- test/unit/authnz/test_psa_authnz.py | 71 ----------------------------- 1 file changed, 71 deletions(-) diff --git a/test/unit/authnz/test_psa_authnz.py b/test/unit/authnz/test_psa_authnz.py index 9d1daa19c6a..dbc41917783 100644 --- a/test/unit/authnz/test_psa_authnz.py +++ b/test/unit/authnz/test_psa_authnz.py @@ -667,74 +667,3 @@ def test_authenticate_with_real_backend_does_not_accumulate_extra_scopes(psa_aut assert list(backend_class.DEFAULT_SCOPE or []) == expected_default_scope finally: backend_class.DEFAULT_SCOPE = original_default_scope - - -def make_google_secondary_auth_psa_authnz(extra_scopes=None): - """Build a PSAAuthnz configured for the Google provider with secondary AuthZ.""" - oidc_backend_config = { - "client_id": "gxyclient", - "client_secret": "dummyclientsecret", - "redirect_uri": "https://galaxy.example.com/authnz/callback", - } - if extra_scopes is not None: - oidc_backend_config["extra_scopes"] = extra_scopes - mock_app = MagicMock() - mock_app.config = SimpleNamespace( - oidc_auth_pipeline=None, - oidc_auth_pipeline_extra=None, - fixed_delegated_auth=False, - ) - return PSAAuthnz( - provider="google", - oidc_config={ - "SECONDARY_AUTH_PROVIDER": "secondary-provider", - "SECONDARY_AUTH_ENDPOINT": "https://secondary.example.com/auth", - }, - oidc_backend_config=oidc_backend_config, - app_config=mock_app.config, - ) - - -def test_google_secondary_auth_requests_cloud_platform_scope_without_mutating_default(): - """ - When Google secondary AuthZ is configured, the cloud-platform scope is - requested via the SCOPE setting (combined with DEFAULT_SCOPE by social-core), - so it is sent exactly once per login and the shared class-level DEFAULT_SCOPE - is never mutated. - """ - cloud_platform_scope = "https://www.googleapis.com/auth/cloud-platform" - backend_class = module_member(BACKENDS["google"]) - original_default_scope = backend_class.DEFAULT_SCOPE - expected_default_scope = list(original_default_scope or []) - observed_scopes = [] - - psa_authnz = make_google_secondary_auth_psa_authnz(extra_scopes=["offline_access"]) - - def fake_load_backend(strategy, redirect_uri): - # Real _load_backend builds a fresh backend instance on every call. - return backend_class(strategy, redirect_uri) - - def fake_do_auth(backend): - observed_scopes.append(backend.get_scope()) - return MagicMock() - - try: - with ( - patch("galaxy.authnz.psa_authnz.on_the_fly_config"), - patch.object(psa_authnz, "_load_backend", side_effect=fake_load_backend), - patch("galaxy.authnz.psa_authnz.do_auth", side_effect=fake_do_auth), - ): - psa_authnz.authenticate(make_mock_trans()) - psa_authnz.authenticate(make_mock_trans()) - - for scope in observed_scopes: - # cloud-platform requested exactly once, alongside the configured extra scope - assert scope.count(cloud_platform_scope) == 1 - assert "offline_access" in scope - # identical across logins (no accumulation)... - assert observed_scopes[0] == observed_scopes[1] - # ...and the shared class-level default scope is left untouched. - assert backend_class.DEFAULT_SCOPE is original_default_scope - assert list(backend_class.DEFAULT_SCOPE or []) == expected_default_scope - finally: - backend_class.DEFAULT_SCOPE = original_default_scope