Unify user update methods

cmd/admin_user_change_password.go

@@ -4,13 +4,14 @@
 package cmd
 
 import (
-	"context"
 	"errors"
 	"fmt"
 
 	user_model "code.gitea.io/gitea/models/user"
-	pwd "code.gitea.io/gitea/modules/auth/password"
+	"code.gitea.io/gitea/modules/auth/password"
+	"code.gitea.io/gitea/modules/optional"
 	"code.gitea.io/gitea/modules/setting"
+	user_service "code.gitea.io/gitea/services/user"
 
 	"github.com/urfave/cli/v2"
 )
@@ -50,35 +51,32 @@ func runChangePassword(c *cli.Context) error {
 	if err := initDB(ctx); err != nil {
 		return err
 	}
-	if len(c.String("password")) < setting.MinPasswordLength {
-		return fmt.Errorf("Password is not long enough. Needs to be at least %d", setting.MinPasswordLength)
-	}
 
-	if !pwd.IsComplexEnough(c.String("password")) {
-		return errors.New("Password does not meet complexity requirements")
-	}
-	pwned, err := pwd.IsPwned(context.Background(), c.String("password"))
-	if err != nil {
-		return err
-	}
-	if pwned {
-		return errors.New("The password you chose is on a list of stolen passwords previously exposed in public data breaches. Please try again with a different password.\nFor more details, see https://haveibeenpwned.com/Passwords")
-	}
-	uname := c.String("username")
-	user, err := user_model.GetUserByName(ctx, uname)
+	user, err := user_model.GetUserByName(ctx, c.String("username"))
 	if err != nil {
 		return err
 	}
-	if err = user.SetPassword(c.String("password")); err != nil {
-		return err
-	}
 
+	var mustChangePassword optional.Option[bool]
 	if c.IsSet("must-change-password") {
-		user.MustChangePassword = c.Bool("must-change-password")
+		mustChangePassword = optional.Some(c.Bool("must-change-password"))
 	}
 
-	if err = user_model.UpdateUserCols(ctx, user, "must_change_password", "passwd", "passwd_hash_algo", "salt"); err != nil {
-		return err
+	opts := &user_service.UpdateAuthOptions{
+		Password:           optional.Some(c.String("password")),
+		MustChangePassword: mustChangePassword,
+	}
+	if err := user_service.UpdateAuth(ctx, user, opts); err != nil {
+		switch {
+		case errors.Is(err, password.ErrMinLength):
+			return fmt.Errorf("Password is not long enough. Needs to be at least %d", setting.MinPasswordLength)
+		case errors.Is(err, password.ErrComplexity):
+			return errors.New("Password does not meet complexity requirements")
+		case errors.Is(err, password.ErrIsPwned):
+			return errors.New("The password you chose is on a list of stolen passwords previously exposed in public data breaches. Please try again with a different password.\nFor more details, see https://haveibeenpwned.com/Passwords")
+		default:
+			return err
+		}
 	}
 
 	fmt.Printf("%s's password has been successfully updated!\n", user.Name)

models/fixtures/email_address.yml

@@ -285,3 +285,11 @@
   lower_email: [email protected]
   is_activated: true
   is_primary: false
+
+-
+  id: 37
+  uid: 37
+  email: [email protected]
+  lower_email: [email protected]
+  is_activated: true
+  is_primary: true

models/fixtures/user.yml

@@ -1095,7 +1095,7 @@
   allow_git_hook: false
   allow_import_local: false
   allow_create_organization: true
-  prohibit_login: true
+  prohibit_login: false
   avatar: avatar29
   avatar_email: [email protected]
   use_custom_avatar: false
@@ -1332,3 +1332,40 @@
   repo_admin_change_team_access: false
   theme: ""
   keep_activity_private: false
+
+-
+  id: 37
+  lower_name: user37
+  name: user37
+  full_name: User 37
+  email: [email protected]
+  keep_email_private: false
+  email_notifications_preference: enabled
+  passwd: ZogKvWdyEx:password
+  passwd_hash_algo: dummy
+  must_change_password: false
+  login_source: 0
+  login_name: user37
+  type: 0
+  salt: ZogKvWdyEx
+  max_repo_creation: -1
+  is_active: true
+  is_admin: false
+  is_restricted: false
+  allow_git_hook: false
+  allow_import_local: false
+  allow_create_organization: true
+  prohibit_login: true
+  avatar: avatar29
+  avatar_email: [email protected]
+  use_custom_avatar: false
+  num_followers: 0
+  num_following: 0
+  num_stars: 0
+  num_repos: 0
+  num_teams: 0
+  num_members: 0
+  visibility: 0
+  repo_admin_change_team_access: false
+  theme: ""
+  keep_activity_private: false

models/user/email_address.go

@@ -142,12 +142,24 @@ func (email *EmailAddress) BeforeInsert() {
 	}
 }
 
+func InsertEmailAddress(ctx context.Context, email *EmailAddress) (*EmailAddress, error) {
+	if err := db.Insert(ctx, email); err != nil {
+		return nil, err
+	}
+	return email, nil
+}
+
+func UpdateEmailAddress(ctx context.Context, email *EmailAddress) error {
+	_, err := db.GetEngine(ctx).ID(email.ID).AllCols().Update(email)
+	return err
+}
+
 var emailRegexp = regexp.MustCompile("^[a-zA-Z0-9.!#$%&'*+-/=?^_`{|}~]*@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$")
 
 // ValidateEmail check if email is a allowed address
 func ValidateEmail(email string) error {
 	if len(email) == 0 {
-		return nil
+		return ErrEmailInvalid{email}
 	}
 
 	if !emailRegexp.MatchString(email) {
@@ -177,6 +189,36 @@ func ValidateEmail(email string) error {
 	return nil
 }
 
+func GetEmailAddressByEmail(ctx context.Context, email string) (*EmailAddress, error) {
+	ea := &EmailAddress{}
+	if has, err := db.GetEngine(ctx).Where("lower_email=?", strings.ToLower(email)).Get(ea); err != nil {
+		return nil, err
+	} else if !has {
+		return nil, ErrEmailAddressNotExist{email}
+	}
+	return ea, nil
+}
+
+func GetEmailAddressOfUser(ctx context.Context, email string, uid int64) (*EmailAddress, error) {
+	ea := &EmailAddress{}
+	if has, err := db.GetEngine(ctx).Where("lower_email=? AND uid=?", strings.ToLower(email), uid).Get(ea); err != nil {
+		return nil, err
+	} else if !has {
+		return nil, ErrEmailAddressNotExist{email}
+	}
+	return ea, nil
+}
+
+func GetPrimaryEmailAddressOfUser(ctx context.Context, uid int64) (*EmailAddress, error) {
+	ea := &EmailAddress{}
+	if has, err := db.GetEngine(ctx).Where("uid=? AND is_primary=?", uid, true).Get(ea); err != nil {
+		return nil, err
+	} else if !has {
+		return nil, ErrEmailAddressNotExist{}
+	}
+	return ea, nil
+}
+
 // GetEmailAddresses returns all email addresses belongs to given user.
 func GetEmailAddresses(ctx context.Context, uid int64) ([]*EmailAddress, error) {
 	emails := make([]*EmailAddress, 0, 5)
@@ -235,91 +277,6 @@ func IsEmailUsed(ctx context.Context, email string) (bool, error) {
 	return db.GetEngine(ctx).Where("lower_email=?", strings.ToLower(email)).Get(&EmailAddress{})
 }
 
-// AddEmailAddress adds an email address to given user.
-func AddEmailAddress(ctx context.Context, email *EmailAddress) error {
-	email.Email = strings.TrimSpace(email.Email)
-	used, err := IsEmailUsed(ctx, email.Email)
-	if err != nil {
-		return err
-	} else if used {
-		return ErrEmailAlreadyUsed{email.Email}
-	}
-
-	if err = ValidateEmail(email.Email); err != nil {
-		return err
-	}
-
-	return db.Insert(ctx, email)
-}
-
-// AddEmailAddresses adds an email address to given user.
-func AddEmailAddresses(ctx context.Context, emails []*EmailAddress) error {
-	if len(emails) == 0 {
-		return nil
-	}
-
-	// Check if any of them has been used
-	for i := range emails {
-		emails[i].Email = strings.TrimSpace(emails[i].Email)
-		used, err := IsEmailUsed(ctx, emails[i].Email)
-		if err != nil {
-			return err
-		} else if used {
-			return ErrEmailAlreadyUsed{emails[i].Email}
-		}
-		if err = ValidateEmail(emails[i].Email); err != nil {
-			return err
-		}
-	}
-
-	if err := db.Insert(ctx, emails); err != nil {
-		return fmt.Errorf("Insert: %w", err)
-	}
-
-	return nil
-}
-
-// DeleteEmailAddress deletes an email address of given user.
-func DeleteEmailAddress(ctx context.Context, email *EmailAddress) (err error) {
-	if email.IsPrimary {
-		return ErrPrimaryEmailCannotDelete{Email: email.Email}
-	}
-
-	var deleted int64
-	// ask to check UID
-	address := EmailAddress{
-		UID: email.UID,
-	}
-	if email.ID > 0 {
-		deleted, err = db.GetEngine(ctx).ID(email.ID).Delete(&address)
-	} else {
-		if email.Email != "" && email.LowerEmail == "" {
-			email.LowerEmail = strings.ToLower(email.Email)
-		}
-		deleted, err = db.GetEngine(ctx).
-			Where("lower_email=?", email.LowerEmail).
-			Delete(&address)
-	}
-
-	if err != nil {
-		return err
-	} else if deleted != 1 {
-		return ErrEmailAddressNotExist{Email: email.Email}
-	}
-	return nil
-}
-
-// DeleteEmailAddresses deletes multiple email addresses
-func DeleteEmailAddresses(ctx context.Context, emails []*EmailAddress) (err error) {
-	for i := range emails {
-		if err = DeleteEmailAddress(ctx, emails[i]); err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
-
 // DeleteInactiveEmailAddresses deletes inactive email addresses
 func DeleteInactiveEmailAddresses(ctx context.Context) error {
 	_, err := db.GetEngine(ctx).

models/user/email_address_test.go

@@ -42,96 +42,6 @@ func TestIsEmailUsed(t *testing.T) {
 	assert.False(t, isExist)
 }
 
-func TestAddEmailAddress(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
-
-	assert.NoError(t, user_model.AddEmailAddress(db.DefaultContext, &user_model.EmailAddress{
-		Email:       "[email protected]",
-		LowerEmail:  "[email protected]",
-		IsPrimary:   true,
-		IsActivated: true,
-	}))
-
-	// ErrEmailAlreadyUsed
-	err := user_model.AddEmailAddress(db.DefaultContext, &user_model.EmailAddress{
-		Email:      "[email protected]",
-		LowerEmail: "[email protected]",
-	})
-	assert.Error(t, err)
-	assert.True(t, user_model.IsErrEmailAlreadyUsed(err))
-}
-
-func TestAddEmailAddresses(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
-
-	// insert multiple email address
-	emails := make([]*user_model.EmailAddress, 2)
-	emails[0] = &user_model.EmailAddress{
-		Email:       "[email protected]",
-		LowerEmail:  "[email protected]",
-		IsActivated: true,
-	}
-	emails[1] = &user_model.EmailAddress{
-		Email:       "[email protected]",
-		LowerEmail:  "[email protected]",
-		IsActivated: true,
-	}
-	assert.NoError(t, user_model.AddEmailAddresses(db.DefaultContext, emails))
-
-	// ErrEmailAlreadyUsed
-	err := user_model.AddEmailAddresses(db.DefaultContext, emails)
-	assert.Error(t, err)
-	assert.True(t, user_model.IsErrEmailAlreadyUsed(err))
-}
-
-func TestDeleteEmailAddress(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
-
-	assert.NoError(t, user_model.DeleteEmailAddress(db.DefaultContext, &user_model.EmailAddress{
-		UID:        int64(1),
-		ID:         int64(33),
-		Email:      "[email protected]",
-		LowerEmail: "[email protected]",
-	}))
-
-	assert.NoError(t, user_model.DeleteEmailAddress(db.DefaultContext, &user_model.EmailAddress{
-		UID:        int64(1),
-		Email:      "[email protected]",
-		LowerEmail: "[email protected]",
-	}))
-
-	// Email address does not exist
-	err := user_model.DeleteEmailAddress(db.DefaultContext, &user_model.EmailAddress{
-		UID:        int64(1),
-		Email:      "[email protected]",
-		LowerEmail: "[email protected]",
-	})
-	assert.Error(t, err)
-}
-
-func TestDeleteEmailAddresses(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
-
-	// delete multiple email address
-	emails := make([]*user_model.EmailAddress, 2)
-	emails[0] = &user_model.EmailAddress{
-		UID:        int64(2),
-		ID:         int64(3),
-		Email:      "[email protected]",
-		LowerEmail: "[email protected]",
-	}
-	emails[1] = &user_model.EmailAddress{
-		UID:        int64(2),
-		Email:      "[email protected]",
-		LowerEmail: "[email protected]",
-	}
-	assert.NoError(t, user_model.DeleteEmailAddresses(db.DefaultContext, emails))
-
-	// ErrEmailAlreadyUsed
-	err := user_model.DeleteEmailAddresses(db.DefaultContext, emails)
-	assert.Error(t, err)
-}
-
 func TestMakeEmailPrimary(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())