From 60ab8c1ac8738db5068bd1680b2428fb7e899330 Mon Sep 17 00:00:00 2001 From: Roberto Dip Date: Fri, 22 Sep 2023 21:54:45 -0300 Subject: [PATCH] ensure enrollment commands are sent to devices assigned in ABM to Fleet (#14100) for #13702 --- changes/13702 | 2 ++ server/datastore/mysql/hosts.go | 16 +++++----------- server/datastore/mysql/hosts_test.go | 13 +++++++++++++ server/fleet/hosts.go | 9 +++++---- server/service/apple_mdm.go | 5 ++++- server/service/apple_mdm_test.go | 9 +++++---- server/service/integration_mdm_test.go | 10 ++++++++++ 7 files changed, 44 insertions(+), 20 deletions(-) create mode 100644 changes/13702 diff --git a/changes/13702 b/changes/13702 new file mode 100644 index 0000000000..0bd01b78e8 --- /dev/null +++ b/changes/13702 @@ -0,0 +1,2 @@ +* Ensure post-enrollment commands are sent to devices assigned to Fleet in ABM. +* Ensure hosts assigned to Fleet in ABM come back to pending to the right team after they're deleted. diff --git a/server/datastore/mysql/hosts.go b/server/datastore/mysql/hosts.go index d333b8a348..10124e68e0 100644 --- a/server/datastore/mysql/hosts.go +++ b/server/datastore/mysql/hosts.go @@ -3134,16 +3134,6 @@ func (ds *Datastore) GetHostMDM(ctx context.Context, hostID uint) (*fleet.HostMD } func (ds *Datastore) GetHostMDMCheckinInfo(ctx context.Context, hostUUID string) (*fleet.HostMDMCheckinInfo, error) { - // TODO: consider using host_dep_assignments instead of host_mdm because installed_from_dep can - // be set to false for DEP-assigned host (e.g., ds.UpdateHostTablesOnMDMUnenroll), which may - // lead to unexpected results in certain edge cases where HostMDMCheckinInfo is used to - // determine like bootstrap package installation - // NOTE: Consider joining on host_dep_assignments instead of host_mdm so DEP hosts that - // manually enroll or re-enroll are included in the results so long as they are not unassigned - // in Apple Business Manager. The problem with using host_dep_assignments is that a host can be - // assigned to Fleet in ABM but still manually enroll. We should probably keep using host_mdm, - // but be better at updating the table with the right values when a host enrolls (perhaps adding - // a query param to the enroll endpoint). var hmdm fleet.HostMDMCheckinInfo // use writer as it is used just after creation in some cases @@ -3152,7 +3142,8 @@ func (ds *Datastore) GetHostMDMCheckinInfo(ctx context.Context, hostUUID string) h.hardware_serial, COALESCE(hm.installed_from_dep, false) as installed_from_dep, hd.display_name, - COALESCE(h.team_id, 0) as team_id + COALESCE(h.team_id, 0) as team_id, + hda.host_id IS NOT NULL AND hda.deleted_at IS NULL as dep_assigned_to_fleet FROM hosts h LEFT JOIN @@ -3161,6 +3152,9 @@ func (ds *Datastore) GetHostMDMCheckinInfo(ctx context.Context, hostUUID string) LEFT JOIN host_display_names hd ON h.id = hd.host_id + LEFT JOIN + host_dep_assignments hda + ON h.id = hda.host_id WHERE h.uuid = ? LIMIT 1`, hostUUID) if err != nil { if err == sql.ErrNoRows { diff --git a/server/datastore/mysql/hosts_test.go b/server/datastore/mysql/hosts_test.go index 0cc6846c20..58367713b1 100644 --- a/server/datastore/mysql/hosts_test.go +++ b/server/datastore/mysql/hosts_test.go @@ -6476,6 +6476,19 @@ func testHostsGetHostMDMCheckinInfo(t *testing.T, ds *Datastore) { require.Equal(t, host.HardwareSerial, info.HardwareSerial) require.Equal(t, true, info.InstalledFromDEP) require.EqualValues(t, tm.ID, info.TeamID) + require.False(t, info.DEPAssignedToFleet) + + err = ds.UpsertMDMAppleHostDEPAssignments(ctx, []fleet.Host{*host}) + require.NoError(t, err) + info, err = ds.GetHostMDMCheckinInfo(ctx, host.UUID) + require.NoError(t, err) + require.True(t, info.DEPAssignedToFleet) + + err = ds.DeleteHostDEPAssignments(ctx, []string{host.HardwareSerial}) + require.NoError(t, err) + info, err = ds.GetHostMDMCheckinInfo(ctx, host.UUID) + require.NoError(t, err) + require.False(t, info.DEPAssignedToFleet) } func testHostsLoadHostByOrbitNodeKey(t *testing.T, ds *Datastore) { diff --git a/server/fleet/hosts.go b/server/fleet/hosts.go index bf4466daf2..87a401f71b 100644 --- a/server/fleet/hosts.go +++ b/server/fleet/hosts.go @@ -1040,10 +1040,11 @@ type EnrollHostLimiter interface { } type HostMDMCheckinInfo struct { - HardwareSerial string `json:"hardware_serial" db:"hardware_serial"` - InstalledFromDEP bool `json:"installed_from_dep" db:"installed_from_dep"` - DisplayName string `json:"display_name" db:"display_name"` - TeamID uint `json:"team_id" db:"team_id"` + HardwareSerial string `json:"hardware_serial" db:"hardware_serial"` + InstalledFromDEP bool `json:"installed_from_dep" db:"installed_from_dep"` + DisplayName string `json:"display_name" db:"display_name"` + TeamID uint `json:"team_id" db:"team_id"` + DEPAssignedToFleet bool `json:"dep_assigned_to_fleet" db:"dep_assigned_to_fleet"` } type HostDiskEncryptionKey struct { diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index efe6bb6bf8..5417b8f094 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -2276,7 +2276,10 @@ func (svc *MDMAppleCheckinAndCommandService) TokenUpdate(r *mdm.Request, m *mdm. if err != nil { return err } - if info.InstalledFromDEP { + + // TODO: improve this to not enqueue the job if a host that is + // assigned in ABM is manually enrolling for some reason. + if info.DEPAssignedToFleet || info.InstalledFromDEP { svc.logger.Log("info", "queueing post-enroll task for newly enrolled DEP device", "host_uuid", r.ID) var tmID *uint diff --git a/server/service/apple_mdm_test.go b/server/service/apple_mdm_test.go index b6b3d68253..49f941ce3c 100644 --- a/server/service/apple_mdm_test.go +++ b/server/service/apple_mdm_test.go @@ -1020,10 +1020,11 @@ func TestMDMTokenUpdate(t *testing.T) { ds.GetHostMDMCheckinInfoFunc = func(ct context.Context, hostUUID string) (*fleet.HostMDMCheckinInfo, error) { require.Equal(t, uuid, hostUUID) return &fleet.HostMDMCheckinInfo{ - HardwareSerial: serial, - DisplayName: model, - InstalledFromDEP: true, - TeamID: wantTeamID, + HardwareSerial: serial, + DisplayName: model, + InstalledFromDEP: true, + TeamID: wantTeamID, + DEPAssignedToFleet: true, }, nil } diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index 0fb0ba0843..ddf83f81a2 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -1861,6 +1861,16 @@ func (s *integrationMDMTestSuite) TestDEPProfileAssignment() { // it should get the post-enrollment commands require.NoError(t, mdmDevice.Enroll()) checkPostEnrollmentCommands(mdmDevice, true) + + // delete all MDM info + mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { + _, err := q.ExecContext(ctx, `DELETE FROM host_mdm WHERE host_id = ?`, listHostsRes.Hosts[0].ID) + return err + }) + + // it should still get the post-enrollment commands + require.NoError(t, mdmDevice.Enroll()) + checkPostEnrollmentCommands(mdmDevice, true) } func loadEnrollmentProfileDEPToken(t *testing.T, ds *mysql.Datastore) string {