diff --git a/ee/server/service/certificate_authorities.go b/ee/server/service/certificate_authorities.go index 53080d0e54..317e4e860d 100644 --- a/ee/server/service/certificate_authorities.go +++ b/ee/server/service/certificate_authorities.go @@ -439,26 +439,15 @@ func (svc *Service) getCertificateAuthoritiesBatchOperations(ctx context.Context return nil, err } - // collect existing CA names for duplicate name checking - existingNames := make(map[string]string) - for _, ca := range existing.DigiCert { - existingNames[ca.Name] = "digicert" - } - for _, ca := range existing.CustomScepProxy { - existingNames[ca.Name] = "custom_scep_proxy" - } - for _, ca := range existing.Hydrant { - existingNames[ca.Name] = "hydrant" - } - - // collect all CA names for duplicate name checking across both incoming and existing - allNames := make(map[string]struct{}) - // check for duplicate names across all CA types - checkAllNames := func(name, caType string) error { - if _, ok := allNames[name]; ok { - return fmtDuplicateCANameError(name, caType) + // track processed CA names for duplicate name checking + allNames := make(map[string][]string) + checkAllNames := func(name, caType, displayCAType string) error { + for i := 0; i < len(allNames[name]); i++ { + if allNames[name][i] == caType { + return fmtDuplicateCANameError(name, caType, displayCAType) + } } - allNames[name] = struct{}{} + allNames[name] = append(allNames[name], caType) return nil } @@ -470,13 +459,7 @@ func (svc *Service) getCertificateAuthoritiesBatchOperations(ctx context.Context ca.Name = fleet.Preprocess(ca.Name) ca.URL = fleet.Preprocess(ca.URL) ca.ProfileID = fleet.Preprocess(ca.ProfileID) - // check against existing names, excluding self if updating - if existing, ok := existingNames[ca.Name]; ok { - if existing != "digicert" { - return nil, fmtDuplicateCANameError(ca.Name, "digicert") - } - } - if err := checkAllNames(ca.Name, "digicert"); err != nil { + if err := checkAllNames(ca.Name, "digicert", "DigiCert"); err != nil { return nil, err } } @@ -486,13 +469,7 @@ func (svc *Service) getCertificateAuthoritiesBatchOperations(ctx context.Context return nil, fleet.NewInvalidArgumentError("name", "certificate_authorities.custom_scep_proxy: CA name cannot be empty.") } ca.Name = fleet.Preprocess(ca.Name) - // check against existing names, excluding self if updating - if existing, ok := existingNames[ca.Name]; ok { - if existing != "custom_scep_proxy" { - return nil, fmtDuplicateCANameError(ca.Name, "custom_scep_proxy") - } - } - if err := checkAllNames(ca.Name, "custom_scep_proxy"); err != nil { + if err := checkAllNames(ca.Name, "custom_scep_proxy", "Custom SCEP Proxy"); err != nil { return nil, err } } @@ -503,13 +480,7 @@ func (svc *Service) getCertificateAuthoritiesBatchOperations(ctx context.Context } ca.Name = fleet.Preprocess(ca.Name) ca.URL = fleet.Preprocess(ca.URL) - // check against existing names, excluding self if updating - if existing, ok := existingNames[ca.Name]; ok { - if existing != "hydrant" { - return nil, fmtDuplicateCANameError(ca.Name, "hydrant") - } - } - if err := checkAllNames(ca.Name, "hydrant"); err != nil { + if err := checkAllNames(ca.Name, "hydrant", "Hydrant"); err != nil { return nil, err } } @@ -1175,7 +1146,7 @@ func (svc *Service) validateCustomSCEPProxyUpdate(ctx context.Context, customSCE return nil } -func fmtDuplicateCANameError(name, caType string) error { +func fmtDuplicateCANameError(name, caType, displayCAType string) error { return fleet.NewInvalidArgumentError("name", fmt.Sprintf("certificate_authorities.%s.name: Couldn’t edit certificate authority. "+ - "\"%s\" name is already used by another certificate authority. Please choose a different name and try again.", caType, name)) + "\"%s\" name is already used by another %s certificate authority. Please choose a different name and try again.", caType, name, displayCAType)) } diff --git a/server/service/integration_certificate_authorities_test.go b/server/service/integration_certificate_authorities_test.go index ec80c738b6..a423d220ab 100644 --- a/server/service/integration_certificate_authorities_test.go +++ b/server/service/integration_certificate_authorities_test.go @@ -433,7 +433,7 @@ func (s *integrationMDMTestSuite) TestBatchApplyCertificateAuthorities() { res := s.Do("POST", "/api/v1/fleet/spec/certificate_authorities", duplicateReq, http.StatusUnprocessableEntity) errMsg := extractServerErrorText(res.Body) require.Contains(t, errMsg, "certificate_authorities.digicert") - require.Contains(t, errMsg, "name is already used by another certificate authority") + require.Contains(t, errMsg, "name is already used by another DigiCert certificate authority") // try to create digicert with same name as another custom scep testCopy = goodDigiCertCA @@ -446,10 +446,7 @@ func (s *integrationMDMTestSuite) TestBatchApplyCertificateAuthorities() { }, DryRun: false, } - res = s.Do("POST", "/api/v1/fleet/spec/certificate_authorities", duplicateReq, http.StatusUnprocessableEntity) - errMsg = extractServerErrorText(res.Body) - require.Contains(t, errMsg, "certificate_authorities.digicert") - require.Contains(t, errMsg, "name is already used by another certificate authority") + s.Do("POST", "/api/v1/fleet/spec/certificate_authorities", duplicateReq, http.StatusOK) // try to create digicert with same name as another hydrant testCopy = goodDigiCertCA @@ -462,10 +459,7 @@ func (s *integrationMDMTestSuite) TestBatchApplyCertificateAuthorities() { }, DryRun: false, } - res = s.Do("POST", "/api/v1/fleet/spec/certificate_authorities", duplicateReq, http.StatusUnprocessableEntity) - errMsg = extractServerErrorText(res.Body) - require.Contains(t, errMsg, "certificate_authorities.digicert") - require.Contains(t, errMsg, "name is already used by another certificate authority") + s.Do("POST", "/api/v1/fleet/spec/certificate_authorities", duplicateReq, http.StatusOK) }) t.Run("digicert more than 1 user principal name", func(t *testing.T) { @@ -765,9 +759,9 @@ func (s *integrationMDMTestSuite) TestBatchApplyCertificateAuthorities() { res := s.Do("POST", "/api/v1/fleet/spec/certificate_authorities", duplicateReq, http.StatusUnprocessableEntity) errMsg := extractServerErrorText(res.Body) require.Contains(t, errMsg, "certificate_authorities.custom_scep_proxy") - require.Contains(t, errMsg, "name is already used by another certificate authority") + require.Contains(t, errMsg, "name is already used by another Custom SCEP Proxy certificate authority") - // try to create custom scep with same name as another digicert + // try to create custom scep with same name as the digicert. Should not error testCopy = goodCustomSCEPCA testCopy.Name = goodDigiCertCA.Name duplicateReq = batchApplyCertificateAuthoritiesRequest{ @@ -778,12 +772,9 @@ func (s *integrationMDMTestSuite) TestBatchApplyCertificateAuthorities() { }, DryRun: false, } - res = s.Do("POST", "/api/v1/fleet/spec/certificate_authorities", duplicateReq, http.StatusUnprocessableEntity) - errMsg = extractServerErrorText(res.Body) - require.Contains(t, errMsg, "certificate_authorities.custom_scep_proxy") - require.Contains(t, errMsg, "name is already used by another certificate authority") + s.Do("POST", "/api/v1/fleet/spec/certificate_authorities", duplicateReq, http.StatusOK) - // try to create custom scep with same name as another hydrant + // try to create custom scep with same name as the Hydrant CA. Should not error testCopy = goodCustomSCEPCA testCopy.Name = goodHydrantCA.Name duplicateReq = batchApplyCertificateAuthoritiesRequest{ @@ -794,10 +785,7 @@ func (s *integrationMDMTestSuite) TestBatchApplyCertificateAuthorities() { }, DryRun: false, } - res = s.Do("POST", "/api/v1/fleet/spec/certificate_authorities", duplicateReq, http.StatusUnprocessableEntity) - errMsg = extractServerErrorText(res.Body) - require.Contains(t, errMsg, "certificate_authorities.custom_scep_proxy") - require.Contains(t, errMsg, "name is already used by another certificate authority") + s.Do("POST", "/api/v1/fleet/spec/certificate_authorities", duplicateReq, http.StatusOK) }) t.Run("custom_scep challenge not set", func(t *testing.T) { @@ -974,7 +962,7 @@ func (s *integrationMDMTestSuite) TestBatchApplyCertificateAuthorities() { res := s.Do("POST", "/api/v1/fleet/spec/certificate_authorities", duplicateReq, http.StatusUnprocessableEntity) errMsg := extractServerErrorText(res.Body) require.Contains(t, errMsg, "certificate_authorities.hydrant") - require.Contains(t, errMsg, "name is already used by another certificate authority") + require.Contains(t, errMsg, "name is already used by another Hydrant certificate authority") // try to create hydrant with same name as another digicert testCopy = goodHydrantCA @@ -987,10 +975,7 @@ func (s *integrationMDMTestSuite) TestBatchApplyCertificateAuthorities() { }, DryRun: false, } - res = s.Do("POST", "/api/v1/fleet/spec/certificate_authorities", duplicateReq, http.StatusUnprocessableEntity) - errMsg = extractServerErrorText(res.Body) - require.Contains(t, errMsg, "certificate_authorities.hydrant") - require.Contains(t, errMsg, "name is already used by another certificate authority") + s.Do("POST", "/api/v1/fleet/spec/certificate_authorities", duplicateReq, http.StatusOK) // try to create hydrant with same name as another custom scep testCopy = goodHydrantCA @@ -1003,10 +988,7 @@ func (s *integrationMDMTestSuite) TestBatchApplyCertificateAuthorities() { }, DryRun: false, } - res = s.Do("POST", "/api/v1/fleet/spec/certificate_authorities", duplicateReq, http.StatusUnprocessableEntity) - errMsg = extractServerErrorText(res.Body) - require.Contains(t, errMsg, "certificate_authorities.hydrant") - require.Contains(t, errMsg, "name is already used by another certificate authority") + s.Do("POST", "/api/v1/fleet/spec/certificate_authorities", duplicateReq, http.StatusOK) }) // TODO(hca): hydrant happy path and other specific tests