diff --git a/changes/allow-team-maintainers-to-read-write-schedules b/changes/allow-team-maintainers-to-read-write-schedules new file mode 100644 index 0000000000..fd729876ae --- /dev/null +++ b/changes/allow-team-maintainers-to-read-write-schedules @@ -0,0 +1 @@ +* Team maintainers can now modify their team's schedules. diff --git a/server/authz/authz.go b/server/authz/authz.go index 35d02b96ac..91e8895e4b 100644 --- a/server/authz/authz.go +++ b/server/authz/authz.go @@ -119,41 +119,6 @@ func (a *Authorizer) Authorize(ctx context.Context, object, action interface{}) return nil } -func (a *Authorizer) TeamAuthorize(ctx context.Context, teamID uint, action string) error { - subject := UserFromContext(ctx) - if subject == nil { - return ForbiddenWithInternal("nil subject always forbidden", subject, nil, action) - } - - // global admins and maintainers are authorized to work with teams - if subject.GlobalRole != nil { - switch *subject.GlobalRole { - case fleet.RoleAdmin, fleet.RoleMaintainer: - return nil - case fleet.RoleObserver: - if action == fleet.ActionRead { - return nil - } - } - } - - for _, team := range subject.Teams { - if teamID == team.ID { - switch action { - case fleet.ActionWrite: - if team.Role == fleet.RoleMaintainer { - return nil - } - return ForbiddenWithInternal("team observer cannot write", subject, nil, action) - default: - return nil - } - } - } - - return ForbiddenWithInternal("not a member of the team", subject, nil, action) -} - // AuthzTyper is the interface that may be implemented to get a `type` // property added during marshaling for authorization. Any struct that will be // used as a subject or object in authorization should implement this interface. diff --git a/server/authz/policy.rego b/server/authz/policy.rego index ff9a5ecf8a..0242490165 100644 --- a/server/authz/policy.rego +++ b/server/authz/policy.rego @@ -334,7 +334,7 @@ allow { # Packs ## -# Only global admins and maintainers can read/write packs +# Global admins and maintainers and team maintainers can read/write packs allow { object.type == "pack" subject.global_role == admin @@ -345,6 +345,12 @@ allow { subject.global_role == maintainer action == [read, write][_] } +allow { + object.team_ids[_] == subject.teams[_].id + object.type == "pack" + team_role(subject, subject.teams[_].id) == maintainer + action == [read, write][_] +} ## # File Carves @@ -391,6 +397,7 @@ allow { # Team Maintainers can read and write policies allow { not is_null(object.team_id) + object.team_id == subject.teams[_].id object.type == "policy" team_role(subject, subject.teams[_].id) == maintainer action == [read, write][_] @@ -399,6 +406,7 @@ allow { # Team Observer can read policies allow { not is_null(object.team_id) + object.team_id == subject.teams[_].id object.type == "policy" team_role(subject, subject.teams[_].id) == observer action == [read][_] diff --git a/server/service/service_scheduled_queries.go b/server/service/service_scheduled_queries.go index cfed61955c..942deb87c8 100644 --- a/server/service/service_scheduled_queries.go +++ b/server/service/service_scheduled_queries.go @@ -30,6 +30,10 @@ func (svc *Service) ScheduleQuery(ctx context.Context, sq *fleet.ScheduledQuery) return nil, err } + return svc.unauthorizedScheduleQuery(ctx, sq) +} + +func (svc *Service) unauthorizedScheduleQuery(ctx context.Context, sq *fleet.ScheduledQuery) (*fleet.ScheduledQuery, error) { // Fill in the name with query name if it is unset (because the UI // doesn't provide a way to set it) if sq.Name == "" { @@ -71,7 +75,11 @@ func (svc *Service) ModifyScheduledQuery(ctx context.Context, id uint, p fleet.S return nil, err } - sq, err := svc.GetScheduledQuery(ctx, id) + return svc.unauthorizedModifyScheduledQuery(ctx, id, p) +} + +func (svc *Service) unauthorizedModifyScheduledQuery(ctx context.Context, id uint, p fleet.ScheduledQueryPayload) (*fleet.ScheduledQuery, error) { + sq, err := svc.ds.ScheduledQuery(ctx, id) if err != nil { return nil, errors.Wrap(err, "getting scheduled query to modify") } diff --git a/server/service/team_policies.go b/server/service/team_policies.go index 9ea4febc28..e4bd6fc5e4 100644 --- a/server/service/team_policies.go +++ b/server/service/team_policies.go @@ -36,9 +36,6 @@ func (svc Service) NewTeamPolicy(ctx context.Context, teamID uint, queryID uint) if err := svc.authz.Authorize(ctx, &fleet.Policy{TeamID: ptr.Uint(teamID)}, fleet.ActionWrite); err != nil { return nil, err } - if err := svc.authz.TeamAuthorize(ctx, teamID, fleet.ActionWrite); err != nil { - return nil, err - } return svc.ds.NewTeamPolicy(ctx, teamID, queryID) } @@ -71,9 +68,6 @@ func (svc Service) ListTeamPolicies(ctx context.Context, teamID uint) ([]*fleet. if err := svc.authz.Authorize(ctx, &fleet.Policy{TeamID: ptr.Uint(teamID)}, fleet.ActionRead); err != nil { return nil, err } - if err := svc.authz.TeamAuthorize(ctx, teamID, fleet.ActionRead); err != nil { - return nil, err - } return svc.ds.ListTeamPolicies(ctx, teamID) } @@ -107,9 +101,6 @@ func (svc Service) GetTeamPolicyByIDQueries(ctx context.Context, teamID uint, po if err := svc.authz.Authorize(ctx, &fleet.Policy{TeamID: ptr.Uint(teamID)}, fleet.ActionRead); err != nil { return nil, err } - if err := svc.authz.TeamAuthorize(ctx, teamID, fleet.ActionRead); err != nil { - return nil, err - } teamPolicy, err := svc.ds.TeamPolicy(ctx, teamID, policyID) if err != nil { @@ -148,9 +139,6 @@ func (svc Service) DeleteTeamPolicies(ctx context.Context, teamID uint, ids []ui if err := svc.authz.Authorize(ctx, &fleet.Policy{TeamID: ptr.Uint(teamID)}, fleet.ActionWrite); err != nil { return nil, err } - if err := svc.authz.TeamAuthorize(ctx, teamID, fleet.ActionWrite); err != nil { - return nil, err - } return svc.ds.DeleteTeamPolicies(ctx, teamID, ids) } diff --git a/server/service/team_schedule.go b/server/service/team_schedule.go index 9561cd2595..b427c881de 100644 --- a/server/service/team_schedule.go +++ b/server/service/team_schedule.go @@ -36,10 +36,7 @@ func getTeamScheduleEndpoint(ctx context.Context, request interface{}, svc fleet } func (svc Service) GetTeamScheduledQueries(ctx context.Context, teamID uint, opts fleet.ListOptions) ([]*fleet.ScheduledQuery, error) { - if err := svc.authz.Authorize(ctx, &fleet.Pack{}, fleet.ActionRead); err != nil { - return nil, err - } - if err := svc.authz.TeamAuthorize(ctx, teamID, fleet.ActionRead); err != nil { + if err := svc.authz.Authorize(ctx, &fleet.Pack{TeamIDs: []uint{teamID}}, fleet.ActionRead); err != nil { return nil, err } @@ -102,10 +99,7 @@ func teamScheduleQueryEndpoint(ctx context.Context, request interface{}, svc fle } func (svc Service) TeamScheduleQuery(ctx context.Context, teamID uint, q *fleet.ScheduledQuery) (*fleet.ScheduledQuery, error) { - if err := svc.authz.Authorize(ctx, &fleet.Pack{}, fleet.ActionRead); err != nil { - return nil, err - } - if err := svc.authz.TeamAuthorize(ctx, teamID, fleet.ActionRead); err != nil { + if err := svc.authz.Authorize(ctx, &fleet.Pack{TeamIDs: []uint{teamID}}, fleet.ActionWrite); err != nil { return nil, err } @@ -115,7 +109,7 @@ func (svc Service) TeamScheduleQuery(ctx context.Context, teamID uint, q *fleet. } q.PackID = gp.ID - return svc.ScheduleQuery(ctx, q) + return svc.unauthorizedScheduleQuery(ctx, q) } ///////////////////////////////////////////////////////////////////////////////// @@ -146,10 +140,7 @@ func modifyTeamScheduleEndpoint(ctx context.Context, request interface{}, svc fl } func (svc Service) ModifyTeamScheduledQueries(ctx context.Context, teamID uint, scheduledQueryID uint, query fleet.ScheduledQueryPayload) (*fleet.ScheduledQuery, error) { - if err := svc.authz.Authorize(ctx, &fleet.Pack{}, fleet.ActionWrite); err != nil { - return nil, err - } - if err := svc.authz.TeamAuthorize(ctx, teamID, fleet.ActionWrite); err != nil { + if err := svc.authz.Authorize(ctx, &fleet.Pack{TeamIDs: []uint{teamID}}, fleet.ActionWrite); err != nil { return nil, err } @@ -160,7 +151,7 @@ func (svc Service) ModifyTeamScheduledQueries(ctx context.Context, teamID uint, query.PackID = ptr.Uint(gp.ID) - return svc.ModifyScheduledQuery(ctx, scheduledQueryID, query) + return svc.unauthorizedModifyScheduledQuery(ctx, scheduledQueryID, query) } ///////////////////////////////////////////////////////////////////////////////// @@ -189,11 +180,8 @@ func deleteTeamScheduleEndpoint(ctx context.Context, request interface{}, svc fl } func (svc Service) DeleteTeamScheduledQueries(ctx context.Context, teamID uint, scheduledQueryID uint) error { - if err := svc.authz.Authorize(ctx, &fleet.Pack{}, fleet.ActionWrite); err != nil { + if err := svc.authz.Authorize(ctx, &fleet.Pack{TeamIDs: []uint{teamID}}, fleet.ActionWrite); err != nil { return err } - if err := svc.authz.TeamAuthorize(ctx, teamID, fleet.ActionWrite); err != nil { - return err - } - return svc.DeleteScheduledQuery(ctx, scheduledQueryID) + return svc.ds.DeleteScheduledQuery(ctx, scheduledQueryID) } diff --git a/server/service/team_schedule_test.go b/server/service/team_schedule_test.go new file mode 100644 index 0000000000..1d23274d52 --- /dev/null +++ b/server/service/team_schedule_test.go @@ -0,0 +1,105 @@ +package service + +import ( + "context" + "testing" + + "github.com/fleetdm/fleet/v4/server/contexts/viewer" + "github.com/fleetdm/fleet/v4/server/fleet" + "github.com/fleetdm/fleet/v4/server/mock" + "github.com/fleetdm/fleet/v4/server/ptr" +) + +func TestTeamScheduleAuth(t *testing.T) { + ds := new(mock.Store) + svc := newTestService(ds, nil, nil) + + ds.EnsureTeamPackFunc = func(ctx context.Context, teamID uint) (*fleet.Pack, error) { + return &fleet.Pack{ID: 999}, nil + } + ds.ListScheduledQueriesInPackFunc = func(ctx context.Context, id uint, opts fleet.ListOptions) ([]*fleet.ScheduledQuery, error) { + return nil, nil + } + ds.QueryFunc = func(ctx context.Context, id uint) (*fleet.Query, error) { + return &fleet.Query{}, nil + } + ds.ScheduledQueryFunc = func(ctx context.Context, id uint) (*fleet.ScheduledQuery, error) { + return &fleet.ScheduledQuery{}, nil + } + ds.NewScheduledQueryFunc = func(ctx context.Context, sq *fleet.ScheduledQuery, opts ...fleet.OptionalArg) (*fleet.ScheduledQuery, error) { + return sq, nil + } + ds.SaveScheduledQueryFunc = func(ctx context.Context, sq *fleet.ScheduledQuery) (*fleet.ScheduledQuery, error) { + return sq, nil + } + ds.DeleteScheduledQueryFunc = func(ctx context.Context, id uint) error { + return nil + } + + var testCases = []struct { + name string + user *fleet.User + shouldFailWrite bool + shouldFailRead bool + }{ + { + "global admin", + &fleet.User{GlobalRole: ptr.String(fleet.RoleAdmin)}, + false, + false, + }, + { + "global maintainer", + &fleet.User{GlobalRole: ptr.String(fleet.RoleMaintainer)}, + false, + false, + }, + { + "global observer", + &fleet.User{GlobalRole: ptr.String(fleet.RoleObserver)}, + true, + true, + }, + { + "team maintainer, belongs to team", + &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 1}, Role: fleet.RoleMaintainer}}}, + false, + false, + }, + { + "team observer, belongs to team", + &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 1}, Role: fleet.RoleObserver}}}, + true, + true, + }, + { + "team maintainer, DOES NOT belong to team", + &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 2}, Role: fleet.RoleMaintainer}}}, + true, + true, + }, + { + "team observer, DOES NOT belong to team", + &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 2}, Role: fleet.RoleObserver}}}, + 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.GetTeamScheduledQueries(ctx, 1, fleet.ListOptions{}) + checkAuthErr(t, tt.shouldFailRead, err) + + _, err = svc.TeamScheduleQuery(ctx, 1, &fleet.ScheduledQuery{}) + checkAuthErr(t, tt.shouldFailWrite, err) + + _, err = svc.ModifyTeamScheduledQueries(ctx, 1, 99, fleet.ScheduledQueryPayload{}) + checkAuthErr(t, tt.shouldFailWrite, err) + + err = svc.DeleteTeamScheduledQueries(ctx, 1, 1) + checkAuthErr(t, tt.shouldFailWrite, err) + }) + } +}