From 998e1dfb6bcf48503e4e551cd5213a6cabcab4ba Mon Sep 17 00:00:00 2001 From: Roberto Dip Date: Mon, 14 Aug 2023 19:21:06 -0300 Subject: [PATCH] fix issues when MDM info is empty during migration (#13320) for #13319 --- orbit/pkg/profiles/profiles_darwin.go | 22 ------ orbit/pkg/profiles/profiles_darwin_test.go | 69 ------------------- orbit/pkg/profiles/profiles_notdarwin.go | 4 -- orbit/pkg/profiles/profiles_notdarwin_test.go | 6 -- orbit/pkg/update/notifications.go | 13 ++-- orbit/pkg/update/notifications_test.go | 8 +-- server/fleet/hosts.go | 2 +- 7 files changed, 11 insertions(+), 113 deletions(-) diff --git a/orbit/pkg/profiles/profiles_darwin.go b/orbit/pkg/profiles/profiles_darwin.go index cc2e19b8cc..22490e4638 100644 --- a/orbit/pkg/profiles/profiles_darwin.go +++ b/orbit/pkg/profiles/profiles_darwin.go @@ -11,7 +11,6 @@ import ( "os/exec" "strings" - "github.com/fleetdm/fleet/v4/pkg/fleethttp" "github.com/fleetdm/fleet/v4/server/fleet" "github.com/fleetdm/fleet/v4/server/mdm/apple/mobileconfig" ) @@ -57,27 +56,6 @@ var execScript = func(script string) (*bytes.Buffer, error) { return &outBuf, nil } -// IsEnrolledIntoMatchingURL runs the `profiles` command to get the current MDM -// enrollment information and reports if the hostname of the MDM server -// supervising the device matches the hostname of the provided URL. -func IsEnrolledIntoMatchingURL(serverURL string) (bool, error) { - enrolled, currentURL, err := IsEnrolledInMDM() - if err != nil { - return false, fmt.Errorf("getting enrollment info: %w", err) - } - - if !enrolled { - return false, nil - } - - matches, err := fleethttp.HostnamesMatch(serverURL, currentURL) - if err != nil { - return false, fmt.Errorf("comparing URLs: %w", err) - } - - return matches, nil -} - // IsEnrolledInMDM runs the `profiles` command to get the current MDM // enrollment information and reports if the host is enrolled, and the URL of // the MDM server (if enrolled) diff --git a/orbit/pkg/profiles/profiles_darwin_test.go b/orbit/pkg/profiles/profiles_darwin_test.go index 086cebb57a..5d6668d162 100644 --- a/orbit/pkg/profiles/profiles_darwin_test.go +++ b/orbit/pkg/profiles/profiles_darwin_test.go @@ -69,75 +69,6 @@ func TestGetFleetdConfig(t *testing.T) { } } -func TestIsEnrolledIntoMatchingURL(t *testing.T) { - fleetURL := "https://valid.com" - cases := []struct { - cmdOut *string - cmdErr error - wantOut bool - wantErr bool - }{ - {nil, errors.New("test error"), false, true}, - {ptr.String(""), nil, false, false}, - {ptr.String(` -Enrolled via DEP: No -MDM enrollment: No - `), nil, false, false}, - { - ptr.String(` -Enrolled via DEP: Yes -MDM enrollment: Yes -MDM server: https://test.example.com - `), - nil, - false, - false, - }, - { - ptr.String(` -Enrolled via DEP: Yes -MDM enrollment: Yes -MDM server / https://test.example.com - `), - nil, - false, - false, - }, - { - ptr.String(` -Enrolled via DEP: Yes -MDM enrollment: Yes -MDM server: https://valid.com/mdm/apple/mdm - `), - nil, - true, - false, - }, - } - - origCmd := getMDMInfoFromProfilesCmd - t.Cleanup(func() { getMDMInfoFromProfilesCmd = origCmd }) - for _, c := range cases { - getMDMInfoFromProfilesCmd = func() ([]byte, error) { - if c.cmdOut == nil { - return nil, c.cmdErr - } - - var buf bytes.Buffer - buf.WriteString(*c.cmdOut) - return []byte(*c.cmdOut), nil - } - - out, err := IsEnrolledIntoMatchingURL(fleetURL) - if c.wantErr { - require.Error(t, err) - } else { - require.NoError(t, err) - } - require.Equal(t, c.wantOut, out) - } -} - func TestIsEnrolledInMDM(t *testing.T) { cases := []struct { cmdOut *string diff --git a/orbit/pkg/profiles/profiles_notdarwin.go b/orbit/pkg/profiles/profiles_notdarwin.go index 803532cfe8..1e2d88e35c 100644 --- a/orbit/pkg/profiles/profiles_notdarwin.go +++ b/orbit/pkg/profiles/profiles_notdarwin.go @@ -12,10 +12,6 @@ func IsEnrolledInMDM() (bool, string, error) { return false, "", ErrNotImplemented } -func IsEnrolledIntoMatchingURL(u string) (bool, error) { - return false, ErrNotImplemented -} - func CheckAssignedEnrollmentProfile(expectedURL string) error { return ErrNotImplemented } diff --git a/orbit/pkg/profiles/profiles_notdarwin_test.go b/orbit/pkg/profiles/profiles_notdarwin_test.go index 175a74fbb0..db50dc9933 100644 --- a/orbit/pkg/profiles/profiles_notdarwin_test.go +++ b/orbit/pkg/profiles/profiles_notdarwin_test.go @@ -14,12 +14,6 @@ func TestGetFleetdConfig(t *testing.T) { require.Nil(t, config) } -func TestIsEnrolledIntoMatchingURL(t *testing.T) { - enrolled, err := IsEnrolledIntoMatchingURL("https://test.example.com") - require.ErrorIs(t, ErrNotImplemented, err) - require.False(t, enrolled) -} - func TestIsEnrolledInMDM(t *testing.T) { enrolled, serverURL, err := IsEnrolledInMDM() require.ErrorIs(t, ErrNotImplemented, err) diff --git a/orbit/pkg/update/notifications.go b/orbit/pkg/update/notifications.go index 7510f50b8a..bc4913d895 100644 --- a/orbit/pkg/update/notifications.go +++ b/orbit/pkg/update/notifications.go @@ -12,7 +12,7 @@ import ( type runCmdFunc func() error -type checkEnrollmentFunc func(url string) (bool, error) +type checkEnrollmentFunc func() (bool, string, error) type checkAssignedEnrollmentProfileFunc func(url string) error @@ -71,20 +71,19 @@ func (h *renewEnrollmentProfileConfigFetcher) GetConfig() (*fleet.OrbitConfig, e // See https://github.com/fleetdm/fleet/pull/9409#discussion_r1084382455 if time.Since(h.lastRun) > h.Frequency { // we perform this check locally on the client too to avoid showing the - // dialog if the client has already migrated but the Fleet server - // doesn't know about this state yet. + // dialog if the client is enrolled to an MDM server. enrollFn := h.checkEnrollmentFn if enrollFn == nil { - enrollFn = profiles.IsEnrolledIntoMatchingURL + enrollFn = profiles.IsEnrolledInMDM } - enrolled, err := enrollFn(h.fleetURL) + enrolled, mdmServerURL, err := enrollFn() if err != nil { log.Error().Err(err).Msg("fetching enrollment status") return cfg, nil } if enrolled { - log.Info().Msg("a request to renew the enrollment profile was processed but not executed because the host is already enrolled into Fleet.") - h.lastRun = time.Now() + log.Info().Msgf("a request to renew the enrollment profile was processed but not executed because the host is enrolled into an MDM server with URL: %s", mdmServerURL) + h.lastRun = time.Now().Add(-h.Frequency).Add(2 * time.Minute) return cfg, nil } diff --git a/orbit/pkg/update/notifications_test.go b/orbit/pkg/update/notifications_test.go index 0cfc130ae0..efdeaccc0f 100644 --- a/orbit/pkg/update/notifications_test.go +++ b/orbit/pkg/update/notifications_test.go @@ -50,8 +50,8 @@ func TestRenewEnrollmentProfile(t *testing.T) { cmdGotCalled = true return c.cmdErr }, - checkEnrollmentFn: func(url string) (bool, error) { - return false, nil + checkEnrollmentFn: func() (bool, string, error) { + return false, "", nil }, checkAssignedEnrollmentProfileFn: func(url string) error { depAssignedCheckGotCalled = true @@ -92,9 +92,9 @@ func TestRenewEnrollmentProfilePrevented(t *testing.T) { cmdCallCount++ // no need for sync, single-threaded call of this func is guaranteed by the fetcher's mutex return nil }, - checkEnrollmentFn: func(url string) (bool, error) { + checkEnrollmentFn: func() (bool, string, error) { <-chProceed // will be unblocked only when allowed - return isEnrolled, nil + return isEnrolled, "", nil }, checkAssignedEnrollmentProfileFn: func(url string) error { <-chProceed // will be unblocked only when allowed diff --git a/server/fleet/hosts.go b/server/fleet/hosts.go index bfa9d5b8f0..37a26b1a3e 100644 --- a/server/fleet/hosts.go +++ b/server/fleet/hosts.go @@ -549,7 +549,7 @@ func (h *Host) IsEligibleForDEPMigration() bool { // NeedsDEPEnrollment returns true if the host should be DEP enrolled into // fleet but it's currently unenrolled. func (h *Host) NeedsDEPEnrollment() bool { - return !h.MDMInfo.IsDEPFleetEnrolled() && + return h.MDMInfo != nil && !h.MDMInfo.IsDEPFleetEnrolled() && !h.MDMInfo.IsManualFleetEnrolled() && !h.MDMInfo.IsEnrolledInThirdPartyMDM() && h.IsDEPAssignedToFleet()