Fixed stale MDM profiles after MDM toggle (#43719)
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #42427 # Checklist for submitter If some of the following don't apply, delete the relevant line. - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Pending MDM profile records are cleared when Apple or Windows MDM is turned off, preventing stale profiles from reappearing if MDM is re-enabled. * Pending Windows profile records are removed when a device is unenrolled, avoiding leftover pending installations. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -0,0 +1 @@
|
||||
* Fixed a bug where pending MDM profile rows persisted in the database after Apple or Windows MDM was turned off, causing stale profiles to reappear when MDM was re-enabled. Also fixed cleanup of pending Windows profile rows when a device unenrolls from MDM.
|
||||
@@ -696,6 +696,27 @@ ORDER BY
|
||||
return labels, nil
|
||||
}
|
||||
|
||||
func (ds *Datastore) CleanupAllHostMDMProfilesForPlatform(ctx context.Context, platform string) error {
|
||||
return ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error {
|
||||
switch platform {
|
||||
case "darwin", "ios", "ipados":
|
||||
if _, err := tx.ExecContext(ctx, `DELETE FROM host_mdm_apple_profiles`); err != nil {
|
||||
return ctxerr.Wrap(ctx, err, "deleting all rows from host_mdm_apple_profiles")
|
||||
}
|
||||
if _, err := tx.ExecContext(ctx, `DELETE FROM host_mdm_apple_declarations`); err != nil {
|
||||
return ctxerr.Wrap(ctx, err, "deleting all rows from host_mdm_apple_declarations")
|
||||
}
|
||||
case "windows":
|
||||
if _, err := tx.ExecContext(ctx, `DELETE FROM host_mdm_windows_profiles`); err != nil {
|
||||
return ctxerr.Wrap(ctx, err, "deleting all rows from host_mdm_windows_profiles")
|
||||
}
|
||||
default:
|
||||
return ctxerr.Errorf(ctx, "unsupported platform %s for MDM profile cleanup", platform)
|
||||
}
|
||||
return nil
|
||||
})
|
||||
}
|
||||
|
||||
func (ds *Datastore) BulkSetPendingMDMHostProfiles(
|
||||
ctx context.Context,
|
||||
hostIDs, teamIDs []uint,
|
||||
|
||||
@@ -9208,6 +9208,29 @@ func testDeleteMDMProfilesCancelsInstalls(t *testing.T, ds *Datastore) {
|
||||
}, &fleet.MDMCommandListOptions{Filters: fleet.MDMCommandFilters{HostIdentifier: host1.UUID}})
|
||||
require.NoError(t, err)
|
||||
require.Len(t, cmds, 0)
|
||||
|
||||
// Verify CleanupAllHostMDMProfilesForPlatform removes all remaining profile rows when MDM is
|
||||
// toggled off globally. At this point hosts still have pending remove profiles.
|
||||
|
||||
// Windows hosts have pending removes; cleanup should wipe them.
|
||||
err = ds.CleanupAllHostMDMProfilesForPlatform(ctx, "windows")
|
||||
require.NoError(t, err)
|
||||
assertHostProfileOpStatus(t, ds, host3.UUID)
|
||||
assertHostProfileOpStatus(t, ds, host4.UUID)
|
||||
|
||||
// Apple hosts still have pending removes; verify they survived the Windows cleanup, then clean them up too.
|
||||
appleProfsHost1, err := ds.GetHostMDMAppleProfiles(ctx, host1.UUID)
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, appleProfsHost1)
|
||||
|
||||
appleProfsHost2, err := ds.GetHostMDMAppleProfiles(ctx, host2.UUID)
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, appleProfsHost2)
|
||||
|
||||
err = ds.CleanupAllHostMDMProfilesForPlatform(ctx, "darwin")
|
||||
require.NoError(t, err)
|
||||
assertHostProfileOpStatus(t, ds, host1.UUID)
|
||||
assertHostProfileOpStatus(t, ds, host2.UUID)
|
||||
}
|
||||
|
||||
// testDeleteTeamCancelsWindowsProfileInstalls verifies that when a team is
|
||||
|
||||
@@ -232,20 +232,42 @@ func (ds *Datastore) MDMWindowsDeleteEnrolledDeviceOnReenrollment(ctx context.Co
|
||||
|
||||
// MDMWindowsDeleteEnrolledDeviceWithDeviceID deletes a given
|
||||
// MDMWindowsEnrolledDevice entry from the database using the device id.
|
||||
// It also cleans up all host_mdm_windows_profiles rows for the host since
|
||||
// the device can no longer receive MDM commands after unenrollment.
|
||||
func (ds *Datastore) MDMWindowsDeleteEnrolledDeviceWithDeviceID(ctx context.Context, mdmDeviceID string) error {
|
||||
stmt := "DELETE FROM mdm_windows_enrollments WHERE mdm_device_id = ?"
|
||||
return ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error {
|
||||
// Look up host_uuid before deleting the enrollment so we can clean up profile rows.
|
||||
// Use sql.NullString since host_uuid may be NULL if the enrollment hasn't been linked to a host yet.
|
||||
var hostUUID sql.NullString
|
||||
err := sqlx.GetContext(ctx, tx,
|
||||
&hostUUID, `SELECT host_uuid FROM mdm_windows_enrollments WHERE mdm_device_id = ? ORDER BY created_at DESC LIMIT 1`, mdmDeviceID)
|
||||
if err != nil {
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
return ctxerr.Wrap(ctx, notFound("MDMWindowsEnrolledDevice"))
|
||||
}
|
||||
return ctxerr.Wrap(ctx, err, "looking up host_uuid for enrolled device")
|
||||
}
|
||||
|
||||
res, err := ds.writer(ctx).ExecContext(ctx, stmt, mdmDeviceID)
|
||||
if err != nil {
|
||||
return ctxerr.Wrap(ctx, err, "delete MDMWindowsDeleteEnrolledDeviceWithDeviceID")
|
||||
}
|
||||
res, err := tx.ExecContext(ctx, `DELETE FROM mdm_windows_enrollments WHERE mdm_device_id = ?`, mdmDeviceID)
|
||||
if err != nil {
|
||||
return ctxerr.Wrap(ctx, err, "deleting Windows enrolled device")
|
||||
}
|
||||
|
||||
deleted, _ := res.RowsAffected()
|
||||
if deleted != 1 {
|
||||
return ctxerr.Wrap(ctx, notFound("MDMWindowsEnrolledDevice"))
|
||||
}
|
||||
|
||||
// Clean up all host_mdm_windows_profiles rows for this host since it can no longer receive MDM commands.
|
||||
if hostUUID.Valid && hostUUID.String != "" {
|
||||
if _, err := tx.ExecContext(ctx,
|
||||
`DELETE FROM host_mdm_windows_profiles WHERE host_uuid = ?`, hostUUID.String); err != nil {
|
||||
return ctxerr.Wrap(ctx, err, "cleaning up Windows host MDM profiles after unenrollment")
|
||||
}
|
||||
}
|
||||
|
||||
deleted, _ := res.RowsAffected()
|
||||
if deleted == 1 {
|
||||
return nil
|
||||
}
|
||||
|
||||
return ctxerr.Wrap(ctx, notFound("MDMWindowsDeleteEnrolledDeviceWithDeviceID"))
|
||||
})
|
||||
}
|
||||
|
||||
// this function inserts both the host_mdm_windows_profile entries and the actual mdm_windows_command_queue entries for a given command and list of hosts.
|
||||
|
||||
@@ -54,6 +54,7 @@ func TestMDMWindows(t *testing.T) {
|
||||
{"TestResendWindowsMDMCommand", testResendWindowsMDMCommand},
|
||||
{"TestDeleteProfileLocURIProtection", testDeleteProfileLocURIProtection},
|
||||
{"TestEditProfileDeletesRemovedLocURIs", testEditProfileDeletesRemovedLocURIs},
|
||||
{"TestMDMWindowsUnenrollCleansUpProfiles", testMDMWindowsUnenrollCleansUpProfiles},
|
||||
}
|
||||
|
||||
for _, c := range cases {
|
||||
@@ -4479,3 +4480,41 @@ func testEditProfileDeletesRemovedLocURIs(t *testing.T, ds *Datastore) {
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// testMDMWindowsUnenrollCleansUpProfiles verifies that pending profile rows are cleaned up when a
|
||||
// Windows host unenrolls from MDM (issue #42427, scenario B).
|
||||
func testMDMWindowsUnenrollCleansUpProfiles(t *testing.T, ds *Datastore) {
|
||||
ctx := t.Context()
|
||||
|
||||
// create a Windows host and enroll it
|
||||
host := test.NewHost(t, ds, "win-unenroll", "10.0.0.1", "win-key", "win-unenroll-uuid", time.Now())
|
||||
host.Platform = "windows"
|
||||
require.NoError(t, ds.UpdateHost(ctx, host))
|
||||
deviceID := windowsEnroll(t, ds, host)
|
||||
|
||||
// create a Windows profile and set it as pending install on the host
|
||||
profUUID := InsertWindowsProfileForTest(t, ds, 0)
|
||||
|
||||
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
|
||||
_, err := q.ExecContext(ctx, `INSERT INTO host_mdm_windows_profiles
|
||||
(host_uuid, status, operation_type, command_uuid, profile_name, checksum, profile_uuid)
|
||||
VALUES (?, ?, ?, ?, ?, UNHEX(MD5('test')), ?)`,
|
||||
host.UUID, fleet.MDMDeliveryPending, fleet.MDMOperationTypeInstall, uuid.NewString(), "TestProfile", profUUID)
|
||||
return err
|
||||
})
|
||||
|
||||
// verify the profile row exists
|
||||
winProfs, err := ds.GetHostMDMWindowsProfiles(ctx, host.UUID)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, winProfs, 1)
|
||||
require.Equal(t, profUUID, winProfs[0].ProfileUUID)
|
||||
|
||||
// unenroll the device
|
||||
err = ds.MDMWindowsDeleteEnrolledDeviceWithDeviceID(ctx, deviceID)
|
||||
require.NoError(t, err)
|
||||
|
||||
// verify the profile row has been cleaned up
|
||||
winProfs, err = ds.GetHostMDMWindowsProfiles(ctx, host.UUID)
|
||||
require.NoError(t, err)
|
||||
require.Empty(t, winProfs)
|
||||
}
|
||||
|
||||
@@ -420,6 +420,9 @@ type Datastore interface {
|
||||
CleanupHostMDMCommands(ctx context.Context) error
|
||||
// CleanupHostMDMAppleProfiles removes abandoned host MDM Apple profiles entries.
|
||||
CleanupHostMDMAppleProfiles(ctx context.Context) error
|
||||
// CleanupAllHostMDMProfilesForPlatform deletes all host MDM profile rows for the given platform.
|
||||
// Used when MDM is toggled off globally to prevent stale pending profiles from persisting.
|
||||
CleanupAllHostMDMProfilesForPlatform(ctx context.Context, platform string) error
|
||||
|
||||
// CleanupStaleNanoRefetchCommands deletes up to 3 nano_enrollment_queue and
|
||||
// their corresponding nano_command_results entries for the given enrollment ID
|
||||
|
||||
@@ -325,6 +325,8 @@ type CleanupHostMDMCommandsFunc func(ctx context.Context) error
|
||||
|
||||
type CleanupHostMDMAppleProfilesFunc func(ctx context.Context) error
|
||||
|
||||
type CleanupAllHostMDMProfilesForPlatformFunc func(ctx context.Context, platform string) error
|
||||
|
||||
type CleanupStaleNanoRefetchCommandsFunc func(ctx context.Context, enrollmentID string, commandUUIDPrefix string, currentCommandUUID string) error
|
||||
|
||||
type CleanupOrphanedNanoRefetchCommandsFunc func(ctx context.Context) error
|
||||
@@ -2319,6 +2321,9 @@ type DataStore struct {
|
||||
CleanupHostMDMAppleProfilesFunc CleanupHostMDMAppleProfilesFunc
|
||||
CleanupHostMDMAppleProfilesFuncInvoked bool
|
||||
|
||||
CleanupAllHostMDMProfilesForPlatformFunc CleanupAllHostMDMProfilesForPlatformFunc
|
||||
CleanupAllHostMDMProfilesForPlatformFuncInvoked bool
|
||||
|
||||
CleanupStaleNanoRefetchCommandsFunc CleanupStaleNanoRefetchCommandsFunc
|
||||
CleanupStaleNanoRefetchCommandsFuncInvoked bool
|
||||
|
||||
@@ -5689,6 +5694,13 @@ func (s *DataStore) CleanupHostMDMAppleProfiles(ctx context.Context) error {
|
||||
return s.CleanupHostMDMAppleProfilesFunc(ctx)
|
||||
}
|
||||
|
||||
func (s *DataStore) CleanupAllHostMDMProfilesForPlatform(ctx context.Context, platform string) error {
|
||||
s.mu.Lock()
|
||||
s.CleanupAllHostMDMProfilesForPlatformFuncInvoked = true
|
||||
s.mu.Unlock()
|
||||
return s.CleanupAllHostMDMProfilesForPlatformFunc(ctx, platform)
|
||||
}
|
||||
|
||||
func (s *DataStore) CleanupStaleNanoRefetchCommands(ctx context.Context, enrollmentID string, commandUUIDPrefix string, currentCommandUUID string) error {
|
||||
s.mu.Lock()
|
||||
s.CleanupStaleNanoRefetchCommandsFuncInvoked = true
|
||||
|
||||
@@ -1136,6 +1136,11 @@ func (svc *Service) ModifyAppConfig(ctx context.Context, p []byte, applyOpts fle
|
||||
act = fleet.ActivityTypeEnabledWindowsMDM{}
|
||||
} else {
|
||||
act = fleet.ActivityTypeDisabledWindowsMDM{}
|
||||
|
||||
// Clean up all pending Windows MDM profile rows since hosts can no longer receive MDM commands.
|
||||
if err := svc.ds.CleanupAllHostMDMProfilesForPlatform(ctx, "windows"); err != nil {
|
||||
return nil, ctxerr.Wrap(ctx, err, "cleaning up Windows host MDM profiles")
|
||||
}
|
||||
}
|
||||
if err := svc.NewActivity(ctx, authz.UserFromContext(ctx), act); err != nil {
|
||||
return nil, ctxerr.Wrapf(ctx, err, "create activity %s", act.ActivityName())
|
||||
|
||||
@@ -3395,6 +3395,11 @@ func (svc *Service) DeleteMDMAppleAPNSCert(ctx context.Context) error {
|
||||
return ctxerr.Wrap(ctx, err, "saving app config")
|
||||
}
|
||||
|
||||
// Clean up all pending Apple MDM profile rows since hosts can no longer receive MDM commands.
|
||||
if err := svc.ds.CleanupAllHostMDMProfilesForPlatform(ctx, "darwin"); err != nil {
|
||||
return ctxerr.Wrap(ctx, err, "cleaning up Apple host MDM profiles")
|
||||
}
|
||||
|
||||
// If an install doesn't have a verification_at or verification_failed_at, then
|
||||
// mark it as failed
|
||||
if err := svc.ds.MarkAllPendingAppleVPPAndInHouseInstallsAsFailed(ctx, worker.AppleSoftwareJobName); err != nil {
|
||||
|
||||
@@ -150,6 +150,8 @@ func TestMDMAppleAuthorization(t *testing.T) {
|
||||
|
||||
ds.MarkAllPendingAppleVPPAndInHouseInstallsAsFailedFunc = func(ctx context.Context, jobName string) error { return nil }
|
||||
|
||||
ds.CleanupAllHostMDMProfilesForPlatformFunc = func(ctx context.Context, platform string) error { return nil }
|
||||
|
||||
// use a custom implementation of checkAuthErr as the service call will fail
|
||||
// with a not found error (given that MDM is not really configured) in case
|
||||
// of success, and the package-wide checkAuthErr requires no error.
|
||||
|
||||
Reference in New Issue
Block a user