From db570c8f302c428ed7123b61a0f15663efc21659 Mon Sep 17 00:00:00 2001 From: Jay Hemnani <193022578+jayhemnani9910@users.noreply.github.com> Date: Sat, 10 Oct 2026 20:01:27 +0530 Subject: [PATCH] fix: push group removals made on the user page to SCIM (#1830) --- backend/internal/service/user_service.go | 15 ++++---- backend/internal/service/user_service_test.go | 35 +++++++++++++++++++ 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/backend/internal/service/user_service.go b/backend/internal/service/user_service.go index 7a173679..bc0304ab 100644 --- a/backend/internal/service/user_service.go +++ b/backend/internal/service/user_service.go @@ -594,6 +594,9 @@ func (s *UserService) UpdateUserGroups(ctx context.Context, id string, userGroup return !slices.Contains(userGroupIds, group.ID) }) + // Remember the current groups, because the groups the user leaves change their membership too + previousGroupIDs := userGroupIDs(user.UserGroups) + // Fetch the groups based on userGroupIds var groups []model.UserGroup if len(userGroupIds) > 0 { @@ -623,14 +626,10 @@ func (s *UserService) UpdateUserGroups(ctx context.Context, id string, userGroup return model.User{}, err } - // Update the UpdatedAt field for all affected groups - now := datatype.DateTime(time.Now()) - for _, group := range groups { - group.UpdatedAt = &now - err = tx.WithContext(ctx).Save(&group).Error - if err != nil { - return model.User{}, err - } + // Bump the UpdatedAt of every group the user joined or left, so the SCIM sync pushes the new membership + err = s.touchUserGroups(ctx, tx, append(previousGroupIDs, userGroupIDs(groups)...)) + if err != nil { + return model.User{}, err } err = tx.Commit().Error diff --git a/backend/internal/service/user_service_test.go b/backend/internal/service/user_service_test.go index c6523e71..87941a98 100644 --- a/backend/internal/service/user_service_test.go +++ b/backend/internal/service/user_service_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "strings" "testing" + "time" "uuid" "github.com/stretchr/testify/require" @@ -11,6 +12,8 @@ import ( "github.com/pocket-id/pocket-id/backend/internal/appconfig" "github.com/pocket-id/pocket-id/backend/internal/apperror" "github.com/pocket-id/pocket-id/backend/internal/dto" + "github.com/pocket-id/pocket-id/backend/internal/model" + datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" "github.com/pocket-id/pocket-id/backend/internal/storage" testutils "github.com/pocket-id/pocket-id/backend/internal/utils/testing" ) @@ -141,3 +144,35 @@ func TestCreateUserBumpsDefaultGroupUpdatedAt(t *testing.T) { require.NotNil(t, updated.UpdatedAt, "adding a default group member must bump the group's UpdatedAt") require.Len(t, updated.Users, 1) } + +func TestUpdateUserGroupsBumpsRemovedGroupUpdatedAt(t *testing.T) { + config := &appconfig.AppConfigModel{RequireUserEmail: "false"} + userService, groupService := newTestUserService(t) + + oldGroup, err := groupService.Create(t.Context(), dto.UserGroupCreateDto{Name: "old", FriendlyName: "Old"}) + require.NoError(t, err) + newGroup, err := groupService.Create(t.Context(), dto.UserGroupCreateDto{Name: "new", FriendlyName: "New"}) + require.NoError(t, err) + + user, err := userService.CreateUser(t.Context(), config, dto.UserCreateDto{ + Username: "mover", + FirstName: "Group", + LastName: "Mover", + UserGroupIds: []string{oldGroup.ID}, + }) + require.NoError(t, err) + + // Backdate both groups so a bump is visible regardless of the timestamp precision + past := time.Now().Add(-time.Hour) + require.NoError(t, userService.db.Model(&model.UserGroup{}).Where("id IN ?", []string{oldGroup.ID, newGroup.ID}).Update("updated_at", datatype.DateTime(past)).Error) + + // Move the user from the old group to the new one, as the user's group selection does + _, err = userService.UpdateUserGroups(t.Context(), user.ID, []string{newGroup.ID}) + require.NoError(t, err) + + for _, groupID := range []string{oldGroup.ID, newGroup.ID} { + updated, err := groupService.Get(t.Context(), groupID) + require.NoError(t, err) + require.True(t, updated.LastModified().After(past), "group %s changed membership, so its UpdatedAt must be bumped", updated.Name) + } +}