Add checks on policy endpoints for conditional_access_enabled and platform (#42100)
Resolves #41477 ## Testing - [X] Added/updated automated tests - [X] QA'd all new/changed functionality manually
This commit is contained in:
+25
-10
@@ -109,16 +109,17 @@ type NewTeamPolicyPayload struct {
|
||||
}
|
||||
|
||||
var (
|
||||
errPolicyEmptyName = errors.New("policy name cannot be empty")
|
||||
errPolicyEmptyQuery = errors.New("policy query cannot be empty")
|
||||
errPolicyIDAndQuerySet = errors.New("both fields \"queryID\" and \"query\" cannot be set")
|
||||
errPolicyInvalidPlatform = errors.New("invalid policy platform")
|
||||
errPolicyConflictingLabels = errors.New("policy cannot include both labels_include_any and labels_exclude_any")
|
||||
errPolicyPatchAndQuerySet = errors.New("If the \"type\" is \"patch\", the \"query\" field is not supported.")
|
||||
errPolicyPatchAndPlatformSet = errors.New("If the \"type\" is \"patch\", the \"platform\" field is not supported.")
|
||||
errPolicyPatchNoTitleID = errors.New("If the \"type\" is \"patch\", the \"patch_software_title_id\" field is required.")
|
||||
errPolicyQueryUpdated = errors.New("\"query\" can't be updated")
|
||||
errPolicyPlatformUpdated = errors.New("\"platform\" can't be updated")
|
||||
errPolicyEmptyName = errors.New("policy name cannot be empty")
|
||||
errPolicyEmptyQuery = errors.New("policy query cannot be empty")
|
||||
errPolicyIDAndQuerySet = errors.New("both fields \"queryID\" and \"query\" cannot be set")
|
||||
errPolicyInvalidPlatform = errors.New("invalid policy platform")
|
||||
errPolicyConflictingLabels = errors.New("policy cannot include both labels_include_any and labels_exclude_any")
|
||||
errPolicyPatchAndQuerySet = errors.New("If the \"type\" is \"patch\", the \"query\" field is not supported.")
|
||||
errPolicyPatchAndPlatformSet = errors.New("If the \"type\" is \"patch\", the \"platform\" field is not supported.")
|
||||
errPolicyPatchNoTitleID = errors.New("If the \"type\" is \"patch\", the \"patch_software_title_id\" field is required.")
|
||||
errPolicyQueryUpdated = errors.New("\"query\" can't be updated")
|
||||
errPolicyPlatformUpdated = errors.New("\"platform\" can't be updated")
|
||||
errPolicyConditionalAccessEnabledInvalidPlatform = errors.New("\"conditional_access_enabled\" is only valid on \"darwin\" and \"windows\" policies")
|
||||
)
|
||||
|
||||
// PolicyNoTeamID is the team ID of "No team" policies.
|
||||
@@ -163,6 +164,10 @@ func (p PolicyPayload) Verify() error {
|
||||
if err := verifyPolicyPlatforms(p.Platform); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := PolicyVerifyConditionalAccess(p.ConditionalAccessEnabled, p.Platform); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if len(p.LabelsIncludeAny) > 0 && len(p.LabelsExcludeAny) > 0 {
|
||||
return errPolicyConflictingLabels
|
||||
}
|
||||
@@ -202,6 +207,13 @@ func verifyPolicyPlatforms(platforms string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func PolicyVerifyConditionalAccess(conditionalAccessEnabled bool, platform string) error {
|
||||
if conditionalAccessEnabled && !strings.Contains(platform, "darwin") && !strings.Contains(platform, "windows") {
|
||||
return errPolicyConditionalAccessEnabledInvalidPlatform
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// ModifyPolicyPayload holds data for policy modification.
|
||||
type ModifyPolicyPayload struct {
|
||||
// Name is the name of the policy.
|
||||
@@ -492,6 +504,9 @@ func (p PolicySpec) Verify() error {
|
||||
if err := verifyPolicyPlatforms(p.Platform); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := PolicyVerifyConditionalAccess(p.ConditionalAccessEnabled, p.Platform); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
|
||||
@@ -71,6 +71,7 @@ func (svc Service) NewGlobalPolicy(ctx context.Context, p fleet.PolicyPayload) (
|
||||
if !ok {
|
||||
return nil, errors.New("user must be authenticated to create fleet policies")
|
||||
}
|
||||
|
||||
if err := p.Verify(); err != nil {
|
||||
return nil, ctxerr.Wrap(ctx, &fleet.BadRequestError{
|
||||
Message: fmt.Sprintf("policy payload verification: %s", err),
|
||||
@@ -333,6 +334,8 @@ type modifyGlobalPolicyResponse struct {
|
||||
|
||||
func (r modifyGlobalPolicyResponse) Error() error { return r.Err }
|
||||
|
||||
const errPolicyAllFleetsForConditionalAccess = "\"All fleets\" policy cannot have conditional_access_enabled set"
|
||||
|
||||
func modifyGlobalPolicyEndpoint(ctx context.Context, request interface{}, svc fleet.Service) (fleet.Errorer, error) {
|
||||
req := request.(*modifyGlobalPolicyRequest)
|
||||
resp, err := svc.ModifyGlobalPolicy(ctx, req.PolicyID, req.ModifyPolicyPayload)
|
||||
@@ -564,6 +567,12 @@ func (svc *Service) ApplyPolicySpecs(ctx context.Context, policies []*fleet.Poli
|
||||
|
||||
// After the authorization check, check the policy fields.
|
||||
for _, policy := range policies {
|
||||
if policy.Team == "" && policy.ConditionalAccessEnabled {
|
||||
return ctxerr.Wrap(ctx, &fleet.BadRequestError{
|
||||
Message: fmt.Sprintf("policy spec payload verification: %s", errPolicyAllFleetsForConditionalAccess),
|
||||
})
|
||||
}
|
||||
|
||||
if err := policy.Verify(); err != nil {
|
||||
return ctxerr.Wrap(ctx, &fleet.BadRequestError{
|
||||
Message: fmt.Sprintf("policy spec payload verification: %s", err),
|
||||
|
||||
@@ -21880,6 +21880,7 @@ func (s *integrationEnterpriseTestSuite) TestConditionalAccessPolicies() {
|
||||
s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/teams/%d/policies", t1.ID), teamPolicyRequest{
|
||||
Query: "SELECT 1;",
|
||||
Name: "Compliance check 1",
|
||||
Platform: "darwin,windows",
|
||||
ConditionalAccessEnabled: true,
|
||||
}, http.StatusOK, &pr)
|
||||
cp1 := pr.Policy
|
||||
@@ -21888,12 +21889,14 @@ func (s *integrationEnterpriseTestSuite) TestConditionalAccessPolicies() {
|
||||
Query: "SELECT 2;",
|
||||
Name: "Compliance check 2",
|
||||
ConditionalAccessEnabled: true,
|
||||
Platform: "darwin,windows",
|
||||
}, http.StatusOK, &pr)
|
||||
cp2 := pr.Policy
|
||||
pr = teamPolicyResponse{}
|
||||
s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/teams/%d/policies", t1.ID), teamPolicyRequest{
|
||||
Query: "SELECT 3;",
|
||||
Name: "Other policy",
|
||||
Platform: "darwin,windows",
|
||||
ConditionalAccessEnabled: false,
|
||||
}, http.StatusOK, &pr)
|
||||
p3 := pr.Policy
|
||||
@@ -22244,6 +22247,7 @@ func (s *integrationEnterpriseTestSuite) TestConditionalAccessPolicies() {
|
||||
Query: "SELECT 1;",
|
||||
Name: "Compliance check 1",
|
||||
ConditionalAccessEnabled: true,
|
||||
Platform: "darwin,windows",
|
||||
}, http.StatusOK, &pr)
|
||||
cp1 = pr.Policy
|
||||
pr = teamPolicyResponse{}
|
||||
@@ -22377,6 +22381,7 @@ func (s *integrationEnterpriseTestSuite) TestConditionalAccessPoliciesEntraResul
|
||||
Query: "SELECT 1;",
|
||||
Name: "Compliance check 1",
|
||||
ConditionalAccessEnabled: true,
|
||||
Platform: "darwin,windows",
|
||||
}, http.StatusOK, &pr)
|
||||
cp1 := pr.Policy
|
||||
pr = teamPolicyResponse{}
|
||||
@@ -22384,6 +22389,7 @@ func (s *integrationEnterpriseTestSuite) TestConditionalAccessPoliciesEntraResul
|
||||
Query: "SELECT 2;",
|
||||
Name: "Compliance check 2",
|
||||
ConditionalAccessEnabled: true,
|
||||
Platform: "darwin,windows",
|
||||
}, http.StatusOK, &pr)
|
||||
cp2 := pr.Policy
|
||||
|
||||
@@ -27342,6 +27348,204 @@ func (s *integrationEnterpriseTestSuite) TestPatchPolicies() {
|
||||
})
|
||||
}
|
||||
|
||||
func (s *integrationEnterpriseTestSuite) TestConditionalAccessPlatformValidation() {
|
||||
t := s.T()
|
||||
|
||||
team1, err := s.ds.NewTeam(context.Background(), &fleet.Team{
|
||||
Name: "team_ca_platform_" + t.Name(),
|
||||
Description: "desc",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
//
|
||||
// 1. Create team policy — invalid platforms
|
||||
//
|
||||
|
||||
// conditional_access_enabled=true with platform="linux" should fail
|
||||
res := s.Do("POST", fmt.Sprintf("/api/latest/fleet/teams/%d/policies", team1.ID), map[string]any{
|
||||
"name": "ca-linux",
|
||||
"query": "SELECT 1;",
|
||||
"platform": "linux",
|
||||
"conditional_access_enabled": true,
|
||||
}, http.StatusBadRequest)
|
||||
errMsg := extractServerErrorText(res.Body)
|
||||
require.Contains(t, errMsg, `"conditional_access_enabled" is only valid on "darwin" and "windows" policies`)
|
||||
res.Body.Close()
|
||||
|
||||
// conditional_access_enabled=true with empty platform (all platforms) should fail
|
||||
res = s.Do("POST", fmt.Sprintf("/api/latest/fleet/teams/%d/policies", team1.ID), map[string]any{
|
||||
"name": "ca-all-platforms",
|
||||
"query": "SELECT 1;",
|
||||
"platform": "",
|
||||
"conditional_access_enabled": true,
|
||||
}, http.StatusBadRequest)
|
||||
errMsg = extractServerErrorText(res.Body)
|
||||
require.Contains(t, errMsg, `"conditional_access_enabled" is only valid on "darwin" and "windows" policies`)
|
||||
res.Body.Close()
|
||||
|
||||
// conditional_access_enabled=true with platform="darwin" should succeed
|
||||
var pr teamPolicyResponse
|
||||
s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/teams/%d/policies", team1.ID), teamPolicyRequest{
|
||||
Name: "ca-darwin",
|
||||
Query: "SELECT 1;",
|
||||
Platform: "darwin",
|
||||
ConditionalAccessEnabled: true,
|
||||
}, http.StatusOK, &pr)
|
||||
darwinPolicyID := pr.Policy.ID
|
||||
|
||||
// conditional_access_enabled=true with platform="windows" should succeed
|
||||
pr = teamPolicyResponse{}
|
||||
s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/teams/%d/policies", team1.ID), teamPolicyRequest{
|
||||
Name: "ca-windows",
|
||||
Query: "SELECT 2;",
|
||||
Platform: "windows",
|
||||
ConditionalAccessEnabled: true,
|
||||
}, http.StatusOK, &pr)
|
||||
windowsPolicyID := pr.Policy.ID
|
||||
|
||||
// conditional_access_enabled=true with platform="darwin,windows" should succeed
|
||||
pr = teamPolicyResponse{}
|
||||
s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/teams/%d/policies", team1.ID), teamPolicyRequest{
|
||||
Name: "ca-darwin-windows",
|
||||
Query: "SELECT 3;",
|
||||
Platform: "darwin,windows",
|
||||
ConditionalAccessEnabled: true,
|
||||
}, http.StatusOK, &pr)
|
||||
|
||||
// conditional_access_enabled=false with platform="linux" should succeed (no restriction when disabled)
|
||||
pr = teamPolicyResponse{}
|
||||
s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/teams/%d/policies", team1.ID), teamPolicyRequest{
|
||||
Name: "no-ca-linux",
|
||||
Query: "SELECT 4;",
|
||||
Platform: "linux",
|
||||
ConditionalAccessEnabled: false,
|
||||
}, http.StatusOK, &pr)
|
||||
linuxPolicyID := pr.Policy.ID
|
||||
|
||||
//
|
||||
// 2. Modify team policy — platform validation on update
|
||||
//
|
||||
|
||||
// Modify a darwin policy to enable conditional_access_enabled — should succeed (already darwin)
|
||||
patchResp := &modifyTeamPolicyResponse{}
|
||||
s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d/policies/%d", team1.ID, darwinPolicyID), modifyTeamPolicyRequest{
|
||||
ModifyPolicyPayload: fleet.ModifyPolicyPayload{
|
||||
ConditionalAccessEnabled: new(true),
|
||||
},
|
||||
}, http.StatusOK, patchResp)
|
||||
|
||||
// Modify a linux policy to enable conditional_access_enabled — should fail
|
||||
res = s.Do("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d/policies/%d", team1.ID, linuxPolicyID), map[string]any{
|
||||
"conditional_access_enabled": true,
|
||||
}, http.StatusBadRequest)
|
||||
errMsg = extractServerErrorText(res.Body)
|
||||
require.Contains(t, errMsg, `"conditional_access_enabled" is only valid on "darwin" and "windows" policies`)
|
||||
res.Body.Close()
|
||||
|
||||
// Modify a darwin conditional_access policy to change platform to linux — should fail
|
||||
res = s.Do("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d/policies/%d", team1.ID, darwinPolicyID), map[string]any{
|
||||
"platform": "linux",
|
||||
}, http.StatusBadRequest)
|
||||
errMsg = extractServerErrorText(res.Body)
|
||||
require.Contains(t, errMsg, `"conditional_access_enabled" is only valid on "darwin" and "windows" policies`)
|
||||
res.Body.Close()
|
||||
|
||||
// Modify a windows conditional_access policy to change platform to darwin — should succeed
|
||||
patchResp = &modifyTeamPolicyResponse{}
|
||||
s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d/policies/%d", team1.ID, windowsPolicyID), modifyTeamPolicyRequest{
|
||||
ModifyPolicyPayload: fleet.ModifyPolicyPayload{
|
||||
Platform: new("darwin"),
|
||||
},
|
||||
}, http.StatusOK, patchResp)
|
||||
|
||||
//
|
||||
// 3. Modify global policy — conditional_access_enabled not allowed
|
||||
//
|
||||
|
||||
// Create a global policy
|
||||
createGlobal := &globalPolicyResponse{}
|
||||
s.DoJSON("POST", "/api/latest/fleet/policies", &globalPolicyRequest{
|
||||
Name: "global-policy-ca-test",
|
||||
Query: "SELECT 1;",
|
||||
Platform: "darwin",
|
||||
}, http.StatusOK, createGlobal)
|
||||
globalPolicyID := createGlobal.Policy.ID
|
||||
|
||||
// Try to enable conditional_access_enabled on a global policy — should fail
|
||||
res = s.Do("PATCH", fmt.Sprintf("/api/latest/fleet/policies/%d", globalPolicyID), map[string]any{
|
||||
"conditional_access_enabled": true,
|
||||
}, http.StatusBadRequest)
|
||||
errMsg = extractServerErrorText(res.Body)
|
||||
require.Contains(t, errMsg, `"All fleets" policy cannot have conditional_access_enabled set`)
|
||||
res.Body.Close()
|
||||
|
||||
//
|
||||
// 4. Apply policy specs — platform validation
|
||||
//
|
||||
|
||||
// Team spec with conditional_access_enabled=true and platform="linux" should fail
|
||||
applyResp := applyPolicySpecsResponse{}
|
||||
res = s.Do("POST", "/api/latest/fleet/spec/policies", applyPolicySpecsRequest{
|
||||
Specs: []*fleet.PolicySpec{
|
||||
{
|
||||
Name: "spec-ca-linux",
|
||||
Query: "SELECT 1;",
|
||||
Team: team1.Name,
|
||||
Platform: "linux",
|
||||
ConditionalAccessEnabled: true,
|
||||
Type: fleet.PolicyTypeDynamic,
|
||||
},
|
||||
},
|
||||
}, http.StatusBadRequest)
|
||||
errMsg = extractServerErrorText(res.Body)
|
||||
require.Contains(t, errMsg, `"conditional_access_enabled" is only valid on "darwin" and "windows" policies`)
|
||||
res.Body.Close()
|
||||
|
||||
// Global spec (no team) with conditional_access_enabled=true should fail
|
||||
res = s.Do("POST", "/api/latest/fleet/spec/policies", applyPolicySpecsRequest{
|
||||
Specs: []*fleet.PolicySpec{
|
||||
{
|
||||
Name: "spec-ca-global",
|
||||
Query: "SELECT 1;",
|
||||
Platform: "darwin",
|
||||
ConditionalAccessEnabled: true,
|
||||
Type: fleet.PolicyTypeDynamic,
|
||||
},
|
||||
},
|
||||
}, http.StatusBadRequest)
|
||||
errMsg = extractServerErrorText(res.Body)
|
||||
require.Contains(t, errMsg, `"All fleets" policy cannot have conditional_access_enabled set`)
|
||||
res.Body.Close()
|
||||
|
||||
// Team spec with conditional_access_enabled=true and platform="darwin" should succeed
|
||||
s.DoJSON("POST", "/api/latest/fleet/spec/policies", applyPolicySpecsRequest{
|
||||
Specs: []*fleet.PolicySpec{
|
||||
{
|
||||
Name: "spec-ca-darwin",
|
||||
Query: "SELECT 1;",
|
||||
Team: team1.Name,
|
||||
Platform: "darwin",
|
||||
ConditionalAccessEnabled: true,
|
||||
Type: fleet.PolicyTypeDynamic,
|
||||
},
|
||||
},
|
||||
}, http.StatusOK, &applyResp)
|
||||
|
||||
// Team spec with conditional_access_enabled=true and platform="windows" should succeed
|
||||
s.DoJSON("POST", "/api/latest/fleet/spec/policies", applyPolicySpecsRequest{
|
||||
Specs: []*fleet.PolicySpec{
|
||||
{
|
||||
Name: "spec-ca-windows",
|
||||
Query: "SELECT 2;",
|
||||
Team: team1.Name,
|
||||
Platform: "windows",
|
||||
ConditionalAccessEnabled: true,
|
||||
Type: fleet.PolicyTypeDynamic,
|
||||
},
|
||||
},
|
||||
}, http.StatusOK, &applyResp)
|
||||
}
|
||||
|
||||
func getFleetMaintainedAppID(t *testing.T, ds *mysql.Datastore, slug string) uint {
|
||||
var id uint
|
||||
mysql.ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
|
||||
|
||||
@@ -616,6 +616,12 @@ func (svc *Service) modifyPolicy(ctx context.Context, teamID *uint, id uint, p f
|
||||
})
|
||||
}
|
||||
|
||||
if p.ConditionalAccessEnabled != nil && *p.ConditionalAccessEnabled && teamID == nil {
|
||||
return nil, ctxerr.Wrap(ctx, &fleet.BadRequestError{
|
||||
Message: fmt.Sprintf(`policy payload verification: %s`, errPolicyAllFleetsForConditionalAccess),
|
||||
})
|
||||
}
|
||||
|
||||
p.Type = policy.Type
|
||||
if err := p.Verify(); err != nil {
|
||||
return nil, ctxerr.Wrap(ctx, &fleet.BadRequestError{
|
||||
@@ -708,6 +714,12 @@ func (svc *Service) modifyPolicy(ctx context.Context, teamID *uint, id uint, p f
|
||||
}
|
||||
}
|
||||
|
||||
if err := fleet.PolicyVerifyConditionalAccess(policy.ConditionalAccessEnabled, policy.Platform); err != nil {
|
||||
return nil, ctxerr.Wrap(ctx, &fleet.BadRequestError{
|
||||
Message: fmt.Sprintf("policy payload verification: %s", err),
|
||||
})
|
||||
}
|
||||
|
||||
logging.WithExtras(ctx, "name", policy.Name, "sql", policy.Query)
|
||||
|
||||
err = svc.ds.SavePolicy(ctx, policy, removeAllMemberships, removeStats)
|
||||
|
||||
Reference in New Issue
Block a user