diff --git a/changes/team-features-bug b/changes/team-features-bug new file mode 100644 index 0000000000..dd84a90ae3 --- /dev/null +++ b/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. diff --git a/server/datastore/mysql/teams.go b/server/datastore/mysql/teams.go index cd703a48a3..fc4ba5defe 100644 --- a/server/datastore/mysql/teams.go +++ b/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") } diff --git a/server/datastore/mysql/teams_test.go b/server/datastore/mysql/teams_test.go index 7468dbc05e..dee4831417 100644 --- a/server/datastore/mysql/teams_test.go +++ b/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) { diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 17a5f7efda..2b4a4f24ad 100644 --- a/server/service/integration_enterprise_test.go +++ b/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)