Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 61 additions & 26 deletions server/datastore/mysql/hosts.go
Original file line number Diff line number Diff line change
Expand Up @@ -4253,7 +4253,7 @@ func (ds *Datastore) DeleteHostIDP(ctx context.Context, id uint) error {

// remove an scim associations, if present. Note that this will not delete the associated row in
// scim_users, if it exists
if err := deleteHostSCIMUserMapping(ctx, tx, id); err != nil {
if _, err := deleteHostSCIMUserMapping(ctx, tx, id); err != nil {
return ctxerr.Wrap(ctx, err, "delete existing host SCIM user mapping")
}

Expand Down Expand Up @@ -4816,7 +4816,7 @@ WHERE %s`
// host_id (INSERT ... ON DUPLICATE KEY UPDATE) and handles its own profile resend,
// so calling it once per host is safe.
for _, hid := range hostIDs {
if err := associateHostWithScimUser(ctx, tx, hid, user.ID); err != nil {
if _, err := associateHostWithScimUser(ctx, tx, hid, user.ID); err != nil {
return ctxerr.Wrapf(ctx, err, "maybeAssociateScimUserWithHostMDMIdPAccount: associate host %d with scim user", hid)
}
}
Expand Down Expand Up @@ -4890,7 +4890,7 @@ func maybeAssociateHostMDMIdPWithScimUser(ctx context.Context, tx sqlx.ExtContex
return nil
}

err = associateHostWithScimUser(ctx, tx, hostID, scimUser.ID)
_, err = associateHostWithScimUser(ctx, tx, hostID, scimUser.ID)
if err != nil {
return ctxerr.Wrap(ctx, err, "associate host with scim user")
}
Expand All @@ -4900,11 +4900,12 @@ func maybeAssociateHostMDMIdPWithScimUser(ctx context.Context, tx sqlx.ExtContex

func (ds *Datastore) associateHostWithScimUser(ctx context.Context, hostID uint, scimUserID uint) error {
return ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error {
return associateHostWithScimUser(ctx, tx, hostID, scimUserID)
_, err := associateHostWithScimUser(ctx, tx, hostID, scimUserID)
return err
})
}

func associateHostWithScimUser(ctx context.Context, tx sqlx.ExtContext, hostID uint, scimUserID uint) error {
func associateHostWithScimUser(ctx context.Context, tx sqlx.ExtContext, hostID uint, scimUserID uint) ([]fleet.ActivityTypeResentCertificate, error) {
// On conflict with host_scim_user.PRIMARY (host_id), reassign the host to the current SCIM user.
result, err := tx.ExecContext(
ctx,
Expand All @@ -4913,14 +4914,14 @@ func associateHostWithScimUser(ctx context.Context, tx sqlx.ExtContext, hostID u
hostID, scimUserID,
)
if err != nil {
return ctxerr.Wrap(ctx, err, "insert into host_scim_user")
return nil, ctxerr.Wrap(ctx, err, "insert into host_scim_user")
}
rows, err := result.RowsAffected()
if err != nil {
return ctxerr.Wrap(ctx, err, "host_scim_user rows affected")
return nil, ctxerr.Wrap(ctx, err, "host_scim_user rows affected")
}
if rows == 0 {
return nil
return nil, nil
}
// resend profiles that depend on the user now associated with that host
//
Expand All @@ -4930,37 +4931,71 @@ func associateHostWithScimUser(ctx context.Context, tx sqlx.ExtContext, hostID u
}

// deleteHostSCIMUserMapping is a helper function to delete SCIM user mapping for a host
func deleteHostSCIMUserMapping(ctx context.Context, exec sqlx.ExtContext, hostID uint) error {
_, err := exec.ExecContext(ctx, `DELETE FROM host_scim_user WHERE host_id = ?`, hostID)
func deleteHostSCIMUserMapping(ctx context.Context, exec sqlx.ExtContext, hostID uint) ([]fleet.ActivityTypeResentCertificate, error) {
result, err := exec.ExecContext(ctx, `DELETE FROM host_scim_user WHERE host_id = ?`, hostID)
if err != nil {
return nil, ctxerr.Wrap(ctx, err, "delete host SCIM user mapping")
}
rows, err := result.RowsAffected()
if err != nil {
return ctxerr.Wrap(ctx, err, "delete host SCIM user mapping")
return nil, ctxerr.Wrap(ctx, err, "delete host SCIM user mapping rows affected")
}
if rows == 0 {
return nil, nil
}

return triggerResendProfilesUsingVariables(ctx, exec, []uint{hostID},
[]fleet.FleetVarName{
fleet.FleetVarHostEndUserIDPUsername,
fleet.FleetVarHostEndUserIDPUsernameLocalPart,
fleet.FleetVarHostEndUserIDPGroups,
fleet.FleetVarHostEndUserIDPDepartment,
fleet.FleetVarHostEndUserIDPFullname,
})
vars := []fleet.FleetVarName{
fleet.FleetVarHostEndUserIDPUsername,
fleet.FleetVarHostEndUserIDPUsernameLocalPart,
fleet.FleetVarHostEndUserIDPGroups,
fleet.FleetVarHostEndUserIDPDepartment,
fleet.FleetVarHostEndUserIDPFullname,
}
resentCerts, err := selectCertTemplatesToResend(ctx, exec, []uint{hostID}, fleetVarNamesToDBVars(vars))
if err != nil {
return nil, err
}
if err := triggerResendProfilesUsingVariables(ctx, exec, []uint{hostID}, vars); err != nil {
return nil, err
}
return resentCerts, nil
}

func (ds *Datastore) SetOrUpdateHostSCIMUserMapping(ctx context.Context, hostID uint, scimUserID uint) error {
return ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error {
func (ds *Datastore) SetOrUpdateHostSCIMUserMapping(ctx context.Context, hostID uint, scimUserID uint) ([]fleet.ActivityTypeResentCertificate, error) {
var allResentCerts []fleet.ActivityTypeResentCertificate
err := ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error {
allResentCerts = nil
// Remove any existing SCIM user mapping for this host
if err := deleteHostSCIMUserMapping(ctx, tx, hostID); err != nil {
delCerts, err := deleteHostSCIMUserMapping(ctx, tx, hostID)
if err != nil {
return err
}
allResentCerts = append(allResentCerts, delCerts...)

return associateHostWithScimUser(ctx, tx, hostID, scimUserID)
assocCerts, err := associateHostWithScimUser(ctx, tx, hostID, scimUserID)
if err != nil {
return err
}
allResentCerts = append(allResentCerts, assocCerts...)
return nil
})
if err != nil {
return nil, err
}
return allResentCerts, nil
}
Comment thread
ksykulev marked this conversation as resolved.

func (ds *Datastore) DeleteHostSCIMUserMapping(ctx context.Context, hostID uint) error {
return ds.withTx(ctx, func(tx sqlx.ExtContext) error {
return deleteHostSCIMUserMapping(ctx, tx, hostID)
func (ds *Datastore) DeleteHostSCIMUserMapping(ctx context.Context, hostID uint) ([]fleet.ActivityTypeResentCertificate, error) {
var resentCerts []fleet.ActivityTypeResentCertificate
err := ds.withTx(ctx, func(tx sqlx.ExtContext) error {
var txErr error
resentCerts, txErr = deleteHostSCIMUserMapping(ctx, tx, hostID)
return txErr
})
if err != nil {
return nil, err
}
return resentCerts, nil
}

func (ds *Datastore) GetHostEmails(ctx context.Context, hostUUID string, source string) ([]string, error) {
Expand Down
3 changes: 2 additions & 1 deletion server/datastore/mysql/hosts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9615,7 +9615,8 @@ func testHostsDeleteHosts(t *testing.T, ds *Datastore) {
// Create a SCIM user and link it to host
scimUserID, err := ds.CreateScimUser(ctx, &fleet.ScimUser{UserName: "user"})
require.NoError(t, err)
require.NoError(t, associateHostWithScimUser(ctx, ds.writer(ctx), host.ID, scimUserID))
_, err = associateHostWithScimUser(ctx, ds.writer(ctx), host.ID, scimUserID)
require.NoError(t, err)

script, err := ds.NewScript(ctx, &fleet.Script{
Name: "script.sh",
Expand Down
30 changes: 19 additions & 11 deletions server/datastore/mysql/scim.go
Original file line number Diff line number Diff line change
Expand Up @@ -1266,25 +1266,32 @@ func triggerResendProfilesForIDPGroupChangeByUsers(ctx context.Context, tx sqlx.
[]fleet.FleetVarName{fleet.FleetVarHostEndUserIDPGroups})
}

func triggerResendProfilesForIDPUserAddedToHost(ctx context.Context, tx sqlx.ExtContext, hostID, updatedScimUserID uint) error {
func triggerResendProfilesForIDPUserAddedToHost(ctx context.Context, tx sqlx.ExtContext, hostID, updatedScimUserID uint) ([]fleet.ActivityTypeResentCertificate, error) {
// check that this user is indeed the scim IdP user for this host (and not an
// extra, unused one)
user, err := getScimUserLiteByHostID(ctx, tx, hostID)
if err != nil {
return err
return nil, err
}
if updatedScimUserID != user.ID {
// host is not impacted, updated user is not its IdP user
return nil
return nil, nil
}
return triggerResendProfilesUsingVariables(ctx, tx, []uint{hostID},
[]fleet.FleetVarName{
fleet.FleetVarHostEndUserIDPUsername,
fleet.FleetVarHostEndUserIDPUsernameLocalPart,
fleet.FleetVarHostEndUserIDPDepartment,
fleet.FleetVarHostEndUserIDPGroups,
fleet.FleetVarHostEndUserIDPFullname,
})
vars := []fleet.FleetVarName{
fleet.FleetVarHostEndUserIDPUsername,
fleet.FleetVarHostEndUserIDPUsernameLocalPart,
fleet.FleetVarHostEndUserIDPDepartment,
fleet.FleetVarHostEndUserIDPGroups,
fleet.FleetVarHostEndUserIDPFullname,
}
resentCerts, err := selectCertTemplatesToResend(ctx, tx, []uint{hostID}, fleetVarNamesToDBVars(vars))
if err != nil {
return nil, err
}
if err := triggerResendProfilesUsingVariables(ctx, tx, []uint{hostID}, vars); err != nil {
return nil, err
}
return resentCerts, nil
}

func selectCertTemplatesToResend(ctx context.Context, tx sqlx.ExtContext, hostIDs []uint, vars []any) ([]fleet.ActivityTypeResentCertificate, error) {
Expand Down Expand Up @@ -1355,6 +1362,7 @@ func selectCertTemplatesToResend(ctx context.Context, tx sqlx.ExtContext, hostID
HostDisplayName: fleet.HostDisplayName(r.ComputerName, r.Hostname, r.HardwareModel, r.HardwareSerial),
CertificateTemplateID: r.CertificateTemplateID,
CertificateName: r.CertificateName,
Automated: true,
})
}
return activities, nil
Expand Down
10 changes: 5 additions & 5 deletions server/datastore/mysql/scim_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2681,7 +2681,7 @@ func testSetOrUpdateHostSCIMUserMapping(t *testing.T, ds *Datastore) {
hostID2 := uint(2)

// Create new host-SCIM user mapping
err = ds.SetOrUpdateHostSCIMUserMapping(ctx, hostID1, user1.ID)
_, err = ds.SetOrUpdateHostSCIMUserMapping(ctx, hostID1, user1.ID)
require.NoError(t, err)

// Verify the mapping was created
Expand All @@ -2693,7 +2693,7 @@ func testSetOrUpdateHostSCIMUserMapping(t *testing.T, ds *Datastore) {
assert.Equal(t, user1.ID, scimUserID)

// Test 2: Update existing host-SCIM user mapping
err = ds.SetOrUpdateHostSCIMUserMapping(ctx, hostID1, user2.ID)
_, err = ds.SetOrUpdateHostSCIMUserMapping(ctx, hostID1, user2.ID)
require.NoError(t, err)

// Verify the mapping was updated (should now point to user2)
Expand All @@ -2712,7 +2712,7 @@ func testSetOrUpdateHostSCIMUserMapping(t *testing.T, ds *Datastore) {
assert.Equal(t, 1, count)

// Test 3: Create mapping for a different host
err = ds.SetOrUpdateHostSCIMUserMapping(ctx, hostID2, user1.ID)
_, err = ds.SetOrUpdateHostSCIMUserMapping(ctx, hostID2, user1.ID)
require.NoError(t, err)

// Verify both hosts have mappings
Expand All @@ -2730,7 +2730,7 @@ func testSetOrUpdateHostSCIMUserMapping(t *testing.T, ds *Datastore) {
assert.Equal(t, 2, count)

// Update mapping back to original user for hostID1
err = ds.SetOrUpdateHostSCIMUserMapping(ctx, hostID1, user1.ID)
_, err = ds.SetOrUpdateHostSCIMUserMapping(ctx, hostID1, user1.ID)
require.NoError(t, err)

// Verify hostID1 now maps to user1
Expand All @@ -2742,7 +2742,7 @@ func testSetOrUpdateHostSCIMUserMapping(t *testing.T, ds *Datastore) {

// Error case - non-existent SCIM user
nonExistentUserID := uint(999999)
err = ds.SetOrUpdateHostSCIMUserMapping(ctx, hostID1, nonExistentUserID)
_, err = ds.SetOrUpdateHostSCIMUserMapping(ctx, hostID1, nonExistentUserID)
require.Error(t, err)
assert.Contains(t, err.Error(), "foreign key constraint")

Expand Down
5 changes: 5 additions & 0 deletions server/fleet/activities.go
Original file line number Diff line number Diff line change
Expand Up @@ -1906,6 +1906,7 @@ type ActivityTypeResentCertificate struct {
HostDisplayName string `json:"host_display_name"`
CertificateTemplateID uint `json:"certificate_template_id"`
CertificateName string `json:"certificate_name"`
Automated bool `json:"-"`
}

func (a ActivityTypeResentCertificate) ActivityName() string {
Expand All @@ -1916,6 +1917,10 @@ func (a ActivityTypeResentCertificate) HostIDs() []uint {
return []uint{a.HostID}
}

func (a ActivityTypeResentCertificate) WasFromAutomation() bool {
return a.Automated
}

type ActivityTypeEditedHostIdpData struct {
HostID uint `json:"host_id"`
HostDisplayName string `json:"host_display_name"`
Expand Down
6 changes: 4 additions & 2 deletions server/fleet/datastore.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,9 +388,11 @@ type Datastore interface {
DeleteHostIDP(ctx context.Context, id uint) error
// SetOrUpdateHostSCIMUserMapping associates a host with a SCIM user. If a
// mapping already exists, it will be updated to the new SCIM user.
SetOrUpdateHostSCIMUserMapping(ctx context.Context, hostID uint, scimUserID uint) error
// Returns any resent certificate activities that need to be created.
SetOrUpdateHostSCIMUserMapping(ctx context.Context, hostID uint, scimUserID uint) ([]ActivityTypeResentCertificate, error)
// DeleteHostSCIMUserMapping removes the association between a host and a SCIM user.
DeleteHostSCIMUserMapping(ctx context.Context, hostID uint) error
// Returns any resent certificate activities that need to be created.
DeleteHostSCIMUserMapping(ctx context.Context, hostID uint) ([]ActivityTypeResentCertificate, error)
// ListHostBatteries returns the list of batteries for the given host ID.
ListHostBatteries(ctx context.Context, id uint) ([]*HostBattery, error)
ListUpcomingHostMaintenanceWindows(ctx context.Context, hid uint) ([]*HostMaintenanceWindow, error)
Expand Down
8 changes: 4 additions & 4 deletions server/mock/datastore_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,9 @@ type SetOrUpdateIDPHostDeviceMappingFunc func(ctx context.Context, hostID uint,

type DeleteHostIDPFunc func(ctx context.Context, id uint) error

type SetOrUpdateHostSCIMUserMappingFunc func(ctx context.Context, hostID uint, scimUserID uint) error
type SetOrUpdateHostSCIMUserMappingFunc func(ctx context.Context, hostID uint, scimUserID uint) ([]fleet.ActivityTypeResentCertificate, error)

type DeleteHostSCIMUserMappingFunc func(ctx context.Context, hostID uint) error
type DeleteHostSCIMUserMappingFunc func(ctx context.Context, hostID uint) ([]fleet.ActivityTypeResentCertificate, error)

type ListHostBatteriesFunc func(ctx context.Context, id uint) ([]*fleet.HostBattery, error)

Expand Down Expand Up @@ -6365,14 +6365,14 @@ func (s *DataStore) DeleteHostIDP(ctx context.Context, id uint) error {
return s.DeleteHostIDPFunc(ctx, id)
}

func (s *DataStore) SetOrUpdateHostSCIMUserMapping(ctx context.Context, hostID uint, scimUserID uint) error {
func (s *DataStore) SetOrUpdateHostSCIMUserMapping(ctx context.Context, hostID uint, scimUserID uint) ([]fleet.ActivityTypeResentCertificate, error) {
s.mu.Lock()
s.SetOrUpdateHostSCIMUserMappingFuncInvoked = true
s.mu.Unlock()
return s.SetOrUpdateHostSCIMUserMappingFunc(ctx, hostID, scimUserID)
}

func (s *DataStore) DeleteHostSCIMUserMapping(ctx context.Context, hostID uint) error {
func (s *DataStore) DeleteHostSCIMUserMapping(ctx context.Context, hostID uint) ([]fleet.ActivityTypeResentCertificate, error) {
s.mu.Lock()
s.DeleteHostSCIMUserMappingFuncInvoked = true
s.mu.Unlock()
Expand Down
18 changes: 16 additions & 2 deletions server/service/hosts.go
Original file line number Diff line number Diff line change
Expand Up @@ -2384,15 +2384,29 @@ func (svc *Service) SetHostDeviceMapping(ctx context.Context, hostID uint, email
if err == nil && scimUser != nil {
// User exists in SCIM, create/update the mapping for additional attributes
// This enables fields like idp_full_name, idp_groups, etc. to appear in the API
if err := svc.ds.SetOrUpdateHostSCIMUserMapping(ctx, hostID, scimUser.ID); err != nil {
resentCerts, err := svc.ds.SetOrUpdateHostSCIMUserMapping(ctx, hostID, scimUser.ID)
if err != nil {
// Log the error but don't fail the request since the main IDP mapping succeeded
svc.logger.DebugContext(ctx, "failed to set SCIM user mapping", "err", err)
} else {
for _, cert := range resentCerts {
if err := svc.NewActivity(ctx, nil, cert); err != nil {
svc.logger.DebugContext(ctx, "failed to create resent_certificate activity", "err", err)
}
}
}
} else {
// User doesn't exist in SCIM, remove any existing SCIM mapping for this host
if err := svc.ds.DeleteHostSCIMUserMapping(ctx, hostID); err != nil && !fleet.IsNotFound(err) {
resentCerts, err := svc.ds.DeleteHostSCIMUserMapping(ctx, hostID)
if err != nil && !fleet.IsNotFound(err) {
// Log the error but don't fail the request
svc.logger.DebugContext(ctx, "failed to delete SCIM user mapping", "err", err)
} else {
for _, cert := range resentCerts {
if err := svc.NewActivity(ctx, nil, cert); err != nil {
svc.logger.DebugContext(ctx, "failed to create resent_certificate activity", "err", err)
}
}
}
}

Expand Down
16 changes: 8 additions & 8 deletions server/service/hosts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4327,8 +4327,8 @@ func TestSetHostDeviceMapping(t *testing.T) {
ds.ScimUserByUserNameOrEmailFunc = func(ctx context.Context, userName, email string) (*fleet.ScimUser, error) {
return &fleet.ScimUser{ID: 1, UserName: "user@example.com"}, nil
}
ds.SetOrUpdateHostSCIMUserMappingFunc = func(ctx context.Context, hostID uint, scimUserID uint) error {
return nil
ds.SetOrUpdateHostSCIMUserMappingFunc = func(ctx context.Context, hostID uint, scimUserID uint) ([]fleet.ActivityTypeResentCertificate, error) {
return nil, nil
}
Comment on lines +4330 to 4332
ds.SetOrUpdateIDPHostDeviceMappingFunc = func(ctx context.Context, hostID uint, email string) error {
return nil
Expand Down Expand Up @@ -4368,8 +4368,8 @@ func TestSetHostDeviceMapping(t *testing.T) {
ds.SetOrUpdateIDPHostDeviceMappingFunc = func(ctx context.Context, hostID uint, email string) error {
return nil
}
ds.DeleteHostSCIMUserMappingFunc = func(ctx context.Context, hostID uint) error {
return nil
ds.DeleteHostSCIMUserMappingFunc = func(ctx context.Context, hostID uint) ([]fleet.ActivityTypeResentCertificate, error) {
return nil, nil
}
ds.ListHostDeviceMappingFunc = func(ctx context.Context, hostID uint) ([]*fleet.HostDeviceMapping, error) {
return []*fleet.HostDeviceMapping{{HostID: hostID, Email: "any@username.com", Source: fleet.DeviceMappingMDMIdpAccounts}}, nil
Expand Down Expand Up @@ -4436,8 +4436,8 @@ func TestSetHostDeviceMapping(t *testing.T) {
ds.ScimUserByUserNameOrEmailFunc = func(ctx context.Context, userName, email string) (*fleet.ScimUser, error) {
return &fleet.ScimUser{ID: 1, UserName: "new@example.com"}, nil
}
ds.SetOrUpdateHostSCIMUserMappingFunc = func(ctx context.Context, hostID uint, scimUserID uint) error {
return nil
ds.SetOrUpdateHostSCIMUserMappingFunc = func(ctx context.Context, hostID uint, scimUserID uint) ([]fleet.ActivityTypeResentCertificate, error) {
return nil, nil
}
ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) {
return &fleet.AppConfig{}, nil
Expand Down Expand Up @@ -4472,8 +4472,8 @@ func TestSetHostDeviceMapping(t *testing.T) {
ds.ScimUserByUserNameOrEmailFunc = func(ctx context.Context, userName, email string) (*fleet.ScimUser, error) {
return &fleet.ScimUser{ID: 1, UserName: "user@example.com"}, nil
}
ds.SetOrUpdateHostSCIMUserMappingFunc = func(ctx context.Context, hostID uint, scimUserID uint) error {
return nil
ds.SetOrUpdateHostSCIMUserMappingFunc = func(ctx context.Context, hostID uint, scimUserID uint) ([]fleet.ActivityTypeResentCertificate, error) {
return nil, nil
}
ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) {
return &fleet.AppConfig{}, nil
Expand Down
Loading
Loading