Smallstep qa fixes (#33885)

This commit is contained in:
Magnus Jensen 2025-10-06 18:32:29 -03:00 committed by GitHub
parent b0866dcb10
commit 58c9df8796
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 20 additions and 23 deletions

View file

@ -1852,7 +1852,7 @@ func TestUpdatingCertificateAuthorities(t *testing.T) {
require.EqualError(t, err, "Couldn't edit certificate authority. Invalid challenge URL or credentials. Please correct and try again.")
})
t.Run("Requires all fields when updating URL", func(t *testing.T) {
t.Run("Requires password when updating SCEP URL", func(t *testing.T) {
svc, ctx := baseSetupForCATests()
payload := fleet.CertificateAuthorityUpdatePayload{
SmallstepSCEPProxyCAUpdatePayload: &fleet.SmallstepSCEPProxyCAUpdatePayload{
@ -1861,10 +1861,10 @@ func TestUpdatingCertificateAuthorities(t *testing.T) {
}
err := svc.UpdateCertificateAuthority(ctx, smallstepID, payload)
require.EqualError(t, err, "Couldn't edit certificate authority. \"challenge_url\", \"username\" and \"password\" must be set when modifying \"url\" of an existing certificate authority: Smallstep CA.")
require.EqualError(t, err, "Couldn't edit certificate authority. \"password\" must be set when modifying \"url\", \"challenge_url\", or \"username\" of an existing certificate authority: Smallstep CA")
})
t.Run("Requires password and username when updating challenge URL", func(t *testing.T) {
t.Run("Requires password when updating challenge URL", func(t *testing.T) {
svc, ctx := baseSetupForCATests()
payload := fleet.CertificateAuthorityUpdatePayload{
SmallstepSCEPProxyCAUpdatePayload: &fleet.SmallstepSCEPProxyCAUpdatePayload{
@ -1873,7 +1873,7 @@ func TestUpdatingCertificateAuthorities(t *testing.T) {
}
err := svc.UpdateCertificateAuthority(ctx, smallstepID, payload)
require.EqualError(t, err, "Couldn't edit certificate authority. \"username\" and \"password\" must be set when modifying \"challenge_url\" of an existing certificate authority: Smallstep CA.")
require.EqualError(t, err, "Couldn't edit certificate authority. \"password\" must be set when modifying \"url\", \"challenge_url\", or \"username\" of an existing certificate authority: Smallstep CA")
})
t.Run("Requires password when updating username", func(t *testing.T) {
@ -1885,7 +1885,7 @@ func TestUpdatingCertificateAuthorities(t *testing.T) {
}
err := svc.UpdateCertificateAuthority(ctx, smallstepID, payload)
require.EqualError(t, err, "Couldn't edit certificate authority. \"password\" must be set when modifying \"username\" of an existing certificate authority: Smallstep CA.")
require.EqualError(t, err, "Couldn't edit certificate authority. \"password\" must be set when modifying \"url\", \"challenge_url\", or \"username\" of an existing certificate authority: Smallstep CA")
})
t.Run("Validates password and username on update", func(t *testing.T) {

View file

@ -185,12 +185,14 @@ const NDES_PASSWORD_CACHE_FULL_ERROR =
"The NDES password cache is full. Please increase the number of cached passwords in NDES and try again.";
const INVALID_CHALLENGE_ERROR =
"Invalid challenge. Please correct and try again.";
const INVALID_CHALLENGE_URL_OR_CREDENTIALS_ERROR =
"Invalid challenge URL or credentials. Please correct and try again.";
/**
* Gets the error message we want to display from the api error message.
* This is used in both add and edit certificate authority flows.
*/
export const getDisplayErrMessage = (err: unknown) => {
export const getDisplayErrMessage = (err: unknown): string | JSX.Element => {
let message: string | JSX.Element = DEFAULT_ERROR;
const reason = getErrorReason(err).toLowerCase();
@ -211,6 +213,8 @@ export const getDisplayErrMessage = (err: unknown) => {
message = INVALID_ADMIN_URL_OR_CREDENTIALS_ERROR;
} else if (reason.includes("password cache is full")) {
message = NDES_PASSWORD_CACHE_FULL_ERROR;
} else if (reason.includes("invalid challenge url")) {
message = INVALID_CHALLENGE_URL_OR_CREDENTIALS_ERROR;
} else if (reason.includes("invalid challenge")) {
message = INVALID_CHALLENGE_ERROR;
} else {
@ -220,7 +224,7 @@ export const getDisplayErrMessage = (err: unknown) => {
return message;
};
export const getErrorMessage = (err: unknown) => {
export const getErrorMessage = (err: unknown): JSX.Element => {
return (
<>Couldn&apos;t add certificate authority. {getDisplayErrMessage(err)}</>
);

View file

@ -1,3 +1,5 @@
import React from "react";
import { IEditCertAuthorityBody } from "services/entities/certificates";
import {
ICertificateAuthority,
@ -150,7 +152,7 @@ export const generateEditCertAuthorityData = (
smallstep: deepDifference(
{
name: smallstepName,
scep_url: smallstepURL,
url: smallstepURL,
challenge_url: smallstepChallengeURL,
username: smallstepUsername,
password: smallstepPassword,
@ -255,6 +257,8 @@ export const updateFormData = (
return newData;
};
export const getErrorMessage = (err: unknown) => {
return `Couldn't edit certificate authority. ${getDisplayErrMessage(err)}`;
export const getErrorMessage = (err: unknown): JSX.Element => {
return (
<>Couldn&apos;t edit certificate authority. {getDisplayErrMessage(err)}</>
);
};

View file

@ -160,9 +160,6 @@ export const validateFormData = (
message: getErrorMessage(formData, failedValidation.message),
};
}
console.log("objKey", objKey);
console.log("formValidation", formValidation);
});
return formValidation;

View file

@ -385,16 +385,8 @@ func (sscepp SmallstepSCEPProxyCAUpdatePayload) IsEmpty() bool {
// ValidateRelatedFields verifies that fields that are related to each other are set correctly.
// For example if the Name is provided then the Challenge URL, Username and Password must also be provided
func (sscepp *SmallstepSCEPProxyCAUpdatePayload) ValidateRelatedFields(errPrefix string, certName string) error {
if sscepp.URL != nil && (sscepp.ChallengeURL == nil || sscepp.Username == nil || sscepp.Password == nil) {
return &BadRequestError{Message: fmt.Sprintf(`%s"challenge_url", "username" and "password" must be set when modifying "url" of an existing certificate authority: %s.`, errPrefix, certName)}
}
if sscepp.ChallengeURL != nil && sscepp.Username == nil && sscepp.Password == nil {
return &BadRequestError{Message: fmt.Sprintf(`%s"username" and "password" must be set when modifying "challenge_url" of an existing certificate authority: %s.`, errPrefix, certName)}
}
if sscepp.ChallengeURL == nil && sscepp.URL == nil && sscepp.Username != nil && sscepp.Password == nil {
return &BadRequestError{Message: fmt.Sprintf(`%s"password" must be set when modifying "username" of an existing certificate authority: %s.`, errPrefix, certName)}
if (sscepp.URL != nil || sscepp.ChallengeURL != nil || sscepp.Username != nil) && sscepp.Password == nil {
return &BadRequestError{Message: fmt.Sprintf(`%s"password" must be set when modifying "url", "challenge_url", or "username" of an existing certificate authority: %s`, errPrefix, certName)}
}
return nil