diff --git a/frontend/pages/admin/IntegrationsPage/cards/AccountProvisioning/AccountProvisioning.tests.tsx b/frontend/pages/admin/IntegrationsPage/cards/AccountProvisioning/AccountProvisioning.tests.tsx index f8f0467e26..2fbf795f24 100644 --- a/frontend/pages/admin/IntegrationsPage/cards/AccountProvisioning/AccountProvisioning.tests.tsx +++ b/frontend/pages/admin/IntegrationsPage/cards/AccountProvisioning/AccountProvisioning.tests.tsx @@ -4,10 +4,22 @@ import { screen, waitFor } from "@testing-library/react"; import { createCustomRenderer, createMockRouter } from "test/test-utils"; import createMockConfig from "__mocks__/configMock"; import createMockLicense from "__mocks__/licenseMock"; +import configAPI from "services/entities/config"; +import { notify } from "components/ToastNotification"; import { IAppConfigFormProps } from "pages/admin/OrgSettingsPage/cards/constants"; import AccountProvisioning from "./AccountProvisioning"; +jest.mock("services/entities/config"); +jest.mock("components/ToastNotification", () => ({ + notify: { + success: jest.fn(), + error: jest.fn(), + batch: jest.fn(), + dismiss: jest.fn(), + }, +})); + const defaultProps: IAppConfigFormProps = { appConfig: createMockConfig({ license: createMockLicense({ tier: "premium" }), @@ -16,11 +28,30 @@ const defaultProps: IAppConfigFormProps = { router: createMockRouter(), }; +const savedConfigProps: IAppConfigFormProps = { + ...defaultProps, + appConfig: createMockConfig({ + license: createMockLicense({ tier: "premium" }), + mdm: { + ...createMockConfig().mdm, + apple_account_provisioning: { + oauth_idp_token_url: "https://example.okta.com/oauth2/v1/token", + oauth_idp_client_id: "my-client-id", + oauth_idp_client_secret: "********", + }, + }, + }), +}; + describe("AccountProvisioning", () => { const render = createCustomRenderer({ withBackendMock: true, }); + afterEach(() => { + jest.clearAllMocks(); + }); + it("renders the section heading", () => { render(); expect(screen.getByText("Account provisioning")).toBeInTheDocument(); @@ -88,7 +119,23 @@ describe("AccountProvisioning", () => { await user.type(screen.getByLabelText(/token url/i), "not-a-url"); await user.tab(); await waitFor(() => { - expect(screen.getByText(/must be a valid url/i)).toBeInTheDocument(); + expect( + screen.getByText(/must be a valid https url/i) + ).toBeInTheDocument(); + }); + }); + + it("shows an invalid URL error on blur when the URL is not https", async () => { + const { user } = render(); + await user.type( + screen.getByLabelText(/token url/i), + "http://example.okta.com/oauth2/v1/token" + ); + await user.tab(); + await waitFor(() => { + expect( + screen.getByText(/must be a valid https url/i) + ).toBeInTheDocument(); }); }); @@ -97,7 +144,9 @@ describe("AccountProvisioning", () => { await user.type(screen.getByLabelText(/token url/i), "not-a-url"); await user.tab(); await waitFor(() => { - expect(screen.getByText(/must be a valid url/i)).toBeInTheDocument(); + expect( + screen.getByText(/must be a valid https url/i) + ).toBeInTheDocument(); }); // After the error shows, FormField replaces the label text with the error // message, so we locate the input by its placeholder instead. @@ -111,7 +160,7 @@ describe("AccountProvisioning", () => { ); await waitFor(() => { expect( - screen.queryByText(/must be a valid url/i) + screen.queryByText(/must be a valid https url/i) ).not.toBeInTheDocument(); }); }); @@ -164,7 +213,145 @@ describe("AccountProvisioning", () => { ); await user.click(screen.getByRole("button", { name: /save/i })); await waitFor(() => { - expect(screen.getByText(/must be a valid url/i)).toBeInTheDocument(); + expect( + screen.getByText(/must be a valid https url/i) + ).toBeInTheDocument(); + }); + expect(configAPI.update).not.toHaveBeenCalled(); + }); + }); + + describe("Editing the token URL of a saved configuration", () => { + it("clears the masked client secret and flags it for re-entry", async () => { + const { user } = render(); + await user.type(screen.getByLabelText(/token url/i), "x"); + + // the error replaces the "Client secret" label text + const secretInput = screen.getByLabelText( + /client secret must be re-entered/i + ); + expect(secretInput).toHaveValue(""); + }); + + it("does not clear a client secret the user has already re-entered", async () => { + const { user } = render(); + const secretInput = screen.getByLabelText(/client secret/i); + await user.clear(secretInput); + await user.type(secretInput, "new-secret"); + + await user.type(screen.getByLabelText(/token url/i), "x"); + + expect(secretInput).toHaveValue("new-secret"); + expect( + screen.queryByText(/client secret must be re-entered/i) + ).not.toBeInTheDocument(); + }); + + it("keeps the masked client secret when only the client ID is edited", async () => { + const { user } = render(); + await user.type(screen.getByLabelText(/client id/i), "x"); + expect(screen.getByLabelText(/client secret/i)).toHaveValue("********"); + }); + + it("blocks submission until the client secret is re-entered", async () => { + const { user } = render(); + await user.type(screen.getByLabelText(/token url/i), "x"); + await user.click(screen.getByRole("button", { name: /save/i })); + await waitFor(() => { + expect( + screen.getByText(/client secret is required/i) + ).toBeInTheDocument(); + }); + expect(configAPI.update).not.toHaveBeenCalled(); + + // the error replaces the "Client secret" label text + const secretInput = screen.getByLabelText(/client secret is required/i); + await user.type(secretInput, "new-secret"); + await user.click(screen.getByRole("button", { name: /save/i })); + + await waitFor(() => { + expect(configAPI.update).toHaveBeenCalledWith({ + mdm: { + apple_account_provisioning: { + oauth_idp_token_url: "https://example.okta.com/oauth2/v1/tokenx", + oauth_idp_client_id: "my-client-id", + oauth_idp_client_secret: "new-secret", + }, + }, + }); + }); + }); + }); + + describe("Server errors", () => { + const fillValidForm = async (user: ReturnType["user"]) => { + await user.type( + screen.getByLabelText(/token url/i), + "https://example.okta.com/oauth2/v1/token" + ); + await user.type(screen.getByLabelText(/client id/i), "my-client-id"); + await user.type(screen.getByLabelText(/client secret/i), "my-secret"); + await user.click(screen.getByRole("button", { name: /save/i })); + }; + + it("surfaces field-level server errors inline on the matching fields and in the toast", async () => { + (configAPI.update as jest.Mock).mockRejectedValue({ + status: 422, + data: { + message: "Validation Failed", + errors: [ + { + name: "mdm.apple_account_provisioning.oauth_idp_client_secret", + reason: + "oauth_idp_client_secret must be provided when changing oauth_idp_token_url", + }, + { + name: "mdm.apple_account_provisioning.oauth_idp_token_url", + reason: "must be a valid https URL", + }, + ], + }, + }); + + const { user } = render(); + await fillValidForm(user); + + await waitFor(() => { + expect( + screen.getByText(/must be provided when changing/i) + ).toBeInTheDocument(); + }); + expect( + screen.getByText(/must be a valid https url/i) + ).toBeInTheDocument(); + expect(notify.error).toHaveBeenCalledWith( + expect.stringContaining("must be provided when changing"), + expect.anything() + ); + }); + + it("includes the server reason in the error toast for non-field errors", async () => { + (configAPI.update as jest.Mock).mockRejectedValue({ + status: 422, + data: { + message: "Validation Failed", + errors: [ + { + name: "mdm.apple_account_provisioning", + reason: "Missing required private key", + }, + ], + }, + }); + + const { user } = render(); + await fillValidForm(user); + + await waitFor(() => { + expect(notify.error).toHaveBeenCalledWith( + expect.stringContaining("Missing required private key"), + expect.anything() + ); }); }); }); diff --git a/frontend/pages/admin/IntegrationsPage/cards/AccountProvisioning/AccountProvisioning.tsx b/frontend/pages/admin/IntegrationsPage/cards/AccountProvisioning/AccountProvisioning.tsx index e2bd8aeec8..24f682d5ab 100644 --- a/frontend/pages/admin/IntegrationsPage/cards/AccountProvisioning/AccountProvisioning.tsx +++ b/frontend/pages/admin/IntegrationsPage/cards/AccountProvisioning/AccountProvisioning.tsx @@ -7,6 +7,7 @@ import { UNCHANGED_PASSWORD_API_RESPONSE, } from "utilities/constants"; import configAPI from "services/entities/config"; +import { getErrorReason } from "interfaces/errors"; import { notify } from "components/ToastNotification"; import { IAppConfigFormProps } from "pages/admin/OrgSettingsPage/cards/constants"; @@ -41,11 +42,9 @@ const validate = (formData: IFormData): IFormErrors => { if (!formData.tokenUrl) { errors.tokenUrl = "Token URL is required."; - } else if ( - !validUrl({ url: formData.tokenUrl, protocols: ["http", "https"] }) - ) { + } else if (!validUrl({ url: formData.tokenUrl, protocols: ["https"] })) { errors.tokenUrl = - "Must be a valid URL (e.g. https://yourdomain.okta.com/oauth2/v1/token)"; + "Must be a valid https URL (e.g. https://yourdomain.okta.com/oauth2/v1/token)"; } if (!formData.clientId) { @@ -59,6 +58,25 @@ const validate = (formData: IFormData): IFormErrors => { return errors; }; +const SERVER_ERROR_NAMES: Record = { + tokenUrl: "mdm.apple_account_provisioning.oauth_idp_token_url", + clientId: "mdm.apple_account_provisioning.oauth_idp_client_id", + clientSecret: "mdm.apple_account_provisioning.oauth_idp_client_secret", +}; + +const getServerFieldErrors = (err: unknown): IFormErrors => { + const errors: IFormErrors = {}; + (Object.keys(SERVER_ERROR_NAMES) as (keyof IFormData)[]).forEach((field) => { + const reason = getErrorReason(err, { + nameEquals: SERVER_ERROR_NAMES[field], + }); + if (reason) { + errors[field] = reason; + } + }); + return errors; +}; + const AccountProvisioning = ({ appConfig }: IAppConfigFormProps) => { const { gitOpsModeEnabled } = useGitOpsMode(); const queryClient = useQueryClient(); @@ -83,15 +101,33 @@ const AccountProvisioning = ({ appConfig }: IAppConfigFormProps) => { const onInputChange = ({ name, value }: IInputFieldParseTarget) => { const newFormData = { ...formData, [name]: value }; - setFormData(newFormData); - // only update errors for fields that already have an error - if (formErrors[name as keyof IFormErrors]) { - const newErrors = validate(newFormData); - setFormErrors((prev) => ({ - ...prev, - [name]: newErrors[name as keyof IFormErrors], - })); + + // The server rejects a token URL change that reuses the stored secret + // (the secret would be sent to the new, possibly hostile, URL), so clear + // the masked secret and have the user re-enter it. Same pattern as + // editing a certificate authority. + const secretCleared = + name === "tokenUrl" && + formData.clientSecret === UNCHANGED_PASSWORD_API_RESPONSE; + if (secretCleared) { + newFormData.clientSecret = ""; } + + setFormData(newFormData); + setFormErrors((prev) => { + const next = { ...prev }; + if (secretCleared) { + next.clientSecret = + "Client secret must be re-entered when changing the token URL."; + } + // only update errors for fields that already have an error + if (prev[name as keyof IFormErrors]) { + next[name as keyof IFormErrors] = validate(newFormData)[ + name as keyof IFormErrors + ]; + } + return next; + }); }; const onInputBlur = (field: keyof IFormData) => () => { @@ -127,8 +163,15 @@ const AccountProvisioning = ({ appConfig }: IAppConfigFormProps) => { }); await queryClient.invalidateQueries(["config"]); notify.success("Successfully updated settings."); - } catch { - notify.error("Failed to update settings."); + } catch (err) { + setFormErrors((prev) => ({ ...prev, ...getServerFieldErrors(err) })); + const reason = getErrorReason(err); + notify.error( + reason + ? `Failed to update settings: ${reason}` + : "Failed to update settings.", + { response: err } + ); } finally { setIsUpdating(false); }