From b2a6ee1dcf2f2f4e3def4fbd58a5137c6ecf3bc4 Mon Sep 17 00:00:00 2001 From: Cliff Hall Date: Mon, 29 Jun 2026 17:49:34 -0400 Subject: [PATCH] fix(web): block close on incomplete EMA config, not just an invalid issuer (closes #1546) (#1547) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #1545. The close-block only fired for an invalid issuer, so an EMA config enabled with a valid issuer but a blank client ID was still silently dropped on close by the persist gate — the same no-feedback bug, different field. - validateClientSettings gains a requireComplete mode that also flags the required IdP fields (issuer, clientId) when blank, with distinct per-field messages. Inline (as-you-type) validation is unchanged — required-but-blank errors only surface on reveal, so the form never nags about not-yet-filled fields. - canPersistClientSettingsDraft is redefined in terms of that mode, so the persist gate and the close-time field errors share one source of truth. - ClientSettingsModal.handleClose now blocks + reveals on ANY require-complete error (invalid issuer OR blank required field) and keeps the modal open. The form prop revealIssuerError becomes revealErrors and surfaces the client-ID error too. Escape hatch: disable enterprise IdP (an empty issuer no longer closes, since that's itself incomplete). Tests cover the blank-clientId close path, fix-each-field, and disable-EMA escape, plus requireComplete validation. Co-authored-by: Claude Opus 4.8 (1M context) --- .../ClientSettingsForm.test.tsx | 54 +++++- .../ClientSettingsForm/ClientSettingsForm.tsx | 50 +++--- .../clientSettingsValues.test.ts | 91 ++++++++++ .../clientSettingsValues.ts | 87 +++++++--- .../ClientSettingsModal.test.tsx | 160 ++++++++++++++++-- .../ClientSettingsModal.tsx | 30 ++-- 6 files changed, 383 insertions(+), 89 deletions(-) diff --git a/clients/web/src/components/groups/ClientSettingsForm/ClientSettingsForm.test.tsx b/clients/web/src/components/groups/ClientSettingsForm/ClientSettingsForm.test.tsx index 4ea9c47c..7f3cbec2 100644 --- a/clients/web/src/components/groups/ClientSettingsForm/ClientSettingsForm.test.tsx +++ b/clients/web/src/components/groups/ClientSettingsForm/ClientSettingsForm.test.tsx @@ -10,6 +10,8 @@ import { import { EMPTY_CLIENT_SETTINGS, ISSUER_URL_ERROR, + ISSUER_REQUIRED_ERROR, + CLIENT_ID_REQUIRED_ERROR, type ClientSettingsFormValues, } from "./clientSettingsValues"; @@ -139,7 +141,7 @@ describe("ClientSettingsForm EMA IdP session", () => { expect(screen.queryByText(ISSUER_URL_ERROR)).not.toBeInTheDocument(); }); - it("reveals the issuer error without blur when revealIssuerError is set", () => { + it("reveals the issuer error without blur when revealErrors is set", () => { renderWithMantine( { onExpandedSectionsChange={vi.fn()} onSettingsChange={vi.fn()} emaIdpLoginState="none" - revealIssuerError + revealErrors />, ); @@ -160,23 +162,63 @@ describe("ClientSettingsForm EMA IdP session", () => { expect(screen.getByText(ISSUER_URL_ERROR)).toBeInTheDocument(); }); - it("shows no error for a valid issuer even when revealIssuerError is set", () => { + it("reveals required errors for blank issuer and client ID when revealErrors is set", () => { + renderWithMantine( + , + ); + + // Both required IdP fields are blank; a close/save attempt surfaces them + // instead of silently dropping the config. + expect(screen.getByText(ISSUER_REQUIRED_ERROR)).toBeInTheDocument(); + expect(screen.getByText(CLIENT_ID_REQUIRED_ERROR)).toBeInTheDocument(); + }); + + it("does not show required errors for blank fields before revealErrors", () => { + renderWithMantine( + , + ); + + // Inline validation must not nag about not-yet-filled required fields. + expect(screen.queryByText(ISSUER_REQUIRED_ERROR)).not.toBeInTheDocument(); + expect( + screen.queryByText(CLIENT_ID_REQUIRED_ERROR), + ).not.toBeInTheDocument(); + }); + + it("shows no error for a valid issuer even when revealErrors is set", () => { renderWithMantine( , ); expect(screen.queryByText(ISSUER_URL_ERROR)).not.toBeInTheDocument(); + expect( + screen.queryByText(CLIENT_ID_REQUIRED_ERROR), + ).not.toBeInTheDocument(); }); it("shows no issuer error for a valid URL", () => { @@ -251,7 +293,7 @@ describe("ClientSettingsForm EMA IdP session", () => { ).not.toBeInTheDocument(); }); - it("reveals the CIMD URL error without blur when revealClientMetadataUrlError is set", () => { + it("reveals the CIMD URL error without blur when revealErrors is set", () => { renderWithMantine( { onExpandedSectionsChange={vi.fn()} onSettingsChange={vi.fn()} emaIdpLoginState="none" - revealClientMetadataUrlError + revealErrors />, ); diff --git a/clients/web/src/components/groups/ClientSettingsForm/ClientSettingsForm.tsx b/clients/web/src/components/groups/ClientSettingsForm/ClientSettingsForm.tsx index 209564d7..cf89e2a8 100644 --- a/clients/web/src/components/groups/ClientSettingsForm/ClientSettingsForm.tsx +++ b/clients/web/src/components/groups/ClientSettingsForm/ClientSettingsForm.tsx @@ -30,16 +30,12 @@ export interface ClientSettingsFormProps { emaIdpLoginState?: EmaIdpLoginState; onEmaIdpLogout?: () => void; /** - * Force the issuer error to show even before the field is blurred. The parent - * sets this when a save/close is attempted with an invalid issuer, so the user - * isn't left with a silently-dropped value and no explanation. + * Force all field errors to show, including required-but-blank fields (EMA's + * issuer/clientId, CIMD's metadata URL). The parent sets this when a + * save/close is attempted with an incomplete or invalid config, so nothing is + * silently dropped without explanation. */ - revealIssuerError?: boolean; - /** - * Force the CIMD metadata URL error to show even before the field is blurred. - * Same contract as {@link revealIssuerError} for the CIMD URL field. - */ - revealClientMetadataUrlError?: boolean; + revealErrors?: boolean; } const HintText = Text.withProps({ @@ -54,8 +50,7 @@ export function ClientSettingsForm({ onSettingsChange, emaIdpLoginState = "none", onEmaIdpLogout, - revealIssuerError = false, - revealClientMetadataUrlError = false, + revealErrors = false, }: ClientSettingsFormProps) { function patch(partial: Partial) { onSettingsChange((prev) => ({ ...prev, ...partial })); @@ -64,23 +59,31 @@ export function ClientSettingsForm({ // Defer the issuer error until the field has been blurred so it doesn't nag // mid-typing (e.g. while "https:/…" is still incomplete). Once touched it // updates live, so the error clears as soon as a valid URL is entered. The - // parent also forces it on via `revealIssuerError` when a close/save is - // attempted with an invalid value, so the value is never silently dropped - // without explanation. The persist gate (canPersistClientSettingsDraft) - // validates independently — an invalid issuer is never written regardless. + // parent forces all errors on via `revealErrors` when a close/save is + // attempted with an incomplete or invalid config — including the + // required-but-blank fields, which inline validation otherwise leaves silent. + // The persist gate (canPersistClientSettingsDraft) validates independently, so + // an incomplete/invalid config is never written regardless of these flags. const [issuerTouched, setIssuerTouched] = useState(false); const [clientMetadataUrlTouched, setClientMetadataUrlTouched] = useState(false); - const errors = validateClientSettings(settings); - const showIssuerError = - (issuerTouched || revealIssuerError) && errors.issuer - ? errors.issuer + // Inline errors flag only a filled-in-wrong field (issuer URL); the + // require-complete set adds the blank-required fields surfaced on reveal. + const inlineErrors = validateClientSettings(settings); + const revealedErrors = validateClientSettings(settings, { + requireComplete: true, + }); + const showIssuerError = revealErrors + ? revealedErrors.issuer + : issuerTouched + ? inlineErrors.issuer : undefined; - const showClientMetadataUrlError = - (clientMetadataUrlTouched || revealClientMetadataUrlError) && - errors.clientMetadataUrl - ? errors.clientMetadataUrl + const showClientIdError = revealErrors ? revealedErrors.clientId : undefined; + const showClientMetadataUrlError = revealErrors + ? revealedErrors.clientMetadataUrl + : clientMetadataUrlTouched + ? inlineErrors.clientMetadataUrl : undefined; const showIdpSession = @@ -134,6 +137,7 @@ export function ClientSettingsForm({ description="Client id registered with your enterprise IdP." value={settings.clientId} onChange={(e) => patch({ clientId: e.currentTarget.value })} + error={showClientIdError} rightSectionPointerEvents="auto" rightSection={ settings.clientId ? ( diff --git a/clients/web/src/components/groups/ClientSettingsForm/clientSettingsValues.test.ts b/clients/web/src/components/groups/ClientSettingsForm/clientSettingsValues.test.ts index ff4c8a9a..bd240bb0 100644 --- a/clients/web/src/components/groups/ClientSettingsForm/clientSettingsValues.test.ts +++ b/clients/web/src/components/groups/ClientSettingsForm/clientSettingsValues.test.ts @@ -10,6 +10,9 @@ import { EMPTY_CLIENT_SETTINGS, formValuesToClientConfig, ISSUER_URL_ERROR, + ISSUER_REQUIRED_ERROR, + CLIENT_ID_REQUIRED_ERROR, + CLIENT_METADATA_URL_REQUIRED_ERROR, validateClientSettings, } from "./clientSettingsValues"; @@ -457,4 +460,92 @@ describe("clientSettingsValues", () => { }), ).toEqual({}); }); + + it("validateClientSettings does not flag blank required fields by default", () => { + expect( + validateClientSettings({ + emaEnabled: true, + issuer: "", + clientId: "", + clientSecret: "", + ...emptyCimd, + }), + ).toEqual({}); + }); + + it("validateClientSettings with requireComplete flags blank issuer and clientId", () => { + expect( + validateClientSettings( + { + emaEnabled: true, + issuer: "", + clientId: "", + clientSecret: "", + ...emptyCimd, + }, + { requireComplete: true }, + ), + ).toEqual({ + issuer: ISSUER_REQUIRED_ERROR, + clientId: CLIENT_ID_REQUIRED_ERROR, + }); + }); + + it("validateClientSettings with requireComplete keeps the URL error over the required error", () => { + expect( + validateClientSettings( + { + emaEnabled: true, + issuer: "not-a-url", + clientId: "cid", + clientSecret: "", + ...emptyCimd, + }, + { requireComplete: true }, + ), + ).toEqual({ issuer: ISSUER_URL_ERROR }); + }); + + it("validateClientSettings with requireComplete flags only the blank clientId when the issuer is valid", () => { + expect( + validateClientSettings( + { + emaEnabled: true, + issuer: "https://idp.test", + clientId: "", + clientSecret: "", + ...emptyCimd, + }, + { requireComplete: true }, + ), + ).toEqual({ clientId: CLIENT_ID_REQUIRED_ERROR }); + }); + + it("validateClientSettings with requireComplete flags a blank CIMD URL", () => { + expect( + validateClientSettings( + { + emaEnabled: false, + issuer: "", + clientId: "", + clientSecret: "", + cimdEnabled: true, + clientMetadataUrl: "", + }, + { requireComplete: true }, + ), + ).toEqual({ clientMetadataUrl: CLIENT_METADATA_URL_REQUIRED_ERROR }); + }); + + it("canPersistClientSettingsDraft blocks a blank clientId with a valid issuer", () => { + expect( + canPersistClientSettingsDraft({ + emaEnabled: true, + issuer: "https://idp.test", + clientId: "", + clientSecret: "", + ...emptyCimd, + }), + ).toBe(false); + }); }); diff --git a/clients/web/src/components/groups/ClientSettingsForm/clientSettingsValues.ts b/clients/web/src/components/groups/ClientSettingsForm/clientSettingsValues.ts index ff468479..5e592368 100644 --- a/clients/web/src/components/groups/ClientSettingsForm/clientSettingsValues.ts +++ b/clients/web/src/components/groups/ClientSettingsForm/clientSettingsValues.ts @@ -7,35 +7,72 @@ import { /** Field-level error message for an issuer that is not an http(s) URL. */ export const ISSUER_URL_ERROR = "Must be an http(s) URL, like https://idp.example.com"; +/** Field-level errors for a required field left blank while its feature is enabled. */ +export const ISSUER_REQUIRED_ERROR = + "Issuer is required when enterprise IdP configuration is enabled"; +export const CLIENT_ID_REQUIRED_ERROR = + "Client ID is required when enterprise IdP configuration is enabled"; +export const CLIENT_METADATA_URL_REQUIRED_ERROR = + "Metadata document URL is required when CIMD is enabled"; /** Field-level validation errors for the client settings form. */ export interface ClientSettingsErrors { issuer?: string; + clientId?: string; clientMetadataUrl?: string; } +export interface ValidateClientSettingsOptions { + /** + * Also flag required fields left blank. Off by default so inline (as-you-type) + * validation only flags a field the user has actually filled in wrong; turned + * on when a save/close is attempted so an incomplete config is surfaced rather + * than silently dropped. + */ + requireComplete?: boolean; +} + /** - * Inline validation for the client settings form. Only flags fields that the - * user has actually filled in — empty required fields are gated by - * {@link canPersistClientSettingsDraft} rather than surfaced as errors. + * Validation for the client settings form. By default only flags fields the + * user has filled in wrong (a non-empty, non-http(s) issuer; a malformed CIMD + * metadata URL). With `requireComplete`, also flags the required fields of an + * enabled feature (EMA's issuer/clientId, CIMD's metadata URL) when they are + * blank — used at save/close time so an enabled-but-incomplete config never + * persists silently. */ export function validateClientSettings( values: ClientSettingsFormValues, + options: ValidateClientSettingsOptions = {}, ): ClientSettingsErrors { const errors: ClientSettingsErrors = {}; - if ( - values.emaEnabled && - values.issuer.trim() !== "" && - !isAbsoluteHttpUrl(values.issuer) - ) { - errors.issuer = ISSUER_URL_ERROR; - } - if (values.cimdEnabled && values.clientMetadataUrl.trim() !== "") { - const cimdError = getCimdClientMetadataUrlError(values.clientMetadataUrl); - if (cimdError) { - errors.clientMetadataUrl = cimdError; + + if (values.emaEnabled) { + const issuer = values.issuer.trim(); + if (issuer === "") { + if (options.requireComplete) errors.issuer = ISSUER_REQUIRED_ERROR; + } else if (!isAbsoluteHttpUrl(values.issuer)) { + errors.issuer = ISSUER_URL_ERROR; + } + + if (options.requireComplete && values.clientId.trim() === "") { + errors.clientId = CLIENT_ID_REQUIRED_ERROR; } } + + if (values.cimdEnabled) { + const url = values.clientMetadataUrl.trim(); + if (url === "") { + if (options.requireComplete) { + errors.clientMetadataUrl = CLIENT_METADATA_URL_REQUIRED_ERROR; + } + } else { + const cimdError = getCimdClientMetadataUrlError(values.clientMetadataUrl); + if (cimdError) { + errors.clientMetadataUrl = cimdError; + } + } + } + return errors; } @@ -108,18 +145,18 @@ export function formValuesToClientConfig( return result; } -/** Skip debounced persist while required fields are blank for enabled features. */ +/** + * Skip the debounced persist while an enabled feature's required fields are + * blank or invalid (EMA's issuer/clientId, CIMD's metadata URL). Defers to + * {@link validateClientSettings} in `requireComplete` mode so the persist gate + * and the close-time field errors can never drift: the same condition that + * blocks the write also surfaces the field-level errors that explain why. + */ export function canPersistClientSettingsDraft( values: ClientSettingsFormValues, ): boolean { - if (values.emaEnabled) { - if (values.issuer.trim() === "" || values.clientId.trim() === "") - return false; - } - if (values.cimdEnabled) { - if (!values.clientMetadataUrl.trim()) return false; - } - // Defer to validateClientSettings so the persist gate and inline field errors - // can never drift — invalid values are never sent to the backend. - return Object.keys(validateClientSettings(values)).length === 0; + return ( + Object.keys(validateClientSettings(values, { requireComplete: true })) + .length === 0 + ); } diff --git a/clients/web/src/components/groups/ClientSettingsModal/ClientSettingsModal.test.tsx b/clients/web/src/components/groups/ClientSettingsModal/ClientSettingsModal.test.tsx index 851efb18..5b2a2740 100644 --- a/clients/web/src/components/groups/ClientSettingsModal/ClientSettingsModal.test.tsx +++ b/clients/web/src/components/groups/ClientSettingsModal/ClientSettingsModal.test.tsx @@ -6,6 +6,8 @@ import { ClientSettingsModal } from "./ClientSettingsModal"; import { EMPTY_CLIENT_SETTINGS, ISSUER_URL_ERROR, + CLIENT_ID_REQUIRED_ERROR, + CLIENT_METADATA_URL_REQUIRED_ERROR, type ClientSettingsFormValues, } from "../ClientSettingsForm/clientSettingsValues"; import { CIMD_METADATA_URL_INVALID_ERROR } from "@inspector/core/client/config-parse.js"; @@ -150,6 +152,9 @@ describe("ClientSettingsModal", () => { ).toBeInTheDocument(); }); + const clickClose = (user: ReturnType) => + user.click(document.querySelector("button.mantine-CloseButton-root")!); + it("blocks close and reveals the issuer error when the issuer is invalid", async () => { const user = userEvent.setup(); const onClose = vi.fn(); @@ -160,6 +165,7 @@ describe("ClientSettingsModal", () => { ...EMPTY_CLIENT_SETTINGS, emaEnabled: true, issuer: "not-a-url", + clientId: "client-1", }} onClose={onClose} onSettingsChange={vi.fn()} @@ -169,13 +175,35 @@ describe("ClientSettingsModal", () => { // Error is hidden initially (form gates it on blur)... expect(screen.queryByText(ISSUER_URL_ERROR)).not.toBeInTheDocument(); // ...but a close attempt reveals it and does NOT close (no silent drop). - await user.click( - document.querySelector("button.mantine-CloseButton-root")!, - ); + await clickClose(user); expect(onClose).not.toHaveBeenCalled(); expect(screen.getByText(ISSUER_URL_ERROR)).toBeInTheDocument(); }); + it("blocks close and reveals the required error when the client ID is blank", async () => { + const user = userEvent.setup(); + const onClose = vi.fn(); + renderWithMantine( + , + ); + + expect( + screen.queryByText(CLIENT_ID_REQUIRED_ERROR), + ).not.toBeInTheDocument(); + await clickClose(user); + expect(onClose).not.toHaveBeenCalled(); + expect(screen.getByText(CLIENT_ID_REQUIRED_ERROR)).toBeInTheDocument(); + }); + it("allows close once the invalid issuer is corrected", async () => { const user = userEvent.setup(); const onClose = vi.fn(); @@ -184,6 +212,7 @@ describe("ClientSettingsModal", () => { ...EMPTY_CLIENT_SETTINGS, emaEnabled: true, issuer: "not-a-url", + clientId: "client-1", // complete except for the issuer }); return ( { } renderWithMantine(); - const close = () => - user.click(document.querySelector("button.mantine-CloseButton-root")!); - - await close(); // blocked, error revealed + await clickClose(user); // blocked, error revealed expect(onClose).not.toHaveBeenCalled(); const issuer = screen.getByLabelText("Issuer"); @@ -207,18 +233,18 @@ describe("ClientSettingsModal", () => { await user.type(issuer, "https://idp.test"); expect(screen.queryByText(ISSUER_URL_ERROR)).not.toBeInTheDocument(); - await close(); // now valid -> closes + await clickClose(user); // now complete + valid -> closes expect(onClose).toHaveBeenCalledTimes(1); }); - it("allows close when an invalid issuer is cleared (empty is valid to leave)", async () => { + it("allows close once a blank client ID is filled", async () => { const user = userEvent.setup(); const onClose = vi.fn(); function Host() { const [settings, setSettings] = useState({ ...EMPTY_CLIENT_SETTINGS, emaEnabled: true, - issuer: "not-a-url", + issuer: "https://idp.test", // complete except for the clientId }); return ( { } renderWithMantine(); - await user.clear(screen.getByLabelText("Issuer")); + await clickClose(user); // blocked on blank clientId + expect(onClose).not.toHaveBeenCalled(); + + await user.type(screen.getByLabelText("Client ID"), "client-1"); + await clickClose(user); // now complete -> closes + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it("allows close by disabling enterprise IdP when the config is incomplete", async () => { + const user = userEvent.setup(); + const onClose = vi.fn(); + function Host() { + const [settings, setSettings] = useState({ + ...EMPTY_CLIENT_SETTINGS, + emaEnabled: true, // enabled but issuer + clientId blank + }); + return ( + + ); + } + renderWithMantine(); + + await clickClose(user); // blocked: required fields blank + expect(onClose).not.toHaveBeenCalled(); + + // Disabling enterprise IdP is the escape hatch — nothing required to save. await user.click( - document.querySelector("button.mantine-CloseButton-root")!, + screen.getByRole("checkbox", { + name: "Enable enterprise IdP configuration", + }), ); + await clickClose(user); expect(onClose).toHaveBeenCalledTimes(1); }); @@ -304,14 +363,40 @@ describe("ClientSettingsModal", () => { expect(onClose).toHaveBeenCalledTimes(1); }); - it("allows close when an invalid CIMD URL is cleared (empty is valid to leave)", async () => { + it("blocks close and reveals the required error when the CIMD URL is blank", async () => { + const user = userEvent.setup(); + const onClose = vi.fn(); + renderWithMantine( + , + ); + + expect( + screen.queryByText(CLIENT_METADATA_URL_REQUIRED_ERROR), + ).not.toBeInTheDocument(); + await user.click( + document.querySelector("button.mantine-CloseButton-root")!, + ); + expect(onClose).not.toHaveBeenCalled(); + expect( + screen.getByText(CLIENT_METADATA_URL_REQUIRED_ERROR), + ).toBeInTheDocument(); + }); + + it("allows close once a blank CIMD URL is filled", async () => { const user = userEvent.setup(); const onClose = vi.fn(); function Host() { const [settings, setSettings] = useState({ ...EMPTY_CLIENT_SETTINGS, - cimdEnabled: true, - clientMetadataUrl: "not-a-url", + cimdEnabled: true, // enabled but metadata URL blank }); return ( { } renderWithMantine(); + await user.click( + document.querySelector("button.mantine-CloseButton-root")!, + ); // blocked on blank URL + expect(onClose).not.toHaveBeenCalled(); + await expandCimdSection(user); - await user.clear(screen.getByLabelText("Client ID metadata document URL")); + await user.type( + screen.getByLabelText("Client ID metadata document URL"), + "https://example.com/cimd.json", + ); + await user.click( + document.querySelector("button.mantine-CloseButton-root")!, + ); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it("allows close by disabling CIMD when the URL is blank", async () => { + const user = userEvent.setup(); + const onClose = vi.fn(); + function Host() { + const [settings, setSettings] = useState({ + ...EMPTY_CLIENT_SETTINGS, + cimdEnabled: true, // enabled but metadata URL blank + }); + return ( + + ); + } + renderWithMantine(); + + await user.click( + document.querySelector("button.mantine-CloseButton-root")!, + ); // blocked: URL required + expect(onClose).not.toHaveBeenCalled(); + + // Disabling CIMD is the escape hatch — nothing required to save. + await expandCimdSection(user); + await user.click( + screen.getByRole("checkbox", { name: "Use Client ID Metadata Document" }), + ); await user.click( document.querySelector("button.mantine-CloseButton-root")!, ); diff --git a/clients/web/src/components/groups/ClientSettingsModal/ClientSettingsModal.tsx b/clients/web/src/components/groups/ClientSettingsModal/ClientSettingsModal.tsx index 3b454b29..adf956bd 100644 --- a/clients/web/src/components/groups/ClientSettingsModal/ClientSettingsModal.tsx +++ b/clients/web/src/components/groups/ClientSettingsModal/ClientSettingsModal.tsx @@ -37,9 +37,7 @@ export function ClientSettingsModal({ const [expandedSections, setExpandedSections] = useState< ClientSettingsSection[] >(["ema"]); - const [revealIssuerError, setRevealIssuerError] = useState(false); - const [revealClientMetadataUrlError, setRevealClientMetadataUrlError] = - useState(false); + const [revealErrors, setRevealErrors] = useState(false); const allExpanded = expandedSections.length === ALL_SECTIONS.length; @@ -47,21 +45,16 @@ export function ClientSettingsModal({ setExpandedSections(allExpanded ? [] : ALL_SECTIONS); } - // Closing (X / Esc / overlay) is the implicit "save" for this auto-saving - // modal — the parent flushes the debounced persist on close. If a validated - // URL field is invalid the persist gate would silently drop it, so instead of - // closing we reveal the field error (overriding the form's on-blur gating). - // The user can fix the URL or clear the field — an empty value is valid to - // leave — and then close. Resets via the parent's open/close remount key. + // modal — the parent flushes the debounced persist on close. A client config + // that's incomplete (blank required field) or invalid (bad issuer / CIMD URL) + // would be silently dropped by the persist gate, so instead of closing we + // reveal the field errors (overriding the form's on-blur gating) and keep the + // modal open. The user fixes the fields, or disables the feature, then closes. + // Resets via the parent's open/close remount key. function handleClose() { - const errors = validateClientSettings(settings); - if (errors.issuer) { - setRevealIssuerError(true); - } - if (errors.clientMetadataUrl) { - setRevealClientMetadataUrlError(true); - } - if (errors.issuer || errors.clientMetadataUrl) { + const errors = validateClientSettings(settings, { requireComplete: true }); + if (Object.keys(errors).length > 0) { + setRevealErrors(true); return; } onClose(); @@ -94,8 +87,7 @@ export function ClientSettingsModal({ onSettingsChange={onSettingsChange} emaIdpLoginState={emaIdpLoginState} onEmaIdpLogout={onEmaIdpLogout} - revealIssuerError={revealIssuerError} - revealClientMetadataUrlError={revealClientMetadataUrlError} + revealErrors={revealErrors} />