diff --git a/changes/36682-orphan-mdm-command-refs b/changes/36682-orphan-mdm-command-refs new file mode 100644 index 0000000000..6e564345d6 --- /dev/null +++ b/changes/36682-orphan-mdm-command-refs @@ -0,0 +1 @@ +* Prevented a 500 error on the host details page when an MDM command reference in `host_mdm_actions` pointed to a non-existent command (orphan reference). diff --git a/server/datastore/mysql/apple_mdm_test.go b/server/datastore/mysql/apple_mdm_test.go index 73a217c3d6..69fd94c333 100644 --- a/server/datastore/mysql/apple_mdm_test.go +++ b/server/datastore/mysql/apple_mdm_test.go @@ -26,6 +26,7 @@ import ( "github.com/fleetdm/fleet/v4/server/mdm/nanodep/godep" "github.com/fleetdm/fleet/v4/server/mdm/nanomdm/mdm" "github.com/fleetdm/fleet/v4/server/mdm/nanomdm/push" + "github.com/fleetdm/fleet/v4/server/platform/logging" common_mysql "github.com/fleetdm/fleet/v4/server/platform/mysql" "github.com/fleetdm/fleet/v4/server/ptr" "github.com/fleetdm/fleet/v4/server/test" @@ -105,6 +106,7 @@ func TestMDMApple(t *testing.T) { {"TestDeleteMDMAppleDeclarationWithPendingInstalls", testDeleteMDMAppleDeclarationWithPendingInstalls}, {"TestUpdateNanoMDMUserEnrollmentUsername", testUpdateNanoMDMUserEnrollmentUsername}, {"TestLockUnlockWipeIphone", testLockUnlockWipeIphone}, + {"TestOrphanMDMCommandRef", testOrphanMDMCommandRef}, {"TestGetLatestAppleMDMCommandOfType", testGetLatestAppleMDMCommandOfType}, {"TestSetLockCommandForLostModeCheckin", testSetLockCommandForLostModeCheckin}, {"DeviceLocation", testDeviceLocation}, @@ -5792,6 +5794,110 @@ func testLockUnlockWipeIphone(t *testing.T, ds *Datastore) { checkLockWipeState(t, status, false, false, true, false, false, false) } +func testOrphanMDMCommandRef(t *testing.T, ds *Datastore) { + ctx := t.Context() + const orphanUUID = "orphan-command-uuid" + + oldLogger := ds.logger + t.Cleanup(func() { ds.logger = oldLogger }) + buf := &bytes.Buffer{} + ds.logger = logging.NewSlogLogger(logging.Options{Output: buf, Debug: true}) + + t.Run("darwin orphan lock_ref", func(t *testing.T) { + host, err := ds.NewHost(ctx, &fleet.Host{ + Hostname: "orphan-lock-darwin", + OsqueryHostID: ptr.String("orphan-lock-darwin"), + NodeKey: ptr.String("orphan-lock-darwin"), + UUID: "orphan-lock-darwin-uuid", + Platform: "darwin", + }) + require.NoError(t, err) + + // insert a lock_ref pointing to a non-existent MDM command (orphan reference) + _, err = ds.writer(ctx).ExecContext(ctx, + `INSERT INTO host_mdm_actions (host_id, lock_ref, fleet_platform) VALUES (?, ?, ?)`, + host.ID, orphanUUID, "darwin") + require.NoError(t, err) + + buf.Reset() + // should return no error and appear unlocked (not pending lock, not locked) + status, err := ds.GetHostLockWipeStatus(ctx, host) + require.NoError(t, err) + checkLockWipeState(t, status, true, false, false, false, false, false) + require.Nil(t, status.LockMDMCommand) + require.Contains(t, buf.String(), "orphan lock MDM command reference") + }) + + t.Run("darwin orphan wipe_ref", func(t *testing.T) { + host, err := ds.NewHost(ctx, &fleet.Host{ + Hostname: "orphan-wipe-darwin", + OsqueryHostID: ptr.String("orphan-wipe-darwin"), + NodeKey: ptr.String("orphan-wipe-darwin"), + UUID: "orphan-wipe-darwin-uuid", + Platform: "darwin", + }) + require.NoError(t, err) + + _, err = ds.writer(ctx).ExecContext(ctx, + `INSERT INTO host_mdm_actions (host_id, wipe_ref, fleet_platform) VALUES (?, ?, ?)`, + host.ID, orphanUUID, "darwin") + require.NoError(t, err) + + buf.Reset() + status, err := ds.GetHostLockWipeStatus(ctx, host) + require.NoError(t, err) + checkLockWipeState(t, status, true, false, false, false, false, false) + require.Nil(t, status.WipeMDMCommand) + require.Contains(t, buf.String(), "orphan wipe MDM command reference") + }) + + t.Run("ios orphan lock_ref", func(t *testing.T) { + host, err := ds.NewHost(ctx, &fleet.Host{ + Hostname: "orphan-lock-ios", + OsqueryHostID: ptr.String("orphan-lock-ios"), + NodeKey: ptr.String("orphan-lock-ios"), + UUID: "orphan-lock-ios-uuid", + Platform: "ios", + }) + require.NoError(t, err) + + _, err = ds.writer(ctx).ExecContext(ctx, + `INSERT INTO host_mdm_actions (host_id, lock_ref, fleet_platform) VALUES (?, ?, ?)`, + host.ID, orphanUUID, "ios") + require.NoError(t, err) + + buf.Reset() + status, err := ds.GetHostLockWipeStatus(ctx, host) + require.NoError(t, err) + checkLockWipeState(t, status, true, false, false, false, false, false) + require.Nil(t, status.LockMDMCommand) + require.Contains(t, buf.String(), "orphan lock MDM command reference") + }) + + t.Run("ios orphan unlock_ref", func(t *testing.T) { + host, err := ds.NewHost(ctx, &fleet.Host{ + Hostname: "orphan-unlock-ios", + OsqueryHostID: ptr.String("orphan-unlock-ios"), + NodeKey: ptr.String("orphan-unlock-ios"), + UUID: "orphan-unlock-ios-uuid", + Platform: "ios", + }) + require.NoError(t, err) + + _, err = ds.writer(ctx).ExecContext(ctx, + `INSERT INTO host_mdm_actions (host_id, unlock_ref, fleet_platform) VALUES (?, ?, ?)`, + host.ID, orphanUUID, "ios") + require.NoError(t, err) + + buf.Reset() + status, err := ds.GetHostLockWipeStatus(ctx, host) + require.NoError(t, err) + checkLockWipeState(t, status, true, false, false, false, false, false) + require.Nil(t, status.UnlockMDMCommand) + require.Contains(t, buf.String(), "orphan unlock MDM command reference") + }) +} + func testScreenDEPAssignProfileSerialsForCooldown(t *testing.T, ds *Datastore) { ctx := t.Context() skip, assign, err := ds.ScreenDEPAssignProfileSerialsForCooldown(ctx, []string{}) diff --git a/server/datastore/mysql/mdm.go b/server/datastore/mysql/mdm.go index c578ec05b5..c64b0079fd 100644 --- a/server/datastore/mysql/mdm.go +++ b/server/datastore/mysql/mdm.go @@ -424,6 +424,9 @@ func (ds *Datastore) getMDMCommand(ctx context.Context, q sqlx.QueryerContext, c var cmd fleet.MDMCommand if err := sqlx.GetContext(ctx, q, &cmd, stmt, cmdUUID); err != nil { + if errors.Is(err, sql.ErrNoRows) { + return nil, ctxerr.Wrap(ctx, notFound("MDMCommand").WithName(cmdUUID)) + } return nil, ctxerr.Wrap(ctx, err, "get mdm command by UUID") } return &cmd, nil diff --git a/server/datastore/mysql/scripts.go b/server/datastore/mysql/scripts.go index 1dad0fea9a..677b91f2f1 100644 --- a/server/datastore/mysql/scripts.go +++ b/server/datastore/mysql/scripts.go @@ -1482,9 +1482,12 @@ func (ds *Datastore) GetHostLockWipeStatus(ctx context.Context, host *fleet.Host } else if mdmActions.UnlockRef != nil && hostPlatform != "darwin" { // the unlock reference is an MDM command uuid cmd, cmdRes, err := ds.getHostMDMAppleCommand(ctx, *mdmActions.UnlockRef, host.UUID) - if err != nil { + if err != nil && !fleet.IsNotFound(err) { return nil, ctxerr.Wrap(ctx, err, "get unlock reference") } + if fleet.IsNotFound(err) { + ds.logger.ErrorContext(ctx, "orphan unlock MDM command reference", "host_id", host.ID, "command_uuid", *mdmActions.UnlockRef) + } status.UnlockMDMCommand = cmd status.UnlockMDMCommandResult = cmdRes } @@ -1492,15 +1495,18 @@ func (ds *Datastore) GetHostLockWipeStatus(ctx context.Context, host *fleet.Host if mdmActions.LockRef != nil { // the lock reference is an MDM command cmd, cmdRes, err := ds.getHostMDMAppleCommand(ctx, *mdmActions.LockRef, host.UUID) - if err != nil { + if err != nil && !fleet.IsNotFound(err) { return nil, ctxerr.Wrap(ctx, err, "get lock reference") } + if fleet.IsNotFound(err) { + ds.logger.ErrorContext(ctx, "orphan lock MDM command reference", "host_id", host.ID, "command_uuid", *mdmActions.LockRef) + } status.LockMDMCommand = cmd status.LockMDMCommandResult = cmdRes // for ADE enrolled iDevices, we don't advance to "locked" until we have location data - if hostPlatform == "ios" || hostPlatform == "ipados" { + if status.LockMDMCommand != nil && (hostPlatform == "ios" || hostPlatform == "ipados") { _, err = ds.GetHostLocationData(ctx, host.ID) switch { case fleet.IsNotFound(err): @@ -1514,9 +1520,12 @@ func (ds *Datastore) GetHostLockWipeStatus(ctx context.Context, host *fleet.Host if mdmActions.WipeRef != nil { // the wipe reference is an MDM command cmd, cmdRes, err := ds.getHostMDMAppleCommand(ctx, *mdmActions.WipeRef, host.UUID) - if err != nil { + if err != nil && !fleet.IsNotFound(err) { return nil, ctxerr.Wrap(ctx, err, "get wipe reference") } + if fleet.IsNotFound(err) { + ds.logger.ErrorContext(ctx, "orphan wipe MDM command reference", "host_id", host.ID, "command_uuid", *mdmActions.WipeRef) + } status.WipeMDMCommand = cmd status.WipeMDMCommandResult = cmdRes } @@ -1549,9 +1558,12 @@ func (ds *Datastore) GetHostLockWipeStatus(ctx context.Context, host *fleet.Host if mdmActions.WipeRef != nil { if hostPlatform == "windows" { cmd, cmdRes, err := ds.getHostMDMWindowsCommand(ctx, *mdmActions.WipeRef, host.UUID) - if err != nil { + if err != nil && !fleet.IsNotFound(err) { return nil, ctxerr.Wrap(ctx, err, "get wipe reference") } + if fleet.IsNotFound(err) { + ds.logger.ErrorContext(ctx, "orphan wipe MDM command reference", "host_id", host.ID, "command_uuid", *mdmActions.WipeRef) + } status.WipeMDMCommand = cmd status.WipeMDMCommandResult = cmdRes } else {