From 8cfe2720918466fc35d9017784a65cc9a1b8353a Mon Sep 17 00:00:00 2001 From: Victor Lyuboslavsky Date: Wed, 22 Nov 2023 15:04:48 -0600 Subject: [PATCH] filtering hosts with invalid team_id now returns 400 error. (#15266) #15037 For endpoint fleet/hosts, filtering hosts with invalid team_id now returns 400 error. # Checklist for submitter If some of the following don't apply, delete the relevant line. - [x] Changes file added for user-visible changes in `changes/` or `orbit/changes/`. See [Changes files](https://fleetdm.com/docs/contributing/committing-changes#changes-files) for more information. - [x] Added/updated tests - [x] Manual QA for all new/changed functionality --- changes/15037-hosts-invalid-team_id-filter | 1 + server/datastore/mysql/hosts.go | 42 ++++++++++++++-------- server/datastore/mysql/hosts_test.go | 6 ++++ server/service/integration_core_test.go | 4 +++ 4 files changed, 38 insertions(+), 15 deletions(-) create mode 100644 changes/15037-hosts-invalid-team_id-filter diff --git a/changes/15037-hosts-invalid-team_id-filter b/changes/15037-hosts-invalid-team_id-filter new file mode 100644 index 0000000000..740c99934d --- /dev/null +++ b/changes/15037-hosts-invalid-team_id-filter @@ -0,0 +1 @@ +For endpoint fleet/hosts, filtering hosts with invalid team_id now returns 400 error. \ No newline at end of file diff --git a/server/datastore/mysql/hosts.go b/server/datastore/mysql/hosts.go index 28babe44f4..425c554464 100644 --- a/server/datastore/mysql/hosts.go +++ b/server/datastore/mysql/hosts.go @@ -917,7 +917,10 @@ func (ds *Datastore) ListHosts(ctx context.Context, filter fleet.TeamFilter, opt } // TODO(Sarah): Do we need to reconcile mutually exclusive filters? -func (ds *Datastore) applyHostFilters(ctx context.Context, opt fleet.HostListOptions, sql string, filter fleet.TeamFilter, params []interface{}, leftJoinFailingPolicies bool) (string, []interface{}, error) { +func (ds *Datastore) applyHostFilters( + ctx context.Context, opt fleet.HostListOptions, sqlStmt string, filter fleet.TeamFilter, params []interface{}, + leftJoinFailingPolicies bool, +) (string, []interface{}, error) { opt.OrderKey = defaultHostColumnTableAlias(opt.OrderKey) deviceMappingJoin := `LEFT JOIN ( @@ -977,7 +980,8 @@ func (ds *Datastore) applyHostFilters(ctx context.Context, opt fleet.HostListOpt params = append(params, *opt.LowDiskSpaceFilter) } - sql += fmt.Sprintf(`FROM hosts h + sqlStmt += fmt.Sprintf( + `FROM hosts h LEFT JOIN host_seen_times hst ON (h.id = hst.host_id) LEFT JOIN host_updates hu ON (h.id = hu.host_id) LEFT JOIN teams t ON (h.team_id = t.id) @@ -1009,26 +1013,34 @@ func (ds *Datastore) applyHostFilters(ctx context.Context, opt fleet.HostListOpt ) now := ds.clock.Now() - sql, params = filterHostsByStatus(now, sql, opt, params) - sql, params = filterHostsByTeam(sql, opt, params) - sql, params = filterHostsByPolicy(sql, opt, params) - sql, params = filterHostsByMDM(sql, opt, params) - sql, params = filterHostsByMacOSSettingsStatus(sql, opt, params) - sql, params = filterHostsByMacOSDiskEncryptionStatus(sql, opt, params) + sqlStmt, params = filterHostsByStatus(now, sqlStmt, opt, params) + sqlStmt, params = filterHostsByTeam(sqlStmt, opt, params) + sqlStmt, params = filterHostsByPolicy(sqlStmt, opt, params) + sqlStmt, params = filterHostsByMDM(sqlStmt, opt, params) + sqlStmt, params = filterHostsByMacOSSettingsStatus(sqlStmt, opt, params) + sqlStmt, params = filterHostsByMacOSDiskEncryptionStatus(sqlStmt, opt, params) if enableDiskEncryption, err := ds.getConfigEnableDiskEncryption(ctx, opt.TeamFilter); err != nil { + if errors.Is(err, sql.ErrNoRows) { + return "", nil, ctxerr.Wrap( + ctx, &fleet.BadRequestError{ + Message: fmt.Sprintf("team is invalid"), + InternalErr: err, + }, + ) + } return "", nil, err } else if opt.OSSettingsFilter.IsValid() { - sql, params = ds.filterHostsByOSSettingsStatus(sql, opt, params, enableDiskEncryption) + sqlStmt, params = ds.filterHostsByOSSettingsStatus(sqlStmt, opt, params, enableDiskEncryption) } else if opt.OSSettingsDiskEncryptionFilter.IsValid() { - sql, params = ds.filterHostsByOSSettingsDiskEncryptionStatus(sql, opt, params, enableDiskEncryption) + sqlStmt, params = ds.filterHostsByOSSettingsDiskEncryptionStatus(sqlStmt, opt, params, enableDiskEncryption) } - sql, params = filterHostsByMDMBootstrapPackageStatus(sql, opt, params) - sql, params = filterHostsByOS(sql, opt, params) - sql, params, _ = hostSearchLike(sql, params, opt.MatchQuery, hostSearchColumns...) - sql, params = appendListOptionsWithCursorToSQL(sql, params, &opt.ListOptions) + sqlStmt, params = filterHostsByMDMBootstrapPackageStatus(sqlStmt, opt, params) + sqlStmt, params = filterHostsByOS(sqlStmt, opt, params) + sqlStmt, params, _ = hostSearchLike(sqlStmt, params, opt.MatchQuery, hostSearchColumns...) + sqlStmt, params = appendListOptionsWithCursorToSQL(sqlStmt, params, &opt.ListOptions) - return sql, params, nil + return sqlStmt, params, nil } func filterHostsByTeam(sql string, opt fleet.HostListOptions, params []interface{}) (string, []interface{}) { diff --git a/server/datastore/mysql/hosts_test.go b/server/datastore/mysql/hosts_test.go index 22d76e62b1..aebcfe61f3 100644 --- a/server/datastore/mysql/hosts_test.go +++ b/server/datastore/mysql/hosts_test.go @@ -715,6 +715,7 @@ func listHostsCheckCount(t *testing.T, ds *Datastore, filter fleet.TeamFilter, o func testHostListOptionsTeamFilter(t *testing.T, ds *Datastore) { var teamIDFilterNil *uint // "All teams" option should include all hosts regardless of team assignment var teamIDFilterZero *uint = ptr.Uint(0) // "No team" option should include only hosts that are not assigned to any team + var teamIDFilterBad = ptr.Uint(9999) team1, err := ds.NewTeam(context.Background(), &fleet.Team{Name: "team1"}) require.NoError(t, err) @@ -820,6 +821,11 @@ func testHostListOptionsTeamFilter(t *testing.T, ds *Datastore) { listHostsCheckCount(t, ds, userFilter, fleet.HostListOptions{TeamFilter: teamIDFilterZero, OSSettingsDiskEncryptionFilter: fleet.DiskEncryptionEnforcing}, 1) // hosts[8] listHostsCheckCount(t, ds, userFilter, fleet.HostListOptions{TeamFilter: teamIDFilterNil, OSSettingsDiskEncryptionFilter: fleet.DiskEncryptionEnforcing}, 1) // hosts[8] listHostsCheckCount(t, ds, userFilter, fleet.HostListOptions{OSSettingsDiskEncryptionFilter: fleet.DiskEncryptionEnforcing}, 1) // hosts[8] + + // Bad team filter + _, err = ds.ListHosts(context.Background(), userFilter, fleet.HostListOptions{TeamFilter: teamIDFilterBad}) + require.Error(t, err) + require.True(t, strings.Contains(err.Error(), "team is invalid"), err) } func testHostsListFilterAdditional(t *testing.T, ds *Datastore) { diff --git a/server/service/integration_core_test.go b/server/service/integration_core_test.go index 7c07c24375..a37694f563 100644 --- a/server/service/integration_core_test.go +++ b/server/service/integration_core_test.go @@ -1446,6 +1446,10 @@ func (s *integrationTestSuite) TestListHosts() { resp = listHostsResponse{} s.DoJSON("GET", "/api/latest/fleet/hosts", nil, http.StatusNotFound, &resp, "software_id", fmt.Sprint(9999)) + // Filter by non-existent team. + resp = listHostsResponse{} + s.DoJSON("GET", "/api/latest/fleet/hosts", nil, http.StatusBadRequest, &resp, "team_id", fmt.Sprint(9999)) + // set munki information on a host require.NoError(t, s.ds.SetOrUpdateMunkiInfo(context.Background(), host.ID, "1.2.3", []string{"err"}, []string{"warn"})) var errMunkiID uint