From 94f083c89800eb3c0511846a2b9dfe71940a364f Mon Sep 17 00:00:00 2001 From: Lucas Manuel Rodriguez Date: Fri, 20 Mar 2026 10:29:05 -0300 Subject: [PATCH] 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 --- server/fleet/policies.go | 35 ++- server/service/global_policies.go | 9 + server/service/integration_enterprise_test.go | 204 ++++++++++++++++++ server/service/team_policies.go | 12 ++ 4 files changed, 250 insertions(+), 10 deletions(-) diff --git a/server/fleet/policies.go b/server/fleet/policies.go index 4adcabf7ed..5bbb98dace 100644 --- a/server/fleet/policies.go +++ b/server/fleet/policies.go @@ -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 } diff --git a/server/service/global_policies.go b/server/service/global_policies.go index dbc1f9c9fe..c196d4dd94 100644 --- a/server/service/global_policies.go +++ b/server/service/global_policies.go @@ -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), diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 17288f06ec..099fd31ed8 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -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 { diff --git a/server/service/team_policies.go b/server/service/team_policies.go index ae92e3d96d..dee6674435 100644 --- a/server/service/team_policies.go +++ b/server/service/team_policies.go @@ -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)