Fixed GET /api/v1/fleet/commands timeout in large Fleet deployments (#44297)
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #44170 and Resolves #44422 Pagination is now pushed into each branch of the merged query, so per-tick work scales with page size instead of total commands. The Windows side was rewritten to avoid a disjunctive join that forced a nested-loop plan. `per_page` is capped (default 10), `page` is capped, and `order_key` is enforced against a closed allowlist on both code paths. Cursor pagination is fixed and is the recommended way to traverse beyond the page cap. This PR improves but does not fix the use case of fetching commands from all hosts. Deprecate usage without host_identifier: https://github.com/fleetdm/fleet/pull/44392/changes API doc updates: https://github.com/fleetdm/fleet/pull/44292 # Checklist for submitter - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. - [x] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes ## Testing - [x] Added/updated automated tests - [x] Where appropriate, [automated tests simulate multiple hosts and test for host isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing) (updates to one hosts's records do not affect another) - [x] QA'd all new/changed functionality manually <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Enforced pagination on MDM commands list: per_page defaults to 10 (max 1,000) and page is capped at 100; traversal beyond page 100 requires cursor pagination via after. * **Bug Fixes / Performance** * Improved MDM command listing performance and de-duplication for large queries; fixed SQL error when combining host identifier with cursor pagination. * **Validation** * Requests exceeding pagination caps return 400; invalid sort keys return 422. * **Tests** * Added tests for pagination boundaries, cursor behavior, sort-key validation, and error responses. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -0,0 +1 @@
|
||||
- Fixed slow load times and timeouts on the list MDM commands API (`GET /api/v1/fleet/commands`) on Fleet deployments with many Windows hosts. The endpoint now caps `per_page` at 1,000 (default 10) and `page` at 100; requests above either limit return HTTP 400. To traverse beyond 100 pages, use cursor pagination via the `after` query parameter.
|
||||
@@ -0,0 +1 @@
|
||||
- Fixed `GET /api/v1/fleet/commands` returning a SQL error when called with `host_identifier` and the `after` cursor parameter, particularly with `order_key=command_uuid` or `order_key=hostname`.
|
||||
@@ -108,6 +108,8 @@ func (svc *Service) GetOrbitSetupExperienceStatus(ctx context.Context, orbitNode
|
||||
User: &fleet.User{GlobalRole: ptr.String(fleet.RoleAdmin)},
|
||||
}
|
||||
acctCmds, _, _, err := svc.ds.ListMDMCommands(ctx, adminTeamFilter, &fleet.MDMCommandListOptions{
|
||||
// PerPage 1: only acctCmds[0] is read below.
|
||||
ListOptions: fleet.ListOptions{PerPage: 1},
|
||||
Filters: fleet.MDMCommandFilters{
|
||||
HostIdentifier: host.UUID,
|
||||
RequestType: "AccountConfiguration",
|
||||
|
||||
+166
-51
@@ -6,6 +6,7 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"slices"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
@@ -29,16 +30,6 @@ import (
|
||||
// failures don't churn nano commands and profile renders hourly. See issue #44111.
|
||||
const renewalFailedRetryBackoff = 24 * time.Hour
|
||||
|
||||
var mdmCommandsAllowedOrderKeys = common_mysql.OrderKeyAllowlist{
|
||||
"command_uuid": "command_uuid",
|
||||
"request_type": "request_type",
|
||||
"status": "status",
|
||||
"updated_at": "updated_at",
|
||||
"hostname": "hostname",
|
||||
"host_uuid": "host_uuid",
|
||||
"name": "name",
|
||||
}
|
||||
|
||||
func (ds *Datastore) GetMDMCommandPlatform(ctx context.Context, commandUUID string) (string, error) {
|
||||
stmt := `
|
||||
SELECT CASE
|
||||
@@ -60,8 +51,17 @@ END AS platform
|
||||
return p, nil
|
||||
}
|
||||
|
||||
func getCombinedMDMCommandsQuery(ds *Datastore, hostFilter string) (string, []interface{}) {
|
||||
appleStmt := `
|
||||
// getMDMCommandsSubqueries returns the Apple and Windows command-list
|
||||
// sub-statements separately. The caller is responsible for wrapping each
|
||||
// branch with the per-branch pagination (team filter, request_type filter,
|
||||
// cursor predicate, ORDER BY, inner LIMIT) before merging them with
|
||||
// UNION ALL. Paginating inside each branch keeps work per branch at
|
||||
// O(page_size) instead of O(total commands).
|
||||
//
|
||||
// These subqueries are only used for the all-hosts listing; host-scoped
|
||||
// requests go through listMDMCommandsByHostIdentifier instead.
|
||||
func getMDMCommandsSubqueries() (appleStmt, windowsStmt string) {
|
||||
appleStmt = `
|
||||
SELECT
|
||||
nvq.id as host_uuid,
|
||||
nvq.command_uuid,
|
||||
@@ -81,34 +81,77 @@ WHERE
|
||||
nvq.active = 1
|
||||
`
|
||||
|
||||
windowsStmt := `
|
||||
// The Windows sub-statement is itself a UNION ALL of two branches: one
|
||||
// driven by windows_mdm_command_queue (any command pending or in
|
||||
// flight), the other driven by windows_mdm_command_results (any
|
||||
// command that produced a result). Branch B's NOT EXISTS clause
|
||||
// excludes (command_uuid, enrollment_id) pairs already covered by
|
||||
// branch A so the union does not double-count a single command/host
|
||||
// pair.
|
||||
windowsStmt = `
|
||||
SELECT
|
||||
mwe.host_uuid,
|
||||
wmc.command_uuid,
|
||||
COALESCE(NULLIF(wmcr.status_code, ''), '101') as status,
|
||||
COALESCE(wmc.updated_at, wmc.created_at) as updated_at,
|
||||
COALESCE(wmcr.updated_at, wmc.updated_at, wmc.created_at) as updated_at,
|
||||
wmc.target_loc_uri as request_type,
|
||||
h.hostname,
|
||||
h.team_id,
|
||||
NULL as name
|
||||
FROM windows_mdm_commands wmc
|
||||
LEFT JOIN windows_mdm_command_queue wmcq ON wmcq.command_uuid = wmc.command_uuid
|
||||
LEFT JOIN windows_mdm_command_results wmcr ON wmc.command_uuid = wmcr.command_uuid
|
||||
INNER JOIN mdm_windows_enrollments mwe ON wmcq.enrollment_id = mwe.id OR wmcr.enrollment_id = mwe.id
|
||||
INNER JOIN windows_mdm_command_queue wmcq ON wmcq.command_uuid = wmc.command_uuid
|
||||
INNER JOIN mdm_windows_enrollments mwe ON wmcq.enrollment_id = mwe.id
|
||||
INNER JOIN hosts h ON h.uuid = mwe.host_uuid
|
||||
LEFT JOIN windows_mdm_command_results wmcr
|
||||
ON wmcr.command_uuid = wmc.command_uuid AND wmcr.enrollment_id = mwe.id
|
||||
WHERE TRUE
|
||||
|
||||
UNION ALL
|
||||
|
||||
SELECT
|
||||
mwe.host_uuid,
|
||||
wmc.command_uuid,
|
||||
COALESCE(NULLIF(wmcr.status_code, ''), '101') as status,
|
||||
COALESCE(wmcr.updated_at, wmc.updated_at, wmc.created_at) as updated_at,
|
||||
wmc.target_loc_uri as request_type,
|
||||
h.hostname,
|
||||
h.team_id,
|
||||
NULL as name
|
||||
FROM windows_mdm_commands wmc
|
||||
INNER JOIN windows_mdm_command_results wmcr ON wmcr.command_uuid = wmc.command_uuid
|
||||
INNER JOIN mdm_windows_enrollments mwe ON wmcr.enrollment_id = mwe.id
|
||||
INNER JOIN hosts h ON h.uuid = mwe.host_uuid
|
||||
WHERE NOT EXISTS (
|
||||
SELECT 1 FROM windows_mdm_command_queue wmcq2
|
||||
WHERE wmcq2.command_uuid = wmc.command_uuid AND wmcq2.enrollment_id = mwe.id
|
||||
)
|
||||
`
|
||||
|
||||
var params []interface{}
|
||||
appleStmtWithFilter, params := ds.whereFilterHostsByIdentifier(hostFilter, appleStmt, params)
|
||||
windowsStmtWithFilter, params := ds.whereFilterHostsByIdentifier(hostFilter, windowsStmt, params)
|
||||
return appleStmt, windowsStmt
|
||||
}
|
||||
|
||||
stmt := fmt.Sprintf(
|
||||
// mdmCommandsOrderAllowlist is the closed set of order_key values accepted
|
||||
// by GET /api/v1/fleet/commands and GET /api/v1/fleet/mdm/commands.
|
||||
var mdmCommandsOrderAllowlist = common_mysql.OrderKeyAllowlist{
|
||||
"host_uuid": "host_uuid",
|
||||
"command_uuid": "command_uuid",
|
||||
"status": "status",
|
||||
"updated_at": "updated_at",
|
||||
"request_type": "request_type",
|
||||
"hostname": "hostname",
|
||||
"name": "name",
|
||||
}
|
||||
|
||||
// getCombinedMDMCommandsQuery returns the legacy combined statement
|
||||
// (Apple UNION ALL Windows) ending in `WHERE `. Used by getMDMCommand for
|
||||
// single-command lookups; the list-commands path builds its own form
|
||||
// (see getMDMCommandsSubqueries).
|
||||
func getCombinedMDMCommandsQuery() string {
|
||||
appleStmt, windowsStmt := getMDMCommandsSubqueries()
|
||||
return fmt.Sprintf(
|
||||
`SELECT * FROM ((%s) UNION ALL (%s)) as combined_commands WHERE `,
|
||||
appleStmtWithFilter, windowsStmtWithFilter,
|
||||
appleStmt, windowsStmt,
|
||||
)
|
||||
|
||||
return stmt, params
|
||||
}
|
||||
|
||||
func (ds *Datastore) ListMDMCommands(
|
||||
@@ -116,20 +159,78 @@ func (ds *Datastore) ListMDMCommands(
|
||||
tmFilter fleet.TeamFilter,
|
||||
listOpts *fleet.MDMCommandListOptions,
|
||||
) ([]*fleet.MDMCommand, *int64, *fleet.PaginationMetadata, error) {
|
||||
if listOpts != nil && listOpts.Filters.HostIdentifier != "" {
|
||||
if listOpts == nil || listOpts.PerPage == 0 {
|
||||
return nil, nil, nil, ctxerr.Wrap(ctx, errors.New("ListMDMCommands requires listOpts.PerPage > 0"))
|
||||
}
|
||||
if listOpts.Filters.HostIdentifier != "" {
|
||||
// separate codepath for more performant query by host identifier
|
||||
return ds.listMDMCommandsByHostIdentifier(ctx, tmFilter, listOpts)
|
||||
}
|
||||
|
||||
jointStmt, params := getCombinedMDMCommandsQuery(ds, listOpts.Filters.HostIdentifier)
|
||||
jointStmt += ds.whereFilterHostsByTeams(tmFilter, "combined_commands")
|
||||
jointStmt, params = addRequestTypeFilter(jointStmt, &listOpts.Filters, params)
|
||||
jointStmt, params, err := appendListOptionsWithCursorToSQLSecure(jointStmt, params, &listOpts.ListOptions, mdmCommandsAllowedOrderKeys)
|
||||
if err != nil {
|
||||
return nil, nil, nil, ctxerr.Wrap(ctx, err, "list commands")
|
||||
if listOpts.OrderKey == "" {
|
||||
listOpts.OrderKey = "updated_at"
|
||||
listOpts.OrderDirection = fleet.OrderDescending
|
||||
}
|
||||
|
||||
appleStmt, windowsStmt := getMDMCommandsSubqueries()
|
||||
|
||||
// Per-branch pagination: without this, the UNION ALL would materialize every command on
|
||||
// both sides before pagination, which times out at scale (#44170).
|
||||
innerOpts := listOpts.ListOptions
|
||||
// For page-based pagination, inner LIMIT = page*per_page + per_page;
|
||||
// the secure helper adds +1 because IncludeMetadata is true. For
|
||||
// cursor-based pagination (After != ""), the helper ignores Page, so
|
||||
// don't inflate the inner LIMIT (per_page+1 is sufficient). Inner
|
||||
// Page=0 suppresses the inner OFFSET; the outer wrapper handles
|
||||
// offset slicing.
|
||||
if innerOpts.After == "" {
|
||||
innerOpts.PerPage = innerOpts.PerPage*innerOpts.Page + innerOpts.PerPage
|
||||
}
|
||||
innerOpts.Page = 0
|
||||
innerOpts.IncludeMetadata = true
|
||||
|
||||
paginateBranch := func(branch string, params []any) (string, []any, error) {
|
||||
wrapped := fmt.Sprintf("SELECT * FROM (%s) AS branch WHERE ", branch)
|
||||
wrapped += ds.whereFilterHostsByTeams(tmFilter, "branch")
|
||||
wrapped, params = addRequestTypeFilter(wrapped, &listOpts.Filters, params)
|
||||
return appendListOptionsWithCursorToSQLSecure(wrapped, params, &innerOpts, mdmCommandsOrderAllowlist)
|
||||
}
|
||||
|
||||
// Each branch needs its own params slice; sqlx.SelectContext binds
|
||||
// placeholders left-to-right across the merged statement.
|
||||
var appleParams, windowsParams []any
|
||||
var err error
|
||||
if appleStmt, appleParams, err = paginateBranch(appleStmt, appleParams); err != nil {
|
||||
return nil, nil, nil, ctxerr.Wrap(ctx, err, "paginate apple commands branch")
|
||||
}
|
||||
if windowsStmt, windowsParams, err = paginateBranch(windowsStmt, windowsParams); err != nil {
|
||||
return nil, nil, nil, ctxerr.Wrap(ctx, err, "paginate windows commands branch")
|
||||
}
|
||||
|
||||
mergedStmt := fmt.Sprintf(
|
||||
"SELECT * FROM ((%s) UNION ALL (%s)) AS combined_commands",
|
||||
appleStmt, windowsStmt,
|
||||
)
|
||||
mergedParams := append([]any{}, appleParams...)
|
||||
mergedParams = append(mergedParams, windowsParams...)
|
||||
|
||||
// Outer pagination: ORDER BY + LIMIT + OFFSET only. The cursor
|
||||
// predicate is already applied inside each branch, so clear After
|
||||
// here. If the original request used cursor pagination, also clear
|
||||
// Page so the outer query does not apply an OFFSET on top of the
|
||||
// per-branch cursor filtering.
|
||||
outerOpts := listOpts.ListOptions
|
||||
if outerOpts.After != "" {
|
||||
outerOpts.After = ""
|
||||
outerOpts.Page = 0
|
||||
}
|
||||
mergedStmt, mergedParams, err = appendListOptionsWithCursorToSQLSecure(mergedStmt, mergedParams, &outerOpts, mdmCommandsOrderAllowlist)
|
||||
if err != nil {
|
||||
return nil, nil, nil, ctxerr.Wrap(ctx, err, "merge mdm commands pagination")
|
||||
}
|
||||
|
||||
var results []*fleet.MDMCommand
|
||||
if err := sqlx.SelectContext(ctx, ds.reader(ctx), &results, jointStmt, params...); err != nil {
|
||||
if err := sqlx.SelectContext(ctx, ds.reader(ctx), &results, mergedStmt, mergedParams...); err != nil {
|
||||
return nil, nil, nil, ctxerr.Wrap(ctx, err, "list commands")
|
||||
}
|
||||
|
||||
@@ -163,9 +264,9 @@ func (ds *Datastore) listMDMCommandsByHostIdentifier(
|
||||
|
||||
// First, search for host by identifier (hostname, uuid, or hardware_serial).
|
||||
//
|
||||
// NOTE: We're not using existing methods like ds.whereFilterHostsByIdentifier,
|
||||
// ds.HostIDsByIdentifier, ds.HostLiteByIdentifier because those methods are poorly
|
||||
// optimized for the indexes we currently have on the hosts table.
|
||||
// NOTE: We're not using existing methods like ds.HostIDsByIdentifier or
|
||||
// ds.HostLiteByIdentifier because those methods are poorly optimized for
|
||||
// the indexes we currently have on the hosts table.
|
||||
// They filter with disjunctive conditions like `hostname = ? OR uuid = ?` as well as
|
||||
// `? IN(hostname, uuid)`. These existing queries aren't really suited for either composite
|
||||
// indexes or indexes on individual columns, and the optimizer ends up with executions that
|
||||
@@ -252,10 +353,12 @@ SELECT
|
||||
ELSE 'pending'
|
||||
END AS command_status,
|
||||
request_type,
|
||||
nc.name
|
||||
nc.name,
|
||||
h.hostname
|
||||
FROM
|
||||
nano_enrollment_queue nq
|
||||
JOIN nano_commands nc ON nq.command_uuid = nc.command_uuid
|
||||
JOIN hosts h ON h.uuid = nq.id
|
||||
LEFT JOIN nano_command_results ncr ON nq.id = ncr.id
|
||||
AND nc.command_uuid = ncr.command_uuid
|
||||
WHERE
|
||||
@@ -279,11 +382,13 @@ WHERE
|
||||
'101' AS status,
|
||||
'pending' AS command_status,
|
||||
wc.target_loc_uri AS request_type,
|
||||
NULL AS name
|
||||
NULL AS name,
|
||||
h.hostname
|
||||
FROM
|
||||
windows_mdm_command_queue wq
|
||||
JOIN mdm_windows_enrollments mwe ON mwe.id = wq.enrollment_id
|
||||
JOIN windows_mdm_commands wc ON wc.command_uuid = wq.command_uuid
|
||||
JOIN hosts h ON h.uuid = mwe.host_uuid
|
||||
|
||||
WHERE
|
||||
mwe.host_uuid IN (?)
|
||||
@@ -316,11 +421,13 @@ WHERE
|
||||
) >= 400 THEN 'failed'
|
||||
END AS command_status,
|
||||
wc.target_loc_uri AS request_type,
|
||||
NULL AS name
|
||||
NULL AS name,
|
||||
h.hostname
|
||||
FROM
|
||||
windows_mdm_command_results wcr
|
||||
JOIN mdm_windows_enrollments mwe ON mwe.id = wcr.enrollment_id
|
||||
JOIN windows_mdm_commands wc ON wc.command_uuid = wcr.command_uuid
|
||||
JOIN hosts h ON h.uuid = mwe.host_uuid
|
||||
WHERE
|
||||
mwe.host_uuid IN (?)
|
||||
|
||||
@@ -346,19 +453,25 @@ WHERE
|
||||
|
||||
var listStmt, countStmt string
|
||||
var params []any
|
||||
// Wrap in `SELECT * FROM (...) u WHERE TRUE` so the cursor and ORDER BY
|
||||
// predicates resolve against the unambiguous `u` projection — the inner
|
||||
// branches join multiple tables that all expose `command_uuid` / `updated_at`.
|
||||
// `WHERE TRUE` is required because the cursor helper picks AND vs WHERE by
|
||||
// substring-matching "where", picks AND from the inner branches, and would
|
||||
// otherwise emit a dangling `AND`. See https://github.com/fleetdm/fleet/issues/44422.
|
||||
switch {
|
||||
case len(appleUUIDs) > 0 && len(winUUIDs) > 0:
|
||||
listStmt = fmt.Sprintf(`SELECT * FROM ((%s) UNION ALL (%s)) u`,
|
||||
listStmt = fmt.Sprintf(`SELECT * FROM ((%s) UNION ALL (%s)) u WHERE TRUE`,
|
||||
appleStmt, winStmt)
|
||||
countStmt = fmt.Sprintf(`SELECT COUNT(1) FROM ((%s) UNION ALL (%s)) u`, appleStmt, winStmt)
|
||||
params = append(params, appleParams...)
|
||||
params = append(params, winParams...)
|
||||
case len(appleUUIDs) > 0:
|
||||
listStmt = appleStmt
|
||||
listStmt = `SELECT * FROM (` + appleStmt + `) u WHERE TRUE`
|
||||
countStmt = `SELECT COUNT(1) FROM (` + appleStmt + `) u`
|
||||
params = appleParams
|
||||
case len(winUUIDs) > 0:
|
||||
listStmt = winStmt
|
||||
listStmt = `SELECT * FROM (` + winStmt + `) u WHERE TRUE`
|
||||
countStmt = `SELECT COUNT(1) FROM (` + winStmt + `) u`
|
||||
params = winParams
|
||||
}
|
||||
@@ -378,12 +491,15 @@ WHERE
|
||||
// if listOpts.OrderDirection == "" {
|
||||
// listOpts.OrderDirection = fleet.OrderDescending
|
||||
// }
|
||||
if listOpts.PerPage == 0 {
|
||||
listOpts.PerPage = 10
|
||||
}
|
||||
listStmt, params, err = appendListOptionsWithCursorToSQLSecure(listStmt, params, &listOpts.ListOptions, mdmCommandsAllowedOrderKeys)
|
||||
// Snapshot the params before the cursor helper appends to them. countStmt
|
||||
// has no cursor placeholder, so it must run with the pre-cursor args.
|
||||
countParams := slices.Clone(params)
|
||||
// Validate order_key against the closed allowlist before it reaches
|
||||
// ORDER BY (defense against SQL injection / information disclosure
|
||||
// via arbitrary column references).
|
||||
listStmt, params, err = appendListOptionsWithCursorToSQLSecure(listStmt, params, &listOpts.ListOptions, mdmCommandsOrderAllowlist)
|
||||
if err != nil {
|
||||
return nil, nil, nil, ctxerr.Wrap(ctx, err, "list commands")
|
||||
return nil, nil, nil, ctxerr.Wrap(ctx, err, "list commands pagination")
|
||||
}
|
||||
|
||||
var results []*fleet.MDMCommand
|
||||
@@ -394,15 +510,15 @@ WHERE
|
||||
var total *int64
|
||||
if len(listOpts.Filters.CommandStatuses) == 1 && listOpts.Filters.CommandStatuses[0] == fleet.MDMCommandStatusFilterPending {
|
||||
// Only get count if we only filter by pending
|
||||
if err := sqlx.GetContext(ctx, ds.reader(ctx), &total, countStmt, params...); err != nil {
|
||||
if err := sqlx.GetContext(ctx, ds.reader(ctx), &total, countStmt, countParams...); err != nil {
|
||||
return nil, nil, nil, ctxerr.Wrap(ctx, err, "count commands")
|
||||
}
|
||||
}
|
||||
|
||||
// Add hostname and team info to the results based on the host UUIDs.
|
||||
// Hostname is now projected in SQL on both branches; only team_id
|
||||
// still needs to be merged from the prefetched host lookup.
|
||||
for i := range results {
|
||||
if host, ok := byUUID[results[i].HostUUID]; ok {
|
||||
results[i].Hostname = host.Hostname
|
||||
results[i].TeamID = host.TeamID
|
||||
}
|
||||
}
|
||||
@@ -451,8 +567,7 @@ func addAppleCommandStatusFilter(stmt string, filter *fleet.MDMCommandFilters, p
|
||||
}
|
||||
|
||||
func (ds *Datastore) getMDMCommand(ctx context.Context, q sqlx.QueryerContext, cmdUUID string) (*fleet.MDMCommand, error) {
|
||||
stmt, _ := getCombinedMDMCommandsQuery(ds, "")
|
||||
stmt += "command_uuid = ?"
|
||||
stmt := getCombinedMDMCommandsQuery() + "command_uuid = ?"
|
||||
|
||||
var cmd fleet.MDMCommand
|
||||
if err := sqlx.GetContext(ctx, q, &cmd, stmt, cmdUUID); err != nil {
|
||||
|
||||
@@ -19,6 +19,7 @@ import (
|
||||
microsoft_mdm "github.com/fleetdm/fleet/v4/server/mdm/microsoft"
|
||||
"github.com/fleetdm/fleet/v4/server/mdm/nanomdm/mdm"
|
||||
"github.com/fleetdm/fleet/v4/server/mdm/nanomdm/service/certauth"
|
||||
common_mysql "github.com/fleetdm/fleet/v4/server/platform/mysql"
|
||||
"github.com/fleetdm/fleet/v4/server/ptr"
|
||||
"github.com/fleetdm/fleet/v4/server/test"
|
||||
"github.com/google/uuid"
|
||||
@@ -39,6 +40,8 @@ func TestMDMShared(t *testing.T) {
|
||||
{"TestListMDMCommandsWithTeamFilter", testListMDMCommandsWithTeamFilter},
|
||||
{"TestListMDMCommandsOrderKeys", testListMDMCommandsOrderKeys},
|
||||
{"TestListMDMAppleCommandsOrderKeys", testListMDMAppleCommandsOrderKeys},
|
||||
{"TestListMDMCommandsRequiresPerPage", testListMDMCommandsRequiresPerPage},
|
||||
{"TestListMDMCommandsPagination", testListMDMCommandsPagination},
|
||||
{"TestBatchSetMDMProfiles", testBatchSetMDMProfiles},
|
||||
{"TestListMDMConfigProfiles", testListMDMConfigProfiles},
|
||||
{"TestBulkSetPendingMDMHostProfiles", testBulkSetPendingMDMHostProfiles},
|
||||
@@ -76,7 +79,7 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
|
||||
ctx := context.Background()
|
||||
|
||||
// no commands or devices enrolled => no results
|
||||
cmds, _, _, err := ds.ListMDMCommands(ctx, fleet.TeamFilter{}, &fleet.MDMCommandListOptions{})
|
||||
cmds, _, _, err := ds.ListMDMCommands(ctx, fleet.TeamFilter{}, &fleet.MDMCommandListOptions{ListOptions: fleet.ListOptions{PerPage: 100}})
|
||||
require.NoError(t, err)
|
||||
require.Empty(t, cmds)
|
||||
|
||||
@@ -134,7 +137,7 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
|
||||
cmds, _, _, err = ds.ListMDMCommands(
|
||||
ctx,
|
||||
fleet.TeamFilter{User: test.UserAdmin},
|
||||
&fleet.MDMCommandListOptions{},
|
||||
&fleet.MDMCommandListOptions{ListOptions: fleet.ListOptions{PerPage: 100}},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
require.Empty(t, cmds)
|
||||
@@ -152,7 +155,7 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
|
||||
cmds, total, _, err := ds.ListMDMCommands(
|
||||
ctx,
|
||||
fleet.TeamFilter{User: test.UserAdmin},
|
||||
&fleet.MDMCommandListOptions{},
|
||||
&fleet.MDMCommandListOptions{ListOptions: fleet.ListOptions{PerPage: 100}},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, cmds, 1)
|
||||
@@ -172,7 +175,7 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
|
||||
ctx,
|
||||
fleet.TeamFilter{User: test.UserAdmin},
|
||||
&fleet.MDMCommandListOptions{
|
||||
ListOptions: fleet.ListOptions{OrderKey: "hostname"},
|
||||
ListOptions: fleet.ListOptions{OrderKey: "hostname", PerPage: 100},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.Len(t, cmds, 2)
|
||||
@@ -211,7 +214,7 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
|
||||
ctx,
|
||||
fleet.TeamFilter{User: test.UserAdmin},
|
||||
&fleet.MDMCommandListOptions{
|
||||
ListOptions: fleet.ListOptions{OrderKey: "hostname"},
|
||||
ListOptions: fleet.ListOptions{OrderKey: "hostname", PerPage: 100},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.Len(t, cmds, 2)
|
||||
@@ -255,6 +258,7 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
|
||||
ctx,
|
||||
fleet.TeamFilter{User: test.UserAdmin},
|
||||
&fleet.MDMCommandListOptions{
|
||||
ListOptions: fleet.ListOptions{PerPage: 100},
|
||||
Filters: fleet.MDMCommandFilters{
|
||||
HostIdentifier: "non-existent",
|
||||
},
|
||||
@@ -268,6 +272,7 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
|
||||
ctx,
|
||||
fleet.TeamFilter{User: test.UserAdmin},
|
||||
&fleet.MDMCommandListOptions{
|
||||
ListOptions: fleet.ListOptions{PerPage: 100},
|
||||
Filters: fleet.MDMCommandFilters{
|
||||
RequestType: "non-existent",
|
||||
},
|
||||
@@ -336,6 +341,7 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
|
||||
ctx,
|
||||
fleet.TeamFilter{User: test.UserAdmin},
|
||||
&fleet.MDMCommandListOptions{
|
||||
ListOptions: fleet.ListOptions{PerPage: 100},
|
||||
Filters: fleet.MDMCommandFilters{
|
||||
HostIdentifier: tc.identifier,
|
||||
CommandStatuses: commandStatuses,
|
||||
@@ -383,10 +389,10 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
|
||||
ctx,
|
||||
fleet.TeamFilter{User: test.UserAdmin},
|
||||
&fleet.MDMCommandListOptions{
|
||||
ListOptions: fleet.ListOptions{OrderKey: "hostname", OrderDirection: fleet.OrderAscending, PerPage: 100},
|
||||
Filters: fleet.MDMCommandFilters{
|
||||
RequestType: "InstallProfile",
|
||||
},
|
||||
ListOptions: fleet.ListOptions{OrderKey: "hostname", OrderDirection: fleet.OrderAscending},
|
||||
},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
@@ -399,6 +405,7 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
|
||||
ctx,
|
||||
fleet.TeamFilter{User: test.UserAdmin},
|
||||
&fleet.MDMCommandListOptions{
|
||||
ListOptions: fleet.ListOptions{PerPage: 100},
|
||||
Filters: fleet.MDMCommandFilters{
|
||||
RequestType: "InstallProfile",
|
||||
HostIdentifier: macH.UUID,
|
||||
@@ -414,6 +421,7 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
|
||||
ctx,
|
||||
fleet.TeamFilter{User: test.UserAdmin},
|
||||
&fleet.MDMCommandListOptions{
|
||||
ListOptions: fleet.ListOptions{PerPage: 100},
|
||||
Filters: fleet.MDMCommandFilters{
|
||||
HostIdentifier: "123456",
|
||||
CommandStatuses: []fleet.MDMCommandStatusFilter{fleet.MDMCommandStatusFilterPending},
|
||||
@@ -455,6 +463,7 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
|
||||
ctx,
|
||||
fleet.TeamFilter{User: test.UserAdmin},
|
||||
&fleet.MDMCommandListOptions{
|
||||
ListOptions: fleet.ListOptions{PerPage: 100},
|
||||
Filters: fleet.MDMCommandFilters{
|
||||
HostIdentifier: macH.UUID,
|
||||
CommandStatuses: []fleet.MDMCommandStatusFilter{fleet.MDMCommandStatusFilterRan, fleet.MDMCommandStatusFilterFailed},
|
||||
@@ -486,6 +495,213 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, true, meta.HasNextResults)
|
||||
require.Equal(t, false, meta.HasPreviousResults)
|
||||
|
||||
// Cursor pagination on the host-scoped path. Regression test for
|
||||
// https://github.com/fleetdm/fleet/issues/44422:
|
||||
// "Error 1052 (23000): Column '<col>' in where clause is ambiguous".
|
||||
// Covers all three Path A SQL shapes: Apple-only, Windows-only, and
|
||||
// multi-platform (host_identifier resolves to both an Apple and a
|
||||
// Windows host via the shared hostname "test-host").
|
||||
t.Run("cursor_pagination_44422", func(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
hostIdentifier string
|
||||
wantPlatforms []string
|
||||
}{
|
||||
{"apple_only", macH.UUID, []string{"darwin"}},
|
||||
{"multi_platform", "test-host", []string{"darwin", "windows"}},
|
||||
{"windows_only", windowsH.UUID, []string{"windows"}},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
filters := fleet.MDMCommandFilters{HostIdentifier: tc.hostIdentifier}
|
||||
|
||||
// Sanity: confirm the dispatch reaches the expected branch(es)
|
||||
// and there is data to paginate over.
|
||||
all, _, _, err := ds.ListMDMCommands(
|
||||
ctx,
|
||||
fleet.TeamFilter{User: test.UserAdmin},
|
||||
&fleet.MDMCommandListOptions{
|
||||
ListOptions: fleet.ListOptions{PerPage: 100},
|
||||
Filters: filters,
|
||||
},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, all)
|
||||
platforms := map[string]struct{}{}
|
||||
for _, c := range all {
|
||||
switch c.HostUUID {
|
||||
case macH.UUID:
|
||||
platforms["darwin"] = struct{}{}
|
||||
case windowsH.UUID:
|
||||
platforms["windows"] = struct{}{}
|
||||
}
|
||||
}
|
||||
for _, p := range tc.wantPlatforms {
|
||||
_, ok := platforms[p]
|
||||
require.True(t, ok, "expected commands from platform %s", p)
|
||||
}
|
||||
|
||||
// Cursor on updated_at — `updated_at` is exposed by multiple
|
||||
// inner-FROM tables on every Path A branch, so this would fail
|
||||
// with "Column 'updated_at' ... is ambiguous" without the wrap.
|
||||
page1, _, _, err := ds.ListMDMCommands(
|
||||
ctx,
|
||||
fleet.TeamFilter{User: test.UserAdmin},
|
||||
&fleet.MDMCommandListOptions{
|
||||
ListOptions: fleet.ListOptions{
|
||||
PerPage: 1,
|
||||
OrderKey: "updated_at",
|
||||
OrderDirection: fleet.OrderDescending,
|
||||
},
|
||||
Filters: filters,
|
||||
},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, page1, 1)
|
||||
|
||||
page2, _, _, err := ds.ListMDMCommands(
|
||||
ctx,
|
||||
fleet.TeamFilter{User: test.UserAdmin},
|
||||
&fleet.MDMCommandListOptions{
|
||||
ListOptions: fleet.ListOptions{
|
||||
PerPage: 1,
|
||||
OrderKey: "updated_at",
|
||||
OrderDirection: fleet.OrderDescending,
|
||||
After: page1[0].UpdatedAt.Format(time.RFC3339Nano),
|
||||
},
|
||||
Filters: filters,
|
||||
},
|
||||
)
|
||||
// require.NoError is the regression guard. The cursor predicate
|
||||
// is strict (`updated_at < ?`, no tiebreaker), so page2 may be
|
||||
// empty when adjacent commands share an updated_at second — that
|
||||
// is expected, not a regression. Only assert distinctness when
|
||||
// a row did come back.
|
||||
require.NoError(t, err)
|
||||
if len(page2) > 0 {
|
||||
require.NotEqual(t, page1[0].CommandUUID, page2[0].CommandUUID)
|
||||
}
|
||||
|
||||
// Cursor on command_uuid — separate ambiguity manifestation;
|
||||
// `command_uuid` is the join column on every inner table.
|
||||
first, _, _, err := ds.ListMDMCommands(
|
||||
ctx,
|
||||
fleet.TeamFilter{User: test.UserAdmin},
|
||||
&fleet.MDMCommandListOptions{
|
||||
ListOptions: fleet.ListOptions{
|
||||
PerPage: 1,
|
||||
OrderKey: "command_uuid",
|
||||
OrderDirection: fleet.OrderAscending,
|
||||
},
|
||||
Filters: filters,
|
||||
},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, first, 1)
|
||||
|
||||
next, _, _, err := ds.ListMDMCommands(
|
||||
ctx,
|
||||
fleet.TeamFilter{User: test.UserAdmin},
|
||||
&fleet.MDMCommandListOptions{
|
||||
ListOptions: fleet.ListOptions{
|
||||
PerPage: 1,
|
||||
OrderKey: "command_uuid",
|
||||
OrderDirection: fleet.OrderAscending,
|
||||
After: first[0].CommandUUID,
|
||||
},
|
||||
Filters: filters,
|
||||
},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, next)
|
||||
require.NotEqual(t, first[0].CommandUUID, next[0].CommandUUID)
|
||||
|
||||
// Cursor on hostname. All hosts in this test share hostname
|
||||
// "test-host", and the helper's cursor predicate is a strict
|
||||
// `hostname > 'test-host'` (no tiebreaker), so the result is
|
||||
// expected to be empty. The bug would surface as a SQL error
|
||||
// before the empty check.
|
||||
afterHostname, _, _, err := ds.ListMDMCommands(
|
||||
ctx,
|
||||
fleet.TeamFilter{User: test.UserAdmin},
|
||||
&fleet.MDMCommandListOptions{
|
||||
ListOptions: fleet.ListOptions{
|
||||
PerPage: 5,
|
||||
OrderKey: "hostname",
|
||||
OrderDirection: fleet.OrderAscending,
|
||||
After: "test-host",
|
||||
},
|
||||
Filters: filters,
|
||||
},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
require.Empty(t, afterHostname)
|
||||
|
||||
// Pending-only count + cursor
|
||||
if tc.name == "apple_only" {
|
||||
pendingFilters := fleet.MDMCommandFilters{
|
||||
HostIdentifier: tc.hostIdentifier,
|
||||
CommandStatuses: []fleet.MDMCommandStatusFilter{fleet.MDMCommandStatusFilterPending},
|
||||
}
|
||||
// Establish the unfiltered pending baseline so the post-cursor
|
||||
// assertions don't depend on UUID lexicographic ordering.
|
||||
allPending, totalPending, _, err := ds.ListMDMCommands(
|
||||
ctx,
|
||||
fleet.TeamFilter{User: test.UserAdmin},
|
||||
&fleet.MDMCommandListOptions{
|
||||
ListOptions: fleet.ListOptions{
|
||||
PerPage: 100,
|
||||
OrderKey: "command_uuid",
|
||||
OrderDirection: fleet.OrderAscending,
|
||||
IncludeMetadata: true,
|
||||
},
|
||||
Filters: pendingFilters,
|
||||
},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, totalPending)
|
||||
require.Equal(t, int64(len(allPending)), *totalPending)
|
||||
require.Greater(t, len(allPending), 1, "test setup expects multiple pending Apple commands")
|
||||
|
||||
// Drive the cursor from a known pending UUID so the post-cursor
|
||||
// expectation is deterministic.
|
||||
cursorUUID := allPending[0].CommandUUID
|
||||
expectedAfter := allPending[1:]
|
||||
|
||||
afterPending, totalAfter, _, err := ds.ListMDMCommands(
|
||||
ctx,
|
||||
fleet.TeamFilter{User: test.UserAdmin},
|
||||
&fleet.MDMCommandListOptions{
|
||||
ListOptions: fleet.ListOptions{
|
||||
PerPage: 100,
|
||||
OrderKey: "command_uuid",
|
||||
OrderDirection: fleet.OrderAscending,
|
||||
After: cursorUUID,
|
||||
IncludeMetadata: true,
|
||||
},
|
||||
Filters: pendingFilters,
|
||||
},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
// total comes from countStmt which has no cursor, so it
|
||||
// must report the full pending count regardless of After.
|
||||
require.NotNil(t, totalAfter)
|
||||
require.Equal(t, *totalPending, *totalAfter)
|
||||
require.Len(t, afterPending, len(expectedAfter))
|
||||
gotUUIDs := make([]string, len(afterPending))
|
||||
for i, c := range afterPending {
|
||||
gotUUIDs[i] = c.CommandUUID
|
||||
}
|
||||
expectedUUIDs := make([]string, len(expectedAfter))
|
||||
for i, c := range expectedAfter {
|
||||
expectedUUIDs[i] = c.CommandUUID
|
||||
}
|
||||
require.Equal(t, expectedUUIDs, gotUUIDs)
|
||||
}
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
}
|
||||
|
||||
// testListMDMCommandsWithTeamFilter tests listing MDM commands with team filters
|
||||
@@ -528,7 +744,7 @@ func testListMDMCommandsWithTeamFilter(t *testing.T, ds *Datastore) {
|
||||
cmds, _, _, err := ds.ListMDMCommands(
|
||||
ctx,
|
||||
fleet.TeamFilter{User: teamUser},
|
||||
&fleet.MDMCommandListOptions{},
|
||||
&fleet.MDMCommandListOptions{ListOptions: fleet.ListOptions{PerPage: 100}},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, cmds, 1)
|
||||
@@ -538,7 +754,7 @@ func testListMDMCommandsWithTeamFilter(t *testing.T, ds *Datastore) {
|
||||
cmds, _, _, err = ds.ListMDMCommands(
|
||||
ctx,
|
||||
fleet.TeamFilter{User: teamUser, TeamID: &team.ID},
|
||||
&fleet.MDMCommandListOptions{},
|
||||
&fleet.MDMCommandListOptions{ListOptions: fleet.ListOptions{PerPage: 100}},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, cmds, 1)
|
||||
@@ -566,7 +782,7 @@ func testListMDMCommandsWithTeamFilter(t *testing.T, ds *Datastore) {
|
||||
cmds, _, _, err = ds.ListMDMCommands(
|
||||
ctx,
|
||||
fleet.TeamFilter{User: teamUser},
|
||||
&fleet.MDMCommandListOptions{},
|
||||
&fleet.MDMCommandListOptions{ListOptions: fleet.ListOptions{PerPage: 100}},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, cmds, 1)
|
||||
@@ -577,7 +793,7 @@ func testListMDMCommandsWithTeamFilter(t *testing.T, ds *Datastore) {
|
||||
cmds, _, _, err = ds.ListMDMCommands(
|
||||
ctx,
|
||||
fleet.TeamFilter{User: adminUser},
|
||||
&fleet.MDMCommandListOptions{},
|
||||
&fleet.MDMCommandListOptions{ListOptions: fleet.ListOptions{PerPage: 100}},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, cmds, 2)
|
||||
@@ -588,6 +804,178 @@ func testListMDMCommandsWithTeamFilter(t *testing.T, ds *Datastore) {
|
||||
require.ElementsMatch(t, []string{teamCmdUUID, globalCmdUUID}, got)
|
||||
}
|
||||
|
||||
func testListMDMCommandsRequiresPerPage(t *testing.T, ds *Datastore) {
|
||||
ctx := t.Context()
|
||||
|
||||
_, _, _, err := ds.ListMDMCommands(ctx, fleet.TeamFilter{User: test.UserAdmin}, nil)
|
||||
require.Error(t, err)
|
||||
require.Contains(t, err.Error(), "PerPage")
|
||||
|
||||
_, _, _, err = ds.ListMDMCommands(ctx, fleet.TeamFilter{User: test.UserAdmin}, &fleet.MDMCommandListOptions{})
|
||||
require.Error(t, err)
|
||||
require.Contains(t, err.Error(), "PerPage")
|
||||
}
|
||||
|
||||
// testListMDMCommandsPagination exercises the all-hosts per-branch
|
||||
// pagination across both Apple and Windows branches. The path inflates the
|
||||
// inner LIMIT to per_page*page+per_page so that page N is correct even if all
|
||||
// matching rows came from a single branch, and the outer wrap clears
|
||||
// After/Page when cursor pagination is used.
|
||||
func testListMDMCommandsPagination(t *testing.T, ds *Datastore) {
|
||||
ctx := t.Context()
|
||||
|
||||
// Enroll one Windows host and one macOS host.
|
||||
winHost := test.NewHost(t, ds, "paginate-win", "1.2.3.4", "paginate-node-win", uuid.NewString(), time.Now(), test.WithPlatform("windows"))
|
||||
winDeviceID := windowsEnroll(t, ds, winHost)
|
||||
winEnrollment, err := ds.MDMWindowsGetEnrolledDeviceWithDeviceID(ctx, winDeviceID)
|
||||
require.NoError(t, err)
|
||||
|
||||
macHost := test.NewHost(t, ds, "paginate-mac", "1.2.3.5", "paginate-node-mac", uuid.NewString(), time.Now())
|
||||
nanoEnroll(t, ds, macHost, false)
|
||||
|
||||
// Insert 3 Apple commands and 3 Windows commands so both branches contribute.
|
||||
commander, _ := createMDMAppleCommanderAndStorage(t, ds)
|
||||
const totalApple = 3
|
||||
const totalWin = 3
|
||||
allUUIDs := make([]string, 0, totalApple+totalWin)
|
||||
for range totalApple {
|
||||
cmdUUID := uuid.NewString()
|
||||
raw := createRawAppleCmd("ProfileList", cmdUUID)
|
||||
require.NoError(t, commander.EnqueueCommand(ctx, []string{macHost.UUID}, raw))
|
||||
allUUIDs = append(allUUIDs, cmdUUID)
|
||||
}
|
||||
winUUIDs := make([]string, 0, totalWin)
|
||||
for range totalWin {
|
||||
cmdUUID := uuid.NewString()
|
||||
require.NoError(t, ds.MDMWindowsInsertCommandForHosts(ctx, []string{winDeviceID}, &fleet.MDMWindowsCommand{
|
||||
CommandUUID: cmdUUID,
|
||||
RawCommand: []byte("<Exec></Exec>"),
|
||||
TargetLocURI: "./test/uri",
|
||||
}))
|
||||
winUUIDs = append(winUUIDs, cmdUUID)
|
||||
allUUIDs = append(allUUIDs, cmdUUID)
|
||||
}
|
||||
|
||||
// Mark one Windows command as responded so the results-backed branch of the
|
||||
// internal Windows UNION ALL is exercised. The dedupe NOT EXISTS clause
|
||||
// must still keep the command from appearing twice across the pagination.
|
||||
respondedWinUUID := winUUIDs[0]
|
||||
_, err = ds.MDMWindowsSaveResponse(ctx, winEnrollment, fleet.EnrichedSyncML{
|
||||
SyncML: &fleet.SyncML{Raw: []byte("<xml></xml>")},
|
||||
CmdRefUUIDToStatus: map[string]fleet.SyncMLCmd{
|
||||
respondedWinUUID: {Data: ptr.String("200")},
|
||||
},
|
||||
CmdRefUUIDs: []string{respondedWinUUID},
|
||||
}, []string{})
|
||||
require.NoError(t, err)
|
||||
|
||||
sort.Strings(allUUIDs)
|
||||
totalCount := len(allUUIDs)
|
||||
|
||||
t.Run("page-based across branches", func(t *testing.T) {
|
||||
// PerPage=2 across 6 commands => 3 pages of 2 each.
|
||||
const perPage = 2
|
||||
seen := make(map[string]bool)
|
||||
for page := uint(0); page*perPage < uint(totalCount); page++ { //nolint:gosec
|
||||
cmds, _, meta, err := ds.ListMDMCommands(
|
||||
ctx,
|
||||
fleet.TeamFilter{User: test.UserAdmin},
|
||||
&fleet.MDMCommandListOptions{
|
||||
ListOptions: fleet.ListOptions{
|
||||
OrderKey: "command_uuid",
|
||||
OrderDirection: fleet.OrderAscending,
|
||||
Page: page,
|
||||
PerPage: perPage,
|
||||
IncludeMetadata: true,
|
||||
},
|
||||
},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
require.LessOrEqual(t, len(cmds), perPage)
|
||||
|
||||
start := int(page * perPage)
|
||||
end := min(start+perPage, totalCount)
|
||||
expected := allUUIDs[start:end]
|
||||
got := make([]string, 0, len(cmds))
|
||||
for _, c := range cmds {
|
||||
require.False(t, seen[c.CommandUUID], "duplicate command UUID across pages: %s", c.CommandUUID)
|
||||
seen[c.CommandUUID] = true
|
||||
got = append(got, c.CommandUUID)
|
||||
}
|
||||
require.Equal(t, expected, got, "page %d", page)
|
||||
|
||||
require.Equal(t, page > 0, meta.HasPreviousResults, "page %d HasPreviousResults", page)
|
||||
require.Equal(t, end < totalCount, meta.HasNextResults, "page %d HasNextResults", page)
|
||||
}
|
||||
require.Len(t, seen, totalCount)
|
||||
})
|
||||
|
||||
t.Run("cursor walks all rows without overlap", func(t *testing.T) {
|
||||
const perPage = 2
|
||||
seen := make(map[string]bool)
|
||||
var after string
|
||||
for {
|
||||
cmds, _, _, err := ds.ListMDMCommands(
|
||||
ctx,
|
||||
fleet.TeamFilter{User: test.UserAdmin},
|
||||
&fleet.MDMCommandListOptions{
|
||||
ListOptions: fleet.ListOptions{
|
||||
OrderKey: "command_uuid",
|
||||
OrderDirection: fleet.OrderAscending,
|
||||
PerPage: perPage,
|
||||
After: after,
|
||||
IncludeMetadata: true,
|
||||
},
|
||||
},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
if len(cmds) == 0 {
|
||||
break
|
||||
}
|
||||
for _, c := range cmds {
|
||||
require.False(t, seen[c.CommandUUID], "duplicate command UUID across cursor pages: %s", c.CommandUUID)
|
||||
seen[c.CommandUUID] = true
|
||||
}
|
||||
after = cmds[len(cmds)-1].CommandUUID
|
||||
if len(cmds) < perPage {
|
||||
break
|
||||
}
|
||||
}
|
||||
require.Len(t, seen, totalCount)
|
||||
})
|
||||
|
||||
t.Run("outer clears Page when After is set", func(t *testing.T) {
|
||||
// Without the outer-wrap clearing of Page, Page=99 with PerPage=2 would
|
||||
// add OFFSET 198 on top of the cursor filter and return zero rows even
|
||||
// though plenty of rows follow allUUIDs[0]. With the clearing, the call
|
||||
// must return the page-sized slice immediately after the cursor.
|
||||
cmds, _, _, err := ds.ListMDMCommands(
|
||||
ctx,
|
||||
fleet.TeamFilter{User: test.UserAdmin},
|
||||
&fleet.MDMCommandListOptions{
|
||||
ListOptions: fleet.ListOptions{
|
||||
OrderKey: "command_uuid",
|
||||
OrderDirection: fleet.OrderAscending,
|
||||
PerPage: 2,
|
||||
Page: 99,
|
||||
After: allUUIDs[0],
|
||||
IncludeMetadata: true,
|
||||
},
|
||||
},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, allUUIDs[1:3], extractCommandUUIDs(cmds))
|
||||
})
|
||||
}
|
||||
|
||||
func extractCommandUUIDs(cmds []*fleet.MDMCommand) []string {
|
||||
out := make([]string, 0, len(cmds))
|
||||
for _, c := range cmds {
|
||||
out = append(out, c.CommandUUID)
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
func testListMDMCommandsOrderKeys(t *testing.T, ds *Datastore) {
|
||||
ctx := t.Context()
|
||||
|
||||
@@ -627,10 +1015,12 @@ func testListMDMCommandsOrderKeys(t *testing.T, ds *Datastore) {
|
||||
ctx,
|
||||
fleet.TeamFilter{User: test.UserAdmin},
|
||||
&fleet.MDMCommandListOptions{
|
||||
ListOptions: fleet.ListOptions{OrderKey: "not_a_real_column"},
|
||||
ListOptions: fleet.ListOptions{OrderKey: "not_a_real_column", PerPage: 5},
|
||||
},
|
||||
)
|
||||
require.Error(t, err)
|
||||
var invalidKeyErr common_mysql.InvalidOrderKeyError
|
||||
require.ErrorAs(t, err, &invalidKeyErr)
|
||||
})
|
||||
|
||||
// the host-identifier branch uses a separate query; confirm it shares the allowlist
|
||||
@@ -639,11 +1029,13 @@ func testListMDMCommandsOrderKeys(t *testing.T, ds *Datastore) {
|
||||
ctx,
|
||||
fleet.TeamFilter{User: test.UserAdmin},
|
||||
&fleet.MDMCommandListOptions{
|
||||
ListOptions: fleet.ListOptions{OrderKey: "not_a_real_column"},
|
||||
ListOptions: fleet.ListOptions{OrderKey: "not_a_real_column", PerPage: 5},
|
||||
Filters: fleet.MDMCommandFilters{HostIdentifier: macH.UUID},
|
||||
},
|
||||
)
|
||||
require.Error(t, err)
|
||||
var invalidKeyErr common_mysql.InvalidOrderKeyError
|
||||
require.ErrorAs(t, err, &invalidKeyErr)
|
||||
})
|
||||
|
||||
t.Run("after_pagination_with_allowed_key", func(t *testing.T) {
|
||||
@@ -9566,7 +9958,7 @@ func testDeleteMDMProfilesCancelsInstalls(t *testing.T, ds *Datastore) {
|
||||
cmds, _, _, err := ds.ListMDMCommands(ctx, fleet.TeamFilter{
|
||||
User: test.UserAdmin,
|
||||
IncludeObserver: true,
|
||||
}, &fleet.MDMCommandListOptions{Filters: fleet.MDMCommandFilters{HostIdentifier: host1.UUID}})
|
||||
}, &fleet.MDMCommandListOptions{ListOptions: fleet.ListOptions{PerPage: 100}, Filters: fleet.MDMCommandFilters{HostIdentifier: host1.UUID}})
|
||||
require.NoError(t, err)
|
||||
require.Len(t, cmds, 0)
|
||||
|
||||
@@ -9832,7 +10224,7 @@ func testEnqueueCommandWithName(t *testing.T, ds *Datastore) {
|
||||
require.Equal(t, "Test Profile Name", storedName.String)
|
||||
|
||||
// Also verify via ListMDMCommands
|
||||
cmds, _, _, err := ds.ListMDMCommands(ctx, fleet.TeamFilter{User: test.UserAdmin}, &fleet.MDMCommandListOptions{})
|
||||
cmds, _, _, err := ds.ListMDMCommands(ctx, fleet.TeamFilter{User: test.UserAdmin}, &fleet.MDMCommandListOptions{ListOptions: fleet.ListOptions{PerPage: 100}})
|
||||
require.NoError(t, err)
|
||||
require.Len(t, cmds, 1)
|
||||
require.NotNil(t, cmds[0].Name)
|
||||
@@ -9853,7 +10245,7 @@ func testEnqueueCommandWithName(t *testing.T, ds *Datastore) {
|
||||
|
||||
// Verify name is null in the API
|
||||
// Verify ListMDMCommands also exposes nil Name for unnamed commands
|
||||
cmds, _, _, err = ds.ListMDMCommands(ctx, fleet.TeamFilter{User: test.UserAdmin}, &fleet.MDMCommandListOptions{})
|
||||
cmds, _, _, err = ds.ListMDMCommands(ctx, fleet.TeamFilter{User: test.UserAdmin}, &fleet.MDMCommandListOptions{ListOptions: fleet.ListOptions{PerPage: 100}})
|
||||
require.NoError(t, err)
|
||||
require.Len(t, cmds, 2)
|
||||
|
||||
|
||||
@@ -1110,17 +1110,6 @@ func (ds *Datastore) whereOmitIDs(colName string, omit []uint) string {
|
||||
return fmt.Sprintf("%s NOT IN (%s)", colName, strings.Join(idStrs, ","))
|
||||
}
|
||||
|
||||
func (ds *Datastore) whereFilterHostsByIdentifier(identifier, stmt string, params []interface{}) (string, []interface{}) {
|
||||
if identifier == "" {
|
||||
return stmt, params
|
||||
}
|
||||
|
||||
stmt += " AND ? IN (h.hostname, h.osquery_host_id, h.node_key, h.uuid, h.hardware_serial)"
|
||||
params = append(params, identifier)
|
||||
|
||||
return stmt, params
|
||||
}
|
||||
|
||||
// registerTLS adds client certificate configuration to the mysql connection.
|
||||
func registerTLS(conf config.MysqlConfig) error {
|
||||
tlsCfg := config.TLS{
|
||||
|
||||
@@ -423,6 +423,18 @@ type MDMCommandListOptions struct {
|
||||
Filters MDMCommandFilters
|
||||
}
|
||||
|
||||
// Pagination bounds for the list-MDM-commands endpoints (GET /api/v1/fleet/commands and GET /api/v1/fleet/mdm/commands).
|
||||
const (
|
||||
// DefaultMDMCommandsPerPage is the per_page value used when none is specified on the request.
|
||||
DefaultMDMCommandsPerPage uint = 10
|
||||
// MaxMDMCommandsPerPage caps per_page so a single request can't scan an unbounded number of command rows.
|
||||
MaxMDMCommandsPerPage uint = 1000
|
||||
// MaxMDMCommandsPage caps the offset (page * per_page) so deep
|
||||
// traversal can't cause a timeout issue. Clients that need to walk the full set
|
||||
// should use cursor pagination via the after query parameter.
|
||||
MaxMDMCommandsPage uint = 100
|
||||
)
|
||||
|
||||
type MDMCommandStatusFilter string
|
||||
|
||||
const (
|
||||
|
||||
@@ -4017,6 +4017,40 @@ func (s *integrationMDMTestSuite) TestListMDMCommands() {
|
||||
res = s.DoRaw("GET", fmt.Sprintf("/api/latest/fleet/mdm/commands?host_identifier=%s&command_status=ran", h.UUID), nil, http.StatusBadRequest)
|
||||
errMsg = extractServerErrorText(res.Body)
|
||||
require.Contains(t, errMsg, `Currently, "command_status" filter is only available for macOS, iOS, and iPadOS hosts.`)
|
||||
|
||||
// per_page above the documented maximum is rejected with a clear message.
|
||||
res = s.DoRaw("GET", "/api/latest/fleet/mdm/commands?per_page=1001", nil, http.StatusBadRequest)
|
||||
errMsg = extractServerErrorText(res.Body)
|
||||
require.Contains(t, errMsg, "Please set a per_page limit of 1000 or less")
|
||||
|
||||
// per_page at the cap is accepted.
|
||||
s.DoRaw("GET", "/api/latest/fleet/mdm/commands?per_page=1000", nil, http.StatusOK)
|
||||
|
||||
// page above the cap is rejected so the inner LIMIT (page*per_page+per_page+1)
|
||||
// stays bounded.
|
||||
res = s.DoRaw("GET", "/api/latest/fleet/mdm/commands?page=101", nil, http.StatusBadRequest)
|
||||
errMsg = extractServerErrorText(res.Body)
|
||||
require.Contains(t, errMsg, "Please set page to 100 or less")
|
||||
|
||||
// page at the cap is accepted.
|
||||
s.DoRaw("GET", "/api/latest/fleet/mdm/commands?page=100", nil, http.StatusOK)
|
||||
|
||||
// order_key not in the allowlist is rejected by the secure list-options
|
||||
// helper (defense against SQL injection via crafted ORDER BY). The
|
||||
// helper's InvalidOrderKeyError implements the validation-error
|
||||
// interface, so the response is a 422. Both the unscoped path and
|
||||
// the host-scoped path enforce the same allowlist.
|
||||
res = s.DoRaw("GET", "/api/latest/fleet/mdm/commands?order_key=team_id", nil, http.StatusUnprocessableEntity)
|
||||
errMsg = extractServerErrorText(res.Body)
|
||||
require.Contains(t, errMsg, "team_id")
|
||||
|
||||
res = s.DoRaw("GET", fmt.Sprintf("/api/latest/fleet/mdm/commands?host_identifier=%s&order_key=team_id", h.UUID), nil, http.StatusUnprocessableEntity)
|
||||
errMsg = extractServerErrorText(res.Body)
|
||||
require.Contains(t, errMsg, "team_id")
|
||||
|
||||
// order_key=hostname is supported on both paths.
|
||||
s.DoRaw("GET", "/api/latest/fleet/mdm/commands?order_key=hostname", nil, http.StatusOK)
|
||||
s.DoRaw("GET", fmt.Sprintf("/api/latest/fleet/mdm/commands?host_identifier=%s&order_key=hostname", h.UUID), nil, http.StatusOK)
|
||||
}
|
||||
|
||||
func (s *integrationMDMTestSuite) TestMDMWindowsCommandResults() {
|
||||
|
||||
@@ -1047,6 +1047,18 @@ func (req listMDMCommandsRequest) DecodeBody(ctx context.Context, r io.Reader, u
|
||||
}
|
||||
}
|
||||
|
||||
if req.ListOptions.PerPage > fleet.MaxMDMCommandsPerPage {
|
||||
return &fleet.BadRequestError{
|
||||
Message: fmt.Sprintf("Request could not be processed. Please set a per_page limit of %d or less.", fleet.MaxMDMCommandsPerPage),
|
||||
}
|
||||
}
|
||||
|
||||
if req.ListOptions.Page > fleet.MaxMDMCommandsPage {
|
||||
return &fleet.BadRequestError{
|
||||
Message: fmt.Sprintf("Request could not be processed. Please set page to %d or less, or use cursor pagination via the after parameter for deep traversal.", fleet.MaxMDMCommandsPage),
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -1061,6 +1073,9 @@ func listMDMCommandsEndpoint(ctx context.Context, request interface{}, svc fleet
|
||||
}
|
||||
}
|
||||
|
||||
if req.ListOptions.PerPage == 0 {
|
||||
req.ListOptions.PerPage = fleet.DefaultMDMCommandsPerPage
|
||||
}
|
||||
req.ListOptions.IncludeMetadata = true
|
||||
|
||||
results, total, meta, err := svc.ListMDMCommands(ctx, &fleet.MDMCommandListOptions{
|
||||
|
||||
@@ -687,6 +687,8 @@ func (svc *Service) processReleaseDeviceForOldFleetd(ctx context.Context, host *
|
||||
User: &fleet.User{GlobalRole: ptr.String(fleet.RoleAdmin)},
|
||||
}
|
||||
acctCmds, _, _, err := svc.ds.ListMDMCommands(ctx, adminTeamFilter, &fleet.MDMCommandListOptions{
|
||||
// PerPage 1: only acctCmds[0] is read below.
|
||||
ListOptions: fleet.ListOptions{PerPage: 1},
|
||||
Filters: fleet.MDMCommandFilters{
|
||||
HostIdentifier: host.UUID,
|
||||
RequestType: "AccountConfiguration",
|
||||
|
||||
Reference in New Issue
Block a user