From cbfdbab7730cb471001dbf91d683543ee3d99dea Mon Sep 17 00:00:00 2001 From: williambza Date: Thu, 23 Jul 2026 12:58:32 +0200 Subject: [PATCH 1/3] Allow for missing offline_access scope --- src/Frontend/src/stores/AuthStore.spec.ts | 44 +++++++++++++++++++++++ src/Frontend/src/stores/AuthStore.ts | 10 +++++- 2 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 src/Frontend/src/stores/AuthStore.spec.ts diff --git a/src/Frontend/src/stores/AuthStore.spec.ts b/src/Frontend/src/stores/AuthStore.spec.ts new file mode 100644 index 000000000..d331a8fda --- /dev/null +++ b/src/Frontend/src/stores/AuthStore.spec.ts @@ -0,0 +1,44 @@ +import { beforeEach, describe, expect, test, vi } from "vitest"; +import { createPinia, setActivePinia } from "pinia"; +import serviceControlClient from "@/components/serviceControlClient"; +import { useAuthStore } from "@/stores/AuthStore"; + +describe("AuthStore tests", () => { + const baseResponse = { + enabled: true, + role_based_authorization_enabled: true, + client_id: "test-client-id", + authority: "https://login.example.com", + api_scopes: JSON.stringify(["api://test-audience/.default"]), + audience: "api://test-audience", + }; + + beforeEach(() => { + setActivePinia(createPinia()); + }); + + test("uses the composed scopes field from ServiceControl when present", async () => { + vi.spyOn(serviceControlClient, "fetchTypedFromServiceControl").mockResolvedValue([ + {} as Response, + { + ...baseResponse, + scopes: "api://test-audience/.default openid profile email", + }, + ]); + + const store = useAuthStore(); + await store.refresh(); + + expect(store.authConfig?.scope).toBe("api://test-audience/.default openid profile email"); + }); + + test("falls back to assembling scopes from api_scopes when talking to an older ServiceControl", async () => { + // Older ServiceControl versions don't send the `scopes` field at all. + vi.spyOn(serviceControlClient, "fetchTypedFromServiceControl").mockResolvedValue([{} as Response, { ...baseResponse }]); + + const store = useAuthStore(); + await store.refresh(); + + expect(store.authConfig?.scope).toBe("api://test-audience/.default openid profile email offline_access"); + }); +}); diff --git a/src/Frontend/src/stores/AuthStore.ts b/src/Frontend/src/stores/AuthStore.ts index 64c401213..d759047d3 100644 --- a/src/Frontend/src/stores/AuthStore.ts +++ b/src/Frontend/src/stores/AuthStore.ts @@ -14,6 +14,10 @@ interface AuthConfigResponse { authority: string; api_scopes: string; audience: string; + // Complete scope string composed by ServiceControl (api scopes + openid profile email + offline_access, + // the last omitted if the operator disabled it). Absent on ServiceControl versions older than the one + // that introduced this field, in which case we fall back to assembling it ourselves below. + scopes?: string; } export const useAuthStore = defineStore("auth", () => { @@ -52,7 +56,11 @@ export const useAuthStore = defineStore("auth", () => { } function transformToAuthConfig(config: AuthConfigResponse): AuthConfig { + // ServiceControl composes the full scope string (including whether offline_access is permitted + // by the identity provider). Older ServiceControl versions don't send it, so fall back to the + // previous behaviour of assembling it from api_scopes. const apiScope = JSON.parse(config.api_scopes).join(" "); + const scope = config.scopes ?? `${apiScope} openid profile email offline_access`; // Use hash-based URL for post-logout redirect since the app uses hash routing const postLogoutRedirectUri = `${window.location.origin}${window.location.pathname}#${routeLinks.loggedOut}`; return { @@ -61,7 +69,7 @@ export const useAuthStore = defineStore("auth", () => { redirect_uri: window.location.origin, post_logout_redirect_uri: postLogoutRedirectUri, response_type: "code", - scope: `${apiScope} openid profile email offline_access`, + scope, automaticSilentRenew: true, loadUserInfo: false, includeIdTokenInSilentRenew: true, From 5077557b0aa42fcc6f19a74f6168689faae98ff5 Mon Sep 17 00:00:00 2001 From: Warwick Schroeder Date: Fri, 24 Jul 2026 16:39:02 +0800 Subject: [PATCH 2/3] Add AuthError handling and display component for authentication failures --- src/Frontend/src/AuthApp.vue | 4 +- .../src/components/AuthErrorScreen.spec.ts | 26 ++++++++ .../src/components/AuthErrorScreen.vue | 62 +++++++++++++++++++ .../src/composables/authError.spec.ts | 23 +++++++ src/Frontend/src/composables/authError.ts | 27 ++++++++ src/Frontend/src/composables/useAuth.ts | 9 +-- src/Frontend/src/stores/AuthStore.spec.ts | 7 ++- src/Frontend/src/stores/AuthStore.ts | 6 +- src/Frontend/src/types/auth.ts | 12 ++++ .../auth-callback-error.spec.ts | 45 +++++++++++++- 10 files changed, 208 insertions(+), 13 deletions(-) create mode 100644 src/Frontend/src/components/AuthErrorScreen.spec.ts create mode 100644 src/Frontend/src/components/AuthErrorScreen.vue create mode 100644 src/Frontend/src/composables/authError.spec.ts create mode 100644 src/Frontend/src/composables/authError.ts diff --git a/src/Frontend/src/AuthApp.vue b/src/Frontend/src/AuthApp.vue index 558e74faa..af9dba542 100644 --- a/src/Frontend/src/AuthApp.vue +++ b/src/Frontend/src/AuthApp.vue @@ -6,11 +6,12 @@ import { useAuthStore } from "@/stores/AuthStore"; import routeLinks from "@/router/routeLinks"; import LoadingSpinner from "@/components/LoadingSpinner.vue"; import App from "./App.vue"; +import AuthErrorScreen from "@/components/AuthErrorScreen.vue"; import logger from "@/logger"; const { authenticate } = useAuth(); const authStore = useAuthStore(); -const { isAuthenticating, loading } = storeToRefs(authStore); +const { isAuthenticating, loading, authError } = storeToRefs(authStore); onMounted(async () => { try { @@ -56,6 +57,7 @@ onMounted(async () => {
Authenticating...
+ diff --git a/src/Frontend/src/components/AuthErrorScreen.spec.ts b/src/Frontend/src/components/AuthErrorScreen.spec.ts new file mode 100644 index 000000000..77a0714f2 --- /dev/null +++ b/src/Frontend/src/components/AuthErrorScreen.spec.ts @@ -0,0 +1,26 @@ +import { expect, test, describe, render, screen } from "@component-test-utils"; + +import AuthErrorScreen from "./AuthErrorScreen.vue"; + +describe("AuthErrorScreen", () => { + test("shows scope-specific guidance and the raw detail for invalid_scope", async () => { + render(AuthErrorScreen, { + props: { + error: { code: "invalid_scope", description: "Invalid scopes: Pulse openid profile email offline_access" }, + }, + }); + + expect(await screen.findByText("Unable to sign in")).toBeVisible(); + expect(screen.getByText(/offline_access scope may be disabled/)).toBeVisible(); + expect(screen.getByText(/Invalid scopes: Pulse openid profile email offline_access/)).toBeVisible(); + }); + + test("shows a generic message and the raw detail for an error without a recognized code", async () => { + render(AuthErrorScreen, { + props: { error: { description: "Callback failed" } }, + }); + + expect(await screen.findByText(/Contact your administrator/)).toBeVisible(); + expect(screen.getByText(/Callback failed/)).toBeVisible(); + }); +}); diff --git a/src/Frontend/src/components/AuthErrorScreen.vue b/src/Frontend/src/components/AuthErrorScreen.vue new file mode 100644 index 000000000..b0ee4ef5f --- /dev/null +++ b/src/Frontend/src/components/AuthErrorScreen.vue @@ -0,0 +1,62 @@ + + + + + diff --git a/src/Frontend/src/composables/authError.spec.ts b/src/Frontend/src/composables/authError.spec.ts new file mode 100644 index 000000000..cca122c64 --- /dev/null +++ b/src/Frontend/src/composables/authError.spec.ts @@ -0,0 +1,23 @@ +import { describe, test, expect } from "vitest"; +import { describeAuthError } from "@/composables/authError"; + +describe("describeAuthError", () => { + test("gives scope-specific guidance for invalid_scope", () => { + const result = describeAuthError({ + code: "invalid_scope", + description: "Invalid scopes: Pulse openid profile email offline_access", + }); + expect(result.title).toBe("Unable to sign in"); + expect(result.message).toContain("offline_access"); + }); + + test("gives a generic message for an unrecognized code", () => { + const result = describeAuthError({ code: "server_error", description: "server_error" }); + expect(result.message).toContain("Contact your administrator"); + }); + + test("gives a generic message when there is no code", () => { + const result = describeAuthError({ description: "Callback failed" }); + expect(result.message).toContain("Contact your administrator"); + }); +}); diff --git a/src/Frontend/src/composables/authError.ts b/src/Frontend/src/composables/authError.ts new file mode 100644 index 000000000..89ee75bba --- /dev/null +++ b/src/Frontend/src/composables/authError.ts @@ -0,0 +1,27 @@ +import type { AuthError } from "@/types/auth"; + +export interface AuthErrorDisplay { + title: string; + message: string; +} + +/** + * Maps a captured authentication failure to user-facing copy. Recognised OAuth error codes get + * specific, actionable guidance; everything else — including local/callback exceptions with no + * code — gets a generic message. The raw `error.description` is rendered separately by the + * component, so it is not repeated here. + */ +export function describeAuthError(error: AuthError): AuthErrorDisplay { + switch (error.code) { + case "invalid_scope": + return { + title: "Unable to sign in", + message: "Your identity provider rejected one or more of the requested scopes. The 'offline_access' scope may be disabled in your IdP, ask your administrator to enable it or update ServiceControl so ServicePulse does not request it.", + }; + default: + return { + title: "Unable to sign in", + message: "Something went wrong while signing you in. Contact your administrator if the problem continues.", + }; + } +} diff --git a/src/Frontend/src/composables/useAuth.ts b/src/Frontend/src/composables/useAuth.ts index 8bc2694aa..999fd62cc 100644 --- a/src/Frontend/src/composables/useAuth.ts +++ b/src/Frontend/src/composables/useAuth.ts @@ -29,7 +29,7 @@ export function useAuth() { await userManager?.signinRedirect(); } catch (error) { logger.error("Re-authentication after session loss failed:", error); - authStore.setAuthError(error instanceof Error ? error.message : "Re-authentication after session loss failed"); + authStore.setAuthError({ description: error instanceof Error ? error.message : "Re-authentication after session loss failed" }); } finally { authStore.setAuthenticating(false); } @@ -116,7 +116,7 @@ export function useAuth() { errorMessage: error instanceof Error ? error.message : "Unknown error", errorStack: error instanceof Error ? error.stack : undefined, }); - authStore.setAuthError(error instanceof Error ? error.message : "Callback failed"); + authStore.setAuthError({ description: error instanceof Error ? error.message : "Callback failed" }); // Don't continue - callback failed, user needs to try again return false; } finally { @@ -124,9 +124,10 @@ export function useAuth() { } } else if (hasError) { // OAuth error in callback + const errorCode = params.get("error") ?? undefined; const errorDescription = params.get("error_description") || params.get("error"); logger.error("OAuth error:", errorDescription); - authStore.setAuthError(errorDescription || "Authentication failed"); + authStore.setAuthError({ code: errorCode, description: errorDescription || "Authentication failed" }); return false; } @@ -144,7 +145,7 @@ export function useAuth() { } catch (error) { authStore.setAuthenticating(false); const errorMessage = error instanceof Error ? error.message : "Unknown authentication error"; - authStore.setAuthError(errorMessage); + authStore.setAuthError({ description: errorMessage }); logger.error("Authentication error:", error); throw error; } diff --git a/src/Frontend/src/stores/AuthStore.spec.ts b/src/Frontend/src/stores/AuthStore.spec.ts index d331a8fda..fd140bddc 100644 --- a/src/Frontend/src/stores/AuthStore.spec.ts +++ b/src/Frontend/src/stores/AuthStore.spec.ts @@ -18,18 +18,21 @@ describe("AuthStore tests", () => { }); test("uses the composed scopes field from ServiceControl when present", async () => { + // The store must pass `scopes` through untouched. This value deliberately differs from anything + // derivable from api_scopes (a different scope, and no offline_access) so the test fails if the + // store ever reverts to assembling the scope string itself instead of trusting ServiceControl. vi.spyOn(serviceControlClient, "fetchTypedFromServiceControl").mockResolvedValue([ {} as Response, { ...baseResponse, - scopes: "api://test-audience/.default openid profile email", + scopes: "api://servicecontrol/composed-by-servicecontrol openid profile email", }, ]); const store = useAuthStore(); await store.refresh(); - expect(store.authConfig?.scope).toBe("api://test-audience/.default openid profile email"); + expect(store.authConfig?.scope).toBe("api://servicecontrol/composed-by-servicecontrol openid profile email"); }); test("falls back to assembling scopes from api_scopes when talking to an older ServiceControl", async () => { diff --git a/src/Frontend/src/stores/AuthStore.ts b/src/Frontend/src/stores/AuthStore.ts index d759047d3..1a8141849 100644 --- a/src/Frontend/src/stores/AuthStore.ts +++ b/src/Frontend/src/stores/AuthStore.ts @@ -1,7 +1,7 @@ import { acceptHMRUpdate, defineStore } from "pinia"; import logger from "@/logger"; import { ref } from "vue"; -import type { AuthConfig } from "@/types/auth"; +import type { AuthConfig, AuthError } from "@/types/auth"; import { WebStorageStateStore } from "oidc-client-ts"; import routeLinks from "@/router/routeLinks"; import serviceControlClient from "@/components/serviceControlClient"; @@ -24,7 +24,7 @@ export const useAuthStore = defineStore("auth", () => { const token = ref(null); const isAuthenticated = ref(false); const isAuthenticating = ref(false); - const authError = ref(null); + const authError = ref(null); const authConfig = ref(null); const authEnabled = ref(false); // undefined means ServiceControl didn't report this field (older version) — treat as enabled. @@ -110,7 +110,7 @@ export const useAuthStore = defineStore("auth", () => { isAuthenticating.value = value; } - function setAuthError(error: string | null) { + function setAuthError(error: AuthError | null) { authError.value = error; } diff --git a/src/Frontend/src/types/auth.ts b/src/Frontend/src/types/auth.ts index cdcafba80..9e1bc96a3 100644 --- a/src/Frontend/src/types/auth.ts +++ b/src/Frontend/src/types/auth.ts @@ -5,3 +5,15 @@ import type { UserManagerSettings } from "oidc-client-ts"; * This provides type-safe configuration for any OIDC-compliant identity provider */ export type AuthConfig = UserManagerSettings; + +/** + * A captured authentication failure. + * `code` is the OAuth error code from an identity-provider error redirect (e.g. "invalid_scope"); + * it is absent for local/callback exceptions that carry no OAuth code. + * `description` is the human-readable detail (the IdP's error_description or an exception message) + * and is always shown to the user. + */ +export interface AuthError { + code?: string; + description: string; +} diff --git a/src/Frontend/test/specs/authentication/auth-callback-error.spec.ts b/src/Frontend/test/specs/authentication/auth-callback-error.spec.ts index 218a4e1d4..a26adca73 100644 --- a/src/Frontend/test/specs/authentication/auth-callback-error.spec.ts +++ b/src/Frontend/test/specs/authentication/auth-callback-error.spec.ts @@ -52,7 +52,7 @@ describe("FEATURE: OAuth Callback Error Handling (Scenario 16)", () => { }); // Verify the error message contains the description - expect(authStore.authError).toContain("cancelled"); + expect(authStore.authError?.description).toContain("cancelled"); // User should not be authenticated expect(authStore.isAuthenticated).toBe(false); @@ -94,7 +94,7 @@ describe("FEATURE: OAuth Callback Error Handling (Scenario 16)", () => { }); // Verify the error captures the description - expect(authStore.authError).toContain("Missing"); + expect(authStore.authError?.description).toContain("Missing"); // User should not be authenticated expect(authStore.isAuthenticated).toBe(false); @@ -136,7 +136,7 @@ describe("FEATURE: OAuth Callback Error Handling (Scenario 16)", () => { }); // When no description, the error code should be used - expect(authStore.authError).toBe("server_error"); + expect(authStore.authError?.description).toBe("server_error"); // User should not be authenticated expect(authStore.isAuthenticated).toBe(false); @@ -148,5 +148,44 @@ describe("FEATURE: OAuth Callback Error Handling (Scenario 16)", () => { configurable: true, }); }); + + test("EXAMPLE: invalid_scope error captures the OAuth error code", async ({ driver }) => { + const mockSearch = "?error=invalid_scope&error_description=Invalid%20scopes%3A%20Pulse%20openid%20profile%20email%20offline_access"; + + const mockLocation = { + ...originalLocation, + search: mockSearch, + hash: "#/dashboard", + href: `http://localhost:5173${mockSearch}#/dashboard`, + }; + + Object.defineProperty(window, "location", { + value: mockLocation, + writable: true, + configurable: true, + }); + + await driver.setUp(precondition.serviceControlWithMonitoring); + await driver.setUp(precondition.hasAuthenticationEnabled()); + + await driver.goTo("/dashboard"); + + const authStore = useAuthStore(); + + await waitFor(() => { + expect(authStore.authError).toBeTruthy(); + }); + + expect(authStore.authError?.code).toBe("invalid_scope"); + expect(authStore.authError?.description).toContain("Invalid scopes"); + + expect(authStore.isAuthenticated).toBe(false); + + Object.defineProperty(window, "location", { + value: originalLocation, + writable: true, + configurable: true, + }); + }); }); }); From 020041299c301b8c2e3bb9cb6d4dfe0ce9ab7eb0 Mon Sep 17 00:00:00 2001 From: Warwick Schroeder Date: Fri, 24 Jul 2026 16:49:17 +0800 Subject: [PATCH 3/3] Fix typo in AuthErrorScreen test for offline_access scope message --- src/Frontend/src/components/AuthErrorScreen.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Frontend/src/components/AuthErrorScreen.spec.ts b/src/Frontend/src/components/AuthErrorScreen.spec.ts index 77a0714f2..3bb4bef77 100644 --- a/src/Frontend/src/components/AuthErrorScreen.spec.ts +++ b/src/Frontend/src/components/AuthErrorScreen.spec.ts @@ -11,7 +11,7 @@ describe("AuthErrorScreen", () => { }); expect(await screen.findByText("Unable to sign in")).toBeVisible(); - expect(screen.getByText(/offline_access scope may be disabled/)).toBeVisible(); + expect(screen.getByText(/'offline_access' scope may be disabled/)).toBeVisible(); expect(screen.getByText(/Invalid scopes: Pulse openid profile email offline_access/)).toBeVisible(); });