From 4af18cd136ebe05bcd905a5ff75910b2c12afcd8 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Mon, 9 Dec 2024 08:29:08 -0600 Subject: [PATCH] Allow team admins/maintainers to view Fleet maintained apps (#24516) For #23305. # 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/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/Committing-Changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) - [x] Added/updated tests - [x] Manual QA for all new/changed functionality --- changes/23305-team-admin-tma | 1 + ee/server/service/maintained_apps.go | 6 +- ee/server/service/maintained_apps_test.go | 79 +++++++++++++++++++++++ server/authz/policy.rego | 14 ++++ server/fleet/maintained_apps.go | 5 ++ 5 files changed, 102 insertions(+), 3 deletions(-) create mode 100644 changes/23305-team-admin-tma create mode 100644 ee/server/service/maintained_apps_test.go diff --git a/changes/23305-team-admin-tma b/changes/23305-team-admin-tma new file mode 100644 index 0000000000..17b4bdeddb --- /dev/null +++ b/changes/23305-team-admin-tma @@ -0,0 +1 @@ +* Fixed missing read permission for team maintainers and admins on Fleet maintained apps diff --git a/ee/server/service/maintained_apps.go b/ee/server/service/maintained_apps.go index d189491921..3b995e39d4 100644 --- a/ee/server/service/maintained_apps.go +++ b/ee/server/service/maintained_apps.go @@ -175,9 +175,9 @@ func (svc *Service) ListFleetMaintainedApps(ctx context.Context, teamID uint, op } func (svc *Service) GetFleetMaintainedApp(ctx context.Context, appID uint) (*fleet.MaintainedApp, error) { - if err := svc.authz.Authorize(ctx, &fleet.SoftwareInstaller{ - TeamID: nil, - }, fleet.ActionRead); err != nil { + // Special case auth for maintained apps (vs. normal installers) as maintained apps are not scoped to a team; + // use SoftwareInstaller for authorization elsewhere. + if err := svc.authz.Authorize(ctx, &fleet.MaintainedApp{}, fleet.ActionRead); err != nil { return nil, err } diff --git a/ee/server/service/maintained_apps_test.go b/ee/server/service/maintained_apps_test.go new file mode 100644 index 0000000000..c357db8539 --- /dev/null +++ b/ee/server/service/maintained_apps_test.go @@ -0,0 +1,79 @@ +package service + +import ( + "context" + "testing" + + "github.com/fleetdm/fleet/v4/server/authz" + "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" + "github.com/stretchr/testify/require" +) + +func TestGetMaintainedAppAuth(t *testing.T) { + t.Parallel() + ds := new(mock.Store) + ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) { + return &fleet.AppConfig{}, nil + } + ds.GetMaintainedAppByIDFunc = func(ctx context.Context, appID uint) (*fleet.MaintainedApp, error) { + return &fleet.MaintainedApp{}, nil + } + authorizer, err := authz.NewAuthorizer() + require.NoError(t, err) + svc := &Service{authz: authorizer, ds: ds} + + testCases := []struct { + name string + user *fleet.User + shouldFail bool + }{ + { + "global admin", + &fleet.User{GlobalRole: ptr.String(fleet.RoleAdmin)}, + false, + }, + { + "global maintainer", + &fleet.User{GlobalRole: ptr.String(fleet.RoleMaintainer)}, + false, + }, + { + "global observer", + &fleet.User{GlobalRole: ptr.String(fleet.RoleObserver)}, + true, + }, + { + "team admin", + &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 1}, Role: fleet.RoleAdmin}}}, + false, + }, + { + "team maintainer", + &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 1}, Role: fleet.RoleMaintainer}}}, + false, + }, + { + "team observer", + &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 1}, Role: fleet.RoleObserver}}}, + true, + }, + } + + var forbiddenError *authz.Forbidden + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + ctx := viewer.NewContext(context.Background(), viewer.Viewer{User: tt.user}) + _, err := svc.GetFleetMaintainedApp(ctx, 123) + + if tt.shouldFail { + require.Error(t, err) + require.ErrorAs(t, err, &forbiddenError) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/server/authz/policy.rego b/server/authz/policy.rego index d24d09bf87..a57393e964 100644 --- a/server/authz/policy.rego +++ b/server/authz/policy.rego @@ -643,6 +643,20 @@ allow { action == read } +# Global admins and maintainers can read all maintained apps. +allow { + object.type == "maintained_app" + subject.global_role == [admin, maintainer][_] + action == read +} + +# Team admins and maintainers can read all maintained apps (no team constraint, unlike installers) +allow { + object.type == "maintained_app" + team_role(subject, subject.teams[_].id) == [admin, maintainer][_] + action == read +} + # Global admins and maintainers can read any installable entity (software installer or VPP app) allow { object.type == "installable_entity" diff --git a/server/fleet/maintained_apps.go b/server/fleet/maintained_apps.go index 18ea1ec493..efa0c290d4 100644 --- a/server/fleet/maintained_apps.go +++ b/server/fleet/maintained_apps.go @@ -22,3 +22,8 @@ type MaintainedApp struct { // UpdatedAt is the timestamp when the fleet maintained app data was last updated. UpdatedAt *time.Time `json:"-" db:"updated_at"` } + +// AuthzType implements authz.AuthzTyper. +func (s *MaintainedApp) AuthzType() string { + return "maintained_app" +}