diff --git a/changes/45217-always-update-apple-enrollment-type b/changes/45217-always-update-apple-enrollment-type new file mode 100644 index 0000000000..99f5d40185 --- /dev/null +++ b/changes/45217-always-update-apple-enrollment-type @@ -0,0 +1 @@ +- Fixed an issue where re-enrolling an Apple device with a different type, e.g. Manual -> ADE, would not update the enrollment type correctly. \ No newline at end of file diff --git a/server/datastore/mysql/apple_mdm.go b/server/datastore/mysql/apple_mdm.go index ec61ad40df..a8aee928d4 100644 --- a/server/datastore/mysql/apple_mdm.go +++ b/server/datastore/mysql/apple_mdm.go @@ -1607,7 +1607,7 @@ func updateMDMAppleHostDB( } } - if err := upsertMDMAppleHostMDMInfoDB(ctx, tx, appCfg, false, fromPersonalEnrollment, hostID); err != nil { + if err := upsertMDMAppleHostMDMInfoDB(ctx, tx, appCfg, appleMDMInfoFromCheckin, fromPersonalEnrollment, hostID); err != nil { return ctxerr.Wrap(ctx, err, "ingest mdm apple host upsert MDM info") } @@ -1688,7 +1688,7 @@ func insertMDMAppleHostDB( return ctxerr.Wrap(ctx, err, "ingest mdm apple host upsert label membership") } - if err := upsertMDMAppleHostMDMInfoDB(ctx, tx, appCfg, false, fromPersonalEnrollment, mdmHost.ID); err != nil { + if err := upsertMDMAppleHostMDMInfoDB(ctx, tx, appCfg, appleMDMInfoFromCheckin, fromPersonalEnrollment, mdmHost.ID); err != nil { return ctxerr.Wrap(ctx, err, "ingest mdm apple host upsert MDM info") } return nil @@ -1718,7 +1718,7 @@ func createHostFromMDMDB( tx sqlx.ExtContext, logger *slog.Logger, devices []hostToCreateFromMDM, - fromADE bool, + source appleMDMInfoSource, macOSTeam, iosTeam, ipadTeam *uint, ) (int64, []fleet.Host, error) { // NOTE: order of arguments for teams is important, see statement. @@ -1858,7 +1858,7 @@ func createHostFromMDMDB( ctx, tx, appCfg, - fromADE, + source, false, unmanagedHostIDs..., ); err != nil { @@ -1883,7 +1883,7 @@ func (ds *Datastore) IngestMDMAppleDeviceFromOTAEnrollment( UUID: &deviceInfo.UDID, }, } - _, hosts, err := createHostFromMDMDB(ctx, tx, ds.logger, toInsert, false, teamID, teamID, teamID) + _, hosts, err := createHostFromMDMDB(ctx, tx, ds.logger, toInsert, appleMDMInfoFromOTAEnrollment, teamID, teamID, teamID) if idpUUID != "" && len(hosts) > 0 { host := hosts[0] ds.logger.InfoContext(ctx, fmt.Sprintf("associating host %s with idp account %s", host.UUID, idpUUID)) @@ -1981,7 +1981,7 @@ func (ds *Datastore) IngestMDMAppleDevicesFromDEPSync( tx, ds.logger, htc, - true, + appleMDMInfoFromDEPSync, teamIDs[0], teamIDs[1], teamIDs[2], ) if err != nil { @@ -2031,7 +2031,8 @@ func upsertHostDEPAssignmentsDB(ctx context.Context, tx sqlx.ExtContext, hosts [ mdm_migration_deadline = VALUES(mdm_migration_deadline), hardware_serial = VALUES(hardware_serial)` - args := []interface{}{} + hostIDs := []uint{} + args := []any{} values := []string{} for _, host := range hosts { var deadline *time.Time @@ -2040,6 +2041,8 @@ func upsertHostDEPAssignmentsDB(ctx context.Context, tx sqlx.ExtContext, hosts [ } args = append(args, host.ID, abmTokenID, deadline, host.HardwareSerial) values = append(values, "(?, ?, ?, ?)") + + hostIDs = append(hostIDs, host.ID) } _, err := tx.ExecContext(ctx, fmt.Sprintf(stmt, strings.Join(values, ",")), args...) @@ -2047,6 +2050,22 @@ func upsertHostDEPAssignmentsDB(ctx context.Context, tx sqlx.ExtContext, hosts [ return ctxerr.Wrap(ctx, err, "upsert host dep assignments") } + // Cover a case where an ADE enrolled host enrolls before the DEP sync comes in and fix previous installed_from_dep=0 if set. + stmt, args, err = sqlx.In(`UPDATE host_mdm hm +JOIN mobile_device_management_solutions mdms ON mdms.id = hm.mdm_id +SET hm.installed_from_dep = 1 +WHERE hm.host_id IN (?) + AND hm.enrolled = 1 + AND hm.is_personal_enrollment = 0 + AND mdms.name = 'Fleet'`, hostIDs) + if err != nil { + return ctxerr.Wrap(ctx, err, "upsert host dep assignments update installed_from_dep") + } + _, err = tx.ExecContext(ctx, stmt, args...) + if err != nil { + return ctxerr.Wrap(ctx, err, "upsert host dep assignments update installed_from_dep") + } + return nil } @@ -2099,7 +2118,15 @@ func insertHostDisplayNamesIfAbsent(ctx context.Context, tx sqlx.ExtContext, hos return nil } -func upsertMDMAppleHostMDMInfoDB(ctx context.Context, tx sqlx.ExtContext, appCfg *fleet.AppConfig, fromSync, fromPersonalEnrollment bool, hostIDs ...uint) error { +type appleMDMInfoSource int + +const ( + appleMDMInfoFromDEPSync appleMDMInfoSource = iota // enrolled=0, from_dep=1, narrow ON DUPLICATE + appleMDMInfoFromOTAEnrollment // enrolled=1, from_dep=0, narrow ON DUPLICATE + appleMDMInfoFromCheckin // enrolled=1, from_dep=derived, wide ON DUPLICATE +) + +func upsertMDMAppleHostMDMInfoDB(ctx context.Context, tx sqlx.ExtContext, appCfg *fleet.AppConfig, source appleMDMInfoSource, fromPersonalEnrollment bool, hostIDs ...uint) error { if len(hostIDs) == 0 { return nil } @@ -2111,7 +2138,7 @@ func upsertMDMAppleHostMDMInfoDB(ctx context.Context, tx sqlx.ExtContext, appCfg // if the device is coming from the DEP sync, we don't consider it // enrolled yet. - enrolled := !fromSync + enrolled := source != appleMDMInfoFromDEPSync result, err := tx.ExecContext(ctx, ` INSERT INTO mobile_device_management_solutions (name, server_url) VALUES (?, ?) @@ -2131,16 +2158,49 @@ func upsertMDMAppleHostMDMInfoDB(ctx context.Context, tx sqlx.ExtContext, appCfg } } + depAssignedSet := map[uint]struct{}{} + if source == appleMDMInfoFromCheckin && !fromPersonalEnrollment { + stmt, args, err := sqlx.In(` + SELECT host_id FROM host_dep_assignments + WHERE host_id IN (?) AND deleted_at IS NULL`, hostIDs) + if err != nil { + return ctxerr.Wrap(ctx, err, "query dep assigned hosts") + } + var depAssigned []uint + if err := sqlx.SelectContext(ctx, tx, &depAssigned, stmt, args...); err != nil { + return ctxerr.Wrap(ctx, err, "query dep assigned hosts") + } + + for _, id := range depAssigned { + depAssignedSet[id] = struct{}{} + } + } + args := []interface{}{} parts := []string{} for _, id := range hostIDs { - args = append(args, enrolled, serverURL, fromSync, mdmID, false, id, fromPersonalEnrollment) + var isDepAssigned bool + switch source { + case appleMDMInfoFromCheckin: + _, isDepAssigned = depAssignedSet[id] + case appleMDMInfoFromDEPSync: + isDepAssigned = true + default: + isDepAssigned = false + } + args = append(args, enrolled, serverURL, isDepAssigned, mdmID, false, id, fromPersonalEnrollment) parts = append(parts, "(?, ?, ?, ?, ?, ?, ?)") } - _, err = tx.ExecContext(ctx, fmt.Sprintf(` + stmt := fmt.Sprintf(` INSERT INTO host_mdm (enrolled, server_url, installed_from_dep, mdm_id, is_server, host_id, is_personal_enrollment) VALUES %s - ON DUPLICATE KEY UPDATE enrolled = VALUES(enrolled), is_personal_enrollment = VALUES(is_personal_enrollment)`, strings.Join(parts, ",")), args...) + ON DUPLICATE KEY UPDATE enrolled = VALUES(enrolled), is_personal_enrollment = VALUES(is_personal_enrollment)`, strings.Join(parts, ",")) + + if source == appleMDMInfoFromCheckin { + stmt += `, installed_from_dep = VALUES(installed_from_dep)` + } + + _, err = tx.ExecContext(ctx, stmt, args...) return ctxerr.Wrap(ctx, err, "upsert host mdm info") } @@ -2625,7 +2685,7 @@ INSERT INTO hosts ( if err := upsertMDMAppleHostLabelMembershipDB(ctx, tx, ds.logger, *host); err != nil { return ctxerr.Wrap(ctx, err, "restore pending dep host label membership") } - if err := upsertMDMAppleHostMDMInfoDB(ctx, tx, ac, true, false, host.ID); err != nil { + if err := upsertMDMAppleHostMDMInfoDB(ctx, tx, ac, appleMDMInfoFromDEPSync, false, host.ID); err != nil { return ctxerr.Wrap(ctx, err, "ingest mdm apple host upsert MDM info") } diff --git a/server/datastore/mysql/apple_mdm_test.go b/server/datastore/mysql/apple_mdm_test.go index 93f8b0df1a..029d17c994 100644 --- a/server/datastore/mysql/apple_mdm_test.go +++ b/server/datastore/mysql/apple_mdm_test.go @@ -109,6 +109,7 @@ func TestMDMApple(t *testing.T) { {"MDMAppleUpsertHostPersonalEnrollment", testMDMAppleUpsertHostPersonalEnrollment}, {"MDMAppleUpsertHostPersonalEnrollmentClearsStaleVitals", testMDMAppleUpsertHostPersonalEnrollmentClearsStaleVitals}, {"MDMAppleUpsertHostPersonalEnrollmentClearsStaleVitalsUUIDChange", testMDMAppleUpsertHostPersonalEnrollmentClearsStaleVitalsUUIDChange}, + {"MDMAppleUpsertHostEnrollmentTypeOnReenrollment", testMDMAppleUpsertHostEnrollmentTypeOnReenrollment}, {"IngestMDMAppleDevicesFromDEPSyncIOSIPadOS", testIngestMDMAppleDevicesFromDEPSyncIOSIPadOS}, {"MDMAppleProfilesOnIOSIPadOS", testMDMAppleProfilesOnIOSIPadOS}, {"ReconcileAppleProfilesDuplicateHostUUID", testReconcileAppleProfilesDuplicateHostUUID}, @@ -8313,6 +8314,198 @@ func testMDMAppleUpsertHostPersonalEnrollmentClearsStaleVitalsUUIDChange(t *test "service subscriptions keyed by the host's previous UUID must be cleared on transition to BYOD") } +// Tests that we upsert the correct enrollment type on re-enrollment sync etc. Check-in becomes an authoritative source +// of truth to set all values, and sets installed_from_dep to true if a host_dep_assignment row exists. +func testMDMAppleUpsertHostEnrollmentTypeOnReenrollment(t *testing.T, ds *Datastore) { + ctx := t.Context() + createBuiltinLabels(t, ds) + + abmToken, err := ds.InsertABMToken(ctx, &fleet.ABMToken{ + OrganizationName: "unused", + EncryptedToken: []byte(uuid.NewString()), + RenewAt: time.Now().Add(365 * 24 * time.Hour), + }) + require.NoError(t, err) + + enrollmentStatus := func(t *testing.T, hostID uint) *string { + t.Helper() + var status *string + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext(ctx, q, &status, + `SELECT enrollment_status FROM host_mdm WHERE host_id = ?`, hostID) + }) + return status + } + + requireEnrollment := func(t *testing.T, hostID uint, wantFromDEP, wantPersonal bool, wantStatus string) { + t.Helper() + hmdm, err := ds.GetHostMDM(ctx, hostID) + require.NoError(t, err) + require.True(t, hmdm.Enrolled, "host should be enrolled") + require.Equal(t, wantFromDEP, hmdm.InstalledFromDep, "installed_from_dep") + require.Equal(t, wantPersonal, hmdm.IsPersonalEnrollment, "is_personal_enrollment") + status := enrollmentStatus(t, hostID) + require.NotNil(t, status, "enrollment_status must not be NULL") + require.Equal(t, wantStatus, *status) + } + + // checkin simulates the Apple Authenticate flow + // (resetApple -> MDMAppleUpsertHost) for the device with this serial. + checkin := func(t *testing.T, serial, hostUUID string, personal bool) uint { + t.Helper() + require.NoError(t, ds.MDMAppleUpsertHost(ctx, &fleet.Host{ + UUID: hostUUID, + HardwareSerial: serial, + HardwareModel: "iPhone14,2", + Platform: "ios", + }, personal)) + h, err := ds.HostByIdentifier(ctx, hostUUID) + require.NoError(t, err) + return h.ID + } + + // assignInABM is the "existing host newly assigned in ABM" path that + // DEPService.RunAssigner takes for serials Fleet already knows about. + assignInABM := func(t *testing.T, hostID uint, serial string) { + t.Helper() + require.NoError(t, ds.UpsertMDMAppleHostDEPAssignments(ctx, + []fleet.Host{{ID: hostID, HardwareSerial: serial}}, + abmToken.ID, make(map[uint]time.Time))) + } + + // depSync is the "serial appeared in ABM for the first time" path, which + // creates the host row up front in the Pending state. + depSync := func(t *testing.T, serial string) uint { + t.Helper() + _, err := ds.IngestMDMAppleDevicesFromDEPSync(ctx, + []godep.Device{{SerialNumber: serial, DeviceFamily: "iPhone", OpType: "added"}}, + abmToken.ID, nil, nil, nil) + require.NoError(t, err) + h, err := ds.HostByIdentifier(ctx, serial) + require.NoError(t, err) + return h.ID + } + + t.Run("manual enrollment then ADE after a wipe", func(t *testing.T) { + const serial = "REENROLL-MANUAL-TO-ADE" + + hostID := checkin(t, serial, "uuid-manual-to-ade", false) + requireEnrollment(t, hostID, false, false, fleet.MDMEnrollmentStatusManual) + + // Device is wiped locally: Fleet gets no CheckOut, so host_mdm keeps + // saying "enrolled, manual". IT then assigns it in ABM. + assignInABM(t, hostID, serial) + + // It comes back through Setup Assistant as an ADE device. + require.Equal(t, hostID, checkin(t, serial, "uuid-manual-to-ade", false)) + requireEnrollment(t, hostID, true, false, fleet.MDMEnrollmentStatusAutomatic) + }) + + t.Run("ADE enrollment then removed from AB then manual", func(t *testing.T) { + // The reverse transition: once the AB assignment is gone the host is + // no longer company-owned and must stop reporting as such. + // Side note: only happens if released in AB (and we don't get the op_type=removed), not via the new release from AB in fleet as that deletes the host_dep_assignment row. + const serial = "REENROLL-ADE-TO-MANUAL" + + hostID := depSync(t, serial) + pending := enrollmentStatus(t, hostID) + require.NotNil(t, pending) + require.Equal(t, fleet.MDMEnrollmentStatusPending, *pending) + + require.Equal(t, hostID, checkin(t, serial, "uuid-ade-to-manual", false)) + requireEnrollment(t, hostID, true, false, fleet.MDMEnrollmentStatusAutomatic) + + require.NoError(t, ds.DeleteHostDEPAssignments(ctx, abmToken.ID, []string{serial})) + + require.Equal(t, hostID, checkin(t, serial, "uuid-ade-to-manual", false)) + requireEnrollment(t, hostID, false, false, fleet.MDMEnrollmentStatusManual) + }) + + t.Run("ADE enrollment then personal re-enrollment", func(t *testing.T) { + // installed_from_dep and is_personal_enrollment must never both be set: + // the generated column has no CASE arm for that pair, so the host would + // drop out of every enrollment-status filter with a NULL status. + const serial = "REENROLL-ADE-TO-PERSONAL" + + hostID := depSync(t, serial) + require.Equal(t, hostID, checkin(t, serial, "uuid-ade-to-personal", false)) + requireEnrollment(t, hostID, true, false, fleet.MDMEnrollmentStatusAutomatic) + + // Re-enrolls as BYOD while the ABM assignment is still live. Personal + // wins over the DEP assignment. + require.Equal(t, hostID, checkin(t, serial, "uuid-ade-to-personal", true)) + requireEnrollment(t, hostID, false, true, fleet.MDMEnrollmentStatusPersonal) + }) + + t.Run("ADE check-in lands before the AB sync records the assignment", func(t *testing.T) { + // Opposite ordering, same wrong outcome: the check-in creates the host + // row before host_dep_assignments exists, and the later sync skips it + // because unmanagedHostIDs only covers hosts with enrolled = 0. + const serial = "REENROLL-CHECKIN-FIRST" + + hostID := checkin(t, serial, "uuid-checkin-first", false) + requireEnrollment(t, hostID, false, false, fleet.MDMEnrollmentStatusManual) + + assignInABM(t, hostID, serial) + requireEnrollment(t, hostID, true, false, fleet.MDMEnrollmentStatusAutomatic) + }) + + t.Run("Fleet-enrolled host whose serial first appears in a full ABM sync", func(t *testing.T) { + // Same promote step as the subtest above, reached through the other + // caller: IngestMDMAppleDevicesFromDEPSync rather than + // UpsertMDMAppleHostDEPAssignments. createHostFromMDMDB skips the host + // (unmanagedHostIDs excludes enrolled = 1), so only the assignment + // upsert can put it right. + const serial = "REENROLL-SYNC-AFTER-MANUAL" + + hostID := checkin(t, serial, "uuid-sync-after-manual", false) + requireEnrollment(t, hostID, false, false, fleet.MDMEnrollmentStatusManual) + + require.Equal(t, hostID, depSync(t, serial)) + requireEnrollment(t, hostID, true, false, fleet.MDMEnrollmentStatusAutomatic) + }) + + t.Run("host enrolled in a third-party MDM is left alone", func(t *testing.T) { + // Guards the reason the narrow ON DUPLICATE existed in the first place: + // a host being migrated from another MDM shows up in ABM before it ever + // talks to Fleet, and neither the sync nor the assignment upsert may + // rewrite its MDM info. + const serial = "REENROLL-THIRD-PARTY" + + host, err := ds.NewHost(ctx, &fleet.Host{ + Hostname: "third-party-mdm-host", + OsqueryHostID: new(serial), + NodeKey: new(serial), + UUID: "uuid-third-party", + HardwareSerial: serial, + Platform: "darwin", + }) + require.NoError(t, err) + + require.NoError(t, ds.SetOrUpdateMDMData(ctx, host.ID, + false, // isServer + true, // enrolled + "https://test.jamfcloud.com/mdm", + false, // installedFromDep + fleet.WellKnownMDMJamf, + "", // fleetEnrollmentRef + false, // isPersonalEnrollment + )) + + assignInABM(t, host.ID, serial) + _, err = ds.IngestMDMAppleDevicesFromDEPSync(ctx, + []godep.Device{{SerialNumber: serial, DeviceFamily: "Mac", OpType: "added"}}, + abmToken.ID, nil, nil, nil) + require.NoError(t, err) + + hmdm, err := ds.GetHostMDM(ctx, host.ID) + require.NoError(t, err) + require.Equal(t, fleet.WellKnownMDMJamf, hmdm.Name, "third-party MDM solution must not be rewritten to Fleet") + require.Equal(t, "https://test.jamfcloud.com/mdm", hmdm.ServerURL) + require.False(t, hmdm.InstalledFromDep, "ABM assignment alone must not mark a third-party-enrolled host as ADE") + }) +} + func testIngestMDMAppleDevicesFromDEPSyncIOSIPadOS(t *testing.T, ds *Datastore) { ctx := t.Context()