Skip to content

Cherry-pick: Android certificates resent_certificate #49229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ksykulev merged 1 commit into from
Jul 13, 2026
Merged

Cherry-pick: Android certificates resent_certificate #49229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
31 changes: 27 additions & 4 deletions ee/server/scim/users.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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
}

Expand Down Expand Up @@ -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)
Expand All @@ -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,
Expand Down
60 changes: 30 additions & 30 deletions ee/server/scim/users_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions server/cron/google_workspace_cron.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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++
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions server/cron/google_workspace_cron_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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++
Expand Down
Loading
Loading