fix: sync group membership for users added at creation time (#1600)

This commit is contained in:
Alessandro (Ale) Segala
2026-07-17 11:15:00 -07:00
committed by GitHub
parent 31c416ab2c
commit b254d969a5
2 changed files with 149 additions and 2 deletions
+45 -2
View File
@@ -301,15 +301,26 @@ func (s *UserService) CreateUserInternal(ctx context.Context, input dto.UserCrea
return model.User{}, err
}
// Bump the UpdatedAt timestamp of the groups the new user was added to
// This is necessary for SCIM to work with the newly-created user, or groups may not be synced via SCIM
if len(userGroups) > 0 {
err = s.touchUserGroups(ctx, tx, groupIDs(userGroups))
if err != nil {
return model.User{}, err
}
}
// Apply default groups and claims for new non-LDAP users
if !isLdapSync {
if len(input.UserGroupIds) == 0 {
if err := s.applyDefaultGroups(ctx, &user, tx); err != nil {
err = s.applyDefaultGroups(ctx, &user, tx)
if err != nil {
return model.User{}, err
}
}
if err := s.applyDefaultCustomClaims(ctx, &user, tx); err != nil {
err = s.applyDefaultCustomClaims(ctx, &user, tx)
if err != nil {
return model.User{}, err
}
}
@@ -348,11 +359,43 @@ func (s *UserService) applyDefaultGroups(ctx context.Context, user *model.User,
if err != nil {
return fmt.Errorf("failed to associate default user groups: %w", err)
}
// Bump the groups' UpdatedAt so the SCIM sync picks up the new
// membership (see touchUserGroups for details).
touchIDs := make([]string, len(groups))
for i := range groups {
touchIDs[i] = groups[i].ID
}
if err := s.touchUserGroups(ctx, tx, touchIDs); err != nil {
return fmt.Errorf("failed to update default user groups timestamp: %w", err)
}
}
}
return nil
}
// touchUserGroups updates the UpdatedAt timestamp of the given user groups.
//
// Group membership is stored in the user_groups_users join table, so adding or
// removing a member does not modify the group row itself. The SCIM sync only
// re-pushes a group to the provider when its UpdatedAt is not older than the
// remote resource's last-modified time, so any code path that changes a group's
// membership must bump this timestamp explicitly. Otherwise the membership
// change is never synced to the SCIM provider.
func (s *UserService) touchUserGroups(ctx context.Context, tx *gorm.DB, ids []string) error {
if len(ids) == 0 {
return nil
}
now := datatype.DateTime(time.Now())
return tx.
WithContext(ctx).
Model(&model.UserGroup{}).
Where("id IN ?", ids).
Update("updated_at", now).
Error
}
func (s *UserService) applyDefaultCustomClaims(ctx context.Context, user *model.User, tx *gorm.DB) error {
config := s.appConfigService.GetDbConfig()
@@ -0,0 +1,104 @@
package service
import (
"encoding/json"
"testing"
"github.com/stretchr/testify/require"
"github.com/pocket-id/pocket-id/backend/internal/dto"
"github.com/pocket-id/pocket-id/backend/internal/model"
"github.com/pocket-id/pocket-id/backend/internal/storage"
testutils "github.com/pocket-id/pocket-id/backend/internal/utils/testing"
)
func newTestUserService(t *testing.T, appConfig *AppConfigService) (*UserService, *UserGroupService) {
t.Helper()
db := testutils.NewDatabaseForTest(t)
fileStorage, err := storage.NewDatabaseStorage(db)
require.NoError(t, err)
userService := NewUserService(
db,
nil,
nil,
nil,
appConfig,
NewCustomClaimService(db),
NewAppImagesService(map[string]string{}, fileStorage),
nil,
fileStorage,
)
groupService := NewUserGroupService(db, appConfig, nil)
return userService, groupService
}
func TestCreateUserBumpsGroupUpdatedAt(t *testing.T) {
appConfig := NewTestAppConfigService(&model.AppConfig{
RequireUserEmail: model.AppConfigVariable{Value: "false"},
})
userService, groupService := newTestUserService(t, appConfig)
group, err := groupService.Create(t.Context(), dto.UserGroupCreateDto{
Name: "members",
FriendlyName: "Members",
})
require.NoError(t, err)
require.Nil(t, group.UpdatedAt, "a freshly created group has no UpdatedAt yet")
// Create a user that is a member of the group
// This mirrors signing up via an invite link that adds the user to a group
email := "member@example.com"
_, err = userService.CreateUser(t.Context(), dto.UserCreateDto{
Username: "member",
Email: &email,
FirstName: "Group",
LastName: "Member",
UserGroupIds: []string{group.ID},
})
require.NoError(t, err)
// The group's UpdatedAt must now be set
updated, err := groupService.Get(t.Context(), group.ID)
require.NoError(t, err)
require.NotNil(t, updated.UpdatedAt, "creating a group member must bump the group's UpdatedAt")
require.False(t, updated.LastModified().Before(updated.CreatedAt.ToTime()), "group LastModified must not predate its CreatedAt after a membership change")
require.Len(t, updated.Users, 1, "the user should be a member of the group")
}
func TestCreateUserBumpsDefaultGroupUpdatedAt(t *testing.T) {
appConfig := NewTestAppConfigService(&model.AppConfig{
RequireUserEmail: model.AppConfigVariable{Value: "false"},
})
userService, groupService := newTestUserService(t, appConfig)
group, err := groupService.Create(t.Context(), dto.UserGroupCreateDto{
Name: "default",
FriendlyName: "Default",
})
require.NoError(t, err)
require.Nil(t, group.UpdatedAt)
// Configure the group as a default signup group
defaultGroups, err := json.Marshal([]string{group.ID})
require.NoError(t, err)
appConfig.dbConfig.Load().SignupDefaultUserGroupIDs.Value = string(defaultGroups)
// Create a user without explicit group IDs, so the default groups apply
email := "default@example.com"
_, err = userService.CreateUser(t.Context(), dto.UserCreateDto{
Username: "defaultmember",
Email: &email,
FirstName: "Default",
LastName: "Member",
})
require.NoError(t, err)
updated, err := groupService.Get(t.Context(), group.ID)
require.NoError(t, err)
require.NotNil(t, updated.UpdatedAt, "adding a default group member must bump the group's UpdatedAt")
require.Len(t, updated.Users, 1)
}