diff --git a/internal/application/identra/service.go b/internal/application/identra/service.go index b159299..8474a5c 100644 --- a/internal/application/identra/service.go +++ b/internal/application/identra/service.go @@ -44,6 +44,7 @@ type Service struct { emailCodeStore cache.EmailCodeStore oauthStateStore oauth.StateStore userStore domain.UserStore + externalIdentityStore domain.ExternalIdentityStore userStoreCleanup func(context.Context) error keyManager *security.KeyManager tokenCfg security.TokenConfig @@ -100,7 +101,7 @@ func NewService(ctx context.Context, cfg Config) (*Service, error) { stateTTL = DefaultOAuthStateExpiration } - userStore, cleanup, storeErr := buildUserStore(ctx, cfg) + userStore, externalIdentityStore, cleanup, storeErr := buildStores(ctx, cfg) if storeErr != nil { return nil, storeErr } @@ -164,6 +165,7 @@ func NewService(ctx context.Context, cfg Config) (*Service, error) { return &Service{ userStore: userStore, + externalIdentityStore: externalIdentityStore, keyManager: km, tokenCfg: tokenCfg, oauthStateStore: oauthStore, @@ -409,34 +411,44 @@ func (s *Service) BindUserByOAuth( slog.ErrorContext(ctx, "failed to fetch user info (bind)", "error", err) return nil, status.Error(codes.Unauthenticated, "failed to fetch user info") } - s.maybeFillOAuthEmail(ctx, userProvider, token.AccessToken, &userInfo) + if userInfo.ID == "" { + slog.ErrorContext(ctx, "provider returned empty user id (bind)", "provider", stateData.Provider) + return nil, status.Error(codes.Internal, "provider returned empty user id") + } - providerUser, err := s.userStore.GetByGithubID(ctx, userInfo.ID) + providerIdentity, err := s.externalIdentityStore.GetByProviderID(ctx, stateData.Provider, userInfo.ID) switch { - case err == nil && providerUser.ID != bindingUser.ID: + case err == nil && providerIdentity.UserID != bindingUser.ID: return nil, status.Error(codes.AlreadyExists, "oauth account already linked to another user") case err == nil: - // Already linked to this user; continue to refresh token pair and email if needed. + // Already linked to this user; continue to refresh token pair. case errors.Is(err, domain.ErrNotFound): - // Not linked yet. + // Not linked yet; create the external identity. + identity := &domain.ExternalIdentityModel{ + UserID: bindingUser.ID, + Provider: stateData.Provider, + ProviderUserID: userInfo.ID, + } + if createErr := s.externalIdentityStore.Create(ctx, identity); createErr != nil { + if errors.Is(createErr, domain.ErrAlreadyExists) { + // A concurrent request may have created the same identity. Re-fetch + // and treat as success if it is linked to this user. + existing, refetchErr := s.externalIdentityStore.GetByProviderID(ctx, stateData.Provider, userInfo.ID) + if refetchErr != nil { + return nil, status.Error(codes.Internal, "failed to verify oauth link") + } + if existing.UserID != bindingUser.ID { + return nil, status.Error(codes.AlreadyExists, "oauth account already linked to another user") + } + // Idempotent: identity exists and belongs to this user. + } else { + return nil, status.Error(codes.Internal, "failed to link oauth account") + } + } default: return nil, status.Error(codes.Internal, "failed to check existing oauth link") } - if bindingUser.GithubID == nil || *bindingUser.GithubID == "" { - bindingUser.GithubID = &userInfo.ID - } else if *bindingUser.GithubID != userInfo.ID { - return nil, status.Error(codes.FailedPrecondition, "user already linked to another oauth account") - } - - if _, err := s.updateEmailIfNeeded(ctx, bindingUser, userInfo.Email); err != nil { - return nil, err - } - - if err := s.userStore.Update(ctx, bindingUser); err != nil { - return nil, status.Error(codes.Internal, "failed to link oauth account") - } - s.recordLogin(ctx, bindingUser) tokenPair, err := security.NewTokenPair(bindingUser.ID, s.tokenCfg) if err != nil { @@ -789,12 +801,23 @@ func (s *Service) GetCurrentUserLoginInfo( PasswordEnabled: usr.HashedPassword != nil && strings.TrimSpace(*usr.HashedPassword) != "", } - if usr.GithubID != nil && strings.TrimSpace(*usr.GithubID) != "" { - resp.GithubId = usr.GithubID - resp.OauthConnections = append(resp.OauthConnections, &identra_v1_pb.OAuthConnection{ - Provider: "github", - ProviderUserId: *usr.GithubID, - }) + identities, err := s.externalIdentityStore.GetByUserID(ctx, usr.ID) + if err != nil { + slog.WarnContext(ctx, "failed to fetch external identities", "error", err, "user_id", usr.ID) + } else { + var githubID string + for _, identity := range identities { + resp.OauthConnections = append(resp.OauthConnections, &identra_v1_pb.OAuthConnection{ + Provider: identity.Provider, + ProviderUserId: identity.ProviderUserID, + }) + if identity.Provider == "github" && (githubID == "" || identity.ProviderUserID < githubID) { + githubID = identity.ProviderUserID + } + } + if githubID != "" { + resp.GithubId = &githubID + } } return resp, nil @@ -804,41 +827,85 @@ func (s *Service) ensureOAuthUser(ctx context.Context, info UserInfo) (*domain.U if info.ID == "" { return nil, status.Error(codes.Internal, "provider user id is empty") } + if info.Provider == "" { + return nil, status.Error(codes.Internal, "provider name is empty") + } - existing, err := s.userStore.GetByGithubID(ctx, info.ID) + existing, err := s.externalIdentityStore.GetByProviderID(ctx, info.Provider, info.ID) switch { case err == nil: - return s.updateEmailIfNeeded(ctx, existing, info.Email) + // External identity already exists; fetch and return the linked user. + usr, userErr := s.userStore.GetByID(ctx, existing.UserID) + if userErr != nil { + return nil, status.Error(codes.Internal, "failed to fetch user linked to oauth identity") + } + return s.updateEmailIfNeeded(ctx, usr, info.Email) case errors.Is(err, domain.ErrNotFound): - // 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.) + // No existing external identity; try to link by email if available. 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") + // Merge: link the external identity to the existing user. + identity := &domain.ExternalIdentityModel{ + UserID: byEmail.ID, + Provider: info.Provider, + ProviderUserID: info.ID, + } + if createErr := s.externalIdentityStore.Create(ctx, identity); createErr != nil { + if errors.Is(createErr, domain.ErrAlreadyExists) { + // A concurrent request may have created the same identity for + // this user. Re-fetch and proceed if it belongs to the same user. + existingIdentity, getErr := s.externalIdentityStore.GetByProviderID(ctx, info.Provider, info.ID) + if getErr != nil { + return nil, status.Error(codes.Internal, "failed to verify oauth account link") + } + if existingIdentity.UserID == byEmail.ID { + return byEmail, nil + } + return nil, status.Error(codes.AlreadyExists, "oauth account already linked to another user") + } + return nil, status.Error(codes.Internal, "failed to link oauth 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} + // Create new user and link external identity. + userModel := &domain.UserModel{Email: info.Email} if createErr := s.userStore.Create(ctx, userModel); createErr != nil { + if errors.Is(createErr, domain.ErrAlreadyExists) { + return nil, status.Error(codes.AlreadyExists, "user already exists") + } return nil, status.Error(codes.Internal, "failed to create user") } + identity := &domain.ExternalIdentityModel{ + UserID: userModel.ID, + Provider: info.Provider, + ProviderUserID: info.ID, + } + if createErr := s.externalIdentityStore.Create(ctx, identity); createErr != nil { + // Determine the response code before attempting cleanup so that the + // cleanup outcome does not affect the error returned to the caller. + isConflict := errors.Is(createErr, domain.ErrAlreadyExists) + // Compensate: remove the newly created user to avoid orphaned records. + if deleteErr := s.userStore.Delete(ctx, userModel.ID); deleteErr != nil { + slog.ErrorContext(ctx, "failed to clean up orphaned user after identity create failure", + "error", deleteErr, "user_id", userModel.ID) + } + if isConflict { + return nil, status.Error(codes.AlreadyExists, "oauth account already linked") + } + return nil, status.Error(codes.Internal, "failed to create oauth identity") + } return userModel, nil default: return nil, status.Error(codes.Internal, "failed to fetch user by email") } } - // 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") - } - return userModel, nil + // The current persistence layer enforces unique email values, so creating + // a user without an email address can lead to duplicate empty-email + // records and subsequent signup failures. Reject this flow until missing + // emails are represented distinctly at the model/index level. + return nil, status.Error(codes.FailedPrecondition, "oauth provider did not supply an email address") default: return nil, status.Error(codes.Internal, "failed to fetch user by provider id") } @@ -884,41 +951,49 @@ func (s *Service) maybeFillOAuthEmail(ctx context.Context, provider UserInfoProv info.Email = strings.TrimSpace(email) } -func buildUserStore(ctx context.Context, cfg Config) (domain.UserStore, func(context.Context) error, error) { +func buildStores(ctx context.Context, cfg Config) (domain.UserStore, domain.ExternalIdentityStore, func(context.Context) error, error) { repoType := strings.ToLower(strings.TrimSpace(cfg.PersistenceType)) switch repoType { case "mongo", "mongodb": mongoCfg := cfg.MongoClient if strings.TrimSpace(mongoCfg.URI) == "" { - return nil, nil, fmt.Errorf("mongo uri is required when using mongo user repository") + return nil, nil, nil, fmt.Errorf("mongo uri is required when using mongo user repository") } if strings.TrimSpace(mongoCfg.Database) == "" { - return nil, nil, fmt.Errorf("mongo database is required when using mongo user repository") + return nil, nil, nil, fmt.Errorf("mongo database is required when using mongo user repository") } client, err := mongo.Connect(options.Client().ApplyURI(mongoCfg.URI)) if err != nil { - return nil, nil, fmt.Errorf("failed to connect to mongo: %w", err) + return nil, nil, nil, fmt.Errorf("failed to connect to mongo: %w", err) } - repo, repoErr := persistence.NewMongoUserStore(ctx, client, mongoCfg.Database, "users") + userStore, repoErr := persistence.NewMongoUserStore(ctx, client, mongoCfg.Database, "users") if repoErr != nil { _ = client.Disconnect(ctx) - return nil, nil, repoErr + return nil, nil, nil, repoErr + } + + extStore, extErr := persistence.NewMongoExternalIdentityStore(ctx, client, mongoCfg.Database, "external_identities") + if extErr != nil { + _ = client.Disconnect(ctx) + return nil, nil, nil, extErr } cleanup := func(cleanupCtx context.Context) error { return client.Disconnect(cleanupCtx) } - return repo, cleanup, nil + return userStore, extStore, cleanup, nil case "", "gorm", "postgres", "mysql", "sqlite": db := gorm.NewDB(*cfg.GORMClient) - if err := db.AutoMigrate(&domain.UserModel{}); err != nil { + if err := db.AutoMigrate(&domain.UserModel{}, &domain.ExternalIdentityModel{}); err != nil { slog.Error("failed to migrate database", "error", err) } - return persistence.NewGormUserStore(db), func(context.Context) error { return nil }, nil + userStore := persistence.NewGormUserStore(db) + extStore := persistence.NewGormExternalIdentityStore(db) + return userStore, extStore, func(context.Context) error { return nil }, nil default: - return nil, nil, fmt.Errorf("unsupported user repository type: %s", cfg.PersistenceType) + return nil, nil, nil, fmt.Errorf("unsupported user repository type: %s", cfg.PersistenceType) } } diff --git a/internal/application/identra/service_ensure_oauth_user_test.go b/internal/application/identra/service_ensure_oauth_user_test.go index 5181387..ebd8cf3 100644 --- a/internal/application/identra/service_ensure_oauth_user_test.go +++ b/internal/application/identra/service_ensure_oauth_user_test.go @@ -5,6 +5,8 @@ import ( "testing" "github.com/poly-workshop/identra/internal/domain" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) // mockUserStore is a simple in-memory user store for testing. @@ -46,15 +48,6 @@ func (m *mockUserStore) GetByEmail(ctx context.Context, email string) (*domain.U 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 @@ -83,13 +76,61 @@ func (m *mockUserStore) Count(ctx context.Context) (int64, error) { return int64(len(m.users)), nil } +// mockExternalIdentityStore is a simple in-memory external identity store for testing. +type mockExternalIdentityStore struct { + identities []*domain.ExternalIdentityModel + forceCreateErr error +} + +func newMockExternalIdentityStore() *mockExternalIdentityStore { + return &mockExternalIdentityStore{} +} + +func (m *mockExternalIdentityStore) Create(_ context.Context, identity *domain.ExternalIdentityModel) error { + if m.forceCreateErr != nil { + return m.forceCreateErr + } + if identity.ID == "" { + identity.ID = "test-identity-id" + } + m.identities = append(m.identities, identity) + return nil +} + +func (m *mockExternalIdentityStore) GetByProviderID( + _ context.Context, + provider, providerUserID string, +) (*domain.ExternalIdentityModel, error) { + for _, id := range m.identities { + if id.Provider == provider && id.ProviderUserID == providerUserID { + return id, nil + } + } + return nil, domain.ErrNotFound +} + +func (m *mockExternalIdentityStore) GetByUserID( + _ context.Context, + userID string, +) ([]*domain.ExternalIdentityModel, error) { + var result []*domain.ExternalIdentityModel + for _, id := range m.identities { + if id.UserID == userID { + result = append(result, id) + } + } + return result, nil +} + func TestEnsureOAuthUser_WithEmail(t *testing.T) { store := newMockUserStore() - svc := &Service{userStore: store} + extStore := newMockExternalIdentityStore() + svc := &Service{userStore: store, externalIdentityStore: extStore} info := UserInfo{ - ID: "github123", - Email: "user@example.com", + Provider: "github", + ID: "github123", + Email: "user@example.com", } user, err := svc.ensureOAuthUser(context.Background(), info) @@ -100,48 +141,64 @@ func TestEnsureOAuthUser_WithEmail(t *testing.T) { 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) + + // Verify external identity was created with correct fields. + id, lookupErr := extStore.GetByProviderID(context.Background(), "github", "github123") + if lookupErr != nil { + t.Fatalf("expected external identity to exist, got %v", lookupErr) + } + if id.UserID != user.ID { + t.Errorf("expected external identity user_id %q, got %q", user.ID, id.UserID) } } func TestEnsureOAuthUser_WithoutEmail(t *testing.T) { store := newMockUserStore() - svc := &Service{userStore: store} + extStore := newMockExternalIdentityStore() + svc := &Service{userStore: store, externalIdentityStore: extStore} info := UserInfo{ - ID: "github456", - Email: "", + Provider: "github", + ID: "github456", + Email: "", } - user, err := svc.ensureOAuthUser(context.Background(), info) - if err != nil { - t.Fatalf("expected no error, got %v", err) + _, err := svc.ensureOAuthUser(context.Background(), info) + if err == nil { + t.Fatal("expected error for missing email, got nil") } - - if user.Email != "" { - t.Errorf("expected empty email, got %q", user.Email) + if st, ok := status.FromError(err); !ok || st.Code() != codes.FailedPrecondition { + t.Errorf("expected FailedPrecondition, got %v", err) } - if user.GithubID == nil || *user.GithubID != "github456" { - t.Errorf("expected GithubID 'github456', got %v", user.GithubID) + + // No user should have been created. + count, _ := store.Count(context.Background()) + if count != 0 { + t.Errorf("expected no users after no-email rejection, got %d", count) } } -func TestEnsureOAuthUser_ExistingUserByGithubID(t *testing.T) { +func TestEnsureOAuthUser_ExistingUserByProviderID(t *testing.T) { store := newMockUserStore() - svc := &Service{userStore: store} + extStore := newMockExternalIdentityStore() + svc := &Service{userStore: store, externalIdentityStore: extStore} - githubID := "github789" existingUser := &domain.UserModel{ - ID: "existing-user-id", - Email: "existing@example.com", - GithubID: &githubID, + ID: "existing-user-id", + Email: "existing@example.com", } _ = store.Create(context.Background(), existingUser) + _ = extStore.Create(context.Background(), &domain.ExternalIdentityModel{ + ID: "eid1", + UserID: "existing-user-id", + Provider: "github", + ProviderUserID: "github789", + }) info := UserInfo{ - ID: "github789", - Email: "different@example.com", + Provider: "github", + ID: "github789", + Email: "different@example.com", } user, err := svc.ensureOAuthUser(context.Background(), info) @@ -152,7 +209,7 @@ func TestEnsureOAuthUser_ExistingUserByGithubID(t *testing.T) { if user.ID != existingUser.ID { t.Errorf("expected existing user ID, got %q", user.ID) } - // Email should be updated + // Email should be updated on the user. if user.Email != "different@example.com" { t.Errorf("expected email to be updated to 'different@example.com', got %q", user.Email) } @@ -160,7 +217,8 @@ func TestEnsureOAuthUser_ExistingUserByGithubID(t *testing.T) { func TestEnsureOAuthUser_LinkExistingUserByEmail(t *testing.T) { store := newMockUserStore() - svc := &Service{userStore: store} + extStore := newMockExternalIdentityStore() + svc := &Service{userStore: store, externalIdentityStore: extStore} existingUser := &domain.UserModel{ ID: "existing-user-id", @@ -169,8 +227,9 @@ func TestEnsureOAuthUser_LinkExistingUserByEmail(t *testing.T) { _ = store.Create(context.Background(), existingUser) info := UserInfo{ - ID: "github999", - Email: "existing@example.com", + Provider: "github", + ID: "github999", + Email: "existing@example.com", } user, err := svc.ensureOAuthUser(context.Background(), info) @@ -181,7 +240,45 @@ func TestEnsureOAuthUser_LinkExistingUserByEmail(t *testing.T) { 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) + + // Verify external identity was linked to the existing user. + id, lookupErr := extStore.GetByProviderID(context.Background(), "github", "github999") + if lookupErr != nil { + t.Fatalf("expected external identity to exist, got %v", lookupErr) + } + if id.UserID != existingUser.ID { + t.Errorf("expected external identity user_id %q, got %q", existingUser.ID, id.UserID) + } +} + +func TestEnsureOAuthUser_IdentityCreateFailure_OrphanedUserDeleted_WithEmail(t *testing.T) { + store := newMockUserStore() + extStore := newMockExternalIdentityStore() + extStore.forceCreateErr = domain.ErrAlreadyExists + svc := &Service{userStore: store, externalIdentityStore: extStore} + + info := UserInfo{ + Provider: "github", + ID: "github-fail", + Email: "new@example.com", + } + + _, err := svc.ensureOAuthUser(context.Background(), info) + if err == nil { + t.Fatal("expected error, got nil") + } + + st, ok := status.FromError(err) + if !ok { + t.Fatalf("expected gRPC status error, got %v", err) + } + if st.Code() != codes.AlreadyExists { + t.Fatalf("expected gRPC code %v, got %v", codes.AlreadyExists, st.Code()) + } + + // Verify the orphaned user was cleaned up. + count, _ := store.Count(context.Background()) + if count != 0 { + t.Errorf("expected no users after identity create failure, got %d", count) } } diff --git a/internal/application/identra/user_info_provider.go b/internal/application/identra/user_info_provider.go index 361c10f..5457443 100644 --- a/internal/application/identra/user_info_provider.go +++ b/internal/application/identra/user_info_provider.go @@ -8,6 +8,7 @@ import ( ) type UserInfo struct { + Provider string ID string Email string Username string @@ -40,6 +41,7 @@ func (g *GitHubUserInfoProvider) GetUserInfo(ctx context.Context, token string) return UserInfo{}, err } return UserInfo{ + Provider: "github", ID: fmt.Sprintf("%d", user.GetID()), Email: user.GetEmail(), Username: user.GetLogin(), diff --git a/internal/domain/external_identity.go b/internal/domain/external_identity.go new file mode 100644 index 0000000..cb29d6d --- /dev/null +++ b/internal/domain/external_identity.go @@ -0,0 +1,46 @@ +package domain + +import ( + "context" + "time" + + "github.com/google/uuid" + "gorm.io/gorm" +) + +// ExternalIdentityModel represents an OAuth provider identity linked to a user. +// Each row represents the binding (provider, provider_user_id) -> user_id. +type ExternalIdentityModel struct { + ID string `gorm:"type:varchar(36);primaryKey" bson:"_id,omitempty" json:"id"` + UserID string `gorm:"type:varchar(36);index" bson:"user_id" json:"user_id"` + Provider string `gorm:"type:varchar(50);uniqueIndex:idx_external_identities_provider_provider_user_id" bson:"provider" json:"provider"` + ProviderUserID string `gorm:"type:varchar(255);uniqueIndex:idx_external_identities_provider_provider_user_id" bson:"provider_user_id" json:"provider_user_id"` + 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:"-"` +} + +// TableName returns the database table name for ExternalIdentityModel. +func (ExternalIdentityModel) TableName() string { + return "external_identities" +} + +// BeforeCreate generates a UUID for the external identity before creating. +func (e *ExternalIdentityModel) BeforeCreate(_ *gorm.DB) error { + if e.ID == "" { + e.ID = uuid.New().String() + } + return nil +} + +// ExternalIdentityStore defines the interface for external identity persistence operations. +type ExternalIdentityStore interface { + // Create persists a new external identity. Returns ErrAlreadyExists if the + // (provider, provider_user_id) pair is already linked to a user. + Create(ctx context.Context, identity *ExternalIdentityModel) error + // GetByProviderID looks up a single external identity by (provider, provider_user_id). + // Returns ErrNotFound if no match exists. + GetByProviderID(ctx context.Context, provider, providerUserID string) (*ExternalIdentityModel, error) + // GetByUserID returns all external identities linked to the given user. + GetByUserID(ctx context.Context, userID string) ([]*ExternalIdentityModel, error) +} diff --git a/internal/domain/user.go b/internal/domain/user.go index 436de87..ff4c0ba 100644 --- a/internal/domain/user.go +++ b/internal/domain/user.go @@ -22,10 +22,9 @@ type UserModel struct { UpdatedAt time.Time `bson:"updated_at,omitempty" json:"updated_at"` DeletedAt gorm.DeletedAt `gorm:"index" bson:"-" json:"-"` 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"` - LastLoginAt *time.Time ` bson:"last_login_at,omitempty" json:"last_login_at"` + HashedPassword *string `gorm:"column:hashed_password" bson:"hashed_password,omitempty" json:"-"` + VerificationHash *string `gorm:"column:hash" bson:"hash,omitempty" json:"-"` + LastLoginAt *time.Time ` bson:"last_login_at,omitempty" json:"last_login_at"` } // TableName returns the database table name for UserModel. @@ -46,7 +45,6 @@ type UserStore interface { Create(ctx context.Context, user *UserModel) error GetByID(ctx context.Context, id string) (*UserModel, error) GetByEmail(ctx context.Context, email string) (*UserModel, error) - GetByGithubID(ctx context.Context, githubID string) (*UserModel, error) Update(ctx context.Context, user *UserModel) error Delete(ctx context.Context, id string) error List(ctx context.Context, offset, limit int) ([]*UserModel, error) diff --git a/internal/infrastructure/persistence/gorm_external_identity_store.go b/internal/infrastructure/persistence/gorm_external_identity_store.go new file mode 100644 index 0000000..8d3ecfb --- /dev/null +++ b/internal/infrastructure/persistence/gorm_external_identity_store.go @@ -0,0 +1,70 @@ +package persistence + +import ( + "context" + "log/slog" + + "github.com/poly-workshop/identra/internal/domain" + "gorm.io/gorm" +) + +type gormExternalIdentityStore struct { + db *gorm.DB +} + +// NewGormExternalIdentityStore creates a new GORM-based external identity store. +func NewGormExternalIdentityStore(db *gorm.DB) domain.ExternalIdentityStore { + return &gormExternalIdentityStore{db: db} +} + +func (r *gormExternalIdentityStore) Create( + ctx context.Context, + identity *domain.ExternalIdentityModel, +) error { + err := r.db.WithContext(ctx).Create(identity).Error + if err != nil { + slog.ErrorContext( + ctx, + "failed to create external identity", + "error", err, + "provider", identity.Provider, + "provider_user_id", identity.ProviderUserID, + ) + return wrapGormError(err) + } + slog.InfoContext( + ctx, + "external identity created successfully", + "id", identity.ID, + "user_id", identity.UserID, + "provider", identity.Provider, + ) + return nil +} + +func (r *gormExternalIdentityStore) GetByProviderID( + ctx context.Context, + provider, providerUserID string, +) (*domain.ExternalIdentityModel, error) { + var identity domain.ExternalIdentityModel + err := r.db.WithContext(ctx). + Where("provider = ? AND provider_user_id = ?", provider, providerUserID). + First(&identity).Error + if err != nil { + return nil, wrapGormError(err) + } + return &identity, nil +} + +func (r *gormExternalIdentityStore) GetByUserID( + ctx context.Context, + userID string, +) ([]*domain.ExternalIdentityModel, error) { + var identities []*domain.ExternalIdentityModel + err := r.db.WithContext(ctx).Where("user_id = ?", userID).Find(&identities).Error + if err != nil { + slog.ErrorContext(ctx, "failed to list external identities", "error", err, "user_id", userID) + return nil, err + } + return identities, nil +} diff --git a/internal/infrastructure/persistence/gorm_user_store.go b/internal/infrastructure/persistence/gorm_user_store.go index e34e0c1..1876441 100644 --- a/internal/infrastructure/persistence/gorm_user_store.go +++ b/internal/infrastructure/persistence/gorm_user_store.go @@ -69,18 +69,6 @@ func (r *gormUserStore) GetByEmail(ctx context.Context, email string) (*domain.U return &user, nil } -func (r *gormUserStore) GetByGithubID( - ctx context.Context, - githubID string, -) (*domain.UserModel, error) { - var user domain.UserModel - err := r.db.WithContext(ctx).Where("github_id = ?", githubID).First(&user).Error - if err != nil { - return nil, wrapGormError(err) - } - return &user, nil -} - func (r *gormUserStore) Update(ctx context.Context, user *domain.UserModel) error { err := r.db.WithContext(ctx).Save(user).Error if err != nil { diff --git a/internal/infrastructure/persistence/mongo_external_identity_store.go b/internal/infrastructure/persistence/mongo_external_identity_store.go new file mode 100644 index 0000000..fbb28d5 --- /dev/null +++ b/internal/infrastructure/persistence/mongo_external_identity_store.go @@ -0,0 +1,162 @@ +package persistence + +import ( + "context" + "errors" + "log/slog" + "strings" + "time" + + "github.com/google/uuid" + "github.com/poly-workshop/identra/internal/domain" + "go.mongodb.org/mongo-driver/v2/bson" + "go.mongodb.org/mongo-driver/v2/mongo" + "go.mongodb.org/mongo-driver/v2/mongo/options" +) + +type mongoExternalIdentityStore struct { + coll *mongo.Collection +} + +// NewMongoExternalIdentityStore builds a MongoDB-backed external identity store and ensures indexes. +func NewMongoExternalIdentityStore( + ctx context.Context, + client *mongo.Client, + databaseName string, + collectionName string, +) (domain.ExternalIdentityStore, error) { + if client == nil { + return nil, errors.New("mongo client is required") + } + if strings.TrimSpace(databaseName) == "" { + return nil, errors.New("mongo database name is required") + } + if strings.TrimSpace(collectionName) == "" { + collectionName = "external_identities" + } + + store := &mongoExternalIdentityStore{ + coll: client.Database(databaseName).Collection(collectionName), + } + if err := store.ensureIndexes(ctx); err != nil { + return nil, err + } + return store, nil +} + +func (r *mongoExternalIdentityStore) ensureIndexes(ctx context.Context) error { + models := []mongo.IndexModel{ + { + Keys: bson.D{ + {Key: "provider", Value: 1}, + {Key: "provider_user_id", Value: 1}, + }, + Options: options.Index().SetUnique(true).SetName("idx_provider_user_id_unique"), + }, + { + Keys: bson.D{ + {Key: "provider", Value: 1}, + {Key: "user_id", Value: 1}, + }, + Options: options.Index().SetUnique(true).SetName("idx_provider_user_id_per_user_unique"), + }, + { + Keys: bson.D{{Key: "user_id", Value: 1}}, + Options: options.Index().SetName("idx_user_id"), + }, + } + + if _, err := r.coll.Indexes().CreateMany(ctx, models); err != nil { + slog.WarnContext(ctx, "failed to ensure external identity indexes", "error", err) + return err + } + return nil +} + +func (r *mongoExternalIdentityStore) Create( + ctx context.Context, + identity *domain.ExternalIdentityModel, +) error { + now := time.Now().UTC() + if strings.TrimSpace(identity.ID) == "" { + identity.ID = uuid.New().String() + } + if identity.CreatedAt.IsZero() { + identity.CreatedAt = now + } + identity.UpdatedAt = now + + if _, err := r.coll.InsertOne(ctx, identity); err != nil { + slog.ErrorContext( + ctx, + "failed to create external identity (mongo)", + "error", err, + "provider", identity.Provider, + "provider_user_id", identity.ProviderUserID, + ) + if mongo.IsDuplicateKeyError(err) { + return domain.ErrAlreadyExists + } + return err + } + slog.InfoContext( + ctx, + "external identity created successfully (mongo)", + "id", identity.ID, + "user_id", identity.UserID, + "provider", identity.Provider, + ) + return nil +} + +func (r *mongoExternalIdentityStore) GetByProviderID( + ctx context.Context, + provider, providerUserID string, +) (*domain.ExternalIdentityModel, error) { + var identity domain.ExternalIdentityModel + err := r.coll.FindOne(ctx, bson.M{ + "provider": provider, + "provider_user_id": providerUserID, + }).Decode(&identity) + if err != nil { + if errors.Is(err, mongo.ErrNoDocuments) { + return nil, domain.ErrNotFound + } + slog.ErrorContext( + ctx, + "failed to fetch external identity (mongo)", + "error", err, + "provider", provider, + "provider_user_id", providerUserID, + ) + return nil, err + } + return &identity, nil +} + +func (r *mongoExternalIdentityStore) GetByUserID( + ctx context.Context, + userID string, +) ([]*domain.ExternalIdentityModel, error) { + cursor, err := r.coll.Find(ctx, bson.M{"user_id": userID}) + if err != nil { + slog.ErrorContext(ctx, "failed to list external identities (mongo)", "error", err, "user_id", userID) + return nil, err + } + defer func() { _ = cursor.Close(ctx) }() + + var identities []*domain.ExternalIdentityModel + for cursor.Next(ctx) { + var identity domain.ExternalIdentityModel + if decodeErr := cursor.Decode(&identity); decodeErr != nil { + _ = cursor.Close(ctx) + return nil, decodeErr + } + identities = append(identities, &identity) + } + + if err := cursor.Err(); err != nil { + return nil, err + } + return identities, nil +} diff --git a/internal/infrastructure/persistence/mongo_user_store.go b/internal/infrastructure/persistence/mongo_user_store.go index be90ef5..c6770b0 100644 --- a/internal/infrastructure/persistence/mongo_user_store.go +++ b/internal/infrastructure/persistence/mongo_user_store.go @@ -44,19 +44,32 @@ func NewMongoUserStore( return repo, nil } +func isIndexNotFoundError(err error) bool { + var cmdErr mongo.CommandError + return errors.As(err, &cmdErr) && cmdErr.Code == 27 +} + func (r *mongoUserStore) ensureIndexes(ctx context.Context) error { + // Drop the stale github_id index left over from the old schema, if present. + // Ignore only the expected "index not found" case so fresh deployments and + // upgraded ones both work, but fail fast for operational problems. + if err := r.coll.Indexes().DropOne(ctx, "idx_github_id_unique"); err != nil { + if isIndexNotFoundError(err) { + slog.DebugContext(ctx, "stale github_id index not present", "error", err) + } else { + slog.WarnContext(ctx, "failed to drop stale github_id index", "error", err) + return err + } + } + models := []mongo.IndexModel{ { 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. + // Sparse index excludes documents where the email field is absent. + // An empty string is still indexed, so service-layer validation must + // reject blank emails before writes reach this store. Options: options.Index().SetUnique(true).SetSparse(true).SetName("idx_email_unique"), }, - { - Keys: bson.D{{Key: "github_id", Value: 1}}, - Options: options.Index().SetUnique(true).SetSparse(true).SetName("idx_github_id_unique"), - }, } if _, err := r.coll.Indexes().CreateMany(ctx, models); err != nil { @@ -88,10 +101,6 @@ func (r *mongoUserStore) GetByEmail(ctx context.Context, email string) (*domain. return r.findOne(ctx, bson.M{"email": email}, "email", email) } -func (r *mongoUserStore) GetByGithubID(ctx context.Context, githubID string) (*domain.UserModel, error) { - return r.findOne(ctx, bson.M{"github_id": githubID}, "github_id", githubID) -} - func (r *mongoUserStore) Update(ctx context.Context, user *domain.UserModel) error { if strings.TrimSpace(user.ID) == "" { return errors.New("user id is required for update")