From 56d02eae2da7e6ffd63c8f0a78a2d322a000379f Mon Sep 17 00:00:00 2001 From: Tim Lee Date: Tue, 30 Apr 2024 09:03:53 -0600 Subject: [PATCH] 17744 policies backend (#18564) #17744 This change implements a new query parameter on `/teams/%d/policies` to merge inherited policies into the policies array instead of listing them separately. The frontend will key off the existing `team_id` field to mark policies as "inherited" in theUI. I opted for an additive approach in adding a datastore method rather than modifying the existing ListTeamPolicies to avoid a large test refactor. - [ ] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://fleetdm.com/docs/contributing/committing-changes#changes-files) for more information. - [X] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) - [X] Added/updated tests - [X] Manual QA for all new/changed functionality --------- Co-authored-by: RachelElysia --- server/datastore/mysql/policies.go | 34 ++++++++++++ server/datastore/mysql/policies_test.go | 55 +++++++++++++++++++ server/fleet/datastore.go | 2 + server/fleet/service.go | 2 +- server/mock/datastore_mock.go | 12 ++++ server/service/integration_enterprise_test.go | 14 +++++ server/service/team_policies.go | 10 +++- server/service/team_policies_test.go | 2 +- 8 files changed, 127 insertions(+), 4 deletions(-) diff --git a/server/datastore/mysql/policies.go b/server/datastore/mysql/policies.go index 3701c84b48..feb6272eeb 100644 --- a/server/datastore/mysql/policies.go +++ b/server/datastore/mysql/policies.go @@ -608,6 +608,40 @@ func (ds *Datastore) ListTeamPolicies(ctx context.Context, teamID uint, opts fle return teamPolicies, inheritedPolicies, err } +func (ds *Datastore) ListMergedTeamPolicies(ctx context.Context, teamID uint, opts fleet.ListOptions) ([]*fleet.Policy, error) { + var args []interface{} + + query := ` + SELECT + ` + policyCols + `, + COALESCE(u.name, '') AS author_name, + COALESCE(u.email, '') AS author_email, + ps.updated_at as host_count_updated_at, + COALESCE(ps.passing_host_count, 0) as passing_host_count, + COALESCE(ps.failing_host_count, 0) as failing_host_count + FROM policies p + LEFT JOIN users u ON p.author_id = u.id + LEFT JOIN policy_stats ps ON p.id = ps.policy_id + AND ps.inherited_team_id = COALESCE(p.team_id, 0) + WHERE p.team_id = ? OR p.team_id IS NULL + ` + + args = append(args, teamID) + + // We must normalize the name for full Unicode support (Unicode equivalence). + match := norm.NFC.String(opts.MatchQuery) + query, args = searchLike(query, args, match, policySearchColumns...) + query, _ = appendListOptionsToSQL(query, &opts) + + var policies []*fleet.Policy + err := sqlx.SelectContext(ctx, ds.reader(ctx), &policies, query, args...) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "listing merged team policies") + } + + return policies, nil +} + func (ds *Datastore) DeleteTeamPolicies(ctx context.Context, teamID uint, ids []uint) ([]uint, error) { return deletePolicyDB(ctx, ds.writer(ctx), ids, &teamID) } diff --git a/server/datastore/mysql/policies_test.go b/server/datastore/mysql/policies_test.go index 1e6980736e..57514af79b 100644 --- a/server/datastore/mysql/policies_test.go +++ b/server/datastore/mysql/policies_test.go @@ -35,6 +35,7 @@ func TestPolicies(t *testing.T) { {"MembershipViewNotDeferred", func(t *testing.T, ds *Datastore) { testPoliciesMembershipView(false, t, ds) }}, {"TeamPolicyLegacy", testTeamPolicyLegacy}, {"TeamPolicyProprietary", testTeamPolicyProprietary}, + {"ListMergedTeamPolicies", testListMergedTeamPolicies}, {"PolicyQueriesForHost", testPolicyQueriesForHost}, {"PolicyQueriesForHostPlatforms", testPolicyQueriesForHostPlatforms}, {"PoliciesByID", testPoliciesByID}, @@ -709,6 +710,60 @@ func testTeamPolicyProprietary(t *testing.T, ds *Datastore) { require.Equal(t, user1.ID, *team2Policies[0].AuthorID) } +func testListMergedTeamPolicies(t *testing.T, ds *Datastore) { + ctx := context.Background() + gpol, err := ds.NewGlobalPolicy(ctx, nil, fleet.PolicyPayload{ + Name: "query1 global", + Query: "select 1;", + Description: "query1 desc", + Resolution: "query1 resolution", + }) + require.NoError(t, err) + + team1, err := ds.NewTeam(ctx, &fleet.Team{Name: "team1"}) + require.NoError(t, err) + + p, err := ds.NewTeamPolicy(ctx, team1.ID, nil, fleet.PolicyPayload{ + Name: "query2 team1", + Query: "select 1;", + Description: "query1 desc", + Resolution: "query1 resolution", + }) + require.NoError(t, err) + + team2, err := ds.NewTeam(ctx, &fleet.Team{Name: "team2"}) + require.NoError(t, err) + + _, err = ds.NewTeamPolicy(ctx, team2.ID, nil, fleet.PolicyPayload{ + Name: "query3 team2", + Query: "select 2;", + Description: "query2 desc", + Resolution: "query2 resolution", + }) + require.NoError(t, err) + + merged, err := ds.ListMergedTeamPolicies(ctx, team1.ID, fleet.ListOptions{ + OrderKey: "name", + OrderDirection: fleet.OrderAscending, + }) + require.NoError(t, err) + + require.Len(t, merged, 2) + assert.Equal(t, gpol.ID, merged[0].ID) + assert.Equal(t, p.ID, merged[1].ID) + + // Test list options affect both global and team policies + merged, err = ds.ListMergedTeamPolicies(ctx, team1.ID, fleet.ListOptions{ + OrderKey: "name", + OrderDirection: fleet.OrderDescending, + }) + require.NoError(t, err) + + require.Len(t, merged, 2) + assert.Equal(t, p.ID, merged[0].ID) + assert.Equal(t, gpol.ID, merged[1].ID) +} + func newTestHostWithPlatform(t *testing.T, ds *Datastore, hostname, platform string, teamID *uint) *fleet.Host { nodeKey, err := server.GenerateRandomText(32) require.NoError(t, err) diff --git a/server/fleet/datastore.go b/server/fleet/datastore.go index 2070ad98ad..05c1e6cd96 100644 --- a/server/fleet/datastore.go +++ b/server/fleet/datastore.go @@ -661,6 +661,8 @@ type Datastore interface { NewTeamPolicy(ctx context.Context, teamID uint, authorID *uint, args PolicyPayload) (*Policy, error) ListTeamPolicies(ctx context.Context, teamID uint, opts ListOptions, iopts ListOptions) (teamPolicies, inheritedPolicies []*Policy, err error) + ListMergedTeamPolicies(ctx context.Context, teamID uint, opts ListOptions) ([]*Policy, error) + DeleteTeamPolicies(ctx context.Context, teamID uint, ids []uint) ([]uint, error) TeamPolicy(ctx context.Context, teamID uint, policyID uint) (*Policy, error) diff --git a/server/fleet/service.go b/server/fleet/service.go index c7d02a7a97..d47b60fcb3 100644 --- a/server/fleet/service.go +++ b/server/fleet/service.go @@ -642,7 +642,7 @@ type Service interface { // Team Policies NewTeamPolicy(ctx context.Context, teamID uint, p PolicyPayload) (*Policy, error) - ListTeamPolicies(ctx context.Context, teamID uint, opts ListOptions, iopts ListOptions) (teamPolicies, inheritedPolicies []*Policy, err error) + ListTeamPolicies(ctx context.Context, teamID uint, opts ListOptions, iopts ListOptions, mergeInherited bool) (teamPolicies, inheritedPolicies []*Policy, err error) DeleteTeamPolicies(ctx context.Context, teamID uint, ids []uint) ([]uint, error) ModifyTeamPolicy(ctx context.Context, teamID uint, id uint, p ModifyPolicyPayload) (*Policy, error) GetTeamPolicyByIDQueries(ctx context.Context, teamID uint, policyID uint) (*Policy, error) diff --git a/server/mock/datastore_mock.go b/server/mock/datastore_mock.go index 157695382b..50ab787cac 100644 --- a/server/mock/datastore_mock.go +++ b/server/mock/datastore_mock.go @@ -495,6 +495,8 @@ type NewTeamPolicyFunc func(ctx context.Context, teamID uint, authorID *uint, ar type ListTeamPoliciesFunc func(ctx context.Context, teamID uint, opts fleet.ListOptions, iopts fleet.ListOptions) (teamPolicies []*fleet.Policy, inheritedPolicies []*fleet.Policy, err error) +type ListMergedTeamPoliciesFunc func(ctx context.Context, teamID uint, opts fleet.ListOptions) ([]*fleet.Policy, error) + type DeleteTeamPoliciesFunc func(ctx context.Context, teamID uint, ids []uint) ([]uint, error) type TeamPolicyFunc func(ctx context.Context, teamID uint, policyID uint) (*fleet.Policy, error) @@ -1636,6 +1638,9 @@ type DataStore struct { ListTeamPoliciesFunc ListTeamPoliciesFunc ListTeamPoliciesFuncInvoked bool + ListMergedTeamPoliciesFunc ListMergedTeamPoliciesFunc + ListMergedTeamPoliciesFuncInvoked bool + DeleteTeamPoliciesFunc DeleteTeamPoliciesFunc DeleteTeamPoliciesFuncInvoked bool @@ -3944,6 +3949,13 @@ func (s *DataStore) ListTeamPolicies(ctx context.Context, teamID uint, opts flee return s.ListTeamPoliciesFunc(ctx, teamID, opts, iopts) } +func (s *DataStore) ListMergedTeamPolicies(ctx context.Context, teamID uint, opts fleet.ListOptions) ([]*fleet.Policy, error) { + s.mu.Lock() + s.ListMergedTeamPoliciesFuncInvoked = true + s.mu.Unlock() + return s.ListMergedTeamPoliciesFunc(ctx, teamID, opts) +} + func (s *DataStore) DeleteTeamPolicies(ctx context.Context, teamID uint, ids []uint) ([]uint, error) { s.mu.Lock() s.DeleteTeamPoliciesFuncInvoked = true diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 90b4c14b16..7c043d7dcd 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -844,6 +844,20 @@ func (s *integrationEnterpriseTestSuite) TestTeamPolicies() { assert.Equal(t, gpol.Name, ts.InheritedPolicies[0].Name) assert.Equal(t, gpol.ID, ts.InheritedPolicies[0].ID) + // Test merge inherited + ts = listTeamPoliciesResponse{} + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/teams/%d/policies", team1.ID), nil, http.StatusOK, &ts, "merge_inherited", "true", "order_key", "team_id", "order_direction", "desc") + require.Len(t, ts.Policies, 2) + require.Nil(t, ts.InheritedPolicies) + assert.Equal(t, "TestQuery2", ts.Policies[0].Name) + assert.Equal(t, "select * from osquery;", ts.Policies[0].Query) + assert.Equal(t, "Some description", ts.Policies[0].Description) + require.NotNil(t, ts.Policies[0].Resolution) + assert.Equal(t, "some team resolution", *ts.Policies[0].Resolution) + assert.Equal(t, gpol.Name, ts.Policies[1].Name) + assert.Equal(t, gpol.ID, ts.Policies[1].ID) + + // Test delete deletePolicyParams := deleteTeamPoliciesRequest{IDs: []uint{ts.Policies[0].ID}} deletePolicyResp := deleteTeamPoliciesResponse{} s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/teams/%d/policies/delete", team1.ID), deletePolicyParams, http.StatusOK, &deletePolicyResp) diff --git a/server/service/team_policies.go b/server/service/team_policies.go index 75cbe3ae96..ee55fa2a7f 100644 --- a/server/service/team_policies.go +++ b/server/service/team_policies.go @@ -106,6 +106,7 @@ type listTeamPoliciesRequest struct { InheritedPerPage uint `query:"inherited_per_page,optional"` InheritedOrderDirection fleet.OrderDirection `query:"inherited_order_direction,optional"` InheritedOrderKey string `query:"inherited_order_key,optional"` + MergeInherited bool `query:"merge_inherited,optional"` } type listTeamPoliciesResponse struct { @@ -126,14 +127,14 @@ func listTeamPoliciesEndpoint(ctx context.Context, request interface{}, svc flee OrderKey: req.InheritedOrderKey, } - tmPols, inheritedPols, err := svc.ListTeamPolicies(ctx, req.TeamID, req.Opts, inheritedListOptions) + tmPols, inheritedPols, err := svc.ListTeamPolicies(ctx, req.TeamID, req.Opts, inheritedListOptions, req.MergeInherited) if err != nil { return listTeamPoliciesResponse{Err: err}, nil } return listTeamPoliciesResponse{Policies: tmPols, InheritedPolicies: inheritedPols}, nil } -func (svc *Service) ListTeamPolicies(ctx context.Context, teamID uint, opts fleet.ListOptions, iopts fleet.ListOptions) (teamPolicies, inheritedPolicies []*fleet.Policy, err error) { +func (svc *Service) ListTeamPolicies(ctx context.Context, teamID uint, opts fleet.ListOptions, iopts fleet.ListOptions, mergeInherited bool) (teamPolicies, inheritedPolicies []*fleet.Policy, err error) { if err := svc.authz.Authorize(ctx, &fleet.Policy{ PolicyData: fleet.PolicyData{ TeamID: ptr.Uint(teamID), @@ -146,6 +147,11 @@ func (svc *Service) ListTeamPolicies(ctx context.Context, teamID uint, opts flee return nil, nil, ctxerr.Wrapf(ctx, err, "loading team %d", teamID) } + if mergeInherited { + p, err := svc.ds.ListMergedTeamPolicies(ctx, teamID, opts) + return p, nil, err + } + return svc.ds.ListTeamPolicies(ctx, teamID, opts, iopts) } diff --git a/server/service/team_policies_test.go b/server/service/team_policies_test.go index 9e1a502f67..6a2d35d4ff 100644 --- a/server/service/team_policies_test.go +++ b/server/service/team_policies_test.go @@ -149,7 +149,7 @@ func TestTeamPoliciesAuth(t *testing.T) { }) checkAuthErr(t, tt.shouldFailWrite, err) - _, _, err = svc.ListTeamPolicies(ctx, 1, fleet.ListOptions{}, fleet.ListOptions{}) + _, _, err = svc.ListTeamPolicies(ctx, 1, fleet.ListOptions{}, fleet.ListOptions{}, false) checkAuthErr(t, tt.shouldFailRead, err) _, err = svc.GetTeamPolicyByIDQueries(ctx, 1, 1)