diff --git a/changes/47475-fleet-maintained-apps-pagination b/changes/47475-fleet-maintained-apps-pagination new file mode 100644 index 0000000000..fcf9ae3f37 --- /dev/null +++ b/changes/47475-fleet-maintained-apps-pagination @@ -0,0 +1,2 @@ +- Fixed the Fleet-maintained apps list being cut off so that apps near the end of the alphabet were unreachable. The list is now paginated (100 apps per page), and the platform and "Hide added apps" filters are applied across the full library instead of only the loaded apps. +- Updated the Fleet-maintained apps item count to reflect the total number of apps, counting an app's macOS and Windows versions separately (for example, a search for "Zoom" that returns Zoom and Zoom Rooms on both platforms shows 4 items). diff --git a/cmd/fleetctl/fleetctl/gitops_test.go b/cmd/fleetctl/fleetctl/gitops_test.go index b626733ba8..da8bef67a8 100644 --- a/cmd/fleetctl/fleetctl/gitops_test.go +++ b/cmd/fleetctl/fleetctl/gitops_test.go @@ -1251,7 +1251,7 @@ func TestGitOpsSoftwareExceptionPolicyValidation(t *testing.T) { }, }, 3, nil, nil } - ds.ListAvailableFleetMaintainedAppsFunc = func(ctx context.Context, teamID *uint, opt fleet.ListOptions) ([]fleet.MaintainedApp, *fleet.PaginationMetadata, error) { + ds.ListAvailableFleetMaintainedAppsFunc = func(ctx context.Context, teamID *uint, opt fleet.MaintainedAppListOptions) ([]fleet.MaintainedApp, *fleet.PaginationMetadata, error) { return []fleet.MaintainedApp{ {ID: 100, Slug: "zoom/darwin"}, }, nil, nil diff --git a/docs/REST API/rest-api.md b/docs/REST API/rest-api.md index 3c968b7119..608e3847d9 100644 --- a/docs/REST API/rest-api.md +++ b/docs/REST API/rest-api.md @@ -11764,8 +11764,13 @@ List available Fleet-maintained apps. | Name | Type | In | Description | | ---- | ---- | -- | ----------- | | fleet_id | integer | query | If specified, each app includes the `software_title_id` if the software has already been added to that fleet. | -| page | integer | query | Page number of the results to fetch. | -| per_page | integer | query | Results per page. | +| query | string | query | Search query keywords. Searches app name. | +| order_key | string | query | What to order results by. Currently only `name` is supported. | +| order_direction | string | query | **Requires `order_key`**. The direction of the order. Options are `"asc"` and `"desc"`. Default is `"asc"`. | +| page | integer | query | Page number of the results to fetch. Pagination is by app: an app's macOS and Windows entries are counted as one result and always returned on the same page. | +| per_page | integer | query | Results (apps) per page. | +| platform | string | query | Filter to apps available on a platform. Options are `"darwin"` (macOS) and `"windows"`. | +| available | boolean | query | If `true`, only return apps that have not yet been added to the fleet. Requires `fleet_id`. | #### Example @@ -11804,6 +11809,7 @@ List available Fleet-maintained apps. }, ... ], + "count": 250, "meta": { "has_next_results": false, "has_previous_results": false @@ -11811,6 +11817,8 @@ List available Fleet-maintained apps. } ``` +> `count` is the total number of Fleet-maintained apps matching the query. An app's macOS and Windows versions are counted separately, even though the UI combines them into a single row. + ### Get Fleet-maintained app Returns information about the specified Fleet-maintained app. diff --git a/ee/server/service/maintained_apps.go b/ee/server/service/maintained_apps.go index 45360c0063..9936e1db37 100644 --- a/ee/server/service/maintained_apps.go +++ b/ee/server/service/maintained_apps.go @@ -237,7 +237,7 @@ func (svc *Service) AddFleetMaintainedApp( return titleID, 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.MaintainedAppListOptions) ([]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 { diff --git a/ee/server/service/maintained_apps_test.go b/ee/server/service/maintained_apps_test.go index 452550dcd2..eb0a3b6992 100644 --- a/ee/server/service/maintained_apps_test.go +++ b/ee/server/service/maintained_apps_test.go @@ -30,7 +30,7 @@ func TestListMaintainedAppsAuth(t *testing.T) { 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) { + ds.ListAvailableFleetMaintainedAppsFunc = func(ctx context.Context, teamID *uint, opt fleet.MaintainedAppListOptions) ([]fleet.MaintainedApp, *fleet.PaginationMetadata, error) { return []fleet.MaintainedApp{}, &fleet.PaginationMetadata{}, nil } authorizer, err := authz.NewAuthorizer() @@ -107,7 +107,7 @@ func TestListMaintainedAppsAuth(t *testing.T) { 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{}) + _, _, err := svc.ListFleetMaintainedApps(ctx, nil, fleet.MaintainedAppListOptions{}) if tt.shouldFailWithNoTeam { require.Error(t, err) require.ErrorAs(t, err, &forbiddenError) @@ -115,7 +115,7 @@ func TestListMaintainedAppsAuth(t *testing.T) { require.NoError(t, err) } - _, _, err = svc.ListFleetMaintainedApps(ctx, ptr.Uint(1), fleet.ListOptions{}) + _, _, err = svc.ListFleetMaintainedApps(ctx, new(uint(1)), fleet.MaintainedAppListOptions{}) if tt.shouldFailWithMatchingTeam { require.Error(t, err) require.ErrorAs(t, err, &forbiddenError) @@ -123,7 +123,7 @@ func TestListMaintainedAppsAuth(t *testing.T) { require.NoError(t, err) } - _, _, err = svc.ListFleetMaintainedApps(ctx, ptr.Uint(2), fleet.ListOptions{}) + _, _, err = svc.ListFleetMaintainedApps(ctx, new(uint(2)), fleet.MaintainedAppListOptions{}) if tt.shouldFailWithDifferentTeam { require.Error(t, err) require.ErrorAs(t, err, &forbiddenError) diff --git a/frontend/pages/SoftwarePage/SoftwareAddPage/SoftwareFleetMaintained/FleetMaintainedAppsTable/FleetMaintainedAppsTable.tsx b/frontend/pages/SoftwarePage/SoftwareAddPage/SoftwareFleetMaintained/FleetMaintainedAppsTable/FleetMaintainedAppsTable.tsx index e1cb62f721..48e43164b8 100644 --- a/frontend/pages/SoftwarePage/SoftwareAddPage/SoftwareFleetMaintained/FleetMaintainedAppsTable/FleetMaintainedAppsTable.tsx +++ b/frontend/pages/SoftwarePage/SoftwareAddPage/SoftwareFleetMaintained/FleetMaintainedAppsTable/FleetMaintainedAppsTable.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useMemo, useState } from "react"; +import React, { useCallback, useMemo } from "react"; import { InjectedRouter } from "react-router"; import PATHS from "router/paths"; @@ -104,10 +104,11 @@ const FleetMaintainedAppsTable = ({ orderKey, currentPage, }: IFleetMaintainedAppsTableProps) => { - const [status, setStatus] = useState(statusParam || "all"); - const [platform, setPlatform] = useState( - platformParam || "all" - ); + // Filter values are driven by the URL, which is also the source of truth for + // the server-side query. Derive them from props rather than local state so + // the controls stay in sync on back/forward navigation. + const status: FmaStatusValue = statusParam || "all"; + const platform: FmaPlatformValue = platformParam || "all"; const determineQueryParamChange = useCallback( (newTableQuery: ITableQueryData) => { @@ -196,43 +197,20 @@ const FleetMaintainedAppsTable = ({ return generateTableConfig(router, teamId); }, [data, router, teamId]); - // Note: Serverside filtering will be buggy with pagination if > 20 apps - // API will need to be refactored to combine macOS/windows apps - // for correct pagination, sort, and counts when we go over 20 apps + // Pagination, platform/"hide added apps" filtering, sort, and counts are all + // handled server-side. The API returns every platform row for the apps on the + // current page, so combining macOS and Windows entries here always yields + // complete rows (an app is never split across a page boundary). const combinedAppsByPlatform = (data && combineAppsByPlatform(data.fleet_maintained_apps ?? [])) ?? []; - const filteredApps = combinedAppsByPlatform.filter((app) => { - const macAvailable = !!app.macos && !app.macos.software_title_id; - const winAvailable = !!app.windows && !app.windows.software_title_id; - - // platform filter - if (platform === "macos" && !app.macos) return false; - if (platform === "windows" && !app.windows) return false; - - // status filter - if (status === "all") { - return true; - } - - if (status === "available") { - if (platform === "macos") return macAvailable; - if (platform === "windows") return winAvailable; - return macAvailable || winAvailable; - } - - return true; - }); - const renderCount = () => { - if (!filteredApps) return null; + if (!data) return null; - return ; + return ; }; const handleFmaStatusDropdownChange = (newStatus: FmaStatusValue) => { - setStatus(newStatus); - const newRoute = getNextLocationPath({ pathPrefix: PATHS.SOFTWARE_ADD_FLEET_MAINTAINED, routeTemplate: "", @@ -254,8 +232,6 @@ const FleetMaintainedAppsTable = ({ }; const handleFmaPlatformDropdownChange = (newPlatform: FmaPlatformValue) => { - setPlatform(newPlatform); - const newRoute = getNextLocationPath({ pathPrefix: PATHS.SOFTWARE_ADD_FLEET_MAINTAINED, routeTemplate: "", @@ -295,7 +271,7 @@ const FleetMaintainedAppsTable = ({ className={baseClass} columnConfigs={tableHeadersConfig} - data={filteredApps} + data={combinedAppsByPlatform} isLoading={isLoading} resultsTitle="items" emptyComponent={EmptyFleetAppsTable} diff --git a/frontend/pages/SoftwarePage/SoftwareAddPage/SoftwareFleetMaintained/SoftwareFleetMaintained.tsx b/frontend/pages/SoftwarePage/SoftwareAddPage/SoftwareFleetMaintained/SoftwareFleetMaintained.tsx index ec6d5cd375..441b635ac5 100644 --- a/frontend/pages/SoftwarePage/SoftwareAddPage/SoftwareFleetMaintained/SoftwareFleetMaintained.tsx +++ b/frontend/pages/SoftwarePage/SoftwareAddPage/SoftwareFleetMaintained/SoftwareFleetMaintained.tsx @@ -40,12 +40,10 @@ interface ISoftwareFleetMaintainedProps { // default values for query params used on this page if not provided const DEFAULT_SORT_DIRECTION = "asc"; const DEFAULT_SORT_HEADER = "name"; -/** Team decision to avoid UI pagination because API needs revamp to properly - * handle pagination serverside, so rather break API than add more helper logic to - * handle clientside pagination when we know API will be revamped and would need - * to convert back to serverside after API fix. - */ -const DEFAULT_PAGE_SIZE = 999; +// The list is paginated server-side by app (an app's macOS and Windows entries +// are combined into a single row). 100 apps per page keeps the full library +// reachable without an unbounded response. +const DEFAULT_PAGE_SIZE = 100; const DEFAULT_PAGE = 0; const SoftwareFleetMaintained = ({ @@ -65,6 +63,18 @@ const SoftwareFleetMaintained = ({ } = location.query; const currentPage = page ? parseInt(page, 10) : DEFAULT_PAGE; + // Platform and "hide added apps" are filtered server-side. Map the UI's + // platform value ("macos"/"windows") to the API's ("darwin"/"windows") and + // the status toggle to the `available` flag. Undefined values are omitted + // from the request. + let apiPlatform: "darwin" | "windows" | undefined; + if (platform === "macos") { + apiPlatform = "darwin"; + } else if (platform === "windows") { + apiPlatform = "windows"; + } + const availableOnly = status === "available" ? true : undefined; + const { data, isLoading, isFetching, isError } = useQuery< ISoftwareFleetMaintainedAppsResponse, AxiosError, @@ -80,6 +90,8 @@ const SoftwareFleetMaintained = ({ order_direction, order_key, team_id: currentTeamId, + platform: apiPlatform, + available: availableOnly, }, ], ({ queryKey: [queryKey] }) => { diff --git a/frontend/services/entities/software.ts b/frontend/services/entities/software.ts index c618f0dbf9..4aa627665c 100644 --- a/frontend/services/entities/software.ts +++ b/frontend/services/entities/software.ts @@ -132,6 +132,11 @@ export interface ISoftwareFleetMaintainedAppsQueryParams { order_direction?: "asc" | "desc"; page?: number; per_page?: number; + /** Filter to apps available on a given platform. Uses the API's platform + * vocabulary ("darwin"/"windows"), not the UI's ("macos"/"windows"). */ + platform?: "darwin" | "windows"; + /** When true, only return apps not yet added to the fleet ("Hide added apps"). */ + available?: boolean; } export interface ISoftwareFleetMaintainedAppsResponse { diff --git a/server/datastore/mysql/maintained_apps.go b/server/datastore/mysql/maintained_apps.go index 3aa45a078a..5523eebf4b 100644 --- a/server/datastore/mysql/maintained_apps.go +++ b/server/datastore/mysql/maintained_apps.go @@ -12,11 +12,11 @@ import ( "github.com/jmoiron/sqlx" ) +// maintainedAppsAllowedOrderKeys allowlists order keys for listing +// Fleet-maintained apps. The list is a combined-by-name view, so name is the +// only meaningful key; it's validation-only, since ORDER BY is hard-coded below. var maintainedAppsAllowedOrderKeys = common_mysql.OrderKeyAllowlist{ - "id": "fma.id", - "name": "fma.name", - "platform": "fma.platform", - "slug": "fma.slug", + "name": "fma.name", } func (ds *Datastore) UpsertMaintainedApp(ctx context.Context, app *fleet.MaintainedApp) (*fleet.MaintainedApp, error) { @@ -86,8 +86,13 @@ ON DUPLICATE KEY UPDATE return app, nil } -const teamFMATitlesJoin = ` - team_titles.id software_title_id FROM fleet_maintained_apps fma +// fleetMaintainedAppsTeamJoin is the FROM clause plus the LEFT JOIN that +// determines, for a given team, whether each Fleet-maintained app has already +// been added (via a software installer or VPP app). team_titles.id is non-NULL +// when the app is already added to the team. It expects two `?` args, both the +// team's global_or_team_id. +const fleetMaintainedAppsTeamJoin = ` + FROM fleet_maintained_apps fma LEFT JOIN ( SELECT DISTINCT st.id, st.unique_identifier, st.name, si.platform FROM software_titles st @@ -105,17 +110,21 @@ const teamFMATitlesJoin = ` AND vat.platform = va.platform AND vat.global_or_team_id = ? WHERE si.id IS NOT NULL OR vat.id IS NOT NULL - ) team_titles + ) team_titles ON team_titles.unique_identifier = fma.unique_identifier -- pattern match fma name to a similar title name, since upgrade_code is not surfaced in fma table OR ( - team_titles.platform = fma.platform - AND fma.platform = 'windows' + team_titles.platform = fma.platform + AND fma.platform = 'windows' -- Box Drive is the only FMA at the point of writing this where unique_identifier is shorter than name AND team_titles.name LIKE CONCAT(LEAST(fma.name, fma.unique_identifier), '%') ) ` +// teamFMATitlesJoin selects software_title_id alongside the team join, for use +// directly after `SELECT fma.id, fma.name, ..., `. +const teamFMATitlesJoin = `team_titles.id software_title_id ` + fleetMaintainedAppsTeamJoin + func (ds *Datastore) GetMaintainedAppByID(ctx context.Context, appID uint, teamID *uint) (*fleet.MaintainedApp, error) { stmt := `SELECT fma.id, fma.name, fma.platform, fma.unique_identifier, fma.slug, ` var args []any @@ -168,58 +177,120 @@ func (ds *Datastore) GetMaintainedAppBySlug(ctx context.Context, slug string, te return &app, nil } -func (ds *Datastore) ListAvailableFleetMaintainedApps(ctx context.Context, teamID *uint, opt fleet.ListOptions) ([]fleet.MaintainedApp, *fleet.PaginationMetadata, error) { - stmt := `SELECT fma.id, fma.name, fma.platform, fma.slug, ` - var args []any - - if teamID != nil { - stmt += teamFMATitlesJoin + ` WHERE TRUE` - args = []any{teamID, teamID} - } else { - stmt += `NULL software_title_id FROM fleet_maintained_apps fma` - } - - if match := opt.MatchQuery; match != "" { - match = likePattern(match) - stmt += ` AND (fma.name LIKE ?)` - args = append(args, match) - } - - // perform a second query to grab the filtered count. Build the count statement before - // adding the pagination constraints to the stmt but after including the - // MatchQuery option sql. +func (ds *Datastore) ListAvailableFleetMaintainedApps(ctx context.Context, teamID *uint, opt fleet.MaintainedAppListOptions) ([]fleet.MaintainedApp, *fleet.PaginationMetadata, error) { dbReader := ds.reader(ctx) - getAppsCountStmt := fmt.Sprintf(`SELECT COUNT(DISTINCT s.id) FROM (%s) AS s`, stmt) + + // We paginate by distinct app NAME, because the UI combines an app's macOS + // and Windows entries into a single row and an app must not be split across a + // page boundary. The count, by contrast, is the total number of apps (each + // platform entry is its own installable app). The team join lets us tell + // whether each app has already been added, which the "available only" filter + // needs. + fromClause := `FROM fleet_maintained_apps fma` + var fromArgs []any + if teamID != nil { + fromClause = fleetMaintainedAppsTeamJoin + fromArgs = []any{teamID, teamID} + } + + // Build the filter conditions shared by the count and page-name queries. + where := ` WHERE TRUE` + var whereArgs []any + if match := opt.MatchQuery; match != "" { + where += ` AND fma.name LIKE ?` + whereArgs = append(whereArgs, likePattern(match)) + } + if opt.Platform == "darwin" || opt.Platform == "windows" { + where += ` AND fma.platform = ?` + whereArgs = append(whereArgs, opt.Platform) + } + if opt.AvailableOnly && teamID != nil { + // "Hide added apps": keep only entries not yet added to this team. + where += ` AND team_titles.id IS NULL` + } + + // Total count of matching apps. We count distinct rows (by primary key), not + // distinct names: an app's macOS and Windows entries are separate installable + // apps and are each counted, even though the UI combines them into one row. + // DISTINCT fma.id also collapses any duplicate rows from the team join's + // fan-out. + countArgs := append(append([]any{}, fromArgs...), whereArgs...) var filteredCount int - if err := sqlx.GetContext(ctx, dbReader, &filteredCount, getAppsCountStmt, args...); err != nil { + if err := sqlx.GetContext(ctx, dbReader, &filteredCount, `SELECT COUNT(DISTINCT fma.id) `+fromClause+where, countArgs...); err != nil { return nil, nil, ctxerr.Wrap(ctx, err, "get fleet maintained apps count") } - if filteredCount == 0 { // check if we have nothing in the full apps list, in which case provide an error back + if filteredCount == 0 { + // Distinguish an empty library (an error) from filters matching nothing + // (an empty, non-error result). var totalCount int if err := sqlx.GetContext(ctx, dbReader, &totalCount, `SELECT COUNT(id) FROM fleet_maintained_apps`); err != nil { return nil, nil, ctxerr.Wrap(ctx, err, "get fleet maintained apps total count") } - if totalCount == 0 { return nil, nil, &fleet.NoMaintainedAppsInDatabaseError{} } + return []fleet.MaintainedApp{}, &fleet.PaginationMetadata{HasPreviousResults: opt.Page > 0}, nil } - stmtPaged, args, err := appendListOptionsWithCursorToSQLSecure(stmt, args, &opt, maintainedAppsAllowedOrderKeys) - if err != nil { - return nil, nil, ctxerr.Wrap(ctx, err, "list fleet maintained apps") + // Validate the requested order key against the allowlist, which permits only + // "name" (the apps are always ordered by name below; see the allowlist + // declaration). Any other key, including an empty one, is handled here: an + // empty key skips validation and falls through to the default name ordering. + if key := opt.OrderKey; key != "" { + if _, ok := maintainedAppsAllowedOrderKeys[key]; !ok { + return nil, nil, ctxerr.Wrap(ctx, common_mysql.InvalidOrderKeyError{Key: key, Allowed: maintainedAppsAllowedOrderKeys.AllowedKeys()}, "list fleet maintained apps") + } + } + direction := "ASC" + if opt.IsDescending() { + direction = "DESC" } - var avail []fleet.MaintainedApp - if err := sqlx.SelectContext(ctx, ds.reader(ctx), &avail, stmtPaged, args...); err != nil { - return nil, nil, ctxerr.Wrap(ctx, err, "selecting available fleet maintained apps") + // Select the page of app names, fetching one extra to detect a next page. + perPage := opt.GetPerPage() + pageNamesStmt := fmt.Sprintf( + `SELECT DISTINCT fma.name %s%s ORDER BY fma.name %s LIMIT %d OFFSET %d`, + fromClause, where, direction, perPage+1, perPage*opt.Page, + ) + pageNamesArgs := append(append([]any{}, fromArgs...), whereArgs...) + var pageNames []string + if err := sqlx.SelectContext(ctx, dbReader, &pageNames, pageNamesStmt, pageNamesArgs...); err != nil { + return nil, nil, ctxerr.Wrap(ctx, err, "selecting fleet maintained app page names") } meta := &fleet.PaginationMetadata{HasPreviousResults: opt.Page > 0, TotalResults: uint(filteredCount)} //nolint:gosec // dismiss G115 - if len(avail) > int(opt.PerPage) { //nolint:gosec // dismiss G115 + if uint(len(pageNames)) > perPage { //nolint:gosec // dismiss G115 meta.HasNextResults = true - avail = avail[:len(avail)-1] + pageNames = pageNames[:perPage] + } + if len(pageNames) == 0 { + // Page is past the last result. + return []fleet.MaintainedApp{}, meta, nil + } + + // Fetch every platform row for the apps on this page so the UI can combine + // an app's macOS and Windows entries into a single row. + selectStmt := `SELECT fma.id, fma.name, fma.platform, fma.slug, ` + var rowsArgs []any + if teamID != nil { + selectStmt += teamFMATitlesJoin + ` WHERE fma.name IN (?)` + rowsArgs = []any{teamID, teamID, pageNames} + } else { + selectStmt += `NULL software_title_id FROM fleet_maintained_apps fma WHERE fma.name IN (?)` + rowsArgs = []any{pageNames} + } + selectStmt += fmt.Sprintf(` ORDER BY fma.name %s, fma.platform ASC`, direction) + + selectStmt, rowsArgs, err := sqlx.In(selectStmt, rowsArgs...) + if err != nil { + return nil, nil, ctxerr.Wrap(ctx, err, "building list fleet maintained apps query") + } + selectStmt = dbReader.Rebind(selectStmt) + + var avail []fleet.MaintainedApp + if err := sqlx.SelectContext(ctx, dbReader, &avail, selectStmt, rowsArgs...); err != nil { + return nil, nil, ctxerr.Wrap(ctx, err, "selecting available fleet maintained apps") } return avail, meta, nil diff --git a/server/datastore/mysql/maintained_apps_test.go b/server/datastore/mysql/maintained_apps_test.go index 7137fb86f4..2b2d6077d2 100644 --- a/server/datastore/mysql/maintained_apps_test.go +++ b/server/datastore/mysql/maintained_apps_test.go @@ -23,6 +23,7 @@ func TestMaintainedApps(t *testing.T) { {"UpsertMaintainedApps", testUpsertMaintainedApps}, {"Sync", testSync}, {"ListAndGetAvailableApps", testListAndGetAvailableApps}, + {"ListAvailableAppsByNameAndFilters", testListAvailableAppsByNameAndFilters}, {"SyncAndRemoveApps", testSyncAndRemoveApps}, {"GetMaintainedAppBySlug", testGetMaintainedAppBySlug}, {"ListAvailableAppsWindows", testListAvailableAppsWindows}, @@ -60,11 +61,11 @@ func testUpsertMaintainedApps(t *testing.T, ds *Datastore) { }) } - require.Equal(t, expectedAppsBaseInfo, listSavedApps()) + require.ElementsMatch(t, expectedAppsBaseInfo, listSavedApps()) // ingesting again results in no changes maintainedappstest.SyncApps(t, ds) - require.Equal(t, expectedAppsBaseInfo, listSavedApps()) + require.ElementsMatch(t, expectedAppsBaseInfo, listSavedApps()) // upsert the figma app, changing the version _, err := ds.UpsertMaintainedApp(ctx, &fleet.MaintainedApp{ @@ -82,7 +83,7 @@ func testUpsertMaintainedApps(t *testing.T, ds *Datastore) { } } - require.Equal(t, expectedAppsBaseInfo, listSavedApps()) + require.ElementsMatch(t, expectedAppsBaseInfo, listSavedApps()) } func testSync(t *testing.T, ds *Datastore) { @@ -106,7 +107,7 @@ func testListAndGetAvailableApps(t *testing.T, ds *Datastore) { require.NoError(t, err) // Testing search that returns no results; nothing inserted yet case - _, _, err = ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.ListOptions{IncludeMetadata: true}) + _, _, err = ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.MaintainedAppListOptions{ListOptions: fleet.ListOptions{IncludeMetadata: true}}) require.ErrorIs(t, err, &fleet.NoMaintainedAppsInDatabaseError{}) maintained1, err := ds.UpsertMaintainedApp(ctx, &fleet.MaintainedApp{ @@ -175,21 +176,21 @@ func testListAndGetAvailableApps(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.MaintainedAppListOptions{ListOptions: fleet.ListOptions{IncludeMetadata: true}}) require.NoError(t, err) require.Len(t, apps, 4) require.EqualValues(t, meta.TotalResults, 4) 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.MaintainedAppListOptions{ListOptions: fleet.ListOptions{PerPage: 1, IncludeMetadata: true}}) require.NoError(t, err) require.Len(t, apps, 1) require.EqualValues(t, meta.TotalResults, 4) 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.MaintainedAppListOptions{ListOptions: fleet.ListOptions{PerPage: 1, Page: 1, IncludeMetadata: true}}) require.NoError(t, err) require.Len(t, apps, 1) require.EqualValues(t, meta.TotalResults, 4) @@ -197,7 +198,7 @@ func testListAndGetAvailableApps(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.MaintainedAppListOptions{ListOptions: fleet.ListOptions{PerPage: 1, Page: 2, IncludeMetadata: true}}) require.NoError(t, err) require.Len(t, apps, 1) require.EqualValues(t, meta.TotalResults, 4) @@ -205,7 +206,7 @@ func testListAndGetAvailableApps(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: 3, IncludeMetadata: true}) + apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.MaintainedAppListOptions{ListOptions: fleet.ListOptions{PerPage: 1, Page: 3, IncludeMetadata: true}}) require.NoError(t, err) require.Len(t, apps, 1) require.EqualValues(t, meta.TotalResults, 4) @@ -214,7 +215,7 @@ func testListAndGetAvailableApps(t *testing.T, ds *Datastore) { require.True(t, meta.HasPreviousResults) // Testing search - apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.ListOptions{MatchQuery: "Maintained4", IncludeMetadata: true}) + apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.MaintainedAppListOptions{ListOptions: fleet.ListOptions{MatchQuery: "Maintained4", IncludeMetadata: true}}) require.NoError(t, err) require.Len(t, apps, 1) require.EqualValues(t, 1, meta.TotalResults) @@ -223,7 +224,7 @@ func testListAndGetAvailableApps(t *testing.T, ds *Datastore) { require.False(t, meta.HasPreviousResults) // Testing search that returns no results; non-error case - apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.ListOptions{MatchQuery: "Maintained5", IncludeMetadata: true}) + apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.MaintainedAppListOptions{ListOptions: fleet.ListOptions{MatchQuery: "Maintained5", IncludeMetadata: true}}) require.NoError(t, err) require.Len(t, apps, 0) require.EqualValues(t, 0, meta.TotalResults) @@ -246,7 +247,7 @@ func testListAndGetAvailableApps(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.MaintainedAppListOptions{ListOptions: fleet.ListOptions{IncludeMetadata: true}}) require.NoError(t, err) require.Len(t, apps, 4) require.EqualValues(t, meta.TotalResults, 4) @@ -265,7 +266,7 @@ func testListAndGetAvailableApps(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.MaintainedAppListOptions{ListOptions: fleet.ListOptions{IncludeMetadata: true}}) require.NoError(t, err) require.Len(t, apps, 4) require.EqualValues(t, meta.TotalResults, 4) @@ -284,7 +285,7 @@ func testListAndGetAvailableApps(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.MaintainedAppListOptions{ListOptions: fleet.ListOptions{IncludeMetadata: true}}) require.NoError(t, err) require.Len(t, apps, 4) require.EqualValues(t, meta.TotalResults, 4) @@ -300,7 +301,7 @@ func testListAndGetAvailableApps(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.MaintainedAppListOptions{ListOptions: fleet.ListOptions{IncludeMetadata: true}}) require.NoError(t, err) require.Len(t, apps, 4) require.EqualValues(t, meta.TotalResults, 4) @@ -317,7 +318,7 @@ func testListAndGetAvailableApps(t *testing.T, ds *Datastore) { require.Equal(t, maintained1, gotApp) // we haven't added the windows app yet, so we shouldn't have a title ID for it - apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.ListOptions{IncludeMetadata: true}) + apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.MaintainedAppListOptions{ListOptions: fleet.ListOptions{IncludeMetadata: true}}) require.NoError(t, err) require.Len(t, apps, 4) require.EqualValues(t, meta.TotalResults, 4) @@ -335,7 +336,7 @@ func testListAndGetAvailableApps(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.MaintainedAppListOptions{ListOptions: fleet.ListOptions{IncludeMetadata: true}}) require.NoError(t, err) require.Len(t, apps, 4) require.EqualValues(t, meta.TotalResults, 4) @@ -366,7 +367,7 @@ func testListAndGetAvailableApps(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.MaintainedAppListOptions{ListOptions: fleet.ListOptions{IncludeMetadata: true}}) require.NoError(t, err) require.Len(t, apps, 4) require.EqualValues(t, meta.TotalResults, 4) @@ -386,7 +387,7 @@ func testListAndGetAvailableApps(t *testing.T, ds *Datastore) { vppApp, 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.MaintainedAppListOptions{ListOptions: fleet.ListOptions{IncludeMetadata: true}}) require.NoError(t, err) require.Len(t, apps, 4) require.EqualValues(t, meta.TotalResults, 4) @@ -407,7 +408,7 @@ func testListAndGetAvailableApps(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.MaintainedAppListOptions{ListOptions: fleet.ListOptions{IncludeMetadata: true}}) require.NoError(t, err) require.Len(t, apps, 4) require.EqualValues(t, meta.TotalResults, 4) @@ -421,7 +422,7 @@ func testListAndGetAvailableApps(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.MaintainedAppListOptions{ListOptions: fleet.ListOptions{IncludeMetadata: true}}) require.NoError(t, err) require.Len(t, apps, 4) require.EqualValues(t, meta.TotalResults, 4) @@ -434,7 +435,7 @@ func testListAndGetAvailableApps(t *testing.T, ds *Datastore) { require.Equal(t, maintained2, gotApp) // viewing with no team selected shouldn't include any title IDs - apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, nil, fleet.ListOptions{IncludeMetadata: true}) + apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, nil, fleet.MaintainedAppListOptions{ListOptions: fleet.ListOptions{IncludeMetadata: true}}) require.NoError(t, err) require.Len(t, apps, 4) require.EqualValues(t, meta.TotalResults, 4) @@ -453,18 +454,49 @@ func testListAndGetAvailableApps(t *testing.T, ds *Datastore) { maintained3.TitleID = nil require.Equal(t, maintained3, gotApp) - for _, key := range []string{"id", "name", "platform", "slug"} { - t.Run("order_"+key, func(t *testing.T) { - result, _, err := ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.ListOptions{OrderKey: key, PerPage: 10, IncludeMetadata: true}) - require.NoError(t, err) - require.NotEmpty(t, result) - }) + // Ordering: the combined-by-name view is only meaningfully sortable by name, + // so "name" is the one allowed order key. expectedApps is declared in + // ascending name order, so we derive the expected name sequences from it. + appNames := func(apps []fleet.MaintainedApp) []string { + got := make([]string, 0, len(apps)) + for _, a := range apps { + got = append(got, a.Name) + } + return got + } + ascNames := appNames(expectedApps) + descNames := make([]string, len(ascNames)) + for i, name := range ascNames { + descNames[len(ascNames)-1-i] = name } - t.Run("rejects_unknown_key", func(t *testing.T) { - _, _, err := ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.ListOptions{OrderKey: "h.node_key", IncludeMetadata: true}) - require.Error(t, err) + t.Run("order_name_ascending", func(t *testing.T) { + result, _, err := ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.MaintainedAppListOptions{ListOptions: fleet.ListOptions{OrderKey: "name", OrderDirection: fleet.OrderAscending, PerPage: 10, IncludeMetadata: true}}) + require.NoError(t, err) + require.Equal(t, ascNames, appNames(result)) }) + + t.Run("order_name_descending", func(t *testing.T) { + result, _, err := ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.MaintainedAppListOptions{ListOptions: fleet.ListOptions{OrderKey: "name", OrderDirection: fleet.OrderDescending, PerPage: 10, IncludeMetadata: true}}) + require.NoError(t, err) + require.Equal(t, descNames, appNames(result)) + }) + + t.Run("empty_order_key_defaults_to_name", func(t *testing.T) { + result, _, err := ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.MaintainedAppListOptions{ListOptions: fleet.ListOptions{PerPage: 10, IncludeMetadata: true}}) + require.NoError(t, err) + require.Equal(t, ascNames, appNames(result)) + }) + + // Only "name" is allowed. Keys that used to be in the allowlist (id, + // platform, slug) and any other column must now be rejected, rather than + // silently falling back to name ordering. + for _, key := range []string{"id", "platform", "slug", "h.node_key"} { + t.Run("rejects_"+key, func(t *testing.T) { + _, _, err := ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.MaintainedAppListOptions{ListOptions: fleet.ListOptions{OrderKey: key, IncludeMetadata: true}}) + require.Error(t, err) + }) + } } func testSyncAndRemoveApps(t *testing.T, ds *Datastore) { @@ -597,7 +629,7 @@ func testListAvailableAppsWindows(t *testing.T, ds *Datastore) { Slug: "maintained2", }, } - apps, _, err := ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.ListOptions{IncludeMetadata: true}) + apps, _, err := ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.MaintainedAppListOptions{ListOptions: fleet.ListOptions{IncludeMetadata: true}}) require.NoError(t, err) require.Len(t, apps, 2) require.Nil(t, apps[0].TitleID) @@ -638,7 +670,7 @@ func testListAvailableAppsWindows(t *testing.T, ds *Datastore) { require.NoError(t, err) // the windows app should be found using using name, because the existing software title has an upgrade code - apps, _, err = ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.ListOptions{IncludeMetadata: true}) + apps, _, err = ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.MaintainedAppListOptions{ListOptions: fleet.ListOptions{IncludeMetadata: true}}) require.NoError(t, err) require.Len(t, apps, 2) require.NotNil(t, apps[0].TitleID) @@ -647,6 +679,119 @@ func testListAvailableAppsWindows(t *testing.T, ds *Datastore) { require.Nil(t, apps[1].TitleID) } +// testListAvailableAppsByNameAndFilters verifies that the list paginates by +// distinct app NAME (an app's macOS and Windows entries are combined into one +// logical app in the UI) while the total count is by distinct app row (each +// platform entry counted separately), and that the platform and available-only +// filters work server-side. +func testListAvailableAppsByNameAndFilters(t *testing.T, ds *Datastore) { + ctx := context.Background() + + team, err := ds.NewTeam(ctx, &fleet.Team{Name: "Team Filters"}) + require.NoError(t, err) + user := test.NewUser(t, ds, "Filter Tester", "filters@example.com", true) + + mkApp := func(name, slug, platform, ident string) *fleet.MaintainedApp { + app, err := ds.UpsertMaintainedApp(ctx, &fleet.MaintainedApp{ + Name: name, Slug: slug, Platform: platform, UniqueIdentifier: ident, + }) + require.NoError(t, err) + return app + } + // Alpha and Delta exist on both platforms; Beta is macOS-only; Gamma is + // Windows-only. That's 4 distinct apps across 6 rows. + mkApp("Alpha", "alpha/darwin", "darwin", "com.example.alpha") + mkApp("Alpha", "alpha/windows", "windows", "Alpha (MSI)") + beta := mkApp("Beta", "beta/darwin", "darwin", "com.example.beta") + mkApp("Gamma", "gamma/windows", "windows", "Gamma (MSI)") + mkApp("Delta", "delta/darwin", "darwin", "com.example.delta") + mkApp("Delta", "delta/windows", "windows", "Delta (MSI)") + + appNames := func(apps []fleet.MaintainedApp) []string { + out := make([]string, len(apps)) + for i, a := range apps { + out[i] = a.Name + } + return out + } + listOpts := func(o fleet.ListOptions) fleet.MaintainedAppListOptions { + o.IncludeMetadata = true + return fleet.MaintainedAppListOptions{ListOptions: o} + } + + // Unfiltered: 6 apps (the count, one per platform entry) across 4 names, 6 + // rows returned. + apps, meta, err := ds.ListAvailableFleetMaintainedApps(ctx, &team.ID, listOpts(fleet.ListOptions{})) + require.NoError(t, err) + require.EqualValues(t, 6, meta.TotalResults) + require.Len(t, apps, 6) + require.False(t, meta.HasNextResults) + + // Pagination is by app name: a page of 2 names that includes a dual-platform + // app returns ALL of that app's rows, so an app is never split across a page + // boundary. Page 0 => Alpha (darwin+windows) + Beta (darwin) = 3 rows. + apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team.ID, listOpts(fleet.ListOptions{PerPage: 2})) + require.NoError(t, err) + require.EqualValues(t, 6, meta.TotalResults) + require.True(t, meta.HasNextResults) + require.False(t, meta.HasPreviousResults) + require.Equal(t, []string{"Alpha", "Alpha", "Beta"}, appNames(apps)) + + // Page 1 => Delta (darwin+windows) + Gamma (windows) = 3 rows. + apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team.ID, listOpts(fleet.ListOptions{PerPage: 2, Page: 1})) + require.NoError(t, err) + require.True(t, meta.HasPreviousResults) + require.False(t, meta.HasNextResults) + require.Equal(t, []string{"Delta", "Delta", "Gamma"}, appNames(apps)) + + // Platform filter (darwin): keeps apps that have a macOS entry (Alpha, Beta, + // Delta) and returns all of their rows so the UI can still show both columns. + apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team.ID, fleet.MaintainedAppListOptions{Platform: "darwin", ListOptions: fleet.ListOptions{IncludeMetadata: true}}) + require.NoError(t, err) + require.EqualValues(t, 3, meta.TotalResults) + require.ElementsMatch(t, []string{"Alpha", "Alpha", "Beta", "Delta", "Delta"}, appNames(apps)) + + // Platform filter (windows): keeps Alpha, Gamma, Delta. + apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team.ID, fleet.MaintainedAppListOptions{Platform: "windows", ListOptions: fleet.ListOptions{IncludeMetadata: true}}) + require.NoError(t, err) + require.EqualValues(t, 3, meta.TotalResults) + require.ElementsMatch(t, []string{"Alpha", "Alpha", "Gamma", "Delta", "Delta"}, appNames(apps)) + + // Add Beta (macOS-only) to the team so it is no longer "available". + _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ + Title: beta.Name, + TeamID: &team.ID, + InstallScript: "nothing", + Filename: "beta.pkg", + UserID: user.ID, + Platform: string(fleet.MacOSPlatform), + BundleIdentifier: beta.UniqueIdentifier, + ValidatedLabels: &fleet.LabelIdentsWithScope{}, + }) + require.NoError(t, err) + + // Available-only hides Beta (its only platform is added) but keeps the other + // three apps, which still have at least one not-yet-added platform. The count + // is the 5 not-yet-added platform entries (Alpha macOS+Windows, Gamma + // Windows, Delta macOS+Windows). + apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team.ID, fleet.MaintainedAppListOptions{AvailableOnly: true, ListOptions: fleet.ListOptions{IncludeMetadata: true}}) + require.NoError(t, err) + require.EqualValues(t, 5, meta.TotalResults) + require.NotContains(t, appNames(apps), "Beta") + + // Without the filter, Beta is still listed (as added). + apps, _, err = ds.ListAvailableFleetMaintainedApps(ctx, &team.ID, listOpts(fleet.ListOptions{})) + require.NoError(t, err) + require.Contains(t, appNames(apps), "Beta") + + // Platform and available-only combine: macOS apps not yet added are Alpha + // and Delta (Beta's macOS entry is added; Gamma has no macOS entry). + apps, meta, err = ds.ListAvailableFleetMaintainedApps(ctx, &team.ID, fleet.MaintainedAppListOptions{Platform: "darwin", AvailableOnly: true, ListOptions: fleet.ListOptions{IncludeMetadata: true}}) + require.NoError(t, err) + require.EqualValues(t, 2, meta.TotalResults) + require.ElementsMatch(t, []string{"Alpha", "Alpha", "Delta", "Delta"}, appNames(apps)) +} + func testSoftwareTitleRenamingWindows(t *testing.T, ds *Datastore) { ctx := context.Background() diff --git a/server/fleet/datastore.go b/server/fleet/datastore.go index 6f54701ddc..423a3a89b0 100644 --- a/server/fleet/datastore.go +++ b/server/fleet/datastore.go @@ -2977,7 +2977,7 @@ type Datastore interface { // ListAvailableFleetMaintainedApps returns a list of Fleet-maintained apps, including software title ID if // either the maintained app or a custom package/VPP app for the same app is installed on the specified team, // if a team is specified. - ListAvailableFleetMaintainedApps(ctx context.Context, teamID *uint, opt ListOptions) ([]MaintainedApp, *PaginationMetadata, error) + ListAvailableFleetMaintainedApps(ctx context.Context, teamID *uint, opt MaintainedAppListOptions) ([]MaintainedApp, *PaginationMetadata, error) // ClearRemovedFleetMaintainedApps deletes all Fleet-maintained apps that are not in the given // set of slugs. diff --git a/server/fleet/maintained_apps.go b/server/fleet/maintained_apps.go index ad9e86842f..2f2ebf2040 100644 --- a/server/fleet/maintained_apps.go +++ b/server/fleet/maintained_apps.go @@ -42,6 +42,27 @@ func (s *MaintainedApp) AuthzType() string { return "maintained_app" } +// MaintainedAppListOptions contains the options for listing Fleet-maintained +// apps. Pagination operates on distinct app names (an app's macOS and Windows +// entries are combined into a single row in the UI), so an app is never split +// across a page boundary. The count, however, is the total number of +// installable apps, with each platform entry counted separately. +type MaintainedAppListOptions struct { + ListOptions + + // Platform optionally filters to apps that have an entry on the given + // platform ("darwin" or "windows"); an empty value returns all platforms. + // This restricts which apps appear (and the count), not which platform rows + // are returned: every platform entry of a matching app is still included so + // the UI can render all of an app's platforms. + Platform string + + // AvailableOnly, when true, returns only apps that have not yet been added + // to the team (the "Hide added apps" filter). It has no effect when no team + // is specified, since the added/available distinction is team-scoped. + AvailableOnly bool +} + // NoMaintainedAppsInDatabaseError is the error type for no Fleet Maintained Apps in the database type NoMaintainedAppsInDatabaseError struct { ErrorWithUUID diff --git a/server/fleet/service.go b/server/fleet/service.go index d6dbbc445a..9ef525eb00 100644 --- a/server/fleet/service.go +++ b/server/fleet/service.go @@ -1462,7 +1462,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, automaticInstall bool, labelsIncludeAny, labelsExcludeAny, labelsIncludeAll []string) (uint, error) // ListFleetMaintainedApps lists Fleet-maintained apps, including associated software title for supplied team ID (if any) - ListFleetMaintainedApps(ctx context.Context, teamID *uint, opts ListOptions) ([]MaintainedApp, *PaginationMetadata, error) + ListFleetMaintainedApps(ctx context.Context, teamID *uint, opts MaintainedAppListOptions) ([]MaintainedApp, *PaginationMetadata, error) // GetFleetMaintainedApp returns a Fleet-maintained app by ID, including associated software title for supplied team ID (if any) GetFleetMaintainedApp(ctx context.Context, appID uint, teamID *uint) (*MaintainedApp, error) diff --git a/server/mdm/maintainedapps/maintainedappstest/maintainedappstest.go b/server/mdm/maintainedapps/maintainedappstest/maintainedappstest.go index 29b996cc67..0671575fad 100644 --- a/server/mdm/maintainedapps/maintainedappstest/maintainedappstest.go +++ b/server/mdm/maintainedapps/maintainedappstest/maintainedappstest.go @@ -65,9 +65,11 @@ func SyncApps(t *testing.T, ds fleet.Datastore) []fleet.MaintainedApp { err := maintained_apps.SyncAppsList(context.Background(), ds) require.NoError(t, err) - apps, _, err := ds.ListAvailableFleetMaintainedApps(context.Background(), nil, fleet.ListOptions{ - OrderKey: "slug", - }) + // The list endpoint paginates and orders by app name. With default options + // GetPerPage returns DefaultPerPage (effectively unbounded), so this helper + // gets the full set in a single page for tests, which should not depend on + // the order. + apps, _, err := ds.ListAvailableFleetMaintainedApps(context.Background(), nil, fleet.MaintainedAppListOptions{}) require.NoError(t, err) return apps } @@ -119,7 +121,7 @@ func SyncAndRemoveApps(t *testing.T, ds fleet.Datastore) { err = maintained_apps.SyncAppsList(context.Background(), ds) require.NoError(t, err) - originalApps, _, err := ds.ListAvailableFleetMaintainedApps(context.Background(), nil, fleet.ListOptions{}) + originalApps, _, err := ds.ListAvailableFleetMaintainedApps(context.Background(), nil, fleet.MaintainedAppListOptions{}) require.NoError(t, err) require.Len(t, originalApps, len(appsFile.Apps)) @@ -131,7 +133,7 @@ func SyncAndRemoveApps(t *testing.T, ds fleet.Datastore) { err = maintained_apps.SyncAppsList(context.Background(), ds) require.NoError(t, err) - modifiedApps, _, err := ds.ListAvailableFleetMaintainedApps(context.Background(), nil, fleet.ListOptions{}) + modifiedApps, _, err := ds.ListAvailableFleetMaintainedApps(context.Background(), nil, fleet.MaintainedAppListOptions{}) require.NoError(t, err) require.Len(t, modifiedApps, len(appsFile.Apps)) @@ -148,7 +150,7 @@ func SyncAndRemoveApps(t *testing.T, ds fleet.Datastore) { err = maintained_apps.SyncAppsList(context.Background(), ds) require.NoError(t, err) - modifiedApps, _, err = ds.ListAvailableFleetMaintainedApps(context.Background(), nil, fleet.ListOptions{}) + modifiedApps, _, err = ds.ListAvailableFleetMaintainedApps(context.Background(), nil, fleet.MaintainedAppListOptions{}) require.ErrorIs(t, err, &fleet.NoMaintainedAppsInDatabaseError{}) require.Empty(t, modifiedApps) } diff --git a/server/mdm/maintainedapps/testing_utils_test.go b/server/mdm/maintainedapps/testing_utils_test.go index 024f62e731..566100e0c1 100644 --- a/server/mdm/maintainedapps/testing_utils_test.go +++ b/server/mdm/maintainedapps/testing_utils_test.go @@ -49,9 +49,11 @@ func SyncApps(t *testing.T, ds fleet.Datastore) []fleet.MaintainedApp { err := SyncAppsList(context.Background(), ds) require.NoError(t, err) - apps, _, err := ds.ListAvailableFleetMaintainedApps(context.Background(), nil, fleet.ListOptions{ - OrderKey: "slug", - }) + // The list endpoint paginates and orders by app name. With default options + // GetPerPage returns DefaultPerPage (effectively unbounded), so this helper + // gets the full set in a single page for tests, which should not depend on + // the order. + apps, _, err := ds.ListAvailableFleetMaintainedApps(context.Background(), nil, fleet.MaintainedAppListOptions{}) require.NoError(t, err) return apps } @@ -105,7 +107,7 @@ func SyncAndRemoveApps(t *testing.T, ds fleet.Datastore) { err = SyncAppsList(context.Background(), ds) require.NoError(t, err) - originalApps, _, err := ds.ListAvailableFleetMaintainedApps(context.Background(), nil, fleet.ListOptions{}) + originalApps, _, err := ds.ListAvailableFleetMaintainedApps(context.Background(), nil, fleet.MaintainedAppListOptions{}) require.NoError(t, err) require.Equal(t, len(appsFile.Apps), len(originalApps)) @@ -117,7 +119,7 @@ func SyncAndRemoveApps(t *testing.T, ds fleet.Datastore) { err = SyncAppsList(context.Background(), ds) require.NoError(t, err) - modifiedApps, _, err := ds.ListAvailableFleetMaintainedApps(context.Background(), nil, fleet.ListOptions{}) + modifiedApps, _, err := ds.ListAvailableFleetMaintainedApps(context.Background(), nil, fleet.MaintainedAppListOptions{}) require.NoError(t, err) require.Equal(t, len(appsFile.Apps), len(modifiedApps)) @@ -132,7 +134,7 @@ func SyncAndRemoveApps(t *testing.T, ds fleet.Datastore) { err = SyncAppsList(context.Background(), ds) require.NoError(t, err) - modifiedApps, _, err = ds.ListAvailableFleetMaintainedApps(context.Background(), nil, fleet.ListOptions{}) + modifiedApps, _, err = ds.ListAvailableFleetMaintainedApps(context.Background(), nil, fleet.MaintainedAppListOptions{}) require.ErrorIs(t, err, &fleet.NoMaintainedAppsInDatabaseError{}) require.Empty(t, modifiedApps) } diff --git a/server/mock/datastore_mock.go b/server/mock/datastore_mock.go index 46cf75d248..2c95128ea1 100644 --- a/server/mock/datastore_mock.go +++ b/server/mock/datastore_mock.go @@ -1740,7 +1740,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.MaintainedAppListOptions) ([]fleet.MaintainedApp, *fleet.PaginationMetadata, error) type ClearRemovedFleetMaintainedAppsFunc func(ctx context.Context, slugsToKeep []string) error @@ -11244,7 +11244,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.MaintainedAppListOptions) ([]fleet.MaintainedApp, *fleet.PaginationMetadata, error) { s.mu.Lock() s.ListAvailableFleetMaintainedAppsFuncInvoked = true s.mu.Unlock() diff --git a/server/mock/service/service_mock.go b/server/mock/service/service_mock.go index 6c57753460..2a109e2bb7 100644 --- a/server/mock/service/service_mock.go +++ b/server/mock/service/service_mock.go @@ -892,7 +892,7 @@ type IsAllSetupExperienceSoftwareRequiredFunc func(ctx context.Context, host *fl type AddFleetMaintainedAppFunc func(ctx context.Context, teamID *uint, appID uint, installScript string, preInstallQuery string, postInstallScript string, uninstallScript string, selfService bool, automaticInstall bool, labelsIncludeAny []string, labelsExcludeAny []string, labelsIncludeAll []string) (uint, error) -type ListFleetMaintainedAppsFunc func(ctx context.Context, teamID *uint, opts fleet.ListOptions) ([]fleet.MaintainedApp, *fleet.PaginationMetadata, error) +type ListFleetMaintainedAppsFunc func(ctx context.Context, teamID *uint, opts fleet.MaintainedAppListOptions) ([]fleet.MaintainedApp, *fleet.PaginationMetadata, error) type GetFleetMaintainedAppFunc func(ctx context.Context, appID uint, teamID *uint) (*fleet.MaintainedApp, error) @@ -5376,7 +5376,7 @@ func (s *Service) AddFleetMaintainedApp(ctx context.Context, teamID *uint, appID return s.AddFleetMaintainedAppFunc(ctx, teamID, appID, installScript, preInstallQuery, postInstallScript, uninstallScript, selfService, automaticInstall, labelsIncludeAny, labelsExcludeAny, labelsIncludeAll) } -func (s *Service) ListFleetMaintainedApps(ctx context.Context, teamID *uint, opts fleet.ListOptions) ([]fleet.MaintainedApp, *fleet.PaginationMetadata, error) { +func (s *Service) ListFleetMaintainedApps(ctx context.Context, teamID *uint, opts fleet.MaintainedAppListOptions) ([]fleet.MaintainedApp, *fleet.PaginationMetadata, error) { s.mu.Lock() s.ListFleetMaintainedAppsFuncInvoked = true s.mu.Unlock() diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 3cf5f6ee90..9362d36b44 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -20778,6 +20778,12 @@ func (s *integrationEnterpriseTestSuite) TestMaintainedApps() { require.False(t, listMAResp.Meta.HasNextResults) require.Len(t, listMAResp.FleetMaintainedApps, len(expectedApps)) + // Count is the total number of platform-specific maintained apps: an app's + // macOS and Windows entries are counted separately, even though the UI + // combines them into a single row. The full unfiltered list returns one row + // per app, so the count equals the number of rows returned. + require.Equal(t, len(expectedApps), listMAResp.Count) + sortFMAs := func(a, b fleet.MaintainedApp) int { if c := cmp.Compare(a.Name, b.Name); c != 0 { return c @@ -20805,6 +20811,23 @@ func (s *integrationEnterpriseTestSuite) TestMaintainedApps() { require.Len(t, listMAResp2.FleetMaintainedApps, 2) require.Contains(t, listMAResp.FleetMaintainedApps, listMAResp2.FleetMaintainedApps[0]) + // The platform filter narrows the list to apps available on that platform. + // Fewer apps ship a Windows installer than a macOS one, so the Windows-only + // count is a non-empty strict subset of the full count. + var listMAWin listFleetMaintainedAppsResponse + s.DoJSON( + http.MethodGet, + "/api/latest/fleet/software/fleet_maintained_apps", + listFleetMaintainedAppsRequest{}, + http.StatusOK, + &listMAWin, + "team_id", fmt.Sprint(team.ID), + "platform", "windows", + ) + require.NoError(t, listMAWin.Err) + require.Positive(t, listMAWin.Count) + require.Less(t, listMAWin.Count, listMAResp.Count) + // Check individual app fetch var getMAResp getFleetMaintainedAppResponse s.DoJSON(http.MethodGet, fmt.Sprintf("/api/latest/fleet/software/fleet_maintained_apps/%d", listMAResp.FleetMaintainedApps[0].ID), getFleetMaintainedAppRequest{}, http.StatusOK, &getMAResp) diff --git a/server/service/maintained_apps.go b/server/service/maintained_apps.go index 90d1602a9e..d1816cec8b 100644 --- a/server/service/maintained_apps.go +++ b/server/service/maintained_apps.go @@ -129,10 +129,17 @@ func (svc *Service) AddFleetMaintainedApp(ctx context.Context, _ *uint, _ uint, type listFleetMaintainedAppsRequest struct { fleet.ListOptions TeamID *uint `query:"team_id,optional" renameto:"fleet_id"` + // Platform optionally filters to apps available on the given platform + // ("darwin" or "windows"). + Platform string `query:"platform,optional"` + // AvailableOnly, when true, returns only apps not yet added to the team + // (the "Hide added apps" filter). + AvailableOnly bool `query:"available,optional"` } type listFleetMaintainedAppsResponse struct { FleetMaintainedApps []fleet.MaintainedApp `json:"fleet_maintained_apps"` + Count int `json:"count"` Meta *fleet.PaginationMetadata `json:"meta"` Err error `json:"error,omitempty"` } @@ -142,7 +149,13 @@ func (r listFleetMaintainedAppsResponse) Error() error { return r.Err } func listFleetMaintainedAppsEndpoint(ctx context.Context, request any, svc fleet.Service) (fleet.Errorer, error) { req := request.(*listFleetMaintainedAppsRequest) - apps, meta, err := svc.ListFleetMaintainedApps(ctx, req.TeamID, req.ListOptions) + opts := fleet.MaintainedAppListOptions{ + ListOptions: req.ListOptions, + Platform: req.Platform, + AvailableOnly: req.AvailableOnly, + } + + apps, meta, err := svc.ListFleetMaintainedApps(ctx, req.TeamID, opts) if err != nil { return listFleetMaintainedAppsResponse{Err: err}, nil } @@ -151,11 +164,14 @@ func listFleetMaintainedAppsEndpoint(ctx context.Context, request any, svc fleet FleetMaintainedApps: apps, Meta: meta, } + if meta != nil { + listResp.Count = int(meta.TotalResults) //nolint:gosec // dismiss G115 + } 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.MaintainedAppListOptions) ([]fleet.MaintainedApp, *fleet.PaginationMetadata, error) { // skipauth: No authorization check needed due to implementation returning // only license error. svc.authz.SkipAuthorization(ctx)