diff --git a/cmd/admin_user_change_password.go b/cmd/admin_user_change_password.go index 22764318fdbb..824d66d11259 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 ce4d5208df39..67a99f43e2c0 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 79fbb981f647..aa0daedd8583 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 2af2621f5fea..957e72fe8981 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 7f3ca75cfdc0..140443f82f9e 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 f51299416956..ef572c178a88 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 269a1be725f4..e5245dfbb018 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 65aebea43a95..f3e5a95b1ead 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 2172dc8b446c..2c7205b70823 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 df425ac65941..e00205ea1939 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 b5a015fb9c57..f77ce9f40b20 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 148208b964d2..f9deadc8d7e5 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 000000000000..af9e5ac85291 --- /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 000000000000..7ec345b6ba0a --- /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 b4cc42ea5d82..272996f43d36 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 d5fac1e5b8cf..255e28c70646 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 68f6c974a59c..3dcea9083cd5 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 53794c82f838..062df1ca43f7 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 8f6995b96f46..af184fa9eb38 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 474bae98e4a4..3de1f3373dc1 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 00305a36ee2b..07140b667433 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 def9c2bcaac2..5af1696a64e8 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 f0d9259d3fe4..47d0063f767d 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 5f4a219aa331..ee49649654ca 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 7a306636e0a5..c7f194a3b52c 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 00614565d203..95b350528c04 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 713463a3d47e..6746dc2a5441 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 a7ea61b81cd5..8f641ed5415a 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 5c65ca8dc27b..eee7bb585a1b 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 16c30088cdbd..ca27336f9265 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 cc4e6baf0b3a..e48b5d399d96 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 4130d07c3802..2d6c3faf9a50 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 000000000000..07e19bc6883b --- /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 000000000000..8f419b69f991 --- /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 000000000000..849757c8b0a4 --- /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 000000000000..7ed764b53952 --- /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 8bf083192fa0..f2648db409f5 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 73f1491c1224..2ebcded92521 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 ff7c2ddca39b..0748a75ba4bb 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 a727aea3ce8d..876fb5ac13e6 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 6eeb54744455..6441e2ed8ee4 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 8842de5f6f55..ea393a606167 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)