From 3bebd7f347259a7ce44c3640e297701afb18084d Mon Sep 17 00:00:00 2001 From: Jahziel Villasana-Espinoza Date: Thu, 5 Sep 2024 10:07:44 -0400 Subject: [PATCH] fix: better UX when attempting to turn off MDM on an offline host (#21770) > Related issue: #20868 # 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`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/Committing-Changes.md#changes-files) for more information. - [x] Manual QA for all new/changed functionality --- changes/20868-turn-off-mdm | 1 + .../UnenrollMdmModal/UnenrollMdmModal.tsx | 8 ++- server/service/apple_mdm.go | 50 ++++--------------- server/service/integration_mdm_test.go | 4 +- 4 files changed, 19 insertions(+), 44 deletions(-) create mode 100644 changes/20868-turn-off-mdm diff --git a/changes/20868-turn-off-mdm b/changes/20868-turn-off-mdm new file mode 100644 index 0000000000..bfcd35d315 --- /dev/null +++ b/changes/20868-turn-off-mdm @@ -0,0 +1 @@ +- Improves the UX of turning off MDM on an offline host (endpoint doesn't error anymore) \ No newline at end of file diff --git a/frontend/pages/hosts/details/HostDetailsPage/modals/UnenrollMdmModal/UnenrollMdmModal.tsx b/frontend/pages/hosts/details/HostDetailsPage/modals/UnenrollMdmModal/UnenrollMdmModal.tsx index ef2135dc19..a254ea2fa4 100644 --- a/frontend/pages/hosts/details/HostDetailsPage/modals/UnenrollMdmModal/UnenrollMdmModal.tsx +++ b/frontend/pages/hosts/details/HostDetailsPage/modals/UnenrollMdmModal/UnenrollMdmModal.tsx @@ -25,11 +25,15 @@ const UnenrollMdmModal = ({ hostId, onClose }: IUnenrollMdmModalProps) => { setRequestState("unenrolling"); try { await mdmAPI.unenrollHostFromMdm(hostId, 5000); - renderFlash("success", "Successfully turned off MDM."); + renderFlash( + "success", + "Turning off MDM or will turn off when the host comes online." + ); onClose(); } catch (unenrollMdmError: unknown) { + renderFlash("error", "Couldn't turn off MDM. Please try again."); console.log(unenrollMdmError); - setRequestState("error"); + onClose(); } }; diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index f870d33b71..6460719268 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -1570,47 +1570,17 @@ func (svc *Service) EnqueueMDMAppleCommandRemoveEnrollmentProfile(ctx context.Co return ctxerr.Wrap(ctx, err, "logging activity for mdm apple remove profile command") } - return svc.pollResultMDMAppleCommandRemoveEnrollmentProfile(ctx, cmdUUID, h.UUID, info.Platform) -} - -func (svc *Service) pollResultMDMAppleCommandRemoveEnrollmentProfile(ctx context.Context, cmdUUID string, deviceID string, platform string) error { - ctx, cancelFn := context.WithDeadline(ctx, time.Now().Add(5*time.Second)) - ticker := time.NewTicker(300 * time.Millisecond) - defer func() { - ticker.Stop() - cancelFn() - }() - - for { - select { - case <-ctx.Done(): - // time out after 5 seconds - return fleet.MDMAppleCommandTimeoutError{} - case <-ticker.C: - nanoEnroll, err := svc.ds.GetNanoMDMEnrollment(ctx, deviceID) - if err != nil { - level.Error(svc.logger).Log("err", "get nanomdm enrollment status", "details", err, "id", deviceID, "command_uuid", cmdUUID) - return err - } - if nanoEnroll != nil && nanoEnroll.Enabled { - // check again on the next tick - continue - } - // success, mdm enrollment is no longer enabled for the device - level.Info(svc.logger).Log("msg", "mdm disabled for device", "id", deviceID, "command_uuid", cmdUUID) - - mdmLifecycle := mdmlifecycle.New(svc.ds, svc.logger) - err = mdmLifecycle.Do(ctx, mdmlifecycle.HostOptions{ - Action: mdmlifecycle.HostActionTurnOff, - Platform: platform, - UUID: deviceID, - }) - if err != nil { - return err - } - return nil - } + mdmLifecycle := mdmlifecycle.New(svc.ds, svc.logger) + err = mdmLifecycle.Do(ctx, mdmlifecycle.HostOptions{ + Action: mdmlifecycle.HostActionTurnOff, + Platform: info.Platform, + UUID: h.UUID, + }) + if err != nil { + return ctxerr.Wrap(ctx, err, "running turn off action in mdm lifecycle") } + + return nil } type mdmAppleGetInstallerRequest struct { diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index 7a1dcfd9f1..e83e919247 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -1378,8 +1378,8 @@ func (s *integrationMDMTestSuite) TestMDMAppleUnenroll() { // 3 profiles added + 1 profile with fleetd configuration + 1 root CA config require.Len(t, *hostResp.Host.MDM.Profiles, 5) - // try to unenroll the host, fails since the host doesn't respond - s.Do("DELETE", fmt.Sprintf("/api/latest/fleet/hosts/%d/mdm", h.ID), nil, http.StatusGatewayTimeout) + // returns success, but this is effectively a no-op because the host isn't enrolled yet. + s.Do("DELETE", fmt.Sprintf("/api/latest/fleet/hosts/%d/mdm", h.ID), nil, http.StatusOK) // we're going to modify this mock, make sure we restore its default originalPushMock := s.pushProvider.PushFunc