HCA: add dirty state to CA forms and backend validation for empty update payload (#32642)

fixes: #32608 

# Checklist for submitter

## Testing

- [x] Added/updated automated tests
- [x] QA'd all new/changed functionality manually
This commit is contained in:
Magnus Jensen 2025-09-05 16:05:09 +03:00 committed by GitHub
parent ebd32fa2f4
commit a1f168ac84
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 211 additions and 9 deletions

View file

@ -866,6 +866,10 @@ func (svc *Service) UpdateCertificateAuthority(ctx context.Context, id uint, p f
var caActivityName string
if p.DigiCertCAUpdatePayload != nil {
if p.DigiCertCAUpdatePayload.IsEmpty() {
return &fleet.BadRequestError{Message: fmt.Sprintf("%sDigiCert CA update payload is empty", errPrefix)}
}
if err := p.DigiCertCAUpdatePayload.ValidateRelatedFields(errPrefix, *oldCA.Name); err != nil {
return err
}
@ -890,6 +894,10 @@ func (svc *Service) UpdateCertificateAuthority(ctx context.Context, id uint, p f
activity = fleet.ActivityEditedDigiCert{Name: caActivityName}
}
if p.HydrantCAUpdatePayload != nil {
if p.HydrantCAUpdatePayload.IsEmpty() {
return &fleet.BadRequestError{Message: fmt.Sprintf("%sHydrant CA update payload is empty", errPrefix)}
}
if err := p.HydrantCAUpdatePayload.ValidateRelatedFields(errPrefix, *oldCA.Name); err != nil {
return err
}
@ -910,6 +918,10 @@ func (svc *Service) UpdateCertificateAuthority(ctx context.Context, id uint, p f
activity = fleet.ActivityEditedHydrant{Name: caActivityName}
}
if p.NDESSCEPProxyCAUpdatePayload != nil {
if p.NDESSCEPProxyCAUpdatePayload.IsEmpty() {
return &fleet.BadRequestError{Message: fmt.Sprintf("%sNDES SCEP Proxy CA update payload is empty", errPrefix)}
}
if err := p.NDESSCEPProxyCAUpdatePayload.ValidateRelatedFields(errPrefix, *oldCA.Name); err != nil {
return err
}
@ -930,6 +942,10 @@ func (svc *Service) UpdateCertificateAuthority(ctx context.Context, id uint, p f
activity = fleet.ActivityEditedNDESSCEPProxy{}
}
if p.CustomSCEPProxyCAUpdatePayload != nil {
if p.CustomSCEPProxyCAUpdatePayload.IsEmpty() {
return &fleet.BadRequestError{Message: fmt.Sprintf("%sCustom SCEP Proxy CA update payload is empty", errPrefix)}
}
if err := p.CustomSCEPProxyCAUpdatePayload.ValidateRelatedFields(errPrefix, *oldCA.Name); err != nil {
return err
}

View file

@ -1009,6 +1009,30 @@ func TestUpdatingCertificateAuthorities(t *testing.T) {
require.EqualError(t, err, "Couldn't edit certificate authority. Certificate authority with ID 999 does not exist.")
})
t.Run("Errors on empty inner update payload", func(t *testing.T) {
svc, ctx := baseSetupForCATests()
payloadMap := map[uint]fleet.CertificateAuthorityUpdatePayload{
digicertID: {
DigiCertCAUpdatePayload: &fleet.DigiCertCAUpdatePayload{},
},
hydrantID: {
HydrantCAUpdatePayload: &fleet.HydrantCAUpdatePayload{},
},
scepID: {
CustomSCEPProxyCAUpdatePayload: &fleet.CustomSCEPProxyCAUpdatePayload{},
},
ndesID: {
NDESSCEPProxyCAUpdatePayload: &fleet.NDESSCEPProxyCAUpdatePayload{},
},
}
for id, payload := range payloadMap {
err := svc.UpdateCertificateAuthority(ctx, id, payload)
require.Contains(t, err.Error(), "update payload is empty")
}
})
t.Run("Errors on wrong existing CA type", func(t *testing.T) {
svc, ctx := baseSetupForCATests()

View file

@ -40,7 +40,7 @@ describe("CustomSCEPForm", () => {
/>
);
// data is valid, submit should be enabled
// data is valid, so submit should be enabled
expect(screen.getByRole("button", { name: "Submit" })).toBeEnabled();
// name input is invalidated, submit should be disabled
@ -62,4 +62,36 @@ describe("CustomSCEPForm", () => {
expect(screen.getByRole("button", { name: "Submit" })).toBeDisabled();
});
it("submit button is disabled if isDirty is false", () => {
render(
<CustomSCEPForm
formData={createTestFormData()}
isSubmitting={false}
submitBtnText="Submit"
isDirty={false}
onChange={noop}
onSubmit={noop}
onCancel={noop}
/>
);
expect(screen.getByRole("button", { name: "Submit" })).toBeDisabled();
});
it("submit button is enabled if isDirty", () => {
render(
<CustomSCEPForm
formData={createTestFormData()}
isSubmitting={false}
submitBtnText="Submit"
isDirty
onChange={noop}
onSubmit={noop}
onCancel={noop}
/>
);
expect(screen.getByRole("button", { name: "Submit" })).toBeEnabled();
});
});

View file

@ -27,6 +27,7 @@ interface ICustomSCEPFormProps {
submitBtnText: string;
isSubmitting: boolean;
isEditing?: boolean;
isDirty?: boolean;
onChange: (update: { name: string; value: string }) => void;
onSubmit: () => void;
onCancel: () => void;
@ -38,6 +39,7 @@ const CustomSCEPForm = ({
submitBtnText,
isSubmitting,
isEditing = false,
isDirty = true,
onChange,
onSubmit,
onCancel,
@ -113,7 +115,7 @@ const CustomSCEPForm = ({
<Button
type="submit"
isLoading={isSubmitting}
disabled={!formValidation.isValid || isSubmitting}
disabled={!formValidation.isValid || isSubmitting || !isDirty}
>
{submitBtnText}
</Button>

View file

@ -44,7 +44,7 @@ describe("DigicertForm", () => {
/>
);
// data is valid, submit should be enabled
// data is valid, so submit should be enabled
expect(screen.getByRole("button", { name: "Submit" })).toBeEnabled();
// name input is invalidated, submit should be disabled
@ -66,4 +66,36 @@ describe("DigicertForm", () => {
expect(screen.getByRole("button", { name: "Submit" })).toBeDisabled();
});
it("submit button is disabled if isDirty is false", () => {
render(
<DigicertForm
formData={createTestFormData()}
isSubmitting={false}
submitBtnText="Submit"
isDirty={false}
onChange={noop}
onSubmit={noop}
onCancel={noop}
/>
);
expect(screen.getByRole("button", { name: "Submit" })).toBeDisabled();
});
it("submit button is enabled if isDirty", () => {
render(
<DigicertForm
formData={createTestFormData()}
isSubmitting={false}
submitBtnText="Submit"
isDirty
onChange={noop}
onSubmit={noop}
onCancel={noop}
/>
);
expect(screen.getByRole("button", { name: "Submit" })).toBeEnabled();
});
});

View file

@ -31,6 +31,7 @@ interface IDigicertFormProps {
submitBtnText: string;
isSubmitting: boolean;
isEditing?: boolean;
isDirty?: boolean;
onChange: (update: { name: string; value: string }) => void;
onSubmit: () => void;
onCancel: () => void;
@ -42,6 +43,7 @@ const DigicertForm = ({
submitBtnText,
isSubmitting,
isEditing = false,
isDirty = true,
onChange,
onSubmit,
onCancel,
@ -166,7 +168,7 @@ const DigicertForm = ({
>
<Button
isLoading={isSubmitting}
disabled={!formValidation.isValid || isSubmitting}
disabled={!formValidation.isValid || isSubmitting || !isDirty}
type="submit"
>
{submitBtnText}

View file

@ -36,6 +36,7 @@ const EditCertAuthorityModal = ({
}: IEditCertAuthorityModalProps) => {
const { renderFlash } = useContext(NotificationContext);
const [isUpdating, setIsUpdating] = useState(false);
const [isDirty, setIsDirty] = useState(false);
const [formData, setFormData] = useState<ICertFormData | undefined>();
const { data: fullCertAuthority, isLoading, isError } = useQuery(
@ -55,6 +56,8 @@ const EditCertAuthorityModal = ({
if (!prevFormData) return prevFormData;
return updateFormData(fullCertAuthority, prevFormData, update);
});
setIsDirty(true);
};
const onEditCertAuthority = async () => {
@ -111,6 +114,7 @@ const EditCertAuthorityModal = ({
submitBtnText="Save"
isSubmitting={isUpdating}
isEditing
isDirty={isDirty}
onChange={onChangeForm}
onSubmit={onEditCertAuthority}
onCancel={onExit}

View file

@ -41,7 +41,7 @@ describe("HydrantForm", () => {
/>
);
// data is valid, submit should be enabled
// data is valid, so submit should be enabled
expect(screen.getByRole("button", { name: "Submit" })).toBeEnabled();
// name input is invalidated, submit should be disabled
@ -63,4 +63,36 @@ describe("HydrantForm", () => {
expect(screen.getByRole("button", { name: "Submit" })).toBeDisabled();
});
it("submit button is disabled if isDirty is false", () => {
render(
<HydrantForm
formData={createTestFormData()}
isSubmitting={false}
submitBtnText="Submit"
isDirty={false}
onChange={noop}
onSubmit={noop}
onCancel={noop}
/>
);
expect(screen.getByRole("button", { name: "Submit" })).toBeDisabled();
});
it("submit button is enabled if isDirty", () => {
render(
<HydrantForm
formData={createTestFormData()}
isSubmitting={false}
submitBtnText="Submit"
isDirty
onChange={noop}
onSubmit={noop}
onCancel={noop}
/>
);
expect(screen.getByRole("button", { name: "Submit" })).toBeEnabled();
});
});

View file

@ -27,6 +27,7 @@ interface IHydrantFormProps {
submitBtnText: string;
isSubmitting: boolean;
isEditing?: boolean;
isDirty?: boolean;
onChange: (update: { name: string; value: string }) => void;
onSubmit: () => void;
onCancel: () => void;
@ -38,6 +39,7 @@ const HydrantForm = ({
submitBtnText,
isSubmitting,
isEditing = false,
isDirty = true,
onChange,
onSubmit,
onCancel,
@ -118,7 +120,7 @@ const HydrantForm = ({
>
<Button
isLoading={isSubmitting}
disabled={!formValidation.isValid || isSubmitting}
disabled={!formValidation.isValid || isSubmitting || !isDirty}
type="submit"
>
{submitBtnText}

View file

@ -41,10 +41,10 @@ describe("NDESForm", () => {
/>
);
// data is valid, submit should be enabled
// data is valid, so submit should be enabled
expect(screen.getByRole("button", { name: "Submit" })).toBeEnabled();
// name input is invalidated, submit should be disabled
// scepURL input is invalidated, submit should be disabled
await user.clear(screen.getByLabelText("SCEP URL"));
expect(screen.getByRole("button", { name: "Submit" })).toBeDisabled();
});
@ -63,4 +63,36 @@ describe("NDESForm", () => {
expect(screen.getByRole("button", { name: "Submit" })).toBeDisabled();
});
it("submit button is disabled if isDirty is false", () => {
render(
<NDESForm
formData={createTestFormData()}
isSubmitting={false}
submitBtnText="Submit"
isDirty={false}
onChange={noop}
onSubmit={noop}
onCancel={noop}
/>
);
expect(screen.getByRole("button", { name: "Submit" })).toBeDisabled();
});
it("submit button is enabled if isDirty", () => {
render(
<NDESForm
formData={createTestFormData()}
isSubmitting={false}
submitBtnText="Submit"
isDirty
onChange={noop}
onSubmit={noop}
onCancel={noop}
/>
);
expect(screen.getByRole("button", { name: "Submit" })).toBeEnabled();
});
});

View file

@ -21,6 +21,7 @@ interface INDESFormProps {
submitBtnText: string;
isSubmitting: boolean;
isEditing?: boolean;
isDirty?: boolean;
onChange: (update: { name: string; value: string }) => void;
onSubmit: () => void;
onCancel: () => void;
@ -31,6 +32,7 @@ const NDESForm = ({
submitBtnText,
isSubmitting,
isEditing = false,
isDirty = true,
onChange,
onSubmit,
onCancel,
@ -107,7 +109,7 @@ const NDESForm = ({
<Button
type="submit"
isLoading={isSubmitting}
disabled={!formValidation.isValid || isSubmitting}
disabled={!formValidation.isValid || isSubmitting || !isDirty}
>
{submitBtnText}
</Button>

View file

@ -207,6 +207,13 @@ type DigiCertCAUpdatePayload struct {
CertificateSeatID *string `json:"certificate_seat_id"`
}
// IsEmpty checks if the struct only has all empty values
func (dcp DigiCertCAUpdatePayload) IsEmpty() bool {
return dcp.Name == nil && dcp.URL == nil && dcp.APIToken == nil &&
dcp.ProfileID == nil && dcp.CertificateCommonName == nil &&
dcp.CertificateUserPrincipalNames == nil && dcp.CertificateSeatID == nil
}
// ValidateRelatedFields verifies that fields that are related to each other are set correctly.
// For example if the URL is provided then the API token must also be provided.
func (dcp *DigiCertCAUpdatePayload) ValidateRelatedFields(errPrefix string, certName string) error {
@ -236,6 +243,11 @@ type NDESSCEPProxyCAUpdatePayload struct {
Password *string `json:"password"`
}
// IsEmpty checks if the struct only has all empty values
func (ndesp *NDESSCEPProxyCAUpdatePayload) IsEmpty() bool {
return ndesp.URL == nil && ndesp.AdminURL == nil && ndesp.Username == nil && ndesp.Password == nil
}
// ValidateRelatedFields verifies that fields that are related to each other are set correctly.
// For example if the Admin URL is provided then the Password must also be provided.
func (ndesp *NDESSCEPProxyCAUpdatePayload) ValidateRelatedFields(errPrefix string, certName string) error {
@ -264,6 +276,11 @@ type CustomSCEPProxyCAUpdatePayload struct {
Challenge *string `json:"challenge"`
}
// IsEmpty checks if the struct only has all empty values
func (cscepp CustomSCEPProxyCAUpdatePayload) IsEmpty() bool {
return cscepp.Name == nil && cscepp.URL == nil && cscepp.Challenge == nil
}
// ValidateRelatedFields verifies that fields that are related to each other are set correctly.
// For example if the Name is provided then the Challenge must also be provided.
func (cscepp *CustomSCEPProxyCAUpdatePayload) ValidateRelatedFields(errPrefix string, certName string) error {
@ -290,6 +307,11 @@ type HydrantCAUpdatePayload struct {
ClientSecret *string `json:"client_secret"`
}
// IsEmpty checks if the struct only has all empty values
func (hp HydrantCAUpdatePayload) IsEmpty() bool {
return hp.Name == nil && hp.URL == nil && hp.ClientID == nil && hp.ClientSecret == nil
}
// ValidateRelatedFields verifies that fields that are related to each other are set correctly.
// For example if the Name is provided then the Client ID and Client Secret must also be provided
func (hp *HydrantCAUpdatePayload) ValidateRelatedFields(errPrefix string, certName string) error {