diff --git a/src/Frontend/src/AuthApp.vue b/src/Frontend/src/AuthApp.vue index 558e74faa3..af9dba5422 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 0000000000..3bb4bef77a --- /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 0000000000..b0ee4ef5fb --- /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 0000000000..cca122c640 --- /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 0000000000..89ee75bbae --- /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 8bc2694aa1..999fd62ccf 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 new file mode 100644 index 0000000000..fd140bddc6 --- /dev/null +++ b/src/Frontend/src/stores/AuthStore.spec.ts @@ -0,0 +1,47 @@ +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 () => { + // 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://servicecontrol/composed-by-servicecontrol openid profile email", + }, + ]); + + const store = useAuthStore(); + await store.refresh(); + + 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 () => { + // 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 64c4012136..1a81418498 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"; @@ -14,13 +14,17 @@ 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", () => { 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. @@ -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, @@ -102,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 cdcafba80f..9e1bc96a34 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 218a4e1d46..a26adca737 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, + }); + }); }); });