From ee1be26878063214876f7a0267d8bb08b90db63b Mon Sep 17 00:00:00 2001 From: Konstantin Sykulev Date: Mon, 8 Dec 2025 10:11:15 -0600 Subject: [PATCH] certificate gitops fixes (#36708) **Related issue:** Resolves #36649 Cleaned up some gitops messaging Changed the team parameter to accept a name rather than an id. the certificate_template is still using an id. This matches what other entities in gitops. Here is a sample post to the spec endpoint for a certificate_template ``` { specs: [ { "name": "certificate template", "team": "workstations", "certificate_authority_id": 1, "subject_name": "CN:hello" } ] } ``` # Checklist for submitter - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) ## 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 --- server/fleet/certificate_templates.go | 2 +- server/service/certificates.go | 52 +++++++++++++++---------- server/service/client.go | 39 ++++++++++++++----- server/service/integration_core_test.go | 45 ++++++++++++++------- 4 files changed, 93 insertions(+), 45 deletions(-) diff --git a/server/fleet/certificate_templates.go b/server/fleet/certificate_templates.go index 8a86b1d6e3..4c4a8b6b54 100644 --- a/server/fleet/certificate_templates.go +++ b/server/fleet/certificate_templates.go @@ -2,7 +2,7 @@ package fleet type CertificateRequestSpec struct { Name string `json:"name"` - Team string `json:"team"` + Team string `json:"team,omitempty"` CertificateAuthorityId uint `json:"certificate_authority_id"` SubjectName string `json:"subject_name"` } diff --git a/server/service/certificates.go b/server/service/certificates.go index 38716667cf..ae9f21ee04 100644 --- a/server/service/certificates.go +++ b/server/service/certificates.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "strconv" "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" hostctx "github.com/fleetdm/fleet/v4/server/contexts/host" @@ -273,21 +272,32 @@ func applyCertificateTemplateSpecsEndpoint(ctx context.Context, request interfac return applyCertificateTemplateSpecsResponse{}, nil } -func (svc *Service) checkCertificateTemplateSpecAuthorization(ctx context.Context, specs []*fleet.CertificateRequestSpec) error { - teamIDs := make(map[uint]bool) +func (svc *Service) resolveTeamNamesForSpecs(ctx context.Context, specs []*fleet.CertificateRequestSpec) (map[string]uint, error) { + teamNameToID := make(map[string]uint) + for _, spec := range specs { - var teamID uint - if spec.Team != "" { - parsed, err := strconv.ParseUint(spec.Team, 10, 0) - if err != nil { - return ctxerr.Wrap(ctx, err, "parsing team ID") - } - teamID = uint(parsed) + if _, ok := teamNameToID[spec.Team]; ok { + continue } - teamIDs[teamID] = true + + // Handle empty string and "No team" as teamID = 0 + if spec.Team == "" || spec.Team == "No team" { + teamNameToID[spec.Team] = 0 + continue + } + + team, err := svc.ds.TeamByName(ctx, spec.Team) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "getting team by name") + } + teamNameToID[spec.Team] = team.ID } - for teamID := range teamIDs { + return teamNameToID, nil +} + +func (svc *Service) checkCertificateTemplateSpecAuthorization(ctx context.Context, teamNameToID map[string]uint) error { + for _, teamID := range teamNameToID { if err := svc.authz.Authorize(ctx, &fleet.CertificateTemplate{TeamID: teamID}, fleet.ActionWrite); err != nil { return err } @@ -297,7 +307,13 @@ func (svc *Service) checkCertificateTemplateSpecAuthorization(ctx context.Contex } func (svc *Service) ApplyCertificateTemplateSpecs(ctx context.Context, specs []*fleet.CertificateRequestSpec) error { - if err := svc.checkCertificateTemplateSpecAuthorization(ctx, specs); err != nil { + teamNameToID, err := svc.resolveTeamNamesForSpecs(ctx, specs) + if err != nil { + svc.authz.SkipAuthorization(ctx) + return err + } + + if err := svc.checkCertificateTemplateSpecAuthorization(ctx, teamNameToID); err != nil { return err } @@ -322,20 +338,14 @@ func (svc *Service) ApplyCertificateTemplateSpecs(ctx context.Context, specs []* if ca.Type != string(fleet.CATypeCustomSCEPProxy) { return &fleet.BadRequestError{Message: fmt.Sprintf("Ccertificate `%s`: Currently, only the custom_scep_proxy certificate authority is supported.", spec.Name)} } - var teamID uint - if spec.Team != "" { - parsed, err := strconv.ParseUint(spec.Team, 10, 0) - if err != nil { - return ctxerr.Wrap(ctx, err, "parsing team ID") - } - teamID = uint(parsed) - } // Validate Fleet variables in subject name if err := validateCertificateTemplateFleetVariables(spec.SubjectName); err != nil { return &fleet.BadRequestError{Message: fmt.Sprintf("%s (certificate %s)", err.Error(), spec.Name)} } + teamID := teamNameToID[spec.Team] + cert := &fleet.CertificateTemplate{ Name: spec.Name, CertificateAuthorityID: spec.CertificateAuthorityId, diff --git a/server/service/client.go b/server/service/client.go index 6e41c1a1f4..791c40672f 100644 --- a/server/service/client.go +++ b/server/service/client.go @@ -2276,6 +2276,21 @@ func (c *Client) DoGitOps( if ok && teamID == 0 { if dryRun { logFn("[+] would've added any policies/queries to new team %s\n", *incoming.TeamName) + + numCerts := 0 + if incoming.Controls.AndroidSettings != nil { + if androidSettings, ok := incoming.Controls.AndroidSettings.(fleet.AndroidSettings); ok { + if androidSettings.Certificates.Valid { + numCerts = len(androidSettings.Certificates.Value) + } + } + } + if numCerts > 0 { + logFn("[+] would've added %s to new team %s\n", + numberWithPluralization(numCerts, "Android certificate", "Android certificates"), + *incoming.TeamName) + } + return nil, postOps, nil } return nil, nil, fmt.Errorf("team %s not created", *incoming.TeamName) @@ -2883,12 +2898,14 @@ func (c *Client) doGitOpsAndroidCertificates(config *spec.GitOps, logFn func(for numCerts := len(certificates) teamID := "" + teamName := "" switch { - case config.TeamID != nil: - teamID = fmt.Sprintf("%d", *config.TeamID) case config.IsNoTeam(): - // "No team" + teamName = "No team" teamID = "0" + case config.TeamID != nil: + teamName = *config.TeamName + teamID = fmt.Sprintf("%d", *config.TeamID) default: // global config, ignore return nil @@ -2904,7 +2921,9 @@ func (c *Client) doGitOpsAndroidCertificates(config *spec.GitOps, logFn func(for return nil } - if numCerts > 0 { + if dryRun { + logFn("[+] would have attempted to apply %s\n", numberWithPluralization(numCerts, "Android certificate", "Android certificates")) + } else { logFn("[+] attempting to apply %s\n", numberWithPluralization(numCerts, "Android certificate", "Android certificates")) } @@ -2946,7 +2965,7 @@ func (c *Client) doGitOpsAndroidCertificates(config *spec.GitOps, logFn func(for certRequests[i] = &fleet.CertificateRequestSpec{ Name: certificates[i].Name, - Team: teamID, + Team: teamName, CertificateAuthorityId: ca.ID, SubjectName: certificates[i].SubjectName, } @@ -2997,15 +3016,15 @@ func (c *Client) doGitOpsAndroidCertificates(config *spec.GitOps, logFn func(for } } - if numCerts > 0 { - if dryRun { - logFn("[+] would've applied %s\n", numberWithPluralization(numCerts, "Android certificate", "Android certificates")) - } else { + if dryRun { + logFn("[+] would've applied %s\n", numberWithPluralization(numCerts, "Android certificate", "Android certificates")) + } else { + if numCerts > 0 { if err := c.ApplyCertificateSpecs(certRequests); err != nil { return fmt.Errorf("applying Android certificates: %w", err) } - logFn("[+] applied %s\n", numberWithPluralization(numCerts, "Android certificate", "Android certificates")) } + logFn("[+] applied %s\n", numberWithPluralization(numCerts, "Android certificate", "Android certificates")) } return nil diff --git a/server/service/integration_core_test.go b/server/service/integration_core_test.go index e2b95ce3aa..9d4aad0baa 100644 --- a/server/service/integration_core_test.go +++ b/server/service/integration_core_test.go @@ -7976,31 +7976,43 @@ func (s *integrationTestSuite) TestCertificatesSpecs() { }) require.NoError(t, err) - // apply certificate templates + // invalid Fleet variable in subject name var applyResp applyCertificateTemplateSpecsResponse s.DoJSON("POST", "/api/latest/fleet/spec/certificates", applyCertificateTemplateSpecsRequest{ Specs: []*fleet.CertificateRequestSpec{ { Name: "Invalid Template", - Team: fmt.Sprint(team.ID), + Team: team.Name, CertificateAuthorityId: ca.ID, SubjectName: "CN=$FLEET_VAR_NOT_VALID/OU=$FLEET_VAR_HOST_UUID", }, }, }, http.StatusBadRequest, &applyResp) - // valid templates + // test with non-existent team name + s.DoJSON("POST", "/api/latest/fleet/spec/certificates", applyCertificateTemplateSpecsRequest{ + Specs: []*fleet.CertificateRequestSpec{ + { + Name: "Invalid Team Template", + Team: "NonExistentTeam", + CertificateAuthorityId: ca.ID, + SubjectName: "CN=$FLEET_VAR_HOST_UUID", + }, + }, + }, http.StatusNotFound, &applyResp) + + // valid templates - test team name (not team ID) s.DoJSON("POST", "/api/latest/fleet/spec/certificates", applyCertificateTemplateSpecsRequest{ Specs: []*fleet.CertificateRequestSpec{ { Name: "Template 1", - Team: fmt.Sprint(team.ID), + Team: team.Name, CertificateAuthorityId: ca.ID, SubjectName: "CN=$FLEET_VAR_HOST_END_USER_IDP_USERNAME/OU=$FLEET_VAR_HOST_UUID/ST=$FLEET_VAR_HOST_HARDWARE_SERIAL", }, { Name: "Template 2", - Team: fmt.Sprint(team.ID), + Team: team.Name, CertificateAuthorityId: ca.ID, SubjectName: "CN=$FLEET_VAR_HOST_END_USER_IDP_USERNAME/OU=$FLEET_VAR_HOST_UUID", }, @@ -8088,29 +8100,36 @@ func (s *integrationTestSuite) TestCertificatesSpecs() { s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/certificates?team_id=%d", team.ID), nil, http.StatusOK, &listCertifcatesResp) require.Len(t, listCertifcatesResp.Certificates, 0) - // certificate templates for "No team" (team_id 0) + // certificate templates for "No team" s.DoJSON("POST", "/api/latest/fleet/spec/certificates", applyCertificateTemplateSpecsRequest{ Specs: []*fleet.CertificateRequestSpec{ { Name: "No Team Template 1", - Team: "0", + Team: "No team", CertificateAuthorityId: ca.ID, SubjectName: "CN=$FLEET_VAR_HOST_END_USER_IDP_USERNAME/OU=$FLEET_VAR_HOST_UUID", }, { Name: "No Team Template 2", - Team: "0", + Team: "", CertificateAuthorityId: ca.ID, SubjectName: "CN=$FLEET_VAR_HOST_HARDWARE_SERIAL", }, + { + Name: "No Team Template 3", + // No Team field, should default to empty string + CertificateAuthorityId: ca.ID, + SubjectName: "CN=$FLEET_VAR_HOST_UUID", + }, }, }, http.StatusOK, &applyResp) // list specs for "no team" (team_id 0) var noTeamCertificatesResp listCertificateTemplatesResponse s.DoJSON("GET", "/api/latest/fleet/certificates", nil, http.StatusOK, &noTeamCertificatesResp) - require.Len(t, noTeamCertificatesResp.Certificates, 2) - assert.ElementsMatch(t, []string{"No Team Template 1", "No Team Template 2"}, []string{noTeamCertificatesResp.Certificates[0].Name, noTeamCertificatesResp.Certificates[1].Name}) + require.Len(t, noTeamCertificatesResp.Certificates, 3) + certNames := []string{noTeamCertificatesResp.Certificates[0].Name, noTeamCertificatesResp.Certificates[1].Name, noTeamCertificatesResp.Certificates[2].Name} + assert.ElementsMatch(t, []string{"No Team Template 1", "No Team Template 2", "No Team Template 3"}, certNames) // Create a host noTeamHost, err := s.ds.NewHost(ctx, &fleet.Host{ @@ -8142,7 +8161,7 @@ func (s *integrationTestSuite) TestCertificatesSpecs() { savedNoTeamCertTemplates, _, err := s.ds.GetCertificateTemplatesByTeamID(ctx, 0, fleet.ListOptions{Page: 0, PerPage: 10}) require.NoError(t, err) - require.Len(t, savedNoTeamCertTemplates, 2) + require.Len(t, savedNoTeamCertTemplates, 3) noTeamCertID := savedNoTeamCertTemplates[0].ID // Get certificate with orbit node_key (should return replaced variables) @@ -8188,11 +8207,11 @@ func (s *integrationTestSuite) TestCertificatesSpecs() { }, http.StatusOK, &delBatchResp2) s.DoJSON("GET", "/api/latest/fleet/certificates", nil, http.StatusOK, &noTeamCertificatesResp) - require.Len(t, noTeamCertificatesResp.Certificates, 1) + require.Len(t, noTeamCertificatesResp.Certificates, 2) require.Equal(t, noTeamCertificatesResp.Certificates[0].ID, noTeamCertificatesResp.Certificates[0].ID) s.DoJSON("DELETE", "/api/latest/fleet/spec/certificates", map[string]interface{}{ - "ids": []uint{noTeamCertificatesResp.Certificates[0].ID}, + "ids": []uint{noTeamCertificatesResp.Certificates[0].ID, noTeamCertificatesResp.Certificates[1].ID}, "team_id": uint(0), }, http.StatusOK, &delBatchResp)