diff --git a/changes/40881-fix-stuck-setup-experience-with-scep-renewal b/changes/40881-fix-stuck-setup-experience-with-scep-renewal new file mode 100644 index 0000000000..3fae5a0b76 --- /dev/null +++ b/changes/40881-fix-stuck-setup-experience-with-scep-renewal @@ -0,0 +1 @@ +- Fixed an issue where Apple setup experience could get stuck, if the device was in the middle of a SCEP renewal, and then re-enrolled. \ No newline at end of file diff --git a/pkg/mdm/mdmtest/apple.go b/pkg/mdm/mdmtest/apple.go index 77b9e906b4..8e51fe9499 100644 --- a/pkg/mdm/mdmtest/apple.go +++ b/pkg/mdm/mdmtest/apple.go @@ -302,6 +302,10 @@ func (c *TestAppleMDMClient) SetDEPToken(tok string) { // profile from the Fleet server and then runs the SCEP enrollment, Authenticate and TokenUpdate // steps. func (c *TestAppleMDMClient) Enroll() error { + return c.enrollDevice(true) +} + +func (c *TestAppleMDMClient) enrollDevice(awaitingConfiguration bool) error { switch { case c.fetchEnrollmentProfileFromDesktop: if err := c.fetchEnrollmentProfileFromDesktopURL(); err != nil { @@ -346,12 +350,17 @@ func (c *TestAppleMDMClient) Enroll() error { if err := c.Authenticate(); err != nil { return fmt.Errorf("authenticate: %w", err) } - if err := c.TokenUpdate(true); err != nil { + if err := c.TokenUpdate(awaitingConfiguration); err != nil { return fmt.Errorf("token update: %w", err) } return nil } +// Re-enroll runs the MDM enroll protocol on the simulated device, but TokenUpdate is not set as AwaitingConfiguration +func (c *TestAppleMDMClient) Reenroll() error { + return c.enrollDevice(false) +} + func (c *TestAppleMDMClient) UserEnroll() error { c.UserUUID = strings.ToUpper(uuid.New().String()) c.Username = "fleetie" + randStr(5) diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index 1e244baf6f..84960476c3 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -3452,12 +3452,21 @@ func (svc *MDMAppleCheckinAndCommandService) TokenUpdate(r *mdm.Request, m *mdm. // much more difficult to reason about the state of the host. We should try instead // to centralize the flow control in the lifecycle methods. if info.SCEPRenewalInProgress { - svc.logger.InfoContext(r.Context, "token update received for a SCEP renewal in process, cleaning SCEP refs", "host_uuid", r.ID) + svc.logger.InfoContext(r.Context, "token update received with known SCEP renewal in process, cleaning SCEP refs", "host_uuid", r.ID) if err := svc.ds.CleanSCEPRenewRefs(r.Context, r.ID); err != nil { return ctxerr.Wrap(r.Context, err, "cleaning SCEP refs") } - svc.logger.InfoContext(r.Context, "cleaned SCEP refs, skipping setup experience and mdm lifecycle turn on action", "host_uuid", r.ID) - return nil + + if !m.AwaitingConfiguration { + // Normal SCEP renewal - device is NOT at Setup Assistant. Clean refs and short-circuit. + svc.logger.InfoContext(r.Context, "cleaned SCEP refs, skipping setup experience and mdm lifecycle turn on action", "host_uuid", r.ID) + return nil + } + + // Device is awaiting configuration (wiped DEP device re-enrolling). The pending SCEP + // renewal was from the previous enrollment. Continue the normal enrollment flow so + // the device gets released from the setup assistant. + svc.logger.InfoContext(r.Context, "continuing with token update, due to awaiting configuration from new enrollment", "host_uuid", r.ID) } var hasSetupExpItems bool diff --git a/server/service/apple_mdm_test.go b/server/service/apple_mdm_test.go index 9c1fe79f62..3686b532df 100644 --- a/server/service/apple_mdm_test.go +++ b/server/service/apple_mdm_test.go @@ -6896,3 +6896,141 @@ func TestToValidSemVer(t *testing.T) { } } } + +func TestMDMTokenUpdateSCEPRenewal(t *testing.T) { + ctx := license.NewContext(context.Background(), &fleet.LicenseInfo{Tier: fleet.TierPremium}) + ds := new(mock.Store) + mdmStorage := &mdmmock.MDMAppleStore{} + logger := slog.New(slog.NewJSONHandler(os.Stdout, nil)) + pushFactory, _ := newMockAPNSPushProviderFactory() + pusher := nanomdm_pushsvc.New( + mdmStorage, + mdmStorage, + pushFactory, + NewNanoMDMLogger(logger), + ) + cmdr := apple_mdm.NewMDMAppleCommander(mdmStorage, pusher) + uuid, serial, model, wantTeamID := "ABC-DEF-GHI", "XYZABC", "MacBookPro 16,1", uint(12) + + t.Run("awaiting configuration continues enrollment", func(t *testing.T) { + // When a host re-enrolls via DEP (AwaitingConfiguration=true) while a + // SCEP renewal is pending, the handler should clear SCEP refs and + // continue with the normal enrollment flow (not short-circuit). + + var newActivityFuncInvoked bool + mdmLifecycle := mdmlifecycle.New(ds, logger, func(_ context.Context, _ *fleet.User, activity fleet.ActivityDetails) error { + newActivityFuncInvoked = true + _, ok := activity.(*fleet.ActivityTypeMDMEnrolled) + require.True(t, ok) + return nil + }) + svc := MDMAppleCheckinAndCommandService{ + ds: ds, + mdmLifecycle: mdmLifecycle, + commander: cmdr, + logger: logger, + } + scepRenewalInProgress := true + ds.GetHostMDMCheckinInfoFunc = func(ct context.Context, hostUUID string) (*fleet.HostMDMCheckinInfo, error) { + return &fleet.HostMDMCheckinInfo{ + HostID: 1337, + HardwareSerial: serial, + DisplayName: model, + InstalledFromDEP: true, + TeamID: wantTeamID, + DEPAssignedToFleet: true, + SCEPRenewalInProgress: scepRenewalInProgress, + Platform: "darwin", + }, nil + } + ds.CleanSCEPRenewRefsFunc = func(ctx context.Context, hostUUID string) error { + require.Equal(t, uuid, hostUUID) + scepRenewalInProgress = false + return nil + } + ds.EnqueueSetupExperienceItemsFunc = func(ctx context.Context, hostPlatformLike string, hostUUID string, teamID uint) (bool, error) { + require.Equal(t, "darwin", hostPlatformLike) + require.Equal(t, uuid, hostUUID) + require.Equal(t, wantTeamID, teamID) + return true, nil + } + ds.GetNanoMDMEnrollmentFunc = func(ctx context.Context, hostUUID string) (*fleet.NanoEnrollment, error) { + return &fleet.NanoEnrollment{Enabled: true, Type: "Device", TokenUpdateTally: 1}, nil + } + ds.AppConfigFunc = func(context.Context) (*fleet.AppConfig, error) { + return &fleet.AppConfig{}, nil + } + ds.GetMDMIdPAccountByHostUUIDFunc = func(ctx context.Context, hostUUID string) (*fleet.MDMIdPAccount, error) { + return nil, nil + } + ds.NewJobFunc = func(ctx context.Context, j *fleet.Job) (*fleet.Job, error) { + return j, nil + } + + err := svc.TokenUpdate( + &mdm.Request{Context: ctx, EnrollID: &mdm.EnrollID{ID: uuid}}, + &mdm.TokenUpdate{ + Enrollment: mdm.Enrollment{ + AwaitingConfiguration: true, + UDID: uuid, + }, + }, + ) + require.NoError(t, err) + require.True(t, ds.CleanSCEPRenewRefsFuncInvoked) + require.True(t, ds.EnqueueSetupExperienceItemsFuncInvoked) + require.True(t, ds.NewJobFuncInvoked) + require.True(t, newActivityFuncInvoked) + }) + + t.Run("not awaiting configuration short-circuits", func(t *testing.T) { + // When a SCEP renewal is in progress but the host is NOT awaiting + // configuration (normal renewal), the handler should clean SCEP refs + // and return early without enqueueing setup experience or lifecycle. + var newActivityFuncInvoked bool + mdmLifecycle := mdmlifecycle.New(ds, logger, func(_ context.Context, _ *fleet.User, _ fleet.ActivityDetails) error { + newActivityFuncInvoked = true + return nil + }) + svc := MDMAppleCheckinAndCommandService{ + ds: ds, + mdmLifecycle: mdmLifecycle, + commander: cmdr, + logger: logger, + } + + ds.GetHostMDMCheckinInfoFunc = func(ct context.Context, hostUUID string) (*fleet.HostMDMCheckinInfo, error) { + return &fleet.HostMDMCheckinInfo{ + HostID: 1337, + HardwareSerial: serial, + DisplayName: model, + InstalledFromDEP: true, + TeamID: wantTeamID, + DEPAssignedToFleet: true, + SCEPRenewalInProgress: true, + Platform: "darwin", + }, nil + } + ds.CleanSCEPRenewRefsFunc = func(ctx context.Context, hostUUID string) error { + require.Equal(t, uuid, hostUUID) + return nil + } + ds.CleanSCEPRenewRefsFuncInvoked = false + ds.EnqueueSetupExperienceItemsFuncInvoked = false + ds.NewJobFuncInvoked = false + + err := svc.TokenUpdate( + &mdm.Request{Context: ctx, EnrollID: &mdm.EnrollID{ID: uuid}}, + &mdm.TokenUpdate{ + Enrollment: mdm.Enrollment{ + UDID: uuid, + }, + }, + ) + require.NoError(t, err) + require.True(t, ds.CleanSCEPRenewRefsFuncInvoked) + require.False(t, ds.EnqueueSetupExperienceItemsFuncInvoked) + require.False(t, ds.NewJobFuncInvoked) + require.False(t, newActivityFuncInvoked) + }) +} diff --git a/server/service/integration_mdm_lifecycle_test.go b/server/service/integration_mdm_lifecycle_test.go index 52073e3d76..f888d56899 100644 --- a/server/service/integration_mdm_lifecycle_test.go +++ b/server/service/integration_mdm_lifecycle_test.go @@ -1042,11 +1042,11 @@ func (s *integrationMDMTestSuite) TestLifecycleSCEPCertExpiration() { require.Nil(t, cmd) // devices renew their SCEP cert by re-enrolling. - require.NoError(t, manualEnrolledDevice.Enroll()) - require.NoError(t, automaticEnrolledDevice.Enroll()) - require.NoError(t, automaticEnrolledDeviceWithRef.Enroll()) - require.NoError(t, migratedDevice.Enroll()) - require.NoError(t, iPhoneMdmDevice.Enroll()) + require.NoError(t, manualEnrolledDevice.Reenroll()) + require.NoError(t, automaticEnrolledDevice.Reenroll()) + require.NoError(t, automaticEnrolledDeviceWithRef.Reenroll()) + require.NoError(t, migratedDevice.Reenroll()) + require.NoError(t, iPhoneMdmDevice.Reenroll()) // no new commands are enqueued right after enrollment cmd, err = manualEnrolledDevice.Idle()