diff --git a/admin/src/AppRoutes.tsx b/admin/src/AppRoutes.tsx index 5a251b2..5d6b1ab 100644 --- a/admin/src/AppRoutes.tsx +++ b/admin/src/AppRoutes.tsx @@ -4,7 +4,7 @@ * enough and keeps the bundle small. Change screens listen to popstate. */ import { useEffect, useState } from "react"; -import { config } from "./config"; +import { ConfigError, config } from "./config"; import { AuthError, buildAuthorizeUrl, @@ -31,16 +31,13 @@ async function probeApi(setScreen: (screen: Screen) => void) { // A cheap authenticated probe so a 403 surfaces as an explicit refusal // right after login instead of a silent empty shell. try { - const response = await apiFetch("/api/v1/children", {}, config); - if (response.status === 403) { - setScreen({ - kind: "refusal", - message: "Your account is not in the nestquest-admins group.", - }); - return; - } + await apiFetch("/api/v1/children", {}, config); setScreen({ kind: "app" }); } catch (error) { + if (error instanceof ConfigError) { + setScreen({ kind: "config-error", message: error.message }); + return; + } if (error instanceof ApiForbiddenError) { setScreen({ kind: "refusal", message: error.message }); return; diff --git a/admin/src/api/client.test.ts b/admin/src/api/client.test.ts index 4d62824..9cd828a 100644 --- a/admin/src/api/client.test.ts +++ b/admin/src/api/client.test.ts @@ -72,4 +72,23 @@ describe("API client", () => { ); expect(url.searchParams.get("code_challenge_method")).toBe("S256"); }); + + it("preserves non-401/403 statuses on the post-refresh retry (500 is not mislabeled)", async () => { + storeTokens({ access_token: "stale", refresh_token: "rt-1", expires_in: 600 }); + const fetchMock = vi + .fn() + .mockResolvedValueOnce(new Response("", { status: 401 })) // initial API call + .mockResolvedValueOnce( + jsonResponse({ access_token: "fresh", expires_in: 600 }), // refresh grant + ) + .mockResolvedValueOnce(new Response("", { status: 500 })); // retry + + const response = await apiFetch("/api/v1/children", {}, app, fetchMock as any); + + expect(fetchMock).toHaveBeenCalledTimes(3); + expect(fetchMock.mock.calls[1][0]).toBe(app.tokenEndpoint); + expect(fetchMock.mock.calls[2][1].headers.Authorization).toBe("Bearer fresh"); + // A server failure must surface as the real status, not ApiUnauthorizedError. + expect(response.status).toBe(500); + }); }); diff --git a/admin/src/api/client.ts b/admin/src/api/client.ts index f65bfd3..4ad468b 100644 --- a/admin/src/api/client.ts +++ b/admin/src/api/client.ts @@ -75,7 +75,10 @@ export async function apiFetch( }); if (retryResponse.status === 403) throw new ApiForbiddenError(); if (retryResponse.ok) return retryResponse; - throw new ApiUnauthorizedError(); + // Only 401 means "unauthorized"; preserve every other status (e.g. a + // 500 from the API) so callers can react to the real response. + if (retryResponse.status === 401) throw new ApiUnauthorizedError(); + return retryResponse; } await triggerLogin(app); throw new ApiUnauthorizedError(); diff --git a/admin/src/auth/oidc.test.ts b/admin/src/auth/oidc.test.ts index 3175367..cd63b02 100644 --- a/admin/src/auth/oidc.test.ts +++ b/admin/src/auth/oidc.test.ts @@ -117,13 +117,13 @@ describe("OIDC flow", () => { expect(sessionStorage.length).toBe(0); }); - it("logout clears auth and redirects to the end-session endpoint", () => { + it("logout clears auth and redirects to the end-session endpoint", async () => { storeTokens({ access_token: "a", id_token: "idt", expires_in: 60 }); const assign = vi.fn(); const original = window.location; vi.stubGlobal("location", { ...original, assign, origin: "http://localhost:5173" }); - logout(testApp()); + await logout(testApp()); expect(assign).toHaveBeenCalledTimes(1); const url = new URL(assign.mock.calls[0][0] as string); @@ -133,4 +133,41 @@ describe("OIDC flow", () => { expect(getToken()).toBeUndefined(); vi.unstubAllGlobals(); }); + + it("buildAuthorizeUrl resolves the authorize endpoint via discovery for a non-default issuer", async () => { + const issuer = "https://sso.discovery.example/application/o/discovered"; + const discoveryDoc = { + issuer, + authorization_endpoint: "https://sso.discovery.example/application/o/authorize/", + token_endpoint: "https://sso.discovery.example/application/o/token/", + userinfo_endpoint: "https://sso.discovery.example/application/o/userinfo/", + end_session_endpoint: `${issuer}/end-session/`, + jwks_uri: `${issuer}/jwks/`, + }; + const fetchMock = vi.fn().mockImplementation((url: string) => + Promise.resolve( + String(url).endsWith("/.well-known/openid-configuration") + ? jsonResponse(discoveryDoc) + : jsonResponse({}), + ), + ); + const app: AppConfig = { + issuer, + clientId: "test-client-id", + apiBaseUrl: "https://api.example.com", + redirectUri: "http://localhost:5173/auth/callback", + }; + + const url = new URL(await buildAuthorizeUrl(app, fetchMock as any)); + + // The authorize endpoint comes from the discovery document — Authentik + // serves it outside the app-specific issuer path, so it is NOT + // `${issuer}/authorize/` and must never be a hardcoded production host. + expect(url.origin + url.pathname).toBe(discoveryDoc.authorization_endpoint); + expect(url.searchParams.get("client_id")).toBe("test-client-id"); + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(String(fetchMock.mock.calls[0][0])).toBe( + `${issuer}/.well-known/openid-configuration`, + ); + }); }); diff --git a/admin/src/auth/oidc.ts b/admin/src/auth/oidc.ts index 76330d1..f2fe723 100644 --- a/admin/src/auth/oidc.ts +++ b/admin/src/auth/oidc.ts @@ -4,7 +4,7 @@ * Never sends a client_secret anywhere. Tokens are stored in sessionStorage * (per-tab, cleared on tab close) — never localStorage. */ -import { config, type AppConfig } from "../config"; +import { config, resolveEndpoints, type AppConfig, type ResolvedAppConfig } from "../config"; import { computeCodeChallenge, generateCodeVerifier, @@ -36,14 +36,19 @@ function makeScopes(): string { /** * Build the Authentik authorization redirect URL, storing state + verifier in - * sessionStorage so the callback can validate them. + * sessionStorage so the callback can validate them. Endpoints are resolved + * from the issuer's OIDC discovery document when not set on the config. */ -export async function buildAuthorizeUrl(app: AppConfig = config): Promise { +export async function buildAuthorizeUrl( + app: AppConfig = config, + fetchImpl: typeof fetch = fetch, +): Promise { if (!app.clientId) { throw new AuthError( "Missing VITE_AUTHENTIK_CLIENT_ID configuration. Set it in your .env before building the admin PWA.", ); } + const resolved = await resolveEndpoints(app, fetchImpl); const state = generateState(); const codeVerifier = generateCodeVerifier(); const codeChallenge = await computeCodeChallenge(codeVerifier); @@ -51,7 +56,7 @@ export async function buildAuthorizeUrl(app: AppConfig = config): Promise { + const resolved = await resolveEndpoints(app, fetchImpl); const idTokenHint = getIdToken(); clearAuth(); const params = new URLSearchParams(); if (idTokenHint) params.set("id_token_hint", idTokenHint); if (app.clientId) params.set("client_id", app.clientId); params.set("post_logout_redirect_uri", window.location.origin); - window.location.assign(`${app.endSessionEndpoint}?${params.toString()}`); + window.location.assign(`${resolved.endSessionEndpoint}?${params.toString()}`); } diff --git a/admin/src/config.test.ts b/admin/src/config.test.ts new file mode 100644 index 0000000..9fcf9bf --- /dev/null +++ b/admin/src/config.test.ts @@ -0,0 +1,215 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { + ConfigError, + discoveryUrlForIssuer, + readConfig, + resolveEndpoints, + resolveEndpointsFromDiscovery, + type AppConfig, +} from "./config"; + +function jsonResponse(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { + status, + headers: { "Content-Type": "application/json" }, + }); +} + +// Mirrors Authentik's layout: authorize/token/userinfo live OUTSIDE the +// application-specific issuer path, end-session/jwks under it. +function discoveryDocument(issuer: string) { + const normalized = issuer.replace(/\/+$/, ""); + const origin = new URL(normalized).origin; + return { + issuer, + authorization_endpoint: `${origin}/application/o/authorize/`, + token_endpoint: `${origin}/application/o/token/`, + userinfo_endpoint: `${origin}/application/o/userinfo/`, + end_session_endpoint: `${normalized}/end-session/`, + jwks_uri: `${normalized}/jwks/`, + }; +} + +describe("config", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("readConfig keeps the production issuer as the default", () => { + const cfg = readConfig({}); + expect(cfg.issuer).toBe("https://auth.cubecraftlabs.com/application/o/nestquest"); + expect(cfg.clientId).toBeUndefined(); + }); + + it("readConfig normalizes a trailing slash off a custom issuer", () => { + expect( + readConfig({ VITE_AUTHENTIK_ISSUER: "https://sso.custom.example/application/o/custom/" }) + .issuer, + ).toBe("https://sso.custom.example/application/o/custom"); + }); + + it("readConfig hardcodes no OIDC endpoints — they come from discovery", () => { + const cfg = readConfig({}); + expect(cfg.authorizeEndpoint).toBeUndefined(); + expect(cfg.tokenEndpoint).toBeUndefined(); + expect(cfg.userinfoEndpoint).toBeUndefined(); + expect(cfg.endSessionEndpoint).toBeUndefined(); + expect(cfg.jwksUri).toBeUndefined(); + }); + + it("builds the discovery URL for issuers with and without a trailing slash", () => { + expect(discoveryUrlForIssuer("https://sso.example.com/application/o/app/")).toBe( + "https://sso.example.com/application/o/app/.well-known/openid-configuration", + ); + expect(discoveryUrlForIssuer("https://sso.example.com/application/o/app")).toBe( + "https://sso.example.com/application/o/app/.well-known/openid-configuration", + ); + }); + + it("resolves every endpoint from the discovery document of a non-default issuer", async () => { + const issuer = "https://sso.other-host.example/application/o/other-app"; + const doc = discoveryDocument(issuer); + const fetchMock = vi.fn().mockResolvedValue(jsonResponse(doc)); + const app: AppConfig = { + issuer, + clientId: "client-id", + apiBaseUrl: "https://api.example.com", + redirectUri: "http://localhost:5173/auth/callback", + }; + + const resolved = await resolveEndpoints(app, fetchMock as unknown as typeof fetch); + + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(fetchMock).toHaveBeenCalledWith( + "https://sso.other-host.example/application/o/other-app/.well-known/openid-configuration", + ); + // Every endpoint comes verbatim from the (non-default) discovery document. + expect(resolved.authorizeEndpoint).toBe(doc.authorization_endpoint); + expect(resolved.tokenEndpoint).toBe(doc.token_endpoint); + expect(resolved.userinfoEndpoint).toBe(doc.userinfo_endpoint); + expect(resolved.endSessionEndpoint).toBe(doc.end_session_endpoint); + expect(resolved.jwksUri).toBe(doc.jwks_uri); + // Proves no hardcoded production host leaked into the resolved endpoints. + for (const endpoint of [ + resolved.authorizeEndpoint, + resolved.tokenEndpoint, + resolved.userinfoEndpoint, + resolved.endSessionEndpoint, + resolved.jwksUri, + ]) { + expect(endpoint).not.toMatch(/cubecraftlabs/); + } + }); + + it("resolves discovery for an issuer configured with a trailing slash", async () => { + const issuer = "https://sso.slashy.example/application/o/slashy/"; + const doc = discoveryDocument(issuer); + const fetchMock = vi.fn().mockResolvedValue(jsonResponse(doc)); + + const resolved = await resolveEndpointsFromDiscovery(issuer, fetchMock as any); + + expect(fetchMock).toHaveBeenCalledWith( + "https://sso.slashy.example/application/o/slashy/.well-known/openid-configuration", + ); + expect(resolved.authorizeEndpoint).toBe(doc.authorization_endpoint); + expect(resolved.endSessionEndpoint).toBe(doc.end_session_endpoint); + }); + + it("caches discovery per issuer for the session", async () => { + const issuer = "https://sso.cached.example/application/o/cached"; + const fetchMock = vi.fn().mockResolvedValue(jsonResponse(discoveryDocument(issuer))); + + const first = await resolveEndpointsFromDiscovery(issuer, fetchMock as any); + const second = await resolveEndpointsFromDiscovery(issuer, fetchMock as any); + + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(second).toBe(first); + }); + + it("surfaces a clear configuration error when discovery returns an HTTP error", async () => { + const issuer = "https://sso.http-error.example/application/o/broken"; + const fetchMock = vi.fn().mockResolvedValue(new Response("", { status: 500 })); + + const error = await resolveEndpointsFromDiscovery(issuer, fetchMock as any).then( + () => null, + (error: unknown) => error, + ); + + expect(error).toBeInstanceOf(ConfigError); + expect((error as Error).message).toContain(issuer); + expect((error as Error).message).toContain("HTTP 500"); + }); + + it("surfaces a clear configuration error when discovery is unreachable", async () => { + const issuer = "https://sso.unreachable.example/application/o/unreachable"; + const fetchMock = vi.fn().mockRejectedValue(new TypeError("network down")); + + await expect( + resolveEndpointsFromDiscovery(issuer, fetchMock as any), + ).rejects.toBeInstanceOf(ConfigError); + }); + + it("surfaces a clear configuration error when the discovery document is incomplete", async () => { + const issuer = "https://sso.incomplete.example/application/o/incomplete"; + const doc = discoveryDocument(issuer) as Record; + delete doc.end_session_endpoint; + const fetchMock = vi.fn().mockResolvedValue(jsonResponse(doc)); + + await expect( + resolveEndpointsFromDiscovery(issuer, fetchMock as any), + ).rejects.toThrow(/end_session_endpoint/); + }); + + it("rejects a discovery document advertising a different issuer", async () => { + const issuer = "https://sso.mismatch.example/application/o/mismatch"; + const doc = { + ...discoveryDocument(issuer), + issuer: "https://sso.elsewhere.example/application/o/elsewhere", + }; + const fetchMock = vi.fn().mockResolvedValue(jsonResponse(doc)); + + await expect( + resolveEndpointsFromDiscovery(issuer, fetchMock as any), + ).rejects.toThrow(/issuer mismatch/i); + }); + + it("the bundled config resolves its endpoints from discovery at runtime", async () => { + const issuer = "https://sso.bundled.example/application/o/bundled"; + const fetchMock = vi.fn().mockResolvedValue(jsonResponse(discoveryDocument(issuer))); + const bundled = readConfig({ VITE_AUTHENTIK_ISSUER: `${issuer}/`, VITE_AUTHENTIK_CLIENT_ID: "bundled-client" }); + expect(bundled.authorizeEndpoint).toBeUndefined(); + + const resolved = await resolveEndpoints(bundled, fetchMock as any); + + expect(resolved.authorizeEndpoint).toBe( + "https://sso.bundled.example/application/o/authorize/", + ); + expect(resolved.tokenEndpoint).toBe("https://sso.bundled.example/application/o/token/"); + expect(resolved.userinfoEndpoint).toBe( + "https://sso.bundled.example/application/o/userinfo/", + ); + expect(resolved.endSessionEndpoint).toBe( + "https://sso.bundled.example/application/o/bundled/end-session/", + ); + expect(resolved.jwksUri).toBe("https://sso.bundled.example/application/o/bundled/jwks/"); + }); + + it("does not re-fetch discovery when the config already carries endpoints", async () => { + const app: AppConfig = { + issuer: "https://auth.example.com/application/o/nestquest", + clientId: "client-id", + apiBaseUrl: "https://api.example.com", + authorizeEndpoint: "https://auth.example.com/application/o/authorize/", + tokenEndpoint: "https://auth.example.com/application/o/token/", + userinfoEndpoint: "https://auth.example.com/application/o/userinfo/", + endSessionEndpoint: "https://auth.example.com/application/o/nestquest/end-session/", + redirectUri: "http://localhost:5173/auth/callback", + }; + const fetchMock = vi.fn(); + + const resolved = await resolveEndpoints(app, fetchMock as any); + + expect(fetchMock).not.toHaveBeenCalled(); + expect(resolved).toBe(app); + }); +}); diff --git a/admin/src/config.ts b/admin/src/config.ts index 5e9309c..9880be9 100644 --- a/admin/src/config.ts +++ b/admin/src/config.ts @@ -1,19 +1,61 @@ /** * Runtime configuration, read from Vite env (import.meta.env). * See .env.example for the variables and their defaults. + * + * OIDC endpoints are NOT hardcoded here. They are resolved lazily from the + * issuer's OIDC discovery document (see resolveEndpoints), because Authentik + * serves authorize/token/userinfo OUTSIDE the application-specific issuer + * path (issuer is /application/o//, authorize is /application/o/authorize/). + * Both hardcoding the production host and naively concatenating onto the + * issuer would break non-default issuers; discovery is the only correct way. */ export interface AppConfig { issuer: string; clientId: string | undefined; apiBaseUrl: string; + /** Resolved from the issuer's OIDC discovery document — see resolveEndpoints(). */ + authorizeEndpoint?: string; + tokenEndpoint?: string; + userinfoEndpoint?: string; + endSessionEndpoint?: string; + jwksUri?: string; + redirectUri: string; +} + +/** AppConfig whose OIDC endpoints have been filled in by resolveEndpoints(). */ +export type ResolvedAppConfig = AppConfig & + Required< + Pick< + AppConfig, + "authorizeEndpoint" | "tokenEndpoint" | "userinfoEndpoint" | "endSessionEndpoint" + > + >; + +export interface DiscoveredEndpoints { + issuer: string; authorizeEndpoint: string; tokenEndpoint: string; userinfoEndpoint: string; endSessionEndpoint: string; - redirectUri: string; + jwksUri: string; } +/** Raised when OIDC discovery fails or the configuration is unusable. */ +export class ConfigError extends Error { + constructor(message: string) { + super(message); + this.name = "ConfigError"; + } +} + +const REQUIRED_ENDPOINT_NAMES = [ + "authorizeEndpoint", + "tokenEndpoint", + "userinfoEndpoint", + "endSessionEndpoint", +] as const; + export function readConfig(env: Record = import.meta.env as any): AppConfig { const issuer = ( env.VITE_AUTHENTIK_ISSUER || "https://auth.cubecraftlabs.com/application/o/nestquest/" @@ -27,12 +69,153 @@ export function readConfig(env: Record = import.meta // screen, not a broken redirect to Authentik. clientId: clientId || undefined, apiBaseUrl, - authorizeEndpoint: "https://auth.cubecraftlabs.com/application/o/authorize/", - tokenEndpoint: "https://auth.cubecraftlabs.com/application/o/token/", - userinfoEndpoint: "https://auth.cubecraftlabs.com/application/o/userinfo/", - endSessionEndpoint: `${issuer}/end-session/`, + // authorize/token/userinfo/end-session endpoints are resolved from the + // issuer's discovery document at runtime — never hardcoded here. redirectUri: `${window.location.origin}/auth/callback`, }; } export const config: AppConfig = readConfig(); + +interface DiscoveryDocument { + issuer?: unknown; + authorization_endpoint?: unknown; + token_endpoint?: unknown; + userinfo_endpoint?: unknown; + end_session_endpoint?: unknown; + jwks_uri?: unknown; +} + +function normalizeIssuer(issuer: string): string { + return issuer.replace(/\/+$/, ""); +} + +function nonEmptyString(value: unknown): string | undefined { + return typeof value === "string" && value.length > 0 ? value : undefined; +} + +/** Discovery document location for an issuer, tolerant of a trailing slash. */ +export function discoveryUrlForIssuer(issuer: string): string { + return `${normalizeIssuer(issuer)}/.well-known/openid-configuration`; +} + +async function fetchDiscoveredEndpoints( + issuer: string, + fetchImpl: typeof fetch, +): Promise { + const normalizedIssuer = normalizeIssuer(issuer); + const discoveryUrl = `${normalizedIssuer}/.well-known/openid-configuration`; + + let response: Response; + try { + response = await fetchImpl(discoveryUrl); + } catch (error) { + const reason = error instanceof Error ? error.message : String(error); + throw new ConfigError( + `OIDC discovery request to ${discoveryUrl} failed (${reason}). ` + + "Check VITE_AUTHENTIK_ISSUER and network reachability.", + ); + } + if (!response.ok) { + throw new ConfigError( + `OIDC discovery failed: ${discoveryUrl} returned HTTP ${response.status}. ` + + "Check VITE_AUTHENTIK_ISSUER.", + ); + } + + let doc: DiscoveryDocument; + try { + doc = (await response.json()) as DiscoveryDocument; + } catch { + throw new ConfigError( + `OIDC discovery document at ${discoveryUrl} is not valid JSON. Check VITE_AUTHENTIK_ISSUER.`, + ); + } + + const authorizationEndpoint = nonEmptyString(doc.authorization_endpoint); + const tokenEndpoint = nonEmptyString(doc.token_endpoint); + const userinfoEndpoint = nonEmptyString(doc.userinfo_endpoint); + const endSessionEndpoint = nonEmptyString(doc.end_session_endpoint); + const jwksUri = nonEmptyString(doc.jwks_uri); + const discoveredIssuer = nonEmptyString(doc.issuer); + + const endpointFields: Array<[string, string | undefined]> = [ + ["authorization_endpoint", authorizationEndpoint], + ["token_endpoint", tokenEndpoint], + ["userinfo_endpoint", userinfoEndpoint], + ["end_session_endpoint", endSessionEndpoint], + ["jwks_uri", jwksUri], + ]; + const missing = endpointFields.filter(([, value]) => !value).map(([name]) => name); + if (missing.length > 0) { + throw new ConfigError( + `OIDC discovery document at ${discoveryUrl} is missing: ${missing.join(", ")}. ` + + "The issuer must point at an OIDC provider that publishes these endpoints.", + ); + } + + if (discoveredIssuer && normalizeIssuer(discoveredIssuer) !== normalizedIssuer) { + throw new ConfigError( + `OIDC discovery issuer mismatch: ${discoveryUrl} advertises "${discoveredIssuer}" ` + + `but VITE_AUTHENTIK_ISSUER is "${normalizedIssuer}".`, + ); + } + + return { + issuer: normalizedIssuer, + authorizeEndpoint: authorizationEndpoint!, + tokenEndpoint: tokenEndpoint!, + userinfoEndpoint: userinfoEndpoint!, + endSessionEndpoint: endSessionEndpoint!, + jwksUri: jwksUri!, + }; +} + +/** + * Session cache: one discovery fetch per issuer per SPA session. Failed + * lookups are evicted so a transient network error can be retried instead of + * poisoning the whole session. + */ +const discoveryCache = new Map>(); + +export function resolveEndpointsFromDiscovery( + issuer: string, + fetchImpl: typeof fetch = fetch, +): Promise { + const normalizedIssuer = normalizeIssuer(issuer); + const cached = discoveryCache.get(normalizedIssuer); + if (cached) return cached; + + const pending = fetchDiscoveredEndpoints(normalizedIssuer, fetchImpl); + discoveryCache.set(normalizedIssuer, pending); + pending.catch(() => { + discoveryCache.delete(normalizedIssuer); + }); + return pending; +} + +/** + * Return `app` with all OIDC endpoints filled in. When the config already + * carries them (pre-resolved config, tests) this is a no-op; otherwise the + * issuer's discovery document is fetched (cached for the session) and its + * authorization/token/userinfo/end-session/jwks endpoints are used. + */ +export async function resolveEndpoints( + app: AppConfig, + fetchImpl: typeof fetch = fetch, +): Promise { + const missing = REQUIRED_ENDPOINT_NAMES.filter((name) => !app[name]); + if (missing.length === 0) { + return app as ResolvedAppConfig; + } + + const discovered = await resolveEndpointsFromDiscovery(app.issuer, fetchImpl); + return { + ...app, + authorizeEndpoint: discovered.authorizeEndpoint, + tokenEndpoint: discovered.tokenEndpoint, + userinfoEndpoint: discovered.userinfoEndpoint, + endSessionEndpoint: discovered.endSessionEndpoint, + jwksUri: discovered.jwksUri, + }; +}