Validate username, password and challenge URL on any changes for Smallstep (#33501)

This commit is contained in:
Magnus Jensen 2025-09-26 17:24:48 +03:00 committed by GitHub
parent ee4fae8d69
commit 3a3a0ca480
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 52 additions and 24 deletions

View file

@ -1311,30 +1311,45 @@ func (svc *Service) validateSmallstepSCEPProxyUpdate(ctx context.Context, smalls
return &fleet.BadRequestError{Message: fmt.Sprintf("%sInvalid SCEP URL. Please correct and try again.", errPrefix)}
}
}
if smallstep.ChallengeURL != nil {
if err := validateURL(*smallstep.ChallengeURL, "Challenge", errPrefix); err != nil {
return err
}
// Call challenge URL to validate all fields are valid
if smallstep.ChallengeURL != nil || smallstep.Username != nil || smallstep.Password != nil {
// We want to generate a SmallsteSCEPProxyCA struct with all required fields to verify the admin URL.
// If URL, Username or Password are not being updated we use the existing values from oldCA
smallstepSCEPProxy := fleet.SmallstepSCEPProxyCA{
ChallengeURL: *smallstep.ChallengeURL,
}
ChallengeURL: *oldCa.ChallengeURL,
Username: *oldCa.Username,
Password: *oldCa.Password,
} // The object we are building to validate fields are valid.
if smallstep.URL != nil {
smallstepSCEPProxy.URL = *smallstep.URL
} else {
smallstepSCEPProxy.URL = *oldCa.URL
}
if smallstep.Username != nil {
smallstepSCEPProxy.Username = *smallstep.Username
} else {
smallstepSCEPProxy.Username = *oldCa.Username
// Additional validation if url was updated
if smallstep.ChallengeURL != nil {
if err := validateURL(*smallstep.ChallengeURL, "Challenge", errPrefix); err != nil {
return err
}
smallstepSCEPProxy.ChallengeURL = *smallstep.ChallengeURL
}
if smallstep.Username != nil {
if *smallstep.Username == "" {
return &fleet.BadRequestError{
Message: fmt.Sprintf("%sSmallstep SCEP Proxy username cannot be empty", errPrefix),
}
}
smallstepSCEPProxy.Username = *smallstep.Username
}
if smallstep.Password != nil {
if *smallstep.Password == "" {
return &fleet.BadRequestError{
Message: fmt.Sprintf("%sSmallstep SCEP Proxy password cannot be empty", errPrefix),
}
}
smallstepSCEPProxy.Password = *smallstep.Password
} else {
smallstepSCEPProxy.Password = *oldCa.Password
}
if err := svc.scepConfigService.ValidateSmallstepChallengeURL(ctx, smallstepSCEPProxy); err != nil {
@ -1342,16 +1357,7 @@ func (svc *Service) validateSmallstepSCEPProxyUpdate(ctx context.Context, smalls
return &fleet.BadRequestError{Message: fmt.Sprintf("%sInvalid challenge URL or credentials. Please correct and try again.", errPrefix)}
}
}
if smallstep.Username != nil && *smallstep.Username == "" {
return &fleet.BadRequestError{
Message: fmt.Sprintf("%sSmallstep SCEP Proxy username cannot be empty", errPrefix),
}
}
if smallstep.Password != nil && *smallstep.Password == "" {
return &fleet.BadRequestError{
Message: fmt.Sprintf("%sSmallstep SCEP Proxy password cannot be empty", errPrefix),
}
}
return nil
}

View file

@ -1888,6 +1888,28 @@ func TestUpdatingCertificateAuthorities(t *testing.T) {
require.EqualError(t, err, "Couldn't edit certificate authority. \"password\" must be set when modifying \"username\" of an existing certificate authority: Smallstep CA.")
})
t.Run("Validates password and username on update", func(t *testing.T) {
svc, ctx := baseSetupForCATests()
payload := fleet.CertificateAuthorityUpdatePayload{
SmallstepSCEPProxyCAUpdatePayload: &fleet.SmallstepSCEPProxyCAUpdatePayload{
Username: ptr.String("updated-username"),
Password: ptr.String("updated-password"),
},
}
mockScep := &scep_mock.SCEPConfigService{
ValidateSmallstepChallengeURLFunc: func(_ context.Context, _ fleet.SmallstepSCEPProxyCA) error {
return errors.New("bad password or username")
},
ValidateSmallstepChallengeURLFuncInvoked: false, // Reset so we can check it was invoked
}
svc.scepConfigService = mockScep
err := svc.UpdateCertificateAuthority(ctx, smallstepID, payload)
require.True(t, mockScep.ValidateSmallstepChallengeURLFuncInvoked)
require.EqualError(t, err, "Couldn't edit certificate authority. Invalid challenge URL or credentials. Please correct and try again.")
})
t.Run("Errors on empty username", func(t *testing.T) {
svc, ctx := baseSetupForCATests()
payload := fleet.CertificateAuthorityUpdatePayload{