diff --git a/ee/server/scim/users.go b/ee/server/scim/users.go index 95459e211fa..fb86bd5da9a 100644 --- a/ee/server/scim/users.go +++ b/ee/server/scim/users.go @@ -88,11 +88,16 @@ func (u *UserHandler) Create(r *http.Request, attributes scim.ResourceAttributes return scim.Resource{}, err } user.ID = existingUser.ID - err = u.ds.ReplaceScimUser(ctx, user) + resentCerts, err := u.ds.ReplaceScimUser(ctx, user) if err != nil { u.logger.ErrorContext(ctx, "failed to reactivate user", userNameAttr, userName, "err", err) return scim.Resource{}, err } + for _, cert := range resentCerts { + if err := u.newActivity(ctx, nil, cert); err != nil { + u.logger.ErrorContext(ctx, "failed to create resent_certificate activity", "err", err) + } + } return createUserResource(user), nil } u.logger.InfoContext(ctx, "user already exists", userNameAttr, userName) @@ -484,7 +489,7 @@ func (u *UserHandler) Replace(r *http.Request, id string, attributes scim.Resour previousActive = existingUser.Active } - err = u.ds.ReplaceScimUser(ctx, user) + resentCerts, err := u.ds.ReplaceScimUser(ctx, user) switch { case fleet.IsNotFound(err): u.logger.InfoContext(ctx, "failed to find user to replace", "id", id) @@ -498,6 +503,12 @@ func (u *UserHandler) Replace(r *http.Request, id string, attributes scim.Resour return scim.Resource{}, err } + for _, cert := range resentCerts { + if err := u.newActivity(ctx, nil, cert); err != nil { + u.logger.ErrorContext(ctx, "failed to create resent_certificate activity", "err", err) + } + } + // Check if user was deactivated and delete matching Fleet user if so if wasDeactivated(previousActive, user.Active) { if err := u.deleteMatchingFleetUser(ctx, user); err != nil { @@ -536,7 +547,7 @@ func (u *UserHandler) Delete(r *http.Request, id string) error { } } - err = u.ds.DeleteScimUser(ctx, idUint) + resentCerts, err := u.ds.DeleteScimUser(ctx, idUint) switch { case fleet.IsNotFound(err): u.logger.InfoContext(ctx, "failed to find user to delete", "id", id) @@ -546,6 +557,12 @@ func (u *UserHandler) Delete(r *http.Request, id string) error { return err } + for _, cert := range resentCerts { + if err := u.newActivity(ctx, nil, cert); err != nil { + u.logger.ErrorContext(ctx, "failed to create resent_certificate activity", "err", err) + } + } + return nil } @@ -751,7 +768,7 @@ func (u *UserHandler) Patch(r *http.Request, id string, operations []scim.PatchO } if !allUnknown { - err = u.ds.ReplaceScimUser(ctx, user) + resentCerts, err := u.ds.ReplaceScimUser(ctx, user) switch { case fleet.IsNotFound(err): u.logger.InfoContext(ctx, "failed to find user to patch", "id", id) @@ -765,6 +782,12 @@ func (u *UserHandler) Patch(r *http.Request, id string, operations []scim.PatchO return scim.Resource{}, err } + for _, cert := range resentCerts { + if err := u.newActivity(ctx, nil, cert); err != nil { + u.logger.ErrorContext(ctx, "failed to create resent_certificate activity", "err", err) + } + } + // Check if user was deactivated and delete matching Fleet user if so. // This sits inside `if !allUnknown` because patchActive only runs when at // least one recognized op was applied; if every op was unrecognized, diff --git a/ee/server/scim/users_test.go b/ee/server/scim/users_test.go index 64a0da7f4a8..258e625f195 100644 --- a/ee/server/scim/users_test.go +++ b/ee/server/scim/users_test.go @@ -381,8 +381,8 @@ func TestUserHandlerDelete(t *testing.T) { mocks.ds.DeleteUserFunc = func(ctx context.Context, id uint) error { return nil } - mocks.ds.DeleteScimUserFunc = func(ctx context.Context, id uint) error { - return nil + mocks.ds.DeleteScimUserFunc = func(ctx context.Context, id uint) ([]fleet.ActivityTypeResentCertificate, error) { + return nil, nil } mocks.svc.NewActivityFunc = func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails) error { return nil @@ -419,8 +419,8 @@ func TestUserHandlerDelete(t *testing.T) { return fleet.ErrLastGlobalAdmin } // SCIM user deletion should still succeed - mocks.ds.DeleteScimUserFunc = func(ctx context.Context, id uint) error { - return nil + mocks.ds.DeleteScimUserFunc = func(ctx context.Context, id uint) ([]fleet.ActivityTypeResentCertificate, error) { + return nil, nil } handler := mocks.newTestHandler() @@ -439,8 +439,8 @@ func TestUserHandlerDelete(t *testing.T) { return nil, platform_mysql.NotFound("ScimUser") } // DeleteScimUser is still called to ensure triggerResendProfilesForIDPUserDeleted runs - mocks.ds.DeleteScimUserFunc = func(ctx context.Context, id uint) error { - return platform_mysql.NotFound("ScimUser") + mocks.ds.DeleteScimUserFunc = func(ctx context.Context, id uint) ([]fleet.ActivityTypeResentCertificate, error) { + return nil, platform_mysql.NotFound("ScimUser") } handler := mocks.newTestHandler() @@ -539,8 +539,8 @@ func TestUserHandlerReplaceDeactivation(t *testing.T) { mocks.ds.ScimUserByUserNameFunc = func(ctx context.Context, userName string) (*fleet.ScimUser, error) { return existingScimUser, nil } - mocks.ds.ReplaceScimUserFunc = func(ctx context.Context, user *fleet.ScimUser) error { - return nil + mocks.ds.ReplaceScimUserFunc = func(ctx context.Context, user *fleet.ScimUser) ([]fleet.ActivityTypeResentCertificate, error) { + return nil, nil } mocks.ds.UserByEmailFunc = func(ctx context.Context, email string) (*fleet.User, error) { return fleetUser, nil @@ -580,8 +580,8 @@ func TestUserHandlerReplaceDeactivation(t *testing.T) { mocks.ds.ScimUserByUserNameFunc = func(ctx context.Context, userName string) (*fleet.ScimUser, error) { return existingScimUser, nil } - mocks.ds.ReplaceScimUserFunc = func(ctx context.Context, user *fleet.ScimUser) error { - return nil + mocks.ds.ReplaceScimUserFunc = func(ctx context.Context, user *fleet.ScimUser) ([]fleet.ActivityTypeResentCertificate, error) { + return nil, nil } handler := mocks.newTestHandler() @@ -607,8 +607,8 @@ func TestUserHandlerReplaceDeactivation(t *testing.T) { mocks.ds.ScimUserByUserNameFunc = func(ctx context.Context, userName string) (*fleet.ScimUser, error) { return existingScimUser, nil } - mocks.ds.ReplaceScimUserFunc = func(ctx context.Context, user *fleet.ScimUser) error { - return nil + mocks.ds.ReplaceScimUserFunc = func(ctx context.Context, user *fleet.ScimUser) ([]fleet.ActivityTypeResentCertificate, error) { + return nil, nil } handler := mocks.newTestHandler() @@ -636,8 +636,8 @@ func TestUserHandlerReplaceDeactivation(t *testing.T) { return nil, platform_mysql.NotFound("ScimUser") } // ReplaceScimUser returns a wrapped AlreadyExistsError (race condition: concurrent update took the username) - mocks.ds.ReplaceScimUserFunc = func(ctx context.Context, user *fleet.ScimUser) error { - return fmt.Errorf("update scim user: %w", &alreadyExistsErr{msg: "user_name already exists"}) + mocks.ds.ReplaceScimUserFunc = func(ctx context.Context, user *fleet.ScimUser) ([]fleet.ActivityTypeResentCertificate, error) { + return nil, fmt.Errorf("update scim user: %w", &alreadyExistsErr{msg: "user_name already exists"}) } handler := mocks.newTestHandler() @@ -667,8 +667,8 @@ func TestUserHandlerReplaceDeactivation(t *testing.T) { return existingScimUser, nil } // ReplaceScimUser returns a validation error (field too long) - mocks.ds.ReplaceScimUserFunc = func(ctx context.Context, user *fleet.ScimUser) error { - return &fleet.SCIMValidationError{Field: "given_name", Message: "exceeds maximum length of 255 characters"} + mocks.ds.ReplaceScimUserFunc = func(ctx context.Context, user *fleet.ScimUser) ([]fleet.ActivityTypeResentCertificate, error) { + return nil, &fleet.SCIMValidationError{Field: "given_name", Message: "exceeds maximum length of 255 characters"} } handler := mocks.newTestHandler() @@ -697,8 +697,8 @@ func TestUserHandlerPatchDeactivation(t *testing.T) { mocks.ds.ScimUserByIDFunc = func(ctx context.Context, id uint) (*fleet.ScimUser, error) { return existingScimUser, nil } - mocks.ds.ReplaceScimUserFunc = func(ctx context.Context, user *fleet.ScimUser) error { - return nil + mocks.ds.ReplaceScimUserFunc = func(ctx context.Context, user *fleet.ScimUser) ([]fleet.ActivityTypeResentCertificate, error) { + return nil, nil } mocks.ds.UserByEmailFunc = func(ctx context.Context, email string) (*fleet.User, error) { return fleetUser, nil @@ -739,8 +739,8 @@ func TestUserHandlerPatchDeactivation(t *testing.T) { mocks.ds.ScimUserByIDFunc = func(ctx context.Context, id uint) (*fleet.ScimUser, error) { return existingScimUser, nil } - mocks.ds.ReplaceScimUserFunc = func(ctx context.Context, user *fleet.ScimUser) error { - return nil + mocks.ds.ReplaceScimUserFunc = func(ctx context.Context, user *fleet.ScimUser) ([]fleet.ActivityTypeResentCertificate, error) { + return nil, nil } mocks.ds.UserByEmailFunc = func(ctx context.Context, email string) (*fleet.User, error) { return fleetUser, nil @@ -777,8 +777,8 @@ func TestUserHandlerPatchDeactivation(t *testing.T) { mocks.ds.ScimUserByIDFunc = func(ctx context.Context, id uint) (*fleet.ScimUser, error) { return existingScimUser, nil } - mocks.ds.ReplaceScimUserFunc = func(ctx context.Context, user *fleet.ScimUser) error { - return nil + mocks.ds.ReplaceScimUserFunc = func(ctx context.Context, user *fleet.ScimUser) ([]fleet.ActivityTypeResentCertificate, error) { + return nil, nil } handler := mocks.newTestHandler() @@ -808,8 +808,8 @@ func TestUserHandlerPatchDeactivation(t *testing.T) { mocks.ds.ScimUserByIDFunc = func(ctx context.Context, id uint) (*fleet.ScimUser, error) { return existingScimUser, nil } - mocks.ds.ReplaceScimUserFunc = func(ctx context.Context, user *fleet.ScimUser) error { - return nil + mocks.ds.ReplaceScimUserFunc = func(ctx context.Context, user *fleet.ScimUser) ([]fleet.ActivityTypeResentCertificate, error) { + return nil, nil } handler := mocks.newTestHandler() @@ -840,8 +840,8 @@ func TestUserHandlerPatchDeactivation(t *testing.T) { return existingScimUser, nil } // ReplaceScimUser returns a wrapped AlreadyExistsError (race condition: concurrent update took the username) - mocks.ds.ReplaceScimUserFunc = func(ctx context.Context, user *fleet.ScimUser) error { - return fmt.Errorf("update scim user: %w", &alreadyExistsErr{msg: "user_name already exists"}) + mocks.ds.ReplaceScimUserFunc = func(ctx context.Context, user *fleet.ScimUser) ([]fleet.ActivityTypeResentCertificate, error) { + return nil, fmt.Errorf("update scim user: %w", &alreadyExistsErr{msg: "user_name already exists"}) } handler := mocks.newTestHandler() @@ -878,9 +878,9 @@ func TestUserHandlerCreateReactivation(t *testing.T) { } var replacedUser *fleet.ScimUser - mocks.ds.ReplaceScimUserFunc = func(ctx context.Context, user *fleet.ScimUser) error { + mocks.ds.ReplaceScimUserFunc = func(ctx context.Context, user *fleet.ScimUser) ([]fleet.ActivityTypeResentCertificate, error) { replacedUser = user - return nil + return nil, nil } handler := mocks.newTestHandler() @@ -1041,9 +1041,9 @@ func TestUserHandlerPatchUnknownAttributes(t *testing.T) { return existingUser, nil } var saved *fleet.ScimUser - mocks.ds.ReplaceScimUserFunc = func(ctx context.Context, user *fleet.ScimUser) error { + mocks.ds.ReplaceScimUserFunc = func(ctx context.Context, user *fleet.ScimUser) ([]fleet.ActivityTypeResentCertificate, error) { saved = user - return nil + return nil, nil } return mocks, existingUser, &saved } diff --git a/server/cron/google_workspace_cron.go b/server/cron/google_workspace_cron.go index 427179b1e61..06d2a84ee37 100644 --- a/server/cron/google_workspace_cron.go +++ b/server/cron/google_workspace_cron.go @@ -181,7 +181,7 @@ func syncGoogleWorkspaceUsers(ctx context.Context, ds fleet.Datastore, gwUsers [ gu.ID = ex.ID extIDToScimUserID[extID] = ex.ID if scimUserNeedsUpdate(ex, gu) { - if err := ds.ReplaceScimUser(ctx, gu); err != nil { + if _, err := ds.ReplaceScimUser(ctx, gu); err != nil { // Best-effort: log and skip this user so one bad record doesn't // abort the whole sync. failed++ @@ -214,7 +214,7 @@ func syncGoogleWorkspaceUsers(ctx context.Context, ds fleet.Datastore, gwUsers [ if _, ok := seen[extID]; ok { continue } - if err := ds.DeleteScimUser(ctx, ex.ID); err != nil { + if _, err := ds.DeleteScimUser(ctx, ex.ID); err != nil { failed++ logger.ErrorContext(ctx, "google workspace sync: failed to delete user no longer in directory", "scim_user_id", ex.ID, "external_id", extID, "err", err) diff --git a/server/cron/google_workspace_cron_test.go b/server/cron/google_workspace_cron_test.go index dce2d1dcfa2..52e260660ee 100644 --- a/server/cron/google_workspace_cron_test.go +++ b/server/cron/google_workspace_cron_test.go @@ -80,13 +80,13 @@ func newSyncRecorder(appConfig *fleet.AppConfig, existingUsers []fleet.ScimUser, r.createdUsers = append(r.createdUsers, user) return nextID, nil } - r.ds.ReplaceScimUserFunc = func(_ context.Context, user *fleet.ScimUser) error { + r.ds.ReplaceScimUserFunc = func(_ context.Context, user *fleet.ScimUser) ([]fleet.ActivityTypeResentCertificate, error) { r.replacedUsers = append(r.replacedUsers, user) - return nil + return nil, nil } - r.ds.DeleteScimUserFunc = func(_ context.Context, id uint) error { + r.ds.DeleteScimUserFunc = func(_ context.Context, id uint) ([]fleet.ActivityTypeResentCertificate, error) { r.deletedUsers = append(r.deletedUsers, id) - return nil + return nil, nil } r.ds.CreateScimGroupFunc = func(_ context.Context, group *fleet.ScimGroup) (uint, error) { nextID++ diff --git a/server/datastore/mysql/scim.go b/server/datastore/mysql/scim.go index 6c4f5ff733d..0d7c001463b 100644 --- a/server/datastore/mysql/scim.go +++ b/server/datastore/mysql/scim.go @@ -249,9 +249,9 @@ func getScimUserLiteByHostID(ctx context.Context, q sqlx.QueryerContext, hostID } // ReplaceScimUser replaces an existing SCIM user in the database -func (ds *Datastore) ReplaceScimUser(ctx context.Context, user *fleet.ScimUser) error { +func (ds *Datastore) ReplaceScimUser(ctx context.Context, user *fleet.ScimUser) ([]fleet.ActivityTypeResentCertificate, error) { if err := validateScimUserFields(user); err != nil { - return err + return nil, err } // Validate that at most one email is marked as primary @@ -262,17 +262,19 @@ func (ds *Datastore) ReplaceScimUser(ctx context.Context, user *fleet.ScimUser) } } if primaryCount > 1 { - return ctxerr.New(ctx, "only one email can be marked as primary") + return nil, ctxerr.New(ctx, "only one email can be marked as primary") } // Get current emails and check if they need to be updated currentEmails, err := ds.getScimUserEmails(ctx, user.ID) if err != nil { - return err + return nil, err } emailsNeedUpdate := emailsRequireUpdate(currentEmails, user.Emails) - return ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error { + var resentCerts []fleet.ActivityTypeResentCertificate + err = ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error { + resentCerts = nil // load the username and department before updating the user, to check if it changed old := struct { UserName string `db:"user_name"` @@ -356,14 +358,19 @@ func (ds *Datastore) ReplaceScimUser(ctx context.Context, user *fleet.ScimUser) // resend profiles that depend on this username if it changed if usernameChanged || departmentChanged || nameChanged { - err = triggerResendProfilesForIDPUserChange(ctx, tx, user.ID) + certs, err := triggerResendProfilesForIDPUserChange(ctx, tx, user.ID) if err != nil { return err } + resentCerts = append(resentCerts, certs...) } return nil }) + if err != nil { + return nil, err + } + return resentCerts, nil } func insertEmails(ctx context.Context, tx sqlx.ExtContext, user *fleet.ScimUser) error { @@ -400,14 +407,18 @@ func insertEmails(ctx context.Context, tx sqlx.ExtContext, user *fleet.ScimUser) } // DeleteScimUser deletes a SCIM user from the database -func (ds *Datastore) DeleteScimUser(ctx context.Context, id uint) error { - return ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error { +func (ds *Datastore) DeleteScimUser(ctx context.Context, id uint) ([]fleet.ActivityTypeResentCertificate, error) { + var resentCerts []fleet.ActivityTypeResentCertificate + err := ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error { + resentCerts = nil + // trigger resend of profiles that depend on this SCIM user (must be done // _before_ deleting the scim user so that we can find the affected hosts) - err := triggerResendProfilesForIDPUserDeleted(ctx, tx, id) + certs, err := triggerResendProfilesForIDPUserDeleted(ctx, tx, id) if err != nil { return err } + resentCerts = append(resentCerts, certs...) // Delete the user const deleteUserQuery = `DELETE FROM scim_users WHERE id = ?` @@ -427,6 +438,10 @@ func (ds *Datastore) DeleteScimUser(ctx context.Context, id uint) error { return nil }) + if err != nil { + return nil, err + } + return resentCerts, nil } // ListScimUsers retrieves a list of SCIM users with optional filtering @@ -1176,33 +1191,47 @@ func getHostIDsHavingScimIDPUsers(ctx context.Context, tx sqlx.ExtContext, scimU return hostIDs, nil } -func triggerResendProfilesForIDPUserChange(ctx context.Context, tx sqlx.ExtContext, updatedScimUserID uint) error { +func triggerResendProfilesForIDPUserChange(ctx context.Context, tx sqlx.ExtContext, updatedScimUserID uint) ([]fleet.ActivityTypeResentCertificate, error) { hostIDs, err := getHostIDsHavingScimIDPUser(ctx, tx, updatedScimUserID) if err != nil { - return err + return nil, err } - return triggerResendProfilesUsingVariables(ctx, tx, hostIDs, - []fleet.FleetVarName{ - fleet.FleetVarHostEndUserIDPUsername, - fleet.FleetVarHostEndUserIDPUsernameLocalPart, - fleet.FleetVarHostEndUserIDPDepartment, - fleet.FleetVarHostEndUserIDPFullname, - }) + vars := []fleet.FleetVarName{ + fleet.FleetVarHostEndUserIDPUsername, + fleet.FleetVarHostEndUserIDPUsernameLocalPart, + fleet.FleetVarHostEndUserIDPDepartment, + fleet.FleetVarHostEndUserIDPFullname, + } + resentCerts, err := selectCertTemplatesToResend(ctx, tx, hostIDs, fleetVarNamesToDBVars(vars)) + if err != nil { + return nil, err + } + if err := triggerResendProfilesUsingVariables(ctx, tx, hostIDs, vars); err != nil { + return nil, err + } + return resentCerts, nil } -func triggerResendProfilesForIDPUserDeleted(ctx context.Context, tx sqlx.ExtContext, deletedScimUserID uint) error { +func triggerResendProfilesForIDPUserDeleted(ctx context.Context, tx sqlx.ExtContext, deletedScimUserID uint) ([]fleet.ActivityTypeResentCertificate, error) { hostIDs, err := getHostIDsHavingScimIDPUser(ctx, tx, deletedScimUserID) if err != nil { - return err + return nil, err } - return triggerResendProfilesUsingVariables(ctx, tx, hostIDs, - []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, tx, hostIDs, fleetVarNamesToDBVars(vars)) + if err != nil { + return nil, err + } + if err := triggerResendProfilesUsingVariables(ctx, tx, hostIDs, vars); err != nil { + return nil, err + } + return resentCerts, nil } func triggerResendProfilesForIDPGroupChange(ctx context.Context, tx sqlx.ExtContext, updatedScimGroupID uint) error { @@ -1258,6 +1287,87 @@ func triggerResendProfilesForIDPUserAddedToHost(ctx context.Context, tx sqlx.Ext }) } +func selectCertTemplatesToResend(ctx context.Context, tx sqlx.ExtContext, hostIDs []uint, vars []any) ([]fleet.ActivityTypeResentCertificate, error) { + if len(hostIDs) == 0 || len(vars) == 0 { + return nil, nil + } + + const query = ` + SELECT DISTINCT + h.id AS host_id, + COALESCE(h.computer_name, '') AS computer_name, + COALESCE(h.hostname, '') AS hostname, + COALESCE(h.hardware_model, '') AS hardware_model, + COALESCE(h.hardware_serial, '') AS hardware_serial, + ct.id AS certificate_template_id, + ct.name AS certificate_name + FROM + host_certificate_templates hct + JOIN hosts h + ON h.uuid = hct.host_uuid + JOIN certificate_templates ct + ON ct.id = hct.certificate_template_id AND + ct.team_id = COALESCE(h.team_id, 0) + JOIN mdm_configuration_profile_variables mcpv + ON mcpv.certificate_template_id = ct.id + JOIN fleet_variables fv + ON mcpv.fleet_variable_id = fv.id + WHERE + h.id IN (:host_ids) AND + hct.operation_type = :operation_type_install AND + hct.status IS NOT NULL AND + fv.name IN (:affected_vars) +` + + namedParams := map[string]any{ + "host_ids": hostIDs, + "operation_type_install": fleet.MDMOperationTypeInstall, + "affected_vars": vars, + } + + stmt, args, err := sqlx.Named(query, namedParams) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "prepare select cert templates to resend names") + } + stmt, args, err = sqlx.In(stmt, args...) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "prepare select cert templates to resend arguments") + } + + type row struct { + HostID uint `db:"host_id"` + ComputerName string `db:"computer_name"` + Hostname string `db:"hostname"` + HardwareModel string `db:"hardware_model"` + HardwareSerial string `db:"hardware_serial"` + CertificateTemplateID uint `db:"certificate_template_id"` + CertificateName string `db:"certificate_name"` + } + var rows []row + if err := sqlx.SelectContext(ctx, tx, &rows, stmt, args...); err != nil { + return nil, ctxerr.Wrap(ctx, err, "select cert templates to resend") + } + + activities := make([]fleet.ActivityTypeResentCertificate, 0, len(rows)) + for _, r := range rows { + activities = append(activities, fleet.ActivityTypeResentCertificate{ + HostID: r.HostID, + HostDisplayName: fleet.HostDisplayName(r.ComputerName, r.Hostname, r.HardwareModel, r.HardwareSerial), + CertificateTemplateID: r.CertificateTemplateID, + CertificateName: r.CertificateName, + }) + } + return activities, nil +} + +func fleetVarNamesToDBVars(vars []fleet.FleetVarName) []any { + result := make([]any, len(vars)) + for i, v := range vars { + result[i] = "FLEET_VAR_" + string(v) + } + return result +} + func triggerResendProfilesUsingVariables(ctx context.Context, tx sqlx.ExtContext, hostIDs []uint, affectedVars []fleet.FleetVarName) error { if len(hostIDs) == 0 || len(affectedVars) == 0 { return nil diff --git a/server/datastore/mysql/scim_test.go b/server/datastore/mysql/scim_test.go index e15b074b4c3..2021fcbdd63 100644 --- a/server/datastore/mysql/scim_test.go +++ b/server/datastore/mysql/scim_test.go @@ -386,7 +386,7 @@ func testReplaceScimUser(t *testing.T, ds *Datastore) { } // Replace the user - err = ds.ReplaceScimUser(t.Context(), &updatedUser) + _, err = ds.ReplaceScimUser(t.Context(), &updatedUser) require.Nil(t, err) // Verify the user was updated correctly @@ -433,7 +433,7 @@ func testReplaceScimUser(t *testing.T, ds *Datastore) { Active: ptr.Bool(true), } - err = ds.ReplaceScimUser(t.Context(), &nonExistentUser) + _, err = ds.ReplaceScimUser(t.Context(), &nonExistentUser) assert.True(t, fleet.IsNotFound(err)) } @@ -475,7 +475,7 @@ func testReplaceScimUserEmails(t *testing.T, ds *Datastore) { } // Replace the user - err = ds.ReplaceScimUser(t.Context(), &sameEmailsUser) + _, err = ds.ReplaceScimUser(t.Context(), &sameEmailsUser) require.NoError(t, err) // Verify the user was updated correctly but emails remain the same @@ -518,7 +518,7 @@ func testReplaceScimUserEmails(t *testing.T, ds *Datastore) { } // This should fail with a validation error - err = ds.ReplaceScimUser(t.Context(), &multiPrimaryUser) + _, err = ds.ReplaceScimUser(t.Context(), &multiPrimaryUser) assert.Error(t, err) assert.Contains(t, err.Error(), "only one email can be marked as primary") @@ -540,7 +540,7 @@ func testReplaceScimUserEmails(t *testing.T, ds *Datastore) { }, } - err = ds.ReplaceScimUser(t.Context(), &userWithAllFields) + _, err = ds.ReplaceScimUser(t.Context(), &userWithAllFields) require.NoError(t, err) // Now create a user with the same email but with nil Primary field @@ -561,7 +561,7 @@ func testReplaceScimUserEmails(t *testing.T, ds *Datastore) { } // This should update the emails since the Primary field changed - err = ds.ReplaceScimUser(t.Context(), &userWithNilPrimary) + _, err = ds.ReplaceScimUser(t.Context(), &userWithNilPrimary) require.NoError(t, err) // Verify the email was updated @@ -590,7 +590,7 @@ func testReplaceScimUserEmails(t *testing.T, ds *Datastore) { } // This should update the emails since the Type field changed - err = ds.ReplaceScimUser(t.Context(), &userWithNilType) + _, err = ds.ReplaceScimUser(t.Context(), &userWithNilType) require.NoError(t, err) // Verify the email was updated @@ -629,7 +629,7 @@ func testDeleteScimUser(t *testing.T, ds *Datastore) { assert.Equal(t, user.UserName, createdUser.UserName) // Delete the user - err = ds.DeleteScimUser(t.Context(), user.ID) + _, err = ds.DeleteScimUser(t.Context(), user.ID) require.NoError(t, err) // Verify the user was deleted @@ -637,7 +637,7 @@ func testDeleteScimUser(t *testing.T, ds *Datastore) { assert.True(t, fleet.IsNotFound(err)) // Test deleting a non-existent user - err = ds.DeleteScimUser(t.Context(), 99999) // Non-existent ID + _, err = ds.DeleteScimUser(t.Context(), 99999) // Non-existent ID assert.True(t, fleet.IsNotFound(err)) } @@ -1652,7 +1652,7 @@ func testScimUserReplaceValidation(t *testing.T, ds *Datastore) { Active: ptr.Bool(true), Department: ptr.String("Customer support"), } - err = ds.ReplaceScimUser(t.Context(), &userWithLongExternalID) + _, err = ds.ReplaceScimUser(t.Context(), &userWithLongExternalID) assert.Error(t, err) assert.Contains(t, err.Error(), "external_id exceeds maximum length") @@ -1666,7 +1666,7 @@ func testScimUserReplaceValidation(t *testing.T, ds *Datastore) { Active: ptr.Bool(true), Department: ptr.String("Customer support"), } - err = ds.ReplaceScimUser(t.Context(), &userWithLongUserName) + _, err = ds.ReplaceScimUser(t.Context(), &userWithLongUserName) assert.Error(t, err) assert.Contains(t, err.Error(), "user_name exceeds maximum length") @@ -1680,7 +1680,7 @@ func testScimUserReplaceValidation(t *testing.T, ds *Datastore) { Active: ptr.Bool(true), Department: ptr.String("Customer support"), } - err = ds.ReplaceScimUser(t.Context(), &userWithLongGivenName) + _, err = ds.ReplaceScimUser(t.Context(), &userWithLongGivenName) assert.Error(t, err) assert.Contains(t, err.Error(), "given_name exceeds maximum length") @@ -1694,7 +1694,7 @@ func testScimUserReplaceValidation(t *testing.T, ds *Datastore) { Active: ptr.Bool(true), Department: ptr.String("Customer support"), } - err = ds.ReplaceScimUser(t.Context(), &userWithLongFamilyName) + _, err = ds.ReplaceScimUser(t.Context(), &userWithLongFamilyName) assert.Error(t, err) assert.Contains(t, err.Error(), "family_name exceeds maximum length") @@ -1708,7 +1708,7 @@ func testScimUserReplaceValidation(t *testing.T, ds *Datastore) { Active: ptr.Bool(true), Department: ptr.String(longString), } - err = ds.ReplaceScimUser(t.Context(), &userWithLongDepartment) + _, err = ds.ReplaceScimUser(t.Context(), &userWithLongDepartment) assert.Error(t, err) assert.Contains(t, err.Error(), "department exceeds maximum length") @@ -1722,7 +1722,7 @@ func testScimUserReplaceValidation(t *testing.T, ds *Datastore) { Active: ptr.Bool(true), Department: ptr.String("Customer support updated"), } - err = ds.ReplaceScimUser(t.Context(), &validUser) + _, err = ds.ReplaceScimUser(t.Context(), &validUser) assert.NoError(t, err) updated, err := ds.ScimUserByID(t.Context(), user.ID) @@ -1924,7 +1924,7 @@ func testTriggerResendIdPProfiles(t *testing.T, ds *Datastore) { forceSetWindowsHostProfileStatus(t, ds, hostW3.UUID, profWAll, fleet.MDMOperationTypeInstall, fleet.MDMDeliveryVerifying) // change username of scim user 1 - err = ds.ReplaceScimUser(ctx, &fleet.ScimUser{ID: scimUser1, UserName: "A@example.com"}) + _, err = ds.ReplaceScimUser(ctx, &fleet.ScimUser{ID: scimUser1, UserName: "A@example.com"}) require.NoError(t, err) // this triggered a resend of profUsername and profAll on host1 and hostW1 @@ -2015,7 +2015,7 @@ func testTriggerResendIdPProfiles(t *testing.T, ds *Datastore) { // user1, does not trigger anything group2, err := ds.CreateScimGroup(ctx, &fleet.ScimGroup{DisplayName: "g2"}) require.NoError(t, err) - err = ds.ReplaceScimUser(ctx, &fleet.ScimUser{ID: scimUser1, UserName: "A@example.com", ExternalID: ptr.String("A")}) + _, err = ds.ReplaceScimUser(ctx, &fleet.ScimUser{ID: scimUser1, UserName: "A@example.com", ExternalID: new("A")}) require.NoError(t, err) assertHostProfileStatus(t, ds, host1.UUID, @@ -2223,7 +2223,7 @@ func testTriggerResendIdPProfiles(t *testing.T, ds *Datastore) { hostProfileStatus{profWAll.ProfileUUID, fleet.MDMDeliveryVerifying}) // delete user3, affects only host3 (not the official IdP user for host1) - err = ds.DeleteScimUser(ctx, scimUser3) + _, err = ds.DeleteScimUser(ctx, scimUser3) require.NoError(t, err) assertHostProfileStatus(t, ds, host1.UUID, @@ -2266,7 +2266,7 @@ func testTriggerResendIdPProfiles(t *testing.T, ds *Datastore) { forceSetWindowsHostProfileStatus(t, ds, hostW3.UUID, profWAll, fleet.MDMOperationTypeInstall, fleet.MDMDeliveryVerifying) // delete user1 - err = ds.DeleteScimUser(ctx, scimUser1) + _, err = ds.DeleteScimUser(ctx, scimUser1) require.NoError(t, err) // add user2 as new user for host1 err = ds.associateHostWithScimUser(ctx, host1.ID, scimUser2) @@ -2315,7 +2315,7 @@ func testTriggerResendIdPProfiles(t *testing.T, ds *Datastore) { // update name of user2, will affect host1 and host2, but NOT the // profUsername of host1 because it is not installed (it is removed) - err = ds.ReplaceScimUser(ctx, &fleet.ScimUser{ID: scimUser2, UserName: "B@example.com", GivenName: ptr.String("B")}) + _, err = ds.ReplaceScimUser(ctx, &fleet.ScimUser{ID: scimUser2, UserName: "B@example.com", GivenName: new("B")}) require.NoError(t, err) assertHostProfileStatus(t, ds, host1.UUID, @@ -2819,9 +2819,16 @@ func testTriggerResendCertTemplatesAndAppConfigs(t *testing.T, ds *Datastore) { }) // Change the SCIM user's username — this should trigger resends. - err = ds.ReplaceScimUser(ctx, &fleet.ScimUser{ID: scimUser, UserName: "new-user@example.com"}) + activities, err := ds.ReplaceScimUser(ctx, &fleet.ScimUser{ID: scimUser, UserName: "new-user@example.com"}) require.NoError(t, err) + // Assert that resent_certificate activities were returned. + require.Len(t, activities, 1) + assert.Equal(t, host.ID, activities[0].HostID) + assert.Equal(t, certResp.ID, activities[0].CertificateTemplateID) + assert.Equal(t, "wifi-cert", activities[0].CertificateName) + assert.NotEmpty(t, activities[0].HostDisplayName) + // (1) Assert certificate template was reset to pending. var certStatus string ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { diff --git a/server/fleet/datastore.go b/server/fleet/datastore.go index 138acf1566f..0aa15ea7957 100644 --- a/server/fleet/datastore.go +++ b/server/fleet/datastore.go @@ -3259,9 +3259,9 @@ type Datastore interface { // If the slice is empty, it returns true ScimUsersExist(ctx context.Context, ids []uint) (bool, error) // ReplaceScimUser replaces an existing SCIM user in the database - ReplaceScimUser(ctx context.Context, user *ScimUser) error + ReplaceScimUser(ctx context.Context, user *ScimUser) ([]ActivityTypeResentCertificate, error) // DeleteScimUser deletes a SCIM user from the database - DeleteScimUser(ctx context.Context, id uint) error + DeleteScimUser(ctx context.Context, id uint) ([]ActivityTypeResentCertificate, error) // ListScimUsers retrieves a list of SCIM users with optional filtering ListScimUsers(ctx context.Context, opts ScimUsersListOptions) (users []ScimUser, totalResults uint, err error) // CreateScimGroup creates a new SCIM group in the database diff --git a/server/mock/datastore_mock.go b/server/mock/datastore_mock.go index f17b61cb868..610adb04a13 100644 --- a/server/mock/datastore_mock.go +++ b/server/mock/datastore_mock.go @@ -1972,9 +1972,9 @@ type ScimUserByHostIDFunc func(ctx context.Context, hostID uint) (*fleet.ScimUse type ScimUsersExistFunc func(ctx context.Context, ids []uint) (bool, error) -type ReplaceScimUserFunc func(ctx context.Context, user *fleet.ScimUser) error +type ReplaceScimUserFunc func(ctx context.Context, user *fleet.ScimUser) ([]fleet.ActivityTypeResentCertificate, error) -type DeleteScimUserFunc func(ctx context.Context, id uint) error +type DeleteScimUserFunc func(ctx context.Context, id uint) ([]fleet.ActivityTypeResentCertificate, error) type ListScimUsersFunc func(ctx context.Context, opts fleet.ScimUsersListOptions) (users []fleet.ScimUser, totalResults uint, err error) @@ -12146,14 +12146,14 @@ func (s *DataStore) ScimUsersExist(ctx context.Context, ids []uint) (bool, error return s.ScimUsersExistFunc(ctx, ids) } -func (s *DataStore) ReplaceScimUser(ctx context.Context, user *fleet.ScimUser) error { +func (s *DataStore) ReplaceScimUser(ctx context.Context, user *fleet.ScimUser) ([]fleet.ActivityTypeResentCertificate, error) { s.mu.Lock() s.ReplaceScimUserFuncInvoked = true s.mu.Unlock() return s.ReplaceScimUserFunc(ctx, user) } -func (s *DataStore) DeleteScimUser(ctx context.Context, id uint) error { +func (s *DataStore) DeleteScimUser(ctx context.Context, id uint) ([]fleet.ActivityTypeResentCertificate, error) { s.mu.Lock() s.DeleteScimUserFuncInvoked = true s.mu.Unlock() diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 3900e8fbc05..4194cff6ace 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -25672,7 +25672,7 @@ func (s *integrationEnterpriseTestSuite) TestHostDeviceMappingIDP() { createdUserID, err := s.ds.CreateScimUser(ctx, scimUser) require.NoError(t, err) - defer func() { _ = s.ds.DeleteScimUser(ctx, createdUserID) }() + defer func() { _, _ = s.ds.DeleteScimUser(ctx, createdUserID) }() // Test IDP device mapping with premium license and valid SCIM user var putResp putHostDeviceMappingResponse diff --git a/server/service/integration_mdm_profiles_test.go b/server/service/integration_mdm_profiles_test.go index c7f62cc1f36..b6304cef641 100644 --- a/server/service/integration_mdm_profiles_test.go +++ b/server/service/integration_mdm_profiles_test.go @@ -8682,7 +8682,7 @@ func (s *integrationMDMTestSuite) TestAppleProfileResendRaceCondition() { // we trigger a resend before the acknowledgement comes back // 1. Trigger an IDP variable change by updating SCIM user - err = s.ds.ReplaceScimUser(ctx, &fleet.ScimUser{ID: scimUserID, UserName: "newuser@example.com"}) + _, err = s.ds.ReplaceScimUser(ctx, &fleet.ScimUser{ID: scimUserID, UserName: "newuser@example.com"}) require.NoError(t, err) // 2. At this point, the profile should be marked for resend (status = NULL)