fix a bug causing features to be false (#8240)

Related to https://github.com/fleetdm/fleet/issues/8010 and https://github.com/fleetdm/fleet/issues/8013 this prevents a bug that happens when:

1. A team doesn't have a `config.features` key in the JSON stored in the table or `config` is `NULL`
2. The team is edited from the UI

All `config.features` will default to `false`, which can be a problem if your global settings are `true` for both (which is the default)

changes/team-features-bug

@@ -0,0 +1 @@
+* Fixed a bug where if a team didn't have a `config.features` settings and was edited via the UI, the both `features.enable_host_users` and `features.enable_software_inventory` would be `false` instad of the global default.

server/datastore/mysql/teams.go

@@ -74,6 +74,9 @@ func teamDB(ctx context.Context, q sqlx.QueryerContext, tid uint) (*fleet.Team,
 	if err := loadHostCountForTeamDB(ctx, q, team); err != nil {
 		return nil, err
 	}
+	if err := loadFeaturesForTeamDB(ctx, q, team); err != nil {
+		return nil, err
+	}
 
 	return team, nil
 }
@@ -128,6 +131,9 @@ func (ds *Datastore) TeamByName(ctx context.Context, name string) (*fleet.Team,
 	if err := loadHostCountForTeamDB(ctx, ds.reader, team); err != nil {
 		return nil, err
 	}
+	if err := loadFeaturesForTeamDB(ctx, ds.reader, team); err != nil {
+		return nil, err
+	}
 
 	return team, nil
 }
@@ -161,6 +167,15 @@ func loadHostCountForTeamDB(ctx context.Context, q sqlx.QueryerContext, team *fl
 	return nil
 }
 
+func loadFeaturesForTeamDB(ctx context.Context, q sqlx.QueryerContext, team *fleet.Team) error {
+	features, err := teamFeaturesDB(ctx, q, team.ID)
+	if err != nil {
+		return err
+	}
+	team.Config.Features = *features
+	return nil
+}
+
 func saveUsersForTeamDB(ctx context.Context, exec sqlx.ExecerContext, team *fleet.Team) error {
 	// Do a full user update by deleting existing users and then inserting all
 	// the current users in a single transaction.
@@ -323,9 +338,13 @@ func (ds *Datastore) TeamAgentOptions(ctx context.Context, tid uint) (*json.RawM
 
 // TeamFeatures loads the features enabled for a team.
 func (ds *Datastore) TeamFeatures(ctx context.Context, tid uint) (*fleet.Features, error) {
+	return teamFeaturesDB(ctx, ds.reader, tid)
+}
+
+func teamFeaturesDB(ctx context.Context, q sqlx.QueryerContext, tid uint) (*fleet.Features, error) {
 	sql := `SELECT config->'$.features' as features FROM teams WHERE id = ?`
 	var raw *json.RawMessage
-	if err := sqlx.GetContext(ctx, ds.reader, &raw, sql, tid); err != nil {
+	if err := sqlx.GetContext(ctx, q, &raw, sql, tid); err != nil {
 		return nil, ctxerr.Wrap(ctx, err, "get team config features")
 	}
 

server/datastore/mysql/teams_test.go

@@ -81,7 +81,7 @@ func testTeamsGetSetDelete(t *testing.T, ds *Datastore) {
 			require.NoError(t, err)
 			require.Empty(t, newP.Teams)
 
-			team, err = ds.TeamByName(context.Background(), tt.name)
+			_, err = ds.TeamByName(context.Background(), tt.name)
 			require.Error(t, err)
 
 			require.NoError(t, ds.DeletePack(context.Background(), newP.Name))
@@ -453,10 +453,20 @@ func testTeamsFeatures(t *testing.T, ds *Datastore) {
 			)
 			return err
 		})
+
 		features, err := ds.TeamFeatures(ctx, team.ID)
 		require.NoError(t, err)
-
 		assert.Equal(t, &defaultFeatures, features)
+
+		// retrieving a team also returns a team with the default
+		// features
+		team, err = ds.Team(ctx, team.ID)
+		require.NoError(t, err)
+		assert.Equal(t, defaultFeatures, team.Config.Features)
+
+		team, err = ds.TeamByName(ctx, team.Name)
+		require.NoError(t, err)
+		assert.Equal(t, defaultFeatures, team.Config.Features)
 	})
 
 	t.Run("NULL config.features in the database", func(t *testing.T) {
@@ -470,10 +480,20 @@ func testTeamsFeatures(t *testing.T, ds *Datastore) {
 			)
 			return err
 		})
+
 		features, err := ds.TeamFeatures(ctx, team.ID)
 		require.NoError(t, err)
-
 		assert.Equal(t, &defaultFeatures, features)
+
+		// retrieving a team also returns a team with the default
+		// features
+		team, err = ds.Team(ctx, team.ID)
+		require.NoError(t, err)
+		assert.Equal(t, defaultFeatures, team.Config.Features)
+
+		team, err = ds.TeamByName(ctx, team.Name)
+		require.NoError(t, err)
+		assert.Equal(t, defaultFeatures, team.Config.Features)
 	})
 
 	t.Run("saves and retrieves configs", func(t *testing.T) {

server/service/integration_enterprise_test.go

@@ -489,6 +489,26 @@ func (s *integrationEnterpriseTestSuite) TestTeamEndpoints() {
 	s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d", tm1ID), team, http.StatusOK, &tmResp)
 	assert.Contains(t, tmResp.Team.Description, "Alt ")
 
+	// modify a team with a NULL config
+	defaultFeatures := fleet.Features{}
+	defaultFeatures.ApplyDefaultsForNewInstalls()
+	mysql.ExecAdhocSQL(t, s.ds, func(db sqlx.ExtContext) error {
+		_, err := db.ExecContext(context.Background(), `UPDATE teams SET config = NULL WHERE id = ? `, team.ID)
+		return err
+	})
+	tmResp.Team = nil
+	s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d", tm1ID), team, http.StatusOK, &tmResp)
+	assert.Equal(t, defaultFeatures, tmResp.Team.Config.Features)
+
+	// modify a team with an empty config
+	mysql.ExecAdhocSQL(t, s.ds, func(db sqlx.ExtContext) error {
+		_, err := db.ExecContext(context.Background(), `UPDATE teams SET config = '{}' WHERE id = ? `, team.ID)
+		return err
+	})
+	tmResp.Team = nil
+	s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d", tm1ID), team, http.StatusOK, &tmResp)
+	assert.Equal(t, defaultFeatures, tmResp.Team.Config.Features)
+
 	// modify non-existing team
 	tmResp.Team = nil
 	s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d", tm1ID+1), team, http.StatusNotFound, &tmResp)