From 3575a9ae840eec36bf102a0a567cb6cb07fea472 Mon Sep 17 00:00:00 2001 From: "Christoph Engelbert (noctarius)" Date: Fri, 24 Apr 2026 13:21:29 +0200 Subject: [PATCH 1/2] Link social login to existing email principal Fix social identity linking to reuse an existing principal when the provider email matches an existing principal slug. This prevents duplicate user records during Google sign-in for already provisioned accounts. Add an integration regression test that reproduces the duplicate-account case and verifies only one principal remains while the social identity is linked. Update the slug-collision test to continue covering non-email fallback slug collision behavior. Co-authored-by: Codex --- designs/TASKS.md | 8 +++++ internal/social/identity.go | 44 +++++++++-------------- internal/social/identity_test.go | 61 +++++++++++++++++++++++++++++--- 3 files changed, 82 insertions(+), 31 deletions(-) diff --git a/designs/TASKS.md b/designs/TASKS.md index 610162e5..0932634e 100644 --- a/designs/TASKS.md +++ b/designs/TASKS.md @@ -25,6 +25,14 @@ ## Implementation Tasks +- [x] 2026-04-24: Fixed social login duplicate principal creation when email slug already exists (TDD-first): + - Added integration regression coverage in `internal/social/identity_test.go`: + - `TestIdentityStore_FindOrCreate_ExistingEmailSlug_LinksPrincipal` + - Updated `internal/social/identity.go` `FindOrCreateWithPolicy` flow to: + - link social identities to an existing principal when `info.Email` matches an existing principal slug, + - only create a new principal when no social identity and no matching email principal exist. + - Updated slug-collision coverage in `internal/social/identity_test.go` to validate fallback collision handling for non-email slugs. + - [x] 2026-04-14: Fixed failing unit/integration tests after constructor signature changes (TDD green pass): - Updated `internal/codegraph/syncer_test.go` and `internal/api/rest/scopes_test.go` to use the current diff --git a/internal/social/identity.go b/internal/social/identity.go index baa881e2..f4ce0066 100644 --- a/internal/social/identity.go +++ b/internal/social/identity.go @@ -79,17 +79,7 @@ func (s *IdentityStore) FindOrCreateWithPolicy(ctx context.Context, provider str displayName = baseSlug } - if !policy.AutoCreateUsers { - if info.Email == "" { - return nil, ErrPrincipalNotProvisioned - } - principal, lookupErr := q.GetPrincipalBySlug(ctx, info.Email) - if lookupErr != nil { - if errors.Is(lookupErr, pgx.ErrNoRows) { - return nil, ErrPrincipalNotProvisioned - } - return nil, fmt.Errorf("lookup principal by email slug: %w", lookupErr) - } + linkPrincipal := func(principal *db.Principal) (*db.Principal, error) { if _, err := q.UpsertSocialIdentity(ctx, db.UpsertSocialIdentityParams{ PrincipalID: principal.ID, Provider: provider, @@ -107,6 +97,21 @@ func (s *IdentityStore) FindOrCreateWithPolicy(ctx context.Context, provider str return principal, nil } + emailSlug := strings.TrimSpace(info.Email) + if emailSlug != "" { + principal, lookupErr := q.GetPrincipalBySlug(ctx, emailSlug) + if lookupErr == nil { + return linkPrincipal(principal) + } + if !errors.Is(lookupErr, pgx.ErrNoRows) { + return nil, fmt.Errorf("lookup principal by email slug: %w", lookupErr) + } + } + + if !policy.AutoCreateUsers { + return nil, ErrPrincipalNotProvisioned + } + slug := baseSlug if _, slugErr := q.GetPrincipalBySlug(ctx, baseSlug); slugErr == nil { slug = baseSlug + "-" + info.ProviderID @@ -124,22 +129,7 @@ func (s *IdentityStore) FindOrCreateWithPolicy(ctx context.Context, provider str return nil, err } - if _, err := q.UpsertSocialIdentity(ctx, db.UpsertSocialIdentityParams{ - PrincipalID: principal.ID, - Provider: provider, - ProviderID: info.ProviderID, - Email: optionalString(info.Email), - DisplayName: optionalString(info.DisplayName), - AvatarUrl: optionalString(info.AvatarURL), - RawProfile: info.RawProfile, - }); err != nil { - return nil, err - } - - if err := tx.Commit(ctx); err != nil { - return nil, err - } - return principal, nil + return linkPrincipal(principal) } func principalSlug(provider string, info *UserInfo) string { diff --git a/internal/social/identity_test.go b/internal/social/identity_test.go index f6f0e102..6db05577 100644 --- a/internal/social/identity_test.go +++ b/internal/social/identity_test.go @@ -100,7 +100,7 @@ func TestIdentityStore_FindOrCreate_SlugCollision_AppendsProviderID(t *testing.T q := db.New(pool) _, err := q.CreatePrincipal(ctx, db.CreatePrincipalParams{ Kind: "user", - Slug: "collision@example.com", + Slug: "github-gh-collision-1", DisplayName: "Existing User", Meta: []byte(`{}`), }) @@ -110,7 +110,6 @@ func TestIdentityStore_FindOrCreate_SlugCollision_AppendsProviderID(t *testing.T principal, err := store.FindOrCreate(ctx, "github", &UserInfo{ ProviderID: "gh-collision-1", - Email: "collision@example.com", DisplayName: "Colliding User", AvatarURL: "https://cdn.example/collision.png", RawProfile: []byte(`{"id":"gh-collision-1"}`), @@ -118,8 +117,8 @@ func TestIdentityStore_FindOrCreate_SlugCollision_AppendsProviderID(t *testing.T if err != nil { t.Fatalf("FindOrCreate collision: %v", err) } - if principal.Slug != "collision@example.com-gh-collision-1" { - t.Fatalf("principal slug = %q, want collision@example.com-gh-collision-1", principal.Slug) + if principal.Slug != "github-gh-collision-1-gh-collision-1" { + t.Fatalf("principal slug = %q, want github-gh-collision-1-gh-collision-1", principal.Slug) } var linkedPrincipalID string @@ -135,6 +134,60 @@ func TestIdentityStore_FindOrCreate_SlugCollision_AppendsProviderID(t *testing.T } } +func TestIdentityStore_FindOrCreate_ExistingEmailSlug_LinksPrincipal(t *testing.T) { + pool := testhelper.NewTestPool(t) + store := NewIdentityStore(pool) + ctx := context.Background() + + q := db.New(pool) + existing, err := q.CreatePrincipal(ctx, db.CreatePrincipalParams{ + Kind: "user", + Slug: "manohar@simplyblock.io", + DisplayName: "Manohar Reddy", + Meta: []byte(`{}`), + }) + if err != nil { + t.Fatalf("seed existing principal: %v", err) + } + + principal, err := store.FindOrCreate(ctx, "google", &UserInfo{ + ProviderID: "112743087521354650483", + Email: "manohar@simplyblock.io", + EmailVerified: true, + DisplayName: "Manohar Reddy", + RawProfile: []byte(`{"sub":"112743087521354650483"}`), + }) + if err != nil { + t.Fatalf("FindOrCreate: %v", err) + } + if principal.ID != existing.ID { + t.Fatalf("principal ID = %s, want %s", principal.ID, existing.ID) + } + if principal.Slug != existing.Slug { + t.Fatalf("principal slug = %q, want %q", principal.Slug, existing.Slug) + } + + var principalCount int + if err := pool.QueryRow(ctx, `SELECT COUNT(*) FROM principals`).Scan(&principalCount); err != nil { + t.Fatalf("count principals: %v", err) + } + if principalCount != 1 { + t.Fatalf("principal count = %d, want 1", principalCount) + } + + var linkedPrincipalID string + err = pool.QueryRow(ctx, ` + SELECT principal_id::text FROM social_identities + WHERE provider = 'google' AND provider_id = '112743087521354650483' + `).Scan(&linkedPrincipalID) + if err != nil { + t.Fatalf("query social identity: %v", err) + } + if linkedPrincipalID != existing.ID.String() { + t.Fatalf("linked principal_id = %s, want %s", linkedPrincipalID, existing.ID) + } +} + func TestIdentityStore_FindOrCreateWithPolicy_AutoCreateDisabled_PreprovisionedEmailLinksPrincipal(t *testing.T) { pool := testhelper.NewTestPool(t) store := NewIdentityStore(pool) From 76e4259aaa0fe0b34ac057ece9b32167c00f7876 Mon Sep 17 00:00:00 2001 From: noctarius aka Christoph Engelbert Date: Fri, 24 Apr 2026 13:49:56 +0200 Subject: [PATCH 2/2] Update internal/social/identity.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- internal/social/identity.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/social/identity.go b/internal/social/identity.go index f4ce0066..f0da25bb 100644 --- a/internal/social/identity.go +++ b/internal/social/identity.go @@ -101,6 +101,9 @@ func (s *IdentityStore) FindOrCreateWithPolicy(ctx context.Context, provider str if emailSlug != "" { principal, lookupErr := q.GetPrincipalBySlug(ctx, emailSlug) if lookupErr == nil { + if principal.Kind != "user" { + return nil, fmt.Errorf("email slug %q matched non-user principal kind %q", emailSlug, principal.Kind) + } return linkPrincipal(principal) } if !errors.Is(lookupErr, pgx.ErrNoRows) {