Handle SCIM user email rename via external ID fallback

When a user's email is renamed in the identity provider (e.g. Google
Workspace), the external ID stays the same but the email changes. The
SCIM CreateUser now falls back to external ID lookup when no profile is
found by identity, and reassociates the existing profile to the new
identity instead of failing with a 409 uniqueness error.

Also removes user emails from bridge sync error messages to avoid
logging PII, using external IDs instead.

Signed-off-by: Sacha Al Himdani <sacha@getprobo.com>
This commit is contained in:
Sacha Al Himdani
2026-04-22 17:54:10 +02:00
parent b5a8781816
commit e5e17d59ac
5 changed files with 462 additions and 71 deletions

275
e2e/console/scim_test.go Normal file
View File

@@ -0,0 +1,275 @@
// Copyright (c) 2026 Probo Inc <hello@getprobo.com>.
//
// 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 console_test
import (
"bytes"
"encoding/json"
"io"
"net/http"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.probo.inc/probo/e2e/internal/factory"
"go.probo.inc/probo/e2e/internal/testutil"
)
type scimClient struct {
t testing.TB
client *http.Client
token string
endpoint string
}
func newSCIMClient(t testing.TB, owner *testutil.Client) *scimClient {
t.Helper()
const query = `
mutation($input: CreateSCIMConfigurationInput!) {
createSCIMConfiguration(input: $input) {
scimConfiguration { id }
token
}
}
`
var result struct {
CreateSCIMConfiguration struct {
ScimConfiguration struct {
ID string `json:"id"`
} `json:"scimConfiguration"`
Token string `json:"token"`
} `json:"createSCIMConfiguration"`
}
err := owner.ExecuteConnect(query, map[string]any{
"input": map[string]any{
"organizationId": owner.GetOrganizationID().String(),
},
}, &result)
require.NoError(t, err, "GraphQL request failed")
require.NotEmpty(t, result.CreateSCIMConfiguration.Token)
return &scimClient{
t: t,
client: &http.Client{},
token: result.CreateSCIMConfiguration.Token,
endpoint: testutil.GetBaseURL() + "/api/connect/v1/scim/2.0",
}
}
func (sc *scimClient) createUser(userName, fullName, externalID string, active bool) (string, int) {
sc.t.Helper()
payload := map[string]any{
"schemas": []string{"urn:ietf:params:scim:schemas:core:2.0:User"},
"userName": userName,
"active": active,
"externalId": externalID,
"name": map[string]any{
"givenName": "Test",
"familyName": "User",
},
"displayName": fullName,
"emails": []map[string]any{
{"value": userName, "primary": true},
},
}
return sc.doRequest("POST", "/Users", payload)
}
func (sc *scimClient) listUsers() (string, int) {
sc.t.Helper()
return sc.doRequest("GET", "/Users", nil)
}
func (sc *scimClient) getUser(id string) (string, int) {
sc.t.Helper()
return sc.doRequest("GET", "/Users/"+id, nil)
}
func (sc *scimClient) deleteUser(id string) (string, int) {
sc.t.Helper()
return sc.doRequest("DELETE", "/Users/"+id, nil)
}
func (sc *scimClient) doRequest(method, path string, payload any) (string, int) {
sc.t.Helper()
var body io.Reader
if payload != nil {
data, err := json.Marshal(payload)
require.NoError(sc.t, err)
body = bytes.NewReader(data)
}
req, err := http.NewRequest(method, sc.endpoint+path, body)
require.NoError(sc.t, err)
req.Header.Set("Authorization", "Bearer "+sc.token)
if payload != nil {
req.Header.Set("Content-Type", "application/scim+json")
}
resp, err := sc.client.Do(req)
require.NoError(sc.t, err)
defer func() { _ = resp.Body.Close() }()
respBody, err := io.ReadAll(resp.Body)
require.NoError(sc.t, err)
return string(respBody), resp.StatusCode
}
func TestSCIM_CreateUser(t *testing.T) {
t.Parallel()
owner := testutil.NewClient(t, testutil.RoleOwner)
sc := newSCIMClient(t, owner)
t.Run("create a new user", func(t *testing.T) {
t.Parallel()
email := factory.SafeEmail()
body, status := sc.createUser(email, "New User", "ext-create-1", true)
assert.Equal(t, http.StatusCreated, status, body)
var resource map[string]any
require.NoError(t, json.Unmarshal([]byte(body), &resource))
assert.Equal(t, email, resource["userName"])
assert.NotEmpty(t, resource["id"])
})
t.Run("duplicate user returns 409", func(t *testing.T) {
t.Parallel()
email := factory.SafeEmail()
_, status := sc.createUser(email, "Dup User", "ext-dup-1", true)
require.Equal(t, http.StatusCreated, status)
_, status = sc.createUser(email, "Dup User", "ext-dup-1", true)
assert.Equal(t, http.StatusConflict, status)
})
}
func TestSCIM_ListUsers(t *testing.T) {
t.Parallel()
owner := testutil.NewClient(t, testutil.RoleOwner)
sc := newSCIMClient(t, owner)
email := factory.SafeEmail()
_, status := sc.createUser(email, "List User", "ext-list-1", true)
require.Equal(t, http.StatusCreated, status)
body, status := sc.listUsers()
require.Equal(t, http.StatusOK, status, body)
var response map[string]any
require.NoError(t, json.Unmarshal([]byte(body), &response))
resources := response["Resources"].([]any)
assert.GreaterOrEqual(t, len(resources), 1)
}
func TestSCIM_ExternalIDFallback(t *testing.T) {
t.Parallel()
owner := testutil.NewClient(t, testutil.RoleOwner)
sc := newSCIMClient(t, owner)
t.Run("email rename reuses profile via external ID", func(t *testing.T) {
t.Parallel()
externalID := "google-" + factory.SafeName("")
oldEmail := factory.SafeEmail()
newEmail := factory.SafeEmail()
// Create user with old email
body, status := sc.createUser(oldEmail, "Rename User", externalID, true)
require.Equal(t, http.StatusCreated, status, body)
var created map[string]any
require.NoError(t, json.Unmarshal([]byte(body), &created))
originalID := created["id"].(string)
// Create user with new email but same external ID (simulates email rename)
body, status = sc.createUser(newEmail, "Rename User", externalID, true)
require.Equal(t, http.StatusCreated, status, body)
var updated map[string]any
require.NoError(t, json.Unmarshal([]byte(body), &updated))
// Should reuse the same profile (same ID)
assert.Equal(t, originalID, updated["id"].(string), "profile ID should be preserved after email rename")
assert.Equal(t, newEmail, updated["userName"], "email should be updated")
// Verify via GET that the profile is consistent
body, status = sc.getUser(originalID)
require.Equal(t, http.StatusOK, status, body)
var fetched map[string]any
require.NoError(t, json.Unmarshal([]byte(body), &fetched))
assert.Equal(t, newEmail, fetched["userName"])
})
t.Run("different external ID creates new profile", func(t *testing.T) {
t.Parallel()
email := factory.SafeEmail()
_, status := sc.createUser(email, "User A", "ext-a-"+factory.SafeName(""), true)
require.Equal(t, http.StatusCreated, status)
// Same email, different external ID — should fail (email already taken)
_, status = sc.createUser(email, "User B", "ext-b-"+factory.SafeName(""), true)
assert.Equal(t, http.StatusConflict, status)
})
}
func TestSCIM_DeleteUser(t *testing.T) {
t.Parallel()
owner := testutil.NewClient(t, testutil.RoleOwner)
sc := newSCIMClient(t, owner)
email := factory.SafeEmail()
body, status := sc.createUser(email, "Delete User", "ext-del-1", true)
require.Equal(t, http.StatusCreated, status, body)
var created map[string]any
require.NoError(t, json.Unmarshal([]byte(body), &created))
userID := created["id"].(string)
_, status = sc.deleteUser(userID)
assert.Equal(t, http.StatusNoContent, status)
_, status = sc.getUser(userID)
assert.Equal(t, http.StatusNotFound, status)
}
func TestSCIM_Unauthorized(t *testing.T) {
t.Parallel()
client := &http.Client{}
req, err := http.NewRequest("GET", testutil.GetBaseURL()+"/api/connect/v1/scim/2.0/Users", nil)
require.NoError(t, err)
resp, err := client.Do(req)
require.NoError(t, err)
defer func() { _ = resp.Body.Close() }()
assert.Equal(t, http.StatusUnauthorized, resp.StatusCode)
}

View File

@@ -286,6 +286,7 @@ func (m *Membership) Update(ctx context.Context, conn pg.Tx, scope Scoper) error
UPDATE
iam_memberships
SET
identity_id = @identity_id,
role = @role,
updated_at = @updated_at
WHERE
@@ -296,9 +297,10 @@ WHERE
query = fmt.Sprintf(query, scope.SQLFragment())
args := pgx.StrictNamedArgs{
"id": m.ID,
"role": m.Role,
"updated_at": m.UpdatedAt,
"id": m.ID,
"identity_id": m.IdentityID,
"role": m.Role,
"updated_at": m.UpdatedAt,
}
maps.Copy(args, scope.SQLArguments())

View File

@@ -262,6 +262,87 @@ LIMIT 1;
return nil
}
func (p *MembershipProfile) LoadByExternalIDAndOrganizationID(
ctx context.Context,
conn pg.Querier,
scope Scoper,
externalID string,
organizationID gid.GID,
) error {
q := `
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,
'' AS organization_name,
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.external_id = @external_id
AND p.organization_id = @organization_id
LIMIT 1;
`
q = fmt.Sprintf(q, scope.SQLFragment())
args := pgx.StrictNamedArgs{
"external_id": externalID,
"organization_id": organizationID,
}
maps.Copy(args, scope.SQLArguments())
rows, err := conn.Query(ctx, q, args)
if err != nil {
return fmt.Errorf("cannot query profile: %w", err)
}
profile, err := pgx.CollectExactlyOneRow(rows, pgx.RowToStructByName[MembershipProfile])
if err != nil {
if errors.Is(err, pgx.ErrNoRows) {
return ErrResourceNotFound
}
return fmt.Errorf("cannot collect profile: %w", err)
}
*p = profile
return nil
}
func (p *MembershipProfiles) LoadByIDs(
ctx context.Context,
conn pg.Querier,
@@ -1149,6 +1230,7 @@ func (p *MembershipProfile) Update(
UPDATE
iam_membership_profiles
SET
identity_id = @identity_id,
source = @source,
state = @state,
full_name = @full_name,
@@ -1186,6 +1268,7 @@ WHERE
args := pgx.StrictNamedArgs{
"id": p.ID,
"identity_id": p.IdentityID,
"source": p.Source,
"state": p.State,
"full_name": p.FullName,

View File

@@ -83,7 +83,7 @@ func (s *Bridge) Run(ctx context.Context) (created, updated, deleted, deactivate
existingSCIM, exists := scimUsersByEmail[email]
if !exists {
if err := s.scimClient.CreateUser(ctx, &pu); err != nil {
errs = append(errs, fmt.Errorf("cannot create user %q %q: %w", pu.ExternalID, pu.UserName, err))
errs = append(errs, fmt.Errorf("cannot create user %q: %w", pu.ExternalID, err))
continue
}
created++
@@ -104,7 +104,7 @@ func (s *Bridge) Run(ctx context.Context) (created, updated, deleted, deactivate
if needsUpdate {
if err := s.scimClient.UpdateUser(ctx, existingSCIM.ID, &pu); err != nil {
errs = append(errs, fmt.Errorf("cannot update user %q: %w", pu.UserName, err))
errs = append(errs, fmt.Errorf("cannot update user %q: %w", pu.ExternalID, err))
continue
}
updated++
@@ -121,7 +121,7 @@ func (s *Bridge) Run(ctx context.Context) (created, updated, deleted, deactivate
if s.isExcluded(email) {
if err := s.scimClient.DeleteUser(ctx, scimUser.ID); err != nil {
errs = append(errs, fmt.Errorf("cannot delete user %q: %w", email, err))
errs = append(errs, fmt.Errorf("cannot delete user %q: %w", scimUser.ExternalID, err))
continue
}
deleted++
@@ -133,7 +133,7 @@ func (s *Bridge) Run(ctx context.Context) (created, updated, deleted, deactivate
}
if err := s.scimClient.DeactivateUser(ctx, scimUser.ID); err != nil {
errs = append(errs, fmt.Errorf("cannot deactivate user %q: %w", email, err))
errs = append(errs, fmt.Errorf("cannot deactivate user %q: %w", scimUser.ExternalID, err))
continue
}
deactivated++

View File

@@ -177,6 +177,7 @@ func (s *Service) CreateUser(
eventType := coredata.WebhookEventTypeUserUpdated
profile = &coredata.MembershipProfile{}
if err := profile.LoadByIdentityIDAndOrganizationID(
ctx,
tx,
@@ -184,42 +185,63 @@ func (s *Service) CreateUser(
identity.ID,
config.OrganizationID,
); err != nil {
if errors.Is(err, coredata.ErrResourceNotFound) {
if !errors.Is(err, coredata.ErrResourceNotFound) {
return fmt.Errorf("cannot load profile: %w", err)
}
// Profile not found by identity. Try by external ID to
// handle email renames in identity providers (e.g. Google
// Workspace) where the external ID stays the same but the
// email changes. If found, update it to point to the new
// identity.
if externalIdPtr != nil {
if err := profile.LoadByExternalIDAndOrganizationID(
ctx,
tx,
scope,
*externalIdPtr,
config.OrganizationID,
); err == nil {
// Migrate the existing membership to the new identity
// so the user's role is preserved.
oldIdentityID := profile.IdentityID
existingMembership := &coredata.Membership{}
if err := existingMembership.LoadByIdentityIDAndOrganizationID(
ctx,
tx,
scope,
oldIdentityID,
config.OrganizationID,
); err == nil {
existingMembership.IdentityID = identity.ID
existingMembership.UpdatedAt = now
if err := existingMembership.Update(ctx, tx, scope); err != nil {
return fmt.Errorf("cannot update membership identity: %w", err)
}
} else if !errors.Is(err, coredata.ErrResourceNotFound) {
return fmt.Errorf("cannot load membership for identity migration: %w", err)
}
profile.IdentityID = identity.ID
profile.EmailAddress = emailAddr
applyUserAttributes(profile, attrs, externalIdPtr, profileState, now)
if err := profile.Update(ctx, tx, scope); err != nil {
return fmt.Errorf("cannot update profile: %w", err)
}
} else if !errors.Is(err, coredata.ErrResourceNotFound) {
return fmt.Errorf("cannot load profile by external id: %w", err)
}
}
if profile.ID == (gid.GID{}) {
profile = &coredata.MembershipProfile{
ID: gid.New(config.OrganizationID.TenantID(), coredata.MembershipProfileEntityType),
IdentityID: identity.ID,
OrganizationID: config.OrganizationID,
EmailAddress: emailAddr,
Source: coredata.ProfileSourceSCIM,
State: profileState,
FullName: attrs.FullName,
Position: &attrs.Title,
UserName: &attrs.UserName,
ExternalID: externalIdPtr,
Nickname: ref.RefOrNil(attrs.Nickname),
Locale: ref.RefOrNil(attrs.Locale),
Timezone: ref.RefOrNil(attrs.Timezone),
ProfileUrl: ref.RefOrNil(attrs.ProfileUrl),
PreferredLanguage: ref.RefOrNil(attrs.PreferredLanguage),
GivenName: ref.RefOrNil(attrs.GivenName),
FamilyName: ref.RefOrNil(attrs.FamilyName),
FormattedName: ref.RefOrNil(attrs.FormattedName),
MiddleName: ref.RefOrNil(attrs.MiddleName),
HonorificPrefix: ref.RefOrNil(attrs.HonorificPrefix),
HonorificSuffix: ref.RefOrNil(attrs.HonorificSuffix),
EmployeeNumber: ref.RefOrNil(attrs.EmployeeNumber),
Department: ref.RefOrNil(attrs.Department),
CostCenter: ref.RefOrNil(attrs.CostCenter),
EnterpriseOrganization: ref.RefOrNil(attrs.EnterpriseOrganization),
Division: ref.RefOrNil(attrs.Division),
ManagerValue: ref.RefOrNil(attrs.ManagerValue),
CreatedAt: now,
UpdatedAt: now,
}
if attrs.UserType != "" {
kind := attrs.UserType
profile.Kind = &kind
ID: gid.New(config.OrganizationID.TenantID(), coredata.MembershipProfileEntityType),
IdentityID: identity.ID,
OrganizationID: config.OrganizationID,
EmailAddress: emailAddr,
CreatedAt: now,
}
applyUserAttributes(profile, attrs, externalIdPtr, profileState, now)
err = profile.Insert(ctx, tx)
if err != nil {
@@ -229,8 +251,6 @@ func (s *Service) CreateUser(
return fmt.Errorf("cannot insert profile: %w", err)
}
eventType = coredata.WebhookEventTypeUserCreated
} else {
return fmt.Errorf("cannot load profile: %w", err)
}
} else {
if profile.Source == coredata.ProfileSourceSCIM {
@@ -249,34 +269,7 @@ func (s *Service) CreateUser(
}
}
profile.Source = coredata.ProfileSourceSCIM
profile.State = profileState
profile.FullName = attrs.FullName
profile.Position = &attrs.Title
profile.UserName = &attrs.UserName
profile.ExternalID = externalIdPtr
profile.Nickname = ref.RefOrNil(attrs.Nickname)
profile.Locale = ref.RefOrNil(attrs.Locale)
profile.Timezone = ref.RefOrNil(attrs.Timezone)
profile.ProfileUrl = ref.RefOrNil(attrs.ProfileUrl)
profile.PreferredLanguage = ref.RefOrNil(attrs.PreferredLanguage)
profile.GivenName = ref.RefOrNil(attrs.GivenName)
profile.FamilyName = ref.RefOrNil(attrs.FamilyName)
profile.FormattedName = ref.RefOrNil(attrs.FormattedName)
profile.MiddleName = ref.RefOrNil(attrs.MiddleName)
profile.HonorificPrefix = ref.RefOrNil(attrs.HonorificPrefix)
profile.HonorificSuffix = ref.RefOrNil(attrs.HonorificSuffix)
profile.EmployeeNumber = ref.RefOrNil(attrs.EmployeeNumber)
profile.Department = ref.RefOrNil(attrs.Department)
profile.CostCenter = ref.RefOrNil(attrs.CostCenter)
profile.EnterpriseOrganization = ref.RefOrNil(attrs.EnterpriseOrganization)
profile.Division = ref.RefOrNil(attrs.Division)
profile.ManagerValue = ref.RefOrNil(attrs.ManagerValue)
profile.UpdatedAt = now
if attrs.UserType != "" {
kind := attrs.UserType
profile.Kind = &kind
}
applyUserAttributes(profile, attrs, externalIdPtr, profileState, now)
if err := profile.Update(ctx, tx, scope); err != nil {
return fmt.Errorf("cannot update profile: %w", err)
}
@@ -742,6 +735,44 @@ func (s *Service) updateUser(
return profile, nil
}
func applyUserAttributes(
profile *coredata.MembershipProfile,
attrs scimUserAttributes,
externalID *string,
state coredata.ProfileState,
now time.Time,
) {
profile.Source = coredata.ProfileSourceSCIM
profile.State = state
profile.FullName = attrs.FullName
profile.Position = &attrs.Title
profile.UserName = &attrs.UserName
profile.ExternalID = externalID
profile.Nickname = ref.RefOrNil(attrs.Nickname)
profile.Locale = ref.RefOrNil(attrs.Locale)
profile.Timezone = ref.RefOrNil(attrs.Timezone)
profile.ProfileUrl = ref.RefOrNil(attrs.ProfileUrl)
profile.PreferredLanguage = ref.RefOrNil(attrs.PreferredLanguage)
profile.GivenName = ref.RefOrNil(attrs.GivenName)
profile.FamilyName = ref.RefOrNil(attrs.FamilyName)
profile.FormattedName = ref.RefOrNil(attrs.FormattedName)
profile.MiddleName = ref.RefOrNil(attrs.MiddleName)
profile.HonorificPrefix = ref.RefOrNil(attrs.HonorificPrefix)
profile.HonorificSuffix = ref.RefOrNil(attrs.HonorificSuffix)
profile.EmployeeNumber = ref.RefOrNil(attrs.EmployeeNumber)
profile.Department = ref.RefOrNil(attrs.Department)
profile.CostCenter = ref.RefOrNil(attrs.CostCenter)
profile.EnterpriseOrganization = ref.RefOrNil(attrs.EnterpriseOrganization)
profile.Division = ref.RefOrNil(attrs.Division)
profile.ManagerValue = ref.RefOrNil(attrs.ManagerValue)
profile.UpdatedAt = now
if attrs.UserType != "" {
kind := attrs.UserType
profile.Kind = &kind
}
}
func (s *Service) DeleteUser(
ctx context.Context,
config *coredata.SCIMConfiguration,