From 2962e0c2ae654369db58206ea691106795ad47e6 Mon Sep 17 00:00:00 2001 From: Shaheer Sarfaraz <53654735+DaKheera47@users.noreply.github.com> Date: Tue, 10 Feb 2026 18:05:47 +0000 Subject: [PATCH] Fix password manager autofill for pipeline auth (#92) (#127) * Fix basic auth flow to support password manager autofill * fix orchestrator CI typecheck in api client * clear basic auth fields when prompt closes * update basic auth dialog description copy --- orchestrator/src/client/App.tsx | 2 + .../src/client/api/client.auth.test.ts | 113 ++++++++ .../src/client/api/client.demo.test.ts | 1 + orchestrator/src/client/api/client.ts | 241 +++++++++++++++--- .../src/client/components/BasicAuthPrompt.tsx | 162 ++++++++++++ orchestrator/src/server/app.ts | 1 - orchestrator/src/server/basic-auth.test.ts | 2 +- 7 files changed, 487 insertions(+), 35 deletions(-) create mode 100644 orchestrator/src/client/api/client.auth.test.ts create mode 100644 orchestrator/src/client/components/BasicAuthPrompt.tsx diff --git a/orchestrator/src/client/App.tsx b/orchestrator/src/client/App.tsx index 4541a38..62b35ed 100644 --- a/orchestrator/src/client/App.tsx +++ b/orchestrator/src/client/App.tsx @@ -7,6 +7,7 @@ import { Navigate, Route, Routes, useLocation } from "react-router-dom"; import { CSSTransition, SwitchTransition } from "react-transition-group"; import { Toaster } from "@/components/ui/sonner"; +import { BasicAuthPrompt } from "./components/BasicAuthPrompt"; import { OnboardingGate } from "./components/OnboardingGate"; import { useDemoInfo } from "./hooks/useDemoInfo"; import { HomePage } from "./pages/HomePage"; @@ -32,6 +33,7 @@ export const App: React.FC = () => { return ( <> + {demoInfo?.demoMode && (
Demo mode: integrations are simulated and data resets every{" "} diff --git a/orchestrator/src/client/api/client.auth.test.ts b/orchestrator/src/client/api/client.auth.test.ts new file mode 100644 index 0000000..369aebf --- /dev/null +++ b/orchestrator/src/client/api/client.auth.test.ts @@ -0,0 +1,113 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import * as api from "./client"; + +function createJsonResponse(status: number, payload: unknown): Response { + return { + status, + text: async () => JSON.stringify(payload), + } as Response; +} + +describe("API client basic auth prompt flow", () => { + beforeEach(() => { + vi.restoreAllMocks(); + api.__resetApiClientAuthForTests(); + }); + + afterEach(() => { + api.__resetApiClientAuthForTests(); + }); + + it("retries write requests with prompted credentials after unauthorized", async () => { + const fetchSpy = vi.spyOn(global, "fetch"); + fetchSpy + .mockResolvedValueOnce( + createJsonResponse(401, { + ok: false, + error: { code: "UNAUTHORIZED", message: "Authentication required" }, + meta: { requestId: "req-1" }, + }), + ) + .mockResolvedValueOnce( + createJsonResponse(200, { + ok: true, + data: { message: "ok" }, + meta: { requestId: "req-2" }, + }), + ); + + const promptHandler = vi + .fn() + .mockResolvedValue({ username: "user", password: "pass" }); + api.setBasicAuthPromptHandler(promptHandler); + + await expect(api.runPipeline()).resolves.toEqual({ message: "ok" }); + expect(promptHandler).toHaveBeenCalledWith( + expect.objectContaining({ + endpoint: "/pipeline/run", + method: "POST", + attempt: 1, + }), + ); + expect(fetchSpy).toHaveBeenCalledTimes(2); + const retryHeaders = fetchSpy.mock.calls[1]?.[1]?.headers as Record< + string, + string + >; + expect(retryHeaders.Authorization).toMatch(/^Basic /); + }); + + it("reuses cached credentials for later write requests", async () => { + const fetchSpy = vi.spyOn(global, "fetch"); + fetchSpy + .mockResolvedValueOnce( + createJsonResponse(401, { + ok: false, + error: { code: "UNAUTHORIZED", message: "Authentication required" }, + meta: { requestId: "req-1" }, + }), + ) + .mockResolvedValueOnce( + createJsonResponse(200, { + ok: true, + data: { message: "first" }, + meta: { requestId: "req-2" }, + }), + ) + .mockResolvedValueOnce( + createJsonResponse(200, { + ok: true, + data: { message: "second" }, + meta: { requestId: "req-3" }, + }), + ); + + const promptHandler = vi + .fn() + .mockResolvedValue({ username: "user", password: "pass" }); + api.setBasicAuthPromptHandler(promptHandler); + + await expect(api.runPipeline()).resolves.toEqual({ message: "first" }); + await expect(api.runPipeline()).resolves.toEqual({ message: "second" }); + + expect(promptHandler).toHaveBeenCalledTimes(1); + const secondRequestHeaders = fetchSpy.mock.calls[2]?.[1]?.headers as Record< + string, + string + >; + expect(secondRequestHeaders.Authorization).toMatch(/^Basic /); + }); + + it("throws unauthorized when the prompt is cancelled", async () => { + vi.spyOn(global, "fetch").mockResolvedValueOnce( + createJsonResponse(401, { + ok: false, + error: { code: "UNAUTHORIZED", message: "Authentication required" }, + meta: { requestId: "req-1" }, + }), + ); + api.setBasicAuthPromptHandler(vi.fn().mockResolvedValue(null)); + + await expect(api.runPipeline()).rejects.toThrow("Authentication required"); + }); +}); diff --git a/orchestrator/src/client/api/client.demo.test.ts b/orchestrator/src/client/api/client.demo.test.ts index b938bea..61123f2 100644 --- a/orchestrator/src/client/api/client.demo.test.ts +++ b/orchestrator/src/client/api/client.demo.test.ts @@ -13,6 +13,7 @@ describe("API client demo toasts", () => { beforeEach(() => { customToast.mockClear(); vi.restoreAllMocks(); + api.__resetApiClientAuthForTests(); }); it("shows simulated toast when response meta.simulated is true", async () => { diff --git a/orchestrator/src/client/api/client.ts b/orchestrator/src/client/api/client.ts index c612e6f..e916651 100644 --- a/orchestrator/src/client/api/client.ts +++ b/orchestrator/src/client/api/client.ts @@ -40,11 +40,19 @@ const API_BASE = "/api"; class ApiClientError extends Error { requestId?: string; + status?: number; + code?: string; - constructor(message: string, requestId?: string) { + constructor( + message: string, + options?: { requestId?: string; status?: number; code?: string }, + ) { + const requestId = options?.requestId; super(requestId ? `${message} (requestId: ${requestId})` : message); this.name = "ApiClientError"; this.requestId = requestId; + this.status = options?.status; + this.code = options?.code; } } @@ -61,6 +69,43 @@ type LegacyApiResponse = details?: unknown; }; +export type BasicAuthCredentials = { + username: string; + password: string; +}; + +export type BasicAuthPromptRequest = { + endpoint: string; + method: string; + attempt: number; + usernameHint?: string; + errorMessage?: string; +}; + +type BasicAuthPromptHandler = ( + request: BasicAuthPromptRequest, +) => Promise; + +let basicAuthPromptHandler: BasicAuthPromptHandler | null = null; +let basicAuthPromptInFlight: Promise | null = null; +let cachedBasicAuthCredentials: BasicAuthCredentials | null = null; + +export function setBasicAuthPromptHandler( + handler: BasicAuthPromptHandler | null, +): void { + basicAuthPromptHandler = handler; +} + +export function clearBasicAuthCredentials(): void { + cachedBasicAuthCredentials = null; +} + +export function __resetApiClientAuthForTests(): void { + basicAuthPromptHandler = null; + basicAuthPromptInFlight = null; + cachedBasicAuthCredentials = null; +} + function normalizeApiResponse( payload: unknown, ): ApiResponse | LegacyApiResponse { @@ -104,16 +149,98 @@ function describeAction(endpoint: string, method?: string): string { return "This action ran in demo simulation mode."; } -async function fetchApi( +function encodeBasicAuth(credentials: BasicAuthCredentials): string { + return `Basic ${btoa(`${credentials.username}:${credentials.password}`)}`; +} + +function normalizeHeaders(headers?: HeadersInit): Record { + if (!headers) return {}; + if (headers instanceof Headers) { + const next: Record = {}; + headers.forEach((value, key) => { + next[key] = value; + }); + return next; + } + if (Array.isArray(headers)) { + return Object.fromEntries(headers); + } + return { ...headers }; +} + +function isWriteMethod(method: string): boolean { + return !["GET", "HEAD", "OPTIONS"].includes(method.toUpperCase()); +} + +function isUnauthorizedResponse( + response: Response, + parsed: ApiResponse | LegacyApiResponse, +): boolean { + if (response.status !== 401) return false; + if ("ok" in parsed) { + return parsed.ok ? false : parsed.error.code === "UNAUTHORIZED"; + } + return !parsed.success; +} + +function toApiError( + response: Response, + parsed: ApiResponse | LegacyApiResponse, +): ApiClientError { + if ("ok" in parsed) { + if (!parsed.ok) { + return new ApiClientError(parsed.error.message || "API request failed", { + requestId: parsed.meta?.requestId, + status: response.status, + code: parsed.error.code, + }); + } + return new ApiClientError("API request failed", { + requestId: parsed.meta?.requestId, + status: response.status, + }); + } + if (parsed.success) { + return new ApiClientError(parsed.message || "API request failed", { + status: response.status, + }); + } + return new ApiClientError( + parsed.error || parsed.message || "API request failed", + { + status: response.status, + }, + ); +} + +async function requestBasicAuthCredentials( + request: BasicAuthPromptRequest, +): Promise { + if (!basicAuthPromptHandler) return null; + if (!basicAuthPromptInFlight) { + basicAuthPromptInFlight = basicAuthPromptHandler(request).finally(() => { + basicAuthPromptInFlight = null; + }); + } + return basicAuthPromptInFlight; +} + +async function fetchAndParse( endpoint: string, - options?: RequestInit, -): Promise { + options: RequestInit | undefined, + authHeader?: string, +): Promise<{ + response: Response; + parsed: ApiResponse | LegacyApiResponse; +}> { + const headers: Record = { + "Content-Type": "application/json", + ...normalizeHeaders(options?.headers), + }; + if (authHeader) headers.Authorization = authHeader; const response = await fetch(`${API_BASE}${endpoint}`, { ...options, - headers: { - "Content-Type": "application/json", - ...options?.headers, - }, + headers, }); const text = await response.text(); @@ -122,40 +249,88 @@ async function fetchApi( try { payload = JSON.parse(text); } catch { - // If the response is not JSON, it's likely an HTML error page - console.error("API returned non-JSON response:", text.substring(0, 500)); + // If the response is not JSON, it's likely an HTML error page. throw new ApiClientError( `Server error (${response.status}): Expected JSON but received HTML. Is the backend server running?`, + { status: response.status }, ); } const parsed = normalizeApiResponse(payload); + return { response, parsed }; +} - if ("ok" in parsed) { - if (!parsed.ok) { - if (parsed.meta?.blockedReason) { - showDemoBlockedToast(parsed.meta.blockedReason); - } - throw new ApiClientError( - parsed.error.message || "API request failed", - parsed.meta?.requestId, - ); - } - if (parsed.meta?.simulated) { - showDemoSimulatedToast(describeAction(endpoint, options?.method)); - } - return parsed.data as T; - } +async function fetchApi( + endpoint: string, + options?: RequestInit, +): Promise { + const method = (options?.method || "GET").toUpperCase(); + let authHeader = cachedBasicAuthCredentials + ? encodeBasicAuth(cachedBasicAuthCredentials) + : undefined; + let authAttempt = 0; + let usernameHint = cachedBasicAuthCredentials?.username; - if (!parsed.success) { - throw new ApiClientError( - parsed.error || parsed.message || "API request failed", + while (true) { + const { response, parsed } = await fetchAndParse( + endpoint, + options, + authHeader, ); - } - const data = parsed.data; - if (data !== undefined) return data as T; - if (parsed.message !== undefined) return { message: parsed.message } as T; - return null as T; + if ( + isWriteMethod(method) && + isUnauthorizedResponse(response, parsed) && + basicAuthPromptHandler && + authAttempt < 2 + ) { + const credentials = await requestBasicAuthCredentials({ + endpoint, + method, + attempt: authAttempt + 1, + usernameHint, + errorMessage: + authAttempt > 0 + ? "Invalid credentials. Please try again." + : undefined, + }); + if (!credentials) { + throw toApiError(response, parsed); + } + cachedBasicAuthCredentials = credentials; + usernameHint = credentials.username; + authHeader = encodeBasicAuth(credentials); + authAttempt += 1; + continue; + } + + if ("ok" in parsed) { + if (!parsed.ok) { + if (parsed.error.code === "UNAUTHORIZED") { + clearBasicAuthCredentials(); + } + if (parsed.meta?.blockedReason) { + showDemoBlockedToast(parsed.meta.blockedReason); + } + throw toApiError(response, parsed); + } + if (parsed.meta?.simulated) { + showDemoSimulatedToast(describeAction(endpoint, options?.method)); + } + return parsed.data as T; + } + + if (!parsed.success) { + if (response.status === 401) { + clearBasicAuthCredentials(); + } + throw toApiError(response, parsed); + } + + const data = parsed.data; + if (data !== undefined) return data as T; + if (parsed.message !== undefined) return { message: parsed.message } as T; + return null as T; + } } // Jobs API diff --git a/orchestrator/src/client/components/BasicAuthPrompt.tsx b/orchestrator/src/client/components/BasicAuthPrompt.tsx new file mode 100644 index 0000000..585417c --- /dev/null +++ b/orchestrator/src/client/components/BasicAuthPrompt.tsx @@ -0,0 +1,162 @@ +import { + type BasicAuthCredentials, + type BasicAuthPromptRequest, + setBasicAuthPromptHandler, +} from "@client/api/client"; +import React from "react"; +import { + AlertDialog, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from "@/components/ui/alert-dialog"; +import { Button } from "@/components/ui/button"; +import { Input } from "@/components/ui/input"; + +type PendingPrompt = { + request: BasicAuthPromptRequest; + resolve: (credentials: BasicAuthCredentials | null) => void; +}; + +export const BasicAuthPrompt: React.FC = () => { + const [pendingRequest, setPendingRequest] = + React.useState(null); + const pendingRequestRef = React.useRef(null); + const usernameInputRef = React.useRef(null); + const [username, setUsername] = React.useState(""); + const [password, setPassword] = React.useState(""); + const [errorMessage, setErrorMessage] = React.useState(null); + + const resolvePrompt = React.useCallback( + (credentials: BasicAuthCredentials | null) => { + const activePrompt = pendingRequestRef.current; + pendingRequestRef.current = null; + setPendingRequest(null); + setUsername(""); + setPassword(""); + setErrorMessage(null); + activePrompt?.resolve(credentials); + }, + [], + ); + + React.useEffect(() => { + setBasicAuthPromptHandler((request) => { + return new Promise((resolve) => { + if (pendingRequestRef.current) { + pendingRequestRef.current.resolve(null); + } + const nextPrompt = { request, resolve }; + pendingRequestRef.current = nextPrompt; + setPendingRequest(nextPrompt); + }); + }); + + return () => { + setBasicAuthPromptHandler(null); + if (pendingRequestRef.current) { + pendingRequestRef.current.resolve(null); + pendingRequestRef.current = null; + } + }; + }, []); + + React.useEffect(() => { + if (!pendingRequest) return; + setUsername(pendingRequest.request.usernameHint ?? ""); + setPassword(""); + setErrorMessage(pendingRequest.request.errorMessage ?? null); + const timeout = window.setTimeout(() => { + usernameInputRef.current?.focus(); + }, 0); + return () => { + window.clearTimeout(timeout); + }; + }, [pendingRequest]); + + const handleSubmit = React.useCallback( + (event: React.FormEvent) => { + event.preventDefault(); + const normalizedUsername = username.trim(); + if (!normalizedUsername || !password) { + setErrorMessage("Enter both username and password."); + return; + } + resolvePrompt({ username: normalizedUsername, password }); + }, + [password, resolvePrompt, username], + ); + + const request = pendingRequest?.request; + + return ( + { + if (!open) resolvePrompt(null); + }} + > + + + Authentication required + + You are required to authenticate to access this application. + + +
+
+ + setUsername(event.target.value)} + placeholder="Enter username" + /> +
+
+ + setPassword(event.target.value)} + placeholder="Enter password" + /> +
+ {errorMessage && ( +

+ {errorMessage} +

+ )} + + + + +
+
+
+ ); +}; diff --git a/orchestrator/src/server/app.ts b/orchestrator/src/server/app.ts index 42b784b..c5ea6f5 100644 --- a/orchestrator/src/server/app.ts +++ b/orchestrator/src/server/app.ts @@ -76,7 +76,6 @@ function createBasicAuthGuard() { const { enabled } = getAuthConfig(); if (!enabled || !requiresAuth(req.method, req.path)) return next(); if (isAuthorized(req)) return next(); - res.setHeader("WWW-Authenticate", 'Basic realm="Job Ops"'); fail(res, unauthorized("Authentication required")); }; diff --git a/orchestrator/src/server/basic-auth.test.ts b/orchestrator/src/server/basic-auth.test.ts index 2f67913..196463a 100644 --- a/orchestrator/src/server/basic-auth.test.ts +++ b/orchestrator/src/server/basic-auth.test.ts @@ -71,7 +71,7 @@ describe.sequential("Basic Auth read-only enforcement", () => { method: "POST", }); expect(postRes.status).toBe(401); - expect(postRes.headers.get("www-authenticate")).toMatch(/Basic/); + expect(postRes.headers.get("www-authenticate")).toBeNull(); const patchRes = await fetch(`${baseUrl}/api/jobs/123`, { method: "PATCH",