From ab75f2ccc727760cc5d3e8a9c854ed87e71c4bf7 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Wed, 11 Dec 2024 21:30:20 -0600 Subject: [PATCH] Cherry-Pick: Allow pulling the base list of Fleet Maintained Apps without requiring a team ID (#24693) For #24509, merged into `main` in #24595. --- changes/24509-fma-no-team | 1 + ee/server/service/maintained_apps.go | 16 +++- ee/server/service/maintained_apps_test.go | 96 +++++++++++++++++++ server/datastore/mysql/maintained_apps.go | 63 ++++++------ .../datastore/mysql/maintained_apps_test.go | 32 ++++--- server/fleet/datastore.go | 4 +- server/fleet/service.go | 2 +- server/mock/datastore_mock.go | 4 +- server/service/maintained_apps.go | 4 +- 9 files changed, 165 insertions(+), 57 deletions(-) create mode 100644 changes/24509-fma-no-team diff --git a/changes/24509-fma-no-team b/changes/24509-fma-no-team new file mode 100644 index 000000000000..64fa83bc92a0 --- /dev/null +++ b/changes/24509-fma-no-team @@ -0,0 +1 @@ +* Allowed calling `/api/v1/fleet/software/fleet_maintained_apps` with no team ID to retrieve the full global list of maintained apps diff --git a/ee/server/service/maintained_apps.go b/ee/server/service/maintained_apps.go index 3b995e39d4b7..9a765409ca1c 100644 --- a/ee/server/service/maintained_apps.go +++ b/ee/server/service/maintained_apps.go @@ -159,11 +159,17 @@ func (svc *Service) AddFleetMaintainedApp( return titleId, nil } -func (svc *Service) ListFleetMaintainedApps(ctx context.Context, teamID uint, opts fleet.ListOptions) ([]fleet.MaintainedApp, *fleet.PaginationMetadata, error) { - if err := svc.authz.Authorize(ctx, &fleet.SoftwareInstaller{ - TeamID: &teamID, - }, fleet.ActionRead); err != nil { - return nil, nil, err +func (svc *Service) ListFleetMaintainedApps(ctx context.Context, teamID *uint, opts fleet.ListOptions) ([]fleet.MaintainedApp, *fleet.PaginationMetadata, error) { + var authErr error + // viewing the maintained app list without showing team-specific info can be done by anyone who can view individual FMAs + if teamID == nil { + authErr = svc.authz.Authorize(ctx, &fleet.MaintainedApp{}, fleet.ActionRead) + } else { // viewing the maintained app list when showing team-specific info requires access to that team + authErr = svc.authz.Authorize(ctx, &fleet.SoftwareInstaller{TeamID: teamID}, fleet.ActionRead) + } + + if authErr != nil { + return nil, nil, authErr } avail, meta, err := svc.ds.ListAvailableFleetMaintainedApps(ctx, teamID, opts) diff --git a/ee/server/service/maintained_apps_test.go b/ee/server/service/maintained_apps_test.go index c357db8539be..332bf555ff24 100644 --- a/ee/server/service/maintained_apps_test.go +++ b/ee/server/service/maintained_apps_test.go @@ -12,6 +12,102 @@ import ( "github.com/stretchr/testify/require" ) +func TestListMaintainedAppsAuth(t *testing.T) { + t.Parallel() + ds := new(mock.Store) + ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) { + return &fleet.AppConfig{}, nil + } + ds.ListAvailableFleetMaintainedAppsFunc = func(ctx context.Context, teamID *uint, opt fleet.ListOptions) ([]fleet.MaintainedApp, *fleet.PaginationMetadata, error) { + return []fleet.MaintainedApp{}, &fleet.PaginationMetadata{}, nil + } + authorizer, err := authz.NewAuthorizer() + require.NoError(t, err) + svc := &Service{authz: authorizer, ds: ds} + + testCases := []struct { + name string + user *fleet.User + shouldFailWithNoTeam bool + shouldFailWithMatchingTeam bool + shouldFailWithDifferentTeam bool + }{ + { + "global admin", + &fleet.User{GlobalRole: ptr.String(fleet.RoleAdmin)}, + false, + false, + false, + }, + { + "global maintainer", + &fleet.User{GlobalRole: ptr.String(fleet.RoleMaintainer)}, + false, + false, + false, + }, + { + "global observer", + &fleet.User{GlobalRole: ptr.String(fleet.RoleObserver)}, + true, + true, + true, + }, + { + "team admin", + &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 1}, Role: fleet.RoleAdmin}}}, + false, + false, + true, + }, + { + "team maintainer", + &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 1}, Role: fleet.RoleMaintainer}}}, + false, + false, + true, + }, + { + "team observer", + &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 1}, Role: fleet.RoleObserver}}}, + true, + true, + 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.ListFleetMaintainedApps(ctx, nil, fleet.ListOptions{}) + if tt.shouldFailWithNoTeam { + require.Error(t, err) + require.ErrorAs(t, err, &forbiddenError) + } else { + require.NoError(t, err) + } + + _, _, err = svc.ListFleetMaintainedApps(ctx, ptr.Uint(1), fleet.ListOptions{}) + if tt.shouldFailWithMatchingTeam { + require.Error(t, err) + require.ErrorAs(t, err, &forbiddenError) + } else { + require.NoError(t, err) + } + + _, _, err = svc.ListFleetMaintainedApps(ctx, ptr.Uint(2), fleet.ListOptions{}) + if tt.shouldFailWithDifferentTeam { + require.Error(t, err) + require.ErrorAs(t, err, &forbiddenError) + } else { + require.NoError(t, err) + } + }) + } +} + func TestGetMaintainedAppAuth(t *testing.T) { t.Parallel() ds := new(mock.Store) diff --git a/server/datastore/mysql/maintained_apps.go b/server/datastore/mysql/maintained_apps.go index a0985f85042e..4a75f4742994 100644 --- a/server/datastore/mysql/maintained_apps.go +++ b/server/datastore/mysql/maintained_apps.go @@ -100,40 +100,37 @@ WHERE return &app, nil } -func (ds *Datastore) ListAvailableFleetMaintainedApps(ctx context.Context, teamID uint, opt fleet.ListOptions) ([]fleet.MaintainedApp, *fleet.PaginationMetadata, error) { - stmt := ` -SELECT - fla.id, - fla.name, - fla.version, - fla.platform, - fla.updated_at -FROM - fleet_library_apps fla -WHERE NOT EXISTS ( - SELECT - 1 - FROM - software_titles st - LEFT JOIN - software_installers si - ON si.title_id = st.id - LEFT JOIN - vpp_apps va - ON va.title_id = st.id - LEFT JOIN - vpp_apps_teams vat - ON vat.adam_id = va.adam_id - WHERE - st.bundle_identifier = fla.bundle_identifier - AND ( - (si.platform = fla.platform AND si.global_or_team_id = ?) - OR - (va.platform = fla.platform AND vat.global_or_team_id = ?) - ) -)` +func (ds *Datastore) ListAvailableFleetMaintainedApps(ctx context.Context, teamID *uint, opt fleet.ListOptions) ([]fleet.MaintainedApp, *fleet.PaginationMetadata, error) { + stmt := `SELECT fla.id, fla.name, fla.version, fla.platform, fla.updated_at FROM fleet_library_apps fla ` + var args []any - args := []any{teamID, teamID} + if teamID != nil { + stmt += `WHERE NOT EXISTS ( + SELECT + 1 + FROM + software_titles st + LEFT JOIN + software_installers si + ON si.title_id = st.id + LEFT JOIN + vpp_apps va + ON va.title_id = st.id + LEFT JOIN + vpp_apps_teams vat + ON vat.adam_id = va.adam_id + WHERE + st.bundle_identifier = fla.bundle_identifier + AND ( + (si.platform = fla.platform AND si.global_or_team_id = ?) + OR + (va.platform = fla.platform AND vat.global_or_team_id = ?) + ) + )` + args = []any{teamID, teamID} + } else { + stmt += `WHERE TRUE` + } if match := opt.MatchQuery; match != "" { match = likePattern(match) diff --git a/server/datastore/mysql/maintained_apps_test.go b/server/datastore/mysql/maintained_apps_test.go index 95d6155c5619..6506ffae35dd 100644 --- a/server/datastore/mysql/maintained_apps_test.go +++ b/server/datastore/mysql/maintained_apps_test.go @@ -170,7 +170,7 @@ func testListAvailableApps(t *testing.T, ds *Datastore) { } // Testing pagination - apps, meta, err := ds.ListAvailableFleetMaintainedApps(ctx, team1.ID, fleet.ListOptions{IncludeMetadata: true}) + apps, meta, err := ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.ListOptions{IncludeMetadata: true}) require.NoError(t, err) require.Len(t, apps, 3) require.EqualValues(t, meta.TotalResults, 3) @@ -178,7 +178,7 @@ func testListAvailableApps(t *testing.T, ds *Datastore) { require.Equal(t, expectedApps, apps) require.False(t, meta.HasNextResults) - apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, team1.ID, fleet.ListOptions{PerPage: 1, IncludeMetadata: true}) + apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.ListOptions{PerPage: 1, IncludeMetadata: true}) require.NoError(t, err) require.Len(t, apps, 1) require.EqualValues(t, meta.TotalResults, 3) @@ -186,7 +186,7 @@ func testListAvailableApps(t *testing.T, ds *Datastore) { require.Equal(t, expectedApps[:1], apps) require.True(t, meta.HasNextResults) - apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, team1.ID, fleet.ListOptions{PerPage: 1, Page: 1, IncludeMetadata: true}) + apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.ListOptions{PerPage: 1, Page: 1, IncludeMetadata: true}) require.NoError(t, err) require.Len(t, apps, 1) require.EqualValues(t, meta.TotalResults, 3) @@ -195,7 +195,7 @@ func testListAvailableApps(t *testing.T, ds *Datastore) { require.True(t, meta.HasNextResults) require.True(t, meta.HasPreviousResults) - apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, team1.ID, fleet.ListOptions{PerPage: 1, Page: 2, IncludeMetadata: true}) + apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.ListOptions{PerPage: 1, Page: 2, IncludeMetadata: true}) require.NoError(t, err) require.Len(t, apps, 1) require.EqualValues(t, meta.TotalResults, 3) @@ -219,7 +219,7 @@ func testListAvailableApps(t *testing.T, ds *Datastore) { }) require.NoError(t, err) - apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, team1.ID, fleet.ListOptions{IncludeMetadata: true}) + apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.ListOptions{IncludeMetadata: true}) require.NoError(t, err) require.Len(t, apps, 3) require.EqualValues(t, meta.TotalResults, 3) @@ -238,7 +238,7 @@ func testListAvailableApps(t *testing.T, ds *Datastore) { }) require.NoError(t, err) - apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, team1.ID, fleet.ListOptions{IncludeMetadata: true}) + apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.ListOptions{IncludeMetadata: true}) require.NoError(t, err) require.Len(t, apps, 3) require.EqualValues(t, meta.TotalResults, 3) @@ -257,7 +257,7 @@ func testListAvailableApps(t *testing.T, ds *Datastore) { }) require.NoError(t, err) - apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, team1.ID, fleet.ListOptions{IncludeMetadata: true}) + apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.ListOptions{IncludeMetadata: true}) require.NoError(t, err) require.Len(t, apps, 3) require.EqualValues(t, meta.TotalResults, 3) @@ -270,7 +270,7 @@ func testListAvailableApps(t *testing.T, ds *Datastore) { return err }) - apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, team1.ID, fleet.ListOptions{IncludeMetadata: true}) + apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.ListOptions{IncludeMetadata: true}) require.NoError(t, err) require.Len(t, apps, 2) require.EqualValues(t, meta.TotalResults, 2) @@ -296,7 +296,7 @@ func testListAvailableApps(t *testing.T, ds *Datastore) { _, err = ds.InsertVPPAppWithTeam(ctx, vppIrrelevant, &team1.ID) require.NoError(t, err) - apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, team1.ID, fleet.ListOptions{IncludeMetadata: true}) + apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.ListOptions{IncludeMetadata: true}) require.NoError(t, err) require.Len(t, apps, 2) require.EqualValues(t, meta.TotalResults, 2) @@ -317,7 +317,7 @@ func testListAvailableApps(t *testing.T, ds *Datastore) { _, err = ds.InsertVPPAppWithTeam(ctx, vppMaintained2, &team2.ID) require.NoError(t, err) - apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, team1.ID, fleet.ListOptions{IncludeMetadata: true}) + apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.ListOptions{IncludeMetadata: true}) require.NoError(t, err) require.Len(t, apps, 2) require.EqualValues(t, meta.TotalResults, 2) @@ -328,7 +328,7 @@ func testListAvailableApps(t *testing.T, ds *Datastore) { _, err = ds.InsertVPPAppWithTeam(ctx, vppMaintained2, &team1.ID) require.NoError(t, err) - apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, team1.ID, fleet.ListOptions{IncludeMetadata: true}) + apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.ListOptions{IncludeMetadata: true}) require.NoError(t, err) require.Len(t, apps, 1) require.EqualValues(t, meta.TotalResults, 1) @@ -350,12 +350,20 @@ func testListAvailableApps(t *testing.T, ds *Datastore) { _, err = ds.InsertVPPAppWithTeam(ctx, vppMaintained3, &team1.ID) require.NoError(t, err) - apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, team1.ID, fleet.ListOptions{IncludeMetadata: true}) + apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.ListOptions{IncludeMetadata: true}) require.NoError(t, err) require.Len(t, apps, 1) require.EqualValues(t, meta.TotalResults, 1) assertUpdatedAt(apps) require.Equal(t, expectedApps[2:], apps) + + // viewing with no team selected shouldn't exclude any results + apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, nil, fleet.ListOptions{IncludeMetadata: true}) + require.NoError(t, err) + require.Len(t, apps, 3) + require.EqualValues(t, meta.TotalResults, 3) + assertUpdatedAt(apps) + require.Equal(t, expectedApps, apps) } func testGetMaintainedAppByID(t *testing.T, ds *Datastore) { diff --git a/server/fleet/datastore.go b/server/fleet/datastore.go index cbdcb28f6b68..f2acb2255d6f 100644 --- a/server/fleet/datastore.go +++ b/server/fleet/datastore.go @@ -1851,8 +1851,8 @@ type Datastore interface { // // ListAvailableFleetMaintainedApps returns a list of - // Fleet-maintained apps available to a specific team - ListAvailableFleetMaintainedApps(ctx context.Context, teamID uint, opt ListOptions) ([]MaintainedApp, *PaginationMetadata, error) + // Fleet-maintained apps available to a specific team (or the full list of apps if no team is specified) + ListAvailableFleetMaintainedApps(ctx context.Context, teamID *uint, opt ListOptions) ([]MaintainedApp, *PaginationMetadata, error) // GetMaintainedAppByID gets a Fleet-maintained app by its ID. GetMaintainedAppByID(ctx context.Context, appID uint) (*MaintainedApp, error) diff --git a/server/fleet/service.go b/server/fleet/service.go index 52e877481f75..a37a79a93e84 100644 --- a/server/fleet/service.go +++ b/server/fleet/service.go @@ -1165,7 +1165,7 @@ type Service interface { // AddFleetMaintainedApp adds a Fleet-maintained app to the given team. AddFleetMaintainedApp(ctx context.Context, teamID *uint, appID uint, installScript, preInstallQuery, postInstallScript, uninstallScript string, selfService bool) (uint, error) // ListFleetMaintainedApps lists Fleet-maintained apps available to a specific team - ListFleetMaintainedApps(ctx context.Context, teamID uint, opts ListOptions) ([]MaintainedApp, *PaginationMetadata, error) + ListFleetMaintainedApps(ctx context.Context, teamID *uint, opts ListOptions) ([]MaintainedApp, *PaginationMetadata, error) // GetFleetMaintainedApp returns a Fleet-maintained app by ID GetFleetMaintainedApp(ctx context.Context, appID uint) (*MaintainedApp, error) diff --git a/server/mock/datastore_mock.go b/server/mock/datastore_mock.go index 5d8d067d037d..1b5be67e242d 100644 --- a/server/mock/datastore_mock.go +++ b/server/mock/datastore_mock.go @@ -1161,7 +1161,7 @@ type MaybeUpdateSetupExperienceSoftwareInstallStatusFunc func(ctx context.Contex type MaybeUpdateSetupExperienceVPPStatusFunc func(ctx context.Context, hostUUID string, commandUUID string, status fleet.SetupExperienceStatusResultStatus) (bool, error) -type ListAvailableFleetMaintainedAppsFunc func(ctx context.Context, teamID uint, opt fleet.ListOptions) ([]fleet.MaintainedApp, *fleet.PaginationMetadata, error) +type ListAvailableFleetMaintainedAppsFunc func(ctx context.Context, teamID *uint, opt fleet.ListOptions) ([]fleet.MaintainedApp, *fleet.PaginationMetadata, error) type GetMaintainedAppByIDFunc func(ctx context.Context, appID uint) (*fleet.MaintainedApp, error) @@ -6900,7 +6900,7 @@ func (s *DataStore) MaybeUpdateSetupExperienceVPPStatus(ctx context.Context, hos return s.MaybeUpdateSetupExperienceVPPStatusFunc(ctx, hostUUID, commandUUID, status) } -func (s *DataStore) ListAvailableFleetMaintainedApps(ctx context.Context, teamID uint, opt fleet.ListOptions) ([]fleet.MaintainedApp, *fleet.PaginationMetadata, error) { +func (s *DataStore) ListAvailableFleetMaintainedApps(ctx context.Context, teamID *uint, opt fleet.ListOptions) ([]fleet.MaintainedApp, *fleet.PaginationMetadata, error) { s.mu.Lock() s.ListAvailableFleetMaintainedAppsFuncInvoked = true s.mu.Unlock() diff --git a/server/service/maintained_apps.go b/server/service/maintained_apps.go index b8edc671fb6a..04d788357995 100644 --- a/server/service/maintained_apps.go +++ b/server/service/maintained_apps.go @@ -60,7 +60,7 @@ func (svc *Service) AddFleetMaintainedApp(ctx context.Context, teamID *uint, app type listFleetMaintainedAppsRequest struct { fleet.ListOptions - TeamID uint `query:"team_id"` + TeamID *uint `query:"team_id,optional"` } type listFleetMaintainedAppsResponse struct { @@ -102,7 +102,7 @@ func listFleetMaintainedAppsEndpoint(ctx context.Context, request any, svc fleet return listResp, nil } -func (svc *Service) ListFleetMaintainedApps(ctx context.Context, teamID uint, opts fleet.ListOptions) ([]fleet.MaintainedApp, *fleet.PaginationMetadata, error) { +func (svc *Service) ListFleetMaintainedApps(ctx context.Context, teamID *uint, opts fleet.ListOptions) ([]fleet.MaintainedApp, *fleet.PaginationMetadata, error) { // skipauth: No authorization check needed due to implementation returning // only license error. svc.authz.SkipAuthorization(ctx)