From a622c610d7e4af2853fa1d4703bbd002191ad3ff Mon Sep 17 00:00:00 2001 From: Sacha Al Himdani Date: Tue, 21 Apr 2026 15:46:09 +0200 Subject: [PATCH] Fix SCIM bridge PUT loop and pagination MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs caused SCIM sync failures: 1. buildUserPayload conditionally omitted empty fields. When a field was cleared in the identity provider, the PUT payload didn't include it, so the SCIM handler never cleared the stored value. The bridge kept detecting a mismatch every sync cycle, causing a perpetual PUT loop. Fix: always include all fields unconditionally. 2. ListUsers ignored the startIndex parameter — the cursor always started from nil, so every page returned the same first N users. Organizations with more than 100 SCIM-managed users never got a full listing; users beyond the first page appeared missing, causing CreateUser calls that failed with 409 (uniqueness conflict) and eventually disabled the bridge. Fix: replace cursor-based pagination with OFFSET/LIMIT to honor SCIM's 1-based startIndex. Signed-off-by: Sacha Al Himdani --- pkg/coredata/membership_profile.go | 112 +++++++++++++++++++++++++++ pkg/iam/scim/bridge/client/client.go | 56 +++++--------- pkg/iam/scim/service.go | 19 +---- 3 files changed, 132 insertions(+), 55 deletions(-) diff --git a/pkg/coredata/membership_profile.go b/pkg/coredata/membership_profile.go index 972f49bf6..a0ad61bf2 100644 --- a/pkg/coredata/membership_profile.go +++ b/pkg/coredata/membership_profile.go @@ -447,6 +447,118 @@ WHERE return nil } +func (p *MembershipProfiles) LoadAllByOrganizationID( + ctx context.Context, + conn pg.Querier, + scope Scoper, + organizationID gid.GID, + filter *MembershipProfileFilter, +) error { + q := ` +WITH profiles AS ( + SELECT + p.id, + p.identity_id, + p.organization_id, + i.email_address, + p.source, + p.state, + p.full_name, + p.kind, + p.additional_email_addresses, + p.position, + p.contract_start_date, + p.contract_end_date, + p.user_name, + p.external_id, + p.nickname, + p.locale, + p.timezone, + p.profile_url, + p.preferred_language, + p.given_name, + p.family_name, + p.formatted_name, + p.middle_name, + p.honorific_prefix, + p.honorific_suffix, + p.employee_number, + p.department, + p.cost_center, + p.enterprise_organization, + p.division, + p.manager_value, + p.created_at, + p.updated_at + FROM + iam_membership_profiles p + INNER JOIN identities i ON i.id = p.identity_id + WHERE + p.%s + AND p.organization_id = @organization_id + AND %s +) +SELECT + id, + identity_id, + organization_id, + email_address, + source, + state, + full_name, + kind, + additional_email_addresses, + position, + contract_start_date, + contract_end_date, + '' AS organization_name, + user_name, + external_id, + nickname, + locale, + timezone, + profile_url, + preferred_language, + given_name, + family_name, + formatted_name, + middle_name, + honorific_prefix, + honorific_suffix, + employee_number, + department, + cost_center, + enterprise_organization, + division, + manager_value, + created_at, + updated_at +FROM profiles +` + + q = fmt.Sprintf(q, scope.SQLFragment(), filter.SQLFragment()) + + args := pgx.NamedArgs{ + "organization_id": organizationID, + } + maps.Copy(args, scope.SQLArguments()) + maps.Copy(args, filter.SQLArguments()) + + rows, err := conn.Query(ctx, q, args) + if err != nil { + return fmt.Errorf("cannot query profiles: %w", err) + } + + profiles, err := pgx.CollectRows(rows, pgx.RowToAddrOfStructByName[MembershipProfile]) + if err != nil { + return fmt.Errorf("cannot collect profiles: %w", err) + } + + *p = profiles + + return nil +} + func (p *MembershipProfiles) LoadByIdentityID( ctx context.Context, conn pg.Querier, diff --git a/pkg/iam/scim/bridge/client/client.go b/pkg/iam/scim/bridge/client/client.go index 8d696f1b1..4d58bd388 100644 --- a/pkg/iam/scim/bridge/client/client.go +++ b/pkg/iam/scim/bridge/client/client.go @@ -185,26 +185,18 @@ func (c *Client) UpdateUser(ctx context.Context, userID string, user *User) erro } func buildUserPayload(user *User) map[string]any { - schemas := []string{"urn:ietf:params:scim:schemas:core:2.0:User"} + schemas := []string{ + "urn:ietf:params:scim:schemas:core:2.0:User", + "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User", + } - enterprise := map[string]any{} - if user.EmployeeNumber != "" { - enterprise["employeeNumber"] = user.EmployeeNumber - } - if user.Department != "" { - enterprise["department"] = user.Department - } - if user.CostCenter != "" { - enterprise["costCenter"] = user.CostCenter - } - if user.EnterpriseOrganization != "" { - enterprise["organization"] = user.EnterpriseOrganization - } - if user.Division != "" { - enterprise["division"] = user.Division - } - if user.ManagerValue != "" { - enterprise["manager"] = map[string]string{"value": user.ManagerValue} + enterprise := map[string]any{ + "employeeNumber": user.EmployeeNumber, + "department": user.Department, + "costCenter": user.CostCenter, + "organization": user.EnterpriseOrganization, + "division": user.Division, + "manager": map[string]string{"value": user.ManagerValue}, } payload := map[string]any{ @@ -215,8 +207,12 @@ func buildUserPayload(user *User) map[string]any { "familyName": user.FamilyName, "formatted": user.DisplayName, }, - "displayName": user.DisplayName, - "active": user.Active, + "displayName": user.DisplayName, + "active": user.Active, + "title": user.Title, + "externalId": user.ExternalID, + "userType": user.UserType, + "preferredLanguage": user.PreferredLanguage, "emails": []map[string]any{ { "value": user.UserName, @@ -224,23 +220,7 @@ func buildUserPayload(user *User) map[string]any { "primary": true, }, }, - "title": user.Title, - } - - if user.ExternalID != "" { - payload["externalId"] = user.ExternalID - } - if user.UserType != "" { - payload["userType"] = user.UserType - } - if user.PreferredLanguage != "" { - payload["preferredLanguage"] = user.PreferredLanguage - } - - if len(enterprise) > 0 { - schemas = append(schemas, "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User") - payload["schemas"] = schemas - payload["urn:ietf:params:scim:schemas:extension:enterprise:2.0:User"] = enterprise + "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User": enterprise, } return payload diff --git a/pkg/iam/scim/service.go b/pkg/iam/scim/service.go index aadac0530..45bd7f959 100644 --- a/pkg/iam/scim/service.go +++ b/pkg/iam/scim/service.go @@ -38,7 +38,6 @@ import ( "go.probo.inc/probo/pkg/crypto/rand" "go.probo.inc/probo/pkg/gid" "go.probo.inc/probo/pkg/mail" - "go.probo.inc/probo/pkg/page" "go.probo.inc/probo/pkg/webhook" webhooktypes "go.probo.inc/probo/pkg/webhook/types" ) @@ -395,25 +394,11 @@ func (s *Service) ListUsers( scope := coredata.NewScopeFromObjectID(config.OrganizationID) var profiles coredata.MembershipProfiles - var totalCount int err = s.pg.WithConn( ctx, func(ctx context.Context, conn pg.Querier) error { - var err error - totalCount, err = profiles.CountByOrganizationID(ctx, conn, scope, config.OrganizationID, filter) - if err != nil { - return fmt.Errorf("cannot count profiles: %w", err) - } - - orderBy := page.OrderBy[coredata.MembershipProfileOrderField]{ - Field: coredata.MembershipProfileOrderFieldCreatedAt, - Direction: page.OrderDirectionDesc, - } - cursor := page.NewCursor(count, nil, page.Head, orderBy) - - err = profiles.LoadByOrganizationID(ctx, conn, scope, config.OrganizationID, cursor, filter) - if err != nil { + if err := profiles.LoadAllByOrganizationID(ctx, conn, scope, config.OrganizationID, filter); err != nil { return fmt.Errorf("cannot load profiles: %w", err) } @@ -430,7 +415,7 @@ func (s *Service) ListUsers( resources = append(resources, userToResource(p)) } - return resources, totalCount, nil + return resources, len(resources), nil } func (s *Service) ReplaceUser(