From f8b471ace1b59bd3fc3a04c9ddb5f62dd1dd5396 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Sun, 4 Feb 2024 14:29:09 +0100 Subject: [PATCH] Unify user update methods (#28733) Fixes #28660 Fixes an admin api bug related to `user.LoginSource` Fixed `/user/emails` response not identical to GitHub api This PR unifies the user update methods. The goal is to keep the logic only at one place (having audit logs in mind). For example, do the password checks only in one method not everywhere a password is updated. After that PR is merged, the user creation should be next. --- cmd/admin_user_change_password.go | 44 ++-- models/fixtures/email_address.yml | 8 + models/fixtures/user.yml | 39 +++- models/user/email_address.go | 129 ++++------- models/user/email_address_test.go | 90 -------- models/user/error.go | 15 -- models/user/user.go | 154 +------------ models/user/user_test.go | 67 +----- modules/auth/password/password.go | 17 +- modules/auth/password/pwn.go | 36 ++- modules/auth/password/pwn/pwn.go | 2 +- modules/auth/password/pwn/pwn_test.go | 83 +++---- modules/optional/option.go | 45 ++++ modules/optional/option_test.go | 48 ++++ routers/api/v1/admin/user.go | 154 +++++-------- routers/api/v1/org/org.go | 36 +-- routers/api/v1/user/email.go | 43 ++-- routers/api/v1/user/settings.go | 43 ++-- routers/web/admin/users.go | 190 ++++++++-------- routers/web/auth/auth.go | 46 ++-- routers/web/auth/oauth.go | 40 ++-- routers/web/auth/password.go | 117 ++++------ routers/web/org/setting.go | 71 +++--- routers/web/repo/middlewares.go | 10 +- routers/web/user/setting/account.go | 85 +++---- routers/web/user/setting/profile.go | 103 ++++----- services/auth/auth.go | 8 +- .../auth/source/ldap/source_authenticate.go | 16 +- services/auth/source/ldap/source_sync.go | 32 +-- services/mailer/mail.go | 8 +- services/mailer/notify.go | 4 +- services/user/avatar.go | 2 +- services/user/email.go | 166 ++++++++++++++ services/user/email_test.go | 129 +++++++++++ services/user/update.go | 212 ++++++++++++++++++ services/user/update_test.go | 120 ++++++++++ services/user/user.go | 5 +- services/user/user_test.go | 2 +- tests/integration/api_admin_test.go | 10 +- tests/integration/api_nodeinfo_test.go | 2 +- tests/integration/api_user_email_test.go | 10 + tests/integration/empty_repo_test.go | 10 - 42 files changed, 1383 insertions(+), 1068 deletions(-) create mode 100644 modules/optional/option.go create mode 100644 modules/optional/option_test.go create mode 100644 services/user/email.go create mode 100644 services/user/email_test.go create mode 100644 services/user/update.go create mode 100644 services/user/update_test.go diff --git a/cmd/admin_user_change_password.go b/cmd/admin_user_change_password.go index 22764318f..824d66d11 100644 --- a/cmd/admin_user_change_password.go +++ b/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")) + user, err := user_model.GetUserByName(ctx, c.String("username")) 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) - 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) diff --git a/models/fixtures/email_address.yml b/models/fixtures/email_address.yml index ce4d5208d..67a99f43e 100644 --- a/models/fixtures/email_address.yml +++ b/models/fixtures/email_address.yml @@ -285,3 +285,11 @@ lower_email: abcde@gitea.com is_activated: true is_primary: false + +- + id: 37 + uid: 37 + email: user37@example.com + lower_email: user37@example.com + is_activated: true + is_primary: true diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index 79fbb981f..aa0daedd8 100644 --- a/models/fixtures/user.yml +++ b/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: user30@example.com 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: user37@example.com + 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: user37@example.com + 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 diff --git a/models/user/email_address.go b/models/user/email_address.go index 2af2621f5..957e72fe8 100644 --- a/models/user/email_address.go +++ b/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). diff --git a/models/user/email_address_test.go b/models/user/email_address_test.go index 7f3ca75cf..140443f82 100644 --- a/models/user/email_address_test.go +++ b/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: "user1234567890@example.com", - LowerEmail: "user1234567890@example.com", - IsPrimary: true, - IsActivated: true, - })) - - // ErrEmailAlreadyUsed - err := user_model.AddEmailAddress(db.DefaultContext, &user_model.EmailAddress{ - Email: "user1234567890@example.com", - LowerEmail: "user1234567890@example.com", - }) - 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: "user1234@example.com", - LowerEmail: "user1234@example.com", - IsActivated: true, - } - emails[1] = &user_model.EmailAddress{ - Email: "user5678@example.com", - LowerEmail: "user5678@example.com", - 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: "user1-2@example.com", - LowerEmail: "user1-2@example.com", - })) - - assert.NoError(t, user_model.DeleteEmailAddress(db.DefaultContext, &user_model.EmailAddress{ - UID: int64(1), - Email: "user1-3@example.com", - LowerEmail: "user1-3@example.com", - })) - - // Email address does not exist - err := user_model.DeleteEmailAddress(db.DefaultContext, &user_model.EmailAddress{ - UID: int64(1), - Email: "user1234567890@example.com", - LowerEmail: "user1234567890@example.com", - }) - 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: "user2@example.com", - LowerEmail: "user2@example.com", - } - emails[1] = &user_model.EmailAddress{ - UID: int64(2), - Email: "user2-2@example.com", - LowerEmail: "user2-2@example.com", - } - 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()) diff --git a/models/user/error.go b/models/user/error.go index f51299416..ef572c178 100644 --- a/models/user/error.go +++ b/models/user/error.go @@ -108,18 +108,3 @@ func IsErrUserIsNotLocal(err error) bool { _, ok := err.(ErrUserIsNotLocal) return ok } - -type ErrUsernameNotChanged struct { - UID int64 - Name string -} - -func (err ErrUsernameNotChanged) Error() string { - return fmt.Sprintf("username hasn't been changed[uid: %d, name: %s]", err.UID, err.Name) -} - -// IsErrUsernameNotChanged -func IsErrUsernameNotChanged(err error) bool { - _, ok := err.(ErrUsernameNotChanged) - return ok -} diff --git a/models/user/user.go b/models/user/user.go index 269a1be72..e5245dfbb 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -196,18 +196,6 @@ func (u *User) SetLastLogin() { u.LastLoginUnix = timeutil.TimeStampNow() } -// UpdateUserDiffViewStyle updates the users diff view style -func UpdateUserDiffViewStyle(ctx context.Context, u *User, style string) error { - u.DiffViewStyle = style - return UpdateUserCols(ctx, u, "diff_view_style") -} - -// UpdateUserTheme updates a users' theme irrespective of the site wide theme -func UpdateUserTheme(ctx context.Context, u *User, themeName string) error { - u.Theme = themeName - return UpdateUserCols(ctx, u, "theme") -} - // GetPlaceholderEmail returns an noreply email func (u *User) GetPlaceholderEmail() string { return fmt.Sprintf("%s@%s", u.LowerName, setting.Service.NoReplyAddress) @@ -378,13 +366,6 @@ func (u *User) NewGitSig() *git.Signature { // SetPassword hashes a password using the algorithm defined in the config value of PASSWORD_HASH_ALGO // change passwd, salt and passwd_hash_algo fields func (u *User) SetPassword(passwd string) (err error) { - if len(passwd) == 0 { - u.Passwd = "" - u.Salt = "" - u.PasswdHashAlgo = "" - return nil - } - if u.Salt, err = GetUserSalt(); err != nil { return err } @@ -488,21 +469,6 @@ func (u *User) IsMailable() bool { return u.IsActive } -// EmailNotifications returns the User's email notification preference -func (u *User) EmailNotifications() string { - return u.EmailNotificationsPreference -} - -// SetEmailNotifications sets the user's email notification preference -func SetEmailNotifications(ctx context.Context, u *User, set string) error { - u.EmailNotificationsPreference = set - if err := UpdateUserCols(ctx, u, "email_notifications_preference"); err != nil { - log.Error("SetEmailNotifications: %v", err) - return err - } - return nil -} - // IsUserExist checks if given user name exist, // the user name should be noncased unique. // If uid is presented, then check will rule out that one, @@ -705,8 +671,13 @@ func CreateUser(ctx context.Context, u *User, overwriteDefault ...*CreateUserOve if u.Rands, err = GetUserSalt(); err != nil { return err } - if err = u.SetPassword(u.Passwd); err != nil { - return err + if u.Passwd != "" { + if err = u.SetPassword(u.Passwd); err != nil { + return err + } + } else { + u.Salt = "" + u.PasswdHashAlgo = "" } // save changes to database @@ -817,24 +788,6 @@ func VerifyUserActiveCode(ctx context.Context, code string) (user *User) { return nil } -// checkDupEmail checks whether there are the same email with the user -func checkDupEmail(ctx context.Context, u *User) error { - u.Email = strings.ToLower(u.Email) - has, err := db.GetEngine(ctx). - Where("id!=?", u.ID). - And("type=?", u.Type). - And("email=?", u.Email). - Get(new(User)) - if err != nil { - return err - } else if has { - return ErrEmailAlreadyUsed{ - Email: u.Email, - } - } - return nil -} - // ValidateUser check if user is valid to insert / update into database func ValidateUser(u *User, cols ...string) error { if len(cols) == 0 || util.SliceContainsString(cols, "visibility", true) { @@ -843,81 +796,9 @@ func ValidateUser(u *User, cols ...string) error { } } - if len(cols) == 0 || util.SliceContainsString(cols, "email", true) { - u.Email = strings.ToLower(u.Email) - if err := ValidateEmail(u.Email); err != nil { - return err - } - } return nil } -// UpdateUser updates user's information. -func UpdateUser(ctx context.Context, u *User, changePrimaryEmail bool, cols ...string) error { - err := ValidateUser(u, cols...) - if err != nil { - return err - } - - e := db.GetEngine(ctx) - - if changePrimaryEmail { - var emailAddress EmailAddress - has, err := e.Where("lower_email=?", strings.ToLower(u.Email)).Get(&emailAddress) - if err != nil { - return err - } - if has && emailAddress.UID != u.ID { - return ErrEmailAlreadyUsed{ - Email: u.Email, - } - } - // 1. Update old primary email - if _, err = e.Where("uid=? AND is_primary=?", u.ID, true).Cols("is_primary").Update(&EmailAddress{ - IsPrimary: false, - }); err != nil { - return err - } - - if !has { - emailAddress.Email = u.Email - emailAddress.UID = u.ID - emailAddress.IsActivated = true - emailAddress.IsPrimary = true - if _, err := e.Insert(&emailAddress); err != nil { - return err - } - } else if _, err := e.ID(emailAddress.ID).Cols("is_primary").Update(&EmailAddress{ - IsPrimary: true, - }); err != nil { - return err - } - } else if !u.IsOrganization() { // check if primary email in email_address table - primaryEmailExist, err := e.Where("uid=? AND is_primary=?", u.ID, true).Exist(&EmailAddress{}) - if err != nil { - return err - } - - if !primaryEmailExist { - if _, err := e.Insert(&EmailAddress{ - Email: u.Email, - UID: u.ID, - IsActivated: true, - IsPrimary: true, - }); err != nil { - return err - } - } - } - - if len(cols) == 0 { - _, err = e.ID(u.ID).AllCols().Update(u) - } else { - _, err = e.ID(u.ID).Cols(cols...).Update(u) - } - return err -} - // UpdateUserCols update user according special columns func UpdateUserCols(ctx context.Context, u *User, cols ...string) error { if err := ValidateUser(u, cols...); err != nil { @@ -928,25 +809,6 @@ func UpdateUserCols(ctx context.Context, u *User, cols ...string) error { return err } -// UpdateUserSetting updates user's settings. -func UpdateUserSetting(ctx context.Context, u *User) (err error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - if !u.IsOrganization() { - if err = checkDupEmail(ctx, u); err != nil { - return err - } - } - if err = UpdateUser(ctx, u, false); err != nil { - return err - } - return committer.Commit() -} - // GetInactiveUsers gets all inactive users func GetInactiveUsers(ctx context.Context, olderThan time.Duration) ([]*User, error) { var cond builder.Cond = builder.Eq{"is_active": false} @@ -1044,7 +906,7 @@ func GetUserEmailsByNames(ctx context.Context, names []string) []string { if err != nil { continue } - if u.IsMailable() && u.EmailNotifications() != EmailNotificationsDisabled { + if u.IsMailable() && u.EmailNotificationsPreference != EmailNotificationsDisabled { mails = append(mails, u.Email) } } diff --git a/models/user/user_test.go b/models/user/user_test.go index 65aebea43..f3e5a95b1 100644 --- a/models/user/user_test.go +++ b/models/user/user_test.go @@ -101,13 +101,13 @@ func TestSearchUsers(t *testing.T) { } testUserSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}}, - []int64{1, 2, 4, 5, 8, 9, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34}) + []int64{1, 2, 4, 5, 8, 9, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34, 37}) testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsActive: util.OptionalBoolFalse}, []int64{9}) testUserSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: util.OptionalBoolTrue}, - []int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34}) + []int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34, 37}) testUserSuccess(&user_model.SearchUserOptions{Keyword: "user1", OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: util.OptionalBoolTrue}, []int64{1, 10, 11, 12, 13, 14, 15, 16, 18}) @@ -123,7 +123,7 @@ func TestSearchUsers(t *testing.T) { []int64{29}) testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsProhibitLogin: util.OptionalBoolTrue}, - []int64{30}) + []int64{37}) testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsTwoFactorEnabled: util.OptionalBoolTrue}, []int64{24}) @@ -147,20 +147,7 @@ func TestEmailNotificationPreferences(t *testing.T) { {user_model.EmailNotificationsOnMention, 9}, } { user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: test.userID}) - assert.Equal(t, test.expected, user.EmailNotifications()) - - // Try all possible settings - assert.NoError(t, user_model.SetEmailNotifications(db.DefaultContext, user, user_model.EmailNotificationsEnabled)) - assert.Equal(t, user_model.EmailNotificationsEnabled, user.EmailNotifications()) - - assert.NoError(t, user_model.SetEmailNotifications(db.DefaultContext, user, user_model.EmailNotificationsOnMention)) - assert.Equal(t, user_model.EmailNotificationsOnMention, user.EmailNotifications()) - - assert.NoError(t, user_model.SetEmailNotifications(db.DefaultContext, user, user_model.EmailNotificationsDisabled)) - assert.Equal(t, user_model.EmailNotificationsDisabled, user.EmailNotifications()) - - assert.NoError(t, user_model.SetEmailNotifications(db.DefaultContext, user, user_model.EmailNotificationsAndYourOwn)) - assert.Equal(t, user_model.EmailNotificationsAndYourOwn, user.EmailNotifications()) + assert.Equal(t, test.expected, user.EmailNotificationsPreference) } } @@ -343,42 +330,6 @@ func TestGetMaileableUsersByIDs(t *testing.T) { } } -func TestUpdateUser(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - - user.KeepActivityPrivate = true - assert.NoError(t, user_model.UpdateUser(db.DefaultContext, user, false)) - user = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - assert.True(t, user.KeepActivityPrivate) - - setting.Service.AllowedUserVisibilityModesSlice = []bool{true, false, false} - user.KeepActivityPrivate = false - user.Visibility = structs.VisibleTypePrivate - assert.Error(t, user_model.UpdateUser(db.DefaultContext, user, false)) - user = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - assert.True(t, user.KeepActivityPrivate) - - newEmail := "new_" + user.Email - user.Email = newEmail - assert.NoError(t, user_model.UpdateUser(db.DefaultContext, user, true)) - user = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - assert.Equal(t, newEmail, user.Email) - - user.Email = "no mail@mail.org" - assert.Error(t, user_model.UpdateUser(db.DefaultContext, user, true)) -} - -func TestUpdateUserEmailAlreadyUsed(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - org3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}) - - user2.Email = org3.Email - err := user_model.UpdateUser(db.DefaultContext, user2, true) - assert.True(t, user_model.IsErrEmailAlreadyUsed(err)) -} - func TestNewUserRedirect(t *testing.T) { // redirect to a completely new name assert.NoError(t, unittest.PrepareTestDatabase()) @@ -534,14 +485,12 @@ func Test_ValidateUser(t *testing.T) { }() setting.Service.AllowedUserVisibilityModesSlice = []bool{true, false, true} kases := map[*user_model.User]bool{ - {ID: 1, Visibility: structs.VisibleTypePublic}: true, - {ID: 2, Visibility: structs.VisibleTypeLimited}: false, - {ID: 2, Visibility: structs.VisibleTypeLimited, Email: "invalid"}: false, - {ID: 2, Visibility: structs.VisibleTypePrivate, Email: "valid@valid.com"}: true, + {ID: 1, Visibility: structs.VisibleTypePublic}: true, + {ID: 2, Visibility: structs.VisibleTypeLimited}: false, + {ID: 2, Visibility: structs.VisibleTypePrivate}: true, } for kase, expected := range kases { - err := user_model.ValidateUser(kase) - assert.EqualValues(t, expected, err == nil, fmt.Sprintf("case: %+v", kase)) + assert.EqualValues(t, expected, nil == user_model.ValidateUser(kase), fmt.Sprintf("case: %+v", kase)) } } diff --git a/modules/auth/password/password.go b/modules/auth/password/password.go index 2172dc8b4..2c7205b70 100644 --- a/modules/auth/password/password.go +++ b/modules/auth/password/password.go @@ -5,8 +5,9 @@ package password import ( "bytes" - goContext "context" + "context" "crypto/rand" + "errors" "math/big" "strings" "sync" @@ -15,6 +16,11 @@ import ( "code.gitea.io/gitea/modules/translation" ) +var ( + ErrComplexity = errors.New("password not complex enough") + ErrMinLength = errors.New("password not long enough") +) + // complexity contains information about a particular kind of password complexity type complexity struct { ValidChars string @@ -101,11 +107,14 @@ func Generate(n int) (string, error) { } buffer[j] = validChars[rnd.Int64()] } - pwned, err := IsPwned(goContext.Background(), string(buffer)) - if err != nil { + + if err := IsPwned(context.Background(), string(buffer)); err != nil { + if errors.Is(err, ErrIsPwned) { + continue + } return "", err } - if IsComplexEnough(string(buffer)) && !pwned && string(buffer[0]) != " " && string(buffer[n-1]) != " " { + if IsComplexEnough(string(buffer)) && string(buffer[0]) != " " && string(buffer[n-1]) != " " { return string(buffer), nil } } diff --git a/modules/auth/password/pwn.go b/modules/auth/password/pwn.go index df425ac65..e00205ea1 100644 --- a/modules/auth/password/pwn.go +++ b/modules/auth/password/pwn.go @@ -5,24 +5,48 @@ package password import ( "context" + "errors" + "fmt" "code.gitea.io/gitea/modules/auth/password/pwn" "code.gitea.io/gitea/modules/setting" ) +var ErrIsPwned = errors.New("password has been pwned") + +type ErrIsPwnedRequest struct { + err error +} + +func IsErrIsPwnedRequest(err error) bool { + _, ok := err.(ErrIsPwnedRequest) + return ok +} + +func (err ErrIsPwnedRequest) Error() string { + return fmt.Sprintf("using Have-I-Been-Pwned service failed: %v", err.err) +} + +func (err ErrIsPwnedRequest) Unwrap() error { + return err.err +} + // IsPwned checks whether a password has been pwned -// NOTE: This func returns true if it encounters an error under the assumption that you ALWAYS want to check against -// HIBP, so not getting a response should block a password until it can be verified. -func IsPwned(ctx context.Context, password string) (bool, error) { +// If a password has not been pwned, no error is returned. +func IsPwned(ctx context.Context, password string) error { if !setting.PasswordCheckPwn { - return false, nil + return nil } client := pwn.New(pwn.WithContext(ctx)) count, err := client.CheckPassword(password, true) if err != nil { - return true, err + return ErrIsPwnedRequest{err} } - return count > 0, nil + if count > 0 { + return ErrIsPwned + } + + return nil } diff --git a/modules/auth/password/pwn/pwn.go b/modules/auth/password/pwn/pwn.go index b5a015fb9..f77ce9f40 100644 --- a/modules/auth/password/pwn/pwn.go +++ b/modules/auth/password/pwn/pwn.go @@ -73,7 +73,7 @@ func newRequest(ctx context.Context, method, url string, body io.ReadCloser) (*h // because artificial responses will be added to the response // For more information, see https://www.troyhunt.com/enhancing-pwned-passwords-privacy-with-padding/ func (c *Client) CheckPassword(pw string, padding bool) (int, error) { - if strings.TrimSpace(pw) == "" { + if pw == "" { return -1, ErrEmptyPassword } diff --git a/modules/auth/password/pwn/pwn_test.go b/modules/auth/password/pwn/pwn_test.go index 148208b96..f9deadc8d 100644 --- a/modules/auth/password/pwn/pwn_test.go +++ b/modules/auth/password/pwn/pwn_test.go @@ -4,13 +4,14 @@ package pwn import ( - "errors" "math/rand" "net/http" "os" "strings" "testing" "time" + + "github.com/stretchr/testify/assert" ) var client = New(WithHTTP(&http.Client{ @@ -25,78 +26,44 @@ func TestMain(m *testing.M) { func TestPassword(t *testing.T) { // Check input error _, err := client.CheckPassword("", false) - if err == nil { - t.Log("blank input should return an error") - t.Fail() - } - if !errors.Is(err, ErrEmptyPassword) { - t.Log("blank input should return ErrEmptyPassword") - t.Fail() - } + assert.ErrorIs(t, err, ErrEmptyPassword, "blank input should return ErrEmptyPassword") // Should fail fail := "password1234" count, err := client.CheckPassword(fail, false) - if err != nil { - t.Log(err) - t.Fail() - } - if count == 0 { - t.Logf("%s should fail as a password\n", fail) - t.Fail() - } + assert.NotEmpty(t, count, "%s should fail as a password", fail) + assert.NoError(t, err) // Should fail (with padding) failPad := "administrator" count, err = client.CheckPassword(failPad, true) - if err != nil { - t.Log(err) - t.Fail() - } - if count == 0 { - t.Logf("%s should fail as a password\n", failPad) - t.Fail() - } + assert.NotEmpty(t, count, "%s should fail as a password", failPad) + assert.NoError(t, err) // Checking for a "good" password isn't going to be perfect, but we can give it a good try // with hopefully minimal error. Try five times? - var good bool - var pw string - for idx := 0; idx <= 5; idx++ { - pw = testPassword() - count, err = client.CheckPassword(pw, false) - if err != nil { - t.Log(err) - t.Fail() + assert.Condition(t, func() bool { + for i := 0; i <= 5; i++ { + count, err = client.CheckPassword(testPassword(), false) + assert.NoError(t, err) + if count == 0 { + return true + } } - if count == 0 { - good = true - break - } - } - if !good { - t.Log("no generated passwords passed. there is a chance this is a fluke") - t.Fail() - } + return false + }, "no generated passwords passed. there is a chance this is a fluke") // Again, but with padded responses - good = false - for idx := 0; idx <= 5; idx++ { - pw = testPassword() - count, err = client.CheckPassword(pw, true) - if err != nil { - t.Log(err) - t.Fail() + assert.Condition(t, func() bool { + for i := 0; i <= 5; i++ { + count, err = client.CheckPassword(testPassword(), true) + assert.NoError(t, err) + if count == 0 { + return true + } } - if count == 0 { - good = true - break - } - } - if !good { - t.Log("no generated passwords passed. there is a chance this is a fluke") - t.Fail() - } + return false + }, "no generated passwords passed. there is a chance this is a fluke") } // Credit to https://golangbyexample.com/generate-random-password-golang/ diff --git a/modules/optional/option.go b/modules/optional/option.go new file mode 100644 index 000000000..af9e5ac85 --- /dev/null +++ b/modules/optional/option.go @@ -0,0 +1,45 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package optional + +type Option[T any] []T + +func None[T any]() Option[T] { + return nil +} + +func Some[T any](v T) Option[T] { + return Option[T]{v} +} + +func FromPtr[T any](v *T) Option[T] { + if v == nil { + return None[T]() + } + return Some(*v) +} + +func FromNonDefault[T comparable](v T) Option[T] { + var zero T + if v == zero { + return None[T]() + } + return Some(v) +} + +func (o Option[T]) Has() bool { + return o != nil +} + +func (o Option[T]) Value() T { + var zero T + return o.ValueOrDefault(zero) +} + +func (o Option[T]) ValueOrDefault(v T) T { + if o.Has() { + return o[0] + } + return v +} diff --git a/modules/optional/option_test.go b/modules/optional/option_test.go new file mode 100644 index 000000000..7ec345b6b --- /dev/null +++ b/modules/optional/option_test.go @@ -0,0 +1,48 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package optional + +import ( + "testing" + + "code.gitea.io/gitea/modules/util" + + "github.com/stretchr/testify/assert" +) + +func TestOption(t *testing.T) { + var uninitialized Option[int] + assert.False(t, uninitialized.Has()) + assert.Equal(t, int(0), uninitialized.Value()) + assert.Equal(t, int(1), uninitialized.ValueOrDefault(1)) + + none := None[int]() + assert.False(t, none.Has()) + assert.Equal(t, int(0), none.Value()) + assert.Equal(t, int(1), none.ValueOrDefault(1)) + + some := Some[int](1) + assert.True(t, some.Has()) + assert.Equal(t, int(1), some.Value()) + assert.Equal(t, int(1), some.ValueOrDefault(2)) + + var ptr *int + assert.False(t, FromPtr(ptr).Has()) + + opt1 := FromPtr(util.ToPointer(1)) + assert.True(t, opt1.Has()) + assert.Equal(t, int(1), opt1.Value()) + + assert.False(t, FromNonDefault("").Has()) + + opt2 := FromNonDefault("test") + assert.True(t, opt2.Has()) + assert.Equal(t, "test", opt2.Value()) + + assert.False(t, FromNonDefault(0).Has()) + + opt3 := FromNonDefault(1) + assert.True(t, opt3.Has()) + assert.Equal(t, int(1), opt3.Value()) +} diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go index b4cc42ea5..272996f43 100644 --- a/routers/api/v1/admin/user.go +++ b/routers/api/v1/admin/user.go @@ -8,7 +8,6 @@ import ( "errors" "fmt" "net/http" - "strings" "code.gitea.io/gitea/models" asymkey_model "code.gitea.io/gitea/models/asymkey" @@ -18,6 +17,7 @@ import ( "code.gitea.io/gitea/modules/auth/password" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" @@ -107,9 +107,8 @@ func CreateUser(ctx *context.APIContext) { return } - pwned, err := password.IsPwned(ctx, form.Password) - if pwned { - if err != nil { + if err := password.IsPwned(ctx, form.Password); err != nil { + if password.IsErrIsPwnedRequest(err) { log.Error(err.Error()) } ctx.Error(http.StatusBadRequest, "PasswordPwned", errors.New("PasswordPwned")) @@ -192,115 +191,65 @@ func EditUser(ctx *context.APIContext) { form := web.GetForm(ctx).(*api.EditUserOption) - parseAuthSource(ctx, ctx.ContextUser, form.SourceID, form.LoginName) - if ctx.Written() { + authOpts := &user_service.UpdateAuthOptions{ + LoginSource: optional.FromNonDefault(form.SourceID), + LoginName: optional.Some(form.LoginName), + Password: optional.FromNonDefault(form.Password), + MustChangePassword: optional.FromPtr(form.MustChangePassword), + ProhibitLogin: optional.FromPtr(form.ProhibitLogin), + } + if err := user_service.UpdateAuth(ctx, ctx.ContextUser, authOpts); err != nil { + switch { + case errors.Is(err, password.ErrMinLength): + ctx.Error(http.StatusBadRequest, "PasswordTooShort", fmt.Errorf("password must be at least %d characters", setting.MinPasswordLength)) + case errors.Is(err, password.ErrComplexity): + ctx.Error(http.StatusBadRequest, "PasswordComplexity", err) + case errors.Is(err, password.ErrIsPwned), password.IsErrIsPwnedRequest(err): + ctx.Error(http.StatusBadRequest, "PasswordIsPwned", err) + default: + ctx.Error(http.StatusInternalServerError, "UpdateAuth", err) + } return } - if len(form.Password) != 0 { - if len(form.Password) < setting.MinPasswordLength { - ctx.Error(http.StatusBadRequest, "PasswordTooShort", fmt.Errorf("password must be at least %d characters", setting.MinPasswordLength)) - return - } - if !password.IsComplexEnough(form.Password) { - err := errors.New("PasswordComplexity") - ctx.Error(http.StatusBadRequest, "PasswordComplexity", err) - return - } - pwned, err := password.IsPwned(ctx, form.Password) - if pwned { - if err != nil { - log.Error(err.Error()) - } - ctx.Error(http.StatusBadRequest, "PasswordPwned", errors.New("PasswordPwned")) - return - } - if ctx.ContextUser.Salt, err = user_model.GetUserSalt(); err != nil { - ctx.Error(http.StatusInternalServerError, "UpdateUser", err) - return - } - if err = ctx.ContextUser.SetPassword(form.Password); err != nil { - ctx.InternalServerError(err) - return - } - } - - if form.MustChangePassword != nil { - ctx.ContextUser.MustChangePassword = *form.MustChangePassword - } - - ctx.ContextUser.LoginName = form.LoginName - - if form.FullName != nil { - ctx.ContextUser.FullName = *form.FullName - } - var emailChanged bool if form.Email != nil { - email := strings.TrimSpace(*form.Email) - if len(email) == 0 { - ctx.Error(http.StatusUnprocessableEntity, "", fmt.Errorf("email is not allowed to be empty string")) + if err := user_service.AddOrSetPrimaryEmailAddress(ctx, ctx.ContextUser, *form.Email); err != nil { + switch { + case user_model.IsErrEmailCharIsNotSupported(err), user_model.IsErrEmailInvalid(err): + ctx.Error(http.StatusBadRequest, "EmailInvalid", err) + case user_model.IsErrEmailAlreadyUsed(err): + ctx.Error(http.StatusBadRequest, "EmailUsed", err) + default: + ctx.Error(http.StatusInternalServerError, "AddOrSetPrimaryEmailAddress", err) + } return } - - if err := user_model.ValidateEmail(email); err != nil { - ctx.InternalServerError(err) - return - } - - emailChanged = !strings.EqualFold(ctx.ContextUser.Email, email) - ctx.ContextUser.Email = email - } - if form.Website != nil { - ctx.ContextUser.Website = *form.Website - } - if form.Location != nil { - ctx.ContextUser.Location = *form.Location - } - if form.Description != nil { - ctx.ContextUser.Description = *form.Description - } - if form.Active != nil { - ctx.ContextUser.IsActive = *form.Active - } - if len(form.Visibility) != 0 { - ctx.ContextUser.Visibility = api.VisibilityModes[form.Visibility] - } - if form.Admin != nil { - if !*form.Admin && user_model.IsLastAdminUser(ctx, ctx.ContextUser) { - ctx.Error(http.StatusBadRequest, "LastAdmin", ctx.Tr("auth.last_admin")) - return - } - ctx.ContextUser.IsAdmin = *form.Admin - } - if form.AllowGitHook != nil { - ctx.ContextUser.AllowGitHook = *form.AllowGitHook - } - if form.AllowImportLocal != nil { - ctx.ContextUser.AllowImportLocal = *form.AllowImportLocal - } - if form.MaxRepoCreation != nil { - ctx.ContextUser.MaxRepoCreation = *form.MaxRepoCreation - } - if form.AllowCreateOrganization != nil { - ctx.ContextUser.AllowCreateOrganization = *form.AllowCreateOrganization - } - if form.ProhibitLogin != nil { - ctx.ContextUser.ProhibitLogin = *form.ProhibitLogin - } - if form.Restricted != nil { - ctx.ContextUser.IsRestricted = *form.Restricted } - if err := user_model.UpdateUser(ctx, ctx.ContextUser, emailChanged); err != nil { - if user_model.IsErrEmailAlreadyUsed(err) || - user_model.IsErrEmailCharIsNotSupported(err) || - user_model.IsErrEmailInvalid(err) { - ctx.Error(http.StatusUnprocessableEntity, "", err) + opts := &user_service.UpdateOptions{ + FullName: optional.FromPtr(form.FullName), + Website: optional.FromPtr(form.Website), + Location: optional.FromPtr(form.Location), + Description: optional.FromPtr(form.Description), + IsActive: optional.FromPtr(form.Active), + IsAdmin: optional.FromPtr(form.Admin), + Visibility: optional.FromNonDefault(api.VisibilityModes[form.Visibility]), + AllowGitHook: optional.FromPtr(form.AllowGitHook), + AllowImportLocal: optional.FromPtr(form.AllowImportLocal), + MaxRepoCreation: optional.FromPtr(form.MaxRepoCreation), + AllowCreateOrganization: optional.FromPtr(form.AllowCreateOrganization), + IsRestricted: optional.FromPtr(form.Restricted), + } + + if err := user_service.UpdateUser(ctx, ctx.ContextUser, opts); err != nil { + if models.IsErrDeleteLastAdminUser(err) { + ctx.Error(http.StatusBadRequest, "LastAdmin", err) } else { ctx.Error(http.StatusInternalServerError, "UpdateUser", err) } return } + log.Trace("Account profile updated by admin (%s): %s", ctx.Doer.Name, ctx.ContextUser.Name) ctx.JSON(http.StatusOK, convert.ToUser(ctx, ctx.ContextUser, ctx.Doer)) @@ -527,9 +476,6 @@ func RenameUser(ctx *context.APIContext) { // Check if user name has been changed if err := user_service.RenameUser(ctx, ctx.ContextUser, newName); err != nil { switch { - case user_model.IsErrUsernameNotChanged(err): - // Noop as username is not changed - ctx.Status(http.StatusNoContent) case user_model.IsErrUserAlreadyExist(err): ctx.Error(http.StatusUnprocessableEntity, "", ctx.Tr("form.username_been_taken")) case db.IsErrNameReserved(err): @@ -545,5 +491,5 @@ func RenameUser(ctx *context.APIContext) { } log.Trace("User name changed: %s -> %s", oldName, newName) - ctx.Status(http.StatusOK) + ctx.Status(http.StatusNoContent) } diff --git a/routers/api/v1/org/org.go b/routers/api/v1/org/org.go index d5fac1e5b..255e28c70 100644 --- a/routers/api/v1/org/org.go +++ b/routers/api/v1/org/org.go @@ -13,12 +13,14 @@ import ( "code.gitea.io/gitea/models/perm" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/optional" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers/api/v1/user" "code.gitea.io/gitea/routers/api/v1/utils" "code.gitea.io/gitea/services/convert" "code.gitea.io/gitea/services/org" + user_service "code.gitea.io/gitea/services/user" ) func listUserOrgs(ctx *context.APIContext, u *user_model.User) { @@ -337,28 +339,30 @@ func Edit(ctx *context.APIContext) { // "$ref": "#/responses/Organization" // "404": // "$ref": "#/responses/notFound" + form := web.GetForm(ctx).(*api.EditOrgOption) - org := ctx.Org.Organization - org.FullName = form.FullName - org.Email = form.Email - org.Description = form.Description - org.Website = form.Website - org.Location = form.Location - if form.Visibility != "" { - org.Visibility = api.VisibilityModes[form.Visibility] + + if form.Email != "" { + if err := user_service.ReplacePrimaryEmailAddress(ctx, ctx.Org.Organization.AsUser(), form.Email); err != nil { + ctx.Error(http.StatusInternalServerError, "ReplacePrimaryEmailAddress", err) + return + } } - if form.RepoAdminChangeTeamAccess != nil { - org.RepoAdminChangeTeamAccess = *form.RepoAdminChangeTeamAccess + + opts := &user_service.UpdateOptions{ + FullName: optional.Some(form.FullName), + Description: optional.Some(form.Description), + Website: optional.Some(form.Website), + Location: optional.Some(form.Location), + Visibility: optional.FromNonDefault(api.VisibilityModes[form.Visibility]), + RepoAdminChangeTeamAccess: optional.FromPtr(form.RepoAdminChangeTeamAccess), } - if err := user_model.UpdateUserCols(ctx, org.AsUser(), - "full_name", "description", "website", "location", - "visibility", "repo_admin_change_team_access", - ); err != nil { - ctx.Error(http.StatusInternalServerError, "EditOrganization", err) + if err := user_service.UpdateUser(ctx, ctx.Org.Organization.AsUser(), opts); err != nil { + ctx.Error(http.StatusInternalServerError, "UpdateUser", err) return } - ctx.JSON(http.StatusOK, convert.ToOrganization(ctx, org)) + ctx.JSON(http.StatusOK, convert.ToOrganization(ctx, ctx.Org.Organization)) } // Delete an organization diff --git a/routers/api/v1/user/email.go b/routers/api/v1/user/email.go index 68f6c974a..3dcea9083 100644 --- a/routers/api/v1/user/email.go +++ b/routers/api/v1/user/email.go @@ -9,10 +9,10 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/context" - "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/services/convert" + user_service "code.gitea.io/gitea/services/user" ) // ListEmails list all of the authenticated user's email addresses @@ -56,22 +56,14 @@ func AddEmail(ctx *context.APIContext) { // "$ref": "#/responses/EmailList" // "422": // "$ref": "#/responses/validationError" + form := web.GetForm(ctx).(*api.CreateEmailOption) if len(form.Emails) == 0 { ctx.Error(http.StatusUnprocessableEntity, "", "Email list empty") return } - emails := make([]*user_model.EmailAddress, len(form.Emails)) - for i := range form.Emails { - emails[i] = &user_model.EmailAddress{ - UID: ctx.Doer.ID, - Email: form.Emails[i], - IsActivated: !setting.Service.RegisterEmailConfirm, - } - } - - if err := user_model.AddEmailAddresses(ctx, emails); err != nil { + if err := user_service.AddEmailAddresses(ctx, ctx.Doer, form.Emails); err != nil { if user_model.IsErrEmailAlreadyUsed(err) { ctx.Error(http.StatusUnprocessableEntity, "", "Email address has been used: "+err.(user_model.ErrEmailAlreadyUsed).Email) } else if user_model.IsErrEmailCharIsNotSupported(err) || user_model.IsErrEmailInvalid(err) { @@ -91,11 +83,17 @@ func AddEmail(ctx *context.APIContext) { return } - apiEmails := make([]*api.Email, len(emails)) - for i := range emails { - apiEmails[i] = convert.ToEmail(emails[i]) + emails, err := user_model.GetEmailAddresses(ctx, ctx.Doer.ID) + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetEmailAddresses", err) + return } - ctx.JSON(http.StatusCreated, &apiEmails) + + apiEmails := make([]*api.Email, 0, len(emails)) + for _, email := range emails { + apiEmails = append(apiEmails, convert.ToEmail(email)) + } + ctx.JSON(http.StatusCreated, apiEmails) } // DeleteEmail delete email @@ -115,26 +113,19 @@ func DeleteEmail(ctx *context.APIContext) { // "$ref": "#/responses/empty" // "404": // "$ref": "#/responses/notFound" + form := web.GetForm(ctx).(*api.DeleteEmailOption) if len(form.Emails) == 0 { ctx.Status(http.StatusNoContent) return } - emails := make([]*user_model.EmailAddress, len(form.Emails)) - for i := range form.Emails { - emails[i] = &user_model.EmailAddress{ - Email: form.Emails[i], - UID: ctx.Doer.ID, - } - } - - if err := user_model.DeleteEmailAddresses(ctx, emails); err != nil { + if err := user_service.DeleteEmailAddresses(ctx, ctx.Doer, form.Emails); err != nil { if user_model.IsErrEmailAddressNotExist(err) { ctx.Error(http.StatusNotFound, "DeleteEmailAddresses", err) - return + } else { + ctx.Error(http.StatusInternalServerError, "DeleteEmailAddresses", err) } - ctx.Error(http.StatusInternalServerError, "DeleteEmailAddresses", err) return } ctx.Status(http.StatusNoContent) diff --git a/routers/api/v1/user/settings.go b/routers/api/v1/user/settings.go index 53794c82f..062df1ca4 100644 --- a/routers/api/v1/user/settings.go +++ b/routers/api/v1/user/settings.go @@ -6,11 +6,12 @@ package user import ( "net/http" - user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/optional" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/services/convert" + user_service "code.gitea.io/gitea/services/user" ) // GetUserSettings returns user settings @@ -44,36 +45,18 @@ func UpdateUserSettings(ctx *context.APIContext) { form := web.GetForm(ctx).(*api.UserSettingsOptions) - if form.FullName != nil { - ctx.Doer.FullName = *form.FullName + opts := &user_service.UpdateOptions{ + FullName: optional.FromPtr(form.FullName), + Description: optional.FromPtr(form.Description), + Website: optional.FromPtr(form.Website), + Location: optional.FromPtr(form.Location), + Language: optional.FromPtr(form.Language), + Theme: optional.FromPtr(form.Theme), + DiffViewStyle: optional.FromPtr(form.DiffViewStyle), + KeepEmailPrivate: optional.FromPtr(form.HideEmail), + KeepActivityPrivate: optional.FromPtr(form.HideActivity), } - if form.Description != nil { - ctx.Doer.Description = *form.Description - } - if form.Website != nil { - ctx.Doer.Website = *form.Website - } - if form.Location != nil { - ctx.Doer.Location = *form.Location - } - if form.Language != nil { - ctx.Doer.Language = *form.Language - } - if form.Theme != nil { - ctx.Doer.Theme = *form.Theme - } - if form.DiffViewStyle != nil { - ctx.Doer.DiffViewStyle = *form.DiffViewStyle - } - - if form.HideEmail != nil { - ctx.Doer.KeepEmailPrivate = *form.HideEmail - } - if form.HideActivity != nil { - ctx.Doer.KeepActivityPrivate = *form.HideActivity - } - - if err := user_model.UpdateUser(ctx, ctx.Doer, false); err != nil { + if err := user_service.UpdateUser(ctx, ctx.Doer, opts); err != nil { ctx.InternalServerError(err) return } diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index 8f6995b96..af184fa9e 100644 --- a/routers/web/admin/users.go +++ b/routers/web/admin/users.go @@ -5,6 +5,7 @@ package admin import ( + "errors" "net/http" "net/url" "strconv" @@ -20,6 +21,7 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" @@ -162,11 +164,10 @@ func NewUserPost(ctx *context.Context) { ctx.RenderWithErr(password.BuildComplexityError(ctx.Locale), tplUserNew, &form) return } - pwned, err := password.IsPwned(ctx, form.Password) - if pwned { + if err := password.IsPwned(ctx, form.Password); err != nil { ctx.Data["Err_Password"] = true errMsg := ctx.Tr("auth.password_pwned") - if err != nil { + if password.IsErrIsPwnedRequest(err) { log.Error(err.Error()) errMsg = ctx.Tr("auth.password_pwned_err") } @@ -184,10 +185,7 @@ func NewUserPost(ctx *context.Context) { case user_model.IsErrEmailAlreadyUsed(err): ctx.Data["Err_Email"] = true ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplUserNew, &form) - case user_model.IsErrEmailCharIsNotSupported(err): - ctx.Data["Err_Email"] = true - ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplUserNew, &form) - case user_model.IsErrEmailInvalid(err): + case user_model.IsErrEmailInvalid(err), user_model.IsErrEmailCharIsNotSupported(err): ctx.Data["Err_Email"] = true ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplUserNew, &form) case db.IsErrNameReserved(err): @@ -348,68 +346,111 @@ func EditUserPost(ctx *context.Context) { return } + if form.UserName != "" { + if err := user_service.RenameUser(ctx, u, form.UserName); err != nil { + switch { + case user_model.IsErrUserIsNotLocal(err): + ctx.Data["Err_UserName"] = true + ctx.RenderWithErr(ctx.Tr("form.username_change_not_local_user"), tplUserEdit, &form) + case user_model.IsErrUserAlreadyExist(err): + ctx.Data["Err_UserName"] = true + ctx.RenderWithErr(ctx.Tr("form.username_been_taken"), tplUserEdit, &form) + case db.IsErrNameReserved(err): + ctx.Data["Err_UserName"] = true + ctx.RenderWithErr(ctx.Tr("user.form.name_reserved", form.UserName), tplUserEdit, &form) + case db.IsErrNamePatternNotAllowed(err): + ctx.Data["Err_UserName"] = true + ctx.RenderWithErr(ctx.Tr("user.form.name_pattern_not_allowed", form.UserName), tplUserEdit, &form) + case db.IsErrNameCharsNotAllowed(err): + ctx.Data["Err_UserName"] = true + ctx.RenderWithErr(ctx.Tr("user.form.name_chars_not_allowed", form.UserName), tplUserEdit, &form) + default: + ctx.ServerError("RenameUser", err) + } + return + } + } + + authOpts := &user_service.UpdateAuthOptions{ + Password: optional.FromNonDefault(form.Password), + LoginName: optional.Some(form.LoginName), + } + + // skip self Prohibit Login + if ctx.Doer.ID == u.ID { + authOpts.ProhibitLogin = optional.Some(false) + } else { + authOpts.ProhibitLogin = optional.Some(form.ProhibitLogin) + } + fields := strings.Split(form.LoginType, "-") if len(fields) == 2 { - loginType, _ := strconv.ParseInt(fields[0], 10, 0) authSource, _ := strconv.ParseInt(fields[1], 10, 64) - if u.LoginSource != authSource { - u.LoginSource = authSource - u.LoginType = auth.Type(loginType) - } + authOpts.LoginSource = optional.Some(authSource) } - if len(form.Password) > 0 && (u.IsLocal() || u.IsOAuth2()) { - var err error - if len(form.Password) < setting.MinPasswordLength { + if err := user_service.UpdateAuth(ctx, u, authOpts); err != nil { + switch { + case errors.Is(err, password.ErrMinLength): ctx.Data["Err_Password"] = true ctx.RenderWithErr(ctx.Tr("auth.password_too_short", setting.MinPasswordLength), tplUserEdit, &form) - return - } - if !password.IsComplexEnough(form.Password) { - ctx.RenderWithErr(password.BuildComplexityError(ctx.Locale), tplUserEdit, &form) - return - } - pwned, err := password.IsPwned(ctx, form.Password) - if pwned { + case errors.Is(err, password.ErrComplexity): ctx.Data["Err_Password"] = true - errMsg := ctx.Tr("auth.password_pwned") - if err != nil { - log.Error(err.Error()) - errMsg = ctx.Tr("auth.password_pwned_err") - } - ctx.RenderWithErr(errMsg, tplUserEdit, &form) - return - } - - if err := user_model.ValidateEmail(form.Email); err != nil { - ctx.Data["Err_Email"] = true - ctx.RenderWithErr(ctx.Tr("form.email_error"), tplUserEdit, &form) - return - } - - if u.Salt, err = user_model.GetUserSalt(); err != nil { + ctx.RenderWithErr(password.BuildComplexityError(ctx.Locale), tplUserEdit, &form) + case errors.Is(err, password.ErrIsPwned): + ctx.Data["Err_Password"] = true + ctx.RenderWithErr(ctx.Tr("auth.password_pwned"), tplUserEdit, &form) + case password.IsErrIsPwnedRequest(err): + log.Error("%s", err.Error()) + ctx.Data["Err_Password"] = true + ctx.RenderWithErr(ctx.Tr("auth.password_pwned_err"), tplUserEdit, &form) + default: ctx.ServerError("UpdateUser", err) - return } - if err = u.SetPassword(form.Password); err != nil { - ctx.ServerError("SetPassword", err) + return + } + + if form.Email != "" { + if err := user_service.AddOrSetPrimaryEmailAddress(ctx, u, form.Email); err != nil { + switch { + case user_model.IsErrEmailCharIsNotSupported(err), user_model.IsErrEmailInvalid(err): + ctx.Data["Err_Email"] = true + ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplUserEdit, &form) + case user_model.IsErrEmailAlreadyUsed(err): + ctx.Data["Err_Email"] = true + ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplUserEdit, &form) + default: + ctx.ServerError("AddOrSetPrimaryEmailAddress", err) + } return } } - if len(form.UserName) != 0 && u.Name != form.UserName { - if err := user_setting.HandleUsernameChange(ctx, u, form.UserName); err != nil { - if ctx.Written() { - return - } - ctx.RenderWithErr(ctx.Flash.ErrorMsg, tplUserEdit, &form) - return - } - u.Name = form.UserName - u.LowerName = strings.ToLower(form.UserName) + opts := &user_service.UpdateOptions{ + FullName: optional.Some(form.FullName), + Website: optional.Some(form.Website), + Location: optional.Some(form.Location), + IsActive: optional.Some(form.Active), + IsAdmin: optional.Some(form.Admin), + AllowGitHook: optional.Some(form.AllowGitHook), + AllowImportLocal: optional.Some(form.AllowImportLocal), + MaxRepoCreation: optional.Some(form.MaxRepoCreation), + AllowCreateOrganization: optional.Some(form.AllowCreateOrganization), + IsRestricted: optional.Some(form.Restricted), + Visibility: optional.Some(form.Visibility), } + if err := user_service.UpdateUser(ctx, u, opts); err != nil { + if models.IsErrDeleteLastAdminUser(err) { + ctx.RenderWithErr(ctx.Tr("auth.last_admin"), tplUserEdit, &form) + } else { + ctx.ServerError("UpdateUser", err) + } + return + } + log.Trace("Account profile updated by admin (%s): %s", ctx.Doer.Name, u.Name) + if form.Reset2FA { tf, err := auth.GetTwoFactorByUID(ctx, u.ID) if err != nil && !auth.IsErrTwoFactorNotEnrolled(err) { @@ -433,53 +474,8 @@ func EditUserPost(ctx *context.Context) { return } } - } - // Check whether user is the last admin - if !form.Admin && user_model.IsLastAdminUser(ctx, u) { - ctx.RenderWithErr(ctx.Tr("auth.last_admin"), tplUserEdit, &form) - return - } - - u.LoginName = form.LoginName - u.FullName = form.FullName - emailChanged := !strings.EqualFold(u.Email, form.Email) - u.Email = form.Email - u.Website = form.Website - u.Location = form.Location - u.MaxRepoCreation = form.MaxRepoCreation - u.IsActive = form.Active - u.IsAdmin = form.Admin - u.IsRestricted = form.Restricted - u.AllowGitHook = form.AllowGitHook - u.AllowImportLocal = form.AllowImportLocal - u.AllowCreateOrganization = form.AllowCreateOrganization - - u.Visibility = form.Visibility - - // skip self Prohibit Login - if ctx.Doer.ID == u.ID { - u.ProhibitLogin = false - } else { - u.ProhibitLogin = form.ProhibitLogin - } - - if err := user_model.UpdateUser(ctx, u, emailChanged); err != nil { - if user_model.IsErrEmailAlreadyUsed(err) { - ctx.Data["Err_Email"] = true - ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplUserEdit, &form) - } else if user_model.IsErrEmailCharIsNotSupported(err) || - user_model.IsErrEmailInvalid(err) { - ctx.Data["Err_Email"] = true - ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplUserEdit, &form) - } else { - ctx.ServerError("UpdateUser", err) - } - return - } - log.Trace("Account profile updated by admin (%s): %s", ctx.Doer.Name, u.Name) - ctx.Flash.Success(ctx.Tr("admin.users.update_profile_success")) ctx.Redirect(setting.AppSubURL + "/admin/users/" + url.PathEscape(ctx.Params(":userid"))) } diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index 474bae98e..3de1f3373 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -18,6 +18,7 @@ import ( "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/eventsource" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/session" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" @@ -30,6 +31,7 @@ import ( "code.gitea.io/gitea/services/externalaccount" "code.gitea.io/gitea/services/forms" "code.gitea.io/gitea/services/mailer" + user_service "code.gitea.io/gitea/services/user" "github.com/markbates/goth" ) @@ -104,9 +106,11 @@ func autoSignIn(ctx *context.Context) (bool, error) { func resetLocale(ctx *context.Context, u *user_model.User) error { // Language setting of the user overwrites the one previously set // If the user does not have a locale set, we save the current one. - if len(u.Language) == 0 { - u.Language = ctx.Locale.Language() - if err := user_model.UpdateUserCols(ctx, u, "language"); err != nil { + if u.Language == "" { + opts := &user_service.UpdateOptions{ + Language: optional.Some(ctx.Locale.Language()), + } + if err := user_service.UpdateUser(ctx, u, opts); err != nil { return err } } @@ -330,10 +334,12 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe // Language setting of the user overwrites the one previously set // If the user does not have a locale set, we save the current one. - if len(u.Language) == 0 { - u.Language = ctx.Locale.Language() - if err := user_model.UpdateUserCols(ctx, u, "language"); err != nil { - ctx.ServerError("UpdateUserCols Language", fmt.Errorf("Error updating user language [user: %d, locale: %s]", u.ID, u.Language)) + if u.Language == "" { + opts := &user_service.UpdateOptions{ + Language: optional.Some(ctx.Locale.Language()), + } + if err := user_service.UpdateUser(ctx, u, opts); err != nil { + ctx.ServerError("UpdateUser Language", fmt.Errorf("Error updating user language [user: %d, locale: %s]", u.ID, ctx.Locale.Language())) return setting.AppSubURL + "/" } } @@ -348,9 +354,8 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe ctx.Csrf.DeleteCookie(ctx) // Register last login - u.SetLastLogin() - if err := user_model.UpdateUserCols(ctx, u, "last_login_unix"); err != nil { - ctx.ServerError("UpdateUserCols", err) + if err := user_service.UpdateUser(ctx, u, &user_service.UpdateOptions{SetLastLogin: true}); err != nil { + ctx.ServerError("UpdateUser", err) return setting.AppSubURL + "/" } @@ -482,10 +487,9 @@ func SignUpPost(ctx *context.Context) { ctx.RenderWithErr(password.BuildComplexityError(ctx.Locale), tplSignUp, &form) return } - pwned, err := password.IsPwned(ctx, form.Password) - if pwned { + if err := password.IsPwned(ctx, form.Password); err != nil { errMsg := ctx.Tr("auth.password_pwned") - if err != nil { + if password.IsErrIsPwnedRequest(err) { log.Error(err.Error()) errMsg = ctx.Tr("auth.password_pwned_err") } @@ -589,10 +593,12 @@ func createUserInContext(ctx *context.Context, tpl base.TplName, form any, u *us func handleUserCreated(ctx *context.Context, u *user_model.User, gothUser *goth.User) (ok bool) { // Auto-set admin for the only user. if user_model.CountUsers(ctx, nil) == 1 { - u.IsAdmin = true - u.IsActive = true - u.SetLastLogin() - if err := user_model.UpdateUserCols(ctx, u, "is_admin", "is_active", "last_login_unix"); err != nil { + opts := &user_service.UpdateOptions{ + IsActive: optional.Some(true), + IsAdmin: optional.Some(true), + SetLastLogin: true, + } + if err := user_service.UpdateUser(ctx, u, opts); err != nil { ctx.ServerError("UpdateUser", err) return false } @@ -752,10 +758,8 @@ func handleAccountActivation(ctx *context.Context, user *user_model.User) { return } - // Register last login - user.SetLastLogin() - if err := user_model.UpdateUserCols(ctx, user, "last_login_unix"); err != nil { - ctx.ServerError("UpdateUserCols", err) + if err := user_service.UpdateUser(ctx, user, &user_service.UpdateOptions{SetLastLogin: true}); err != nil { + ctx.ServerError("UpdateUser", err) return } diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index 00305a36e..07140b667 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -24,6 +24,7 @@ import ( "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" @@ -990,7 +991,9 @@ func SignInOAuthCallback(ctx *context.Context) { source := authSource.Cfg.(*oauth2.Source) - setUserAdminAndRestrictedFromGroupClaims(source, u, &gothUser) + isAdmin, isRestricted := getUserAdminAndRestrictedFromGroupClaims(source, &gothUser) + u.IsAdmin = isAdmin.ValueOrDefault(false) + u.IsRestricted = isRestricted.ValueOrDefault(false) if !createAndHandleCreatedUser(ctx, base.TplName(""), nil, u, overwriteDefault, &gothUser, setting.OAuth2Client.AccountLinking != setting.OAuth2AccountLinkingDisabled) { // error already handled @@ -1054,19 +1057,17 @@ func getClaimedGroups(source *oauth2.Source, gothUser *goth.User) container.Set[ return claimValueToStringSet(groupClaims) } -func setUserAdminAndRestrictedFromGroupClaims(source *oauth2.Source, u *user_model.User, gothUser *goth.User) bool { +func getUserAdminAndRestrictedFromGroupClaims(source *oauth2.Source, gothUser *goth.User) (isAdmin, isRestricted optional.Option[bool]) { groups := getClaimedGroups(source, gothUser) - wasAdmin, wasRestricted := u.IsAdmin, u.IsRestricted - if source.AdminGroup != "" { - u.IsAdmin = groups.Contains(source.AdminGroup) + isAdmin = optional.Some(groups.Contains(source.AdminGroup)) } if source.RestrictedGroup != "" { - u.IsRestricted = groups.Contains(source.RestrictedGroup) + isRestricted = optional.Some(groups.Contains(source.RestrictedGroup)) } - return wasAdmin != u.IsAdmin || wasRestricted != u.IsRestricted + return isAdmin, isRestricted } func showLinkingLogin(ctx *context.Context, gothUser goth.User) { @@ -1133,18 +1134,12 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model // Clear whatever CSRF cookie has right now, force to generate a new one ctx.Csrf.DeleteCookie(ctx) - // Register last login - u.SetLastLogin() - - // Update GroupClaims - changed := setUserAdminAndRestrictedFromGroupClaims(oauth2Source, u, &gothUser) - cols := []string{"last_login_unix"} - if changed { - cols = append(cols, "is_admin", "is_restricted") + opts := &user_service.UpdateOptions{ + SetLastLogin: true, } - - if err := user_model.UpdateUserCols(ctx, u, cols...); err != nil { - ctx.ServerError("UpdateUserCols", err) + opts.IsAdmin, opts.IsRestricted = getUserAdminAndRestrictedFromGroupClaims(oauth2Source, &gothUser) + if err := user_service.UpdateUser(ctx, u, opts); err != nil { + ctx.ServerError("UpdateUser", err) return } @@ -1177,10 +1172,11 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model return } - changed := setUserAdminAndRestrictedFromGroupClaims(oauth2Source, u, &gothUser) - if changed { - if err := user_model.UpdateUserCols(ctx, u, "is_admin", "is_restricted"); err != nil { - ctx.ServerError("UpdateUserCols", err) + opts := &user_service.UpdateOptions{} + opts.IsAdmin, opts.IsRestricted = getUserAdminAndRestrictedFromGroupClaims(oauth2Source, &gothUser) + if opts.IsAdmin.Has() || opts.IsRestricted.Has() { + if err := user_service.UpdateUser(ctx, u, opts); err != nil { + ctx.ServerError("UpdateUser", err) return } } diff --git a/routers/web/auth/password.go b/routers/web/auth/password.go index def9c2bca..5af1696a6 100644 --- a/routers/web/auth/password.go +++ b/routers/web/auth/password.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/web" @@ -21,6 +22,7 @@ import ( "code.gitea.io/gitea/routers/utils" "code.gitea.io/gitea/services/forms" "code.gitea.io/gitea/services/mailer" + user_service "code.gitea.io/gitea/services/user" ) var ( @@ -165,30 +167,6 @@ func ResetPasswdPost(ctx *context.Context) { return } - // Validate password length. - passwd := ctx.FormString("password") - if len(passwd) < setting.MinPasswordLength { - ctx.Data["IsResetForm"] = true - ctx.Data["Err_Password"] = true - ctx.RenderWithErr(ctx.Tr("auth.password_too_short", setting.MinPasswordLength), tplResetPassword, nil) - return - } else if !password.IsComplexEnough(passwd) { - ctx.Data["IsResetForm"] = true - ctx.Data["Err_Password"] = true - ctx.RenderWithErr(password.BuildComplexityError(ctx.Locale), tplResetPassword, nil) - return - } else if pwned, err := password.IsPwned(ctx, passwd); pwned || err != nil { - errMsg := ctx.Tr("auth.password_pwned") - if err != nil { - log.Error(err.Error()) - errMsg = ctx.Tr("auth.password_pwned_err") - } - ctx.Data["IsResetForm"] = true - ctx.Data["Err_Password"] = true - ctx.RenderWithErr(errMsg, tplResetPassword, nil) - return - } - // Handle two-factor regenerateScratchToken := false if twofa != nil { @@ -221,18 +199,27 @@ func ResetPasswdPost(ctx *context.Context) { } } } - var err error - if u.Rands, err = user_model.GetUserSalt(); err != nil { - ctx.ServerError("UpdateUser", err) - return + + opts := &user_service.UpdateAuthOptions{ + Password: optional.Some(ctx.FormString("password")), + MustChangePassword: optional.Some(false), } - if err = u.SetPassword(passwd); err != nil { - ctx.ServerError("UpdateUser", err) - return - } - u.MustChangePassword = false - if err := user_model.UpdateUserCols(ctx, u, "must_change_password", "passwd", "passwd_hash_algo", "rands", "salt"); err != nil { - ctx.ServerError("UpdateUser", err) + if err := user_service.UpdateAuth(ctx, ctx.Doer, opts); err != nil { + ctx.Data["IsResetForm"] = true + ctx.Data["Err_Password"] = true + switch { + case errors.Is(err, password.ErrMinLength): + ctx.RenderWithErr(ctx.Tr("auth.password_too_short", setting.MinPasswordLength), tplResetPassword, nil) + case errors.Is(err, password.ErrComplexity): + ctx.RenderWithErr(password.BuildComplexityError(ctx.Locale), tplResetPassword, nil) + case errors.Is(err, password.ErrIsPwned): + ctx.RenderWithErr(ctx.Tr("auth.password_pwned"), tplResetPassword, nil) + case password.IsErrIsPwnedRequest(err): + log.Error("%s", err.Error()) + ctx.RenderWithErr(ctx.Tr("auth.password_pwned_err"), tplResetPassword, nil) + default: + ctx.ServerError("UpdateAuth", err) + } return } @@ -242,7 +229,7 @@ func ResetPasswdPost(ctx *context.Context) { if regenerateScratchToken { // Invalidate the scratch token. - _, err = twofa.GenerateScratchToken() + _, err := twofa.GenerateScratchToken() if err != nil { ctx.ServerError("UserSignIn", err) return @@ -282,11 +269,11 @@ func MustChangePasswordPost(ctx *context.Context) { ctx.HTML(http.StatusOK, tplMustChangePassword) return } - u := ctx.Doer + // Make sure only requests for users who are eligible to change their password via // this method passes through - if !u.MustChangePassword { - ctx.ServerError("MustUpdatePassword", errors.New("cannot update password.. Please visit the settings page")) + if !ctx.Doer.MustChangePassword { + ctx.ServerError("MustUpdatePassword", errors.New("cannot update password. Please visit the settings page")) return } @@ -296,44 +283,34 @@ func MustChangePasswordPost(ctx *context.Context) { return } - if len(form.Password) < setting.MinPasswordLength { - ctx.Data["Err_Password"] = true - ctx.RenderWithErr(ctx.Tr("auth.password_too_short", setting.MinPasswordLength), tplMustChangePassword, &form) - return + opts := &user_service.UpdateAuthOptions{ + Password: optional.Some(form.Password), + MustChangePassword: optional.Some(false), } - - if !password.IsComplexEnough(form.Password) { - ctx.Data["Err_Password"] = true - ctx.RenderWithErr(password.BuildComplexityError(ctx.Locale), tplMustChangePassword, &form) - return - } - pwned, err := password.IsPwned(ctx, form.Password) - if pwned { - ctx.Data["Err_Password"] = true - errMsg := ctx.Tr("auth.password_pwned") - if err != nil { - log.Error(err.Error()) - errMsg = ctx.Tr("auth.password_pwned_err") + if err := user_service.UpdateAuth(ctx, ctx.Doer, opts); err != nil { + switch { + case errors.Is(err, password.ErrMinLength): + ctx.Data["Err_Password"] = true + ctx.RenderWithErr(ctx.Tr("auth.password_too_short", setting.MinPasswordLength), tplMustChangePassword, &form) + case errors.Is(err, password.ErrComplexity): + ctx.Data["Err_Password"] = true + ctx.RenderWithErr(password.BuildComplexityError(ctx.Locale), tplMustChangePassword, &form) + case errors.Is(err, password.ErrIsPwned): + ctx.Data["Err_Password"] = true + ctx.RenderWithErr(ctx.Tr("auth.password_pwned"), tplMustChangePassword, &form) + case password.IsErrIsPwnedRequest(err): + log.Error("%s", err.Error()) + ctx.Data["Err_Password"] = true + ctx.RenderWithErr(ctx.Tr("auth.password_pwned_err"), tplMustChangePassword, &form) + default: + ctx.ServerError("UpdateAuth", err) } - ctx.RenderWithErr(errMsg, tplMustChangePassword, &form) - return - } - - if err = u.SetPassword(form.Password); err != nil { - ctx.ServerError("UpdateUser", err) - return - } - - u.MustChangePassword = false - - if err := user_model.UpdateUserCols(ctx, u, "must_change_password", "passwd", "passwd_hash_algo", "salt"); err != nil { - ctx.ServerError("UpdateUser", err) return } ctx.Flash.Success(ctx.Tr("settings.change_password_success")) - log.Trace("User updated password: %s", u.Name) + log.Trace("User updated password: %s", ctx.Doer.Name) if redirectTo := ctx.GetSiteCookie("redirect_to"); len(redirectTo) > 0 && !utils.IsExternalURL(redirectTo) { middleware.DeleteRedirectToCookie(ctx.Resp) diff --git a/routers/web/org/setting.go b/routers/web/org/setting.go index f0d9259d3..47d0063f7 100644 --- a/routers/web/org/setting.go +++ b/routers/web/org/setting.go @@ -7,7 +7,6 @@ package org import ( "net/http" "net/url" - "strings" "code.gitea.io/gitea/models" "code.gitea.io/gitea/models/db" @@ -17,6 +16,7 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/optional" repo_module "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/web" @@ -71,53 +71,50 @@ func SettingsPost(ctx *context.Context) { } org := ctx.Org.Organization - nameChanged := org.Name != form.Name - // Check if organization name has been changed. - if nameChanged { - err := user_service.RenameUser(ctx, org.AsUser(), form.Name) - switch { - case user_model.IsErrUserAlreadyExist(err): - ctx.Data["OrgName"] = true - ctx.RenderWithErr(ctx.Tr("form.username_been_taken"), tplSettingsOptions, &form) - return - case db.IsErrNameReserved(err): - ctx.Data["OrgName"] = true - ctx.RenderWithErr(ctx.Tr("repo.form.name_reserved", err.(db.ErrNameReserved).Name), tplSettingsOptions, &form) - return - case db.IsErrNamePatternNotAllowed(err): - ctx.Data["OrgName"] = true - ctx.RenderWithErr(ctx.Tr("repo.form.name_pattern_not_allowed", err.(db.ErrNamePatternNotAllowed).Pattern), tplSettingsOptions, &form) - return - case err != nil: - ctx.ServerError("org_service.RenameOrganization", err) + if org.Name != form.Name { + if err := user_service.RenameUser(ctx, org.AsUser(), form.Name); err != nil { + if user_model.IsErrUserAlreadyExist(err) { + ctx.Data["Err_Name"] = true + ctx.RenderWithErr(ctx.Tr("form.username_been_taken"), tplSettingsOptions, &form) + } else if db.IsErrNameReserved(err) { + ctx.Data["Err_Name"] = true + ctx.RenderWithErr(ctx.Tr("repo.form.name_reserved", err.(db.ErrNameReserved).Name), tplSettingsOptions, &form) + } else if db.IsErrNamePatternNotAllowed(err) { + ctx.Data["Err_Name"] = true + ctx.RenderWithErr(ctx.Tr("repo.form.name_pattern_not_allowed", err.(db.ErrNamePatternNotAllowed).Pattern), tplSettingsOptions, &form) + } else { + ctx.ServerError("RenameUser", err) + } return } - // reset ctx.org.OrgLink with new name - ctx.Org.OrgLink = setting.AppSubURL + "/org/" + url.PathEscape(form.Name) - log.Trace("Organization name changed: %s -> %s", org.Name, form.Name) + ctx.Org.OrgLink = setting.AppSubURL + "/org/" + url.PathEscape(org.Name) } - // In case it's just a case change. - org.Name = form.Name - org.LowerName = strings.ToLower(form.Name) + if form.Email != "" { + if err := user_service.ReplacePrimaryEmailAddress(ctx, org.AsUser(), form.Email); err != nil { + ctx.Data["Err_Email"] = true + ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplSettingsOptions, &form) + return + } + } + opts := &user_service.UpdateOptions{ + FullName: optional.Some(form.FullName), + Description: optional.Some(form.Description), + Website: optional.Some(form.Website), + Location: optional.Some(form.Location), + Visibility: optional.Some(form.Visibility), + RepoAdminChangeTeamAccess: optional.Some(form.RepoAdminChangeTeamAccess), + } if ctx.Doer.IsAdmin { - org.MaxRepoCreation = form.MaxRepoCreation + opts.MaxRepoCreation = optional.Some(form.MaxRepoCreation) } - org.FullName = form.FullName - org.Email = form.Email - org.Description = form.Description - org.Website = form.Website - org.Location = form.Location - org.RepoAdminChangeTeamAccess = form.RepoAdminChangeTeamAccess + visibilityChanged := org.Visibility != form.Visibility - visibilityChanged := form.Visibility != org.Visibility - org.Visibility = form.Visibility - - if err := user_model.UpdateUser(ctx, org.AsUser(), false); err != nil { + if err := user_service.UpdateUser(ctx, org.AsUser(), opts); err != nil { ctx.ServerError("UpdateUser", err) return } diff --git a/routers/web/repo/middlewares.go b/routers/web/repo/middlewares.go index 5f4a219aa..ee4964965 100644 --- a/routers/web/repo/middlewares.go +++ b/routers/web/repo/middlewares.go @@ -11,6 +11,8 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/optional" + user_service "code.gitea.io/gitea/services/user" ) // SetEditorconfigIfExists set editor config as render variable @@ -55,8 +57,12 @@ func SetDiffViewStyle(ctx *context.Context) { } ctx.Data["IsSplitStyle"] = style == "split" - if err := user_model.UpdateUserDiffViewStyle(ctx, ctx.Doer, style); err != nil { - ctx.ServerError("ErrUpdateDiffViewStyle", err) + + opts := &user_service.UpdateOptions{ + DiffViewStyle: optional.Some(style), + } + if err := user_service.UpdateUser(ctx, ctx.Doer, opts); err != nil { + ctx.ServerError("UpdateUser", err) } } diff --git a/routers/web/user/setting/account.go b/routers/web/user/setting/account.go index 7a306636e..c7f194a3b 100644 --- a/routers/web/user/setting/account.go +++ b/routers/web/user/setting/account.go @@ -15,6 +15,7 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/web" @@ -53,33 +54,33 @@ func AccountPost(ctx *context.Context) { return } - if len(form.Password) < setting.MinPasswordLength { - ctx.Flash.Error(ctx.Tr("auth.password_too_short", setting.MinPasswordLength)) - } else if ctx.Doer.IsPasswordSet() && !ctx.Doer.ValidatePassword(form.OldPassword) { + if ctx.Doer.IsPasswordSet() && !ctx.Doer.ValidatePassword(form.OldPassword) { ctx.Flash.Error(ctx.Tr("settings.password_incorrect")) } else if form.Password != form.Retype { ctx.Flash.Error(ctx.Tr("form.password_not_match")) - } else if !password.IsComplexEnough(form.Password) { - ctx.Flash.Error(password.BuildComplexityError(ctx.Locale)) - } else if pwned, err := password.IsPwned(ctx, form.Password); pwned || err != nil { - errMsg := ctx.Tr("auth.password_pwned") - if err != nil { - log.Error(err.Error()) - errMsg = ctx.Tr("auth.password_pwned_err") - } - ctx.Flash.Error(errMsg) } else { - var err error - if err = ctx.Doer.SetPassword(form.Password); err != nil { - ctx.ServerError("UpdateUser", err) - return + opts := &user.UpdateAuthOptions{ + Password: optional.Some(form.Password), + MustChangePassword: optional.Some(false), } - if err := user_model.UpdateUserCols(ctx, ctx.Doer, "salt", "passwd_hash_algo", "passwd"); err != nil { - ctx.ServerError("UpdateUser", err) - return + if err := user.UpdateAuth(ctx, ctx.Doer, opts); err != nil { + switch { + case errors.Is(err, password.ErrMinLength): + ctx.Flash.Error(ctx.Tr("auth.password_too_short", setting.MinPasswordLength)) + case errors.Is(err, password.ErrComplexity): + ctx.Flash.Error(password.BuildComplexityError(ctx.Locale)) + case errors.Is(err, password.ErrIsPwned): + ctx.Flash.Error(ctx.Tr("auth.password_pwned")) + case password.IsErrIsPwnedRequest(err): + log.Error("%s", err.Error()) + ctx.Flash.Error(ctx.Tr("auth.password_pwned_err")) + default: + ctx.ServerError("UpdateAuth", err) + return + } + } else { + ctx.Flash.Success(ctx.Tr("settings.change_password_success")) } - log.Trace("User password updated: %s", ctx.Doer.Name) - ctx.Flash.Success(ctx.Tr("settings.change_password_success")) } ctx.Redirect(setting.AppSubURL + "/user/settings/account") @@ -137,7 +138,7 @@ func EmailPost(ctx *context.Context) { // Only fired when the primary email is inactive (Wrong state) mailer.SendActivateAccountMail(ctx.Locale, ctx.Doer) } else { - mailer.SendActivateEmailMail(ctx.Doer, email) + mailer.SendActivateEmailMail(ctx.Doer, email.Email) } address = email.Email @@ -160,9 +161,12 @@ func EmailPost(ctx *context.Context) { ctx.ServerError("SetEmailPreference", errors.New("option unrecognized")) return } - if err := user_model.SetEmailNotifications(ctx, ctx.Doer, preference); err != nil { + opts := &user.UpdateOptions{ + EmailNotificationsPreference: optional.Some(preference), + } + if err := user.UpdateUser(ctx, ctx.Doer, opts); err != nil { log.Error("Set Email Notifications failed: %v", err) - ctx.ServerError("SetEmailNotifications", err) + ctx.ServerError("UpdateUser", err) return } log.Trace("Email notifications preference made %s: %s", preference, ctx.Doer.Name) @@ -178,48 +182,47 @@ func EmailPost(ctx *context.Context) { return } - email := &user_model.EmailAddress{ - UID: ctx.Doer.ID, - Email: form.Email, - IsActivated: !setting.Service.RegisterEmailConfirm, - } - if err := user_model.AddEmailAddress(ctx, email); err != nil { + if err := user.AddEmailAddresses(ctx, ctx.Doer, []string{form.Email}); err != nil { if user_model.IsErrEmailAlreadyUsed(err) { loadAccountData(ctx) ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplSettingsAccount, &form) - return - } else if user_model.IsErrEmailCharIsNotSupported(err) || - user_model.IsErrEmailInvalid(err) { + } else if user_model.IsErrEmailCharIsNotSupported(err) || user_model.IsErrEmailInvalid(err) { loadAccountData(ctx) ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplSettingsAccount, &form) - return + } else { + ctx.ServerError("AddEmailAddresses", err) } - ctx.ServerError("AddEmailAddress", err) return } // Send confirmation email if setting.Service.RegisterEmailConfirm { - mailer.SendActivateEmailMail(ctx.Doer, email) + mailer.SendActivateEmailMail(ctx.Doer, form.Email) if err := ctx.Cache.Put("MailResendLimit_"+ctx.Doer.LowerName, ctx.Doer.LowerName, 180); err != nil { log.Error("Set cache(MailResendLimit) fail: %v", err) } - ctx.Flash.Info(ctx.Tr("settings.add_email_confirmation_sent", email.Email, timeutil.MinutesToFriendly(setting.Service.ActiveCodeLives, ctx.Locale))) + ctx.Flash.Info(ctx.Tr("settings.add_email_confirmation_sent", form.Email, timeutil.MinutesToFriendly(setting.Service.ActiveCodeLives, ctx.Locale))) } else { ctx.Flash.Success(ctx.Tr("settings.add_email_success")) } - log.Trace("Email address added: %s", email.Email) + log.Trace("Email address added: %s", form.Email) ctx.Redirect(setting.AppSubURL + "/user/settings/account") } // DeleteEmail response for delete user's email func DeleteEmail(ctx *context.Context) { - if err := user_model.DeleteEmailAddress(ctx, &user_model.EmailAddress{ID: ctx.FormInt64("id"), UID: ctx.Doer.ID}); err != nil { - ctx.ServerError("DeleteEmail", err) + email, err := user_model.GetEmailAddressByID(ctx, ctx.Doer.ID, ctx.FormInt64("id")) + if err != nil || email == nil { + ctx.ServerError("GetEmailAddressByID", err) + return + } + + if err := user.DeleteEmailAddresses(ctx, ctx.Doer, []string{email.Email}); err != nil { + ctx.ServerError("DeleteEmailAddresses", err) return } log.Trace("Email address deleted: %s", ctx.Doer.Name) @@ -293,7 +296,7 @@ func loadAccountData(ctx *context.Context) { emails[i] = &email } ctx.Data["Emails"] = emails - ctx.Data["EmailNotificationsPreference"] = ctx.Doer.EmailNotifications() + ctx.Data["EmailNotificationsPreference"] = ctx.Doer.EmailNotificationsPreference ctx.Data["ActivationsPending"] = pendingActivation ctx.Data["CanAddEmails"] = !pendingActivation || !setting.Service.RegisterEmailConfirm diff --git a/routers/web/user/setting/profile.go b/routers/web/user/setting/profile.go index 00614565d..95b350528 100644 --- a/routers/web/user/setting/profile.go +++ b/routers/web/user/setting/profile.go @@ -22,6 +22,7 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/translation" "code.gitea.io/gitea/modules/typesniffer" @@ -49,40 +50,8 @@ func Profile(ctx *context.Context) { ctx.HTML(http.StatusOK, tplSettingsProfile) } -// HandleUsernameChange handle username changes from user settings and admin interface -func HandleUsernameChange(ctx *context.Context, user *user_model.User, newName string) error { - oldName := user.Name - // rename user - if err := user_service.RenameUser(ctx, user, newName); err != nil { - switch { - // Noop as username is not changed - case user_model.IsErrUsernameNotChanged(err): - ctx.Flash.Error(ctx.Tr("form.username_has_not_been_changed")) - // Non-local users are not allowed to change their username. - case user_model.IsErrUserIsNotLocal(err): - ctx.Flash.Error(ctx.Tr("form.username_change_not_local_user")) - case user_model.IsErrUserAlreadyExist(err): - ctx.Flash.Error(ctx.Tr("form.username_been_taken")) - case user_model.IsErrEmailAlreadyUsed(err): - ctx.Flash.Error(ctx.Tr("form.email_been_used")) - case db.IsErrNameReserved(err): - ctx.Flash.Error(ctx.Tr("user.form.name_reserved", newName)) - case db.IsErrNamePatternNotAllowed(err): - ctx.Flash.Error(ctx.Tr("user.form.name_pattern_not_allowed", newName)) - case db.IsErrNameCharsNotAllowed(err): - ctx.Flash.Error(ctx.Tr("user.form.name_chars_not_allowed", newName)) - default: - ctx.ServerError("ChangeUserName", err) - } - return err - } - log.Trace("User name changed: %s -> %s", oldName, newName) - return nil -} - // ProfilePost response for change user's profile func ProfilePost(ctx *context.Context) { - form := web.GetForm(ctx).(*forms.UpdateProfileForm) ctx.Data["Title"] = ctx.Tr("settings") ctx.Data["PageIsSettingsProfile"] = true ctx.Data["AllowedUserVisibilityModes"] = setting.Service.AllowedUserVisibilityModesSlice.ToVisibleTypeSlice() @@ -93,29 +62,40 @@ func ProfilePost(ctx *context.Context) { return } - if len(form.Name) != 0 && ctx.Doer.Name != form.Name { - log.Debug("Changing name for %s to %s", ctx.Doer.Name, form.Name) - if err := HandleUsernameChange(ctx, ctx.Doer, form.Name); err != nil { + form := web.GetForm(ctx).(*forms.UpdateProfileForm) + + if form.Name != "" { + if err := user_service.RenameUser(ctx, ctx.Doer, form.Name); err != nil { + switch { + case user_model.IsErrUserIsNotLocal(err): + ctx.Flash.Error(ctx.Tr("form.username_change_not_local_user")) + case user_model.IsErrUserAlreadyExist(err): + ctx.Flash.Error(ctx.Tr("form.username_been_taken")) + case db.IsErrNameReserved(err): + ctx.Flash.Error(ctx.Tr("user.form.name_reserved", form.Name)) + case db.IsErrNamePatternNotAllowed(err): + ctx.Flash.Error(ctx.Tr("user.form.name_pattern_not_allowed", form.Name)) + case db.IsErrNameCharsNotAllowed(err): + ctx.Flash.Error(ctx.Tr("user.form.name_chars_not_allowed", form.Name)) + default: + ctx.ServerError("RenameUser", err) + return + } ctx.Redirect(setting.AppSubURL + "/user/settings") return } - ctx.Doer.Name = form.Name - ctx.Doer.LowerName = strings.ToLower(form.Name) } - ctx.Doer.FullName = form.FullName - ctx.Doer.KeepEmailPrivate = form.KeepEmailPrivate - ctx.Doer.Website = form.Website - ctx.Doer.Location = form.Location - ctx.Doer.Description = form.Description - ctx.Doer.KeepActivityPrivate = form.KeepActivityPrivate - ctx.Doer.Visibility = form.Visibility - if err := user_model.UpdateUserSetting(ctx, ctx.Doer); err != nil { - if _, ok := err.(user_model.ErrEmailAlreadyUsed); ok { - ctx.Flash.Error(ctx.Tr("form.email_been_used")) - ctx.Redirect(setting.AppSubURL + "/user/settings") - return - } + opts := &user_service.UpdateOptions{ + FullName: optional.Some(form.FullName), + KeepEmailPrivate: optional.Some(form.KeepEmailPrivate), + Description: optional.Some(form.Description), + Website: optional.Some(form.Website), + Location: optional.Some(form.Location), + Visibility: optional.Some(form.Visibility), + KeepActivityPrivate: optional.Some(form.KeepActivityPrivate), + } + if err := user_service.UpdateUser(ctx, ctx.Doer, opts); err != nil { ctx.ServerError("UpdateUser", err) return } @@ -170,7 +150,7 @@ func UpdateAvatarSetting(ctx *context.Context, form *forms.AvatarForm, ctxUser * } if err := user_model.UpdateUserCols(ctx, ctxUser, "avatar", "avatar_email", "use_custom_avatar"); err != nil { - return fmt.Errorf("UpdateUser: %w", err) + return fmt.Errorf("UpdateUserCols: %w", err) } return nil @@ -371,14 +351,15 @@ func UpdateUIThemePost(ctx *context.Context) { return } - if err := user_model.UpdateUserTheme(ctx, ctx.Doer, form.Theme); err != nil { + opts := &user_service.UpdateOptions{ + Theme: optional.Some(form.Theme), + } + if err := user_service.UpdateUser(ctx, ctx.Doer, opts); err != nil { ctx.Flash.Error(ctx.Tr("settings.theme_update_error")) - ctx.Redirect(setting.AppSubURL + "/user/settings/appearance") - return + } else { + ctx.Flash.Success(ctx.Tr("settings.theme_update_success")) } - log.Trace("Update user theme: %s", ctx.Doer.Name) - ctx.Flash.Success(ctx.Tr("settings.theme_update_success")) ctx.Redirect(setting.AppSubURL + "/user/settings/appearance") } @@ -388,17 +369,19 @@ func UpdateUserLang(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("settings") ctx.Data["PageIsSettingsAppearance"] = true - if len(form.Language) != 0 { + if form.Language != "" { if !util.SliceContainsString(setting.Langs, form.Language) { ctx.Flash.Error(ctx.Tr("settings.update_language_not_found", form.Language)) ctx.Redirect(setting.AppSubURL + "/user/settings/appearance") return } - ctx.Doer.Language = form.Language } - if err := user_model.UpdateUserSetting(ctx, ctx.Doer); err != nil { - ctx.ServerError("UpdateUserSetting", err) + opts := &user_service.UpdateOptions{ + Language: optional.Some(form.Language), + } + if err := user_service.UpdateUser(ctx, ctx.Doer, opts); err != nil { + ctx.ServerError("UpdateUser", err) return } diff --git a/services/auth/auth.go b/services/auth/auth.go index 713463a3d..6746dc2a5 100644 --- a/services/auth/auth.go +++ b/services/auth/auth.go @@ -14,9 +14,11 @@ import ( "code.gitea.io/gitea/modules/auth/webauthn" gitea_context "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/session" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/web/middleware" + user_service "code.gitea.io/gitea/services/user" ) // Init should be called exactly once when the application starts to allow plugins @@ -85,8 +87,10 @@ func handleSignIn(resp http.ResponseWriter, req *http.Request, sess SessionStore // If the user does not have a locale set, we save the current one. if len(user.Language) == 0 { lc := middleware.Locale(resp, req) - user.Language = lc.Language() - if err := user_model.UpdateUserCols(req.Context(), user, "language"); err != nil { + opts := &user_service.UpdateOptions{ + Language: optional.Some(lc.Language()), + } + if err := user_service.UpdateUser(req.Context(), user, opts); err != nil { log.Error(fmt.Sprintf("Error updating user language [user: %d, locale: %s]", user.ID, user.Language)) return } diff --git a/services/auth/source/ldap/source_authenticate.go b/services/auth/source/ldap/source_authenticate.go index a7ea61b81..8f641ed54 100644 --- a/services/auth/source/ldap/source_authenticate.go +++ b/services/auth/source/ldap/source_authenticate.go @@ -12,6 +12,7 @@ import ( "code.gitea.io/gitea/models/auth" user_model "code.gitea.io/gitea/models/user" auth_module "code.gitea.io/gitea/modules/auth" + "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/util" source_service "code.gitea.io/gitea/services/auth/source" user_service "code.gitea.io/gitea/services/user" @@ -49,20 +50,17 @@ func (source *Source) Authenticate(ctx context.Context, user *user_model.User, u } } if user != nil && !user.ProhibitLogin { - cols := make([]string, 0) + opts := &user_service.UpdateOptions{} if len(source.AdminFilter) > 0 && user.IsAdmin != sr.IsAdmin { // Change existing admin flag only if AdminFilter option is set - user.IsAdmin = sr.IsAdmin - cols = append(cols, "is_admin") + opts.IsAdmin = optional.Some(sr.IsAdmin) } - if !user.IsAdmin && len(source.RestrictedFilter) > 0 && user.IsRestricted != sr.IsRestricted { + if !sr.IsAdmin && len(source.RestrictedFilter) > 0 && user.IsRestricted != sr.IsRestricted { // Change existing restricted flag only if RestrictedFilter option is set - user.IsRestricted = sr.IsRestricted - cols = append(cols, "is_restricted") + opts.IsRestricted = optional.Some(sr.IsRestricted) } - if len(cols) > 0 { - err = user_model.UpdateUserCols(ctx, user, cols...) - if err != nil { + if opts.IsAdmin.Has() || opts.IsRestricted.Has() { + if err := user_service.UpdateUser(ctx, user, opts); err != nil { return nil, err } } diff --git a/services/auth/source/ldap/source_sync.go b/services/auth/source/ldap/source_sync.go index 5c65ca8dc..eee7bb585 100644 --- a/services/auth/source/ldap/source_sync.go +++ b/services/auth/source/ldap/source_sync.go @@ -15,6 +15,7 @@ import ( auth_module "code.gitea.io/gitea/modules/auth" "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/util" source_service "code.gitea.io/gitea/services/auth/source" user_service "code.gitea.io/gitea/services/user" @@ -158,23 +159,25 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error { log.Trace("SyncExternalUsers[%s]: Updating user %s", source.authSource.Name, usr.Name) - usr.FullName = fullName - emailChanged := usr.Email != su.Mail - usr.Email = su.Mail - // Change existing admin flag only if AdminFilter option is set - if len(source.AdminFilter) > 0 { - usr.IsAdmin = su.IsAdmin + opts := &user_service.UpdateOptions{ + FullName: optional.Some(fullName), + IsActive: optional.Some(true), + } + if source.AdminFilter != "" { + opts.IsAdmin = optional.Some(su.IsAdmin) } // Change existing restricted flag only if RestrictedFilter option is set - if !usr.IsAdmin && len(source.RestrictedFilter) > 0 { - usr.IsRestricted = su.IsRestricted + if !su.IsAdmin && source.RestrictedFilter != "" { + opts.IsRestricted = optional.Some(su.IsRestricted) } - usr.IsActive = true - err = user_model.UpdateUser(ctx, usr, emailChanged, "full_name", "email", "is_admin", "is_restricted", "is_active") - if err != nil { + if err := user_service.UpdateUser(ctx, usr, opts); err != nil { log.Error("SyncExternalUsers[%s]: Error updating user %s: %v", source.authSource.Name, usr.Name, err) } + + if err := user_service.ReplacePrimaryEmailAddress(ctx, usr, su.Mail); err != nil { + log.Error("SyncExternalUsers[%s]: Error updating user %s primary email %s: %v", source.authSource.Name, usr.Name, su.Mail, err) + } } if usr.IsUploadAvatarChanged(su.Avatar) { @@ -215,9 +218,10 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error { log.Trace("SyncExternalUsers[%s]: Deactivating user %s", source.authSource.Name, usr.Name) - usr.IsActive = false - err = user_model.UpdateUserCols(ctx, usr, "is_active") - if err != nil { + opts := &user_service.UpdateOptions{ + IsActive: optional.Some(false), + } + if err := user_service.UpdateUser(ctx, usr, opts); err != nil { log.Error("SyncExternalUsers[%s]: Error deactivating user %s: %v", source.authSource.Name, usr.Name, err) } } diff --git a/services/mailer/mail.go b/services/mailer/mail.go index 16c30088c..ca27336f9 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -108,7 +108,7 @@ func SendResetPasswordMail(u *user_model.User) { } // SendActivateEmailMail sends confirmation email to confirm new email address -func SendActivateEmailMail(u *user_model.User, email *user_model.EmailAddress) { +func SendActivateEmailMail(u *user_model.User, email string) { if setting.MailService == nil { // No mail service configured return @@ -118,8 +118,8 @@ func SendActivateEmailMail(u *user_model.User, email *user_model.EmailAddress) { "locale": locale, "DisplayName": u.DisplayName(), "ActiveCodeLives": timeutil.MinutesToFriendly(setting.Service.ActiveCodeLives, locale), - "Code": u.GenerateEmailActivateCode(email.Email), - "Email": email.Email, + "Code": u.GenerateEmailActivateCode(email), + "Email": email, "Language": locale.Language(), } @@ -130,7 +130,7 @@ func SendActivateEmailMail(u *user_model.User, email *user_model.EmailAddress) { return } - msg := NewMessage(email.Email, locale.Tr("mail.activate_email"), content.String()) + msg := NewMessage(email, locale.Tr("mail.activate_email"), content.String()) msg.Info = fmt.Sprintf("UID: %d, activate email", u.ID) SendAsync(msg) diff --git a/services/mailer/notify.go b/services/mailer/notify.go index cc4e6baf0..e48b5d399 100644 --- a/services/mailer/notify.go +++ b/services/mailer/notify.go @@ -114,7 +114,7 @@ func (m *mailNotifier) PullRequestCodeComment(ctx context.Context, pr *issues_mo func (m *mailNotifier) IssueChangeAssignee(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, assignee *user_model.User, removed bool, comment *issues_model.Comment) { // mail only sent to added assignees and not self-assignee - if !removed && doer.ID != assignee.ID && assignee.EmailNotifications() != user_model.EmailNotificationsDisabled { + if !removed && doer.ID != assignee.ID && assignee.EmailNotificationsPreference != user_model.EmailNotificationsDisabled { ct := fmt.Sprintf("Assigned #%d.", issue.Index) if err := SendIssueAssignedMail(ctx, issue, doer, ct, comment, []*user_model.User{assignee}); err != nil { log.Error("Error in SendIssueAssignedMail for issue[%d] to assignee[%d]: %v", issue.ID, assignee.ID, err) @@ -123,7 +123,7 @@ func (m *mailNotifier) IssueChangeAssignee(ctx context.Context, doer *user_model } func (m *mailNotifier) PullRequestReviewRequest(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, reviewer *user_model.User, isRequest bool, comment *issues_model.Comment) { - if isRequest && doer.ID != reviewer.ID && reviewer.EmailNotifications() != user_model.EmailNotificationsDisabled { + if isRequest && doer.ID != reviewer.ID && reviewer.EmailNotificationsPreference != user_model.EmailNotificationsDisabled { ct := fmt.Sprintf("Requested to review %s.", issue.HTMLURL()) if err := SendIssueAssignedMail(ctx, issue, doer, ct, comment, []*user_model.User{reviewer}); err != nil { log.Error("Error in SendIssueAssignedMail for issue[%d] to reviewer[%d]: %v", issue.ID, reviewer.ID, err) diff --git a/services/user/avatar.go b/services/user/avatar.go index 4130d07c3..2d6c3faf9 100644 --- a/services/user/avatar.go +++ b/services/user/avatar.go @@ -57,7 +57,7 @@ func DeleteAvatar(ctx context.Context, u *user_model.User) error { u.UseCustomAvatar = false u.Avatar = "" if _, err := db.GetEngine(ctx).ID(u.ID).Cols("avatar, use_custom_avatar").Update(u); err != nil { - return fmt.Errorf("UpdateUser: %w", err) + return fmt.Errorf("DeleteAvatar: %w", err) } return nil } diff --git a/services/user/email.go b/services/user/email.go new file mode 100644 index 000000000..07e19bc68 --- /dev/null +++ b/services/user/email.go @@ -0,0 +1,166 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package user + +import ( + "context" + "errors" + "strings" + + "code.gitea.io/gitea/models/db" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" +) + +func AddOrSetPrimaryEmailAddress(ctx context.Context, u *user_model.User, emailStr string) error { + if strings.EqualFold(u.Email, emailStr) { + return nil + } + + if err := user_model.ValidateEmail(emailStr); err != nil { + return err + } + + // Check if address exists already + email, err := user_model.GetEmailAddressByEmail(ctx, emailStr) + if err != nil && !errors.Is(err, util.ErrNotExist) { + return err + } + if email != nil && email.UID != u.ID { + return user_model.ErrEmailAlreadyUsed{Email: emailStr} + } + + // Update old primary address + primary, err := user_model.GetPrimaryEmailAddressOfUser(ctx, u.ID) + if err != nil { + return err + } + + primary.IsPrimary = false + if err := user_model.UpdateEmailAddress(ctx, primary); err != nil { + return err + } + + // Insert new or update existing address + if email != nil { + email.IsPrimary = true + email.IsActivated = true + if err := user_model.UpdateEmailAddress(ctx, email); err != nil { + return err + } + } else { + email = &user_model.EmailAddress{ + UID: u.ID, + Email: emailStr, + IsActivated: true, + IsPrimary: true, + } + if _, err := user_model.InsertEmailAddress(ctx, email); err != nil { + return err + } + } + + u.Email = emailStr + + return user_model.UpdateUserCols(ctx, u, "email") +} + +func ReplacePrimaryEmailAddress(ctx context.Context, u *user_model.User, emailStr string) error { + if strings.EqualFold(u.Email, emailStr) { + return nil + } + + if err := user_model.ValidateEmail(emailStr); err != nil { + return err + } + + if !u.IsOrganization() { + // Check if address exists already + email, err := user_model.GetEmailAddressByEmail(ctx, emailStr) + if err != nil && !errors.Is(err, util.ErrNotExist) { + return err + } + if email != nil { + if email.IsPrimary && email.UID == u.ID { + return nil + } + return user_model.ErrEmailAlreadyUsed{Email: emailStr} + } + + // Remove old primary address + primary, err := user_model.GetPrimaryEmailAddressOfUser(ctx, u.ID) + if err != nil { + return err + } + if _, err := db.DeleteByID[user_model.EmailAddress](ctx, primary.ID); err != nil { + return err + } + + // Insert new primary address + email = &user_model.EmailAddress{ + UID: u.ID, + Email: emailStr, + IsActivated: true, + IsPrimary: true, + } + if _, err := user_model.InsertEmailAddress(ctx, email); err != nil { + return err + } + } + + u.Email = emailStr + + return user_model.UpdateUserCols(ctx, u, "email") +} + +func AddEmailAddresses(ctx context.Context, u *user_model.User, emails []string) error { + for _, emailStr := range emails { + if err := user_model.ValidateEmail(emailStr); err != nil { + return err + } + + // Check if address exists already + email, err := user_model.GetEmailAddressByEmail(ctx, emailStr) + if err != nil && !errors.Is(err, util.ErrNotExist) { + return err + } + if email != nil { + return user_model.ErrEmailAlreadyUsed{Email: emailStr} + } + + // Insert new address + email = &user_model.EmailAddress{ + UID: u.ID, + Email: emailStr, + IsActivated: !setting.Service.RegisterEmailConfirm, + IsPrimary: false, + } + if _, err := user_model.InsertEmailAddress(ctx, email); err != nil { + return err + } + } + + return nil +} + +func DeleteEmailAddresses(ctx context.Context, u *user_model.User, emails []string) error { + for _, emailStr := range emails { + // Check if address exists + email, err := user_model.GetEmailAddressOfUser(ctx, emailStr, u.ID) + if err != nil { + return err + } + if email.IsPrimary { + return user_model.ErrPrimaryEmailCannotDelete{Email: emailStr} + } + + // Remove address + if _, err := db.DeleteByID[user_model.EmailAddress](ctx, email.ID); err != nil { + return err + } + } + + return nil +} diff --git a/services/user/email_test.go b/services/user/email_test.go new file mode 100644 index 000000000..8f419b69f --- /dev/null +++ b/services/user/email_test.go @@ -0,0 +1,129 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package user + +import ( + "testing" + + "code.gitea.io/gitea/models/db" + organization_model "code.gitea.io/gitea/models/organization" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + + "github.com/stretchr/testify/assert" +) + +func TestAddOrSetPrimaryEmailAddress(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 27}) + + emails, err := user_model.GetEmailAddresses(db.DefaultContext, user.ID) + assert.NoError(t, err) + assert.Len(t, emails, 1) + + primary, err := user_model.GetPrimaryEmailAddressOfUser(db.DefaultContext, user.ID) + assert.NoError(t, err) + assert.NotEqual(t, "new-primary@example.com", primary.Email) + assert.Equal(t, user.Email, primary.Email) + + assert.NoError(t, AddOrSetPrimaryEmailAddress(db.DefaultContext, user, "new-primary@example.com")) + + primary, err = user_model.GetPrimaryEmailAddressOfUser(db.DefaultContext, user.ID) + assert.NoError(t, err) + assert.Equal(t, "new-primary@example.com", primary.Email) + assert.Equal(t, user.Email, primary.Email) + + emails, err = user_model.GetEmailAddresses(db.DefaultContext, user.ID) + assert.NoError(t, err) + assert.Len(t, emails, 2) + + assert.NoError(t, AddOrSetPrimaryEmailAddress(db.DefaultContext, user, "user27@example.com")) + + primary, err = user_model.GetPrimaryEmailAddressOfUser(db.DefaultContext, user.ID) + assert.NoError(t, err) + assert.Equal(t, "user27@example.com", primary.Email) + assert.Equal(t, user.Email, primary.Email) + + emails, err = user_model.GetEmailAddresses(db.DefaultContext, user.ID) + assert.NoError(t, err) + assert.Len(t, emails, 2) +} + +func TestReplacePrimaryEmailAddress(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + t.Run("User", func(t *testing.T) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 13}) + + emails, err := user_model.GetEmailAddresses(db.DefaultContext, user.ID) + assert.NoError(t, err) + assert.Len(t, emails, 1) + + primary, err := user_model.GetPrimaryEmailAddressOfUser(db.DefaultContext, user.ID) + assert.NoError(t, err) + assert.NotEqual(t, "primary-13@example.com", primary.Email) + assert.Equal(t, user.Email, primary.Email) + + assert.NoError(t, ReplacePrimaryEmailAddress(db.DefaultContext, user, "primary-13@example.com")) + + primary, err = user_model.GetPrimaryEmailAddressOfUser(db.DefaultContext, user.ID) + assert.NoError(t, err) + assert.Equal(t, "primary-13@example.com", primary.Email) + assert.Equal(t, user.Email, primary.Email) + + emails, err = user_model.GetEmailAddresses(db.DefaultContext, user.ID) + assert.NoError(t, err) + assert.Len(t, emails, 1) + + assert.NoError(t, ReplacePrimaryEmailAddress(db.DefaultContext, user, "primary-13@example.com")) + }) + + t.Run("Organization", func(t *testing.T) { + org := unittest.AssertExistsAndLoadBean(t, &organization_model.Organization{ID: 3}) + + assert.Equal(t, "org3@example.com", org.Email) + + assert.NoError(t, ReplacePrimaryEmailAddress(db.DefaultContext, org.AsUser(), "primary-org@example.com")) + + assert.Equal(t, "primary-org@example.com", org.Email) + }) +} + +func TestAddEmailAddresses(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + assert.Error(t, AddEmailAddresses(db.DefaultContext, user, []string{" invalid email "})) + + emails := []string{"user1234@example.com", "user5678@example.com"} + + assert.NoError(t, AddEmailAddresses(db.DefaultContext, user, emails)) + + err := AddEmailAddresses(db.DefaultContext, user, emails) + assert.Error(t, err) + assert.True(t, user_model.IsErrEmailAlreadyUsed(err)) +} + +func TestDeleteEmailAddresses(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + emails := []string{"user2-2@example.com"} + + err := DeleteEmailAddresses(db.DefaultContext, user, emails) + assert.NoError(t, err) + + err = DeleteEmailAddresses(db.DefaultContext, user, emails) + assert.Error(t, err) + assert.True(t, user_model.IsErrEmailAddressNotExist(err)) + + emails = []string{"user2@example.com"} + + err = DeleteEmailAddresses(db.DefaultContext, user, emails) + assert.Error(t, err) + assert.True(t, user_model.IsErrPrimaryEmailCannotDelete(err)) +} diff --git a/services/user/update.go b/services/user/update.go new file mode 100644 index 000000000..849757c8b --- /dev/null +++ b/services/user/update.go @@ -0,0 +1,212 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package user + +import ( + "context" + "fmt" + + "code.gitea.io/gitea/models" + auth_model "code.gitea.io/gitea/models/auth" + user_model "code.gitea.io/gitea/models/user" + password_module "code.gitea.io/gitea/modules/auth/password" + "code.gitea.io/gitea/modules/optional" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/structs" +) + +type UpdateOptions struct { + KeepEmailPrivate optional.Option[bool] + FullName optional.Option[string] + Website optional.Option[string] + Location optional.Option[string] + Description optional.Option[string] + AllowGitHook optional.Option[bool] + AllowImportLocal optional.Option[bool] + MaxRepoCreation optional.Option[int] + IsRestricted optional.Option[bool] + Visibility optional.Option[structs.VisibleType] + KeepActivityPrivate optional.Option[bool] + Language optional.Option[string] + Theme optional.Option[string] + DiffViewStyle optional.Option[string] + AllowCreateOrganization optional.Option[bool] + IsActive optional.Option[bool] + IsAdmin optional.Option[bool] + EmailNotificationsPreference optional.Option[string] + SetLastLogin bool + RepoAdminChangeTeamAccess optional.Option[bool] +} + +func UpdateUser(ctx context.Context, u *user_model.User, opts *UpdateOptions) error { + cols := make([]string, 0, 20) + + if opts.KeepEmailPrivate.Has() { + u.KeepEmailPrivate = opts.KeepEmailPrivate.Value() + + cols = append(cols, "keep_email_private") + } + + if opts.FullName.Has() { + u.FullName = opts.FullName.Value() + + cols = append(cols, "full_name") + } + if opts.Website.Has() { + u.Website = opts.Website.Value() + + cols = append(cols, "website") + } + if opts.Location.Has() { + u.Location = opts.Location.Value() + + cols = append(cols, "location") + } + if opts.Description.Has() { + u.Description = opts.Description.Value() + + cols = append(cols, "description") + } + if opts.Language.Has() { + u.Language = opts.Language.Value() + + cols = append(cols, "language") + } + if opts.Theme.Has() { + u.Theme = opts.Theme.Value() + + cols = append(cols, "theme") + } + if opts.DiffViewStyle.Has() { + u.DiffViewStyle = opts.DiffViewStyle.Value() + + cols = append(cols, "diff_view_style") + } + + if opts.AllowGitHook.Has() { + u.AllowGitHook = opts.AllowGitHook.Value() + + cols = append(cols, "allow_git_hook") + } + if opts.AllowImportLocal.Has() { + u.AllowImportLocal = opts.AllowImportLocal.Value() + + cols = append(cols, "allow_import_local") + } + + if opts.MaxRepoCreation.Has() { + u.MaxRepoCreation = opts.MaxRepoCreation.Value() + + cols = append(cols, "max_repo_creation") + } + + if opts.IsActive.Has() { + u.IsActive = opts.IsActive.Value() + + cols = append(cols, "is_active") + } + if opts.IsRestricted.Has() { + u.IsRestricted = opts.IsRestricted.Value() + + cols = append(cols, "is_restricted") + } + if opts.IsAdmin.Has() { + if !opts.IsAdmin.Value() && user_model.IsLastAdminUser(ctx, u) { + return models.ErrDeleteLastAdminUser{UID: u.ID} + } + + u.IsAdmin = opts.IsAdmin.Value() + + cols = append(cols, "is_admin") + } + + if opts.Visibility.Has() { + if !u.IsOrganization() && !setting.Service.AllowedUserVisibilityModesSlice.IsAllowedVisibility(opts.Visibility.Value()) { + return fmt.Errorf("visibility mode not allowed: %s", opts.Visibility.Value().String()) + } + u.Visibility = opts.Visibility.Value() + + cols = append(cols, "visibility") + } + if opts.KeepActivityPrivate.Has() { + u.KeepActivityPrivate = opts.KeepActivityPrivate.Value() + + cols = append(cols, "keep_activity_private") + } + + if opts.AllowCreateOrganization.Has() { + u.AllowCreateOrganization = opts.AllowCreateOrganization.Value() + + cols = append(cols, "allow_create_organization") + } + if opts.RepoAdminChangeTeamAccess.Has() { + u.RepoAdminChangeTeamAccess = opts.RepoAdminChangeTeamAccess.Value() + + cols = append(cols, "repo_admin_change_team_access") + } + + if opts.EmailNotificationsPreference.Has() { + u.EmailNotificationsPreference = opts.EmailNotificationsPreference.Value() + + cols = append(cols, "email_notifications_preference") + } + + if opts.SetLastLogin { + u.SetLastLogin() + + cols = append(cols, "last_login_unix") + } + + return user_model.UpdateUserCols(ctx, u, cols...) +} + +type UpdateAuthOptions struct { + LoginSource optional.Option[int64] + LoginName optional.Option[string] + Password optional.Option[string] + MustChangePassword optional.Option[bool] + ProhibitLogin optional.Option[bool] +} + +func UpdateAuth(ctx context.Context, u *user_model.User, opts *UpdateAuthOptions) error { + if opts.LoginSource.Has() { + source, err := auth_model.GetSourceByID(ctx, opts.LoginSource.Value()) + if err != nil { + return err + } + + u.LoginType = source.Type + u.LoginSource = source.ID + } + if opts.LoginName.Has() { + u.LoginName = opts.LoginName.Value() + } + + if opts.Password.Has() && (u.IsLocal() || u.IsOAuth2()) { + password := opts.Password.Value() + + if len(password) < setting.MinPasswordLength { + return password_module.ErrMinLength + } + if !password_module.IsComplexEnough(password) { + return password_module.ErrComplexity + } + if err := password_module.IsPwned(ctx, password); err != nil { + return err + } + + if err := u.SetPassword(password); err != nil { + return err + } + } + + if opts.MustChangePassword.Has() { + u.MustChangePassword = opts.MustChangePassword.Value() + } + if opts.ProhibitLogin.Has() { + u.ProhibitLogin = opts.ProhibitLogin.Value() + } + + return user_model.UpdateUserCols(ctx, u, "login_type", "login_source", "login_name", "passwd", "passwd_hash_algo", "salt", "must_change_password", "prohibit_login") +} diff --git a/services/user/update_test.go b/services/user/update_test.go new file mode 100644 index 000000000..7ed764b53 --- /dev/null +++ b/services/user/update_test.go @@ -0,0 +1,120 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package user + +import ( + "testing" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + password_module "code.gitea.io/gitea/modules/auth/password" + "code.gitea.io/gitea/modules/optional" + "code.gitea.io/gitea/modules/structs" + + "github.com/stretchr/testify/assert" +) + +func TestUpdateUser(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + admin := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + + assert.Error(t, UpdateUser(db.DefaultContext, admin, &UpdateOptions{ + IsAdmin: optional.Some(false), + })) + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 28}) + + opts := &UpdateOptions{ + KeepEmailPrivate: optional.Some(false), + FullName: optional.Some("Changed Name"), + Website: optional.Some("https://gitea.com/"), + Location: optional.Some("location"), + Description: optional.Some("description"), + AllowGitHook: optional.Some(true), + AllowImportLocal: optional.Some(true), + MaxRepoCreation: optional.Some[int](10), + IsRestricted: optional.Some(true), + IsActive: optional.Some(false), + IsAdmin: optional.Some(true), + Visibility: optional.Some(structs.VisibleTypePrivate), + KeepActivityPrivate: optional.Some(true), + Language: optional.Some("lang"), + Theme: optional.Some("theme"), + DiffViewStyle: optional.Some("split"), + AllowCreateOrganization: optional.Some(false), + EmailNotificationsPreference: optional.Some("disabled"), + SetLastLogin: true, + } + assert.NoError(t, UpdateUser(db.DefaultContext, user, opts)) + + assert.Equal(t, opts.KeepEmailPrivate.Value(), user.KeepEmailPrivate) + assert.Equal(t, opts.FullName.Value(), user.FullName) + assert.Equal(t, opts.Website.Value(), user.Website) + assert.Equal(t, opts.Location.Value(), user.Location) + assert.Equal(t, opts.Description.Value(), user.Description) + assert.Equal(t, opts.AllowGitHook.Value(), user.AllowGitHook) + assert.Equal(t, opts.AllowImportLocal.Value(), user.AllowImportLocal) + assert.Equal(t, opts.MaxRepoCreation.Value(), user.MaxRepoCreation) + assert.Equal(t, opts.IsRestricted.Value(), user.IsRestricted) + assert.Equal(t, opts.IsActive.Value(), user.IsActive) + assert.Equal(t, opts.IsAdmin.Value(), user.IsAdmin) + assert.Equal(t, opts.Visibility.Value(), user.Visibility) + assert.Equal(t, opts.KeepActivityPrivate.Value(), user.KeepActivityPrivate) + assert.Equal(t, opts.Language.Value(), user.Language) + assert.Equal(t, opts.Theme.Value(), user.Theme) + assert.Equal(t, opts.DiffViewStyle.Value(), user.DiffViewStyle) + assert.Equal(t, opts.AllowCreateOrganization.Value(), user.AllowCreateOrganization) + assert.Equal(t, opts.EmailNotificationsPreference.Value(), user.EmailNotificationsPreference) + + user = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 28}) + assert.Equal(t, opts.KeepEmailPrivate.Value(), user.KeepEmailPrivate) + assert.Equal(t, opts.FullName.Value(), user.FullName) + assert.Equal(t, opts.Website.Value(), user.Website) + assert.Equal(t, opts.Location.Value(), user.Location) + assert.Equal(t, opts.Description.Value(), user.Description) + assert.Equal(t, opts.AllowGitHook.Value(), user.AllowGitHook) + assert.Equal(t, opts.AllowImportLocal.Value(), user.AllowImportLocal) + assert.Equal(t, opts.MaxRepoCreation.Value(), user.MaxRepoCreation) + assert.Equal(t, opts.IsRestricted.Value(), user.IsRestricted) + assert.Equal(t, opts.IsActive.Value(), user.IsActive) + assert.Equal(t, opts.IsAdmin.Value(), user.IsAdmin) + assert.Equal(t, opts.Visibility.Value(), user.Visibility) + assert.Equal(t, opts.KeepActivityPrivate.Value(), user.KeepActivityPrivate) + assert.Equal(t, opts.Language.Value(), user.Language) + assert.Equal(t, opts.Theme.Value(), user.Theme) + assert.Equal(t, opts.DiffViewStyle.Value(), user.DiffViewStyle) + assert.Equal(t, opts.AllowCreateOrganization.Value(), user.AllowCreateOrganization) + assert.Equal(t, opts.EmailNotificationsPreference.Value(), user.EmailNotificationsPreference) +} + +func TestUpdateAuth(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 28}) + copy := *user + + assert.NoError(t, UpdateAuth(db.DefaultContext, user, &UpdateAuthOptions{ + LoginName: optional.Some("new-login"), + })) + assert.Equal(t, "new-login", user.LoginName) + + assert.NoError(t, UpdateAuth(db.DefaultContext, user, &UpdateAuthOptions{ + Password: optional.Some("%$DRZUVB576tfzgu"), + MustChangePassword: optional.Some(true), + })) + assert.True(t, user.MustChangePassword) + assert.NotEqual(t, copy.Passwd, user.Passwd) + assert.NotEqual(t, copy.Salt, user.Salt) + + assert.NoError(t, UpdateAuth(db.DefaultContext, user, &UpdateAuthOptions{ + ProhibitLogin: optional.Some(true), + })) + assert.True(t, user.ProhibitLogin) + + assert.ErrorIs(t, UpdateAuth(db.DefaultContext, user, &UpdateAuthOptions{ + Password: optional.Some("aaaa"), + }), password_module.ErrMinLength) +} diff --git a/services/user/user.go b/services/user/user.go index 8bf083192..f2648db40 100644 --- a/services/user/user.go +++ b/services/user/user.go @@ -41,10 +41,7 @@ func RenameUser(ctx context.Context, u *user_model.User, newUserName string) err } if newUserName == u.Name { - return user_model.ErrUsernameNotChanged{ - UID: u.ID, - Name: u.Name, - } + return nil } if err := user_model.IsUsableUsername(newUserName); err != nil { diff --git a/services/user/user_test.go b/services/user/user_test.go index 73f1491c1..2ebcded92 100644 --- a/services/user/user_test.go +++ b/services/user/user_test.go @@ -107,7 +107,7 @@ func TestRenameUser(t *testing.T) { }) t.Run("Same username", func(t *testing.T) { - assert.ErrorIs(t, RenameUser(db.DefaultContext, user, user.Name), user_model.ErrUsernameNotChanged{UID: user.ID, Name: user.Name}) + assert.NoError(t, RenameUser(db.DefaultContext, user, user.Name)) }) t.Run("Non usable username", func(t *testing.T) { diff --git a/tests/integration/api_admin_test.go b/tests/integration/api_admin_test.go index ff7c2ddca..0748a75ba 100644 --- a/tests/integration/api_admin_test.go +++ b/tests/integration/api_admin_test.go @@ -208,11 +208,11 @@ func TestAPIEditUser(t *testing.T) { SourceID: 0, Email: &empty, }).AddTokenAuth(token) - resp := MakeRequest(t, req, http.StatusUnprocessableEntity) + resp := MakeRequest(t, req, http.StatusBadRequest) errMap := make(map[string]any) json.Unmarshal(resp.Body.Bytes(), &errMap) - assert.EqualValues(t, "email is not allowed to be empty string", errMap["message"].(string)) + assert.EqualValues(t, "e-mail invalid [email: ]", errMap["message"].(string)) user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{LoginName: "user2"}) assert.False(t, user2.IsRestricted) @@ -254,14 +254,14 @@ func TestAPIRenameUser(t *testing.T) { // required "new_name": "User2", }).AddTokenAuth(token) - MakeRequest(t, req, http.StatusOK) + MakeRequest(t, req, http.StatusNoContent) urlStr = fmt.Sprintf("/api/v1/admin/users/%s/rename", "User2") req = NewRequestWithValues(t, "POST", urlStr, map[string]string{ // required "new_name": "User2-2-2", }).AddTokenAuth(token) - MakeRequest(t, req, http.StatusOK) + MakeRequest(t, req, http.StatusNoContent) req = NewRequestWithValues(t, "POST", urlStr, map[string]string{ // required @@ -281,7 +281,7 @@ func TestAPIRenameUser(t *testing.T) { // required "new_name": "user2", }).AddTokenAuth(token) - MakeRequest(t, req, http.StatusOK) + MakeRequest(t, req, http.StatusNoContent) } func TestAPICron(t *testing.T) { diff --git a/tests/integration/api_nodeinfo_test.go b/tests/integration/api_nodeinfo_test.go index a727aea3c..876fb5ac1 100644 --- a/tests/integration/api_nodeinfo_test.go +++ b/tests/integration/api_nodeinfo_test.go @@ -32,7 +32,7 @@ func TestNodeinfo(t *testing.T) { DecodeJSON(t, resp, &nodeinfo) assert.True(t, nodeinfo.OpenRegistrations) assert.Equal(t, "gitea", nodeinfo.Software.Name) - assert.Equal(t, 25, nodeinfo.Usage.Users.Total) + assert.Equal(t, 26, nodeinfo.Usage.Users.Total) assert.Equal(t, 20, nodeinfo.Usage.LocalPosts) assert.Equal(t, 3, nodeinfo.Usage.LocalComments) }) diff --git a/tests/integration/api_user_email_test.go b/tests/integration/api_user_email_test.go index 6eeb54744..6441e2ed8 100644 --- a/tests/integration/api_user_email_test.go +++ b/tests/integration/api_user_email_test.go @@ -67,6 +67,16 @@ func TestAPIAddEmail(t *testing.T) { var emails []*api.Email DecodeJSON(t, resp, &emails) assert.EqualValues(t, []*api.Email{ + { + Email: "user2@example.com", + Verified: true, + Primary: true, + }, + { + Email: "user2-2@example.com", + Verified: false, + Primary: false, + }, { Email: "user2-3@example.com", Verified: true, diff --git a/tests/integration/empty_repo_test.go b/tests/integration/empty_repo_test.go index 8842de5f6..ea393a606 100644 --- a/tests/integration/empty_repo_test.go +++ b/tests/integration/empty_repo_test.go @@ -12,7 +12,6 @@ import ( "testing" auth_model "code.gitea.io/gitea/models/auth" - "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" @@ -45,9 +44,6 @@ func TestEmptyRepo(t *testing.T) { func TestEmptyRepoAddFile(t *testing.T) { defer tests.PrepareTestEnv(t)() - err := user_model.UpdateUserCols(db.DefaultContext, &user_model.User{ID: 30, ProhibitLogin: false}, "prohibit_login") - assert.NoError(t, err) - session := loginUser(t, "user30") req := NewRequest(t, "GET", "/user30/empty/_new/"+setting.Repository.DefaultBranch) resp := session.MakeRequest(t, req, http.StatusOK) @@ -72,9 +68,6 @@ func TestEmptyRepoAddFile(t *testing.T) { func TestEmptyRepoUploadFile(t *testing.T) { defer tests.PrepareTestEnv(t)() - err := user_model.UpdateUserCols(db.DefaultContext, &user_model.User{ID: 30, ProhibitLogin: false}, "prohibit_login") - assert.NoError(t, err) - session := loginUser(t, "user30") req := NewRequest(t, "GET", "/user30/empty/_new/"+setting.Repository.DefaultBranch) resp := session.MakeRequest(t, req, http.StatusOK) @@ -112,9 +105,6 @@ func TestEmptyRepoUploadFile(t *testing.T) { func TestEmptyRepoAddFileByAPI(t *testing.T) { defer tests.PrepareTestEnv(t)() - err := user_model.UpdateUserCols(db.DefaultContext, &user_model.User{ID: 30, ProhibitLogin: false}, "prohibit_login") - assert.NoError(t, err) - session := loginUser(t, "user30") token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)