Skip to content

Commit

Permalink
Add fallback checks for MDM unenrollment during migration flow (#19860)
Browse files Browse the repository at this point in the history
Follow up to address additional issues uncovered during QA of #19512
  • Loading branch information
gillespi314 committed Jun 18, 2024
1 parent 2641d50 commit 9f0404f
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 13 deletions.
47 changes: 36 additions & 11 deletions orbit/pkg/useraction/mdm_migration_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"text/template"
"time"

"github.com/fleetdm/fleet/v4/orbit/pkg/profiles"
"github.com/fleetdm/fleet/v4/pkg/file"
"github.com/fleetdm/fleet/v4/pkg/retry"
"github.com/rs/zerolog/log"
Expand Down Expand Up @@ -190,10 +191,13 @@ type swiftDialogMDMMigrator struct {
lastShownMu sync.RWMutex
showCh chan struct{}

// testEnrollmentCheckFn is used in tests to mock the call to verify
// testEnrollmentCheckFileFn is used in tests to mock the call to verify
// the enrollment status of the host
testEnrollmentCheckFn func() (bool, error)
unenrollmentRetryInterval time.Duration
testEnrollmentCheckFileFn func() (bool, error)
// testEnrollmentCheckStatusFn is used in tests to mock the call to verify
// the enrollment status of the host
testEnrollmentCheckStatusFn func() (bool, string, error)
unenrollmentRetryInterval time.Duration
}

/**
Expand Down Expand Up @@ -273,21 +277,42 @@ func (m *swiftDialogMDMMigrator) renderError() (chan swiftDialogExitCode, chan e
// unenroll, an error is returned.
func (m *swiftDialogMDMMigrator) waitForUnenrollment() error {
maxRetries := int(mdmUnenrollmentTotalWaitTime.Seconds() / m.unenrollmentRetryInterval.Seconds())
fn := m.testEnrollmentCheckFn
if fn == nil {
fn = func() (bool, error) {
checkFileFn := m.testEnrollmentCheckFileFn
if checkFileFn == nil {
checkFileFn = func() (bool, error) {
return file.Exists(mdmEnrollmentFile)
}
}
checkStatusFn := m.testEnrollmentCheckStatusFn
if checkStatusFn == nil {
checkStatusFn = func() (bool, string, error) {
return profiles.IsEnrolledInMDM()
}
}
return retry.Do(func() error {
enrolled, err := fn()
if err != nil {
log.Error().Err(err).Msgf("checking enrollment status in migration modal, will retry in %s", m.unenrollmentRetryInterval)
return err
var unenrolled bool

fileExists, fileErr := checkFileFn()
if fileErr != nil {
log.Error().Err(fileErr).Msg("checking for existence of cloudConfigProfileInstalled in migration modal")
} else if fileExists {
log.Info().Msg("checking for existence of cloudConfigProfileInstalled in migration modal: found")
} else {
log.Info().Msg("checking for existence of cloudConfigProfileInstalled in migration modal: not found")
unenrolled = true
}

statusEnrolled, serverURL, statusErr := checkStatusFn()
if statusErr != nil {
log.Error().Err(statusErr).Msgf("checking profiles status in migration modal")
} else if statusEnrolled {
log.Info().Msgf("checking profiles status in migration modal: enrolled to %s", serverURL)
} else {
log.Info().Msg("checking profiles status in migration modal: not enrolled")
unenrolled = true
}

if enrolled {
if !unenrolled {
log.Info().Msgf("device is still enrolled, waiting %s", m.unenrollmentRetryInterval)
return errors.New("host didn't unenroll from MDM")
}
Expand Down
19 changes: 18 additions & 1 deletion orbit/pkg/useraction/mdm_migration_darwin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,18 @@ func TestWaitForUnenrollment(t *testing.T) {
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
tries := 0
m.testEnrollmentCheckFn = func() (bool, error) {
m.testEnrollmentCheckFileFn = func() (bool, error) {
if tries >= c.unenrollAfterNTries {
return false, c.enrollErr
}
tries++
return true, c.enrollErr
}

m.testEnrollmentCheckStatusFn = func() (bool, string, error) {
return true, "example.com", nil
}

outErr := m.waitForUnenrollment()
if c.wantErr {
require.Error(t, outErr)
Expand All @@ -56,4 +60,17 @@ func TestWaitForUnenrollment(t *testing.T) {
}
})
}

t.Run("fallback to enrollment check file", func(t *testing.T) {
m.testEnrollmentCheckFileFn = func() (bool, error) {
return true, nil
}

m.testEnrollmentCheckStatusFn = func() (bool, string, error) {
return false, "", nil
}

outErr := m.waitForUnenrollment()
require.NoError(t, outErr)
})
}
9 changes: 8 additions & 1 deletion server/fleet/hosts.go
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,14 @@ func (h *Host) IsEligibleForDEPMigration(isConnectedToFleetMDM bool) bool {
func (h *Host) NeedsDEPEnrollment(isConnectedToFleetMDM bool) bool {
return h.MDMInfo != nil &&
!h.MDMInfo.Enrolled &&
!isConnectedToFleetMDM &&
// as a special case for migration with user interaction, we
// also check the information stored in host_mdm, and assume
// the host needs migration if it's not Fleet
//
// this is because we can't always rely on nano setting
// `nano_enrollment.active = 1` since sometimes Fleet won't get
// the checkout message from the host.
(!isConnectedToFleetMDM || h.MDMInfo.Name != WellKnownMDMFleet) &&
h.IsDEPAssignedToFleet()
}

Expand Down

0 comments on commit 9f0404f

Please sign in to comment.