Fix manual-personal enrollment for iOS/iPadOS (#48534)
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -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")
|
||||
}
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
@@ -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 = ""
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user