Fixing hosts/delete input validation due to QA issue. (#14942)

Frontend needs to be able to delete all hosts. However, the API
requirement is that host ids or filters must be specified when deleting
hosts. The solution is to allow an empty filter to delete all hosts,
like: `"filters":{}`

REST API updated documentation here:
https://github.com/fleetdm/fleet/pull/14952

[x] Tests updated and added.
[x] Manual testing done.

server/fleet/service.go

@@ -339,7 +339,7 @@ type Service interface {
 	// AddHostsToTeamByFilter adds hosts to an existing team, clearing their team settings if teamID is nil. Hosts are
 	// selected by the label and HostListOptions provided.
 	AddHostsToTeamByFilter(ctx context.Context, teamID *uint, opt HostListOptions, lid *uint) error
-	DeleteHosts(ctx context.Context, ids []uint, opt HostListOptions, lid *uint) error
+	DeleteHosts(ctx context.Context, ids []uint, opt *HostListOptions, lid *uint) error
 	CountHosts(ctx context.Context, labelID *uint, opts HostListOptions) (int, error)
 	// SearchHosts performs a search on the hosts table using the following criteria:
 	//	- matchQuery is the query SQL

server/service/hosts.go

@@ -167,14 +167,17 @@ var (
 	deleteHostsSkipAuthorization = false
 )
 
+type deleteHostsFilters struct {
+	MatchQuery string           `json:"query"`
+	Status     fleet.HostStatus `json:"status"`
+	LabelID    *uint            `json:"label_id"`
+	TeamID     *uint            `json:"team_id"`
+}
+
 type deleteHostsRequest struct {
-	IDs     []uint `json:"ids"`
-	Filters struct {
-		MatchQuery string           `json:"query"`
-		Status     fleet.HostStatus `json:"status"`
-		LabelID    *uint            `json:"label_id"`
-		TeamID     *uint            `json:"team_id"`
-	} `json:"filters"`
+	IDs []uint `json:"ids"`
+	// Using a pointer to help determine whether an empty filter was passed, like: "filters":{}
+	Filters *deleteHostsFilters `json:"filters"`
 }
 
 type deleteHostsResponse struct {
@@ -189,12 +192,17 @@ func (r deleteHostsResponse) Status() int { return r.StatusCode }
 
 func deleteHostsEndpoint(ctx context.Context, request interface{}, svc fleet.Service) (errorer, error) {
 	req := request.(*deleteHostsRequest)
-	listOpt := fleet.HostListOptions{
-		ListOptions: fleet.ListOptions{
-			MatchQuery: req.Filters.MatchQuery,
-		},
-		StatusFilter: req.Filters.Status,
-		TeamFilter:   req.Filters.TeamID,
+	var listOpts *fleet.HostListOptions
+	var labelID *uint
+	if req.Filters != nil {
+		listOpts = &fleet.HostListOptions{
+			ListOptions: fleet.ListOptions{
+				MatchQuery: req.Filters.MatchQuery,
+			},
+			StatusFilter: req.Filters.Status,
+			TeamFilter:   req.Filters.TeamID,
+		}
+		labelID = req.Filters.LabelID
 	}
 
 	// Since bulk deletes can take a long time, after DeleteHostsTimeout, we will return a 202 (Accepted) status code
@@ -203,7 +211,7 @@ func deleteHostsEndpoint(ctx context.Context, request interface{}, svc fleet.Ser
 	deleteDone := make(chan bool, 1)
 	ctx = context.WithoutCancel(ctx) // to make sure DB operations don't get killed after we return a 202
 	go func() {
-		err = svc.DeleteHosts(ctx, req.IDs, listOpt, req.Filters.LabelID)
+		err = svc.DeleteHosts(ctx, req.IDs, listOpts, labelID)
 		if err != nil {
 			// logging the error for future debug in case we already sent http.StatusAccepted
 			logging.WithErr(ctx, err)
@@ -225,16 +233,16 @@ func deleteHostsEndpoint(ctx context.Context, request interface{}, svc fleet.Ser
 	}
 }
 
-func (svc *Service) DeleteHosts(ctx context.Context, ids []uint, opts fleet.HostListOptions, lid *uint) error {
+func (svc *Service) DeleteHosts(ctx context.Context, ids []uint, opts *fleet.HostListOptions, lid *uint) error {
 	if err := svc.authz.Authorize(ctx, &fleet.Host{}, fleet.ActionList); err != nil {
 		return err
 	}
 
-	if len(ids) == 0 && lid == nil && opts.Empty() {
+	if len(ids) == 0 && lid == nil && opts == nil {
 		return &fleet.BadRequestError{Message: "list of ids or filters must be specified"}
 	}
 
-	if len(ids) > 0 && (lid != nil || !opts.Empty()) {
+	if len(ids) > 0 && (lid != nil || (opts != nil && !opts.Empty())) {
 		return &fleet.BadRequestError{Message: "Cannot specify a list of ids and filters at the same time"}
 	}
 
@@ -246,8 +254,11 @@ func (svc *Service) DeleteHosts(ctx context.Context, ids []uint, opts fleet.Host
 		return svc.ds.DeleteHosts(ctx, ids)
 	}
 
+	if opts == nil {
+		opts = &fleet.HostListOptions{}
+	}
 	opts.DisableFailingPolicies = true // don't check policies for hosts that are about to be deleted
-	hostIDs, _, err := svc.hostIDsAndNamesFromFilters(ctx, opts, lid)
+	hostIDs, _, err := svc.hostIDsAndNamesFromFilters(ctx, *opts, lid)
 	if err != nil {
 		return err
 	}

server/service/hosts_test.go

@@ -667,10 +667,10 @@ func TestHostAuth(t *testing.T) {
 			err = svc.DeleteHost(ctx, 2)
 			checkAuthErr(t, tt.shouldFailGlobalWrite, err)
 
-			err = svc.DeleteHosts(ctx, []uint{1}, fleet.HostListOptions{}, nil)
+			err = svc.DeleteHosts(ctx, []uint{1}, nil, nil)
 			checkAuthErr(t, tt.shouldFailTeamWrite, err)
 
-			err = svc.DeleteHosts(ctx, []uint{2}, fleet.HostListOptions{}, nil)
+			err = svc.DeleteHosts(ctx, []uint{2}, &fleet.HostListOptions{}, nil)
 			checkAuthErr(t, tt.shouldFailGlobalWrite, err)
 
 			err = svc.AddHostsToTeam(ctx, ptr.Uint(1), []uint{1}, false)

server/service/integration_core_test.go

@@ -878,12 +878,7 @@ func (s *integrationTestSuite) TestBulkDeleteHostsFromTeam() {
 	require.NoError(t, s.ds.AddHostsToTeam(context.Background(), &team1.ID, []uint{hosts[0].ID}))
 
 	req := deleteHostsRequest{
-		Filters: struct {
-			MatchQuery string           `json:"query"`
-			Status     fleet.HostStatus `json:"status"`
-			LabelID    *uint            `json:"label_id"`
-			TeamID     *uint            `json:"team_id"`
-		}{TeamID: ptr.Uint(team1.ID)},
+		Filters: &deleteHostsFilters{TeamID: ptr.Uint(team1.ID)},
 	}
 	resp := deleteHostsResponse{}
 	s.DoJSON("POST", "/api/latest/fleet/hosts/delete", req, http.StatusOK, &resp)
@@ -920,12 +915,7 @@ func (s *integrationTestSuite) TestBulkDeleteHostsInLabel() {
 	require.NoError(t, s.ds.RecordLabelQueryExecutions(context.Background(), hosts[2], map[uint]*bool{label.ID: ptr.Bool(true)}, time.Now(), false))
 
 	req := deleteHostsRequest{
-		Filters: struct {
-			MatchQuery string           `json:"query"`
-			Status     fleet.HostStatus `json:"status"`
-			LabelID    *uint            `json:"label_id"`
-			TeamID     *uint            `json:"team_id"`
-		}{LabelID: ptr.Uint(label.ID)},
+		Filters: &deleteHostsFilters{LabelID: ptr.Uint(label.ID)},
 	}
 	resp := deleteHostsResponse{}
 	s.DoJSON("POST", "/api/latest/fleet/hosts/delete", req, http.StatusOK, &resp)
@@ -1001,6 +991,26 @@ func (s *integrationTestSuite) TestBulkDeleteHostByIDsWithTimeout() {
 	}
 }
 
+func (s *integrationTestSuite) TestBulkDeleteHostsAll() {
+	t := s.T()
+
+	hosts := s.createHosts(t)
+
+	// All hosts should be deleted when an empty filter is specified
+	req := deleteHostsRequest{
+		Filters: &deleteHostsFilters{},
+	}
+	resp := deleteHostsResponse{}
+	s.DoJSON("POST", "/api/latest/fleet/hosts/delete", req, http.StatusOK, &resp)
+
+	_, err := s.ds.Host(context.Background(), hosts[0].ID)
+	require.Error(t, err)
+	_, err = s.ds.Host(context.Background(), hosts[1].ID)
+	require.Error(t, err)
+	_, err = s.ds.Host(context.Background(), hosts[2].ID)
+	require.Error(t, err)
+}
+
 func (s *integrationTestSuite) createHosts(t *testing.T, platforms ...string) []*fleet.Host {
 	var hosts []*fleet.Host
 	if len(platforms) == 0 {
@@ -1030,16 +1040,15 @@ func (s *integrationTestSuite) TestBulkDeleteHostsErrors() {
 	hosts := s.createHosts(t)
 
 	req := deleteHostsRequest{
-		IDs: []uint{hosts[0].ID, hosts[1].ID},
-		Filters: struct {
-			MatchQuery string           `json:"query"`
-			Status     fleet.HostStatus `json:"status"`
-			LabelID    *uint            `json:"label_id"`
-			TeamID     *uint            `json:"team_id"`
-		}{LabelID: ptr.Uint(1)},
+		IDs:     []uint{hosts[0].ID, hosts[1].ID},
+		Filters: &deleteHostsFilters{LabelID: ptr.Uint(1)},
 	}
 	resp := deleteHostsResponse{}
 	s.DoJSON("POST", "/api/latest/fleet/hosts/delete", req, http.StatusBadRequest, &resp)
+
+	req = deleteHostsRequest{}
+	// No ids or filter specified
+	s.DoJSON("POST", "/api/latest/fleet/hosts/delete", req, http.StatusBadRequest, &resp)
 }
 
 func (s *integrationTestSuite) TestHostsCount() {