diff --git a/changelog/7739-fix-edit-sso-provider-save-button.yaml b/changelog/7739-fix-edit-sso-provider-save-button.yaml new file mode 100644 index 00000000000..6f510f996da --- /dev/null +++ b/changelog/7739-fix-edit-sso-provider-save-button.yaml @@ -0,0 +1,4 @@ +type: Fixed +description: Fixed Edit SSO Provider Save button being permanently disabled after creating a provider +pr: 7739 +labels: [] diff --git a/clients/admin-ui/src/features/openid-authentication/SSOProviderForm.tsx b/clients/admin-ui/src/features/openid-authentication/SSOProviderForm.tsx index 7fb59a8c918..e56e416d45f 100644 --- a/clients/admin-ui/src/features/openid-authentication/SSOProviderForm.tsx +++ b/clients/admin-ui/src/features/openid-authentication/SSOProviderForm.tsx @@ -64,22 +64,28 @@ export const transformOrganizationToFormValues = ( openIDProvider: OpenIDProvider, ): SSOProviderFormValues => ({ ...openIDProvider }); -const SSOProviderFormValidationSchema = Yup.object().shape({ - provider: Yup.string().required().label("Provider"), - name: Yup.string().required().label("Name"), - client_id: Yup.string().required().label("Client ID"), - client_secret: Yup.string().required().label("Client Secret"), - scopes: Yup.array().of(Yup.string()).label("Scopes"), - verify_email: Yup.boolean().optional().label("Verify Email"), - verify_email_field: Yup.string() - .optional() - .nullable() - .label("Userinfo object verify email field"), - email_field: Yup.string() - .optional() - .nullable() - .label("Userinfo object email field"), -}); +export const getSSOProviderFormValidationSchema = (isEditMode: boolean) => + Yup.object().shape({ + provider: Yup.string().required().label("Provider"), + name: Yup.string().required().label("Name"), + client_id: isEditMode + ? Yup.string().optional().label("Client ID") + : Yup.string().required().label("Client ID"), + client_secret: isEditMode + ? Yup.string().optional().label("Client Secret") + : Yup.string().required().label("Client Secret"), + // nullable — the API type allows null for scopes so that the form save button is not disabled (Array | null) + scopes: Yup.array().of(Yup.string()).nullable().label("Scopes"), + verify_email: Yup.boolean().optional().label("Verify Email"), + verify_email_field: Yup.string() + .optional() + .nullable() + .label("Userinfo object verify email field"), + email_field: Yup.string() + .optional() + .nullable() + .label("Userinfo object email field"), + }); const CustomProviderExtraFields = () => { const { @@ -183,6 +189,11 @@ const SSOProviderForm = ({ onSuccess, onClose, }: SSOProviderFormProps) => { + const isEditMode = !!openIDProvider; + const validationSchema = useMemo( + () => getSSOProviderFormValidationSchema(isEditMode), + [isEditMode], + ); const [createOpenIDProviderMutationTrigger] = useCreateOpenIDProviderMutation(); const [updateOpenIDProviderMutation] = useUpdateOpenIDProviderMutation(); @@ -221,8 +232,20 @@ const SSOProviderForm = ({ } } }; - if (initialValues.id) { - const result = await updateOpenIDProviderMutation(values); + if (isEditMode) { + // Strip empty credential fields — the backend preserves existing encrypted + // values when client_id / client_secret are absent from the payload. + const { + client_id: clientId, + client_secret: clientSecret, + ...rest + } = values; + const payload = { + ...rest, + ...(clientId ? { client_id: clientId } : {}), + ...(clientSecret ? { client_secret: clientSecret } : {}), + }; + const result = await updateOpenIDProviderMutation(payload); handleResult(result); } else { const result = await createOpenIDProviderMutationTrigger(values); @@ -274,7 +297,7 @@ const SSOProviderForm = ({ initialValues={initialValues} enableReinitialize onSubmit={handleSubmit} - validationSchema={SSOProviderFormValidationSchema} + validationSchema={validationSchema} > {({ dirty, isValid, values }) => (
@@ -293,7 +316,7 @@ const SSOProviderForm = ({ tooltip="Unique identifier for your provider" variant="stacked" isRequired - disabled={!!initialValues.id} + disabled={isEditMode} /> {values.provider === "azure" && renderAzureProviderExtraFields()} {values.provider === "okta" && renderOktaProviderExtraFields()} diff --git a/clients/admin-ui/src/features/openid-authentication/__tests__/SSOProviderForm.test.tsx b/clients/admin-ui/src/features/openid-authentication/__tests__/SSOProviderForm.test.tsx new file mode 100644 index 00000000000..565f98c9470 --- /dev/null +++ b/clients/admin-ui/src/features/openid-authentication/__tests__/SSOProviderForm.test.tsx @@ -0,0 +1,70 @@ +import { getSSOProviderFormValidationSchema } from "../SSOProviderForm"; + +const BASE_VALUES = { + provider: "google", + name: "My Provider", +}; + +describe("getSSOProviderFormValidationSchema", () => { + describe("create mode", () => { + const schema = getSSOProviderFormValidationSchema(false); + + it("requires client_id", async () => { + await expect( + schema.validateAt("client_id", { ...BASE_VALUES, client_id: "" }), + ).rejects.toThrow("Client ID is a required field"); + }); + + it("requires client_secret", async () => { + await expect( + schema.validateAt("client_secret", { + ...BASE_VALUES, + client_secret: "", + }), + ).rejects.toThrow("Client Secret is a required field"); + }); + + it("passes when credentials are provided", async () => { + await expect( + schema.validate({ + ...BASE_VALUES, + client_id: "my-client-id", + client_secret: "my-secret", + }), + ).resolves.toBeDefined(); + }); + }); + + describe("edit mode", () => { + const schema = getSSOProviderFormValidationSchema(true); + + it("allows empty client_id", async () => { + await expect( + schema.validateAt("client_id", { ...BASE_VALUES, client_id: "" }), + ).resolves.toBeDefined(); + }); + + it("allows empty client_secret", async () => { + await expect( + schema.validateAt("client_secret", { + ...BASE_VALUES, + client_secret: "", + }), + ).resolves.toBeDefined(); + }); + + it("allows undefined client_id and client_secret", async () => { + await expect(schema.validate(BASE_VALUES)).resolves.toBeDefined(); + }); + + it("accepts provided credentials (rotation)", async () => { + await expect( + schema.validate({ + ...BASE_VALUES, + client_id: "new-client-id", + client_secret: "new-secret", + }), + ).resolves.toBeDefined(); + }); + }); +});