fix(user): harden account email and password handling

- normalize emails (trim + lowercase) and enforce uniqueness across registration, OAuth auto-registration, and email binding - serialize concurrent writers on the same normalized email within a transaction to avoid duplicate accounts - resolve password reset to a single matching account and reject ambiguous or absent matches - require an existing password before self-service password change and reject login for accounts without a usable password
2026-07-05 13:15:41 +08:00
parent 1ae757475f
commit 5fc35e28a2
10 changed files with 416 additions and 102 deletions
@@ -11,6 +11,9 @@ var (
 var (
 	ErrInvalidCredentials   = errors.New("invalid credentials")
 	ErrUserEmptyCredentials = errors.New("empty credentials")
+	ErrEmailAlreadyTaken    = errors.New("email already taken")
+	ErrEmailNotFound        = errors.New("email not found")
+	ErrEmailAmbiguous       = errors.New("email matches multiple users")
 )

 // Token auth errors
@@ -183,10 +183,11 @@ func CheckUserExistOrDeleted(username string, email string) (bool, error) {
 	// err := DB.Unscoped().First(&user, "username = ? or email = ?", username, email).Error
 	// check email if empty
 	var err error
+	email = NormalizeEmail(email)
 	if email == "" {
 		err = DB.Unscoped().First(&user, "username = ?", username).Error
 	} else {
-		err = DB.Unscoped().First(&user, "username = ? or email = ?", username, email).Error
+		err = DB.Unscoped().First(&user, "username = ? or LOWER(email) = ?", username, email).Error
 	}
 	if err != nil {
 		if errors.Is(err, gorm.ErrRecordNotFound) {
@@ -200,6 +201,85 @@ func CheckUserExistOrDeleted(username string, email string) (bool, error) {
 	return true, nil
 }

+func NormalizeEmail(email string) string {
+	return strings.ToLower(strings.TrimSpace(email))
+}
+
+func emailQuery(tx *gorm.DB, email string) *gorm.DB {
+	if tx == nil {
+		tx = DB
+	}
+	return tx.Unscoped().Model(&User{}).Where("LOWER(email) = ?", NormalizeEmail(email))
+}
+
+func CountUsersByEmail(email string) (int64, error) {
+	email = NormalizeEmail(email)
+	if email == "" {
+		return 0, nil
+	}
+	var count int64
+	err := emailQuery(DB, email).Count(&count).Error
+	return count, err
+}
+
+func IsEmailAvailable(email string, excludeUserID int) (bool, error) {
+	email = NormalizeEmail(email)
+	if email == "" {
+		return true, nil
+	}
+	query := emailQuery(DB, email)
+	if excludeUserID > 0 {
+		query = query.Where("id <> ?", excludeUserID)
+	}
+	var count int64
+	if err := query.Count(&count).Error; err != nil {
+		return false, err
+	}
+	return count == 0, nil
+}
+
+func EnsureEmailAvailable(email string, excludeUserID int) error {
+	available, err := IsEmailAvailable(email, excludeUserID)
+	if err != nil {
+		return err
+	}
+	if !available {
+		return ErrEmailAlreadyTaken
+	}
+	return nil
+}
+
+// withNormalizedEmailLock serializes concurrent writers that target the same
+// normalized email inside tx, so a "check then write" sequence cannot be raced
+// by two transactions. It must be called inside an active transaction; the lock
+// is scoped to that transaction and released on commit/rollback.
+//
+//   - PostgreSQL: transaction-level advisory lock keyed by the normalized email.
+//   - MySQL (default REPEATABLE READ): a locking read that takes a next-key/gap
+//     lock on the email index, blocking concurrent inserts of the same value.
+//   - SQLite: no explicit lock; the single-writer model already serializes the
+//     write, so a racing second write fails instead of duplicating.
+//
+// An empty email is allowed to repeat and needs no serialization.
+func withNormalizedEmailLock(tx *gorm.DB, email string, fn func(tx *gorm.DB) error) error {
+	email = NormalizeEmail(email)
+	if email == "" {
+		return fn(tx)
+	}
+	switch {
+	case common.UsingMainDatabase(common.DatabaseTypePostgreSQL):
+		if err := tx.Exec("SELECT pg_advisory_xact_lock(hashtext(?))", email).Error; err != nil {
+			return err
+		}
+	case common.UsingMainDatabase(common.DatabaseTypeMySQL):
+		var ids []int
+		if err := tx.Raw("SELECT id FROM users WHERE email = ? FOR UPDATE", email).Scan(&ids).Error; err != nil {
+			return err
+		}
+	}
+	return fn(tx)
+}
+
 func GetMaxUserId() int {
 	var user User
 	DB.Unscoped().Last(&user)
@@ -399,28 +479,77 @@ func (user *User) TransferAffQuotaToQuota(quota int) error {
 	return tx.Commit().Error
 }

-func (user *User) Insert(inviterId int) error {
+func (user *User) prepareForInsert(tx *gorm.DB) error {
+	user.Email = NormalizeEmail(user.Email)
+	if err := ensureEmailAvailableWithTx(tx, user.Email, 0); err != nil {
+		return err
+	}
+	if user.Password == "" {
+		return nil
+	}
 	var err error
-	if user.Password != "" {
-		user.Password, err = common.Password2Hash(user.Password)
-		if err != nil {
-			return err
-		}
-	}
-	user.Quota = common.QuotaForNewUser
-	//user.SetAccessToken(common.GetUUID())
-	user.AffCode = common.GetRandomString(4)
+	user.Password, err = common.Password2Hash(user.Password)
+	return err
+}

-	// 初始化用户设置，包括默认的边栏配置
-	if user.Setting == "" {
-		defaultSetting := dto.UserSetting{}
-		// 这里暂时不设置SidebarModules，因为需要在用户创建后根据角色设置
-		user.SetSetting(defaultSetting)
+// BindEmailToUser atomically checks email availability and assigns it to the
+// user, serializing concurrent binds of the same email so two accounts cannot
+// end up sharing one address. The email is normalized before check and store.
+func BindEmailToUser(user *User, email string) error {
+	email = NormalizeEmail(email)
+	if err := DB.Transaction(func(tx *gorm.DB) error {
+		return withNormalizedEmailLock(tx, email, func(tx *gorm.DB) error {
+			if err := ensureEmailAvailableWithTx(tx, email, user.Id); err != nil {
+				return err
+			}
+			user.Email = email
+			return user.UpdateWithTx(tx, false)
+		})
+	}); err != nil {
+		return err
 	}
+	return updateUserCache(*user)
+}

-	result := DB.Create(user)
-	if result.Error != nil {
-		return result.Error
+func ensureEmailAvailableWithTx(tx *gorm.DB, email string, excludeUserID int) error {
+	email = NormalizeEmail(email)
+	if email == "" {
+		return nil
+	}
+	query := emailQuery(tx, email)
+	if excludeUserID > 0 {
+		query = query.Where("id <> ?", excludeUserID)
+	}
+	var count int64
+	if err := query.Count(&count).Error; err != nil {
+		return err
+	}
+	if count > 0 {
+		return ErrEmailAlreadyTaken
+	}
+	return nil
+}
+
+func (user *User) Insert(inviterId int) error {
+	if err := DB.Transaction(func(tx *gorm.DB) error {
+		return withNormalizedEmailLock(tx, user.Email, func(tx *gorm.DB) error {
+			if err := user.prepareForInsert(tx); err != nil {
+				return err
+			}
+			user.Quota = common.QuotaForNewUser
+			user.AffCode = common.GetRandomString(4)
+
+			// 初始化用户设置，包括默认的边栏配置
+			if user.Setting == "" {
+				defaultSetting := dto.UserSetting{}
+				// 这里暂时不设置SidebarModules，因为需要在用户创建后根据角色设置
+				user.SetSetting(defaultSetting)
+			}
+
+			return tx.Create(user).Error
+		})
+	}); err != nil {
+		return err
 	}

 	user.finishInsert(inviterId)
@@ -467,28 +596,21 @@ func (user *User) FinishInsert(inviterId int) {
 // This is used for OAuth registration where user creation and binding need to be atomic.
 // Post-creation tasks (sidebar config, logs, inviter rewards) are handled after the transaction commits.
 func (user *User) InsertWithTx(tx *gorm.DB, inviterId int) error {
-	var err error
-	if user.Password != "" {
-		user.Password, err = common.Password2Hash(user.Password)
-		if err != nil {
+	return withNormalizedEmailLock(tx, user.Email, func(tx *gorm.DB) error {
+		if err := user.prepareForInsert(tx); err != nil {
 			return err
 		}
-	}
-	user.Quota = common.QuotaForNewUser
-	user.AffCode = common.GetRandomString(4)
+		user.Quota = common.QuotaForNewUser
+		user.AffCode = common.GetRandomString(4)

-	// 初始化用户设置
-	if user.Setting == "" {
-		defaultSetting := dto.UserSetting{}
-		user.SetSetting(defaultSetting)
-	}
+		// 初始化用户设置
+		if user.Setting == "" {
+			defaultSetting := dto.UserSetting{}
+			user.SetSetting(defaultSetting)
+		}

-	result := tx.Create(user)
-	if result.Error != nil {
-		return result.Error
-	}
-
-	return nil
+		return tx.Create(user).Error
+	})
 }

 // FinalizeOAuthUserCreation performs post-transaction tasks for OAuth user creation.
@@ -658,6 +780,9 @@ func (user *User) ValidateAndFill() (err error) {
 		}
 		return fmt.Errorf("%w: %v", ErrDatabase, err)
 	}
+	if user.Password == "" {
+		return ErrInvalidCredentials
+	}
 	okay := common.ValidatePasswordAndHash(password, user.Password)
 	if !okay || user.Status != common.UserStatusEnabled {
 		return ErrInvalidCredentials
@@ -733,7 +858,27 @@ func (user *User) FillUserByTelegramId() error {
 }

 func IsEmailAlreadyTaken(email string) bool {
-	return DB.Unscoped().Where("email = ?", email).Find(&User{}).RowsAffected == 1
+	count, err := CountUsersByEmail(email)
+	return err == nil && count > 0
+}
+
+func GetUniqueUserByEmail(email string) (*User, error) {
+	email = NormalizeEmail(email)
+	if email == "" {
+		return nil, ErrEmailNotFound
+	}
+	var users []User
+	if err := DB.Where("LOWER(email) = ?", email).Limit(2).Find(&users).Error; err != nil {
+		return nil, err
+	}
+	switch len(users) {
+	case 0:
+		return nil, ErrEmailNotFound
+	case 1:
+		return &users[0], nil
+	default:
+		return nil, ErrEmailAmbiguous
+	}
 }

 func IsWeChatIdAlreadyTaken(wechatId string) bool {
@@ -760,11 +905,15 @@ func ResetUserPasswordByEmail(email string, password string) error {
 	if email == "" || password == "" {
 		return errors.New("邮箱地址或密码为空！")
 	}
+	user, err := GetUniqueUserByEmail(email)
+	if err != nil {
+		return err
+	}
 	hashedPassword, err := common.Password2Hash(password)
 	if err != nil {
 		return err
 	}
-	err = DB.Model(&User{}).Where("email = ?", email).Update("password", hashedPassword).Error
+	err = DB.Model(&User{}).Where("id = ?", user.Id).Update("password", hashedPassword).Error
 	return err
 }

@@ -781,36 +930,6 @@ func IsAdmin(userId int) bool {
 	return user.Role >= common.RoleAdminUser
 }

-//// IsUserEnabled checks user status from Redis first, falls back to DB if needed
-//func IsUserEnabled(id int, fromDB bool) (status bool, err error) {
-//	defer func() {
-//		// Update Redis cache asynchronously on successful DB read
-//		if shouldUpdateRedis(fromDB, err) {
-//			gopool.Go(func() {
-//				if err := updateUserStatusCache(id, status); err != nil {
-//					common.SysError("failed to update user status cache: " + err.Error())
-//				}
-//			})
-//		}
-//	}()
-//	if !fromDB && common.RedisEnabled {
-//		// Try Redis first
-//		status, err := getUserStatusCache(id)
-//		if err == nil {
-//			return status == common.UserStatusEnabled, nil
-//		}
-//		// Don't return error - fall through to DB
-//	}
-//	fromDB = true
-//	var user User
-//	err = DB.Where("id = ?", id).Select("status").Find(&user).Error
-//	if err != nil {
-//		return false, err
-//	}
-//
-//	return user.Status == common.UserStatusEnabled, nil
-//}
-
 func ValidateAccessToken(token string) (*User, error) {
 	if token == "" {
 		return nil, nil
@@ -1,6 +1,7 @@
 package model

 import (
+	"errors"
 	"testing"

 	"github.com/QuantumNous/new-api/common"
@@ -90,3 +91,130 @@ func TestUpdateUserSettingOnlyUpdatesSetting(t *testing.T) {
 	assert.Equal(t, 4, got.RequestCount)
 	assert.Equal(t, "zh", got.GetSetting().Language)
 }
+
+func TestEnsureEmailAvailableRejectsExistingEmailCaseInsensitive(t *testing.T) {
+	setupUserUpdateTestState(t)
+
+	require.NoError(t, DB.Create(&User{
+		Username: "existing",
+		Password: "old-password",
+		Email:    "Taken@Example.com",
+		Status:   common.UserStatusEnabled,
+	}).Error)
+
+	err := EnsureEmailAvailable(" taken@example.COM ", 0)
+	require.ErrorIs(t, err, ErrEmailAlreadyTaken)
+
+	user, err := GetUniqueUserByEmail("TAKEN@example.com")
+	require.NoError(t, err)
+	assert.Equal(t, "existing", user.Username)
+
+	require.NoError(t, EnsureEmailAvailable("taken@example.com", user.Id))
+}
+
+func TestInsertRejectsDuplicateEmailWithoutUniqueIndex(t *testing.T) {
+	setupUserUpdateTestState(t)
+
+	require.NoError(t, DB.Create(&User{
+		Username: "existing",
+		Password: "old-password",
+		Email:    "taken@example.com",
+		Status:   common.UserStatusEnabled,
+	}).Error)
+
+	user := &User{
+		Username: "oauth-user",
+		Email:    "TAKEN@example.com",
+		Role:     common.RoleCommonUser,
+		Status:   common.UserStatusEnabled,
+	}
+
+	err := user.Insert(0)
+	require.ErrorIs(t, err, ErrEmailAlreadyTaken)
+
+	var count int64
+	require.NoError(t, DB.Model(&User{}).Where("username = ?", "oauth-user").Count(&count).Error)
+	assert.Zero(t, count)
+}
+
+func TestInsertKeepsBlankPasswordForPasswordlessUser(t *testing.T) {
+	setupUserUpdateTestState(t)
+
+	user := &User{
+		Username: "passwordless-user",
+		Role:     common.RoleCommonUser,
+		Status:   common.UserStatusEnabled,
+	}
+
+	require.NoError(t, user.Insert(0))
+
+	var stored User
+	require.NoError(t, DB.Where("username = ?", user.Username).First(&stored).Error)
+	assert.Empty(t, stored.Password)
+}
+
+func TestValidateAndFillRejectsPasswordlessUser(t *testing.T) {
+	setupUserUpdateTestState(t)
+
+	require.NoError(t, DB.Create(&User{
+		Username: "passwordless-user",
+		Password: "",
+		Status:   common.UserStatusEnabled,
+	}).Error)
+
+	loginUser := User{
+		Username: "passwordless-user",
+		Password: "NewPassword123",
+	}
+	err := loginUser.ValidateAndFill()
+	require.ErrorIs(t, err, ErrInvalidCredentials)
+
+	var stored User
+	require.NoError(t, DB.Where("username = ?", "passwordless-user").First(&stored).Error)
+	assert.Empty(t, stored.Password)
+}
+
+func TestResetUserPasswordByEmailRequiresSingleActiveMatch(t *testing.T) {
+	setupUserUpdateTestState(t)
+
+	require.NoError(t, DB.Create(&User{
+		Username: "duplicate-1",
+		Password: "old-1",
+		Email:    "legacy@example.com",
+		AffCode:  "dupe1",
+		Status:   common.UserStatusEnabled,
+	}).Error)
+	require.NoError(t, DB.Create(&User{
+		Username: "duplicate-2",
+		Password: "old-2",
+		Email:    "LEGACY@example.com",
+		AffCode:  "dupe2",
+		Status:   common.UserStatusEnabled,
+	}).Error)
+
+	err := ResetUserPasswordByEmail("legacy@example.com", "NewPassword123")
+	require.ErrorIs(t, err, ErrEmailAmbiguous)
+
+	var duplicates []User
+	require.NoError(t, DB.Where("LOWER(email) = ?", "legacy@example.com").Order("username asc").Find(&duplicates).Error)
+	require.Len(t, duplicates, 2)
+	assert.Equal(t, "old-1", duplicates[0].Password)
+	assert.Equal(t, "old-2", duplicates[1].Password)
+
+	require.NoError(t, DB.Create(&User{
+		Username: "unique",
+		Password: "old",
+		Email:    "unique@example.com",
+		AffCode:  "unique",
+		Status:   common.UserStatusEnabled,
+	}).Error)
+
+	require.NoError(t, ResetUserPasswordByEmail("UNIQUE@example.com", "NewPassword123"))
+
+	var unique User
+	require.NoError(t, DB.Where("username = ?", "unique").First(&unique).Error)
+	assert.True(t, common.ValidatePasswordAndHash("NewPassword123", unique.Password))
+
+	err = ResetUserPasswordByEmail("missing@example.com", "NewPassword123")
+	require.True(t, errors.Is(err, ErrEmailNotFound))
+}