From ddc6b300d43542176831c28287692a8b026387b3 Mon Sep 17 00:00:00 2001 From: Tomas Touceda Date: Tue, 5 Oct 2021 15:15:05 -0300 Subject: [PATCH] Allow team maintainers to delete hosts from their teams (#2373) --- ...issue-2357-team-maintainer-can-delete-host | 1 + server/authz/policy.rego | 7 + server/authz/policy_test.go | 2 +- server/service/service_hosts.go | 12 +- server/service/service_hosts_test.go | 143 ++++++++++++++++++ 5 files changed, 160 insertions(+), 5 deletions(-) create mode 100644 changes/issue-2357-team-maintainer-can-delete-host diff --git a/changes/issue-2357-team-maintainer-can-delete-host b/changes/issue-2357-team-maintainer-can-delete-host new file mode 100644 index 0000000000..bcc851fe97 --- /dev/null +++ b/changes/issue-2357-team-maintainer-can-delete-host @@ -0,0 +1 @@ +* Team maintainers can delete hosts from their teams. diff --git a/server/authz/policy.rego b/server/authz/policy.rego index ca1f093e54..ca26a4ee57 100644 --- a/server/authz/policy.rego +++ b/server/authz/policy.rego @@ -209,6 +209,13 @@ allow { action == read } +# Team maintainers can write to hosts of their own team +allow { + object.type == "host" + team_role(subject, object.team_id) == maintainer + action == write +} + ## # Labels ## diff --git a/server/authz/policy_test.go b/server/authz/policy_test.go index 2bc3994d1e..44c6f307da 100644 --- a/server/authz/policy_test.go +++ b/server/authz/policy_test.go @@ -318,7 +318,7 @@ func TestAuthorizeHost(t *testing.T) { {user: teamMaintainer, object: host, action: write, allow: false}, {user: teamMaintainer, object: host, action: list, allow: true}, {user: teamMaintainer, object: hostTeam1, action: read, allow: true}, - {user: teamMaintainer, object: hostTeam1, action: write, allow: false}, + {user: teamMaintainer, object: hostTeam1, action: write, allow: true}, {user: teamMaintainer, object: hostTeam2, action: read, allow: false}, {user: teamMaintainer, object: hostTeam2, action: write, allow: false}, }) diff --git a/server/service/service_hosts.go b/server/service/service_hosts.go index 2e0c6b237d..a5a10cb3a1 100644 --- a/server/service/service_hosts.go +++ b/server/service/service_hosts.go @@ -43,7 +43,7 @@ func (svc Service) GetHost(ctx context.Context, id uint) (*fleet.HostDetail, err } func (svc Service) HostByIdentifier(ctx context.Context, identifier string) (*fleet.HostDetail, error) { - if err := svc.authz.Authorize(ctx, &fleet.Host{}, fleet.ActionRead); err != nil { + if err := svc.authz.Authorize(ctx, &fleet.Host{}, fleet.ActionList); err != nil { return nil, err } @@ -101,7 +101,7 @@ func (svc Service) GetHostSummary(ctx context.Context) (*fleet.HostSummary, erro } func (svc Service) DeleteHost(ctx context.Context, id uint) error { - if err := svc.authz.Authorize(ctx, &fleet.Host{}, fleet.ActionWrite); err != nil { + if err := svc.authz.Authorize(ctx, &fleet.Host{}, fleet.ActionList); err != nil { return err } @@ -130,7 +130,7 @@ func (svc Service) AddHostsToTeam(ctx context.Context, teamID *uint, hostIDs []u // besides global admins permissions to modify team hosts, we will need to // check that the user has permissions for both the source and destination // teams. - if err := svc.authz.Authorize(ctx, &fleet.Host{}, fleet.ActionWrite); err != nil { + if err := svc.authz.Authorize(ctx, &fleet.Host{TeamID: teamID}, fleet.ActionWrite); err != nil { return err } @@ -142,7 +142,7 @@ func (svc Service) AddHostsToTeamByFilter(ctx context.Context, teamID *uint, opt // besides global admins permissions to modify team hosts, we will need to // check that the user has permissions for both the source and destination // teams. - if err := svc.authz.Authorize(ctx, &fleet.Host{}, fleet.ActionWrite); err != nil { + if err := svc.authz.Authorize(ctx, &fleet.Host{TeamID: teamID}, fleet.ActionWrite); err != nil { return err } hostIDs, err := svc.hostIDsFromFilters(ctx, opt, lid) @@ -194,6 +194,10 @@ func (svc Service) hostIDsFromFilters(ctx context.Context, opt fleet.HostListOpt } func (svc *Service) RefetchHost(ctx context.Context, id uint) error { + if err := svc.authz.Authorize(ctx, &fleet.Host{}, fleet.ActionList); err != nil { + return err + } + host, err := svc.ds.Host(ctx, id) if err != nil { return errors.Wrap(err, "find host for refetch") diff --git a/server/service/service_hosts_test.go b/server/service/service_hosts_test.go index d288d2ff6e..33263d0a50 100644 --- a/server/service/service_hosts_test.go +++ b/server/service/service_hosts_test.go @@ -6,6 +6,7 @@ import ( "time" "github.com/WatchBeam/clock" + "github.com/fleetdm/fleet/v4/server/contexts/viewer" "github.com/fleetdm/fleet/v4/server/datastore/mysql" "github.com/fleetdm/fleet/v4/server/fleet" "github.com/fleetdm/fleet/v4/server/mock" @@ -210,3 +211,145 @@ func TestAddHostsToTeamByFilterEmptyHosts(t *testing.T) { require.NoError(t, svc.AddHostsToTeamByFilter(test.UserContext(test.UserAdmin), nil, fleet.HostListOptions{}, nil)) } + +func TestHostAuth(t *testing.T) { + ds := new(mock.Store) + svc := newTestService(ds, nil, nil) + + teamHost := &fleet.Host{TeamID: ptr.Uint(1)} + globalHost := &fleet.Host{} + + ds.DeleteHostFunc = func(ctx context.Context, hid uint) error { + return nil + } + ds.HostFunc = func(ctx context.Context, id uint) (*fleet.Host, error) { + if id == 1 { + return teamHost, nil + } + return globalHost, nil + } + ds.HostByIdentifierFunc = func(ctx context.Context, identifier string) (*fleet.Host, error) { + if identifier == "1" { + return teamHost, nil + } + return globalHost, nil + } + ds.ListHostsFunc = func(ctx context.Context, filter fleet.TeamFilter, opt fleet.HostListOptions) ([]*fleet.Host, error) { + return nil, nil + } + ds.LoadHostSoftwareFunc = func(ctx context.Context, host *fleet.Host) error { + return nil + } + ds.ListLabelsForHostFunc = func(ctx context.Context, hid uint) ([]*fleet.Label, error) { + return nil, nil + } + ds.ListPacksForHostFunc = func(ctx context.Context, hid uint) (packs []*fleet.Pack, err error) { + return nil, nil + } + ds.AddHostsToTeamFunc = func(ctx context.Context, teamID *uint, hostIDs []uint) error { + return nil + } + ds.SaveHostFunc = func(ctx context.Context, host *fleet.Host) error { + return nil + } + + var testCases = []struct { + name string + user *fleet.User + shouldFailGlobalWrite bool + shouldFailGlobalRead bool + shouldFailTeamWrite bool + shouldFailTeamRead bool + }{ + { + "global admin", + &fleet.User{GlobalRole: ptr.String(fleet.RoleAdmin)}, + false, + false, + false, + false, + }, + { + "global maintainer", + &fleet.User{GlobalRole: ptr.String(fleet.RoleMaintainer)}, + false, + false, + false, + false, + }, + { + "global observer", + &fleet.User{GlobalRole: ptr.String(fleet.RoleObserver)}, + true, + false, + true, + false, + }, + { + "team maintainer, belongs to team", + &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 1}, Role: fleet.RoleMaintainer}}}, + true, + true, + false, + false, + }, + { + "team observer, belongs to team", + &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 1}, Role: fleet.RoleObserver}}}, + true, + true, + true, + false, + }, + { + "team maintainer, DOES NOT belong to team", + &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 2}, Role: fleet.RoleMaintainer}}}, + true, + true, + true, + true, + }, + { + "team observer, DOES NOT belong to team", + &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 2}, Role: fleet.RoleObserver}}}, + true, + true, + true, + true, + }, + } + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + ctx := viewer.NewContext(context.Background(), viewer.Viewer{User: tt.user}) + + _, err := svc.GetHost(ctx, 1) + checkAuthErr(t, tt.shouldFailTeamRead, err) + + _, err = svc.HostByIdentifier(ctx, "1") + checkAuthErr(t, tt.shouldFailTeamRead, err) + + _, err = svc.GetHost(ctx, 2) + checkAuthErr(t, tt.shouldFailGlobalRead, err) + + _, err = svc.HostByIdentifier(ctx, "2") + checkAuthErr(t, tt.shouldFailGlobalRead, err) + + err = svc.DeleteHost(ctx, 1) + checkAuthErr(t, tt.shouldFailTeamWrite, err) + + err = svc.DeleteHost(ctx, 2) + checkAuthErr(t, tt.shouldFailGlobalWrite, err) + + err = svc.AddHostsToTeam(ctx, ptr.Uint(1), []uint{1}) + checkAuthErr(t, tt.shouldFailTeamWrite, err) + + err = svc.AddHostsToTeamByFilter(ctx, ptr.Uint(1), fleet.HostListOptions{}, nil) + checkAuthErr(t, tt.shouldFailTeamWrite, err) + + err = svc.RefetchHost(ctx, 1) + checkAuthErr(t, tt.shouldFailTeamRead, err) + }) + } + + // List, GetHostSummary, FlushSeenHost work for all +}