Prioritize using IdP email address when available for maintenance window scheduling (#37250)
This pull request updates the logic for selecting which user receives maintenance window calendar events on hosts with multiple users. The changes clarify and enforce a priority system for choosing the recipient email, ensuring that IdP-sourced emails are preferred, followed by Google Chrome profile emails. This affects both user-facing documentation and backend implementation. **User-facing behavior and documentation:** * The end-user documentation now explicitly describes the email selection priority for calendar event recipients: IdP Username email is chosen first, then Google Chrome profile email, and if multiple Chrome emails exist, the first alphabetically is selected. **Backend logic and data selection:** * The comment in `calendar_cron.go` is updated to match the new email selection logic, explaining the prioritization of email sources for host-user assignment. * The SQL query in `policies.go` is refactored to implement the new priority system for selecting user emails per host: - IdP sources (`mdm_idp_accounts`, `idp`) are considered first, - then Google Chrome profiles, - then other sources. - If multiple emails exist at the same priority, the first alphabetically is chosen. --------- Co-authored-by: Mike Thomas <78363703+mike-j-thomas@users.noreply.github.com> Co-authored-by: Noah Talerman <47070608+noahtalerman@users.noreply.github.com> Co-authored-by: Juan Fernandez <juan-fdz-hawa@users.noreply.github.com> Co-authored-by: Juan Fernandez <juan@fleetdm.com>
This commit is contained in:
co-authored by
Mike Thomas
Noah Talerman
Juan Fernandez
Juan Fernandez
parent
c95283c490
commit
518475fc4c
@@ -131,7 +131,9 @@ func cronCalendarEventsForTeam(
|
||||
// NOTEs:
|
||||
// - We ignore hosts that are passing all policies and do not have an associated email.
|
||||
// - We get only one host per email that's failing policies (the one with lower host id).
|
||||
// - On every host, we get only the first email that matches the domain (sorted lexicographically).
|
||||
// - On every host, we prioritize email selection: IdP Username (mdm_idp_accounts or idp sources) first,
|
||||
// then Google Chrome profiles, then other sources. If multiple Google Chrome profile emails exist,
|
||||
// we select the first one alphabetically.
|
||||
// - GetTeamHostsPolicyMemberships returns the hosts that are passing all policies and have a calendar event.
|
||||
//
|
||||
|
||||
|
||||
@@ -2341,18 +2341,38 @@ func (ds *Datastore) GetTeamHostsPolicyMemberships(
|
||||
GROUP BY host_id
|
||||
) pm ON h.id = pm.host_id
|
||||
LEFT JOIN (
|
||||
SELECT host_id, MIN(email) AS email
|
||||
FROM host_emails
|
||||
JOIN hosts ON host_emails.host_id=hosts.id
|
||||
WHERE email LIKE CONCAT('%@', ?) AND team_id = ?
|
||||
GROUP BY host_id
|
||||
SELECT host_id, email
|
||||
FROM (
|
||||
SELECT
|
||||
he.host_id,
|
||||
he.email,
|
||||
ROW_NUMBER() OVER (
|
||||
PARTITION BY he.host_id
|
||||
ORDER BY
|
||||
CASE
|
||||
WHEN he.source IN (?, ?) THEN 1 -- IdP sources (mdm_idp_accounts, idp) have priority 1
|
||||
WHEN he.source = ? THEN 2 -- Google Chrome profiles have priority 2
|
||||
ELSE 3 -- Other sources have lower priority
|
||||
END,
|
||||
he.email -- alphabetical tiebreaker within same priority
|
||||
) AS rn
|
||||
FROM host_emails he
|
||||
JOIN hosts h_email ON he.host_id = h_email.id
|
||||
WHERE he.email LIKE CONCAT('%@', ?) AND h_email.team_id = ?
|
||||
) ranked_emails
|
||||
WHERE rn = 1
|
||||
) sh ON h.id = sh.host_id
|
||||
LEFT JOIN host_display_names hdn ON h.id = hdn.host_id
|
||||
LEFT JOIN host_calendar_events hce ON h.id = hce.host_id
|
||||
WHERE h.team_id = ? AND ((pm.passing IS NOT NULL AND NOT pm.passing) OR (COALESCE(pm.passing, 1) AND hce.host_id IS NOT NULL))
|
||||
`
|
||||
|
||||
query, args, err := sqlx.In(query, policyIDs, domain, teamID, teamID)
|
||||
query, args, err := sqlx.In(query,
|
||||
policyIDs,
|
||||
fleet.DeviceMappingMDMIdpAccounts, fleet.DeviceMappingIDP, // IdP sources
|
||||
fleet.DeviceMappingGoogleChromeProfiles, // Chrome profiles
|
||||
domain, teamID, // domain and team_id for WHERE clause
|
||||
teamID) // h.team_id in main WHERE
|
||||
if err != nil {
|
||||
return nil, ctxerr.Wrapf(ctx, err, "build select get team hosts policy memberships query")
|
||||
}
|
||||
|
||||
@@ -67,6 +67,7 @@ func TestPolicies(t *testing.T) {
|
||||
{"TestPoliciesNameSort", testPoliciesNameSort},
|
||||
{"TestGetCalendarPolicies", testGetCalendarPolicies},
|
||||
{"GetTeamHostsPolicyMemberships", testGetTeamHostsPolicyMemberships},
|
||||
{"GetTeamHostsPolicyMembershipsEmailPriority", testGetTeamHostsPolicyMembershipsEmailPriority},
|
||||
{"TestPoliciesNewGlobalPolicyWithInstaller", testNewGlobalPolicyWithInstaller},
|
||||
{"TestPoliciesTeamPoliciesWithInstaller", testTeamPoliciesWithInstaller},
|
||||
{"TestPoliciesTeamPoliciesWithVPP", testTeamPoliciesWithVPP},
|
||||
@@ -4346,6 +4347,132 @@ func testGetTeamHostsPolicyMemberships(t *testing.T, ds *Datastore) {
|
||||
require.Equal(t, "display_name2", hostsTeam2[0].HostDisplayName)
|
||||
}
|
||||
|
||||
func testGetTeamHostsPolicyMembershipsEmailPriority(t *testing.T, ds *Datastore) {
|
||||
ctx := context.Background()
|
||||
|
||||
team, err := ds.NewTeam(ctx, &fleet.Team{Name: "test-team"})
|
||||
require.NoError(t, err)
|
||||
|
||||
calendarPolicy, err := ds.NewTeamPolicy(ctx, team.ID, nil, fleet.PolicyPayload{
|
||||
Name: "Calendar Policy",
|
||||
Query: "SELECT 1;",
|
||||
CalendarEventsEnabled: true,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
newHost := func(name string) *fleet.Host {
|
||||
h, err := ds.NewHost(ctx, &fleet.Host{
|
||||
OsqueryHostID: ptr.String(name),
|
||||
NodeKey: ptr.String(name),
|
||||
HardwareSerial: name + "-serial",
|
||||
ComputerName: name,
|
||||
TeamID: &team.ID,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
// Make the host fail the calendar policy so it always appears in results.
|
||||
err = ds.RecordPolicyQueryExecutions(ctx, h, map[uint]*bool{calendarPolicy.ID: ptr.Bool(false)}, time.Now(), false)
|
||||
require.NoError(t, err)
|
||||
return h
|
||||
}
|
||||
|
||||
setEmails := func(hostID uint, mappings ...*fleet.HostDeviceMapping) {
|
||||
// Group by source so each source gets its own ReplaceHostDeviceMapping call,
|
||||
// matching production usage (each source replaces its own slice).
|
||||
bySource := make(map[string][]*fleet.HostDeviceMapping)
|
||||
for _, m := range mappings {
|
||||
bySource[m.Source] = append(bySource[m.Source], m)
|
||||
}
|
||||
for src, ms := range bySource {
|
||||
err := ds.ReplaceHostDeviceMapping(ctx, hostID, ms, src)
|
||||
require.NoError(t, err)
|
||||
}
|
||||
}
|
||||
|
||||
getResults := func(domain string) []fleet.HostPolicyMembershipData {
|
||||
results, err := ds.GetTeamHostsPolicyMemberships(ctx, domain, team.ID, []uint{calendarPolicy.ID}, nil)
|
||||
require.NoError(t, err)
|
||||
sort.Slice(results, func(i, j int) bool { return results[i].HostID < results[j].HostID })
|
||||
return results
|
||||
}
|
||||
|
||||
emailFor := func(results []fleet.HostPolicyMembershipData, h *fleet.Host) string {
|
||||
for _, r := range results {
|
||||
if r.HostID == h.ID {
|
||||
return r.Email
|
||||
}
|
||||
}
|
||||
t.Fatalf("host %d not found in results", h.ID)
|
||||
return ""
|
||||
}
|
||||
|
||||
testCases := []struct {
|
||||
name string
|
||||
mappings []*fleet.HostDeviceMapping
|
||||
expected string
|
||||
}{
|
||||
{
|
||||
name: "idp-vs-chrome",
|
||||
mappings: []*fleet.HostDeviceMapping{
|
||||
{Email: "chrome@example.com", Source: fleet.DeviceMappingGoogleChromeProfiles},
|
||||
{Email: "idp@example.com", Source: fleet.DeviceMappingMDMIdpAccounts},
|
||||
},
|
||||
expected: "idp@example.com",
|
||||
},
|
||||
{
|
||||
name: "idpsrc-vs-chrome",
|
||||
mappings: []*fleet.HostDeviceMapping{
|
||||
{Email: "chrome@example.com", Source: fleet.DeviceMappingGoogleChromeProfiles},
|
||||
{Email: "idpsrc@example.com", Source: fleet.DeviceMappingIDP},
|
||||
},
|
||||
expected: "idpsrc@example.com",
|
||||
},
|
||||
{
|
||||
name: "custom-vs-chrome",
|
||||
mappings: []*fleet.HostDeviceMapping{
|
||||
{Email: "chrome@example.com", Source: fleet.DeviceMappingGoogleChromeProfiles},
|
||||
{Email: "custom@example.com", Source: "custom"},
|
||||
},
|
||||
expected: "chrome@example.com",
|
||||
},
|
||||
{
|
||||
name: "multi-idp",
|
||||
mappings: []*fleet.HostDeviceMapping{
|
||||
{Email: "zebra@example.com", Source: fleet.DeviceMappingMDMIdpAccounts},
|
||||
{Email: "apple@example.com", Source: fleet.DeviceMappingMDMIdpAccounts},
|
||||
},
|
||||
expected: "apple@example.com",
|
||||
},
|
||||
{
|
||||
name: "multi-chrome",
|
||||
mappings: []*fleet.HostDeviceMapping{
|
||||
{Email: "zebra@example.com", Source: fleet.DeviceMappingGoogleChromeProfiles},
|
||||
{Email: "apple@example.com", Source: fleet.DeviceMappingGoogleChromeProfiles},
|
||||
},
|
||||
expected: "apple@example.com",
|
||||
},
|
||||
{
|
||||
name: "idp-wrong-domain",
|
||||
mappings: []*fleet.HostDeviceMapping{
|
||||
{Email: "idp@other.com", Source: fleet.DeviceMappingMDMIdpAccounts},
|
||||
{Email: "chrome@example.com", Source: fleet.DeviceMappingGoogleChromeProfiles},
|
||||
},
|
||||
expected: "chrome@example.com",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tC := range testCases {
|
||||
host := newHost(tC.name)
|
||||
|
||||
for _, m := range tC.mappings {
|
||||
m.HostID = host.ID
|
||||
}
|
||||
setEmails(host.ID, tC.mappings...)
|
||||
|
||||
results := getResults("example.com")
|
||||
require.Equal(t, tC.expected, emailFor(results, host), tC.name)
|
||||
}
|
||||
}
|
||||
|
||||
func testNewGlobalPolicyWithInstaller(t *testing.T, ds *Datastore) {
|
||||
user1 := test.NewUser(t, ds, "Alice", "alice@example.com", true)
|
||||
_, err := ds.NewGlobalPolicy(context.Background(), &user1.ID, fleet.PolicyPayload{
|
||||
|
||||
Reference in New Issue
Block a user