From c8250f679439a9486f367749e5f25a8a6835579c Mon Sep 17 00:00:00 2001 From: Bryan Frimin Date: Mon, 9 Mar 2026 16:06:32 +0100 Subject: [PATCH] Add userName and externalID support Signed-off-by: Bryan Frimin --- pkg/coredata/membership_profile.go | 40 +++++++ pkg/coredata/membership_profile_filter.go | 28 +++++ pkg/coredata/migrations/20260306T120000Z.sql | 16 +++ pkg/iam/scim/filter.go | 12 +- pkg/iam/scim/filter_test.go | 69 +++++++++--- pkg/iam/scim/service.go | 109 ++++++++++++++----- 6 files changed, 223 insertions(+), 51 deletions(-) create mode 100644 pkg/coredata/migrations/20260306T120000Z.sql diff --git a/pkg/coredata/membership_profile.go b/pkg/coredata/membership_profile.go index f5e6920e3..ed92d30d5 100644 --- a/pkg/coredata/membership_profile.go +++ b/pkg/coredata/membership_profile.go @@ -44,6 +44,8 @@ type ( ContractStartDate *time.Time `db:"contract_start_date"` ContractEndDate *time.Time `db:"contract_end_date"` OrganizationName string `db:"organization_name"` + UserName *string `db:"user_name"` + ExternalID *string `db:"external_id"` CreatedAt time.Time `db:"created_at"` UpdatedAt time.Time `db:"updated_at"` } @@ -107,6 +109,8 @@ SELECT p.contract_start_date, p.contract_end_date, '' AS organization_name, + p.user_name, + p.external_id, p.created_at, p.updated_at FROM @@ -165,6 +169,8 @@ SELECT p.contract_start_date, p.contract_end_date, '' AS organization_name, + p.user_name, + p.external_id, p.created_at, p.updated_at FROM @@ -226,6 +232,8 @@ SELECT p.contract_start_date, p.contract_end_date, '' AS organization_name, + p.user_name, + p.external_id, p.created_at, p.updated_at FROM @@ -280,6 +288,8 @@ WITH profiles AS ( p.position, p.contract_start_date, p.contract_end_date, + p.user_name, + p.external_id, p.created_at, p.updated_at FROM @@ -304,6 +314,8 @@ SELECT contract_start_date, contract_end_date, '' AS organization_name, + user_name, + external_id, created_at, updated_at FROM profiles @@ -355,6 +367,8 @@ WITH profiles AS ( p.position, p.contract_start_date, p.contract_end_date, + p.user_name, + p.external_id, p.created_at, p.updated_at FROM @@ -378,6 +392,8 @@ SELECT p.contract_start_date, p.contract_end_date, o.name AS organization_name, + p.user_name, + p.external_id, p.created_at, p.updated_at FROM profiles p @@ -428,6 +444,8 @@ WITH profiles AS ( mp.position, mp.contract_start_date, mp.contract_end_date, + mp.user_name, + mp.external_id, mp.created_at, mp.updated_at FROM @@ -455,6 +473,8 @@ SELECT p.contract_start_date, p.contract_end_date, '' AS organization_name, + p.user_name, + p.external_id, p.created_at, p.updated_at FROM profiles p @@ -534,6 +554,8 @@ WITH profiles AS ( mp.position, mp.contract_start_date, mp.contract_end_date, + mp.user_name, + mp.external_id, mp.created_at, mp.updated_at FROM @@ -561,6 +583,8 @@ SELECT p.contract_start_date, p.contract_end_date, '' AS organization_name, + p.user_name, + p.external_id, p.created_at, p.updated_at FROM profiles p @@ -641,6 +665,8 @@ WITH attendees AS ( p.position, p.contract_start_date, p.contract_end_date, + p.user_name, + p.external_id, p.created_at, p.updated_at, ma.created_at AS attendee_created_at @@ -667,6 +693,8 @@ SELECT contract_start_date, contract_end_date, '' AS organization_name, + user_name, + external_id, created_at, updated_at FROM @@ -728,6 +756,8 @@ SELECT p.contract_start_date, p.contract_end_date, '' AS organization_name, + p.user_name, + p.external_id, p.created_at, p.updated_at FROM @@ -881,6 +911,8 @@ INSERT INTO position, contract_start_date, contract_end_date, + user_name, + external_id, created_at, updated_at ) @@ -897,6 +929,8 @@ VALUES ( @position, @contract_start_date, @contract_end_date, + @user_name, + @external_id, @created_at, @updated_at ) @@ -915,6 +949,8 @@ VALUES ( "position": p.Position, "contract_start_date": p.ContractStartDate, "contract_end_date": p.ContractEndDate, + "user_name": p.UserName, + "external_id": p.ExternalID, "created_at": p.CreatedAt, "updated_at": p.UpdatedAt, } @@ -949,6 +985,8 @@ SET position = @position, contract_start_date = @contract_start_date, contract_end_date = @contract_end_date, + user_name = @user_name, + external_id = @external_id, updated_at = @updated_at WHERE id = @id @@ -967,6 +1005,8 @@ WHERE "position": p.Position, "contract_start_date": p.ContractStartDate, "contract_end_date": p.ContractEndDate, + "user_name": p.UserName, + "external_id": p.ExternalID, "updated_at": p.UpdatedAt, } maps.Copy(args, scope.SQLArguments()) diff --git a/pkg/coredata/membership_profile_filter.go b/pkg/coredata/membership_profile_filter.go index 5b29531a5..ee53048bd 100644 --- a/pkg/coredata/membership_profile_filter.go +++ b/pkg/coredata/membership_profile_filter.go @@ -28,6 +28,8 @@ type ( excludeContractEnded *bool currentDate time.Time email *mail.Addr + userName *string + externalID *string state *ProfileState source *ProfileSource } @@ -59,6 +61,16 @@ func (f *MembershipProfileFilter) Email() *mail.Addr { return f.email } +func (f *MembershipProfileFilter) WithUserName(userName string) *MembershipProfileFilter { + f.userName = &userName + return f +} + +func (f *MembershipProfileFilter) WithExternalID(externalID string) *MembershipProfileFilter { + f.externalID = &externalID + return f +} + func (f *MembershipProfileFilter) WithState(state ProfileState) *MembershipProfileFilter { f.state = &state return f @@ -80,6 +92,8 @@ func (f *MembershipProfileFilter) Source() *ProfileSource { func (f *MembershipProfileFilter) SQLArguments() pgx.StrictNamedArgs { return pgx.StrictNamedArgs{ "filter_email": f.email, + "filter_user_name": f.userName, + "filter_external_id": f.externalID, "with_membership": f.withMembership, "with_trust_center_access": f.withTrustCenterAccess, "exclude_contract_ended": f.excludeContractEnded, @@ -137,5 +151,19 @@ AND ( ELSE TRUE END ) +AND ( + CASE + WHEN @filter_user_name::text IS NOT NULL THEN + p.user_name = @filter_user_name::text + ELSE TRUE + END +) +AND ( + CASE + WHEN @filter_external_id::text IS NOT NULL THEN + p.external_id = @filter_external_id::text + ELSE TRUE + END +) ` } diff --git a/pkg/coredata/migrations/20260306T120000Z.sql b/pkg/coredata/migrations/20260306T120000Z.sql new file mode 100644 index 000000000..4bed7f35b --- /dev/null +++ b/pkg/coredata/migrations/20260306T120000Z.sql @@ -0,0 +1,16 @@ +ALTER TABLE iam_membership_profiles +ADD COLUMN user_name TEXT, +ADD COLUMN external_id TEXT; + +UPDATE iam_membership_profiles p +SET user_name = i.email_address +FROM identities i +WHERE i.id = p.identity_id AND p.source = 'SCIM'; + +CREATE UNIQUE INDEX idx_profiles_user_name_organization_id +ON iam_membership_profiles (user_name, organization_id) +WHERE user_name IS NOT NULL; + +CREATE UNIQUE INDEX idx_profiles_external_id_organization_id +ON iam_membership_profiles (external_id, organization_id) +WHERE external_id IS NOT NULL; diff --git a/pkg/iam/scim/filter.go b/pkg/iam/scim/filter.go index c32d38453..451089c88 100644 --- a/pkg/iam/scim/filter.go +++ b/pkg/iam/scim/filter.go @@ -21,7 +21,6 @@ import ( scimerrors "github.com/elimity-com/scim/errors" scimfilter "github.com/scim2/filter-parser/v2" "go.probo.inc/probo/pkg/coredata" - "go.probo.inc/probo/pkg/mail" ) func ParseUserFilter(expr scimfilter.Expression) (*coredata.MembershipProfileFilter, error) { @@ -52,15 +51,12 @@ func ParseUserFilter(expr scimfilter.Expression) (*coredata.MembershipProfileFil attrName := strings.ToLower(e.AttributePath.AttributeName) switch attrName { case "username": - email, err := mail.ParseAddr(value) - if err != nil { - return nil, scimerrors.ScimErrorBadRequest( - fmt.Sprintf("invalid email format for userName: %s", value)) - } - filter.WithEmail(&email) + filter.WithUserName(value) + case "externalid": + filter.WithExternalID(value) default: return nil, scimerrors.ScimErrorBadRequest( - fmt.Sprintf("attribute '%s' is not supported for filtering, only 'userName' is supported", e.AttributePath.AttributeName)) + fmt.Sprintf("attribute '%s' is not supported for filtering, only 'userName' and 'externalId' are supported", e.AttributePath.AttributeName)) } case *scimfilter.LogicalExpression: diff --git a/pkg/iam/scim/filter_test.go b/pkg/iam/scim/filter_test.go index 4b5178346..7a5235715 100644 --- a/pkg/iam/scim/filter_test.go +++ b/pkg/iam/scim/filter_test.go @@ -30,15 +30,28 @@ func TestParseUserFilter(t *testing.T) { assert.Nil(t, filter.Email()) }) - t.Run("simple userName eq filter", func(t *testing.T) { + t.Run("simple userName eq filter with email value", func(t *testing.T) { expr, err := scimfilter.ParseFilter([]byte(`userName eq "test@example.com"`)) require.NoError(t, err) filter, err := ParseUserFilter(expr) require.NoError(t, err) require.NotNil(t, filter) - require.NotNil(t, filter.Email()) - assert.Equal(t, "test@example.com", filter.Email().String()) + args := filter.SQLArguments() + require.NotNil(t, args["filter_user_name"]) + assert.Equal(t, "test@example.com", *args["filter_user_name"].(*string)) + }) + + t.Run("userName eq filter with non-email UPN", func(t *testing.T) { + expr, err := scimfilter.ParseFilter([]byte(`userName eq "john.doe"`)) + require.NoError(t, err) + + filter, err := ParseUserFilter(expr) + require.NoError(t, err) + require.NotNil(t, filter) + args := filter.SQLArguments() + require.NotNil(t, args["filter_user_name"]) + assert.Equal(t, "john.doe", *args["filter_user_name"].(*string)) }) t.Run("userName filter is case insensitive", func(t *testing.T) { @@ -48,8 +61,33 @@ func TestParseUserFilter(t *testing.T) { filter, err := ParseUserFilter(expr) require.NoError(t, err) require.NotNil(t, filter) - require.NotNil(t, filter.Email()) - assert.Equal(t, "test@example.com", filter.Email().String()) + args := filter.SQLArguments() + require.NotNil(t, args["filter_user_name"]) + assert.Equal(t, "test@example.com", *args["filter_user_name"].(*string)) + }) + + t.Run("externalId eq filter", func(t *testing.T) { + expr, err := scimfilter.ParseFilter([]byte(`externalId eq "some-azure-id"`)) + require.NoError(t, err) + + filter, err := ParseUserFilter(expr) + require.NoError(t, err) + require.NotNil(t, filter) + args := filter.SQLArguments() + require.NotNil(t, args["filter_external_id"]) + assert.Equal(t, "some-azure-id", *args["filter_external_id"].(*string)) + }) + + t.Run("externalId filter is case insensitive", func(t *testing.T) { + expr, err := scimfilter.ParseFilter([]byte(`ExternalId eq "azure-obj-id"`)) + require.NoError(t, err) + + filter, err := ParseUserFilter(expr) + require.NoError(t, err) + require.NotNil(t, filter) + args := filter.SQLArguments() + require.NotNil(t, args["filter_external_id"]) + assert.Equal(t, "azure-obj-id", *args["filter_external_id"].(*string)) }) t.Run("logical AND expression", func(t *testing.T) { @@ -59,8 +97,8 @@ func TestParseUserFilter(t *testing.T) { filter, err := ParseUserFilter(expr) require.NoError(t, err) require.NotNil(t, filter) - // Last processed value wins - require.NotNil(t, filter.Email()) + args := filter.SQLArguments() + require.NotNil(t, args["filter_user_name"]) }) t.Run("unsupported operator returns error", func(t *testing.T) { @@ -113,16 +151,21 @@ func TestParseUserFilter(t *testing.T) { filter, err := ParseUserFilter(expr) require.NoError(t, err) require.NotNil(t, filter) - require.NotNil(t, filter.Email()) + args := filter.SQLArguments() + require.NotNil(t, args["filter_user_name"]) }) - t.Run("invalid email format returns error", func(t *testing.T) { - expr, err := scimfilter.ParseFilter([]byte(`userName eq "not-an-email"`)) + t.Run("userName and externalId combined", func(t *testing.T) { + expr, err := scimfilter.ParseFilter([]byte(`userName eq "john@contoso.com" and externalId eq "abc-123"`)) require.NoError(t, err) filter, err := ParseUserFilter(expr) - assert.Error(t, err) - assert.Nil(t, filter) - assert.Contains(t, err.Error(), "invalid email format") + require.NoError(t, err) + require.NotNil(t, filter) + args := filter.SQLArguments() + require.NotNil(t, args["filter_user_name"]) + assert.Equal(t, "john@contoso.com", *args["filter_user_name"].(*string)) + require.NotNil(t, args["filter_external_id"]) + assert.Equal(t, "abc-123", *args["filter_external_id"].(*string)) }) } diff --git a/pkg/iam/scim/service.go b/pkg/iam/scim/service.go index f71479f98..2189b475f 100644 --- a/pkg/iam/scim/service.go +++ b/pkg/iam/scim/service.go @@ -130,9 +130,12 @@ func (s *Service) CreateUser( config *coredata.SCIMConfiguration, attributes scim.ResourceAttributes, ) (scim.Resource, error) { - email, fullName, active, title := ParseUserFromAttributes(attributes) + userName, email, fullName, active, title, externalId := ParseUserFromAttributes(attributes) + if userName == "" { + return scim.Resource{}, scimerrors.ScimErrorBadRequest("userName is required") + } if email == "" { - return scim.Resource{}, scimerrors.ScimErrorBadRequest("userName or email is required") + return scim.Resource{}, scimerrors.ScimErrorBadRequest("a valid email is required (via emails array or userName)") } emailAddr, err := mail.ParseAddr(email) @@ -146,17 +149,20 @@ func (s *Service) CreateUser( profileState = coredata.ProfileStateInactive } + var externalIdPtr *string + if externalId != "" { + externalIdPtr = &externalId + } + var membership *coredata.Membership var profile *coredata.MembershipProfile scope := coredata.NewScopeFromObjectID(config.OrganizationID) err = s.pg.WithTx(ctx, func(tx pg.Conn) error { - // Check if identity exists identity := &coredata.Identity{} if err := identity.LoadByEmail(ctx, tx, emailAddr); err != nil { if errors.Is(err, coredata.ErrResourceNotFound) { - // Create new identity identity = &coredata.Identity{ ID: gid.New(gid.NilTenant, coredata.IdentityEntityType), EmailAddress: emailAddr, @@ -175,7 +181,6 @@ func (s *Service) CreateUser( } } - // Check if profile exists eventType := coredata.WebhookEventTypeUserUpdated profile = &coredata.MembershipProfile{} if err := profile.LoadByIdentityIDAndOrganizationID( @@ -195,12 +200,17 @@ func (s *Service) CreateUser( State: profileState, FullName: fullName, Position: &title, + UserName: &userName, + ExternalID: externalIdPtr, CreatedAt: now, UpdatedAt: now, } err = profile.Insert(ctx, tx) if err != nil { + if errors.Is(err, coredata.ErrResourceAlreadyExists) { + return scimerrors.ScimErrorUniqueness + } return fmt.Errorf("cannot insert profile: %w", err) } eventType = coredata.WebhookEventTypeUserCreated @@ -212,6 +222,8 @@ func (s *Service) CreateUser( profile.State = profileState profile.FullName = fullName profile.Position = &title + profile.UserName = &userName + profile.ExternalID = externalIdPtr profile.UpdatedAt = now if err := profile.Update(ctx, tx, scope); err != nil { return fmt.Errorf("cannot update profile: %w", err) @@ -219,7 +231,6 @@ func (s *Service) CreateUser( } if !active { - // Expire pending invitations for user invitations := &coredata.Invitations{} onlyPending := coredata.NewInvitationFilter([]coredata.InvitationStatus{coredata.InvitationStatusPending}) if err := invitations.ExpireByUserID( @@ -233,7 +244,6 @@ func (s *Service) CreateUser( } } - // Check if membership exists membership = &coredata.Membership{} if err := membership.LoadByIdentityIDAndOrganizationID( ctx, @@ -243,7 +253,6 @@ func (s *Service) CreateUser( config.OrganizationID, ); err != nil { if errors.Is(err, coredata.ErrResourceNotFound) { - // Create new membership membership = &coredata.Membership{ ID: gid.New(config.OrganizationID.TenantID(), coredata.MembershipEntityType), IdentityID: identity.ID, @@ -391,8 +400,8 @@ func (s *Service) ReplaceUser( profileID gid.GID, attributes scim.ResourceAttributes, ) (scim.Resource, error) { - fullName, active, title := ParseUserFromReplaceAttributes(attributes) - profile, err := s.updateUser(ctx, config, profileID, fullName, active, title) + fullName, active, title, userName, externalId := ParseUserFromReplaceAttributes(attributes) + profile, err := s.updateUser(ctx, config, profileID, fullName, active, title, userName, externalId) if err != nil { return scim.Resource{}, err } @@ -406,8 +415,8 @@ func (s *Service) PatchUser( profileID gid.GID, operations []scim.PatchOperation, ) (scim.Resource, error) { - fullName, active, title := ParseUserFromPatchOperations(operations) - profile, err := s.updateUser(ctx, config, profileID, fullName, active, title) + fullName, active, title, userName, externalId := ParseUserFromPatchOperations(operations) + profile, err := s.updateUser(ctx, config, profileID, fullName, active, title, userName, externalId) if err != nil { return scim.Resource{}, err } @@ -422,6 +431,8 @@ func (s *Service) updateUser( fullName string, active *bool, title string, + userName *string, + externalId *string, ) (*coredata.MembershipProfile, error) { scope := coredata.NewScopeFromObjectID(config.OrganizationID) now := time.Now() @@ -466,6 +477,16 @@ func (s *Service) updateUser( profile.Position = &title } + if userName != nil { + profile.UserName = userName + profile.UpdatedAt = now + } + + if externalId != nil { + profile.ExternalID = externalId + profile.UpdatedAt = now + } + if shouldReactivate { profile.State = coredata.ProfileStateActive profile.UpdatedAt = now @@ -621,11 +642,11 @@ func (s *Service) createEvent( return event } -func ParseUserFromAttributes(attributes scim.ResourceAttributes) (email string, fullName string, active bool, title string) { - userName, _ := attributes["userName"].(string) +func ParseUserFromAttributes(attributes scim.ResourceAttributes) (userName, email, fullName string, active bool, title, externalId string) { + userName, _ = attributes["userName"].(string) displayName, _ := attributes["displayName"].(string) + externalId, _ = attributes["externalId"].(string) - // Default to active if the attribute is not present. active = true if a, ok := attributes["active"].(bool); ok { active = a @@ -637,8 +658,7 @@ func ParseUserFromAttributes(attributes scim.ResourceAttributes) (email string, familyName, _ = name["familyName"].(string) } - // Get email from emails array or use userName - email = userName + // Get email from emails array first if emails, ok := attributes["emails"].([]any); ok && len(emails) > 0 { for _, e := range emails { if emailMap, ok := e.(map[string]any); ok { @@ -650,8 +670,7 @@ func ParseUserFromAttributes(attributes scim.ResourceAttributes) (email string, } } } - // If no primary email found, use the first one - if email == userName { + if email == "" { if emailMap, ok := emails[0].(map[string]any); ok { if value, ok := emailMap["value"].(string); ok { email = value @@ -660,7 +679,13 @@ func ParseUserFromAttributes(attributes scim.ResourceAttributes) (email string, } } - // Build full name: prefer displayName, then given+family, then userName + // Fall back to userName only if it parses as a valid email + if email == "" { + if _, err := mail.ParseAddr(userName); err == nil { + email = userName + } + } + fullName = displayName if fullName == "" { fullName = strings.TrimSpace(givenName + " " + familyName) @@ -673,10 +698,10 @@ func ParseUserFromAttributes(attributes scim.ResourceAttributes) (email string, title = t } - return email, fullName, active, title + return userName, email, fullName, active, title, externalId } -func ParseUserFromReplaceAttributes(attributes scim.ResourceAttributes) (fullName string, active *bool, title string) { +func ParseUserFromReplaceAttributes(attributes scim.ResourceAttributes) (fullName string, active *bool, title string, userName *string, externalId *string) { displayName, _ := attributes["displayName"].(string) var givenName, familyName string @@ -699,10 +724,18 @@ func ParseUserFromReplaceAttributes(attributes scim.ResourceAttributes) (fullNam title = t } - return fullName, &activeVal, title + if un, ok := attributes["userName"].(string); ok && un != "" { + userName = &un + } + + if eid, ok := attributes["externalId"].(string); ok && eid != "" { + externalId = &eid + } + + return fullName, &activeVal, title, userName, externalId } -func ParseUserFromPatchOperations(operations []scim.PatchOperation) (fullName string, active *bool, title string) { +func ParseUserFromPatchOperations(operations []scim.PatchOperation) (fullName string, active *bool, title string, userName *string, externalId *string) { var givenName, familyName string for _, op := range operations { @@ -712,8 +745,6 @@ func ParseUserFromPatchOperations(operations []scim.PatchOperation) (fullName st path = op.Path.String() } - // Handle empty path with value map (Okta style) - // e.g., { "op": "Replace", "value": { "active": false } } if path == "" { if valueMap, ok := op.Value.(map[string]any); ok { if a, ok := valueMap["active"].(bool); ok { @@ -730,6 +761,12 @@ func ParseUserFromPatchOperations(operations []scim.PatchOperation) (fullName st familyName = fn } } + if un, ok := valueMap["userName"].(string); ok && un != "" { + userName = &un + } + if eid, ok := valueMap["externalId"].(string); ok && eid != "" { + externalId = &eid + } } continue } @@ -755,24 +792,36 @@ func ParseUserFromPatchOperations(operations []scim.PatchOperation) (fullName st if t, ok := op.Value.(string); ok { title = t } + case "username": + if un, ok := op.Value.(string); ok && un != "" { + userName = &un + } + case "externalid": + if eid, ok := op.Value.(string); ok && eid != "" { + externalId = &eid + } } } } - // If no displayName was set but we have name parts, build full name if fullName == "" && (givenName != "" || familyName != "") { fullName = strings.TrimSpace(givenName + " " + familyName) } - return fullName, active, title + return fullName, active, title, userName, externalId } func userToResource(p *coredata.MembershipProfile) scim.Resource { + externalID := optional.NewString(p.ID.String()) + if p.ExternalID != nil { + externalID = optional.NewString(*p.ExternalID) + } + return scim.Resource{ ID: p.ID.String(), - ExternalID: optional.NewString(p.ID.String()), + ExternalID: externalID, Attributes: scim.ResourceAttributes{ - "userName": p.EmailAddress.String(), + "userName": *p.UserName, "displayName": p.FullName, "active": p.State == coredata.ProfileStateActive, "name": map[string]any{