mirror of
https://github.com/fleetdm/fleet
synced 2026-05-24 09:28:54 +00:00
Update duplicate CA names logic (#33349)
**Related issue:** Resolves #33351 Resolves an unreleased bug with Gitops validation of CA names. Previously only gitops path was validating that a CA didn't have the same name as another CA(I.e. Hydrant didn't have same name as digicert) whereas correct validation per PM is only within a given type of CA, i.e. can't have 2 hydrant with same name. Will also need cherrypick to 4.74. No changes file sinec this is an unreleased bug in the overall Hydrant CA story. # Checklist for submitter If some of the following don't apply, delete the relevant line. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) - [x] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes ## Testing - [x] Added/updated automated tests - [x] Where appropriate, [automated tests simulate multiple hosts and test for host isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing) (updates to one hosts's records do not affect another) - [x] QA'd all new/changed functionality manually For unreleased bug fixes in a release candidate, one of: - [x] Confirmed that the fix is not expected to adversely impact load test results - [x] Alerted the release DRI if additional load testing is needed
This commit is contained in:
parent
6c659050c0
commit
44c0fe8b39
2 changed files with 24 additions and 71 deletions
|
|
@ -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))
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in a new issue