From 92198a22b889b8150eddeb8111c2c8a20eaeff33 Mon Sep 17 00:00:00 2001 From: Jacob Shandling <61553566+jacobshandling@users.noreply.github.com> Date: Mon, 10 Jun 2024 10:46:16 -0700 Subject: [PATCH] Delete team policies: 404 for nonexistent team (#19516) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Addresses #18993 - Return `404` when a user tries to delete team policies from a non-existent team – see [this precedent in the codebase](https://github.com/fleetdm/fleet/blob/6b3310aa51bbc54282a0f2cd7d527997448c961f/server/service/integration_core_test.go#L6212) for a 404 in this situation - Add missing authorization check for this action Screenshot 2024-06-04 at 6 22 02 PM - [x] Changes file added for user-visible changes in `changes/`, - [x] Added/updated tests - [x] Manual QA for all new/changed functionality --------- Co-authored-by: Jacob Shandling --- ...3-404-when-no-team-on-delete-team-policies | 1 + server/service/integration_core_test.go | 3 +++ server/service/team_policies.go | 19 ++++++++++++------- 3 files changed, 16 insertions(+), 7 deletions(-) create mode 100644 changes/18993-404-when-no-team-on-delete-team-policies diff --git a/changes/18993-404-when-no-team-on-delete-team-policies b/changes/18993-404-when-no-team-on-delete-team-policies new file mode 100644 index 0000000000..98a8b3e171 --- /dev/null +++ b/changes/18993-404-when-no-team-on-delete-team-policies @@ -0,0 +1 @@ +* Error with 404 when the user attempts to delete team policies for a non-existent team diff --git a/server/service/integration_core_test.go b/server/service/integration_core_test.go index a5af5c5d36..1f7b1feca3 100644 --- a/server/service/integration_core_test.go +++ b/server/service/integration_core_test.go @@ -6211,6 +6211,9 @@ func (s *integrationTestSuite) TestTeamPoliciesTeamNotExists() { teamPoliciesResponse := listTeamPoliciesResponse{} s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/teams/%d/policies", 9999999), nil, http.StatusNotFound, &teamPoliciesResponse) require.Len(t, teamPoliciesResponse.Policies, 0) + + deleteTeamPoliciesResponse := deleteTeamPoliciesResponse{} + s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/teams/%d/policies/delete", 9999999), deleteTeamPoliciesRequest{IDs: []uint{1, 1000}}, http.StatusNotFound, &deleteTeamPoliciesResponse) } func (s *integrationTestSuite) TestSessionInfo() { diff --git a/server/service/team_policies.go b/server/service/team_policies.go index 62540aef94..81ebee7d40 100644 --- a/server/service/team_policies.go +++ b/server/service/team_policies.go @@ -269,6 +269,18 @@ func deleteTeamPoliciesEndpoint(ctx context.Context, request interface{}, svc fl } func (svc Service) DeleteTeamPolicies(ctx context.Context, teamID uint, ids []uint) ([]uint, error) { + if err := svc.authz.Authorize(ctx, &fleet.Policy{ + PolicyData: fleet.PolicyData{ + TeamID: ptr.Uint(teamID), + }, + }, fleet.ActionWrite); err != nil { + return nil, err + } + + if _, err := svc.ds.Team(ctx, teamID); err != nil { + return nil, ctxerr.Wrapf(ctx, err, "loading team %d", teamID) + } + if len(ids) == 0 { return nil, nil } @@ -277,13 +289,6 @@ func (svc Service) DeleteTeamPolicies(ctx context.Context, teamID uint, ids []ui return nil, ctxerr.Wrap(ctx, err, "getting policies by ID") } - if err := svc.authz.Authorize(ctx, &fleet.Policy{ - PolicyData: fleet.PolicyData{ - TeamID: ptr.Uint(teamID), - }, - }, fleet.ActionWrite); err != nil { - return nil, err - } for _, policy := range policiesByID { if t := policy.PolicyData.TeamID; t == nil || *t != teamID { return nil, authz.ForbiddenWithInternal(