Associate all matching hosts with a SCIM/IdP user (not just the first) (#48351)
Resolves https://github.com/fleetdm/fleet/issues/48378 (issue found while working on the Google Workspace IdP integration). ## Summary Fixes a bug where an IdP user associated with **multiple hosts** only had IdP host vitals populated on **one** of them. `maybeAssociateScimUserWithHostMDMIdP` (called when a SCIM/IdP user is created) matched all hosts whose MDM IdP account corresponds to the user, but then deliberately linked only `hostIDs[0]` (with a `// TODO: confirm desired behavior` / "just use the first one"). So when a user is created *after* the hosts already enrolled — e.g. a directory sync creating users for people who each have a laptop and a desktop — only the first host got a `host_scim_user` row, and therefore only that host received the user's IdP host vitals and profile-variable resends. The fix links **every** matching host. `associateHostWithScimUser` is keyed on `host_id` (`INSERT … ON DUPLICATE KEY UPDATE`) and triggers its own per-host profile resend, so calling it once per host is safe and idempotent. This is shared SCIM linking code, so the fix benefits all IdP sources (Okta/Entra SCIM as well as the Google Workspace directory sync that surfaced it). Deletes and updates already handled multiple hosts correctly; only the initial reverse-link was capped. ## Testing Added `testScimUserCreateAssociatesAllMatchingHosts` (`server/datastore/mysql/scim_test.go`): two hosts share one MDM IdP account, then a SCIM user is created — both hosts must resolve to it via `ScimUserByHostID`. Fails before the fix (host #2 unlinked), passes after. **Related issue:** Resolves #48378 # Checklist for submitter - [x] Changes file added for user-visible changes in `changes/`. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements). ## 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** * SCIM/IdP user provisioning now associates a new SCIM user with **all** matching hosts, not just the first match. * Host end-user details (including IdP username/full name) are now populated consistently on every associated host. * **Tests** * Added SCIM integration and datastore regression coverage to ensure multiple hosts linked to the same IdP account are all associated during user creation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
co-authored by
Copilot Autofix powered by AI
parent
ca4ce12d06
commit
2d70a7b500
@@ -9,6 +9,7 @@ import (
|
||||
"github.com/elimity-com/scim/errors"
|
||||
"github.com/fleetdm/fleet/v4/server/authz"
|
||||
"github.com/fleetdm/fleet/v4/server/datastore/mysql/mysqltest"
|
||||
"github.com/fleetdm/fleet/v4/server/fleet"
|
||||
"github.com/fleetdm/fleet/v4/server/service"
|
||||
"github.com/fleetdm/fleet/v4/server/service/contract"
|
||||
"github.com/fleetdm/fleet/v4/server/test"
|
||||
@@ -28,6 +29,7 @@ func TestSCIM(t *testing.T) {
|
||||
{"Users", testUsersBasicCRUD},
|
||||
{"Groups", testGroupsBasicCRUD},
|
||||
{"CreateUser", testCreateUser},
|
||||
{"CreateUserAssociatesAllMatchingHosts", testCreateUserAssociatesAllMatchingHosts},
|
||||
{"CreateGroup", testCreateGroup},
|
||||
{"UpdateUser", testUpdateUser},
|
||||
{"UpdateGroup", testUpdateGroup},
|
||||
@@ -1064,6 +1066,69 @@ func testCreateUser(t *testing.T, s *Suite) {
|
||||
s.Do(t, "DELETE", scimPath("/Users/"+userID7), nil, http.StatusNoContent)
|
||||
}
|
||||
|
||||
// testCreateUserAssociatesAllMatchingHosts verifies, end to end through the SCIM
|
||||
// API, that provisioning a user links every host whose MDM IdP account matches the
|
||||
// user — not just the first. This is the integration-level counterpart to the
|
||||
// datastore regression test for the multi-host reverse-linker fix.
|
||||
func testCreateUserAssociatesAllMatchingHosts(t *testing.T, s *Suite) {
|
||||
ctx := t.Context()
|
||||
|
||||
// Two hosts belonging to the same person, both authenticated via the same IdP account.
|
||||
host1 := test.NewHost(t, s.DS, "scim-multi-1", "1", "scim-mh1-key", "scim-mh1-uuid", time.Now())
|
||||
host2 := test.NewHost(t, s.DS, "scim-multi-2", "2", "scim-mh2-key", "scim-mh2-uuid", time.Now())
|
||||
|
||||
t.Cleanup(func() {
|
||||
// This test mutates host/MDM IdP tables, but the per-subtest truncation in TestSCIM
|
||||
// only clears SCIM tables. Clean up here to keep subtests isolated.
|
||||
mysqltest.TruncateTables(t, s.DS,
|
||||
"host_mdm_idp_accounts",
|
||||
"mdm_idp_accounts",
|
||||
"host_seen_times",
|
||||
"host_display_names",
|
||||
"hosts",
|
||||
)
|
||||
})
|
||||
const idpUUID = "scim-multi-idp-uuid"
|
||||
const userName = "scim.multi@example.com"
|
||||
require.NoError(t, s.DS.InsertMDMIdPAccount(ctx, &fleet.MDMIdPAccount{
|
||||
UUID: idpUUID,
|
||||
Username: userName,
|
||||
Fullname: "SCIM Multi",
|
||||
Email: userName,
|
||||
}))
|
||||
require.NoError(t, s.DS.AssociateHostMDMIdPAccount(ctx, host1.UUID, idpUUID))
|
||||
require.NoError(t, s.DS.AssociateHostMDMIdPAccount(ctx, host2.UUID, idpUUID))
|
||||
|
||||
// Provision the user through the SCIM API, as an IdP would.
|
||||
createPayload := map[string]any{
|
||||
"schemas": []string{"urn:ietf:params:scim:schemas:core:2.0:User"},
|
||||
"userName": userName,
|
||||
"name": map[string]any{
|
||||
"givenName": "SCIM",
|
||||
"familyName": "Multi",
|
||||
},
|
||||
"emails": []map[string]any{
|
||||
{"value": userName, "type": "work", "primary": true},
|
||||
},
|
||||
"active": true,
|
||||
}
|
||||
var createResp map[string]any
|
||||
s.DoJSON(t, "POST", scimPath("/Users"), createPayload, http.StatusCreated, &createResp)
|
||||
|
||||
// Both hosts must expose the user's IdP host vitals through the host detail API.
|
||||
for _, hostID := range []uint{host1.ID, host2.ID} {
|
||||
var resp struct {
|
||||
Host struct {
|
||||
EndUsers []fleet.HostEndUser `json:"end_users"`
|
||||
} `json:"host"`
|
||||
}
|
||||
s.DoJSON(t, "GET", fmt.Sprintf("/api/latest/fleet/hosts/%d", hostID), nil, http.StatusOK, &resp)
|
||||
require.Len(t, resp.Host.EndUsers, 1, "host %d should expose one IdP end user", hostID)
|
||||
assert.Equal(t, userName, resp.Host.EndUsers[0].IdpUserName, "host %d idp_username", hostID)
|
||||
assert.Equal(t, "SCIM Multi", resp.Host.EndUsers[0].IdpFullName, "host %d idp_full_name", hostID)
|
||||
}
|
||||
}
|
||||
|
||||
func testUpdateUser(t *testing.T, s *Suite) {
|
||||
// Create first user
|
||||
firstUserPayload := map[string]interface{}{
|
||||
|
||||
Reference in New Issue
Block a user