diff --git a/ee/server/service/certificate_authorities.go b/ee/server/service/certificate_authorities.go index 3c85a2ac87..973580b89c 100644 --- a/ee/server/service/certificate_authorities.go +++ b/ee/server/service/certificate_authorities.go @@ -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 } diff --git a/ee/server/service/certificate_authorities_test.go b/ee/server/service/certificate_authorities_test.go index 24e11fb94a..7ca0301db0 100644 --- a/ee/server/service/certificate_authorities_test.go +++ b/ee/server/service/certificate_authorities_test.go @@ -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{