From 21b1eb7cd8f64c04aeb255d7ecffa3accb2c43f8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Feb 2026 15:51:42 +0000 Subject: [PATCH 1/4] Initial plan From 070ea29b67930660b23f366d0f4851fde4d64f47 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Feb 2026 15:56:28 +0000 Subject: [PATCH 2/4] Allow email to be empty for OAuth users Co-authored-by: slhmy <31381093+slhmy@users.noreply.github.com> --- internal/application/identra/service.go | 44 +++-- .../identra/service_ensure_oauth_user_test.go | 183 ++++++++++++++++++ internal/domain/user.go | 2 +- .../persistence/mongo_user_store.go | 2 +- 4 files changed, 210 insertions(+), 21 deletions(-) create mode 100644 internal/application/identra/service_ensure_oauth_user_test.go diff --git a/internal/application/identra/service.go b/internal/application/identra/service.go index a8c1adf..2a9d57a 100644 --- a/internal/application/identra/service.go +++ b/internal/application/identra/service.go @@ -624,28 +624,34 @@ func (s *Service) ensureOAuthUser(ctx context.Context, info UserInfo) (*domain.U case err == nil: return s.updateEmailIfNeeded(ctx, existing, info.Email) case errors.Is(err, domain.ErrNotFound): - // If user is not linked by provider id, we need email to link by email or create a new user. + // If user is not linked by provider id, try to link by email if available. // (Email may be missing for some OAuth users, depending on provider settings / privacy.) - if strings.TrimSpace(info.Email) == "" { - return nil, status.Error(codes.FailedPrecondition, "email is required to create or link user") - } - byEmail, emailErr := s.userStore.GetByEmail(ctx, info.Email) - switch { - case emailErr == nil: - byEmail.GithubID = &info.ID - if updateErr := s.userStore.Update(ctx, byEmail); updateErr != nil { - return nil, status.Error(codes.Internal, "failed to link github account") - } - return byEmail, nil - case errors.Is(emailErr, domain.ErrNotFound): - userModel := &domain.UserModel{Email: info.Email, GithubID: &info.ID} - if createErr := s.userStore.Create(ctx, userModel); createErr != nil { - return nil, status.Error(codes.Internal, "failed to create user") + if strings.TrimSpace(info.Email) != "" { + byEmail, emailErr := s.userStore.GetByEmail(ctx, info.Email) + switch { + case emailErr == nil: + byEmail.GithubID = &info.ID + if updateErr := s.userStore.Update(ctx, byEmail); updateErr != nil { + return nil, status.Error(codes.Internal, "failed to link github account") + } + return byEmail, nil + case errors.Is(emailErr, domain.ErrNotFound): + // Email is provided but no existing user, create a new one + userModel := &domain.UserModel{Email: info.Email, GithubID: &info.ID} + if createErr := s.userStore.Create(ctx, userModel); createErr != nil { + return nil, status.Error(codes.Internal, "failed to create user") + } + return userModel, nil + default: + return nil, status.Error(codes.Internal, "failed to fetch user by email") } - return userModel, nil - default: - return nil, status.Error(codes.Internal, "failed to fetch user by email") } + // No email provided, create a new user without email + userModel := &domain.UserModel{Email: "", GithubID: &info.ID} + if createErr := s.userStore.Create(ctx, userModel); createErr != nil { + return nil, status.Error(codes.Internal, "failed to create user") + } + return userModel, nil default: return nil, status.Error(codes.Internal, "failed to fetch user by provider id") } diff --git a/internal/application/identra/service_ensure_oauth_user_test.go b/internal/application/identra/service_ensure_oauth_user_test.go new file mode 100644 index 0000000..be50d2f --- /dev/null +++ b/internal/application/identra/service_ensure_oauth_user_test.go @@ -0,0 +1,183 @@ +package identra + +import ( + "context" + "testing" + + "github.com/poly-workshop/identra/internal/domain" +) + +// mockUserStore is a simple in-memory user store for testing. +type mockUserStore struct { + users map[string]*domain.UserModel +} + +func newMockUserStore() *mockUserStore { + return &mockUserStore{ + users: make(map[string]*domain.UserModel), + } +} + +func (m *mockUserStore) Create(ctx context.Context, user *domain.UserModel) error { + if user.ID == "" { + user.ID = "test-user-id" + } + m.users[user.ID] = user + return nil +} + +func (m *mockUserStore) GetByID(ctx context.Context, id string) (*domain.UserModel, error) { + if user, ok := m.users[id]; ok { + return user, nil + } + return nil, domain.ErrNotFound +} + +func (m *mockUserStore) GetByEmail(ctx context.Context, email string) (*domain.UserModel, error) { + for _, user := range m.users { + if user.Email == email { + return user, nil + } + } + return nil, domain.ErrNotFound +} + +func (m *mockUserStore) GetByGithubID(ctx context.Context, githubID string) (*domain.UserModel, error) { + for _, user := range m.users { + if user.GithubID != nil && *user.GithubID == githubID { + return user, nil + } + } + return nil, domain.ErrNotFound +} + +func (m *mockUserStore) Update(ctx context.Context, user *domain.UserModel) error { + if _, ok := m.users[user.ID]; !ok { + return domain.ErrNotFound + } + m.users[user.ID] = user + return nil +} + +func (m *mockUserStore) Delete(ctx context.Context, id string) error { + if _, ok := m.users[id]; !ok { + return domain.ErrNotFound + } + delete(m.users, id) + return nil +} + +func (m *mockUserStore) List(ctx context.Context, offset, limit int) ([]*domain.UserModel, error) { + result := make([]*domain.UserModel, 0, len(m.users)) + for _, user := range m.users { + result = append(result, user) + } + return result, nil +} + +func (m *mockUserStore) Count(ctx context.Context) (int64, error) { + return int64(len(m.users)), nil +} + +func TestEnsureOAuthUser_WithEmail(t *testing.T) { + store := newMockUserStore() + svc := &Service{userStore: store} + + info := UserInfo{ + ID: "github123", + Email: "user@example.com", + } + + user, err := svc.ensureOAuthUser(context.Background(), info) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + if user.Email != "user@example.com" { + t.Errorf("expected email 'user@example.com', got %q", user.Email) + } + if user.GithubID == nil || *user.GithubID != "github123" { + t.Errorf("expected GithubID 'github123', got %v", user.GithubID) + } +} + +func TestEnsureOAuthUser_WithoutEmail(t *testing.T) { + store := newMockUserStore() + svc := &Service{userStore: store} + + info := UserInfo{ + ID: "github456", + Email: "", + } + + user, err := svc.ensureOAuthUser(context.Background(), info) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + if user.Email != "" { + t.Errorf("expected empty email, got %q", user.Email) + } + if user.GithubID == nil || *user.GithubID != "github456" { + t.Errorf("expected GithubID 'github456', got %v", user.GithubID) + } +} + +func TestEnsureOAuthUser_ExistingUserByGithubID(t *testing.T) { + store := newMockUserStore() + svc := &Service{userStore: store} + + githubID := "github789" + existingUser := &domain.UserModel{ + ID: "existing-user-id", + Email: "existing@example.com", + GithubID: &githubID, + } + _ = store.Create(context.Background(), existingUser) + + info := UserInfo{ + ID: "github789", + Email: "different@example.com", + } + + user, err := svc.ensureOAuthUser(context.Background(), info) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + if user.ID != existingUser.ID { + t.Errorf("expected existing user ID, got %q", user.ID) + } + // Email should be updated + if user.Email != "different@example.com" { + t.Errorf("expected email to be updated to 'different@example.com', got %q", user.Email) + } +} + +func TestEnsureOAuthUser_LinkExistingUserByEmail(t *testing.T) { + store := newMockUserStore() + svc := &Service{userStore: store} + + existingUser := &domain.UserModel{ + ID: "existing-user-id", + Email: "existing@example.com", + } + _ = store.Create(context.Background(), existingUser) + + info := UserInfo{ + ID: "github999", + Email: "existing@example.com", + } + + user, err := svc.ensureOAuthUser(context.Background(), info) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + if user.ID != existingUser.ID { + t.Errorf("expected existing user ID, got %q", user.ID) + } + if user.GithubID == nil || *user.GithubID != "github999" { + t.Errorf("expected GithubID to be linked to 'github999', got %v", user.GithubID) + } +} diff --git a/internal/domain/user.go b/internal/domain/user.go index 9ba7fb0..0a281ed 100644 --- a/internal/domain/user.go +++ b/internal/domain/user.go @@ -18,7 +18,7 @@ type UserModel struct { CreatedAt time.Time `bson:"created_at,omitempty" json:"created_at"` UpdatedAt time.Time `bson:"updated_at,omitempty" json:"updated_at"` DeletedAt gorm.DeletedAt `gorm:"index" bson:"-" json:"-"` - Email string `gorm:"type:varchar(255);uniqueIndex;not null" bson:"email" json:"email"` + Email string `gorm:"type:varchar(255);uniqueIndex" bson:"email" json:"email"` HashedPassword *string `gorm:"column:hashed_password" bson:"hashed_password,omitempty" json:"-"` VerificationHash *string `gorm:"column:hash" bson:"hash,omitempty" json:"-"` GithubID *string `gorm:"column:github_id;unique" bson:"github_id,omitempty" json:"github_id"` diff --git a/internal/infrastructure/persistence/mongo_user_store.go b/internal/infrastructure/persistence/mongo_user_store.go index 6ec5bde..493e0e9 100644 --- a/internal/infrastructure/persistence/mongo_user_store.go +++ b/internal/infrastructure/persistence/mongo_user_store.go @@ -48,7 +48,7 @@ func (r *mongoUserStore) ensureIndexes(ctx context.Context) error { models := []mongo.IndexModel{ { Keys: bson.D{{Key: "email", Value: 1}}, - Options: options.Index().SetUnique(true).SetName("idx_email_unique"), + Options: options.Index().SetUnique(true).SetSparse(true).SetName("idx_email_unique"), }, { Keys: bson.D{{Key: "github_id", Value: 1}}, From a4c1d13fd8d1bb051a07e403af24759d9f40472f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Feb 2026 15:57:07 +0000 Subject: [PATCH 3/4] Remove redundant empty string assignment for email Co-authored-by: slhmy <31381093+slhmy@users.noreply.github.com> --- internal/application/identra/service.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/application/identra/service.go b/internal/application/identra/service.go index 2a9d57a..7bcc42e 100644 --- a/internal/application/identra/service.go +++ b/internal/application/identra/service.go @@ -646,8 +646,8 @@ func (s *Service) ensureOAuthUser(ctx context.Context, info UserInfo) (*domain.U return nil, status.Error(codes.Internal, "failed to fetch user by email") } } - // No email provided, create a new user without email - userModel := &domain.UserModel{Email: "", GithubID: &info.ID} + // No email provided, create a new user without email (GitHub ID only) + userModel := &domain.UserModel{GithubID: &info.ID} if createErr := s.userStore.Create(ctx, userModel); createErr != nil { return nil, status.Error(codes.Internal, "failed to create user") } From dc2f3206361f39fbb1e14328edcbe0db7ba7a1d5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Feb 2026 15:58:13 +0000 Subject: [PATCH 4/4] Add documentation for sparse index and clarify empty email intent Co-authored-by: slhmy <31381093+slhmy@users.noreply.github.com> --- internal/application/identra/service.go | 3 ++- internal/infrastructure/persistence/mongo_user_store.go | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/application/identra/service.go b/internal/application/identra/service.go index 7bcc42e..6b75a08 100644 --- a/internal/application/identra/service.go +++ b/internal/application/identra/service.go @@ -646,7 +646,8 @@ func (s *Service) ensureOAuthUser(ctx context.Context, info UserInfo) (*domain.U return nil, status.Error(codes.Internal, "failed to fetch user by email") } } - // No email provided, create a new user without email (GitHub ID only) + // No email provided from OAuth provider, create user with GitHub ID only. + // Email is intentionally left as empty string (default value). userModel := &domain.UserModel{GithubID: &info.ID} if createErr := s.userStore.Create(ctx, userModel); createErr != nil { return nil, status.Error(codes.Internal, "failed to create user") diff --git a/internal/infrastructure/persistence/mongo_user_store.go b/internal/infrastructure/persistence/mongo_user_store.go index 493e0e9..dc0d7a0 100644 --- a/internal/infrastructure/persistence/mongo_user_store.go +++ b/internal/infrastructure/persistence/mongo_user_store.go @@ -47,7 +47,10 @@ func NewMongoUserStore( func (r *mongoUserStore) ensureIndexes(ctx context.Context) error { models := []mongo.IndexModel{ { - Keys: bson.D{{Key: "email", Value: 1}}, + Keys: bson.D{{Key: "email", Value: 1}}, + // Sparse index allows multiple documents with NULL/empty email values + // while maintaining uniqueness constraint for non-empty values. + // This enables OAuth users without email to be created. Options: options.Index().SetUnique(true).SetSparse(true).SetName("idx_email_unique"), }, {