Fix SCIM bridge PUT loop and pagination
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 <sacha@getprobo.com>
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user