From dcf81c457542a13ecf6a4b4a81dc05a1eecde56b Mon Sep 17 00:00:00 2001 From: Bryan Frimin Date: Tue, 24 Mar 2026 22:03:52 +0100 Subject: [PATCH] Fix SCIM bridge updating all users on every sync The SCIM client User struct had json:"-" tags on most fields (GivenName, FamilyName, ExternalID, Department, etc.), so ListUsers never populated them from the JSON response. The bridge comparison always saw empty strings on the SCIM side vs actual values from the provider, making needsUpdate true for every user on every sync cycle. Add custom UnmarshalJSON on User to properly parse nested SCIM JSON (name object, enterprise extension) into the flat struct, so the existing diff logic correctly skips unchanged users. Signed-off-by: Bryan Frimin --- pkg/iam/scim/bridge/client/client.go | 50 ++++++++ pkg/iam/scim/bridge/client/client_test.go | 138 ++++++++++++++++++++++ 2 files changed, 188 insertions(+) create mode 100644 pkg/iam/scim/bridge/client/client_test.go diff --git a/pkg/iam/scim/bridge/client/client.go b/pkg/iam/scim/bridge/client/client.go index d727b6826..23ec8cfb5 100644 --- a/pkg/iam/scim/bridge/client/client.go +++ b/pkg/iam/scim/bridge/client/client.go @@ -309,6 +309,56 @@ func (c *Client) DeleteUser(ctx context.Context, userID string) error { return nil } +func (u *User) UnmarshalJSON(data []byte) error { + var raw struct { + ID string `json:"id"` + UserName string `json:"userName"` + DisplayName string `json:"displayName"` + Active bool `json:"active"` + Title string `json:"title"` + ExternalID string `json:"externalId"` + UserType string `json:"userType"` + PreferredLanguage string `json:"preferredLanguage"` + Name struct { + GivenName string `json:"givenName"` + FamilyName string `json:"familyName"` + } `json:"name"` + Enterprise struct { + EmployeeNumber string `json:"employeeNumber"` + Department string `json:"department"` + CostCenter string `json:"costCenter"` + Organization string `json:"organization"` + Division string `json:"division"` + Manager struct { + Value string `json:"value"` + } `json:"manager"` + } `json:"urn:ietf:params:scim:schemas:extension:enterprise:2.0:User"` + } + + if err := json.Unmarshal(data, &raw); err != nil { + return err + } + + u.ID = raw.ID + u.UserName = raw.UserName + u.DisplayName = raw.DisplayName + u.Active = raw.Active + u.Title = raw.Title + u.ExternalID = raw.ExternalID + u.UserType = raw.UserType + u.PreferredLanguage = raw.PreferredLanguage + u.GivenName = raw.Name.GivenName + u.FamilyName = raw.Name.FamilyName + u.EmployeeNumber = raw.Enterprise.EmployeeNumber + u.Department = raw.Enterprise.Department + u.CostCenter = raw.Enterprise.CostCenter + u.EnterpriseOrganization = raw.Enterprise.Organization + u.Division = raw.Enterprise.Division + u.ManagerValue = raw.Enterprise.Manager.Value + + return nil +} + func (c *Client) setHeaders(req *http.Request) { req.Header.Set("Authorization", "Bearer "+c.token) req.Header.Set("Accept", "application/scim+json") diff --git a/pkg/iam/scim/bridge/client/client_test.go b/pkg/iam/scim/bridge/client/client_test.go new file mode 100644 index 000000000..3581a319f --- /dev/null +++ b/pkg/iam/scim/bridge/client/client_test.go @@ -0,0 +1,138 @@ +// Copyright (c) 2025 Probo Inc . +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR +// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +package scimclient_test + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + scimclient "go.probo.inc/probo/pkg/iam/scim/bridge/client" +) + +func TestUser_UnmarshalJSON(t *testing.T) { + t.Parallel() + + t.Run( + "all fields populated", + func(t *testing.T) { + t.Parallel() + + data := []byte(`{ + "id": "user-123", + "userName": "john@example.com", + "displayName": "John Doe", + "active": true, + "title": "Engineer", + "externalId": "ext-456", + "userType": "Employee", + "preferredLanguage": "en", + "name": { + "givenName": "John", + "familyName": "Doe" + }, + "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User": { + "employeeNumber": "EMP001", + "department": "Engineering", + "costCenter": "CC100", + "organization": "Acme Corp", + "division": "Platform", + "manager": { + "value": "jane@example.com" + } + } + }`) + + var user scimclient.User + err := json.Unmarshal(data, &user) + + require.NoError(t, err) + assert.Equal(t, "user-123", user.ID) + assert.Equal(t, "john@example.com", user.UserName) + assert.Equal(t, "John Doe", user.DisplayName) + assert.Equal(t, true, user.Active) + assert.Equal(t, "Engineer", user.Title) + assert.Equal(t, "ext-456", user.ExternalID) + assert.Equal(t, "Employee", user.UserType) + assert.Equal(t, "en", user.PreferredLanguage) + assert.Equal(t, "John", user.GivenName) + assert.Equal(t, "Doe", user.FamilyName) + assert.Equal(t, "EMP001", user.EmployeeNumber) + assert.Equal(t, "Engineering", user.Department) + assert.Equal(t, "CC100", user.CostCenter) + assert.Equal(t, "Acme Corp", user.EnterpriseOrganization) + assert.Equal(t, "Platform", user.Division) + assert.Equal(t, "jane@example.com", user.ManagerValue) + }, + ) + + t.Run( + "no enterprise extension", + func(t *testing.T) { + t.Parallel() + + data := []byte(`{ + "id": "user-789", + "userName": "alice@example.com", + "displayName": "Alice Smith", + "active": false, + "name": { + "givenName": "Alice", + "familyName": "Smith" + } + }`) + + var user scimclient.User + err := json.Unmarshal(data, &user) + + require.NoError(t, err) + assert.Equal(t, "user-789", user.ID) + assert.Equal(t, "alice@example.com", user.UserName) + assert.Equal(t, "Alice Smith", user.DisplayName) + assert.Equal(t, false, user.Active) + assert.Equal(t, "Alice", user.GivenName) + assert.Equal(t, "Smith", user.FamilyName) + assert.Equal(t, "", user.EmployeeNumber) + assert.Equal(t, "", user.Department) + assert.Equal(t, "", user.ManagerValue) + }, + ) + + t.Run( + "minimal fields", + func(t *testing.T) { + t.Parallel() + + data := []byte(`{ + "id": "user-min", + "userName": "bob@example.com", + "active": true + }`) + + var user scimclient.User + err := json.Unmarshal(data, &user) + + require.NoError(t, err) + assert.Equal(t, "user-min", user.ID) + assert.Equal(t, "bob@example.com", user.UserName) + assert.Equal(t, true, user.Active) + assert.Equal(t, "", user.DisplayName) + assert.Equal(t, "", user.GivenName) + assert.Equal(t, "", user.FamilyName) + assert.Equal(t, "", user.Title) + }, + ) +}