From 65015b81f11f4986fab5c38ce97d6cccf890b0e3 Mon Sep 17 00:00:00 2001 From: Uwe Winter Date: Fri, 14 Nov 2025 13:47:36 +1100 Subject: [PATCH 1/4] update error message handling for OIDC --- client/src/components/Login/LoginForm.test.ts | 18 +++++++++++++++++- client/src/components/Login/LoginForm.vue | 17 ++++++++++------- .../webapps/galaxy/controllers/authnz.py | 17 ++++++++++++----- 3 files changed, 39 insertions(+), 13 deletions(-) diff --git a/client/src/components/Login/LoginForm.test.ts b/client/src/components/Login/LoginForm.test.ts index 138804300a62..3d98613b7043 100644 --- a/client/src/components/Login/LoginForm.test.ts +++ b/client/src/components/Login/LoginForm.test.ts @@ -110,7 +110,7 @@ describe("LoginForm", () => { const provider_label = "Provider"; const originalLocation = window.location; - jest.spyOn(window, "location", "get").mockImplementation(() => ({ + const locationSpy = jest.spyOn(window, "location", "get").mockImplementation(() => ({ ...originalLocation, search: `?connect_external_email=${external_email}&connect_external_provider=${provider_id}&connect_external_label=${provider_label}`, })); @@ -150,5 +150,21 @@ describe("LoginForm", () => { const postedURL = axiosMock.history.post?.[0]?.url; expect(postedURL).toBe("/user/login"); await flushPromises(); + + locationSpy.mockRestore(); + }); + + it("renders message from query params", async () => { + const originalUrl = window.location.href; + window.history.replaceState(null, "", "/login/start?message=auth-error&status=info"); + + const wrapper = await mountLoginForm(); + + const alert = wrapper.find(".alert"); + expect(alert.exists()).toBe(true); + expect(alert.text()).toContain("auth-error"); + expect(alert.classes()).toContain("alert-info"); + + window.history.replaceState(null, "", originalUrl); }); }); diff --git a/client/src/components/Login/LoginForm.vue b/client/src/components/Login/LoginForm.vue index a07c13afc37f..c188d320efa9 100644 --- a/client/src/components/Login/LoginForm.vue +++ b/client/src/components/Login/LoginForm.vue @@ -49,18 +49,21 @@ const props = withDefaults(defineProps(), { const router = useRouter(); -const urlParams = new URLSearchParams(window.location.search); +const getUrlParams = () => new URLSearchParams(window.location.search); +const params = getUrlParams(); const login = ref(""); const password = ref(null); const passwordState = ref(null); const loading = ref(false); -const messageText = ref(""); -const messageVariant = ref<"info" | "danger">("info"); -const connectExternalEmail = ref(urlParams.get("connect_external_email")); -const connectExternalLabel = ref(urlParams.get("connect_external_label")); -const connectExternalProvider = ref(urlParams.get("connect_external_provider")); -const confirmURL = ref(urlParams.has("confirm") && urlParams.get("confirm") == "true"); +const networkMessage = params.get("message") || ""; +const messageText = ref(networkMessage); +const statusParam = params.get("status"); +const messageVariant = ref<"info" | "danger">(statusParam === "info" ? "info" : "danger"); +const connectExternalEmail = ref(params.get("connect_external_email")); +const connectExternalLabel = ref(params.get("connect_external_label")); +const connectExternalProvider = ref(params.get("connect_external_provider")); +const confirmURL = ref(params.has("confirm") && params.get("confirm") == "true"); const excludeIdps = computed(() => (connectExternalProvider.value ? [connectExternalProvider.value] : undefined)); diff --git a/lib/galaxy/webapps/galaxy/controllers/authnz.py b/lib/galaxy/webapps/galaxy/controllers/authnz.py index f2e8b00af3d5..f696f04576de 100644 --- a/lib/galaxy/webapps/galaxy/controllers/authnz.py +++ b/lib/galaxy/webapps/galaxy/controllers/authnz.py @@ -5,6 +5,7 @@ import datetime import json import logging +from urllib.parse import quote import jwt @@ -116,11 +117,17 @@ def callback(self, trans, provider, idphint=None, **kwargs): "Error handling authentication callback from `{}` identity provider for user `{}` login request." " Error message: {}".format(provider, user, kwargs.get("error", "None")) ) - return trans.show_error_message( - f"Failed to handle authentication callback from {provider}. " - "Please try again, and if the problem persists, contact " - "the Galaxy instance admin" - ) + error_description = kwargs.get("error_description") + if error_description: + error_msg = error_description + else: + error_msg = ( + f"Failed to handle authentication callback from {provider}. " + "Please try again, and if the problem persists, contact " + "the Galaxy instance admin." + ) + redirect_to = f"{trans.request.url_path + url_for('/')}login/start?message={quote(error_msg, safe='')}&status=danger" + return trans.response.send_redirect(redirect_to) try: success, message, (redirect_url, user) = trans.app.authnz_manager.callback( provider, From 198b7fbaa03b9f254af33ddba83b9cac3f6c6bfb Mon Sep 17 00:00:00 2001 From: Uwe Winter Date: Tue, 18 Nov 2025 09:44:01 +1100 Subject: [PATCH 2/4] Apply suggestion from @nuwang Co-authored-by: Nuwan Goonasekera <2070605+nuwang@users.noreply.github.com> --- lib/galaxy/webapps/galaxy/controllers/authnz.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/webapps/galaxy/controllers/authnz.py b/lib/galaxy/webapps/galaxy/controllers/authnz.py index f696f04576de..ff233a724a33 100644 --- a/lib/galaxy/webapps/galaxy/controllers/authnz.py +++ b/lib/galaxy/webapps/galaxy/controllers/authnz.py @@ -126,7 +126,7 @@ def callback(self, trans, provider, idphint=None, **kwargs): "Please try again, and if the problem persists, contact " "the Galaxy instance admin." ) - redirect_to = f"{trans.request.url_path + url_for('/')}login/start?message={quote(error_msg, safe='')}&status=danger" + redirect_to = trans.url_builder("/login/start", message=error_msg, status="danger") return trans.response.send_redirect(redirect_to) try: success, message, (redirect_url, user) = trans.app.authnz_manager.callback( From 8f260cc14031f073522081252b6fe8866c93ef27 Mon Sep 17 00:00:00 2001 From: Uwe Winter Date: Tue, 18 Nov 2025 10:19:53 +1100 Subject: [PATCH 3/4] revert changes to urlParams --- client/src/components/Login/LoginForm.vue | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/client/src/components/Login/LoginForm.vue b/client/src/components/Login/LoginForm.vue index c188d320efa9..617d17b9f6ba 100644 --- a/client/src/components/Login/LoginForm.vue +++ b/client/src/components/Login/LoginForm.vue @@ -49,21 +49,20 @@ const props = withDefaults(defineProps(), { const router = useRouter(); -const getUrlParams = () => new URLSearchParams(window.location.search); -const params = getUrlParams(); +const urlParams = new URLSearchParams(window.location.search); const login = ref(""); const password = ref(null); const passwordState = ref(null); const loading = ref(false); -const networkMessage = params.get("message") || ""; +const networkMessage = urlParams.get("message") || ""; const messageText = ref(networkMessage); -const statusParam = params.get("status"); +const statusParam = urlParams.get("status"); const messageVariant = ref<"info" | "danger">(statusParam === "info" ? "info" : "danger"); -const connectExternalEmail = ref(params.get("connect_external_email")); -const connectExternalLabel = ref(params.get("connect_external_label")); -const connectExternalProvider = ref(params.get("connect_external_provider")); -const confirmURL = ref(params.has("confirm") && params.get("confirm") == "true"); +const connectExternalEmail = ref(urlParams.get("connect_external_email")); +const connectExternalLabel = ref(urlParams.get("connect_external_label")); +const connectExternalProvider = ref(urlParams.get("connect_external_provider")); +const confirmURL = ref(urlParams.has("confirm") && urlParams.get("confirm") == "true"); const excludeIdps = computed(() => (connectExternalProvider.value ? [connectExternalProvider.value] : undefined)); From 3477af1d44e63f6be557eef5bf2a055ae1571d94 Mon Sep 17 00:00:00 2001 From: Nuwan Goonasekera <2070605+nuwang@users.noreply.github.com> Date: Tue, 18 Nov 2025 10:04:00 +0530 Subject: [PATCH 4/4] Fix linting error by removing unused import lint whitespace --- lib/galaxy/webapps/galaxy/controllers/authnz.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/galaxy/webapps/galaxy/controllers/authnz.py b/lib/galaxy/webapps/galaxy/controllers/authnz.py index ff233a724a33..bcd82cb7d996 100644 --- a/lib/galaxy/webapps/galaxy/controllers/authnz.py +++ b/lib/galaxy/webapps/galaxy/controllers/authnz.py @@ -5,7 +5,6 @@ import datetime import json import logging -from urllib.parse import quote import jwt @@ -126,7 +125,7 @@ def callback(self, trans, provider, idphint=None, **kwargs): "Please try again, and if the problem persists, contact " "the Galaxy instance admin." ) - redirect_to = trans.url_builder("/login/start", message=error_msg, status="danger") + redirect_to = trans.url_builder("/login/start", message=error_msg, status="danger") return trans.response.send_redirect(redirect_to) try: success, message, (redirect_url, user) = trans.app.authnz_manager.callback(