Skip to content

Commit

Permalink
Cherry-Pick: Allow pulling the base list of Fleet Maintained Apps wit…
Browse files Browse the repository at this point in the history 
…hout requiring a team ID (#24693)

For #24509, merged into `main` in #24595.
  • Loading branch information
@iansltx
iansltx authored Dec 12, 2024
1 parent 7e11d5e commit ab75f2c
Show file tree
Hide file tree
Showing 9 changed files with 165 additions and 57 deletions.
1 change: 1 addition & 0 deletions changes/24509-fma-no-team
View file
Original file line number Diff line number Diff line change
@@ -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
16 changes: 11 additions & 5 deletions ee/server/service/maintained_apps.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
96 changes: 96 additions & 0 deletions ee/server/service/maintained_apps_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
63 changes: 30 additions & 33 deletions server/datastore/mysql/maintained_apps.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
32 changes: 20 additions & 12 deletions server/datastore/mysql/maintained_apps_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,23 +170,23 @@ 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)
assertUpdatedAt(apps)
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)
assertUpdatedAt(apps)
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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions server/fleet/datastore.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion server/fleet/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
4 changes: 2 additions & 2 deletions server/mock/datastore_mock.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions server/service/maintained_apps.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit ab75f2c

Please sign in to comment.