From 6223af892ec50f9fa5833e6a35be7fba00fa2a1f Mon Sep 17 00:00:00 2001 From: Jordan Montgomery Date: Wed, 1 Jul 2026 10:59:55 -0400 Subject: [PATCH] Fix manual-personal enrollment for iOS/iPadOS (#48534) **Related issue:** Resolves # # Checklist for submitter If some of the following don't apply, delete the relevant line. Unreleased bug, no changes file - [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] Timeouts are implemented and retries are limited to avoid infinite loops - [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) - [ ] QA'd all new/changed functionality manually For unreleased bug fixes in a release candidate, one of: - [x] Confirmed that the fix is not expected to adversely impact load test results - [ ] Alerted the release DRI if additional load testing is needed ## Summary by CodeRabbit * **Bug Fixes** * Personal enrollment status is now preserved and updated correctly when MDM device records change. * macOS MDM ingestion now keeps the BYOD/personal enrollment flag for Fleet devices instead of defaulting it away. * Incoming server URLs continue to have query parameters removed while still retaining the enrollment status used for processing. * **Tests** * Added coverage for personal enrollment updates and macOS ingestion scenarios, including BYOD and non-BYOD cases. --- server/datastore/mysql/apple_mdm.go | 2 +- server/datastore/mysql/apple_mdm_test.go | 51 +++++++++++++++ server/service/osquery_utils/queries.go | 14 +++-- server/service/osquery_utils/queries_test.go | 65 ++++++++++++++++++++ 4 files changed, 127 insertions(+), 5 deletions(-) diff --git a/server/datastore/mysql/apple_mdm.go b/server/datastore/mysql/apple_mdm.go index 3fdc8932a2..81e38a334b 100644 --- a/server/datastore/mysql/apple_mdm.go +++ b/server/datastore/mysql/apple_mdm.go @@ -1931,7 +1931,7 @@ func upsertMDMAppleHostMDMInfoDB(ctx context.Context, tx sqlx.ExtContext, appCfg _, err = tx.ExecContext(ctx, 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)`, strings.Join(parts, ",")), args...) + ON DUPLICATE KEY UPDATE enrolled = VALUES(enrolled), is_personal_enrollment = VALUES(is_personal_enrollment)`, strings.Join(parts, ",")), args...) return ctxerr.Wrap(ctx, err, "upsert host mdm info") } diff --git a/server/datastore/mysql/apple_mdm_test.go b/server/datastore/mysql/apple_mdm_test.go index f273a49576..f9be3ea79a 100644 --- a/server/datastore/mysql/apple_mdm_test.go +++ b/server/datastore/mysql/apple_mdm_test.go @@ -100,6 +100,7 @@ func TestMDMApple(t *testing.T) { {"TestMDMConfigAsset", testMDMConfigAsset}, {"ListIOSAndIPadOSToRefetch", testListIOSAndIPadOSToRefetch}, {"MDMAppleUpsertHostIOSiPadOS", testMDMAppleUpsertHostIOSIPadOS}, + {"MDMAppleUpsertHostPersonalEnrollment", testMDMAppleUpsertHostPersonalEnrollment}, {"IngestMDMAppleDevicesFromDEPSyncIOSIPadOS", testIngestMDMAppleDevicesFromDEPSyncIOSIPadOS}, {"MDMAppleProfilesOnIOSIPadOS", testMDMAppleProfilesOnIOSIPadOS}, {"GetEnrollmentIDsWithPendingMDMAppleCommands", testGetEnrollmentIDsWithPendingMDMAppleCommands}, @@ -7601,6 +7602,56 @@ func testMDMAppleUpsertHostIOSIPadOS(t *testing.T, ds *Datastore) { require.Equal(t, "macOS", labels[1].Name) } +// testMDMAppleUpsertHostPersonalEnrollment guards the BYOD signal through the +// Apple Authenticate flow: host_mdm.is_personal_enrollment must track the +// fromPersonalEnrollment flag on every upsert, including when a host_mdm row +// already exists. Regression test for the upsert dropping the flag on conflict +// (ON DUPLICATE KEY UPDATE only rewrote `enrolled`), which left re-enrolling +// devices stuck at their previous value. +func testMDMAppleUpsertHostPersonalEnrollment(t *testing.T, ds *Datastore) { + ctx := t.Context() + createBuiltinLabels(t, ds) + + readPersonalEnrollment := func(hostID uint) bool { + var isPersonal bool + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext(ctx, q, &isPersonal, + `SELECT is_personal_enrollment FROM host_mdm WHERE host_id = ?`, hostID) + }) + return isPersonal + } + + upsert := func(uuid string, personal bool) uint { + err := ds.MDMAppleUpsertHost(ctx, &fleet.Host{ + UUID: uuid, + HardwareSerial: "serial-" + uuid, + HardwareModel: "iPad13,1", + Platform: "ipados", + }, personal) + require.NoError(t, err) + h, err := ds.HostByIdentifier(ctx, uuid) + require.NoError(t, err) + return h.ID + } + + // Company-owned device that later re-enrolls as BYOD. The second upsert hits + // updateMDMAppleHostDB with an existing host_mdm row, so the flag must flip. + hostID := upsert("company-then-byod", false) + require.False(t, readPersonalEnrollment(hostID), "initial company-owned enrollment should not be personal") + + require.Equal(t, hostID, upsert("company-then-byod", true)) + require.True(t, readPersonalEnrollment(hostID), "re-enrolling as BYOD must set is_personal_enrollment") + + // Re-enrolling the same device as company-owned again must clear the flag, + // matching the ABM/DEP resync path (fromPersonalEnrollment=false). + require.Equal(t, hostID, upsert("company-then-byod", false)) + require.False(t, readPersonalEnrollment(hostID), "re-enrolling as company-owned must clear is_personal_enrollment") + + // A brand-new host inserted directly as BYOD (insertMDMAppleHostDB path). + byodID := upsert("byod-first", true) + require.True(t, readPersonalEnrollment(byodID), "fresh BYOD enrollment should be personal") +} + func testIngestMDMAppleDevicesFromDEPSyncIOSIPadOS(t *testing.T, ds *Datastore) { ctx := t.Context() diff --git a/server/service/osquery_utils/queries.go b/server/service/osquery_utils/queries.go index 7fb028b9a1..c1aec19032 100644 --- a/server/service/osquery_utils/queries.go +++ b/server/service/osquery_utils/queries.go @@ -2691,10 +2691,16 @@ func directIngestMDMMac(ctx context.Context, logger *slog.Logger, host *fleet.Ho } } - // isPersonalEnrollment is always false for macOS hosts as our current account driven user - // enrollment flow does not support macOS however we will need to detect it here if that ever - // changes. - isPersonalEnrollment := false + // Fleet bakes byod=1 into the enrollment profile's ServerURL for personal + // (BYOD) enrollments (apple_mdm.AddPersonalEnrollmentToFleetURL). osquery + // reports that ServerURL here, so we read the flag back the same way we read + // the enroll reference above. Without this, the detail-query ingest would + // overwrite the is_personal_enrollment set by the Apple Authenticate flow. + // Must be read before RawQuery is cleared below. + var isPersonalEnrollment bool + if mdmSolutionName == fleet.WellKnownMDMFleet { + isPersonalEnrollment = serverURL.Query().Get(apple_mdm.FleetPersonalEnrollmentKey) == "1" + } // strip any query parameters from the URL serverURL.RawQuery = "" diff --git a/server/service/osquery_utils/queries_test.go b/server/service/osquery_utils/queries_test.go index b6862110c6..31c2f95d6c 100644 --- a/server/service/osquery_utils/queries_test.go +++ b/server/service/osquery_utils/queries_test.go @@ -1098,6 +1098,71 @@ func TestDirectIngestMDMFleetEnrollRef(t *testing.T) { }) } +// TestDirectIngestMDMMacPersonalEnrollment guards that the macOS detail-query +// ingest reads the BYOD signal back from the profile's ServerURL (byod=1) rather +// than hardcoding false, which would otherwise clobber the is_personal_enrollment +// set by the Apple Authenticate flow on every check-in. +func TestDirectIngestMDMMacPersonalEnrollment(t *testing.T) { + ds := new(mock.Store) + var host fleet.Host + + generateRows := func(serverURL, payloadIdentifier string) []map[string]string { + return []map[string]string{ + { + "enrolled": "true", + "installed_from_dep": "false", + "server_url": serverURL, + "payload_identifier": payloadIdentifier, + }, + } + } + + for _, tc := range []struct { + name string + mdmData []map[string]string + wantPersonal bool + }{ + { + name: "Fleet byod=1", + mdmData: generateRows("https://test.example.com?byod=1", apple_mdm.FleetPayloadIdentifier), + wantPersonal: true, + }, + { + name: "Fleet no byod", + mdmData: generateRows("https://test.example.com", apple_mdm.FleetPayloadIdentifier), + wantPersonal: false, + }, + { + name: "Fleet byod=1 alongside other params", + mdmData: generateRows("https://test.example.com?enroll_reference=ref&byod=1", apple_mdm.FleetPayloadIdentifier), + wantPersonal: true, + }, + { + name: "Fleet byod=0", + mdmData: generateRows("https://test.example.com?byod=0", apple_mdm.FleetPayloadIdentifier), + wantPersonal: false, + }, + { + name: "non-Fleet byod=1 ignored", + mdmData: generateRows("https://test.example.com?byod=1", "com.unknown.mdm"), + wantPersonal: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + ds.SetOrUpdateMDMDataFunc = func(ctx context.Context, hostID uint, isServer, enrolled bool, serverURL string, installedFromDep bool, name string, fleetEnrollmentRef string, isPersonalEnrollment bool) error { + require.Equal(t, tc.wantPersonal, isPersonalEnrollment) + require.Equal(t, "https://test.example.com", serverURL) // query string is stripped + return nil + } + + err := directIngestMDMMac(t.Context(), slog.New(slog.DiscardHandler), &host, ds, tc.mdmData) + require.NoError(t, err) + require.True(t, ds.SetOrUpdateMDMDataFuncInvoked) + ds.SetOrUpdateMDMDataFuncInvoked = false + }) + } +} + func TestDirectIngestMDMWindows(t *testing.T) { ds := new(mock.Store) cases := []struct {