feat: do not run setup experience on hosts in a team with no software…

… or script configured (#24073)

> Related issue: #24024 

# Checklist for submitter

Demo video: https://www.youtube.com/watch?v=F7p2PyJce7E

If some of the following don't apply, delete the relevant line.

<!-- Note that API documentation changes are now addressed by the
product design team. -->

- [x] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/Committing-Changes.md#changes-files)
for more information.
- [x] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements)
- [x] Added/updated tests
- [x] Manual QA for all new/changed functionality

changes/24024-no-setup-exp

@@ -0,0 +1,2 @@
+- Modifies the Fleet setup experience feature to not run if there is no software or script
+  configured for the setup experience.

server/datastore/mysql/setup_experience.go

@@ -109,8 +109,11 @@ WHERE global_or_team_id = ?`
 		}
 		totalInsertions += uint(inserts) // nolint: gosec
 
-		if err := setHostAwaitingConfiguration(ctx, tx, hostUUID, true); err != nil {
-			return ctxerr.Wrap(ctx, err, "setting host awaiting configuration to true")
+		// Only run setup experience on hosts that have something configured.
+		if totalInsertions > 0 {
+			if err := setHostAwaitingConfiguration(ctx, tx, hostUUID, true); err != nil {
+				return ctxerr.Wrap(ctx, err, "setting host awaiting configuration to true")
+			}
 		}
 
 		return nil
@@ -529,7 +532,7 @@ WHERE host_uuid = ?
 
 	if err := sqlx.GetContext(ctx, ds.reader(ctx), &awaitingConfiguration, stmt, hostUUID); err != nil {
 		if errors.Is(err, sql.ErrNoRows) {
-			return false, nil
+			return false, notFound("HostAwaitingConfiguration")
 		}
 
 		return false, ctxerr.Wrap(ctx, err, "getting host awaiting configuration")

server/datastore/mysql/setup_experience_test.go

@@ -40,20 +40,22 @@ func TestSetupExperience(t *testing.T) {
 	}
 }
 
+// TODO(JVE): this test could probably be simplified and most of the ad-hoc SQL removed.
 func testEnqueueSetupExperienceItems(t *testing.T, ds *Datastore) {
 	ctx := context.Background()
 	test.CreateInsertGlobalVPPToken(t, ds)
 
+	// Create some teams
 	team1, err := ds.NewTeam(ctx, &fleet.Team{Name: "team1"})
 	require.NoError(t, err)
 	team2, err := ds.NewTeam(ctx, &fleet.Team{Name: "team2"})
 	require.NoError(t, err)
-
 	team3, err := ds.NewTeam(ctx, &fleet.Team{Name: "team3"})
 	require.NoError(t, err)
 
 	user1 := test.NewUser(t, ds, "Alice", "[email protected]", true)
 
+	// Create some software installers and add them to setup experience
 	tfr1, err := fleet.NewTempFileReader(strings.NewReader("hello"), t.TempDir)
 	require.NoError(t, err)
 	installerID1, err := ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{
@@ -97,6 +99,7 @@ func testEnqueueSetupExperienceItems(t *testing.T, ds *Datastore) {
 		return err
 	})
 
+	// Create some VPP apps and add them to setup experience
 	app1 := &fleet.VPPApp{Name: "vpp_app_1", VPPAppTeam: fleet.VPPAppTeam{VPPAppID: fleet.VPPAppID{AdamID: "1", Platform: fleet.MacOSPlatform}}, BundleIdentifier: "b1"}
 	vpp1, err := ds.InsertVPPAppWithTeam(ctx, app1, &team1.ID)
 	require.NoError(t, err)
@@ -110,33 +113,17 @@ func testEnqueueSetupExperienceItems(t *testing.T, ds *Datastore) {
 		return err
 	})
 
-	var script1ID, script2ID int64
-	ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
-		res, err := insertScriptContents(ctx, q, "SCRIPT 1")
-		if err != nil {
-			return err
-		}
-		id1, _ := res.LastInsertId()
-		res, err = insertScriptContents(ctx, q, "SCRIPT 2")
-		if err != nil {
-			return err
-		}
-		id2, _ := res.LastInsertId()
-
-		res, err = q.ExecContext(ctx, "INSERT INTO setup_experience_scripts (team_id, global_or_team_id, name, script_content_id) VALUES (?, ?, ?, ?)", team1.ID, team1.ID, "script1", id1)
-		if err != nil {
-			return err
-		}
-		script1ID, _ = res.LastInsertId()
+	// Create some scripts and add them to setup experience
+	err = ds.SetSetupExperienceScript(ctx, &fleet.Script{Name: "script1", ScriptContents: "SCRIPT 1", TeamID: &team1.ID})
+	require.NoError(t, err)
+	err = ds.SetSetupExperienceScript(ctx, &fleet.Script{Name: "script2", ScriptContents: "SCRIPT 2", TeamID: &team2.ID})
+	require.NoError(t, err)
 
-		res, err = q.ExecContext(ctx, "INSERT INTO setup_experience_scripts (team_id, global_or_team_id, name, script_content_id) VALUES (?, ?, ?, ?)", team2.ID, team2.ID, "script2", id2)
-		if err != nil {
-			return err
-		}
-		script2ID, _ = res.LastInsertId()
+	script1, err := ds.GetSetupExperienceScript(ctx, &team1.ID)
+	require.NoError(t, err)
 
-		return nil
-	})
+	script2, err := ds.GetSetupExperienceScript(ctx, &team2.ID)
+	require.NoError(t, err)
 
 	hostTeam1 := "123"
 	hostTeam2 := "456"
@@ -145,14 +132,26 @@ func testEnqueueSetupExperienceItems(t *testing.T, ds *Datastore) {
 	anythingEnqueued, err := ds.EnqueueSetupExperienceItems(ctx, hostTeam1, team1.ID)
 	require.NoError(t, err)
 	require.True(t, anythingEnqueued)
+	awaitingConfig, err := ds.GetHostAwaitingConfiguration(ctx, hostTeam1)
+	require.NoError(t, err)
+	require.True(t, awaitingConfig)
 
 	anythingEnqueued, err = ds.EnqueueSetupExperienceItems(ctx, hostTeam2, team2.ID)
 	require.NoError(t, err)
 	require.True(t, anythingEnqueued)
+	awaitingConfig, err = ds.GetHostAwaitingConfiguration(ctx, hostTeam2)
+	require.NoError(t, err)
+	require.True(t, awaitingConfig)
 
 	anythingEnqueued, err = ds.EnqueueSetupExperienceItems(ctx, hostTeam3, team3.ID)
 	require.NoError(t, err)
 	require.False(t, anythingEnqueued)
+	// Nothing is configured for setup experience in team 3, so we do not set
+	// host_mdm_apple_awaiting_configuration.
+	awaitingConfig, err = ds.GetHostAwaitingConfiguration(ctx, hostTeam3)
+	require.Error(t, err)
+	require.True(t, fleet.IsNotFound(err))
+	require.False(t, awaitingConfig)
 
 	seRows := []setupExperienceInsertTestRows{}
 
@@ -191,13 +190,13 @@ func testEnqueueSetupExperienceItems(t *testing.T, ds *Datastore) {
 			HostUUID: hostTeam1,
 			Name:     "script1",
 			Status:   "pending",
-			ScriptID: nullableUint(uint(script1ID)), // nolint: gosec
+			ScriptID: nullableUint(script1.ID),
 		},
 		{
 			HostUUID: hostTeam2,
 			Name:     "script2",
 			Status:   "pending",
-			ScriptID: nullableUint(uint(script2ID)), // nolint: gosec
+			ScriptID: nullableUint(script2.ID),
 		},
 	} {
 		var found bool
@@ -212,35 +211,28 @@ func testEnqueueSetupExperienceItems(t *testing.T, ds *Datastore) {
 		}
 	}
 
-	for _, row := range seRows {
-		if row.HostUUID == hostTeam3 {
-			t.Error("team 3 shouldn't have any any entries")
-		}
-	}
-
-	ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
-		_, err := q.ExecContext(ctx, "DELETE FROM setup_experience_scripts WHERE global_or_team_id = ?", team2.ID)
-		if err != nil {
-			return err
+	require.Condition(t, func() (success bool) {
+		for _, row := range seRows {
+			if row.HostUUID == hostTeam3 {
+				return false
+			}
 		}
 
-		_, err = q.ExecContext(ctx, "UPDATE software_installers SET install_during_setup = false WHERE global_or_team_id = ?", team2.ID)
-		if err != nil {
-			return err
-		}
+		return true
+	})
 
-		_, err = q.ExecContext(ctx, "UPDATE vpp_apps_teams SET install_during_setup = false WHERE global_or_team_id = ?", team2.ID)
-		if err != nil {
-			return err
-		}
+	// Remove team2's setup experience items
+	err = ds.DeleteSetupExperienceScript(ctx, &team2.ID)
+	require.NoError(t, err)
 
-		return nil
-	})
+	err = ds.SetSetupExperienceSoftwareTitles(ctx, team2.ID, []uint{})
+	require.NoError(t, err)
 
 	anythingEnqueued, err = ds.EnqueueSetupExperienceItems(ctx, hostTeam1, team1.ID)
 	require.NoError(t, err)
 	require.True(t, anythingEnqueued)
 
+	// team2 now has nothing enqueued
 	anythingEnqueued, err = ds.EnqueueSetupExperienceItems(ctx, hostTeam2, team2.ID)
 	require.NoError(t, err)
 	require.False(t, anythingEnqueued)
@@ -272,7 +264,7 @@ func testEnqueueSetupExperienceItems(t *testing.T, ds *Datastore) {
 			HostUUID: hostTeam1,
 			Name:     "script1",
 			Status:   "pending",
-			ScriptID: nullableUint(uint(script1ID)), // nolint: gosec
+			ScriptID: nullableUint(script1.ID),
 		},
 	} {
 		var found bool
@@ -908,6 +900,12 @@ func testHostInSetupExperience(t *testing.T, ds *Datastore) {
 	inSetupExperience, err = ds.GetHostAwaitingConfiguration(ctx, "abc")
 	require.NoError(t, err)
 	require.False(t, inSetupExperience)
+
+	// host without a record in the table returns not found
+	inSetupExperience, err = ds.GetHostAwaitingConfiguration(ctx, "404")
+	require.Error(t, err)
+	require.True(t, fleet.IsNotFound(err))
+	require.False(t, inSetupExperience)
 }
 
 func testGetSetupExperienceScriptByID(t *testing.T, ds *Datastore) {

server/service/integration_mdm_dep_test.go

@@ -121,12 +121,6 @@ func (s *integrationMDMTestSuite) TestDEPEnrollReleaseDeviceGlobal() {
 
 	s.enableABM("fleet_ade_test")
 
-	// test manual and automatic release with the new setup experience flow
-	for _, enableReleaseManually := range []bool{false, true} {
-		t.Run(fmt.Sprintf("enableReleaseManually=%t", enableReleaseManually), func(t *testing.T) {
-			s.runDEPEnrollReleaseDeviceTest(t, globalDevice, enableReleaseManually, nil, "I1", false)
-		})
-	}
 	// test manual and automatic release with the old worker flow
 	for _, enableReleaseManually := range []bool{false, true} {
 		t.Run(fmt.Sprintf("enableReleaseManually=%t", enableReleaseManually), func(t *testing.T) {
@@ -217,12 +211,6 @@ func (s *integrationMDMTestSuite) TestDEPEnrollReleaseDeviceTeam() {
 	// enable FileVault
 	s.Do("PATCH", "/api/latest/fleet/mdm/apple/settings", json.RawMessage([]byte(fmt.Sprintf(`{"enable_disk_encryption":true,"team_id":%d}`, tm.ID))), http.StatusNoContent)
 
-	// test manual and automatic release with the new setup experience flow
-	for _, enableReleaseManually := range []bool{false, true} {
-		t.Run(fmt.Sprintf("enableReleaseManually=%t", enableReleaseManually), func(t *testing.T) {
-			s.runDEPEnrollReleaseDeviceTest(t, teamDevice, enableReleaseManually, &tm.ID, "I2", false)
-		})
-	}
 	// test manual and automatic release with the old worker flow
 	for _, enableReleaseManually := range []bool{false, true} {
 		t.Run(fmt.Sprintf("enableReleaseManually=%t", enableReleaseManually), func(t *testing.T) {
@@ -538,7 +526,7 @@ func (s *integrationMDMTestSuite) runDEPEnrollReleaseDeviceTest(t *testing.T, de
 	require.NoError(t, err)
 	require.NoError(t, json.Unmarshal(b, &orbitConfigResp))
 	// should be notified of the setup experience flow
-	require.True(t, orbitConfigResp.Notifications.RunSetupExperience)
+	require.False(t, orbitConfigResp.Notifications.RunSetupExperience)
 
 	if enableReleaseManually {
 		// get the worker's pending job from the future, there should not be any

server/service/orbit.go

@@ -235,19 +235,29 @@ func (svc *Service) GetOrbitConfig(ctx context.Context) (fleet.OrbitConfig, erro
 		}
 
 		if isConnectedToFleetMDM {
+			// If there is no software or script configured for setup experience and this is the
+			// first time orbit is calling the /config endpoint, then this host
+			// will not have a row in host_mdm_apple_awaiting_configuration.
+			// On subsequent calls to /config, the host WILL have a row in
+			// host_mdm_apple_awaiting_configuration.
 			inSetupAssistant, err := svc.ds.GetHostAwaitingConfiguration(ctx, host.UUID)
-			if err != nil {
+			if err != nil && !fleet.IsNotFound(err) {
 				return fleet.OrbitConfig{}, ctxerr.Wrap(ctx, err, "checking if host is in setup experience")
 			}
 
 			if inSetupAssistant {
 				notifs.RunSetupExperience = true
+			}
 
-				// check if the client is running an old fleetd version that doesn't support the new
-				// setup experience flow.
+			if inSetupAssistant || fleet.IsNotFound(err) {
+				// If the client is running a fleetd that doesn't support setup experience, or if no
+				// software/script has been configured for setup experience, then we should fall back to
+				// the "old way" of releasing the device. We do an additional check for
+				// !inSetupAssistant to prevent enqueuing a new job every time the /config
+				// endpoint is hit.
 				mp, ok := capabilities.FromContext(ctx)
-				if !ok || !mp.Has(fleet.CapabilitySetupExperience) {
-					level.Debug(svc.logger).Log("msg", "host doesn't support Setup experience, falling back to worker-based device release", "host_uuid", host.UUID)
+				if !ok || !mp.Has(fleet.CapabilitySetupExperience) || !inSetupAssistant {
+					level.Debug(svc.logger).Log("msg", "host doesn't support setup experience or no setup experience configured, falling back to worker-based device release", "host_uuid", host.UUID)
 					if err := svc.processReleaseDeviceForOldFleetd(ctx, host); err != nil {
 						return fleet.OrbitConfig{}, err
 					}