From 436733763a9464219684c19fe8ca8a54aae30624 Mon Sep 17 00:00:00 2001 From: Roberto Dip Date: Wed, 18 Oct 2023 11:29:40 -0300 Subject: [PATCH] always assign a DEP profile if the host is assigned in ABM (#14606) for #13703 and #13992, this updates the logic used by the functions that gather hosts that need DEP profile updates to use hosts directly from `host_dep_assignments`, regardless of their MDM status. --- changes/13703-fix-dep-assignment | 1 + server/datastore/mysql/apple_mdm.go | 25 +++------ server/datastore/mysql/apple_mdm_test.go | 61 +++++++++------------ server/service/integration_mdm_test.go | 50 ++++++++++++++++- server/worker/macos_setup_assistant_test.go | 2 +- 5 files changed, 83 insertions(+), 56 deletions(-) create mode 100644 changes/13703-fix-dep-assignment diff --git a/changes/13703-fix-dep-assignment b/changes/13703-fix-dep-assignment new file mode 100644 index 000000000..9026dd7b6 --- /dev/null +++ b/changes/13703-fix-dep-assignment @@ -0,0 +1 @@ +* Fixed a bug that could cause DEP profiles to not be properly assigned to hosts. diff --git a/server/datastore/mysql/apple_mdm.go b/server/datastore/mysql/apple_mdm.go index 8682fac5e..a7e5b7d41 100644 --- a/server/datastore/mysql/apple_mdm.go +++ b/server/datastore/mysql/apple_mdm.go @@ -2857,18 +2857,13 @@ SELECT hardware_serial FROM hosts h - -- mdm join - %s + JOIN host_dep_assignments hda ON hda.host_id = h.id WHERE h.hardware_serial != '' AND -- team_id condition %s AND - hmdm.name = ? AND - -- hmdm.enrolled can be either 0 or 1, does not matter - hmdm.installed_from_dep = 1 AND - NOT COALESCE(hmdm.is_server, false) -`, hostMDMJoin, teamCond) - args = append(args, fleet.WellKnownMDMFleet) + hda.deleted_at IS NULL +`, teamCond) var serials []string if err := sqlx.SelectContext(ctx, ds.reader(ctx), &serials, stmt, args...); err != nil { @@ -2882,23 +2877,19 @@ func (ds *Datastore) ListMDMAppleDEPSerialsInHostIDs(ctx context.Context, hostID return nil, nil } - stmt := fmt.Sprintf(` + stmt := ` SELECT hardware_serial FROM hosts h - -- mdm join - %s + JOIN host_dep_assignments hda ON hda.host_id = h.id WHERE h.hardware_serial != '' AND h.id IN (?) AND - hmdm.name = ? AND - -- hmdm.enrolled can be either 0 or 1, does not matter - hmdm.installed_from_dep = 1 AND - NOT COALESCE(hmdm.is_server, false) -`, hostMDMJoin) + hda.deleted_at IS NULL +` - stmt, args, err := sqlx.In(stmt, hostIDs, fleet.WellKnownMDMFleet) + stmt, args, err := sqlx.In(stmt, hostIDs) if err != nil { return nil, ctxerr.Wrap(ctx, err, "prepare statement arguments") } diff --git a/server/datastore/mysql/apple_mdm_test.go b/server/datastore/mysql/apple_mdm_test.go index d282faa6e..27ca1f0e5 100644 --- a/server/datastore/mysql/apple_mdm_test.go +++ b/server/datastore/mysql/apple_mdm_test.go @@ -3799,10 +3799,10 @@ func testListMDMAppleSerials(t *testing.T, ds *Datastore) { ctx := context.Background() // create a mix of DEP-enrolled hosts, non-Fleet-MDM, pending DEP-enrollment - hosts := make([]*fleet.Host, 10) + hosts := make([]*fleet.Host, 7) for i := 0; i < len(hosts); i++ { serial := fmt.Sprintf("serial-%d", i) - if i == 9 { + if i == 6 { serial = "" } h, err := ds.NewHost(ctx, &fleet.Host{ @@ -3816,30 +3816,21 @@ func testListMDMAppleSerials(t *testing.T, ds *Datastore) { require.NoError(t, err) switch { case i <= 3: - // dep-enrolled in fleet - err = ds.SetOrUpdateMDMData(ctx, h.ID, false, true, "https://example.com", true, fleet.WellKnownMDMFleet) + // assigned in ABM to Fleet + err = ds.UpsertMDMAppleHostDEPAssignments(ctx, []fleet.Host{*h}) + require.NoError(t, err) case i == 4: - // pending dep enrollment in fleet - err = ds.SetOrUpdateMDMData(ctx, h.ID, false, false, "https://example.com", true, fleet.WellKnownMDMFleet) + // not ABM assigned case i == 5: - // manually enrolled in fleet - err = ds.SetOrUpdateMDMData(ctx, h.ID, false, true, "https://example.com", false, fleet.WellKnownMDMFleet) + // ABM assignment was deleted + err = ds.UpsertMDMAppleHostDEPAssignments(ctx, []fleet.Host{*h}) + require.NoError(t, err) + err = ds.DeleteHostDEPAssignments(ctx, []string{h.HardwareSerial}) + require.NoError(t, err) case i == 6: - // dep enrolled in fleet but is a server - err = ds.SetOrUpdateMDMData(ctx, h.ID, true, true, "https://example.com", true, fleet.WellKnownMDMFleet) - case i == 7: - // dep enrolled in non-Fleet - err = ds.SetOrUpdateMDMData(ctx, h.ID, false, true, "https://simplemdm.com", true, fleet.WellKnownMDMSimpleMDM) - case i == 8: - // not mdm-enrolled at all - err = nil - case i == 9: - // dep-enrolled in fleet, but empty serial so not returned - err = ds.SetOrUpdateMDMData(ctx, h.ID, false, true, "https://example.com", true, fleet.WellKnownMDMFleet) - } - require.NoError(t, err) - if i <= 3 { - nanoEnroll(t, ds, h, false) + // assigned in ABM, but we don't have a serial + err = ds.UpsertMDMAppleHostDEPAssignments(ctx, []fleet.Host{*h}) + require.NoError(t, err) } hosts[i] = h t.Logf("host [%d]: %s - %s", i, h.UUID, h.HardwareSerial) @@ -3851,8 +3842,8 @@ func testListMDMAppleSerials(t *testing.T, ds *Datastore) { tm2, err := ds.NewTeam(ctx, &fleet.Team{Name: "team2"}) require.NoError(t, err) - // assign hosts[2,7,8] to tm1 - err = ds.AddHostsToTeam(ctx, &tm1.ID, []uint{hosts[2].ID, hosts[7].ID, hosts[8].ID}) + // assign hosts[2,4,5] to tm1 + err = ds.AddHostsToTeam(ctx, &tm1.ID, []uint{hosts[2].ID, hosts[4].ID, hosts[5].ID}) require.NoError(t, err) // list serials in team 2, has none @@ -3865,33 +3856,33 @@ func testListMDMAppleSerials(t *testing.T, ds *Datastore) { require.NoError(t, err) require.ElementsMatch(t, []string{"serial-2"}, serials) - // list serials in no-team, has 4 (hosts[0,1,3,4]), hosts[4] is pending, the others enrolled + // list serials in no-team, has 3 (hosts[0,1,3]), hosts[6] doesn't have a serial number serials, err = ds.ListMDMAppleDEPSerialsInTeam(ctx, nil) require.NoError(t, err) - require.ElementsMatch(t, []string{"serial-0", "serial-1", "serial-3", "serial-4"}, serials) + require.ElementsMatch(t, []string{"serial-0", "serial-1", "serial-3"}, serials) // list serials with no host IDs returns empty serials, err = ds.ListMDMAppleDEPSerialsInHostIDs(ctx, nil) require.NoError(t, err) require.Empty(t, serials) - // list serials in hosts[0,1,2,3,4] returns all of them - serials, err = ds.ListMDMAppleDEPSerialsInHostIDs(ctx, []uint{hosts[0].ID, hosts[1].ID, hosts[2].ID, hosts[3].ID, hosts[4].ID}) + // list serials in hosts[0,1,2,3] returns all of them + serials, err = ds.ListMDMAppleDEPSerialsInHostIDs(ctx, []uint{hosts[0].ID, hosts[1].ID, hosts[2].ID, hosts[3].ID}) require.NoError(t, err) - require.ElementsMatch(t, []string{"serial-0", "serial-1", "serial-2", "serial-3", "serial-4"}, serials) + require.ElementsMatch(t, []string{"serial-0", "serial-1", "serial-2", "serial-3"}, serials) - // list serials in hosts[5,6,7,8,9] returns none - serials, err = ds.ListMDMAppleDEPSerialsInHostIDs(ctx, []uint{hosts[5].ID, hosts[6].ID, hosts[7].ID, hosts[8].ID, hosts[9].ID}) + // list serials in hosts[4,5,6] returns none + serials, err = ds.ListMDMAppleDEPSerialsInHostIDs(ctx, []uint{hosts[4].ID, hosts[5].ID, hosts[6].ID}) require.NoError(t, err) require.Empty(t, serials) - // list serials in all hosts returns [0-4] + // list serials in all hosts returns [0-3] serials, err = ds.ListMDMAppleDEPSerialsInHostIDs(ctx, []uint{ hosts[0].ID, hosts[1].ID, hosts[2].ID, hosts[3].ID, hosts[4].ID, - hosts[5].ID, hosts[6].ID, hosts[7].ID, hosts[8].ID, hosts[9].ID, + hosts[5].ID, hosts[6].ID, }) require.NoError(t, err) - require.ElementsMatch(t, []string{"serial-0", "serial-1", "serial-2", "serial-3", "serial-4"}, serials) + require.ElementsMatch(t, []string{"serial-0", "serial-1", "serial-2", "serial-3"}, serials) } func testMDMAppleDefaultSetupAssistant(t *testing.T, ds *Datastore) { diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index 8a2b2d5aa..a70c81a8c 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -664,17 +664,17 @@ func (s *integrationMDMTestSuite) TestProfileRetries() { "I1": { Identifier: "I1", DisplayName: "N1", - InstallDate: time.Now(), + InstallDate: time.Now().Add(15 * time.Minute), }, "I2": { Identifier: "I2", DisplayName: "N2", - InstallDate: time.Now(), + InstallDate: time.Now().Add(15 * time.Minute), }, mobileconfig.FleetdConfigPayloadIdentifier: { Identifier: mobileconfig.FleetdConfigPayloadIdentifier, DisplayName: "Fleetd configuration", - InstallDate: time.Now(), + InstallDate: time.Now().Add(15 * time.Minute), }, } reportHostProfs := func(t *testing.T, identifiers ...string) { @@ -1887,6 +1887,50 @@ func (s *integrationMDMTestSuite) TestDEPProfileAssignment() { // it should still get the post-enrollment commands require.NoError(t, mdmDevice.Enroll()) checkPostEnrollmentCommands(mdmDevice, true) + + // enroll a host into Fleet + eHost, err := s.ds.NewHost(context.Background(), &fleet.Host{ + ID: 1, + OsqueryHostID: ptr.String("Desktop-ABCQWE"), + NodeKey: ptr.String("Desktop-ABCQWE"), + UUID: uuid.New().String(), + Hostname: fmt.Sprintf("%sfoo.local", s.T().Name()), + Platform: "darwin", + HardwareSerial: uuid.New().String(), + }) + require.NoError(t, err) + + // on team transfer, we don't assign a DEP profile to the device + s.Do("POST", "/api/v1/fleet/hosts/transfer", + addHostsToTeamRequest{TeamID: &team.ID, HostIDs: []uint{eHost.ID}}, http.StatusOK) + profileAssignmentReqs = []profileAssignmentReq{} + s.runWorker() + require.Empty(t, profileAssignmentReqs) + + // assign the host in ABM + devices = []godep.Device{ + {SerialNumber: eHost.HardwareSerial, Model: "MacBook Pro", OS: "osx", OpType: "modified"}, + } + profileAssignmentReqs = []profileAssignmentReq{} + s.runDEPSchedule() + require.NotEmpty(t, profileAssignmentReqs) + require.Equal(t, eHost.HardwareSerial, profileAssignmentReqs[0].Devices[0]) + + // transfer to "no team", we assign a DEP profile to the device + profileAssignmentReqs = []profileAssignmentReq{} + s.Do("POST", "/api/v1/fleet/hosts/transfer", + addHostsToTeamRequest{TeamID: nil, HostIDs: []uint{eHost.ID}}, http.StatusOK) + s.runWorker() + require.NotEmpty(t, profileAssignmentReqs) + require.Equal(t, eHost.HardwareSerial, profileAssignmentReqs[0].Devices[0]) + + // transfer to the team back again, we assign a DEP profile to the device again + s.Do("POST", "/api/v1/fleet/hosts/transfer", + addHostsToTeamRequest{TeamID: &team.ID, HostIDs: []uint{eHost.ID}}, http.StatusOK) + profileAssignmentReqs = []profileAssignmentReq{} + s.runWorker() + require.NotEmpty(t, profileAssignmentReqs) + require.Equal(t, eHost.HardwareSerial, profileAssignmentReqs[0].Devices[0]) } func loadEnrollmentProfileDEPToken(t *testing.T, ds *mysql.Datastore) string { diff --git a/server/worker/macos_setup_assistant_test.go b/server/worker/macos_setup_assistant_test.go index 6fc974726..34e683517 100644 --- a/server/worker/macos_setup_assistant_test.go +++ b/server/worker/macos_setup_assistant_test.go @@ -38,7 +38,7 @@ func TestMacosSetupAssistant(t *testing.T) { HardwareSerial: fmt.Sprintf("serial-%d", i), }) require.NoError(t, err) - err = ds.SetOrUpdateMDMData(ctx, h.ID, false, true, "https://example.com", true, fleet.WellKnownMDMFleet) + err = ds.UpsertMDMAppleHostDEPAssignments(ctx, []fleet.Host{*h}) require.NoError(t, err) hosts[i] = h t.Logf("host [%d]: %s - %s", i, h.UUID, h.HardwareSerial)