Discharge7443/fleet
1
0
Fork 0
Code Issues Pull Requests Actions 21 Packages Projects Releases Wiki Activity

Fix fleet-scoped host vitals labels (#46953)

Browse Source 
**Related issue:** Resolves #46869

- [X] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.

## Testing

- [X] Added/updated automated tests
- [X] QA'd all new/changed functionality manually

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Host vitals labels based on identity-provider group membership now
correctly apply to both global and team-scoped hosts, preventing
cross-team leakage.

* **Tests**
* Added and updated tests to validate IdP-group-backed vitals label
membership across global and per-team hosts.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Lucas Manuel Rodriguez
2026-06-08 10:46:56 -03:00
committed by GitHub
parent bdbdaaa226
commit 863363561b
8 changed files with 221 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
changes/46869-idp-team-vitals-labels
+1
View File
@@ -0,0 +1 @@
- Fixed a bug where host vitals labels (e.g. IdP group/department labels) scoped to a fleet/team never got any hosts. The membership cron only looked at global labels, and team-scoped IdP labels also failed to populate due to an incorrect SQL join.
cmd/fleet/cron.go
+6 -1
View File
@@ -2068,7 +2068,12 @@ func cronHostVitalsLabelMembership(
) error {
// Get all labels. We don't have a function for labels by membership type
// so we'll filter them later.
labels, err := ds.ListLabels(ctx, fleet.TeamFilter{}, fleet.ListOptions{
//
// We use a global admin filter so that fleet/team-scoped labels are included.
// An empty TeamFilter (nil User) falls back to the "global-only" filter
// (l.team_id IS NULL), which would silently exclude every fleet-scoped host
// vitals label and leave them unpopulated. See #46869.
labels, err := ds.ListLabels(ctx, fleet.TeamFilter{User: &fleet.User{GlobalRole: new(fleet.RoleAdmin)}}, fleet.ListOptions{
PerPage: 0, // No limit.
}, false)
if err != nil {
cmd/fleet/cron_test.go
+92
View File
@@ -11,6 +11,7 @@ import (
"testing"
"time"
"github.com/jmoiron/sqlx"
"github.com/stretchr/testify/require"
"github.com/fleetdm/fleet/v4/server/datastore/mysql/mysqltest"
@@ -345,3 +346,94 @@ func TestBuildChartScopeResolver(t *testing.T) {
require.Nil(t, disabled)
})
}
func TestHostVitalsLabelMembershipCronIDP(t *testing.T) {
ctx := context.Background()
ds := mysqltest.CreateMySQLDS(t)
team1, err := ds.NewTeam(ctx, &fleet.Team{Name: "idp-cron-team1"})
require.NoError(t, err)
team2, err := ds.NewTeam(ctx, &fleet.Team{Name: "idp-cron-team2"})
require.NoError(t, err)
// host0 -> team1, host1 -> team2, host2 -> global (no team).
hosts := make([]*fleet.Host, 3)
teamIDs := []*uint{&team1.ID, &team2.ID, nil}
for i := range 3 {
h, err := ds.NewHost(ctx, &fleet.Host{
OsqueryHostID: new(fmt.Sprintf("idp-cron-%d", i)),
NodeKey: new(fmt.Sprintf("idp-cron-%d", i)),
UUID: fmt.Sprintf("idp-cron-uuid%d", i),
Hostname: fmt.Sprintf("idp-cron-host%d.local", i),
HardwareSerial: fmt.Sprintf("idp-cron-hwd%d", i),
Platform: "darwin",
TeamID: teamIDs[i],
})
require.NoError(t, err)
hosts[i] = h
}
// All three SCIM users are in the same "Engineering" IdP group.
scimUserIDs := make([]uint, 3)
for i := range 3 {
id, err := ds.CreateScimUser(ctx, &fleet.ScimUser{
UserName: fmt.Sprintf("idp-cron-user%d", i),
Active: new(true),
})
require.NoError(t, err)
scimUserIDs[i] = id
hostID, scimUserID := hosts[i].ID, id
mysqltest.ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
_, err := q.ExecContext(ctx,
"INSERT INTO host_scim_user (host_id, scim_user_id) VALUES (?, ?)",
hostID, scimUserID)
return err
})
}
_, err = ds.CreateScimGroup(ctx, &fleet.ScimGroup{DisplayName: "Engineering", ScimUsers: scimUserIDs})
require.NoError(t, err)
criteria, err := json.Marshal(&fleet.HostVitalCriteria{
Vital: new("end_user_idp_group"),
Value: new("Engineering"),
})
require.NoError(t, err)
// Create a global and a team1-scoped IdP host vitals label.
globalLabel, err := ds.NewLabel(ctx, &fleet.Label{
Name: "idp-cron-global",
LabelType: fleet.LabelTypeRegular,
LabelMembershipType: fleet.LabelMembershipTypeHostVitals,
HostVitalsCriteria: new(json.RawMessage(criteria)),
})
require.NoError(t, err)
team1Label, err := ds.NewLabel(ctx, &fleet.Label{
Name: "idp-cron-team1",
TeamID: &team1.ID,
LabelType: fleet.LabelTypeRegular,
LabelMembershipType: fleet.LabelMembershipTypeHostVitals,
HostVitalsCriteria: new(json.RawMessage(criteria)),
})
require.NoError(t, err)
// Run the actual cron.
require.NoError(t, cronHostVitalsLabelMembership(ctx, ds))
filter := fleet.TeamFilter{User: test.UserAdmin}
globalHosts, err := ds.ListHostsInLabel(ctx, filter, globalLabel.ID, fleet.HostListOptions{})
require.NoError(t, err)
gotGlobal := make([]uint, 0, len(globalHosts))
for _, h := range globalHosts {
gotGlobal = append(gotGlobal, h.ID)
}
require.ElementsMatch(t, []uint{hosts[0].ID, hosts[1].ID, hosts[2].ID}, gotGlobal)
team1Hosts, err := ds.ListHostsInLabel(ctx, filter, team1Label.ID, fleet.HostListOptions{})
require.NoError(t, err)
gotTeam1 := make([]uint, 0, len(team1Hosts))
for _, h := range team1Hosts {
gotTeam1 = append(gotTeam1, h.ID)
}
require.ElementsMatch(t, []uint{hosts[0].ID}, gotTeam1)
}
cmd/fleet/serve_test.go
+6
View File
@@ -1315,6 +1315,12 @@ func TestHostVitalsLabelMembershipJob(t *testing.T) {
}
ds.ListLabelsFunc = func(ctx context.Context, filter fleet.TeamFilter, opt fleet.ListOptions, includeHostCounts bool) ([]*fleet.Label, error) {
// The cron must pass a filter that includes fleet/team-scoped labels,
// otherwise host vitals labels on a team never get populated (#46869). An
// empty TeamFilter (nil User) falls back to global-only labels in
// applyLabelTeamFilter, so require a global-admin user here.
require.NotNil(t, filter.User, "cron must pass a user-scoped filter so team labels are included")
require.True(t, filter.User.HasAnyGlobalRole(), "cron must use a global-admin filter to see all team labels")
return labels, nil
}
server/datastore/mysql/labels_test.go
+103
View File
@@ -101,6 +101,7 @@ func TestLabels(t *testing.T) {
{"ApplyLabelSpecSerialUUID", testApplyLabelSpecsForSerialUUID},
{"ApplyLabelSpecsWithPlatformChange", testApplyLabelSpecsWithPlatformChange},
{"UpdateLabelMembershipByHostCriteria", testUpdateLabelMembershipByHostCriteria},
{"UpdateLabelMembershipByHostCriteriaIDP", testUpdateLabelMembershipByHostCriteriaIDP},
{"TeamLabels", testTeamLabels},
{"UpdateLabelMembershipForTransferredHost", testUpdateLabelMembershipForTransferredHost},
{"SetAsideLabels", testSetAsideLabels},
@@ -2917,6 +2918,108 @@ func testUpdateLabelMembershipByHostCriteria(t *testing.T, ds *Datastore) {
}
}
// testUpdateLabelMembershipByHostCriteriaIDP exercises the real IdP foreign
// vital query (end_user_idp_group) for both global and fleet/team-scoped host
// vitals labels. This is the path that broke in #46869: fleet-scoped IdP labels
// never got any hosts.
func testUpdateLabelMembershipByHostCriteriaIDP(t *testing.T, ds *Datastore) {
ctx := t.Context()
team1, err := ds.NewTeam(ctx, &fleet.Team{Name: "idp-team1"})
require.NoError(t, err)
team2, err := ds.NewTeam(ctx, &fleet.Team{Name: "idp-team2"})
require.NoError(t, err)
// host1 -> team1, host2 -> team2, host3 -> no team (global).
hosts := make([]*fleet.Host, 3)
teamIDs := []*uint{&team1.ID, &team2.ID, nil}
for i := range 3 {
host, err := ds.NewHost(ctx, &fleet.Host{
OsqueryHostID: new(fmt.Sprintf("idp-%d", i)),
NodeKey: new(fmt.Sprintf("idp-%d", i)),
UUID: fmt.Sprintf("idp-uuid%d", i),
Hostname: fmt.Sprintf("idp-host%d.local", i),
HardwareSerial: fmt.Sprintf("idp-hwd%d", i),
Platform: "darwin",
TeamID: teamIDs[i],
})
require.NoError(t, err)
hosts[i] = host
}
// Create a SCIM user per host, all in the "Engineering" IdP group.
scimUserIDs := make([]uint, 3)
for i := range 3 {
id, err := ds.CreateScimUser(ctx, &fleet.ScimUser{
UserName: fmt.Sprintf("idp-user%d", i),
Active: new(true),
})
require.NoError(t, err)
scimUserIDs[i] = id
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
_, err := q.ExecContext(ctx,
"INSERT INTO host_scim_user (host_id, scim_user_id) VALUES (?, ?)",
hosts[i].ID, id)
return err
})
}
_, err = ds.CreateScimGroup(ctx, &fleet.ScimGroup{
DisplayName: "Engineering",
ScimUsers: scimUserIDs,
})
require.NoError(t, err)
criteria, err := json.Marshal(&fleet.HostVitalCriteria{
Vital: new("end_user_idp_group"),
Value: new("Engineering"),
})
require.NoError(t, err)
newIDPLabel := func(name string, teamID *uint) *fleet.Label {
lbl, err := ds.NewLabel(ctx, &fleet.Label{
Name: name,
TeamID: teamID,
LabelType: fleet.LabelTypeRegular,
LabelMembershipType: fleet.LabelMembershipTypeHostVitals,
HostVitalsCriteria: new(json.RawMessage(criteria)),
})
require.NoError(t, err)
return lbl
}
globalLabel := newIDPLabel("idp-global", nil)
team1Label := newIDPLabel("idp-team1-label", &team1.ID)
filter := fleet.TeamFilter{User: test.UserAdmin}
// Global label: all three hosts (all SCIM users are in "Engineering").
updated, err := ds.UpdateLabelMembershipByHostCriteria(ctx, globalLabel)
require.NoError(t, err)
require.Equal(t, 3, updated.HostCount)
globalHosts, err := ds.ListHostsInLabel(ctx, filter, globalLabel.ID, fleet.HostListOptions{})
require.NoError(t, err)
require.ElementsMatch(t, []uint{hosts[0].ID, hosts[1].ID, hosts[2].ID}, hostIDs(globalHosts))
// Team1 label: only host1 (in team1) despite host2/host3 also being in the
// "Engineering" IdP group. Before the fix this returned an error / zero hosts.
updated, err = ds.UpdateLabelMembershipByHostCriteria(ctx, team1Label)
require.NoError(t, err)
require.Equal(t, 1, updated.HostCount)
team1Hosts, err := ds.ListHostsInLabel(ctx, filter, team1Label.ID, fleet.HostListOptions{})
require.NoError(t, err)
require.ElementsMatch(t, []uint{hosts[0].ID}, hostIDs(team1Hosts))
_ = team2 // team2 is used only to give host2 an out-of-team membership.
}
func hostIDs(hosts []*fleet.Host) []uint {
ids := make([]uint, 0, len(hosts))
for _, h := range hosts {
ids = append(ids, h.ID)
}
return ids
}
func testTeamLabels(t *testing.T, ds *Datastore) {
ctx := t.Context()
server/fleet/hosts.go
+10 -2
View File
@@ -497,8 +497,16 @@ type HostVital struct {
var hostForeignVitalGroups = map[string]HostForeignVitalGroup{
"idp": {
Name: "Identity Provider",
Query: `RIGHT JOIN host_scim_user ON (hosts.id = host_scim_user.host_id) JOIN scim_users ON (host_scim_user.scim_user_id = scim_users.id) LEFT JOIN scim_user_group ON (host_scim_user.scim_user_id = scim_user_group.scim_user_id) LEFT JOIN scim_groups ON (scim_user_group.group_id = scim_groups.id)`,
Name: "Identity Provider",
// NOTE: This must be an INNER JOIN (not RIGHT JOIN) on host_scim_user. A
// RIGHT JOIN keeps all host_scim_user rows even when the host side has been
// filtered out -- e.g. for fleet/team-scoped labels, where the hosts table
// is pre-filtered to the label's team. An out-of-team scim user that
// matches the criteria would then survive the join with hosts.id = NULL,
// which both leaks cross-team membership and breaks the INSERT into
// label_membership (NULL host_id, which is NOT NULL), rolling back the whole
// update so the fleet label gets zero hosts. See #46869.
Query: `JOIN host_scim_user ON (hosts.id = host_scim_user.host_id) JOIN scim_users ON (host_scim_user.scim_user_id = scim_users.id) LEFT JOIN scim_user_group ON (host_scim_user.scim_user_id = scim_user_group.scim_user_id) LEFT JOIN scim_groups ON (scim_user_group.group_id = scim_groups.id)`,
},
}
server/service/integration_core_test.go
+2 -2
View File
@@ -5700,7 +5700,7 @@ func (s *integrationTestSuite) TestLabels() {
queryValuesJson, err := json.Marshal(queryValues)
require.NoError(t, err)
assert.Equal(t, "SELECT %s FROM %s RIGHT JOIN host_scim_user ON (hosts.id = host_scim_user.host_id) JOIN scim_users ON (host_scim_user.scim_user_id = scim_users.id) LEFT JOIN scim_user_group ON (host_scim_user.scim_user_id = scim_user_group.scim_user_id) LEFT JOIN scim_groups ON (scim_user_group.group_id = scim_groups.id) WHERE scim_groups.display_name = ? GROUP BY hosts.id", query)
assert.Equal(t, "SELECT %s FROM %s JOIN host_scim_user ON (hosts.id = host_scim_user.host_id) JOIN scim_users ON (host_scim_user.scim_user_id = scim_users.id) LEFT JOIN scim_user_group ON (host_scim_user.scim_user_id = scim_user_group.scim_user_id) LEFT JOIN scim_groups ON (scim_user_group.group_id = scim_groups.id) WHERE scim_groups.display_name = ? GROUP BY hosts.id", query)
assert.Equal(t, `["group_good"]`, string(queryValuesJson))
// Update label membership.
@@ -5757,7 +5757,7 @@ func (s *integrationTestSuite) TestLabels() {
queryValuesJson, err := json.Marshal(queryValues)
require.NoError(t, err)
assert.Equal(t, "SELECT %s FROM %s RIGHT JOIN host_scim_user ON (hosts.id = host_scim_user.host_id) JOIN scim_users ON (host_scim_user.scim_user_id = scim_users.id) LEFT JOIN scim_user_group ON (host_scim_user.scim_user_id = scim_user_group.scim_user_id) LEFT JOIN scim_groups ON (scim_user_group.group_id = scim_groups.id) WHERE scim_users.department = ? GROUP BY hosts.id", query)
assert.Equal(t, "SELECT %s FROM %s JOIN host_scim_user ON (hosts.id = host_scim_user.host_id) JOIN scim_users ON (host_scim_user.scim_user_id = scim_users.id) LEFT JOIN scim_user_group ON (host_scim_user.scim_user_id = scim_user_group.scim_user_id) LEFT JOIN scim_groups ON (scim_user_group.group_id = scim_groups.id) WHERE scim_users.department = ? GROUP BY hosts.id", query)
assert.Equal(t, `["department_good"]`, string(queryValuesJson))
// Update label membership.
server/service/labels_test.go
+1 -1
View File
@@ -987,7 +987,7 @@ func TestNewHostVitalsLabel(t *testing.T) {
require.NoError(t, err)
queryValuesJson, err := json.Marshal(queryValues)
require.NoError(t, err)
assert.Equal(t, "SELECT %s FROM %s RIGHT JOIN host_scim_user ON (hosts.id = host_scim_user.host_id) JOIN scim_users ON (host_scim_user.scim_user_id = scim_users.id) LEFT JOIN scim_user_group ON (host_scim_user.scim_user_id = scim_user_group.scim_user_id) LEFT JOIN scim_groups ON (scim_user_group.group_id = scim_groups.id) WHERE scim_groups.display_name = ? GROUP BY hosts.id", query)
assert.Equal(t, "SELECT %s FROM %s JOIN host_scim_user ON (hosts.id = host_scim_user.host_id) JOIN scim_users ON (host_scim_user.scim_user_id = scim_users.id) LEFT JOIN scim_user_group ON (host_scim_user.scim_user_id = scim_user_group.scim_user_id) LEFT JOIN scim_groups ON (scim_user_group.group_id = scim_groups.id) WHERE scim_groups.display_name = ? GROUP BY hosts.id", query)
assert.Equal(t, `["admin"]`, string(queryValuesJson))
})
}