diff --git a/cmd/fleetctl/fleetctl/apply_test.go b/cmd/fleetctl/fleetctl/apply_test.go index 917b785403..179141a7c5 100644 --- a/cmd/fleetctl/fleetctl/apply_test.go +++ b/cmd/fleetctl/fleetctl/apply_test.go @@ -2780,6 +2780,28 @@ spec: assert.YAMLEq(t, expectedWithWindowsRequire, RunAppForTest(t, []string{"get", "teams", "--yaml"})) }) + t.Run("require_all_software_windows rejected when Windows MDM not configured", func(t *testing.T) { + // Spec invariant: setting `require_all_software_windows=true` while + // `MDM.WindowsEnabledAndConfigured=false` MUST be rejected. setupServer's default appConfig leaves + // WindowsEnabledAndConfigured at the zero value (false), which is the precondition this test needs. + ds := setupServer(t, true) + + windowsRequireSpec := ` +apiVersion: v1 +kind: fleet +spec: + team: + name: tm1 + mdm: + setup_experience: + require_all_software_windows: true +` + name := writeTmpYml(t, windowsRequireSpec) + RunAppCheckErr(t, []string{"apply", "-f", name}, "require_all_software_windows") + assert.False(t, ds.SaveTeamFuncInvoked, + "team must not be saved when require_all_software_windows is rejected") + }) + t.Run("new bootstrap package", func(t *testing.T) { cases := []struct { pkgName string diff --git a/server/datastore/mysql/microsoft_mdm.go b/server/datastore/mysql/microsoft_mdm.go index 2f1758c2b9..fee02238c3 100644 --- a/server/datastore/mysql/microsoft_mdm.go +++ b/server/datastore/mysql/microsoft_mdm.go @@ -133,6 +133,44 @@ func (ds *Datastore) MDMWindowsGetEnrolledDeviceWithHostUUID(ctx context.Context return &winMDMDevice, nil } +// HasWindowsSetupExperienceItemsForTeam returns true if any active Windows setup-experience software +// installers with install_during_setup=TRUE are configured for the given team. teamID=0 means "no team / +// global", matching the value EnqueueSetupExperienceItems passes in for hosts on no team. +func (ds *Datastore) HasWindowsSetupExperienceItemsForTeam(ctx context.Context, teamID uint) (bool, error) { + const stmt = ` +SELECT EXISTS ( + SELECT 1 FROM software_installers + WHERE platform = 'windows' + AND install_during_setup = TRUE + AND global_or_team_id = ? + AND is_active = TRUE +)` + var hasItems bool + if err := sqlx.GetContext(ctx, ds.reader(ctx), &hasItems, stmt, teamID); err != nil { + return false, ctxerr.Wrap(ctx, err, "check setup experience items configured") + } + return hasItems, nil +} + +// GetMDMWindowsAwaitingConfigurationByHostUUID returns the awaiting_configuration value for the Windows MDM +// enrollment of the host with the given UUID. Reader-backed; callers that need primary-routed semantics must wrap +// the context with ctxdb.RequirePrimary. +func (ds *Datastore) GetMDMWindowsAwaitingConfigurationByHostUUID(ctx context.Context, hostUUID string) (fleet.WindowsMDMAwaitingConfiguration, error) { + const stmt = `SELECT awaiting_configuration + FROM mdm_windows_enrollments + WHERE host_uuid = ? + ORDER BY created_at DESC, id DESC + LIMIT 1` + var awaiting fleet.WindowsMDMAwaitingConfiguration + if err := sqlx.GetContext(ctx, ds.reader(ctx), &awaiting, stmt, hostUUID); err != nil { + if errors.Is(err, sql.ErrNoRows) { + return 0, ctxerr.Wrap(ctx, notFound("MDMWindowsEnrolledDevice").WithMessage(hostUUID)) + } + return 0, ctxerr.Wrap(ctx, err, "get MDMWindowsAwaitingConfigurationByHostUUID") + } + return awaiting, nil +} + // MDMWindowsInsertEnrolledDevice inserts a new MDMWindowsEnrolledDevice in the // database. func (ds *Datastore) MDMWindowsInsertEnrolledDevice(ctx context.Context, device *fleet.MDMWindowsEnrolledDevice) error { @@ -209,6 +247,13 @@ func (ds *Datastore) MDMWindowsDeleteEnrolledDeviceOnReenrollment(ctx context.Co loadStmt = "SELECT host_uuid FROM mdm_windows_enrollments WHERE mdm_hardware_id = ? LIMIT 1" delActionsStmt = "DELETE FROM host_mdm_actions WHERE host_id = (SELECT id FROM hosts WHERE uuid = ? LIMIT 1)" delProfilesStmt = "DELETE FROM host_mdm_windows_profiles WHERE host_uuid = ?" + // setup_experience_status_results.host_uuid is keyed by fleet.HostUUIDForSetupExperience; for Windows that's the + // host's OsqueryHostID, NOT the Fleet host UUID stored on the MDM enrollment. Resolve via JOIN so we delete by + // whichever identifier matches (works for both shapes). + delSetupExpStmt = `DELETE ser FROM setup_experience_status_results ser + JOIN hosts h ON ser.host_uuid = h.osquery_host_id OR ser.host_uuid = h.uuid + WHERE h.uuid = ?` + delUpcomingStmt = `DELETE ua FROM upcoming_activities ua JOIN hosts h ON h.id = ua.host_id WHERE h.uuid = ?` ) return ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error { @@ -225,6 +270,14 @@ func (ds *Datastore) MDMWindowsDeleteEnrolledDeviceOnReenrollment(ctx context.Co if _, err := tx.ExecContext(ctx, delProfilesStmt, hostUUID.String); err != nil { return ctxerr.Wrap(ctx, err, "delete host_mdm_windows_profiles for host") } + // Clear setup experience results so they get re-enqueued on the new enrollment. + if _, err := tx.ExecContext(ctx, delSetupExpStmt, hostUUID.String); err != nil { + return ctxerr.Wrap(ctx, err, "delete setup_experience_status_results for host") + } + // Clear ALL stale upcoming activities (any activity_type) so they don't block new activities on re-enrollment. + if _, err := tx.ExecContext(ctx, delUpcomingStmt, hostUUID.String); err != nil { + return ctxerr.Wrap(ctx, err, "delete upcoming_activities for host") + } } case sql.ErrNoRows: @@ -439,6 +492,37 @@ func (ds *Datastore) MDMWindowsInsertCommandForHosts(ctx context.Context, hostUU }) } +// MDMWindowsInsertCommandsForHost atomically inserts a batch of Windows MDM commands targeting a single host +// (identified by host UUID or MDM device ID). All commands are inserted in one transaction: either every row +// is committed or none. Used by the ESP finalize path so the dropped-response retry safety net can't end up +// partially written on a transient DB error -- a partial write followed by a fresh-UUID retry would leave +// orphan rows in the queue. +// +// Returns notFound("MDMWindowsEnrolledDevice") if the identifier resolves to zero enrollments. Without this +// guard, mdmWindowsInsertCommandForEnrollmentIDsDB would still INSERT each row into windows_mdm_commands and +// return success while leaving the rows targeted at no host -- the ESP finalize would silently drop the +// retry safety net. +func (ds *Datastore) MDMWindowsInsertCommandsForHost(ctx context.Context, hostUUIDOrDeviceID string, cmds []*fleet.MDMWindowsCommand) error { + if len(cmds) == 0 { + return nil + } + return ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error { + enrollmentIDs, err := ds.getEnrollmentIDsByHostUUIDOrDeviceIDDB(ctx, tx, []string{hostUUIDOrDeviceID}) + if err != nil { + return ctxerr.Wrap(ctx, err, "fetching enrollment IDs for command queue") + } + if len(enrollmentIDs) == 0 { + return ctxerr.Wrap(ctx, notFound("MDMWindowsEnrolledDevice").WithName(hostUUIDOrDeviceID)) + } + for _, cmd := range cmds { + if err := ds.mdmWindowsInsertCommandForEnrollmentIDsDB(ctx, tx, enrollmentIDs, cmd); err != nil { + return err + } + } + return nil + }) +} + func (ds *Datastore) mdmWindowsInsertCommandForHostsDB(ctx context.Context, tx sqlx.ExtContext, hostUUIDsOrDeviceIDs []string, cmd *fleet.MDMWindowsCommand) error { // Resolve host UUIDs / device IDs to enrollment IDs using the general-purpose // lookup (supports both host_uuid and mdm_device_id via subquery). diff --git a/server/datastore/mysql/microsoft_mdm_test.go b/server/datastore/mysql/microsoft_mdm_test.go index f21bfb1895..9579f655fa 100644 --- a/server/datastore/mysql/microsoft_mdm_test.go +++ b/server/datastore/mysql/microsoft_mdm_test.go @@ -8,6 +8,8 @@ import ( "fmt" "slices" "strings" + "sync" + "sync/atomic" "testing" "time" @@ -59,6 +61,9 @@ func TestMDMWindows(t *testing.T) { {"TestEditProfileDeletesRemovedLocURIs", testEditProfileDeletesRemovedLocURIs}, {"TestBatchDeleteMultipleWindowsProfiles", testBatchDeleteMultipleWindowsProfiles}, {"TestMDMWindowsUnenrollCleansUpProfiles", testMDMWindowsUnenrollCleansUpProfiles}, + {"TestMDMWindowsAwaitingConfigurationCAS", testMDMWindowsAwaitingConfigurationCAS}, + {"TestMDMWindowsAwaitingConfigurationByHostUUID", testMDMWindowsAwaitingConfigurationByHostUUID}, + {"TestMDMWindowsHasSetupExperienceItems", testMDMWindowsHasSetupExperienceItems}, {"TestMDMWindowsProfilesToRemoveSkipsOrphanedHosts", testMDMWindowsProfilesToRemoveSkipsOrphanedHosts}, {"TestMDMWindowsInsertCommandSkipsUnenrolledHosts", testMDMWindowsInsertCommandSkipsUnenrolledHosts}, {"TestCleanupWindowsMDMCommandQueue", testCleanupWindowsMDMCommandQueue}, @@ -160,6 +165,91 @@ func testMDMWindowsEnrolledDevice(t *testing.T, ds *Datastore) { require.NoError(t, err) require.Equal(t, fleet.WindowsMDMAwaitingConfigurationNone, gotEnrolledDevice.AwaitingConfiguration) require.Nil(t, gotEnrolledDevice.AwaitingConfigurationAt) + + // Re-enrollment cleanup MUST cascade to host_mdm_windows_profiles, + // setup_experience_status_results, and upcoming_activities for the host + // UUID so a re-enrolled (re-Autopiloted, re-joined) device starts clean. + host := test.NewHost(t, ds, "win-cleanup", "10.0.0.99", "win-cleanup-key", "win-cleanup-uuid", time.Now()) + host.Platform = "windows" + require.NoError(t, ds.UpdateHost(ctx, host)) + + cleanupDevice := &fleet.MDMWindowsEnrolledDevice{ + MDMDeviceID: uuid.New().String(), + MDMHardwareID: uuid.New().String() + uuid.New().String(), + MDMDeviceState: microsoft_mdm.MDMDeviceStateEnrolled, + MDMDeviceType: "CIMClient_Windows", + MDMDeviceName: "DESKTOP-CLEANUP", + MDMEnrollType: "ProgrammaticEnrollment", + MDMEnrollProtoVersion: "5.0", + MDMEnrollClientVersion: "10.0.19045.2965", + MDMNotInOOBE: false, + HostUUID: host.UUID, + } + require.NoError(t, ds.MDMWindowsInsertEnrolledDevice(ctx, cleanupDevice)) + + // setup_experience_status_results.host_uuid is keyed by fleet.HostUUIDForSetupExperience; for Windows that's the + // host's OsqueryHostID, NOT host.UUID. Insert with the production-shape key so this test would catch a regression + // where cleanup deletes by host.UUID and silently misses real Windows rows. + require.NotNil(t, host.OsqueryHostID, "test host must have OsqueryHostID set") + seHostUUID := *host.OsqueryHostID + + profUUID := InsertWindowsProfileForTest(t, ds, 0) + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + if _, 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); err != nil { + return err + } + if _, err := q.ExecContext(ctx, `INSERT INTO setup_experience_status_results + (host_uuid, name, status) VALUES (?, ?, ?)`, + seHostUUID, "TestApp", fleet.SetupExperienceStatusPending); err != nil { + return err + } + _, err := q.ExecContext(ctx, `INSERT INTO upcoming_activities + (host_id, activity_type, execution_id, payload) VALUES (?, ?, ?, ?)`, + host.ID, "script", uuid.NewString(), `{}`) + return err + }) + + // Sanity-check pre-population. + var profCount, resultCount, activityCount int + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + if err := sqlx.GetContext(ctx, q, &profCount, + `SELECT COUNT(*) FROM host_mdm_windows_profiles WHERE host_uuid = ?`, host.UUID); err != nil { + return err + } + if err := sqlx.GetContext(ctx, q, &resultCount, + `SELECT COUNT(*) FROM setup_experience_status_results WHERE host_uuid = ?`, seHostUUID); err != nil { + return err + } + return sqlx.GetContext(ctx, q, &activityCount, + `SELECT COUNT(*) FROM upcoming_activities WHERE host_id = ?`, host.ID) + }) + require.Equal(t, 1, profCount) + require.Equal(t, 1, resultCount) + require.Equal(t, 1, activityCount) + + // Run the re-enrollment cleanup. + require.NoError(t, ds.MDMWindowsDeleteEnrolledDeviceOnReenrollment(ctx, cleanupDevice.MDMHardwareID)) + + // All three related tables must be cleaned for this host. + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + if err := sqlx.GetContext(ctx, q, &profCount, + `SELECT COUNT(*) FROM host_mdm_windows_profiles WHERE host_uuid = ?`, host.UUID); err != nil { + return err + } + if err := sqlx.GetContext(ctx, q, &resultCount, + `SELECT COUNT(*) FROM setup_experience_status_results WHERE host_uuid = ?`, seHostUUID); err != nil { + return err + } + return sqlx.GetContext(ctx, q, &activityCount, + `SELECT COUNT(*) FROM upcoming_activities WHERE host_id = ?`, host.ID) + }) + assert.Equal(t, 0, profCount, "host_mdm_windows_profiles must be cleaned on re-enrollment") + assert.Equal(t, 0, resultCount, + "setup_experience_status_results must be cleaned on re-enrollment, even when keyed by OsqueryHostID") + assert.Equal(t, 0, activityCount, "upcoming_activities must be cleaned on re-enrollment via JOIN on hosts.uuid") } func testMDMWindowsDiskEncryption(t *testing.T, ds *Datastore) { @@ -5529,3 +5619,461 @@ func testMDMWindowsProfilesSummaryEnumeration(t *testing.T, ds *Datastore) { "per-host membership mismatch for OSSettingsFilter=%s", filter) } } + +// windowsEnrollmentFixture describes a Windows MDM enrollment row for tests. The non-zero fields below are the +// only ones tests in this file vary; everything else gets sensible defaults via insertWindowsEnrolledDevice. +type windowsEnrollmentFixture struct { + mdmDeviceID string // defaulted to a fresh UUID if empty + deviceNameSuffix string // appended to "DESKTOP-" for MDMDeviceName; defaulted to "TEST" + hostUUID string // optional, links the enrollment to a host row + awaitingConfiguration fleet.WindowsMDMAwaitingConfiguration + awaitingAt *time.Time +} + +// insertWindowsEnrolledDevice inserts an MDM-enrollment row with sensible defaults for every field tests don't care +// about. Returns the mdm_device_id used (caller-supplied or generated). Use this in test setup so the +// 11-line struct literal doesn't duplicate across every subtest. +func insertWindowsEnrolledDevice(t *testing.T, ctx context.Context, ds *Datastore, f windowsEnrollmentFixture) string { + t.Helper() + if f.mdmDeviceID == "" { + f.mdmDeviceID = uuid.NewString() + } + if f.deviceNameSuffix == "" { + f.deviceNameSuffix = "TEST" + } + require.NoError(t, ds.MDMWindowsInsertEnrolledDevice(ctx, &fleet.MDMWindowsEnrolledDevice{ + MDMDeviceID: f.mdmDeviceID, + MDMHardwareID: uuid.NewString() + uuid.NewString(), + MDMDeviceState: microsoft_mdm.MDMDeviceStateEnrolled, + MDMDeviceType: "CIMClient_Windows", + MDMDeviceName: "DESKTOP-" + strings.ToUpper(f.deviceNameSuffix), + MDMEnrollType: "ProgrammaticEnrollment", + MDMEnrollProtoVersion: "5.0", + MDMEnrollClientVersion: "10.0.19045.2965", + MDMNotInOOBE: false, + HostUUID: f.hostUUID, + AwaitingConfiguration: f.awaitingConfiguration, + AwaitingConfigurationAt: f.awaitingAt, + })) + return f.mdmDeviceID +} + +// testMDMWindowsAwaitingConfigurationCAS verifies the compare-and-swap +// semantics of SetMDMWindowsAwaitingConfiguration: a mismatched expectFrom +// must not transition the row, and concurrent callers must produce exactly +// one winner. This is the critical primitive that prevents two management +// sessions from both running the cancel/persist work at finalization. +func testMDMWindowsAwaitingConfigurationCAS(t *testing.T, ds *Datastore) { + ctx := t.Context() + + // Transition matrix: every meaningfully-distinct (currentState, expectFrom, to) combination, with the expected + // (transitioned, finalState) outcome. Three matched transitions exercise the success cases; three mismatched + // cases exercise idempotency-on-mismatch (no error, no state change). Reverse / weird transitions like + // Active->Pending are deliberately omitted -- they're not valid in the production state machine. + cases := []struct { + name string + currentState fleet.WindowsMDMAwaitingConfiguration + expectFrom fleet.WindowsMDMAwaitingConfiguration + to fleet.WindowsMDMAwaitingConfiguration + wantTransitioned bool + wantFinalState fleet.WindowsMDMAwaitingConfiguration + }{ + { + name: "Pending->Active matched", + currentState: fleet.WindowsMDMAwaitingConfigurationPending, + expectFrom: fleet.WindowsMDMAwaitingConfigurationPending, + to: fleet.WindowsMDMAwaitingConfigurationActive, + wantTransitioned: true, + wantFinalState: fleet.WindowsMDMAwaitingConfigurationActive, + }, + { + name: "Active->None matched", + currentState: fleet.WindowsMDMAwaitingConfigurationActive, + expectFrom: fleet.WindowsMDMAwaitingConfigurationActive, + to: fleet.WindowsMDMAwaitingConfigurationNone, + wantTransitioned: true, + wantFinalState: fleet.WindowsMDMAwaitingConfigurationNone, + }, + { + name: "Pending->None matched (timeout-during-pending shortcut)", + currentState: fleet.WindowsMDMAwaitingConfigurationPending, + expectFrom: fleet.WindowsMDMAwaitingConfigurationPending, + to: fleet.WindowsMDMAwaitingConfigurationNone, + wantTransitioned: true, + wantFinalState: fleet.WindowsMDMAwaitingConfigurationNone, + }, + { + name: "Pending row with expectFrom=Active is no-op (mismatched)", + currentState: fleet.WindowsMDMAwaitingConfigurationPending, + expectFrom: fleet.WindowsMDMAwaitingConfigurationActive, + to: fleet.WindowsMDMAwaitingConfigurationNone, + wantTransitioned: false, + wantFinalState: fleet.WindowsMDMAwaitingConfigurationPending, + }, + { + name: "None row with expectFrom=Pending is no-op (idempotent retry)", + currentState: fleet.WindowsMDMAwaitingConfigurationNone, + expectFrom: fleet.WindowsMDMAwaitingConfigurationPending, + to: fleet.WindowsMDMAwaitingConfigurationActive, + wantTransitioned: false, + wantFinalState: fleet.WindowsMDMAwaitingConfigurationNone, + }, + { + name: "Active row with expectFrom=Pending is no-op (mismatched)", + currentState: fleet.WindowsMDMAwaitingConfigurationActive, + expectFrom: fleet.WindowsMDMAwaitingConfigurationPending, + to: fleet.WindowsMDMAwaitingConfigurationNone, + wantTransitioned: false, + wantFinalState: fleet.WindowsMDMAwaitingConfigurationActive, + }, + } + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + deviceID := insertWindowsEnrolledDevice(t, ctx, ds, windowsEnrollmentFixture{ + deviceNameSuffix: "CAS", + awaitingConfiguration: tt.currentState, + }) + transitioned, err := ds.SetMDMWindowsAwaitingConfiguration(ctx, deviceID, tt.expectFrom, tt.to) + require.NoError(t, err) + require.Equal(t, tt.wantTransitioned, transitioned) + + got, err := ds.MDMWindowsGetEnrolledDeviceWithDeviceID(ctx, deviceID) + require.NoError(t, err) + require.Equal(t, tt.wantFinalState, got.AwaitingConfiguration) + }) + } + + t.Run("concurrent CAS yields exactly one winner", func(t *testing.T) { + // Defends the row-level atomicity contract that handleESPRelease relies on: when two checkins race + // past the wait gate, at most one finalize commits. + deviceID := insertWindowsEnrolledDevice(t, ctx, ds, windowsEnrollmentFixture{ + deviceNameSuffix: "CAS-CONCURRENT", + awaitingConfiguration: fleet.WindowsMDMAwaitingConfigurationActive, + }) + const goroutines = 10 + var wg sync.WaitGroup + var winners atomic.Int32 + for range goroutines { + wg.Go(func() { + tr, err := ds.SetMDMWindowsAwaitingConfiguration(ctx, deviceID, + fleet.WindowsMDMAwaitingConfigurationActive, + fleet.WindowsMDMAwaitingConfigurationNone) + // nolint:testifylint // require.NoError calls t.FailNow which is unsafe in goroutines; assert is correct here. + assert.NoError(t, err) + if tr { + winners.Add(1) + } + }) + } + wg.Wait() + require.Equal(t, int32(1), winners.Load(), + "exactly one concurrent CAS should win the Active->None transition") + + got, err := ds.MDMWindowsGetEnrolledDeviceWithDeviceID(ctx, deviceID) + require.NoError(t, err) + require.Equal(t, fleet.WindowsMDMAwaitingConfigurationNone, got.AwaitingConfiguration) + }) + + t.Run("unknown device id returns false without error", func(t *testing.T) { + transitioned, err := ds.SetMDMWindowsAwaitingConfiguration(ctx, "nonexistent-"+uuid.NewString(), + fleet.WindowsMDMAwaitingConfigurationPending, + fleet.WindowsMDMAwaitingConfigurationActive) + require.NoError(t, err) + require.False(t, transitioned) + }) + + t.Run("awaiting_configuration_at preserved across transitions", func(t *testing.T) { + // The 3-hour ESP timeout in handleESPRelease consumes this timestamp. SetMDMWindowsAwaitingConfiguration + // must NOT touch awaiting_configuration_at on transition; the value set at enrollment time is the source + // of truth for "when did the device start awaiting configuration." If a regression caused the timestamp + // to be reset on Pending->Active, the timeout window would extend by the time spent in Pending. + now := time.Now().UTC().Truncate(time.Microsecond) + deviceID := insertWindowsEnrolledDevice(t, ctx, ds, windowsEnrollmentFixture{ + deviceNameSuffix: "CAS-TIMESTAMP", + awaitingConfiguration: fleet.WindowsMDMAwaitingConfigurationPending, + awaitingAt: &now, + }) + original, err := ds.MDMWindowsGetEnrolledDeviceWithDeviceID(ctx, deviceID) + require.NoError(t, err) + require.NotNil(t, original.AwaitingConfigurationAt) + originalTS := *original.AwaitingConfigurationAt + + // Pending -> Active. + _, err = ds.SetMDMWindowsAwaitingConfiguration(ctx, deviceID, + fleet.WindowsMDMAwaitingConfigurationPending, + fleet.WindowsMDMAwaitingConfigurationActive) + require.NoError(t, err) + afterFirst, err := ds.MDMWindowsGetEnrolledDeviceWithDeviceID(ctx, deviceID) + require.NoError(t, err) + require.NotNil(t, afterFirst.AwaitingConfigurationAt) + require.True(t, afterFirst.AwaitingConfigurationAt.Equal(originalTS), + "timestamp must be preserved across Pending->Active (got %v, want %v)", + *afterFirst.AwaitingConfigurationAt, originalTS) + + // Active -> None. + _, err = ds.SetMDMWindowsAwaitingConfiguration(ctx, deviceID, + fleet.WindowsMDMAwaitingConfigurationActive, + fleet.WindowsMDMAwaitingConfigurationNone) + require.NoError(t, err) + afterSecond, err := ds.MDMWindowsGetEnrolledDeviceWithDeviceID(ctx, deviceID) + require.NoError(t, err) + require.NotNil(t, afterSecond.AwaitingConfigurationAt) + require.True(t, afterSecond.AwaitingConfigurationAt.Equal(originalTS), + "timestamp must be preserved across Active->None (got %v, want %v)", + *afterSecond.AwaitingConfigurationAt, originalTS) + }) +} + +// testMDMWindowsAwaitingConfigurationByHostUUID verifies the GetMDMWindowsAwaitingConfigurationByHostUUID lookup +// used in the orbit-config hot path: returns the awaiting_configuration value for the most-recent enrollment of +// the host UUID, returns NotFound for unknown hosts, and never cross-leaks between host UUIDs. +func testMDMWindowsAwaitingConfigurationByHostUUID(t *testing.T, ds *Datastore) { + ctx := t.Context() + + // newWindowsHost creates a Windows host plus a linked enrollment row in the requested initial state. Each + // subtest gets its own host (distinct UUID) so subtests don't pollute each other's state. + newWindowsHost := func(t *testing.T, hostnameSlug string, initial fleet.WindowsMDMAwaitingConfiguration) (*fleet.Host, string) { + t.Helper() + host := test.NewHost(t, ds, hostnameSlug, "10.0.0.10", hostnameSlug+"-key", uuid.NewString(), time.Now()) + host.Platform = "windows" + require.NoError(t, ds.UpdateHost(ctx, host)) + mdmDeviceID := insertWindowsEnrolledDevice(t, ctx, ds, windowsEnrollmentFixture{ + deviceNameSuffix: hostnameSlug, + hostUUID: host.UUID, + awaitingConfiguration: initial, + }) + return host, mdmDeviceID + } + + t.Run("returns current state and reflects transitions", func(t *testing.T) { + host, mdmDeviceID := newWindowsHost(t, "win-current", fleet.WindowsMDMAwaitingConfigurationPending) + + got, err := ds.GetMDMWindowsAwaitingConfigurationByHostUUID(ctx, host.UUID) + require.NoError(t, err) + require.Equal(t, fleet.WindowsMDMAwaitingConfigurationPending, got) + + tr, err := ds.SetMDMWindowsAwaitingConfiguration(ctx, mdmDeviceID, + fleet.WindowsMDMAwaitingConfigurationPending, + fleet.WindowsMDMAwaitingConfigurationActive) + require.NoError(t, err) + require.True(t, tr) + + got, err = ds.GetMDMWindowsAwaitingConfigurationByHostUUID(ctx, host.UUID) + require.NoError(t, err) + require.Equal(t, fleet.WindowsMDMAwaitingConfigurationActive, got) + }) + + t.Run("returns most recent enrollment when host has multiple", func(t *testing.T) { + // The function ORDER BYs `created_at DESC, id DESC LIMIT 1`. That clause exists for a single concrete + // reason: a host_uuid can have multiple mdm_windows_enrollments rows when a device re-enrolls during + // Autopilot reset. Without this subtest, the ORDER BY clause could be deleted and the rest of the test + // suite would still pass. + host := test.NewHost(t, ds, "win-multi", "10.0.0.20", "win-multi-key", uuid.NewString(), time.Now()) + host.Platform = "windows" + require.NoError(t, ds.UpdateHost(ctx, host)) + + // Older enrollment in Active state. + olderDeviceID := insertWindowsEnrolledDevice(t, ctx, ds, windowsEnrollmentFixture{ + deviceNameSuffix: "OLDER", + hostUUID: host.UUID, + awaitingConfiguration: fleet.WindowsMDMAwaitingConfigurationActive, + }) + + // Backdate the older enrollment so the primary `created_at DESC` sort is exercised (not just the + // `id DESC` fallback that would happen with two rows inserted within the same microsecond). + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + _, err := q.ExecContext(ctx, + "UPDATE mdm_windows_enrollments SET created_at = DATE_SUB(NOW(6), INTERVAL 1 HOUR) WHERE mdm_device_id = ?", + olderDeviceID) + return err + }) + + // Newer enrollment in Pending state (re-enrolled device). + insertWindowsEnrolledDevice(t, ctx, ds, windowsEnrollmentFixture{ + deviceNameSuffix: "NEWER", + hostUUID: host.UUID, + awaitingConfiguration: fleet.WindowsMDMAwaitingConfigurationPending, + }) + + got, err := ds.GetMDMWindowsAwaitingConfigurationByHostUUID(ctx, host.UUID) + require.NoError(t, err) + require.Equal(t, fleet.WindowsMDMAwaitingConfigurationPending, got, + "must return the most-recent enrollment's state, not the older one") + }) + + t.Run("does not cross-leak between hosts", func(t *testing.T) { + // Negative case: hostA's lookup must not return hostB's state. Without this, removing the + // `WHERE host_uuid = ?` clause would silently pass other tests because they each use one host. + hostA, _ := newWindowsHost(t, "win-iso-a", fleet.WindowsMDMAwaitingConfigurationActive) + hostB, _ := newWindowsHost(t, "win-iso-b", fleet.WindowsMDMAwaitingConfigurationPending) + + gotA, err := ds.GetMDMWindowsAwaitingConfigurationByHostUUID(ctx, hostA.UUID) + require.NoError(t, err) + require.Equal(t, fleet.WindowsMDMAwaitingConfigurationActive, gotA) + + gotB, err := ds.GetMDMWindowsAwaitingConfigurationByHostUUID(ctx, hostB.UUID) + require.NoError(t, err) + require.Equal(t, fleet.WindowsMDMAwaitingConfigurationPending, gotB) + }) + + t.Run("unknown host UUID returns NotFound", func(t *testing.T) { + _, err := ds.GetMDMWindowsAwaitingConfigurationByHostUUID(ctx, "nonexistent-"+uuid.NewString()) + require.Error(t, err) + require.True(t, fleet.IsNotFound(err), "unknown host UUID must return NotFound, got: %v", err) + }) +} + +// testMDMWindowsHasSetupExperienceItems verifies HasWindowsSetupExperienceItemsForTeam counts only Windows +// software installers and only when active + install_during_setup. +func testMDMWindowsHasSetupExperienceItems(t *testing.T, ds *Datastore) { + ctx := t.Context() + user := test.NewUser(t, ds, "esp-test-user", "esp-test@example.com", true) + + // makeInstaller creates a fresh software installer via the production datastore method, with the requested + // team scope and platform. Each subtest uses this to build its own isolated fixtures rather than mutating a + // single shared row -- both for clearer per-subtest failure diagnostics and because the row mutations between + // cases are also what we're testing. + makeInstaller := func(t *testing.T, teamID *uint, platform string, installDuringSetup bool, titleSuffix string) uint { + t.Helper() + tfr, err := fleet.NewTempFileReader(strings.NewReader("hello"), t.TempDir) + require.NoError(t, err) + title := "esp-test-" + titleSuffix + installerID, _, err := ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ + TeamID: teamID, + UserID: user.ID, + InstallScript: "echo install", + InstallerFile: tfr, + StorageID: uuid.NewString(), + Filename: title + ".msi", + Title: title, + Version: "1.0", + Source: "apps", + Platform: platform, + ValidatedLabels: &fleet.LabelIdentsWithScope{}, + }) + require.NoError(t, err) + if installDuringSetup { + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + _, err := q.ExecContext(ctx, + "UPDATE software_installers SET install_during_setup = 1 WHERE id = ?", installerID) + return err + }) + } + return installerID + } + + t.Run("no installers configured returns false", func(t *testing.T) { + team, err := ds.NewTeam(ctx, &fleet.Team{Name: "esp-empty-" + uuid.NewString()}) + require.NoError(t, err) + + hasItems, err := ds.HasWindowsSetupExperienceItemsForTeam(ctx, team.ID) + require.NoError(t, err) + require.False(t, hasItems) + }) + + t.Run("Windows installer without install_during_setup returns false", func(t *testing.T) { + team, err := ds.NewTeam(ctx, &fleet.Team{Name: "esp-noflag-" + uuid.NewString()}) + require.NoError(t, err) + makeInstaller(t, &team.ID, "windows", false, "no-flag") + + hasItems, err := ds.HasWindowsSetupExperienceItemsForTeam(ctx, team.ID) + require.NoError(t, err) + require.False(t, hasItems, "Windows installer without install_during_setup must not count") + }) + + t.Run("Windows installer with install_during_setup returns true", func(t *testing.T) { + team, err := ds.NewTeam(ctx, &fleet.Team{Name: "esp-flag-" + uuid.NewString()}) + require.NoError(t, err) + makeInstaller(t, &team.ID, "windows", true, "critical") + + hasItems, err := ds.HasWindowsSetupExperienceItemsForTeam(ctx, team.ID) + require.NoError(t, err) + require.True(t, hasItems) + }) + + t.Run("non-Windows installer is filtered out", func(t *testing.T) { + team, err := ds.NewTeam(ctx, &fleet.Team{Name: "esp-darwin-" + uuid.NewString()}) + require.NoError(t, err) + makeInstaller(t, &team.ID, "darwin", true, "macos-app") + + hasItems, err := ds.HasWindowsSetupExperienceItemsForTeam(ctx, team.ID) + require.NoError(t, err) + require.False(t, hasItems, "darwin installer must not match the windows-only filter") + }) + + t.Run("inactive Windows installer is filtered out", func(t *testing.T) { + // `is_active` is internal versioning state with no public datastore toggle; it is flipped to 0 by the + // upload-versioning flow when a newer installer for the same title supersedes the previous one. Here + // we simulate that end state by flipping it via raw UPDATE and verify the function's WHERE clause + // honors it. + team, err := ds.NewTeam(ctx, &fleet.Team{Name: "esp-inactive-" + uuid.NewString()}) + require.NoError(t, err) + installerID := makeInstaller(t, &team.ID, "windows", true, "inactive") + + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + _, err := q.ExecContext(ctx, + "UPDATE software_installers SET is_active = 0 WHERE id = ?", installerID) + return err + }) + + hasItems, err := ds.HasWindowsSetupExperienceItemsForTeam(ctx, team.ID) + require.NoError(t, err) + require.False(t, hasItems, "is_active=0 installer must not count") + }) + + t.Run("setup_experience_scripts is filtered out (script-exclusion invariant)", func(t *testing.T) { + // Most important non-obvious invariant. The setup_experience_scripts table is platform-agnostic at the + // schema level, but enqueueSetupExperienceItems only enqueues scripts when fleetPlatform=="darwin", so a + // script on a Windows team is never actually enqueued. If HasWindowsSetupExperienceItemsForTeam counted + // scripts, the wait gate would block waiting for an item that will never arrive -- the device hangs + // until the 3-hour ESP timeout. We add the script via SetSetupExperienceScript (the production caller + // path) rather than raw INSERT so a future migration that adds required columns to that table doesn't + // silently leave this test passing. + team, err := ds.NewTeam(ctx, &fleet.Team{Name: "esp-script-" + uuid.NewString()}) + require.NoError(t, err) + require.NoError(t, ds.SetSetupExperienceScript(ctx, &fleet.Script{ + TeamID: &team.ID, + Name: "setup.sh", + ScriptContents: "echo setup", + })) + + hasItems, err := ds.HasWindowsSetupExperienceItemsForTeam(ctx, team.ID) + require.NoError(t, err) + require.False(t, hasItems, "setup_experience_scripts must not count for Windows ESP (scripts are only enqueued for darwin)") + }) + + t.Run("global teamID=0 finds global installers", func(t *testing.T) { + // teamID=0 is the no-team / global value. Real production case: hosts on no team -- the service caller + // passes 0 directly when host.TeamID is nil. The global path is structurally distinct from the + // team-scoped path (TeamID=nil in the payload maps to global_or_team_id=0 in the table), so it warrants + // dedicated coverage. + hasItems, err := ds.HasWindowsSetupExperienceItemsForTeam(ctx, 0) + require.NoError(t, err) + require.False(t, hasItems, "global must start empty before we add a global installer") + + makeInstaller(t, nil, "windows", true, "global-critical-"+uuid.NewString()) + + hasItems, err = ds.HasWindowsSetupExperienceItemsForTeam(ctx, 0) + require.NoError(t, err) + require.True(t, hasItems, "teamID=0 must surface installers with global_or_team_id=0") + }) + + t.Run("does not cross-leak between teams", func(t *testing.T) { + // Negative case for the global_or_team_id WHERE clause. Without this subtest, dropping the clause would + // silently pass the other subtests because each uses a single team -- the regression would only show up + // in production when a host on team B inherited team A's setup experience. + teamA, err := ds.NewTeam(ctx, &fleet.Team{Name: "esp-iso-A-" + uuid.NewString()}) + require.NoError(t, err) + teamB, err := ds.NewTeam(ctx, &fleet.Team{Name: "esp-iso-B-" + uuid.NewString()}) + require.NoError(t, err) + makeInstaller(t, &teamA.ID, "windows", true, "team-a-app-"+uuid.NewString()) + + hasItemsA, err := ds.HasWindowsSetupExperienceItemsForTeam(ctx, teamA.ID) + require.NoError(t, err) + require.True(t, hasItemsA) + + hasItemsB, err := ds.HasWindowsSetupExperienceItemsForTeam(ctx, teamB.ID) + require.NoError(t, err) + require.False(t, hasItemsB, "team-A installer must not appear for team-B") + }) +} diff --git a/server/datastore/mysql/setup_experience.go b/server/datastore/mysql/setup_experience.go index 5992267604..3cdce08997 100644 --- a/server/datastore/mysql/setup_experience.go +++ b/server/datastore/mysql/setup_experience.go @@ -57,15 +57,19 @@ func (ds *Datastore) enqueueSetupExperienceItems(ctx context.Context, hostPlatfo // don't enqueue any items. This handles the edge case where an enrolled host upgrades from an // Orbit version that didn't support setup experience to one that does. // See https://github.com/fleetdm/fleet/issues/35717 + // Match either osquery_host_id or uuid because the hostUUID parameter comes from + // fleet.HostUUIDForSetupExperience, which on Windows/Linux resolves to OsqueryHostID and on + // Apple platforms to host.UUID. Without the OR, the lookup misses Windows/Linux hosts when + // OsqueryHostID and the Fleet host UUID differ (default osquery host_identifier modes). stmtHost := ` SELECT last_enrolled_at FROM hosts - WHERE uuid = ? AND platform = ? + WHERE (osquery_host_id = ? OR uuid = ?) AND platform = ? ` var lastEnrolledAt sql.NullTime - if err := sqlx.GetContext(ctx, ds.reader(ctx), &lastEnrolledAt, stmtHost, hostUUID, hostPlatform); err != nil { + if err := sqlx.GetContext(ctx, ds.reader(ctx), &lastEnrolledAt, stmtHost, hostUUID, hostUUID, hostPlatform); err != nil { if errors.Is(err, sql.ErrNoRows) { // This shouldn't happen but we don't check for it elsewhere, // so we'll log a warning and continue. @@ -77,8 +81,41 @@ func (ds *Datastore) enqueueSetupExperienceItems(ctx context.Context, hostPlatfo // If the host was enrolled more than 24 hours ago, don't enqueue any items. // Note: if the last enroll date is our "zero date" (1/1/2000), treat it as if it's never enrolled. if lastEnrolledAt.Valid && lastEnrolledAt.Time.Before(time.Now().Add(-24*time.Hour)) && lastEnrolledAt.Time.After(time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC)) { - ds.logger.DebugContext(ctx, "Host enrolled more than 24 hours ago, skipping enqueueing setup experience items", "host_uuid", hostUUID, "platform_like", hostPlatformLike, "last_enrolled_at", lastEnrolledAt.Time) - return false, nil + // On Windows, the 24h-old-host guard races with last_enrolled_at on re-Autopilot: + // orbit calls SetupExperienceInit before the new last_enrolled_at lands, so a + // previously-enrolled host that's mid-Autopilot-OOBE looks "old" and gets skipped + // even though it IS in ESP and we DO want setup-experience to run. Fall back to a + // direct check of mdm_windows_enrollments.awaiting_configuration: if the host is + // in Pending/Active, it's actively in ESP, bypass the age guard. + // mdm_windows_enrollments.host_uuid stores the Fleet host UUID (hosts.uuid), but the + // hostUUID parameter here comes from fleet.HostUUIDForSetupExperience, which on Windows + // resolves to OsqueryHostID. Resolve via JOIN and match either identifier so the lookup + // works regardless of how OsqueryHostID and the Fleet host UUID relate (this depends on + // the osquery host_identifier mode). + if hostPlatform == "windows" { + var awaiting fleet.WindowsMDMAwaitingConfiguration + stmtAwaiting := ` + SELECT mwe.awaiting_configuration + FROM mdm_windows_enrollments mwe + JOIN hosts h ON mwe.host_uuid = h.uuid + WHERE (h.osquery_host_id = ? OR h.uuid = ?) AND h.platform = 'windows' + ORDER BY mwe.created_at DESC, mwe.id DESC + LIMIT 1 + ` + if err := sqlx.GetContext(ctx, ds.reader(ctx), &awaiting, stmtAwaiting, hostUUID, hostUUID); err != nil && !errors.Is(err, sql.ErrNoRows) { + return false, ctxerr.Wrap(ctx, err, "checking windows awaiting_configuration for setup experience age guard") + } else if err == nil && awaiting != fleet.WindowsMDMAwaitingConfigurationNone { + ds.logger.DebugContext(ctx, "Windows host enrolled >24h ago but is in awaiting_configuration; running setup experience for re-Autopilot", + "host_uuid", hostUUID, "awaiting_configuration", awaiting) + // fall through to enqueue + } else { + ds.logger.DebugContext(ctx, "Host enrolled more than 24 hours ago, skipping enqueueing setup experience items", "host_uuid", hostUUID, "platform_like", hostPlatformLike, "last_enrolled_at", lastEnrolledAt.Time) + return false, nil + } + } else { + ds.logger.DebugContext(ctx, "Host enrolled more than 24 hours ago, skipping enqueueing setup experience items", "host_uuid", hostUUID, "platform_like", hostPlatformLike, "last_enrolled_at", lastEnrolledAt.Time) + return false, nil + } } } diff --git a/server/datastore/mysql/setup_experience_test.go b/server/datastore/mysql/setup_experience_test.go index f90d61f12a..069dff1618 100644 --- a/server/datastore/mysql/setup_experience_test.go +++ b/server/datastore/mysql/setup_experience_test.go @@ -8,6 +8,7 @@ import ( "time" "github.com/fleetdm/fleet/v4/server/fleet" + microsoft_mdm "github.com/fleetdm/fleet/v4/server/mdm/microsoft" "github.com/fleetdm/fleet/v4/server/ptr" "github.com/fleetdm/fleet/v4/server/test" "github.com/google/uuid" @@ -329,6 +330,45 @@ func testEnqueueSetupExperienceItemsWindows(t *testing.T, ds *Datastore) { anythingEnqueued, err = ds.EnqueueSetupExperienceItems(ctx, "windows", "windows", host2UUID, team2.ID) require.NoError(t, err) require.False(t, anythingEnqueued) + + // Re-Autopilot of an existing host: last_enrolled_at is >24h old (the + // pre-existing record predates this Autopilot cycle), but the host has + // just MDM-enrolled and is in awaiting_configuration=Pending. + host3UUID := "33333333-3333-3333-3333-333333333333" + _, err = ds.NewHost(ctx, &fleet.Host{ + Hostname: "windows-test-3-reautopilot", + OsqueryHostID: ptr.String("osquery-windows-3"), + NodeKey: ptr.String("node-key-windows-3"), + UUID: host3UUID, + Platform: "windows", + HardwareSerial: "654321c-3", + }) + require.NoError(t, err) + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + _, err := q.ExecContext(ctx, "UPDATE hosts SET last_enrolled_at = ? WHERE uuid = ?", time.Now().Add(-25*time.Hour), host3UUID) + return err + }) + // Insert a Windows MDM enrollment with awaiting_configuration=Pending, + // matching what a fresh Autopilot enrollment on the same host would create + // before last_enrolled_at gets refreshed. + require.NoError(t, ds.MDMWindowsInsertEnrolledDevice(ctx, &fleet.MDMWindowsEnrolledDevice{ + MDMDeviceID: "device-host3", + MDMHardwareID: "hw-host3", + MDMDeviceState: microsoft_mdm.MDMDeviceStateEnrolled, + MDMDeviceType: "CIMClient_Windows", + MDMDeviceName: "DESKTOP-H3", + MDMEnrollType: "ProgrammaticEnrollment", + MDMEnrollProtoVersion: "5.0", + MDMEnrollClientVersion: "10.0.19045.2965", + MDMNotInOOBE: false, + HostUUID: host3UUID, + AwaitingConfiguration: fleet.WindowsMDMAwaitingConfigurationPending, + })) + + anythingEnqueued, err = ds.EnqueueSetupExperienceItems(ctx, "windows", "windows", host3UUID, team1.ID) + require.NoError(t, err) + require.True(t, anythingEnqueued, + "re-Autopilot of an existing host (>24h old) with awaiting_configuration!=None must bypass the age guard") } func testEnqueueSetupExperienceItems(t *testing.T, ds *Datastore) { diff --git a/server/fleet/datastore.go b/server/fleet/datastore.go index 4db24e2a56..3534e23e9e 100644 --- a/server/fleet/datastore.go +++ b/server/fleet/datastore.go @@ -1986,6 +1986,11 @@ type Datastore interface { // for each device. MDMWindowsInsertCommandForHosts(ctx context.Context, hostUUIDs []string, cmd *MDMWindowsCommand) error + // MDMWindowsInsertCommandsForHost atomically inserts a batch of Windows MDM commands targeting a single host + // (identified by host UUID or MDM device ID). All commands succeed or none do, in one transaction. Used by + // the ESP finalize path so a partial-insert + fresh-UUID retry can't leave orphan rows in the queue. + MDMWindowsInsertCommandsForHost(ctx context.Context, hostUUIDOrDeviceID string, cmds []*MDMWindowsCommand) error + MDMWindowsInsertCommandAndUpsertHostProfilesForHosts(ctx context.Context, hostUUIDs []string, cmd *MDMWindowsCommand, profilePayloads []*MDMWindowsBulkUpsertHostProfilePayload) error // MDMWindowsGetPendingCommands returns all pending commands for the given enrollment. @@ -2007,6 +2012,18 @@ type Datastore interface { // transition occurred. SetMDMWindowsAwaitingConfiguration(ctx context.Context, mdmDeviceID string, expectFrom, to WindowsMDMAwaitingConfiguration) (bool, error) + // GetMDMWindowsAwaitingConfigurationByHostUUID returns the awaiting + // configuration value for the Windows MDM enrollment of the given host. + // This is a lightweight read for the orbit config polling path. + GetMDMWindowsAwaitingConfigurationByHostUUID(ctx context.Context, hostUUID string) (WindowsMDMAwaitingConfiguration, error) + + // HasWindowsSetupExperienceItemsForTeam returns true if any active Windows setup-experience software + // installers (with install_during_setup) are configured for the given team. teamID=0 means "no team / + // global". Used by the ESP release gate to disambiguate between "no setup configured" (safe to release) + // and "setup configured but orbit hasn't initialized yet" (must wait) when + // setup_experience_status_results is empty. + HasWindowsSetupExperienceItemsForTeam(ctx context.Context, teamID uint) (bool, error) + // GetMDMWindowsConfigProfile returns the Windows MDM profile corresponding // to the specified profile uuid. GetMDMWindowsConfigProfile(ctx context.Context, profileUUID string) (*MDMWindowsConfigProfile, error) diff --git a/server/mdm/microsoft/esp_csp.go b/server/mdm/microsoft/esp_csp.go index 8b7a5df8da..6d087514d4 100644 --- a/server/mdm/microsoft/esp_csp.go +++ b/server/mdm/microsoft/esp_csp.go @@ -2,3 +2,9 @@ package microsoft_mdm // ESPTimeoutSeconds is the default timeout for the Enrollment Status Page (3 hours). const ESPTimeoutSeconds = 3 * 60 * 60 + +// ESPSoftwareFailureErrorText is the message shown on the Windows ESP failure screen when a critical software install fails. +const ESPSoftwareFailureErrorText = "Critical software failed to install. Please try again. If this keeps happening, please contact your IT admin." + +// ESPTimeoutErrorText is the message shown on the Windows ESP failure screen when the 3-hour ESP timeout expires before setup completes. +const ESPTimeoutErrorText = "Setup is taking longer than expected. Please try again. If this keeps happening, please contact your IT admin." diff --git a/server/mock/datastore_mock.go b/server/mock/datastore_mock.go index 0c00e55ff3..54fe4ddb16 100644 --- a/server/mock/datastore_mock.go +++ b/server/mock/datastore_mock.go @@ -1277,6 +1277,8 @@ type MDMWindowsDeleteEnrolledDeviceWithDeviceIDFunc func(ctx context.Context, md type MDMWindowsInsertCommandForHostsFunc func(ctx context.Context, hostUUIDs []string, cmd *fleet.MDMWindowsCommand) error +type MDMWindowsInsertCommandsForHostFunc func(ctx context.Context, hostUUIDOrDeviceID string, cmds []*fleet.MDMWindowsCommand) error + type MDMWindowsInsertCommandAndUpsertHostProfilesForHostsFunc func(ctx context.Context, hostUUIDs []string, cmd *fleet.MDMWindowsCommand, profilePayloads []*fleet.MDMWindowsBulkUpsertHostProfilePayload) error type MDMWindowsGetPendingCommandsFunc func(ctx context.Context, enrollmentID uint) ([]*fleet.MDMWindowsCommand, error) @@ -1289,6 +1291,10 @@ type UpdateMDMWindowsEnrollmentsHostUUIDFunc func(ctx context.Context, hostUUID type SetMDMWindowsAwaitingConfigurationFunc func(ctx context.Context, mdmDeviceID string, expectFrom fleet.WindowsMDMAwaitingConfiguration, to fleet.WindowsMDMAwaitingConfiguration) (bool, error) +type GetMDMWindowsAwaitingConfigurationByHostUUIDFunc func(ctx context.Context, hostUUID string) (fleet.WindowsMDMAwaitingConfiguration, error) + +type HasWindowsSetupExperienceItemsForTeamFunc func(ctx context.Context, teamID uint) (bool, error) + type GetMDMWindowsConfigProfileFunc func(ctx context.Context, profileUUID string) (*fleet.MDMWindowsConfigProfile, error) type DeleteMDMWindowsConfigProfileFunc func(ctx context.Context, profileUUID string) error @@ -3801,6 +3807,9 @@ type DataStore struct { MDMWindowsInsertCommandForHostsFunc MDMWindowsInsertCommandForHostsFunc MDMWindowsInsertCommandForHostsFuncInvoked bool + MDMWindowsInsertCommandsForHostFunc MDMWindowsInsertCommandsForHostFunc + MDMWindowsInsertCommandsForHostFuncInvoked bool + MDMWindowsInsertCommandAndUpsertHostProfilesForHostsFunc MDMWindowsInsertCommandAndUpsertHostProfilesForHostsFunc MDMWindowsInsertCommandAndUpsertHostProfilesForHostsFuncInvoked bool @@ -3819,6 +3828,12 @@ type DataStore struct { SetMDMWindowsAwaitingConfigurationFunc SetMDMWindowsAwaitingConfigurationFunc SetMDMWindowsAwaitingConfigurationFuncInvoked bool + GetMDMWindowsAwaitingConfigurationByHostUUIDFunc GetMDMWindowsAwaitingConfigurationByHostUUIDFunc + GetMDMWindowsAwaitingConfigurationByHostUUIDFuncInvoked bool + + HasWindowsSetupExperienceItemsForTeamFunc HasWindowsSetupExperienceItemsForTeamFunc + HasWindowsSetupExperienceItemsForTeamFuncInvoked bool + GetMDMWindowsConfigProfileFunc GetMDMWindowsConfigProfileFunc GetMDMWindowsConfigProfileFuncInvoked bool @@ -9156,6 +9171,13 @@ func (s *DataStore) MDMWindowsInsertCommandForHosts(ctx context.Context, hostUUI return s.MDMWindowsInsertCommandForHostsFunc(ctx, hostUUIDs, cmd) } +func (s *DataStore) MDMWindowsInsertCommandsForHost(ctx context.Context, hostUUIDOrDeviceID string, cmds []*fleet.MDMWindowsCommand) error { + s.mu.Lock() + s.MDMWindowsInsertCommandsForHostFuncInvoked = true + s.mu.Unlock() + return s.MDMWindowsInsertCommandsForHostFunc(ctx, hostUUIDOrDeviceID, cmds) +} + func (s *DataStore) MDMWindowsInsertCommandAndUpsertHostProfilesForHosts(ctx context.Context, hostUUIDs []string, cmd *fleet.MDMWindowsCommand, profilePayloads []*fleet.MDMWindowsBulkUpsertHostProfilePayload) error { s.mu.Lock() s.MDMWindowsInsertCommandAndUpsertHostProfilesForHostsFuncInvoked = true @@ -9198,6 +9220,20 @@ func (s *DataStore) SetMDMWindowsAwaitingConfiguration(ctx context.Context, mdmD return s.SetMDMWindowsAwaitingConfigurationFunc(ctx, mdmDeviceID, expectFrom, to) } +func (s *DataStore) GetMDMWindowsAwaitingConfigurationByHostUUID(ctx context.Context, hostUUID string) (fleet.WindowsMDMAwaitingConfiguration, error) { + s.mu.Lock() + s.GetMDMWindowsAwaitingConfigurationByHostUUIDFuncInvoked = true + s.mu.Unlock() + return s.GetMDMWindowsAwaitingConfigurationByHostUUIDFunc(ctx, hostUUID) +} + +func (s *DataStore) HasWindowsSetupExperienceItemsForTeam(ctx context.Context, teamID uint) (bool, error) { + s.mu.Lock() + s.HasWindowsSetupExperienceItemsForTeamFuncInvoked = true + s.mu.Unlock() + return s.HasWindowsSetupExperienceItemsForTeamFunc(ctx, teamID) +} + func (s *DataStore) GetMDMWindowsConfigProfile(ctx context.Context, profileUUID string) (*fleet.MDMWindowsConfigProfile, error) { s.mu.Lock() s.GetMDMWindowsConfigProfileFuncInvoked = true diff --git a/server/service/microsoft_mdm.go b/server/service/microsoft_mdm.go index 11e7158e77..ad743a4eb7 100644 --- a/server/service/microsoft_mdm.go +++ b/server/service/microsoft_mdm.go @@ -26,6 +26,7 @@ import ( "github.com/fleetdm/fleet/v4/ee/server/service/scep" "github.com/fleetdm/fleet/v4/pkg/fleetdbase" "github.com/fleetdm/fleet/v4/server" + "github.com/fleetdm/fleet/v4/server/contexts/ctxdb" "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" "github.com/fleetdm/fleet/v4/server/contexts/logging" "github.com/fleetdm/fleet/v4/server/fleet" @@ -1984,11 +1985,13 @@ func (svc *Service) getManagementResponse(ctx context.Context, reqMsg *fleet.Syn } resPendingCmds = pendingCmds - // Build ESP (Enrollment Status Page) commands for Windows Autopilot devices. - // Only run for trusted requests so we don't leak ESP state to unauthenticated devices. - espCmds, err = svc.getESPCommands(ctx, deviceID) - if err != nil { - return nil, fmt.Errorf("ESP commands error: %w", err) + // Build ESP (Enrollment Status Page) commands for Windows Autopilot devices. Only run for trusted requests + // so we don't leak ESP state to unauthenticated devices. + if enrolledDevice.AwaitingConfiguration != fleet.WindowsMDMAwaitingConfigurationNone { + espCmds, err = svc.getESPCommands(ctx, enrolledDevice) + if err != nil { + return nil, fmt.Errorf("ESP commands error: %w", err) + } } } @@ -2006,29 +2009,19 @@ func (svc *Service) getManagementResponse(ctx context.Context, reqMsg *fleet.Syn return msg, nil } -// getESPCommands checks if a Windows device is in the Autopilot setup experience -// and returns appropriate ESP SyncML commands. +// getESPCommands dispatches ESP coordination for a Windows Autopilot device. // -// For awaiting_configuration=Pending: sends hold commands to block the device at -// the ESP during OOBE, then transitions to Active once orbit links the host UUID. +// For awaiting_configuration=Pending: send hold commands to block the device at the ESP during OOBE, then transition +// to Active once orbit links the host UUID. // -// For awaiting_configuration=Active: checks if all profiles have been delivered -// and releases the device when ready. -func (svc *Service) getESPCommands(ctx context.Context, deviceID string) ([]*mdm_types.SyncMLCmd, error) { - enrolledDevice, err := svc.ds.MDMWindowsGetEnrolledDeviceWithDeviceID(ctx, deviceID) - if err != nil { - if fleet.IsNotFound(err) { - // Device may have just unenrolled; nothing to do. - return nil, nil - } - return nil, ctxerr.Wrap(ctx, err, "get enrolled device for ESP") - } - - switch enrolledDevice.AwaitingConfiguration { +// For awaiting_configuration=Active: run the wait gates (profiles + setup-experience software) and release or block +// the device when ready, including the 3-hour timeout. +func (svc *Service) getESPCommands(ctx context.Context, device *fleet.MDMWindowsEnrolledDevice) ([]*mdm_types.SyncMLCmd, error) { + switch device.AwaitingConfiguration { case fleet.WindowsMDMAwaitingConfigurationPending: - return svc.handleESPHoldOrTransition(ctx, enrolledDevice) + return svc.handleESPHoldOrTransition(ctx, device) case fleet.WindowsMDMAwaitingConfigurationActive: - return svc.handleESPRelease(ctx, enrolledDevice) + return svc.handleESPRelease(ctx, device) default: return nil, nil } @@ -2052,8 +2045,12 @@ func (svc *Service) handleESPHoldOrTransition(ctx context.Context, device *fleet // DMClient CSP spec. Both must be false for the ESP to stay visible. newSyncMLCmdBool(fleet.CmdReplace, fmt.Sprintf("./Device/Vendor/MSFT/DMClient/Provider/%s/FirstSyncStatus/SkipDeviceStatusPage", providerID), "false"), newSyncMLCmdBool(fleet.CmdReplace, fmt.Sprintf("./Device/Vendor/MSFT/DMClient/Provider/%s/FirstSyncStatus/SkipUserStatusPage", providerID), "false"), - // BlockInStatusPage: 2 = block user, show "Try again" button on failure. - newSyncMLCmdInt(fleet.CmdReplace, fmt.Sprintf("./Device/Vendor/MSFT/DMClient/Provider/%s/FirstSyncStatus/BlockInStatusPage", providerID), "2"), + // BlockInStatusPage=1: block user, show "Reset PC" button on failure. + // Per DMClient CSP docs: 1=Reset PC, 2=Try Again, 4=Continue Anyway. + // We pre-configure Reset here so it's already set when/if the failure UI renders. + newSyncMLCmdInt(fleet.CmdReplace, fmt.Sprintf("./Device/Vendor/MSFT/DMClient/Provider/%s/FirstSyncStatus/BlockInStatusPage", providerID), "1"), + // AllowCollectLogsButton: pre-configure Collect Logs button so it's visible on both progress and failure pages. + newSyncMLCmdBool(fleet.CmdReplace, fmt.Sprintf("./Device/Vendor/MSFT/DMClient/Provider/%s/FirstSyncStatus/AllowCollectLogsButton", providerID), "true"), // TimeOutUntilSyncFailure is in minutes per DMClient CSP (range 60-1440). newSyncMLCmdInt(fleet.CmdReplace, fmt.Sprintf("./Device/Vendor/MSFT/DMClient/Provider/%s/FirstSyncStatus/TimeOutUntilSyncFailure", providerID), fmt.Sprintf("%d", microsoft_mdm.ESPTimeoutSeconds/60)), // PolicyProviders/{providerID} is a dynamic node -- must be created @@ -2086,33 +2083,104 @@ func (svc *Service) handleESPHoldOrTransition(ctx context.Context, device *fleet return []*mdm_types.SyncMLCmd{dpCmd}, nil } -// handleESPRelease handles awaiting_configuration=Active. It checks if all -// profiles have been delivered and releases the device when ready. +// handleESPRelease handles awaiting_configuration=Active. It waits for all profiles and setup experience items to reach +// a terminal state, then either releases the device or, when require_all_software_windows is true and any item failed +// (or the 3-hour timeout was hit), blocks the device on the ESP failure screen. func (svc *Service) handleESPRelease(ctx context.Context, device *fleet.MDMWindowsEnrolledDevice) ([]*mdm_types.SyncMLCmd, error) { if device.HostUUID == "" { return nil, nil } - // Check timeout first: if we've exceeded the 3-hour window, release - // regardless of profile status. - // TODO(phase 3): check require_all_software_windows. If true, send - // BlockInStatusPage to force "Try again"/reboot instead of releasing. - // If false, release with error text via CustomErrorText. See #42850. + // Check timeout first: if we've exceeded the 3-hour window, finalize regardless of profile/software status. timedOut := device.AwaitingConfigurationAt != nil && time.Since(*device.AwaitingConfigurationAt) > time.Duration(microsoft_mdm.ESPTimeoutSeconds)*time.Second if timedOut { - svc.logger.WarnContext(ctx, "ESP: timeout reached, releasing device", "device_id", device.MDMDeviceID) + svc.logger.WarnContext(ctx, "ESP: timeout reached", "device_id", device.MDMDeviceID) + } + + // hasSoftwareFailure tracks setup-experience software failures only. + var hasSoftwareFailure bool + + // loadHost lazily fetches the host (writer-routed) and memoizes for the rest of this checkin. Writer routing guards + // two replica-lag races: (1) spurious notFound during the brief gap between orbit's host registration and the next + // management session, and (2) stale team_id if a host transferred teams mid-enrollment, which could let + // require_all_software_windows be read from the wrong team and bypass the gate. + // + // We cache the full HostLite (not just team_id) because Stage 3 also needs OsqueryHostID: setup_experience_status_results + // is keyed by fleet.HostUUIDForSetupExperience, which on Windows resolves to OsqueryHostID -- not the Fleet host UUID + // stored on the MDM enrollment record. + var ( + cachedHost *fleet.HostLite + hostLoaded bool + ) + loadHost := func() (*fleet.HostLite, error) { + if hostLoaded { + return cachedHost, nil + } + host, err := svc.ds.HostLiteByIdentifier(ctxdb.RequirePrimary(ctx, true), device.HostUUID) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "lookup host for ESP") + } + cachedHost = host + hostLoaded = true + return host, nil + } + + // setupExperienceHostUUID returns the identifier used as setup_experience_status_results.host_uuid for this host. + // On Windows that's OsqueryHostID per fleet.HostUUIDForSetupExperience; if it's missing for some reason we fall back + // to the Fleet host UUID. + setupExperienceHostUUID := func() (string, error) { + host, err := loadHost() + if err != nil { + return "", err + } + if host.OsqueryHostID != nil && *host.OsqueryHostID != "" { + return *host.OsqueryHostID, nil + } + return device.HostUUID, nil + } + + // loadRequireAll memoizes the host -> team's require_all_software_windows lookup. It is consulted at most twice + // per checkin: once inside Stage 3 (to decide whether to short-circuit on a software failure) and once below + // (to drive the block/release decision). + var ( + cachedRequireAll bool + requireAllLoaded bool + ) + loadRequireAll := func() (bool, error) { + if requireAllLoaded { + return cachedRequireAll, nil + } + host, err := loadHost() + if err != nil { + return false, err + } + if host.TeamID == nil { + ac, err := svc.ds.AppConfig(ctx) + if err != nil { + return false, ctxerr.Wrap(ctx, err, "get app config for ESP finalization") + } + cachedRequireAll = ac.MDM.MacOSSetup.RequireAllSoftwareWindows + } else { + team, err := svc.ds.TeamLite(ctx, *host.TeamID) + if err != nil { + return false, ctxerr.Wrap(ctx, err, "get team for ESP finalization") + } + cachedRequireAll = team.Config.MDM.MacOSSetup.RequireAllSoftwareWindows + } + requireAllLoaded = true + return cachedRequireAll, nil } if !timedOut { // Profile delivery has two stages, each covered by a different query: // - // 1. Profiles configured for the host's team but not yet queued by the - // profile reconciler (ListMDMWindowsProfilesToInstallForHost). - // 2. Profiles queued (rows in host_mdm_windows_profiles) but not yet - // delivered to a terminal state (GetHostMDMWindowsProfiles). + // 1. Profiles configured for the host's team but not yet queued by the profile reconciler + // (ListMDMWindowsProfilesToInstallForHost). + // 2. Profiles queued (rows in host_mdm_windows_profiles) but not yet delivered to a terminal state + // (GetHostMDMWindowsProfiles). // - // We check both so we never release while profiles are pending at - // either stage. Each management checkin re-evaluates both queries. + // We check both so we never release while profiles are pending at either stage. Each management checkin + // re-evaluates both queries. // Stage 1: profiles the reconciler hasn't picked up yet. toInstall, err := svc.ds.ListMDMWindowsProfilesToInstallForHost(ctx, device.HostUUID) @@ -2132,29 +2200,273 @@ func (svc *Service) handleESPRelease(ctx context.Context, device *fleet.MDMWindo if p.OperationType != fleet.MDMOperationTypeInstall { continue } + // Wait for terminal state (verified or failed) before proceeding. Profile failures are NOT propagated to + // the block decision -- see hasSoftwareFailure comment above. if p.Status == nil || (*p.Status != fleet.MDMDeliveryVerified && *p.Status != fleet.MDMDeliveryFailed) { return nil, nil } } + + // Stage 3: setup experience software/scripts still running. Orbit initiates setup experience during startup when it + // is enabled for the current OS/flags, which enqueues items into setup_experience_status_results. + // + // setup_experience_status_results.host_uuid is keyed by fleet.HostUUIDForSetupExperience; on Windows that's the + // host's OsqueryHostID. We pass teamID=0 because that parameter is only used for icon and display-name enrichment + // in the datastore call, not for filtering (the query filters only by host_uuid). + seHostUUID, err := setupExperienceHostUUID() + if err != nil { + return nil, err + } + results, err := svc.ds.ListSetupExperienceResultsByHostUUID(ctx, seHostUUID, 0) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "list setup experience results for ESP release check") + } + svc.logger.DebugContext(ctx, "ESP: setup experience check", + "host_uuid", device.HostUUID, "se_host_uuid", seHostUUID, "results_count", len(results)) + + // Empty results is ambiguous: it can mean "no setup experience is configured for this team" (safe to release) or + // "setup is configured but orbit hasn't called SetupExperienceInit yet" (must wait). Orbit links the host UUID to + // the MDM enrollment independently of when it calls init, so on the first Active checkin after link we can hit + // this race. Disambiguate by checking whether items are configured for the host's team. + if len(results) == 0 { + host, err := loadHost() + if err != nil { + return nil, err + } + var teamIDForQuery uint + if host.TeamID != nil { + teamIDForQuery = *host.TeamID + } + hasItems, err := svc.ds.HasWindowsSetupExperienceItemsForTeam(ctx, teamIDForQuery) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "check setup experience items configured for team") + } + if hasItems { + svc.logger.DebugContext(ctx, "ESP: setup experience configured but not yet initialized; waiting", + "host_uuid", device.HostUUID) + return nil, nil + } + } + + // Single pass: collect hasSoftwareFailure and "are any rows still in flight". We deliberately do NOT bail + // early on the first non-terminal row -- we need to know whether any failure exists in the result set + // before deciding whether to wait, so we can short-circuit when require_all=true and we've already + // observed a failure. IsTerminalStatus() returns true for success/failure; Cancelled is also a completed + // outcome (must not block release). + anyInFlight := false + for _, r := range results { + switch r.Status { + case fleet.SetupExperienceStatusFailure: + hasSoftwareFailure = true + case fleet.SetupExperienceStatusSuccess, fleet.SetupExperienceStatusCancelled: + // terminal, nothing to record + default: + // pending / running + anyInFlight = true + } + } + if anyInFlight { + if !hasSoftwareFailure { + svc.logger.DebugContext(ctx, "ESP: waiting for in-flight setup experience items", + "host_uuid", device.HostUUID, "results_count", len(results)) + return nil, nil + } + // A software install has already failed. If require_all=true, the device is going to block. + requireAll, err := loadRequireAll() + if err != nil { + return nil, err + } + if !requireAll { + svc.logger.DebugContext(ctx, "ESP: software failure observed but require_all=false; waiting for rest", + "host_uuid", device.HostUUID, "results_count", len(results)) + return nil, nil + } + svc.logger.InfoContext(ctx, "ESP: software failure with require_all=true; blocking install", + "host_uuid", device.HostUUID) + } } - // Transition Active -> None. The CAS ensures only one concurrent - // checkin wins and enqueues the release command. + // We're past the wait gate or timed out. Look up require_all_software_windows (memoized via loadRequireAll if + // Stage 3 already consulted it) to decide between block and release. Return the error on lookup failure so the + // device stays Active and retries on the next management session: failing open here would permanently bypass + // the policy after the Active->None transition below. + requireAll, err := loadRequireAll() + if err != nil { + return nil, err + } + + failed := timedOut || hasSoftwareFailure + shouldBlock := failed && requireAll + + // Build commands for the response. + provID := syncml.DocProvisioningAppProviderID + var cmds []*mdm_types.SyncMLCmd + if shouldBlock { + // Pick the user-facing error text to surface on the failure UI. Software failure takes precedence over timeout + // because it's more actionable; pure timeout (no software failed) uses the timeout text so the user sees an + // accurate reason. + errorText := microsoft_mdm.ESPTimeoutErrorText + if hasSoftwareFailure { + errorText = microsoft_mdm.ESPSoftwareFailureErrorText + } + cmds = buildESPBlockCommands(provID, errorText) + } else { + // Release path: device proceeds to login. We do not send CustomErrorText here because the failure UI never renders + // on a release (no BlockInStatusPage, no forced timeout), so any error text would be dead state on the DMClient + // node. + cmds = buildESPReleaseCommands(provID) + } + + // On timeout (regardless of require_all) and on software-failure+require_all=true, cancel any pending items. Run + // this BEFORE the compare-and-swap (CAS) that commits awaiting_configuration=None at the bottom of this function, + // so a transient cancel failure aborts the finalize cleanly -- otherwise we'd commit awaiting=None while leaving + // non-terminal setup-experience rows behind, exactly the state cancellation is supposed to prevent. + // + // We must cancel both halves: the upcoming_activities queue AND the setup_experience_status_results status + // table (so the UI and downstream queries see the cancelled state). + // + // Cancel ordering: upcoming_activities first, then status table. If we crash mid-loop, the next retry sees the same + // status rows still pending, re-iterates, and will tolerate the now-deleted upcoming_activities row via IsNotFound. + // + // The canceled_setup_experience activity is already emitted by maybeCancelPendingSetupExperienceSteps in the + // software-install-result reporting path when require_all=true and a software install fails. Pure-timeout and + // require_all=false cancellations don't emit the activity at all (matching macOS, which only emits on the + // require_all=true software-failure case). + if timedOut || (hasSoftwareFailure && requireAll) { + seHostUUID, err := setupExperienceHostUUID() + if err != nil { + return nil, err + } + // We re-list statuses here rather than reusing Stage 3's `results`: that variable is scoped inside the + // `if !timedOut` block and isn't visible here, AND the timeout path skipped Stage 3 entirely so there's + // nothing to reuse on that branch. Hoisting the variable out to share it would tangle the two paths; one + // extra DB read per finalize keeps this block self-contained. + statuses, err := svc.ds.ListSetupExperienceResultsByHostUUID(ctx, seHostUUID, 0) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "list setup experience results for cancel") + } + host, err := loadHost() + if err != nil { + return nil, err + } + for _, s := range statuses { + if s.Status != fleet.SetupExperienceStatusPending && s.Status != fleet.SetupExperienceStatusRunning { + continue + } + var executionID string + switch { + case s.HostSoftwareInstallsExecutionID != nil: + executionID = *s.HostSoftwareInstallsExecutionID + case s.NanoCommandUUID != nil: + executionID = *s.NanoCommandUUID + case s.ScriptExecutionID != nil: + executionID = *s.ScriptExecutionID + default: + continue + } + // Tolerate notFound: a previous attempt may have cancelled the upcoming_activities row before crashing + // before the status table update, or another path (manual cancel, re-enrollment cleanup) may have removed + // it concurrently. In either case the queue is already in the desired state. + if _, err := svc.ds.CancelHostUpcomingActivity(ctx, host.ID, executionID); err != nil && !fleet.IsNotFound(err) { + return nil, ctxerr.Wrap(ctx, err, "cancel upcoming setup experience activity") + } + } + // CancelPendingSetupExperienceSteps is idempotent (status filter excludes terminal rows) so a retry on the + // next session is safe. + if err := svc.ds.CancelPendingSetupExperienceSteps(ctx, seHostUUID); err != nil { + return nil, ctxerr.Wrap(ctx, err, "cancel pending setup experience steps") + } + } + + // Persist BEFORE the CAS. The persist is the dropped-response retry safety net; if it fails, we want to leave + // awaiting_configuration=Active so the next management session retries the whole finalize from scratch. + // + // The persist is a single transactional batch (MDMWindowsInsertCommandsForHost) so a partial-fail-then-retry can't + // leave orphan rows in the queue. + // + // On concurrent-CAS races (two checkins both reach this point) both callers persist with fresh UUIDs and only one + // wins the CAS. The loser's rows are delivered later by the regular command queue, the device acks them as + // idempotent Replaces of post-ESP-irrelevant DMClient nodes, and the queue clears -- no permanent leak, just brief + // extra traffic. + if err := svc.persistESPFinalCommands(ctx, device.HostUUID, cmds); err != nil { + return nil, ctxerr.Wrap(ctx, err, "persist ESP finalization commands") + } + + // CAS Active -> None: only one concurrent checkin commits the finalize. Cancel and persist above ran for both + // concurrent winners, but cancel is idempotent and persist's losers get harmlessly delivered as orphan Replaces. transitioned, err := svc.ds.SetMDMWindowsAwaitingConfiguration(ctx, device.MDMDeviceID, fleet.WindowsMDMAwaitingConfigurationActive, fleet.WindowsMDMAwaitingConfigurationNone) if err != nil { return nil, ctxerr.Wrap(ctx, err, "set awaiting configuration to none") } if !transitioned { - // Another concurrent checkin already released the device. + // Another concurrent checkin already finalized. return nil, nil } - // Build release commands and send them inline in this response. - // Also persist via MDMWindowsInsertCommandForHosts so the existing - // command retry infrastructure resends if this response is dropped. - provID := syncml.DocProvisioningAppProviderID - releaseCmds := []*mdm_types.SyncMLCmd{ + svc.logger.InfoContext(ctx, "ESP: finalizing", + "device_id", device.MDMDeviceID, + "host_uuid", device.HostUUID, + "timed_out", timedOut, + "has_software_failure", hasSoftwareFailure, + "require_all", requireAll, + "blocking", shouldBlock) + + return cmds, nil +} + +// buildESPBlockCommands builds SyncML commands that put the device's ESP into a failed state with a "Reset device" +// button and "Collect logs" button. +func buildESPBlockCommands(provID, errorText string) []*mdm_types.SyncMLCmd { + cmds := []*mdm_types.SyncMLCmd{ + // CustomErrorText: shown in the ESP failure UI as the failure reason. + newSyncMLCmdText(fleet.CmdReplace, + fmt.Sprintf("./Device/Vendor/MSFT/DMClient/Provider/%s/FirstSyncStatus/CustomErrorText", provID), + errorText), + // BlockInStatusPage=1: show the "Reset PC" button (per Microsoft DMClient CSP docs). Reset triggers an Autopilot + // wipe and re-enrollment, which is the only reliable in-product recovery path for a failed ESP. Documented values: + // 1=Reset PC, 2=Try Again, 4=Continue Anyway. + newSyncMLCmdInt(fleet.CmdReplace, + fmt.Sprintf("./Device/Vendor/MSFT/DMClient/Provider/%s/FirstSyncStatus/BlockInStatusPage", provID), + "1"), + // AllowCollectLogsButton: show the "Collect logs" button so IT can gather diagnostics from the failure screen. + newSyncMLCmdBool(fleet.CmdReplace, + fmt.Sprintf("./Device/Vendor/MSFT/DMClient/Provider/%s/FirstSyncStatus/AllowCollectLogsButton", provID), + "true"), + // TimeOutUntilSyncFailure=1 (minute): force the ESP to time out and enter its failure state quickly. We + // deliberately do NOT send ServerHasFinishedProvisioning=true here -- that would tell the ESP it succeeded + // (Windows treats "server done + no expected items missing" as success and proceeds past the ESP). Instead we + // rely on the timeout to trigger the failure UI, which then renders our BlockInStatusPage + CustomErrorText + + // AllowCollectLogsButton. + // + // NOTE: the documented range for this node is 60-1440 minutes. Below the documented minimum, behavior is + // technically undefined, but Windows builds we have tested honor the smaller value and time out in roughly the + // configured number of minutes (verified empirically on Windows 11 23H2 / Autopilot). If a future Windows build + // clamps to 60, the failure UI would take ~1 hour to appear instead of ~1 minute -- bad UX but the contract + // (eventual failure UI) still holds. + // + // TODO: replace this empirical hack with documented per-tracker InstallationState=4 once orbit reports + // setup-experience progress via the LocalMDM channel (subtask + // https://github.com/fleetdm/fleet/issues/43776). At that point each setup-experience software item becomes + // a TrackedResourceTypes/{tracker} on the device, a failed install reports InstallationError on its tracker, + // and the ESP renders the failure UI natively from per-tracker state -- no timeout trick required. Setting + // InstallationState=4 on the parent PolicyProviders node alone (as we tested) does NOT escalate the UI + // without trackers underneath, which is why we keep the timeout-based approach for now. + newSyncMLCmdInt(fleet.CmdReplace, + fmt.Sprintf("./Device/Vendor/MSFT/DMClient/Provider/%s/FirstSyncStatus/TimeOutUntilSyncFailure", provID), + "1"), + } + for _, cmd := range cmds { + cmd.CmdID = mdm_types.CmdID{Value: uuid.New().String()} + } + return cmds +} + +// buildESPReleaseCommands builds SyncML commands that release the device +// from the ESP. The release path advances DevicePreparation to "complete" and +// signals ServerHasFinishedProvisioning so Windows proceeds to login. +func buildESPReleaseCommands(provID string) []*mdm_types.SyncMLCmd { + cmds := []*mdm_types.SyncMLCmd{ newSyncMLCmdInt(fleet.CmdReplace, fmt.Sprintf("./Device/Vendor/MSFT/EnrollmentStatusTracking/DevicePreparation/PolicyProviders/%s/InstallationState", provID), "3"), newSyncMLCmdBool(fleet.CmdReplace, @@ -2162,34 +2474,52 @@ func (svc *Service) handleESPRelease(ctx context.Context, device *fleet.MDMWindo newSyncMLCmdBool(fleet.CmdReplace, fmt.Sprintf("./User/Vendor/MSFT/DMClient/Provider/%s/FirstSyncStatus/ServerHasFinishedProvisioning", provID), "true"), } - for _, cmd := range releaseCmds { + for _, cmd := range cmds { cmd.CmdID = mdm_types.CmdID{Value: uuid.New().String()} } + return cmds +} - // Persist the ServerHasFinishedProvisioning command as a backup. - // If the inline response is delivered, the device acks and the - // persisted command is cleared. If the response is dropped, the - // command is resent automatically on the next management session. - finishedProvisioningURI := fmt.Sprintf( - "./Device/Vendor/MSFT/DMClient/Provider/%s/FirstSyncStatus/ServerHasFinishedProvisioning", provID) - releaseCmd := newSyncMLCmdBool(fleet.CmdReplace, finishedProvisioningURI, "true") - releaseCmd.CmdID = mdm_types.CmdID{Value: uuid.New().String()} - rawXML, err := xml.Marshal(releaseCmd) - if err != nil { - svc.logger.WarnContext(ctx, "ESP: failed to marshal release command for persistence", "err", err) - } else { - persistCmd := &fleet.MDMWindowsCommand{ - CommandUUID: releaseCmd.CmdID.Value, +// persistESPFinalCommands stores backup copies of every finalization command +// so the existing command-retry infrastructure can resend them if this +// response is dropped. The persisted commands intentionally reuse the same +// CmdIDs as the inline commands: when the device acks the inline send, +// MDMWindowsSaveResponse will match those CmdRefs against the persisted +// rows and clear them, so the backup is only resent if delivery actually +// failed. All commands are idempotent Replaces, so even a duplicate delivery +// is safe. +func (svc *Service) persistESPFinalCommands(ctx context.Context, hostUUID string, cmds []*mdm_types.SyncMLCmd) error { + persistCmds := make([]*fleet.MDMWindowsCommand, 0, len(cmds)) + for _, cmd := range cmds { + // Skip commands without a target URI -- shouldn't happen for the commands we build, but guard against nil-deref. + targetURI := cmd.GetTargetURI() + if targetURI == "" || len(cmd.Items) == 0 { + continue + } + rawXML, err := xml.Marshal(cmd) + if err != nil { + // Marshal of a SyncMLCmd we just built is a deterministic code bug, not a transient failure. Returning an + // error here would just loop forever (every retry hits the same bug); log + Handle + wrapped := ctxerr.Wrap(ctx, err, "marshal ESP final command for persistence") + svc.logger.ErrorContext(ctx, "ESP: failed to marshal final command for persistence", + "err", wrapped, "target_uri", targetURI) + ctxerr.Handle(ctx, wrapped) + continue + } + persistCmds = append(persistCmds, &fleet.MDMWindowsCommand{ + CommandUUID: cmd.CmdID.Value, RawCommand: rawXML, - TargetLocURI: finishedProvisioningURI, - } - if err := svc.ds.MDMWindowsInsertCommandForHosts(ctx, []string{device.HostUUID}, persistCmd); err != nil { - svc.logger.WarnContext(ctx, "ESP: failed to persist release command", "err", err) - } + TargetLocURI: targetURI, + }) } - - svc.logger.InfoContext(ctx, "ESP: releasing device from setup", "device_id", device.MDMDeviceID, "host_uuid", device.HostUUID, "timed_out", timedOut) - return releaseCmds, nil + if len(persistCmds) == 0 { + return nil + } + // Single transactional insert: either every backup row is committed or none. + if err := svc.ds.MDMWindowsInsertCommandsForHost(ctx, hostUUID, persistCmds); err != nil { + return ctxerr.Wrap(ctx, err, "persist ESP finalization commands") + } + return nil } // removeWindowsDeviceIfAlreadyMDMEnrolled removes the device if already MDM enrolled diff --git a/server/service/microsoft_mdm_property_test.go b/server/service/microsoft_mdm_property_test.go new file mode 100644 index 0000000000..f331e0c91e --- /dev/null +++ b/server/service/microsoft_mdm_property_test.go @@ -0,0 +1,356 @@ +package service + +import ( + "context" + "fmt" + "log/slog" + "slices" + "strings" + "testing" + "time" + + "github.com/fleetdm/fleet/v4/server/fleet" + microsoft_mdm "github.com/fleetdm/fleet/v4/server/mdm/microsoft" + "github.com/fleetdm/fleet/v4/server/mock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "pgregory.net/rapid" +) + +// Property-based tests for handleESPRelease. They cover the wait-gate decision and the universal block/release +// command-shape invariants in a single combined property check. +// +// The spec function pbtESPSpec computes the expected (decision, observedHasFailure) from the inputs without +// referencing the production code. We then run getESPCommands against a mock datastore and assert: +// +// - Decision (wait/block/release) matches the spec. +// - Wait → no side effects (no cancel, no persist, no CAS). +// - Block path command shape: BlockInStatusPage=1, AllowCollectLogsButton, TimeOutUntilSyncFailure=1, +// reason-specific CustomErrorText, NO ServerHasFinishedProvisioning, NO InstallationState. +// - Release path command shape: ServerHasFinishedProvisioning, NO CustomErrorText, NO BlockInStatusPage. +// - Persisted CommandUUIDs equal inline CmdID.Value (the ack-clearing invariant). +// - Persist runs as a single batched call (a regression that loops single inserts would split CustomErrorText +// and the block flags across multiple TX boundaries). +// - Cancel block fires iff (timedOut || (observedHasFailure && requireAll)); when it fires, +// CancelHostUpcomingActivity is called once per Pending/Running row in input. Cancel-upcoming runs strictly +// before cancel-status; both run strictly before persist; persist runs strictly before CAS. +// +// Order independence is implicit: pbtESPSpec is a pure function of the multiset of statuses (no positional +// dependency) and rapid samples many orderings, so any introduced order-dependence in production code +// surfaces as a spec mismatch. +// +// Run with more checks: +// go test -run TestPBT_HandleESPRelease ./server/service/ -args -rapid.checks=2000 + +var pbtESPLogger = slog.New(slog.DiscardHandler) + +const ( + pbtESPDeviceID = "pbt-esp-device-id" + pbtESPHostUUID = "pbt-esp-host-uuid" +) + +// pbtESPTrace captures observable side effects of handleESPRelease so the property can assert ordering and +// counts without inspecting the auto-set FuncInvoked flags individually. +type pbtESPTrace struct { + cancelUpcomingExecIDs []string // execution IDs passed to CancelHostUpcomingActivity, in call order + persistedCmdUUIDs []string + callOrder []string // sequence of "cancel-upcoming", "cancel-status", "persist", "cas" +} + +// newPBTESPSvc wires a mock datastore for property-testing handleESPRelease. Stages 1 and 2 (profiles) are +// mocked empty so the property focuses on Stage 3 + finalize. Profile-stage logic is covered by the +// example-based subtests in TestGetESPCommands. +func newPBTESPSvc( + statuses []fleet.SetupExperienceStatusResultStatus, timedOut, requireAll bool, +) (*Service, *fleet.MDMWindowsEnrolledDevice, *pbtESPTrace) { + ds := new(mock.Store) + trace := &pbtESPTrace{} + + osqueryHostID := "pbt-esp-osq" + ds.HostLiteByIdentifierFunc = func(ctx context.Context, id string) (*fleet.HostLite, error) { + return &fleet.HostLite{ID: 1, UUID: id, OsqueryHostID: &osqueryHostID, TeamID: nil}, nil + } + ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) { + ac := &fleet.AppConfig{} + ac.MDM.MacOSSetup.RequireAllSoftwareWindows = requireAll + return ac, nil + } + + // Skip Stages 1 and 2: profiles are out of PBT scope. + ds.ListMDMWindowsProfilesToInstallForHostFunc = func(ctx context.Context, hUUID string) ([]*fleet.MDMWindowsProfilePayload, error) { + return nil, nil + } + ds.GetHostMDMWindowsProfilesFunc = func(ctx context.Context, hUUID string) ([]fleet.HostMDMWindowsProfile, error) { + return nil, nil + } + + // Stage 3 results plus the cancel-block re-list both go through this single mock so they're consistent. + // Each row gets a HostSoftwareInstallsExecutionID so the cancel-upcoming loop has something to cancel on + // non-terminal rows. + results := make([]*fleet.SetupExperienceStatusResult, 0, len(statuses)) + for i, s := range statuses { + execID := fmt.Sprintf("pbt-exec-%d", i) + results = append(results, &fleet.SetupExperienceStatusResult{ + Name: fmt.Sprintf("item-%d", i), + Status: s, + HostSoftwareInstallsExecutionID: &execID, + }) + } + ds.ListSetupExperienceResultsByHostUUIDFunc = func(ctx context.Context, hUUID string, teamID uint) ([]*fleet.SetupExperienceStatusResult, error) { + return results, nil + } + // The empty-results disambiguation case (empty results + has_items=true → wait) is covered by + // "active waits when results empty but setup experience configured" in TestGetESPCommands; here we always + // say "no items configured" so empty input proceeds to finalize cleanly. + ds.HasWindowsSetupExperienceItemsForTeamFunc = func(ctx context.Context, teamID uint) (bool, error) { + return false, nil + } + + // Side-effect hooks. callOrder lets the property assert the cancel-upcoming → cancel-status → persist → + // cas ordering safely. + ds.CancelHostUpcomingActivityFunc = func(ctx context.Context, hostID uint, executionID string) (fleet.ActivityDetails, error) { + trace.cancelUpcomingExecIDs = append(trace.cancelUpcomingExecIDs, executionID) + trace.callOrder = append(trace.callOrder, "cancel-upcoming") + return nil, nil + } + ds.CancelPendingSetupExperienceStepsFunc = func(ctx context.Context, hUUID string) error { + trace.callOrder = append(trace.callOrder, "cancel-status") + return nil + } + ds.MDMWindowsInsertCommandsForHostFunc = func(ctx context.Context, hostUUIDOrDeviceID string, cmds []*fleet.MDMWindowsCommand) error { + trace.callOrder = append(trace.callOrder, "persist") + for _, c := range cmds { + trace.persistedCmdUUIDs = append(trace.persistedCmdUUIDs, c.CommandUUID) + } + return nil + } + ds.SetMDMWindowsAwaitingConfigurationFunc = func(ctx context.Context, mdmDeviceID string, from, to fleet.WindowsMDMAwaitingConfiguration) (bool, error) { + trace.callOrder = append(trace.callOrder, "cas") + return true, nil + } + + svc := &Service{ds: ds, logger: pbtESPLogger} + + device := &fleet.MDMWindowsEnrolledDevice{ + MDMDeviceID: pbtESPDeviceID, + HostUUID: pbtESPHostUUID, + AwaitingConfiguration: fleet.WindowsMDMAwaitingConfigurationActive, + } + if timedOut { + past := time.Now().Add(-4 * time.Hour) + device.AwaitingConfigurationAt = &past + } + return svc, device, trace +} + +type pbtESPDecision string + +const ( + pbtESPWait pbtESPDecision = "wait" + pbtESPBlock pbtESPDecision = "block" + pbtESPRelease pbtESPDecision = "release" +) + +// pbtESPSpec computes the expected outcome from the inputs without referencing production code. It returns +// the decision and the hasSoftwareFailure that production would observe. The latter differs from "results +// contain Failure" when timedOut is true, because Stage 3 is skipped in that case so the production variable +// stays at its zero value. +func pbtESPSpec( + statuses []fleet.SetupExperienceStatusResultStatus, timedOut, requireAll bool, +) (decision pbtESPDecision, observedHasFailure bool) { + var inputAnyInFlight, inputHasFailure bool + for _, s := range statuses { + switch s { + case fleet.SetupExperienceStatusFailure: + inputHasFailure = true + case fleet.SetupExperienceStatusPending, fleet.SetupExperienceStatusRunning: + inputAnyInFlight = true + } + } + if timedOut { + // Wait gates skipped; finalize directly. observedHasFailure stays at its zero value because Stage 3 + // never ran. + if requireAll { + return pbtESPBlock, false + } + return pbtESPRelease, false + } + // !timedOut: Stage 3 ran. observedHasFailure = inputHasFailure. + if inputAnyInFlight { + if !inputHasFailure { + return pbtESPWait, inputHasFailure + } + if !requireAll { + return pbtESPWait, inputHasFailure + } + return pbtESPBlock, inputHasFailure // short-circuit: failure + require_all + in-flight siblings -> block now + } + if inputHasFailure && requireAll { + return pbtESPBlock, inputHasFailure + } + return pbtESPRelease, inputHasFailure +} + +// pbtFindCmdByLocURI returns the first command whose target LocURI contains the given substring. +func pbtFindCmdByLocURI(cmds []*fleet.SyncMLCmd, substr string) *fleet.SyncMLCmd { + for _, c := range cmds { + if c.GetTargetURI() != "" && strings.Contains(c.GetTargetURI(), substr) { + return c + } + } + return nil +} + +func TestPBT_HandleESPRelease(t *testing.T) { + statusGen := rapid.SampledFrom([]fleet.SetupExperienceStatusResultStatus{ + fleet.SetupExperienceStatusPending, + fleet.SetupExperienceStatusRunning, + fleet.SetupExperienceStatusSuccess, + fleet.SetupExperienceStatusFailure, + fleet.SetupExperienceStatusCancelled, + }) + + rapid.Check(t, func(rt *rapid.T) { + statuses := rapid.SliceOfN(statusGen, 0, 8).Draw(rt, "statuses") + timedOut := rapid.Bool().Draw(rt, "timedOut") + requireAll := rapid.Bool().Draw(rt, "requireAll") + + expected, observedHasFailure := pbtESPSpec(statuses, timedOut, requireAll) + svc, device, trace := newPBTESPSvc(statuses, timedOut, requireAll) + cmds, err := svc.getESPCommands(t.Context(), device) + require.NoErrorf(rt, err, "statuses=%v timedOut=%v requireAll=%v", statuses, timedOut, requireAll) + + if expected == pbtESPWait { + require.Nilf(rt, cmds, "expected wait, got cmds=%+v", cmds) + require.Emptyf(rt, trace.callOrder, "wait must not produce side effects; got %v", trace.callOrder) + return + } + + // Block or release: must produce non-empty cmds plus a finalized side-effect sequence. + require.NotEmptyf(rt, cmds, "expected %s, got nil/empty", expected) + + // Persisted CommandUUIDs equal inline CmdID.Value, 1-to-1. Without this, the device's ack of the + // inline command does not clear the backup row and the server re-sends every subsequent session. + inlineCmdUUIDs := make([]string, 0, len(cmds)) + for _, c := range cmds { + inlineCmdUUIDs = append(inlineCmdUUIDs, c.CmdID.Value) + } + assert.ElementsMatchf(rt, inlineCmdUUIDs, trace.persistedCmdUUIDs, + "persisted CommandUUIDs must equal inline CmdID.Value (1-to-1)") + + // Persist must be a single batched call. A regression that loops single inserts would split the + // CustomErrorText / BlockInStatusPage / TimeOutUntilSyncFailure flags across multiple transactions and + // expose orphan rows on partial-fail-then-retry. + persistCount := 0 + for _, ev := range trace.callOrder { + if ev == "persist" { + persistCount++ + } + } + require.Equalf(rt, 1, persistCount, "persist must be a single batched call; callOrder=%v", trace.callOrder) + + switch expected { + case pbtESPBlock: + // Block path NEVER includes ServerHasFinishedProvisioning -- that command would tell Windows the + // ESP succeeded and proceed past the failure UI. Also NEVER InstallationState alone (VM testing + // confirmed setting it on the parent PolicyProviders node without per-tracker state from #43776 + // does not escalate the failure UI). + assert.Nilf(rt, pbtFindCmdByLocURI(cmds, "ServerHasFinishedProvisioning"), + "block path must NOT include ServerHasFinishedProvisioning") + assert.Nilf(rt, pbtFindCmdByLocURI(cmds, "InstallationState"), + "block path uses the timeout-based trigger, not InstallationState") + // Block path always includes BlockInStatusPage=1 (Reset PC), AllowCollectLogsButton, and + // TimeOutUntilSyncFailure=1 (one minute, forces failure UI). + blockCmd := pbtFindCmdByLocURI(cmds, "BlockInStatusPage") + require.NotNilf(rt, blockCmd, "block path must include BlockInStatusPage") + require.NotNilf(rt, blockCmd.Items[0].Data, "BlockInStatusPage must have data") + assert.Equalf(rt, "1", blockCmd.Items[0].Data.Content, + "BlockInStatusPage must be 1 (Reset PC) per DMClient CSP docs") + assert.NotNilf(rt, pbtFindCmdByLocURI(cmds, "AllowCollectLogsButton"), + "block path must include AllowCollectLogsButton") + timeoutCmd := pbtFindCmdByLocURI(cmds, "TimeOutUntilSyncFailure") + require.NotNilf(rt, timeoutCmd, "block path must include TimeOutUntilSyncFailure") + require.NotNilf(rt, timeoutCmd.Items[0].Data, "TimeOutUntilSyncFailure must have data") + assert.Equalf(rt, "1", timeoutCmd.Items[0].Data.Content, + "TimeOutUntilSyncFailure must be 1 minute (force quick failure)") + // errorText is software-failure text iff observedHasFailure (Stage 3 ran AND saw a Failure); else + // timeout text. The pure-timeout path lands here too with observedHasFailure=false. + errCmd := pbtFindCmdByLocURI(cmds, "CustomErrorText") + require.NotNilf(rt, errCmd, "block path must include CustomErrorText") + require.NotNilf(rt, errCmd.Items[0].Data, "CustomErrorText must have data") + if observedHasFailure { + assert.Equalf(rt, microsoft_mdm.ESPSoftwareFailureErrorText, errCmd.Items[0].Data.Content, + "block on software failure must use software-failure error text") + } else { + assert.Equalf(rt, microsoft_mdm.ESPTimeoutErrorText, errCmd.Items[0].Data.Content, + "block on pure timeout must use timeout error text") + } + case pbtESPRelease: + // Release path NEVER includes CustomErrorText -- the failure UI never renders on a release, so + // any error text would be dead state on the DMClient node. + assert.Nilf(rt, pbtFindCmdByLocURI(cmds, "CustomErrorText"), + "release path must NOT include CustomErrorText") + assert.NotNilf(rt, pbtFindCmdByLocURI(cmds, "ServerHasFinishedProvisioning"), + "release path must include ServerHasFinishedProvisioning") + assert.Nilf(rt, pbtFindCmdByLocURI(cmds, "BlockInStatusPage"), + "release path must NOT include BlockInStatusPage") + } + + // Cancel-block invariants. The cancel block fires iff (timedOut || (observedHasFailure && requireAll)) + // -- when timedOut, Stage 3 is skipped so observedHasFailure=false and the OR's first operand carries + // the condition. + expectedCancelBlock := timedOut || (observedHasFailure && requireAll) + expectedCancelUpcomingCount := 0 + if expectedCancelBlock { + for _, s := range statuses { + if s == fleet.SetupExperienceStatusPending || s == fleet.SetupExperienceStatusRunning { + expectedCancelUpcomingCount++ + } + } + } + actualCancelStatus := slices.Contains(trace.callOrder, "cancel-status") + assert.Equalf(rt, expectedCancelBlock, actualCancelStatus, + "cancel-status invocation must match expected cancel-block fire condition") + assert.Lenf(rt, trace.cancelUpcomingExecIDs, expectedCancelUpcomingCount, + "cancel-upcoming count must equal Pending+Running rows in input when cancel block fires") + + // Ordering: cancel-upcoming must precede cancel-status (queue cleanup before status table update so + // a mid-loop crash + retry sees the same pending statuses and can re-cancel). Both must precede + // persist (a transient cancel failure aborts the finalize cleanly). Persist must precede CAS (the + // dropped-response retry safety net runs before we commit awaiting=None). + var lastCancelUpcoming, firstCancelStatus, firstPersist, firstCas int = -1, -1, -1, -1 + for i, ev := range trace.callOrder { + switch ev { + case "cancel-upcoming": + lastCancelUpcoming = i + case "cancel-status": + if firstCancelStatus == -1 { + firstCancelStatus = i + } + case "persist": + if firstPersist == -1 { + firstPersist = i + } + case "cas": + if firstCas == -1 { + firstCas = i + } + } + } + require.NotEqualf(rt, -1, firstPersist, "persist must run for non-wait outcomes") + require.NotEqualf(rt, -1, firstCas, "CAS must run for non-wait outcomes") + require.Lessf(rt, firstPersist, firstCas, "persist must run before CAS; callOrder=%v", trace.callOrder) + if lastCancelUpcoming != -1 && firstCancelStatus != -1 { + require.Lessf(rt, lastCancelUpcoming, firstCancelStatus, + "cancel-upcoming must run before cancel-status; callOrder=%v", trace.callOrder) + } + if firstCancelStatus != -1 { + require.Lessf(rt, firstCancelStatus, firstPersist, + "cancel-status must run before persist; callOrder=%v", trace.callOrder) + } + if lastCancelUpcoming != -1 { + require.Lessf(rt, lastCancelUpcoming, firstPersist, + "cancel-upcoming must run before persist; callOrder=%v", trace.callOrder) + } + }) +} diff --git a/server/service/microsoft_mdm_test.go b/server/service/microsoft_mdm_test.go index 355d354094..b0a055caec 100644 --- a/server/service/microsoft_mdm_test.go +++ b/server/service/microsoft_mdm_test.go @@ -1283,48 +1283,100 @@ func TestGetESPCommands(t *testing.T) { const deviceID = "test-device-id" const hostUUID = "test-host-uuid" + // newSvc returns a mock-backed Service with every datastore method handleESPRelease can call defaulted to a + // no-op success return. Tests override ONLY the methods whose specific behavior they care about, which + // keeps each subtest focused on its one variable instead of being a wall of mock-setup boilerplate. + // + // Tests that need a method to return an error / different value / track invocations install their own + // override; tests that need to assert "this method must NOT be called" install a t.Fatal override or + // assert ds.Invoked == false (the auto-set flag is independent of the func body). newSvc := func(t *testing.T) (*mock.Store, *Service) { ds := new(mock.Store) + // HostLiteByIdentifier exposes OsqueryHostID so Stage 3's setupExperienceHostUUID() resolves to the same + // key Windows orbit uses as setup_experience_status_results.host_uuid (production data shape). + osqueryHostID := "osquery-" + hostUUID + ds.HostLiteByIdentifierFunc = func(ctx context.Context, identifier string) (*fleet.HostLite, error) { + return &fleet.HostLite{ID: 1, UUID: identifier, OsqueryHostID: &osqueryHostID, TeamID: nil}, nil + } + // Stage 1, 2, 3 listings default empty so the wait gates pass through to finalize cleanly. + ds.ListMDMWindowsProfilesToInstallForHostFunc = func(ctx context.Context, hUUID string) ([]*fleet.MDMWindowsProfilePayload, error) { + return nil, nil + } + ds.GetHostMDMWindowsProfilesFunc = func(ctx context.Context, hUUID string) ([]fleet.HostMDMWindowsProfile, error) { + return nil, nil + } + ds.ListSetupExperienceResultsByHostUUIDFunc = func(ctx context.Context, hUUID string, teamID uint) ([]*fleet.SetupExperienceStatusResult, error) { + return nil, nil + } + // No setup-experience items configured: empty Stage 3 disambiguates to "safe to release". Tests that + // expect waiting due to items configured override this to return true. + ds.HasWindowsSetupExperienceItemsForTeamFunc = func(ctx context.Context, teamID uint) (bool, error) { + return false, nil + } + // require_all_software_windows defaults to false via the no-team / app-config path. setRequireAll(ds, true) + // flips it for tests that need require_all=true. + ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) { + return &fleet.AppConfig{}, nil + } + // Finalize side-effects: default no-op success. Tests that need to capture, fail, or assert ordering + // install their own override. + ds.MDMWindowsInsertCommandsForHostFunc = func(ctx context.Context, hostUUIDOrDeviceID string, cmds []*fleet.MDMWindowsCommand) error { + return nil + } + ds.SetMDMWindowsAwaitingConfigurationFunc = func(ctx context.Context, mdmDeviceID string, from, to fleet.WindowsMDMAwaitingConfiguration) (bool, error) { + return true, nil + } + ds.CancelPendingSetupExperienceStepsFunc = func(ctx context.Context, hUUID string) error { + return nil + } + ds.CancelHostUpcomingActivityFunc = func(ctx context.Context, hostID uint, executionID string) (fleet.ActivityDetails, error) { + return nil, nil + } return ds, &Service{ds: ds, logger: testutils.TestLogger(t)} } + // newActiveDevice returns the most common device fixture used by these tests: AwaitingConfiguration=Active + // with the standard test deviceID/hostUUID. Tests that need a different state (Pending, None) or a timeout + // timestamp construct their own struct literal. + newActiveDevice := func() *fleet.MDMWindowsEnrolledDevice { + return &fleet.MDMWindowsEnrolledDevice{ + MDMDeviceID: deviceID, + HostUUID: hostUUID, + AwaitingConfiguration: fleet.WindowsMDMAwaitingConfigurationActive, + } + } + t.Run("no awaiting configuration returns nil", func(t *testing.T) { - ds, svc := newSvc(t) - ds.MDMWindowsGetEnrolledDeviceWithDeviceIDFunc = func(ctx context.Context, mdmDeviceID string) (*fleet.MDMWindowsEnrolledDevice, error) { - return &fleet.MDMWindowsEnrolledDevice{ - MDMDeviceID: deviceID, - AwaitingConfiguration: fleet.WindowsMDMAwaitingConfigurationNone, - }, nil + _, svc := newSvc(t) + device := &fleet.MDMWindowsEnrolledDevice{ + MDMDeviceID: deviceID, + AwaitingConfiguration: fleet.WindowsMDMAwaitingConfigurationNone, } - cmds, err := svc.getESPCommands(t.Context(), deviceID) + cmds, err := svc.getESPCommands(t.Context(), device) require.NoError(t, err) assert.Nil(t, cmds) }) t.Run("pending without host UUID sends hold commands", func(t *testing.T) { - ds, svc := newSvc(t) - ds.MDMWindowsGetEnrolledDeviceWithDeviceIDFunc = func(ctx context.Context, mdmDeviceID string) (*fleet.MDMWindowsEnrolledDevice, error) { - return &fleet.MDMWindowsEnrolledDevice{ - MDMDeviceID: deviceID, - HostUUID: "", - AwaitingConfiguration: fleet.WindowsMDMAwaitingConfigurationPending, - }, nil + _, svc := newSvc(t) + device := &fleet.MDMWindowsEnrolledDevice{ + MDMDeviceID: deviceID, + HostUUID: "", + AwaitingConfiguration: fleet.WindowsMDMAwaitingConfigurationPending, } - cmds, err := svc.getESPCommands(t.Context(), deviceID) + cmds, err := svc.getESPCommands(t.Context(), device) require.NoError(t, err) require.NotEmpty(t, cmds, "should return hold commands") }) t.Run("pending with host UUID transitions to active", func(t *testing.T) { ds, svc := newSvc(t) - ds.MDMWindowsGetEnrolledDeviceWithDeviceIDFunc = func(ctx context.Context, mdmDeviceID string) (*fleet.MDMWindowsEnrolledDevice, error) { - return &fleet.MDMWindowsEnrolledDevice{ - MDMDeviceID: deviceID, - HostUUID: hostUUID, - AwaitingConfiguration: fleet.WindowsMDMAwaitingConfigurationPending, - }, nil + device := &fleet.MDMWindowsEnrolledDevice{ + MDMDeviceID: deviceID, + HostUUID: hostUUID, + AwaitingConfiguration: fleet.WindowsMDMAwaitingConfigurationPending, } transitioned := false ds.SetMDMWindowsAwaitingConfigurationFunc = func(ctx context.Context, mdmDeviceID string, from, to fleet.WindowsMDMAwaitingConfiguration) (bool, error) { @@ -1332,7 +1384,7 @@ func TestGetESPCommands(t *testing.T) { return true, nil } - cmds, err := svc.getESPCommands(t.Context(), deviceID) + cmds, err := svc.getESPCommands(t.Context(), device) require.NoError(t, err) require.NotEmpty(t, cmds, "should return DevicePreparation completed command") assert.True(t, transitioned) @@ -1340,124 +1392,316 @@ func TestGetESPCommands(t *testing.T) { t.Run("active with pending profiles waits", func(t *testing.T) { ds, svc := newSvc(t) - ds.MDMWindowsGetEnrolledDeviceWithDeviceIDFunc = func(ctx context.Context, mdmDeviceID string) (*fleet.MDMWindowsEnrolledDevice, error) { - return &fleet.MDMWindowsEnrolledDevice{ - MDMDeviceID: deviceID, - HostUUID: hostUUID, - AwaitingConfiguration: fleet.WindowsMDMAwaitingConfigurationActive, - }, nil - } - ds.ListMDMWindowsProfilesToInstallForHostFunc = func(ctx context.Context, hUUID string) ([]*fleet.MDMWindowsProfilePayload, error) { - return nil, nil // reconciler already ran - } ds.GetHostMDMWindowsProfilesFunc = func(ctx context.Context, hUUID string) ([]fleet.HostMDMWindowsProfile, error) { return []fleet.HostMDMWindowsProfile{ {ProfileUUID: "prof-1", Name: "WiFi", Status: &fleet.MDMDeliveryPending, OperationType: fleet.MDMOperationTypeInstall}, }, nil } - cmds, err := svc.getESPCommands(t.Context(), deviceID) + cmds, err := svc.getESPCommands(t.Context(), newActiveDevice()) require.NoError(t, err) assert.Nil(t, cmds, "should wait while profiles are pending") }) t.Run("active with verifying profiles waits", func(t *testing.T) { ds, svc := newSvc(t) - ds.MDMWindowsGetEnrolledDeviceWithDeviceIDFunc = func(ctx context.Context, mdmDeviceID string) (*fleet.MDMWindowsEnrolledDevice, error) { - return &fleet.MDMWindowsEnrolledDevice{ - MDMDeviceID: deviceID, - HostUUID: hostUUID, - AwaitingConfiguration: fleet.WindowsMDMAwaitingConfigurationActive, - }, nil - } - ds.ListMDMWindowsProfilesToInstallForHostFunc = func(ctx context.Context, hUUID string) ([]*fleet.MDMWindowsProfilePayload, error) { - return nil, nil // reconciler already ran - } ds.GetHostMDMWindowsProfilesFunc = func(ctx context.Context, hUUID string) ([]fleet.HostMDMWindowsProfile, error) { return []fleet.HostMDMWindowsProfile{ {ProfileUUID: "prof-1", Name: "WiFi", Status: &fleet.MDMDeliveryVerifying, OperationType: fleet.MDMOperationTypeInstall}, }, nil } - cmds, err := svc.getESPCommands(t.Context(), deviceID) + cmds, err := svc.getESPCommands(t.Context(), newActiveDevice()) require.NoError(t, err) assert.Nil(t, cmds, "should wait while profiles are verifying") }) t.Run("active waits when profiles not yet queued by reconciler", func(t *testing.T) { ds, svc := newSvc(t) - ds.MDMWindowsGetEnrolledDeviceWithDeviceIDFunc = func(ctx context.Context, mdmDeviceID string) (*fleet.MDMWindowsEnrolledDevice, error) { - return &fleet.MDMWindowsEnrolledDevice{ - MDMDeviceID: deviceID, - HostUUID: hostUUID, - AwaitingConfiguration: fleet.WindowsMDMAwaitingConfigurationActive, - }, nil - } ds.ListMDMWindowsProfilesToInstallForHostFunc = func(ctx context.Context, hUUID string) ([]*fleet.MDMWindowsProfilePayload, error) { return []*fleet.MDMWindowsProfilePayload{ {ProfileUUID: "prof-1", ProfileName: "WiFi"}, }, nil } - cmds, err := svc.getESPCommands(t.Context(), deviceID) + cmds, err := svc.getESPCommands(t.Context(), newActiveDevice()) require.NoError(t, err) assert.Nil(t, cmds, "should wait when profiles are configured but not yet queued") assert.False(t, ds.GetHostMDMWindowsProfilesFuncInvoked, "should not check delivery status when profiles not yet queued") }) + // setRequireAll flips the require_all_software_windows lookup to the given value via the no-team / + // app-config path. Default in newSvc is false; call this with true when a test needs require_all=true. The + // team-config path is covered explicitly by its own subtest. + setRequireAll := func(ds *mock.Store, requireAll bool) { + ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) { + ac := &fleet.AppConfig{} + ac.MDM.MacOSSetup.RequireAllSoftwareWindows = requireAll + return ac, nil + } + } + t.Run("active with all profiles delivered releases device", func(t *testing.T) { ds, svc := newSvc(t) - ds.MDMWindowsGetEnrolledDeviceWithDeviceIDFunc = func(ctx context.Context, mdmDeviceID string) (*fleet.MDMWindowsEnrolledDevice, error) { - return &fleet.MDMWindowsEnrolledDevice{ - MDMDeviceID: deviceID, - HostUUID: hostUUID, - AwaitingConfiguration: fleet.WindowsMDMAwaitingConfigurationActive, - }, nil - } - ds.ListMDMWindowsProfilesToInstallForHostFunc = func(ctx context.Context, hUUID string) ([]*fleet.MDMWindowsProfilePayload, error) { - return nil, nil - } ds.GetHostMDMWindowsProfilesFunc = func(ctx context.Context, hUUID string) ([]fleet.HostMDMWindowsProfile, error) { return []fleet.HostMDMWindowsProfile{ {ProfileUUID: "prof-1", Name: "WiFi", Status: &fleet.MDMDeliveryVerified, OperationType: fleet.MDMOperationTypeInstall}, }, nil } - ds.SetMDMWindowsAwaitingConfigurationFunc = func(ctx context.Context, mdmDeviceID string, from, to fleet.WindowsMDMAwaitingConfiguration) (bool, error) { - return true, nil - } - ds.MDMWindowsInsertCommandForHostsFunc = func(ctx context.Context, hostUUIDs []string, cmd *fleet.MDMWindowsCommand) error { + // Capture ordering: persist must run BEFORE the CAS so a persist failure can't leave the device finalized + // without the dropped-response retry safety net. + persisted := false + ds.MDMWindowsInsertCommandsForHostFunc = func(ctx context.Context, hostUUIDOrDeviceID string, cmds []*fleet.MDMWindowsCommand) error { + persisted = true return nil } + ds.SetMDMWindowsAwaitingConfigurationFunc = func(ctx context.Context, mdmDeviceID string, from, to fleet.WindowsMDMAwaitingConfiguration) (bool, error) { + require.True(t, persisted, "persist must run BEFORE CAS Active->None") + return true, nil + } - cmds, err := svc.getESPCommands(t.Context(), deviceID) + cmds, err := svc.getESPCommands(t.Context(), newActiveDevice()) require.NoError(t, err) require.NotEmpty(t, cmds, "should return release commands") + assert.True(t, ds.MDMWindowsInsertCommandsForHostFuncInvoked, + "release path must persist final commands as the dropped-response retry backup") + assert.True(t, ds.SetMDMWindowsAwaitingConfigurationFuncInvoked, + "should transition awaiting_configuration out of Active") }) t.Run("active with no profiles releases device", func(t *testing.T) { ds, svc := newSvc(t) - ds.MDMWindowsGetEnrolledDeviceWithDeviceIDFunc = func(ctx context.Context, mdmDeviceID string) (*fleet.MDMWindowsEnrolledDevice, error) { - return &fleet.MDMWindowsEnrolledDevice{ - MDMDeviceID: deviceID, - HostUUID: hostUUID, - AwaitingConfiguration: fleet.WindowsMDMAwaitingConfigurationActive, - }, nil - } - ds.ListMDMWindowsProfilesToInstallForHostFunc = func(ctx context.Context, hUUID string) ([]*fleet.MDMWindowsProfilePayload, error) { - return nil, nil - } - ds.GetHostMDMWindowsProfilesFunc = func(ctx context.Context, hUUID string) ([]fleet.HostMDMWindowsProfile, error) { - return nil, nil - } - ds.SetMDMWindowsAwaitingConfigurationFunc = func(ctx context.Context, mdmDeviceID string, from, to fleet.WindowsMDMAwaitingConfiguration) (bool, error) { - return true, nil - } - ds.MDMWindowsInsertCommandForHostsFunc = func(ctx context.Context, hostUUIDs []string, cmd *fleet.MDMWindowsCommand) error { - return nil - } - cmds, err := svc.getESPCommands(t.Context(), deviceID) + cmds, err := svc.getESPCommands(t.Context(), newActiveDevice()) require.NoError(t, err) require.NotEmpty(t, cmds, "should return release commands when no profiles configured") + assert.True(t, ds.SetMDMWindowsAwaitingConfigurationFuncInvoked, + "should transition awaiting_configuration out of Active") + }) + + // findCmdByLocURI returns the first SyncMLCmd whose target LocURI contains + // the given substring, or nil if none match. + findCmdByLocURI := func(cmds []*fleet.SyncMLCmd, substr string) *fleet.SyncMLCmd { + for _, c := range cmds { + if c.GetTargetURI() != "" && strings.Contains(c.GetTargetURI(), substr) { + return c + } + } + return nil + } + + t.Run("profile failure alone does not block even with require_all=true", func(t *testing.T) { + // Profile delivery failures (e.g. CSP not supported on the host's edition) should not trigger the ESP + // block screen. The require_all_software_windows setting is software-scoped (matching macOS), so a + // failed profile with no software failure must release the device normally. + ds, svc := newSvc(t) + ds.GetHostMDMWindowsProfilesFunc = func(ctx context.Context, hUUID string) ([]fleet.HostMDMWindowsProfile, error) { + return []fleet.HostMDMWindowsProfile{ + {ProfileUUID: "prof-1", Name: "WiFi", Status: &fleet.MDMDeliveryFailed, OperationType: fleet.MDMOperationTypeInstall}, + }, nil + } + setRequireAll(ds, true) + + cmds, err := svc.getESPCommands(t.Context(), newActiveDevice()) + require.NoError(t, err) + require.NotEmpty(t, cmds, "profile failure alone should release the device") + + // Release path: ServerHasFinishedProvisioning is set, BlockInStatusPage is not. + assert.NotNil(t, findCmdByLocURI(cmds, "ServerHasFinishedProvisioning"), + "profile-only failure must release the device") + assert.Nil(t, findCmdByLocURI(cmds, "BlockInStatusPage"), + "profile-only failure must not block the device") + // No software failure and no timeout means no error text on the release. + assert.Nil(t, findCmdByLocURI(cmds, "CustomErrorText"), + "profile-only failure should not surface error text") + // Cancel should NOT be called: profile failures don't trigger cancel. + assert.False(t, ds.CancelPendingSetupExperienceStepsFuncInvoked, + "profile failure must not cancel pending setup experience steps") + }) + + t.Run("profile failure combined with software failure still blocks on software", func(t *testing.T) { + // When BOTH a profile and a software install fail, the software failure still triggers the block (with + // require_all=true) and the software-specific error text wins (because it's more actionable than a + // generic timeout/profile message). + ds, svc := newSvc(t) + ds.GetHostMDMWindowsProfilesFunc = func(ctx context.Context, hUUID string) ([]fleet.HostMDMWindowsProfile, error) { + return []fleet.HostMDMWindowsProfile{ + {ProfileUUID: "prof-1", Name: "WiFi", Status: &fleet.MDMDeliveryFailed, OperationType: fleet.MDMOperationTypeInstall}, + }, nil + } + ds.ListSetupExperienceResultsByHostUUIDFunc = func(ctx context.Context, hUUID string, teamID uint) ([]*fleet.SetupExperienceStatusResult, error) { + return []*fleet.SetupExperienceStatusResult{ + {Name: "Critical App", Status: fleet.SetupExperienceStatusFailure, SoftwareInstallerID: new(uint(7))}, + }, nil + } + setRequireAll(ds, true) + + cmds, err := svc.getESPCommands(t.Context(), newActiveDevice()) + require.NoError(t, err) + require.NotEmpty(t, cmds) + + assert.NotNil(t, findCmdByLocURI(cmds, "BlockInStatusPage"), + "software failure with require_all=true blocks regardless of profile state") + errCmd := findCmdByLocURI(cmds, "CustomErrorText") + require.NotNil(t, errCmd) + require.NotNil(t, errCmd.Items[0].Data) + assert.Equal(t, microsoft_mdm.ESPSoftwareFailureErrorText, errCmd.Items[0].Data.Content, + "software failure error text takes precedence over profile/timeout text") + }) + + t.Run("timeout cancel tolerates upcoming activity already gone", func(t *testing.T) { + // CancelHostUpcomingActivity returns notFound when the row is already absent (e.g., a previous finalize + // attempt cancelled the queue row and crashed before the status table update; the retry sees status + // still Pending and re-tries). Tolerating notFound keeps retries idempotent; anything stricter would + // loop forever on the same checkin until the 3-hour timeout expires server-side. + ds, svc := newSvc(t) + past := time.Now().Add(-4 * time.Hour) + device := newActiveDevice() + device.AwaitingConfigurationAt = &past + ds.ListSetupExperienceResultsByHostUUIDFunc = func(ctx context.Context, hUUID string, teamID uint) ([]*fleet.SetupExperienceStatusResult, error) { + return []*fleet.SetupExperienceStatusResult{ + {Name: "Already Cancelled Queue", Status: fleet.SetupExperienceStatusPending, HostSoftwareInstallsExecutionID: new("exec-gone")}, + }, nil + } + ds.CancelHostUpcomingActivityFunc = func(ctx context.Context, hostID uint, executionID string) (fleet.ActivityDetails, error) { + return nil, newNotFoundError() + } + + _, err := svc.getESPCommands(t.Context(), device) + require.NoError(t, err, + "notFound from CancelHostUpcomingActivity must be tolerated -- otherwise mid-loop crashes loop forever on retry") + assert.True(t, ds.CancelPendingSetupExperienceStepsFuncInvoked, + "after tolerating the notFound, the status-table cancel must still run so the iteration eventually clears "+ + "the rows and the next retry's pending check skips them") + }) + + t.Run("require_all read via team config blocks when team has require_all_software_windows=true", func(t *testing.T) { + // Covers the team-path branch of the require_all_software_windows lookup chain (HostLite returns + // TeamID set -> TeamLite -> team config). Other tests use the no-team path via setRequireAll. + ds, svc := newSvc(t) + ds.ListSetupExperienceResultsByHostUUIDFunc = func(ctx context.Context, hUUID string, teamID uint) ([]*fleet.SetupExperienceStatusResult, error) { + return []*fleet.SetupExperienceStatusResult{ + {Name: "Critical App", Status: fleet.SetupExperienceStatusFailure, SoftwareInstallerID: new(uint(7))}, + }, nil + } + // Team-path overrides: HostLite returns a host with TeamID set; TeamLite returns the team config with + // require_all_software_windows=true. AppConfig MUST NOT be consulted on the team path. + teamID := uint(42) + ds.HostLiteByIdentifierFunc = func(ctx context.Context, identifier string) (*fleet.HostLite, error) { + return &fleet.HostLite{ID: 1, UUID: identifier, TeamID: &teamID}, nil + } + ds.TeamLiteFunc = func(ctx context.Context, tid uint) (*fleet.TeamLite, error) { + require.Equal(t, teamID, tid, "TeamLite must be called with the host's team_id") + return &fleet.TeamLite{ + ID: tid, + Config: fleet.TeamConfigLite{ + MDM: fleet.TeamMDM{MacOSSetup: fleet.MacOSSetup{RequireAllSoftwareWindows: true}}, + }, + }, nil + } + ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) { + t.Fatal("AppConfig must not be called when host has a team_id") + return nil, nil + } + + cmds, err := svc.getESPCommands(t.Context(), newActiveDevice()) + require.NoError(t, err) + require.NotEmpty(t, cmds) + assert.True(t, ds.TeamLiteFuncInvoked, "TeamLite must be called on the team path") + assert.NotNil(t, findCmdByLocURI(cmds, "BlockInStatusPage"), + "team config require_all_software_windows=true must drive the block path") + }) + + t.Run("persist failure aborts finalize without committing CAS", func(t *testing.T) { + // Safety property: if the persist (dropped-response retry safety net) fails, we must NOT commit the CAS + // transition Active -> None. Otherwise the device would be left without an inline send AND without the + // retry backup -- stuck on "Working on it..." forever, since awaiting_configuration=None means subsequent + // management sessions return no ESP commands. Persist runs before the CAS for exactly this reason. + ds, svc := newSvc(t) + ds.ListSetupExperienceResultsByHostUUIDFunc = func(ctx context.Context, hUUID string, teamID uint) ([]*fleet.SetupExperienceStatusResult, error) { + return []*fleet.SetupExperienceStatusResult{ + {Name: "Critical App", Status: fleet.SetupExperienceStatusFailure, SoftwareInstallerID: new(uint(7))}, + }, nil + } + setRequireAll(ds, true) + ds.MDMWindowsInsertCommandsForHostFunc = func(ctx context.Context, hostUUIDOrDeviceID string, cmds []*fleet.MDMWindowsCommand) error { + return errors.New("transient db error") + } + ds.SetMDMWindowsAwaitingConfigurationFunc = func(ctx context.Context, mdmDeviceID string, from, to fleet.WindowsMDMAwaitingConfiguration) (bool, error) { + t.Fatal("CAS Active->None must NOT run when persist fails") + return false, nil + } + + cmds, err := svc.getESPCommands(t.Context(), newActiveDevice()) + require.Error(t, err, "must return error so device retries on next session") + assert.Nil(t, cmds) + assert.False(t, ds.SetMDMWindowsAwaitingConfigurationFuncInvoked, + "CAS must NOT have been invoked when persist fails") + }) + + t.Run("cancel failure aborts finalize without committing CAS", func(t *testing.T) { + // Cancel runs before persist and CAS. A transient cancel failure must abort the finalize cleanly: + // otherwise we'd commit awaiting=None while leaving non-terminal setup-experience rows behind, which is + // exactly the state cancellation is supposed to prevent. CancelPendingSetupExperienceSteps is idempotent + // so a retry on the next session is safe. + ds, svc := newSvc(t) + ds.ListSetupExperienceResultsByHostUUIDFunc = func(ctx context.Context, hUUID string, teamID uint) ([]*fleet.SetupExperienceStatusResult, error) { + return []*fleet.SetupExperienceStatusResult{ + {Name: "Critical App", Status: fleet.SetupExperienceStatusFailure, SoftwareInstallerID: new(uint(7))}, + }, nil + } + setRequireAll(ds, true) + ds.CancelPendingSetupExperienceStepsFunc = func(ctx context.Context, hUUID string) error { + return errors.New("transient db error") + } + ds.MDMWindowsInsertCommandsForHostFunc = func(ctx context.Context, hostUUIDOrDeviceID string, cmds []*fleet.MDMWindowsCommand) error { + t.Fatal("persist must NOT run when cancel fails") + return nil + } + ds.SetMDMWindowsAwaitingConfigurationFunc = func(ctx context.Context, mdmDeviceID string, from, to fleet.WindowsMDMAwaitingConfiguration) (bool, error) { + t.Fatal("CAS Active->None must NOT run when cancel fails") + return false, nil + } + + cmds, err := svc.getESPCommands(t.Context(), newActiveDevice()) + require.Error(t, err, "must return error so device retries on next session") + assert.Nil(t, cmds) + assert.False(t, ds.MDMWindowsInsertCommandsForHostFuncInvoked, + "persist must NOT have been invoked when cancel fails") + assert.False(t, ds.SetMDMWindowsAwaitingConfigurationFuncInvoked, + "CAS must NOT have been invoked when cancel fails") + }) + + t.Run("require_all lookup error returns error and keeps device active", func(t *testing.T) { + // Failing AppConfig (not HostLite) ensures the test exercises the require_all chain itself rather than + // erroring out earlier at setupExperienceHostUUID. With HostLite returning a valid host (default), + // loadRequireAll proceeds to AppConfig and gets the error injected here. Property under test is the + // same regardless of which lookup fails: any error in the finalize path must keep the device Active. + ds, svc := newSvc(t) + ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) { + return nil, errors.New("transient db error") + } + + cmds, err := svc.getESPCommands(t.Context(), newActiveDevice()) + require.Error(t, err, "must return error so device retries on next session") + assert.Nil(t, cmds) + assert.False(t, ds.SetMDMWindowsAwaitingConfigurationFuncInvoked, + "must NOT transition to None on lookup failure") + }) + + t.Run("active waits when results empty but setup experience configured", func(t *testing.T) { + // Setup experience is configured for the team but orbit hasn't called SetupExperienceInit yet, so + // results are empty. The disambiguation must wait for orbit rather than releasing. + ds, svc := newSvc(t) + ds.HasWindowsSetupExperienceItemsForTeamFunc = func(ctx context.Context, teamID uint) (bool, error) { + return true, nil + } + + cmds, err := svc.getESPCommands(t.Context(), newActiveDevice()) + require.NoError(t, err) + assert.Nil(t, cmds, "should wait for orbit to initialize setup experience") + // Must NOT have proceeded to the Active->None transition. + assert.False(t, ds.SetMDMWindowsAwaitingConfigurationFuncInvoked, + "must not transition state while waiting for orbit init") }) } diff --git a/server/service/orbit.go b/server/service/orbit.go index a787d5fa7a..c1734aba1f 100644 --- a/server/service/orbit.go +++ b/server/service/orbit.go @@ -455,6 +455,25 @@ func (svc *Service) GetOrbitConfig(ctx context.Context) (fleet.OrbitConfig, erro } } + // Check if Windows host is in Autopilot setup experience. When awaiting_configuration is Pending or Active, + // orbit should run the setup experience to install software during the ESP. + // + // We also gate on WindowsEnabledAndConfigured: if Windows MDM is being turned off, the unenrollment branch + // above sets NeedsProgrammaticWindowsMDMUnenrollment, and we must not also set RunSetupExperience for the + // same orbit response. + if appConfig.MDM.WindowsEnabledAndConfigured && + host.Platform == "windows" && + isConnectedToFleetMDM { + awaiting, err := svc.ds.GetMDMWindowsAwaitingConfigurationByHostUUID(ctx, host.UUID) + if err != nil && !fleet.IsNotFound(err) { + return fleet.OrbitConfig{}, ctxerr.Wrap(ctx, err, "checking Windows awaiting configuration") + } + if awaiting == fleet.WindowsMDMAwaitingConfigurationPending || + awaiting == fleet.WindowsMDMAwaitingConfigurationActive { + notifs.RunSetupExperience = true + } + } + // load the (active, ready to execute) pending script executions for that host pending, err := svc.ds.ListReadyToExecuteScriptsForHost(ctx, host.ID, appConfig.ServerSettings.ScriptsDisabled) if err != nil { diff --git a/server/service/orbit_test.go b/server/service/orbit_test.go index 1e171c5a47..3b22591ad0 100644 --- a/server/service/orbit_test.go +++ b/server/service/orbit_test.go @@ -23,6 +23,7 @@ import ( "github.com/fleetdm/fleet/v4/server/test" "github.com/google/uuid" "github.com/jmoiron/sqlx" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -1084,3 +1085,128 @@ func TestSoftwareInstallReplicaLag(t *testing.T) { }) require.Equal(t, 1, retryCount, "should have scheduled a retry in upcoming_activities") } + +// TestGetOrbitConfigWindowsSetupExperience verifies that GetOrbitConfig sets +// notifs.RunSetupExperience=true for Windows hosts whose MDM enrollment is +// in awaiting_configuration Pending or Active, and false otherwise (None, +// not-enrolled, non-Windows platforms). +func TestGetOrbitConfigWindowsSetupExperience(t *testing.T) { + setupSvc := func(t *testing.T) (*mock.Store, fleet.Service, context.Context, *fleet.Host) { + ds := new(mock.Store) + license := &fleet.LicenseInfo{Tier: fleet.TierPremium} + svc, ctx := newTestService(t, ds, nil, nil, &TestServerOpts{License: license, SkipCreateTestUsers: true}) + + host := &fleet.Host{ + ID: 1, + OsqueryHostID: ptr.String("test"), + UUID: "host-uuid-1", + Platform: "windows", + } + + appCfg := &fleet.AppConfig{ + MDM: fleet.MDM{ + EnabledAndConfigured: true, + WindowsEnabledAndConfigured: true, + }, + } + ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) { + return appCfg, nil + } + ds.GetHostOperatingSystemFunc = func(ctx context.Context, hostID uint) (*fleet.OperatingSystem, error) { + return &fleet.OperatingSystem{Platform: "windows", Version: "10.0.19045"}, nil + } + ds.ListReadyToExecuteScriptsForHostFunc = func(ctx context.Context, hostID uint, onlyShowInternal bool) ([]*fleet.HostScriptResult, error) { + return nil, nil + } + ds.ListReadyToExecuteSoftwareInstallsFunc = func(ctx context.Context, hostID uint) ([]string, error) { + return nil, nil + } + ds.IsHostConnectedToFleetMDMFunc = func(ctx context.Context, h *fleet.Host) (bool, error) { + return true, nil + } + ds.IsHostPendingEscrowFunc = func(ctx context.Context, hostID uint) bool { + return false + } + ds.GetHostMDMFunc = func(ctx context.Context, hostID uint) (*fleet.HostMDM, error) { + return &fleet.HostMDM{Enrolled: true, Name: fleet.WellKnownMDMFleet}, nil + } + ds.GetHostAwaitingConfigurationFunc = func(ctx context.Context, hostUUID string) (bool, error) { + return false, nil + } + + ctx = test.HostContext(ctx, host) + return ds, svc, ctx, host + } + + t.Run("Windows host awaiting=Pending sets RunSetupExperience", func(t *testing.T) { + ds, svc, ctx, _ := setupSvc(t) + ds.GetMDMWindowsAwaitingConfigurationByHostUUIDFunc = func(ctx context.Context, hostUUID string) (fleet.WindowsMDMAwaitingConfiguration, error) { + return fleet.WindowsMDMAwaitingConfigurationPending, nil + } + + cfg, err := svc.GetOrbitConfig(ctx) + require.NoError(t, err) + assert.True(t, cfg.Notifications.RunSetupExperience) + assert.True(t, ds.GetMDMWindowsAwaitingConfigurationByHostUUIDFuncInvoked) + }) + + t.Run("Windows host awaiting=Active sets RunSetupExperience", func(t *testing.T) { + ds, svc, ctx, _ := setupSvc(t) + ds.GetMDMWindowsAwaitingConfigurationByHostUUIDFunc = func(ctx context.Context, hostUUID string) (fleet.WindowsMDMAwaitingConfiguration, error) { + return fleet.WindowsMDMAwaitingConfigurationActive, nil + } + + cfg, err := svc.GetOrbitConfig(ctx) + require.NoError(t, err) + assert.True(t, cfg.Notifications.RunSetupExperience) + }) + + t.Run("Windows host awaiting=None does not set RunSetupExperience", func(t *testing.T) { + ds, svc, ctx, _ := setupSvc(t) + ds.GetMDMWindowsAwaitingConfigurationByHostUUIDFunc = func(ctx context.Context, hostUUID string) (fleet.WindowsMDMAwaitingConfiguration, error) { + return fleet.WindowsMDMAwaitingConfigurationNone, nil + } + + cfg, err := svc.GetOrbitConfig(ctx) + require.NoError(t, err) + assert.False(t, cfg.Notifications.RunSetupExperience) + }) + + t.Run("Windows host not enrolled (NotFound) does not set RunSetupExperience", func(t *testing.T) { + ds, svc, ctx, _ := setupSvc(t) + ds.GetMDMWindowsAwaitingConfigurationByHostUUIDFunc = func(ctx context.Context, hostUUID string) (fleet.WindowsMDMAwaitingConfiguration, error) { + return 0, &orbitTestNotFoundErr{} + } + + cfg, err := svc.GetOrbitConfig(ctx) + require.NoError(t, err) + assert.False(t, cfg.Notifications.RunSetupExperience) + }) + + t.Run("Windows host with non-NotFound lookup error returns the error", func(t *testing.T) { + ds, svc, ctx, _ := setupSvc(t) + ds.GetMDMWindowsAwaitingConfigurationByHostUUIDFunc = func(ctx context.Context, hostUUID string) (fleet.WindowsMDMAwaitingConfiguration, error) { + return 0, errors.New("transient db error") + } + + _, err := svc.GetOrbitConfig(ctx) + require.Error(t, err) + }) + + t.Run("non-Windows host does not query awaiting_configuration", func(t *testing.T) { + ds, svc, ctx, host := setupSvc(t) + host.Platform = "darwin" + + cfg, err := svc.GetOrbitConfig(ctx) + require.NoError(t, err) + assert.False(t, cfg.Notifications.RunSetupExperience) + assert.False(t, ds.GetMDMWindowsAwaitingConfigurationByHostUUIDFuncInvoked, + "non-Windows hosts must not invoke the Windows lookup") + }) +} + +// orbitTestNotFoundErr is a minimal IsNotFound error type for orbit config tests. +type orbitTestNotFoundErr struct{} + +func (e *orbitTestNotFoundErr) Error() string { return "not found" } +func (e *orbitTestNotFoundErr) IsNotFound() bool { return true } diff --git a/server/service/setup_experience_test.go b/server/service/setup_experience_test.go index 4fd449063e..d83db103c7 100644 --- a/server/service/setup_experience_test.go +++ b/server/service/setup_experience_test.go @@ -700,4 +700,173 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { require.False(t, ds.CancelHostUpcomingActivityFuncInvoked, "cancel upcoming activity should NOT be called again") require.Equal(t, 1, activityCallCount, "activity should still have been emitted only once (no duplicate)") }) + + t.Run("windows software install failure with require_all_software_windows=true emits activity and cancels", func(t *testing.T) { + // Mirror of "software install failure triggers cancel and activity" + // for a Windows host. Asserts that the same emit-once-per-host + // invariant holds when the gating setting is `require_all_software_windows` + // (rather than `require_all_software`, which is the macOS counterpart). + teamID := uint(1) + failedSoftwareTitleID := uint(99) + failedSoftwareName := "WindowsApp" + pendingExecID := "pending-win-exec" + + ds.MaybeUpdateSetupExperienceSoftwareInstallStatusFunc = func(ctx context.Context, hUUID string, executionID string, status fleet.SetupExperienceStatusResultStatus) (bool, error) { + require.Equal(t, hostUUID, hUUID) + require.Equal(t, softwareUUID, executionID) + require.Equal(t, fleet.SetupExperienceStatusFailure, status) + return true, nil + } + ds.MaybeUpdateSetupExperienceSoftwareInstallStatusFuncInvoked = false + // Windows uses OsqueryHostID as the setup-experience host identifier + // (see fleet.HostUUIDForSetupExperience). Set it so the cancel + // helper can locate setup-experience rows. + osqueryHostID := "windows-osquery-id" + ds.HostByIdentifierFunc = func(ctx context.Context, identifier string) (*fleet.Host, error) { + return &fleet.Host{ + ID: 2, UUID: hostUUID, Platform: "windows", + TeamID: &teamID, OsqueryHostID: &osqueryHostID, + }, nil + } + ds.TeamLiteFunc = func(ctx context.Context, tid uint) (*fleet.TeamLite, error) { + require.Equal(t, teamID, tid) + return &fleet.TeamLite{ + ID: teamID, + Config: fleet.TeamConfigLite{ + MDM: fleet.TeamMDM{ + MacOSSetup: fleet.MacOSSetup{ + RequireAllSoftwareWindows: true, + }, + }, + }, + }, nil + } + + installerID := uint(20) + ds.ListSetupExperienceResultsByHostUUIDFunc = func(ctx context.Context, hUUID string, tID uint) ([]*fleet.SetupExperienceStatusResult, error) { + require.Equal(t, osqueryHostID, hUUID, "Windows looks up by OsqueryHostID, not UUID") + return []*fleet.SetupExperienceStatusResult{ + { + ID: 5, + HostUUID: osqueryHostID, + Name: failedSoftwareName, + Status: fleet.SetupExperienceStatusFailure, + SoftwareInstallerID: &installerID, + HostSoftwareInstallsExecutionID: &softwareUUID, + SoftwareTitleID: &failedSoftwareTitleID, + }, + { + ID: 6, + HostUUID: osqueryHostID, + Name: "PendingWinApp", + Status: fleet.SetupExperienceStatusPending, + SoftwareInstallerID: &installerID, + HostSoftwareInstallsExecutionID: &pendingExecID, + }, + }, nil + } + ds.CancelHostUpcomingActivityFuncInvoked = false + ds.CancelHostUpcomingActivityFunc = func(ctx context.Context, hID uint, executionID string) (fleet.ActivityDetails, error) { + require.Equal(t, uint(2), hID) + require.Equal(t, pendingExecID, executionID) + return nil, nil + } + ds.CancelPendingSetupExperienceStepsFuncInvoked = false + ds.CancelPendingSetupExperienceStepsFunc = func(ctx context.Context, hUUID string) error { + require.Equal(t, osqueryHostID, hUUID) + return nil + } + + var activityFnCalled bool + var recordedActivity fleet.ActivityDetails + activityFn := func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails) error { + activityFnCalled = true + recordedActivity = activity + return nil + } + + result := fleet.SetupExperienceSoftwareInstallResult{ + HostUUID: hostUUID, + ExecutionID: softwareUUID, + InstallerStatus: fleet.SoftwareInstallFailed, + } + updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, result, activityFn) + require.NoError(t, err) + require.True(t, updated) + require.True(t, activityFnCalled, "Windows host with require_all_software_windows=true must emit canceled_setup_experience") + require.True(t, ds.CancelPendingSetupExperienceStepsFuncInvoked, "Windows host must cancel pending setup-experience steps") + require.True(t, ds.CancelHostUpcomingActivityFuncInvoked) + + canceledActivity, ok := recordedActivity.(fleet.ActivityTypeCanceledSetupExperience) + require.True(t, ok) + require.Equal(t, uint(2), canceledActivity.HostID) + require.Equal(t, failedSoftwareName, canceledActivity.SoftwareTitle) + require.Equal(t, failedSoftwareTitleID, canceledActivity.SoftwareTitleID) + }) + + t.Run("software install failure with require_all=false does not emit activity or cancel", func(t *testing.T) { + // Spec invariant: when require_all_software (macOS) / + // require_all_software_windows (Windows) is false, a software + // install failure during ESP MUST NOT cancel pending steps and MUST + // NOT emit a canceled_setup_experience activity. The device just + // proceeds to the desktop and the failure is visible only in + // Fleet's host activity feed (via the install-status path, not + // canceled_setup_experience). + teamID := uint(1) + + ds.MaybeUpdateSetupExperienceSoftwareInstallStatusFunc = func(ctx context.Context, hUUID string, executionID string, status fleet.SetupExperienceStatusResultStatus) (bool, error) { + return true, nil + } + ds.MaybeUpdateSetupExperienceSoftwareInstallStatusFuncInvoked = false + // Windows host with OsqueryHostID set so the cancel helper can run if + // it ever (incorrectly) reaches the lookup path. This test asserts + // it does NOT reach that path because of the require_all=false + // early-return. + osqueryHostID := "windows-osquery-id-noreq" + ds.HostByIdentifierFunc = func(ctx context.Context, identifier string) (*fleet.Host, error) { + return &fleet.Host{ + ID: 3, UUID: hostUUID, Platform: "windows", + TeamID: &teamID, OsqueryHostID: &osqueryHostID, + }, nil + } + ds.TeamLiteFunc = func(ctx context.Context, tid uint) (*fleet.TeamLite, error) { + return &fleet.TeamLite{ + ID: teamID, + Config: fleet.TeamConfigLite{ + MDM: fleet.TeamMDM{ + MacOSSetup: fleet.MacOSSetup{ + RequireAllSoftwareWindows: false, + }, + }, + }, + }, nil + } + ds.CancelPendingSetupExperienceStepsFuncInvoked = false + ds.CancelHostUpcomingActivityFuncInvoked = false + ds.ListSetupExperienceResultsByHostUUIDFuncInvoked = false + + var activityFnCalled bool + activityFn := func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails) error { + activityFnCalled = true + return nil + } + + result := fleet.SetupExperienceSoftwareInstallResult{ + HostUUID: hostUUID, + ExecutionID: softwareUUID, + InstallerStatus: fleet.SoftwareInstallFailed, + } + updated, err := maybeUpdateSetupExperienceStatus(ctx, ds, result, activityFn) + require.NoError(t, err) + require.True(t, updated, "the installer status row should still be updated to failure") + require.False(t, activityFnCalled, "no canceled_setup_experience activity when require_all=false") + require.False(t, ds.CancelPendingSetupExperienceStepsFuncInvoked, + "no cancel-pending-steps when require_all=false") + require.False(t, ds.CancelHostUpcomingActivityFuncInvoked, + "no upcoming-activity cancel when require_all=false") + // The early-return path inside maybeCancelPendingSetupExperienceSteps + // should not even reach the ListSetupExperienceResultsByHostUUID query. + require.False(t, ds.ListSetupExperienceResultsByHostUUIDFuncInvoked, + "require_all=false should early-return before listing setup-experience results") + }) }