mirror of
https://github.com/fleetdm/fleet
synced 2026-05-24 09:28:54 +00:00
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
This commit is contained in:
parent
103d537dc5
commit
ee1be26878
4 changed files with 93 additions and 45 deletions
|
|
@ -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"`
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue