From b4513f48ce3e748ac621a6f3ddf082feca67e938 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 4 Feb 2024 21:05:01 +0800 Subject: [PATCH 1/5] Do not render empty comments (#29039) Follow #28654 The `comments` might be empty, so the templates shouldn't (and couldn't) use it to render. When there is no comment, the UI should also be updated to empty, so returning an empty body is good enough. --- routers/web/repo/pull_review.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/routers/web/repo/pull_review.go b/routers/web/repo/pull_review.go index 156b70a999a4..a6b3bd1c8d77 100644 --- a/routers/web/repo/pull_review.go +++ b/routers/web/repo/pull_review.go @@ -153,12 +153,19 @@ func UpdateResolveConversation(ctx *context.Context) { } func renderConversation(ctx *context.Context, comment *issues_model.Comment, origin string) { + ctx.Data["PageIsPullFiles"] = origin == "diff" + comments, err := issues_model.FetchCodeCommentsByLine(ctx, comment.Issue, ctx.Doer, comment.TreePath, comment.Line, ctx.Data["ShowOutdatedComments"].(bool)) if err != nil { ctx.ServerError("FetchCodeCommentsByLine", err) return } - ctx.Data["PageIsPullFiles"] = (origin == "diff") + if len(comments) == 0 { + // if the comments are empty (deleted, outdated, etc), it doesn't need to render anything, just return an empty body to replace "conversation-holder" on the page + ctx.Resp.WriteHeader(http.StatusOK) + return + } + ctx.Data["comments"] = comments if ctx.Data["CanMarkConversation"], err = issues_model.CanMarkConversation(ctx, comment.Issue, ctx.Doer); err != nil { ctx.ServerError("CanMarkConversation", err) @@ -179,6 +186,8 @@ func renderConversation(ctx *context.Context, comment *issues_model.Comment, ori ctx.HTML(http.StatusOK, tplDiffConversation) } else if origin == "timeline" { ctx.HTML(http.StatusOK, tplTimelineConversation) + } else { + ctx.Error(http.StatusBadRequest, "Unknown origin: "+origin) } } From f8b471ace1b59bd3fc3a04c9ddb5f62dd1dd5396 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Sun, 4 Feb 2024 14:29:09 +0100 Subject: [PATCH 2/5] Unify user update methods (#28733) Fixes #28660 Fixes an admin api bug related to `user.LoginSource` Fixed `/user/emails` response not identical to GitHub api This PR unifies the user update methods. The goal is to keep the logic only at one place (having audit logs in mind). For example, do the password checks only in one method not everywhere a password is updated. After that PR is merged, the user creation should be next. --- cmd/admin_user_change_password.go | 44 ++-- models/fixtures/email_address.yml | 8 + models/fixtures/user.yml | 39 +++- models/user/email_address.go | 129 ++++------- models/user/email_address_test.go | 90 -------- models/user/error.go | 15 -- models/user/user.go | 154 +------------ models/user/user_test.go | 67 +----- modules/auth/password/password.go | 17 +- modules/auth/password/pwn.go | 36 ++- modules/auth/password/pwn/pwn.go | 2 +- modules/auth/password/pwn/pwn_test.go | 83 +++---- modules/optional/option.go | 45 ++++ modules/optional/option_test.go | 48 ++++ routers/api/v1/admin/user.go | 148 ++++-------- routers/api/v1/org/org.go | 36 +-- routers/api/v1/user/email.go | 43 ++-- routers/api/v1/user/settings.go | 43 ++-- routers/web/admin/users.go | 184 ++++++++------- routers/web/auth/auth.go | 46 ++-- routers/web/auth/oauth.go | 40 ++-- routers/web/auth/password.go | 117 ++++------ routers/web/org/setting.go | 73 +++--- routers/web/repo/middlewares.go | 10 +- routers/web/user/setting/account.go | 85 +++---- routers/web/user/setting/profile.go | 103 ++++----- services/auth/auth.go | 8 +- .../auth/source/ldap/source_authenticate.go | 16 +- services/auth/source/ldap/source_sync.go | 32 +-- services/mailer/mail.go | 8 +- services/mailer/notify.go | 4 +- services/user/avatar.go | 2 +- services/user/email.go | 166 ++++++++++++++ services/user/email_test.go | 129 +++++++++++ services/user/update.go | 212 ++++++++++++++++++ services/user/update_test.go | 120 ++++++++++ services/user/user.go | 5 +- services/user/user_test.go | 2 +- tests/integration/api_admin_test.go | 10 +- tests/integration/api_nodeinfo_test.go | 2 +- tests/integration/api_user_email_test.go | 10 + tests/integration/empty_repo_test.go | 10 - 42 files changed, 1378 insertions(+), 1063 deletions(-) create mode 100644 modules/optional/option.go create mode 100644 modules/optional/option_test.go create mode 100644 services/user/email.go create mode 100644 services/user/email_test.go create mode 100644 services/user/update.go create mode 100644 services/user/update_test.go diff --git a/cmd/admin_user_change_password.go b/cmd/admin_user_change_password.go index 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")) - if err != nil { - return err - } - if pwned { - return errors.New("The password you chose is on a list of stolen passwords previously exposed in public data breaches. Please try again with a different password.\nFor more details, see https://haveibeenpwned.com/Passwords") - } - uname := c.String("username") - user, err := user_model.GetUserByName(ctx, uname) + user, err := user_model.GetUserByName(ctx, c.String("username")) if err != nil { return err } - if err = user.SetPassword(c.String("password")); err != nil { - return err - } + var mustChangePassword optional.Option[bool] if c.IsSet("must-change-password") { - user.MustChangePassword = c.Bool("must-change-password") + mustChangePassword = optional.Some(c.Bool("must-change-password")) } - if err = user_model.UpdateUserCols(ctx, user, "must_change_password", "passwd", "passwd_hash_algo", "salt"); err != nil { - return err + opts := &user_service.UpdateAuthOptions{ + Password: optional.Some(c.String("password")), + MustChangePassword: mustChangePassword, + } + if err := user_service.UpdateAuth(ctx, user, opts); err != nil { + switch { + case errors.Is(err, password.ErrMinLength): + return fmt.Errorf("Password is not long enough. Needs to be at least %d", setting.MinPasswordLength) + case errors.Is(err, password.ErrComplexity): + return errors.New("Password does not meet complexity requirements") + case errors.Is(err, password.ErrIsPwned): + return errors.New("The password you chose is on a list of stolen passwords previously exposed in public data breaches. Please try again with a different password.\nFor more details, see https://haveibeenpwned.com/Passwords") + default: + return err + } } fmt.Printf("%s's password has been successfully updated!\n", user.Name) 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} + } + + if count > 0 { + return ErrIsPwned } - return count > 0, nil + 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() { - return + 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 len(form.Password) != 0 { - if len(form.Password) < setting.MinPasswordLength { + 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)) - return - } - if !password.IsComplexEnough(form.Password) { - err := errors.New("PasswordComplexity") + case errors.Is(err, password.ErrComplexity): 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 + case errors.Is(err, password.ErrIsPwned), password.IsErrIsPwnedRequest(err): + ctx.Error(http.StatusBadRequest, "PasswordIsPwned", err) + default: + ctx.Error(http.StatusInternalServerError, "UpdateAuth", 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")) - 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")) + 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 } - 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,67 +346,110 @@ 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) { + case errors.Is(err, password.ErrComplexity): + ctx.Data["Err_Password"] = true ctx.RenderWithErr(password.BuildComplexityError(ctx.Locale), tplUserEdit, &form) - return - } - pwned, err := password.IsPwned(ctx, form.Password) - if pwned { + case errors.Is(err, password.ErrIsPwned): 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 + 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 := user_model.ValidateEmail(form.Email); err != nil { - ctx.Data["Err_Email"] = true - ctx.RenderWithErr(ctx.Tr("form.email_error"), tplUserEdit, &form) + 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 u.Salt, err = user_model.GetUserSalt(); err != nil { - ctx.ServerError("UpdateUser", err) - return - } - if err = u.SetPassword(form.Password); err != nil { - ctx.ServerError("SetPassword", err) - return - } + 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 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 + 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) } - u.Name = form.UserName - u.LowerName = strings.ToLower(form.UserName) + 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) @@ -433,52 +474,7 @@ 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 - } - if err = u.SetPassword(passwd); err != nil { - ctx.ServerError("UpdateUser", err) - return + + opts := &user_service.UpdateAuthOptions{ + Password: optional.Some(ctx.FormString("password")), + MustChangePassword: optional.Some(false), } - 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 - } - - if !password.IsComplexEnough(form.Password) { - ctx.Data["Err_Password"] = true - ctx.RenderWithErr(password.BuildComplexityError(ctx.Locale), tplMustChangePassword, &form) - return + opts := &user_service.UpdateAuthOptions{ + Password: optional.Some(form.Password), + MustChangePassword: optional.Some(false), } - 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 := form.Visibility != org.Visibility - org.Visibility = form.Visibility + visibilityChanged := 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) From 688d4a1f719d2df4d2626453f4bc042c1874a375 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Sun, 4 Feb 2024 15:05:26 +0100 Subject: [PATCH 3/5] Unify password changing and invalidate auth tokens (#27625) - Unify the password changing code - Invalidate existing auth tokens when changing passwords --- models/auth/auth_token.go | 5 +++++ services/user/delete.go | 4 ++++ services/user/update.go | 12 +++++++++++- 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/models/auth/auth_token.go b/models/auth/auth_token.go index 65f1b169eb2a..81f07d1a8382 100644 --- a/models/auth/auth_token.go +++ b/models/auth/auth_token.go @@ -54,6 +54,11 @@ func DeleteAuthTokenByID(ctx context.Context, id string) error { return err } +func DeleteAuthTokensByUserID(ctx context.Context, uid int64) error { + _, err := db.GetEngine(ctx).Where(builder.Eq{"user_id": uid}).Delete(&AuthToken{}) + return err +} + func DeleteExpiredAuthTokens(ctx context.Context) error { _, err := db.GetEngine(ctx).Where(builder.Lt{"expires_unix": timeutil.TimeStampNow()}).Delete(&AuthToken{}) return err diff --git a/services/user/delete.go b/services/user/delete.go index 0e9c86617146..000910319a6b 100644 --- a/services/user/delete.go +++ b/services/user/delete.go @@ -187,6 +187,10 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error) } // ***** END: ExternalLoginUser ***** + if err := auth_model.DeleteAuthTokensByUserID(ctx, u.ID); err != nil { + return fmt.Errorf("DeleteAuthTokensByUserID: %w", err) + } + if _, err = db.DeleteByID[user_model.User](ctx, u.ID); err != nil { return fmt.Errorf("delete: %w", err) } diff --git a/services/user/update.go b/services/user/update.go index 849757c8b0a4..cbaf90053a22 100644 --- a/services/user/update.go +++ b/services/user/update.go @@ -183,6 +183,7 @@ func UpdateAuth(ctx context.Context, u *user_model.User, opts *UpdateAuthOptions u.LoginName = opts.LoginName.Value() } + deleteAuthTokens := false if opts.Password.Has() && (u.IsLocal() || u.IsOAuth2()) { password := opts.Password.Value() @@ -199,6 +200,8 @@ func UpdateAuth(ctx context.Context, u *user_model.User, opts *UpdateAuthOptions if err := u.SetPassword(password); err != nil { return err } + + deleteAuthTokens = true } if opts.MustChangePassword.Has() { @@ -208,5 +211,12 @@ func UpdateAuth(ctx context.Context, u *user_model.User, opts *UpdateAuthOptions 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") + if err := user_model.UpdateUserCols(ctx, u, "login_type", "login_source", "login_name", "passwd", "passwd_hash_algo", "salt", "must_change_password", "prohibit_login"); err != nil { + return err + } + + if deleteAuthTokens { + return auth_model.DeleteAuthTokensByUserID(ctx, u.ID) + } + return nil } From 50f55f11c4f785b72a39e59b0fc12ae70ab8d8b5 Mon Sep 17 00:00:00 2001 From: Bram Hagens Date: Sun, 4 Feb 2024 23:37:45 +0100 Subject: [PATCH 4/5] Show whether a PR is WIP inside popups (#28975) Fixes https://codeberg.org/forgejo/forgejo/issues/2257 Draft status of a PR is currently not exposed by the API. This PR adds a 'draft' field to pull requests in the API, which is used to correctly set the PR color/icon in a ContextPopup. --- Before: ![image](https://github.com/go-gitea/gitea/assets/5541521/72cbd30e-1175-4338-aa97-ac99c46c5118) After: ![image](https://github.com/go-gitea/gitea/assets/5541521/111c9eba-460e-4d57-bcca-23a151c3a4f1) --- modules/structs/issue.go | 5 +++-- services/convert/issue.go | 3 ++- templates/swagger/v1_json.tmpl | 4 ++++ web_src/js/components/ContextPopup.vue | 16 ++++++++++++---- web_src/js/svg.js | 2 ++ 5 files changed, 23 insertions(+), 7 deletions(-) diff --git a/modules/structs/issue.go b/modules/structs/issue.go index 1aec5cc6b86c..34eae693299a 100644 --- a/modules/structs/issue.go +++ b/modules/structs/issue.go @@ -26,8 +26,9 @@ const ( // PullRequestMeta PR info if an issue is a PR type PullRequestMeta struct { - HasMerged bool `json:"merged"` - Merged *time.Time `json:"merged_at"` + HasMerged bool `json:"merged"` + Merged *time.Time `json:"merged_at"` + IsWorkInProgress bool `json:"draft"` } // RepositoryMeta basic repository information diff --git a/services/convert/issue.go b/services/convert/issue.go index 39d785e10833..c6e06180c838 100644 --- a/services/convert/issue.go +++ b/services/convert/issue.go @@ -98,7 +98,8 @@ func toIssue(ctx context.Context, issue *issues_model.Issue, getDownloadURL func } if issue.PullRequest != nil { apiIssue.PullRequest = &api.PullRequestMeta{ - HasMerged: issue.PullRequest.HasMerged, + HasMerged: issue.PullRequest.HasMerged, + IsWorkInProgress: issue.PullRequest.IsWorkInProgress(ctx), } if issue.PullRequest.HasMerged { apiIssue.PullRequest.Merged = issue.PullRequest.MergedUnix.AsTimePtr() diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index dc04a97b833c..403f241d7278 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -21610,6 +21610,10 @@ "description": "PullRequestMeta PR info if an issue is a PR", "type": "object", "properties": { + "draft": { + "type": "boolean", + "x-go-name": "IsWorkInProgress" + }, "merged": { "type": "boolean", "x-go-name": "HasMerged" diff --git a/web_src/js/components/ContextPopup.vue b/web_src/js/components/ContextPopup.vue index 303e6d0c8912..d9e6da316cbc 100644 --- a/web_src/js/components/ContextPopup.vue +++ b/web_src/js/components/ContextPopup.vue @@ -30,6 +30,9 @@ export default { icon() { if (this.issue.pull_request !== null) { if (this.issue.state === 'open') { + if (this.issue.pull_request.draft === true) { + return 'octicon-git-pull-request-draft'; // WIP PR + } return 'octicon-git-pull-request'; // Open PR } else if (this.issue.pull_request.merged === true) { return 'octicon-git-merge'; // Merged PR @@ -42,12 +45,17 @@ export default { }, color() { + if (this.issue.pull_request !== null) { + if (this.issue.pull_request.draft === true) { + return 'grey'; // WIP PR + } else if (this.issue.pull_request.merged === true) { + return 'purple'; // Merged PR + } + } if (this.issue.state === 'open') { - return 'green'; - } else if (this.issue.pull_request !== null && this.issue.pull_request.merged === true) { - return 'purple'; + return 'green'; // Open Issue } - return 'red'; + return 'red'; // Closed Issue }, labels() { diff --git a/web_src/js/svg.js b/web_src/js/svg.js index c2a96fba3f04..084256587c51 100644 --- a/web_src/js/svg.js +++ b/web_src/js/svg.js @@ -33,6 +33,7 @@ import octiconGitBranch from '../../public/assets/img/svg/octicon-git-branch.svg import octiconGitCommit from '../../public/assets/img/svg/octicon-git-commit.svg'; import octiconGitMerge from '../../public/assets/img/svg/octicon-git-merge.svg'; import octiconGitPullRequest from '../../public/assets/img/svg/octicon-git-pull-request.svg'; +import octiconGitPullRequestDraft from '../../public/assets/img/svg/octicon-git-pull-request-draft.svg'; import octiconHeading from '../../public/assets/img/svg/octicon-heading.svg'; import octiconHorizontalRule from '../../public/assets/img/svg/octicon-horizontal-rule.svg'; import octiconImage from '../../public/assets/img/svg/octicon-image.svg'; @@ -104,6 +105,7 @@ const svgs = { 'octicon-git-commit': octiconGitCommit, 'octicon-git-merge': octiconGitMerge, 'octicon-git-pull-request': octiconGitPullRequest, + 'octicon-git-pull-request-draft': octiconGitPullRequestDraft, 'octicon-heading': octiconHeading, 'octicon-horizontal-rule': octiconHorizontalRule, 'octicon-image': octiconImage, From 016c77a833d6843c2e18903d6c50a881018064f2 Mon Sep 17 00:00:00 2001 From: GiteaBot Date: Mon, 5 Feb 2024 00:25:25 +0000 Subject: [PATCH 5/5] [skip ci] Updated licenses and gitignores --- options/license/BSD-2-Clause-Darwin | 28 ++++++++++++++ options/license/BSD-3-Clause-acpica | 26 +++++++++++++ options/license/OpenSSL-standalone | 50 +++++++++++++++++++++++++ options/license/SSLeay-standalone | 58 +++++++++++++++++++++++++++++ 4 files changed, 162 insertions(+) create mode 100644 options/license/BSD-2-Clause-Darwin create mode 100644 options/license/BSD-3-Clause-acpica create mode 100644 options/license/OpenSSL-standalone create mode 100644 options/license/SSLeay-standalone diff --git a/options/license/BSD-2-Clause-Darwin b/options/license/BSD-2-Clause-Darwin new file mode 100644 index 000000000000..d582399763d0 --- /dev/null +++ b/options/license/BSD-2-Clause-Darwin @@ -0,0 +1,28 @@ +Copyright (c) Ian F. Darwin 1986, 1987, 1989, 1990, 1991, 1992, 1994, 1995. +Software written by Ian F. Darwin and others; +maintained 1994- Christos Zoulas. + +This software is not subject to any export provision of the United States +Department of Commerce, and may be exported to any country or planet. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions +are met: +1. Redistributions of source code must retain the above copyright + notice immediately at the beginning of the file, without modification, + this list of conditions, and the following disclaimer. +2. Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + +THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND +ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE FOR +ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS +OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) +HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT +LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY +OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF +SUCH DAMAGE. diff --git a/options/license/BSD-3-Clause-acpica b/options/license/BSD-3-Clause-acpica new file mode 100644 index 000000000000..9fb56c585a8f --- /dev/null +++ b/options/license/BSD-3-Clause-acpica @@ -0,0 +1,26 @@ +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions +are met: +1. Redistributions of source code must retain the above copyright + notice, this list of conditions, and the following disclaimer, + without modification. +2. Redistributions in binary form must reproduce at minimum a disclaimer + substantially similar to the "NO WARRANTY" disclaimer below + ("Disclaimer") and any redistribution must be conditioned upon + including a substantially similar Disclaimer requirement for further + binary redistribution. +3. Neither the names of the above-listed copyright holders nor the names + of any contributors may be used to endorse or promote products derived + from this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/options/license/OpenSSL-standalone b/options/license/OpenSSL-standalone new file mode 100644 index 000000000000..82b14c736d8c --- /dev/null +++ b/options/license/OpenSSL-standalone @@ -0,0 +1,50 @@ +Copyright (c) 1998-2019 The OpenSSL Project. All rights reserved. + + Redistribution and use in source and binary forms, with or without + modification, are permitted provided that the following conditions + are met: + + 1. Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + + 2. Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in + the documentation and/or other materials provided with the + distribution. + + 3. All advertising materials mentioning features or use of this + software must display the following acknowledgment: + "This product includes software developed by the OpenSSL Project + for use in the OpenSSL Toolkit. (http://www.openssl.org/)" + + 4. The names "OpenSSL Toolkit" and "OpenSSL Project" must not be used to + endorse or promote products derived from this software without + prior written permission. For written permission, please contact + openssl-core@openssl.org. + + 5. Products derived from this software may not be called "OpenSSL" + nor may "OpenSSL" appear in their names without prior written + permission of the OpenSSL Project. + + 6. Redistributions of any form whatsoever must retain the following + acknowledgment: + "This product includes software developed by the OpenSSL Project + for use in the OpenSSL Toolkit (http://www.openssl.org/)" + + THIS SOFTWARE IS PROVIDED BY THE OpenSSL PROJECT ``AS IS'' AND ANY + EXPRESSED OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR + PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE OpenSSL PROJECT OR + ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED + OF THE POSSIBILITY OF SUCH DAMAGE. + ==================================================================== + + This product includes cryptographic software written by Eric Young + (eay@cryptsoft.com). This product includes software written by Tim + Hudson (tjh@cryptsoft.com). diff --git a/options/license/SSLeay-standalone b/options/license/SSLeay-standalone new file mode 100644 index 000000000000..61618b40eb2a --- /dev/null +++ b/options/license/SSLeay-standalone @@ -0,0 +1,58 @@ +Original SSLeay License + ----------------------- + + Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) + All rights reserved. + + This package is an SSL implementation written + by Eric Young (eay@cryptsoft.com). + The implementation was written so as to conform with Netscapes SSL. + + This library is free for commercial and non-commercial use as long as + the following conditions are aheared to. The following conditions + apply to all code found in this distribution, be it the RC4, RSA, + lhash, DES, etc., code; not just the SSL code. The SSL documentation + included with this distribution is covered by the same copyright terms + except that the holder is Tim Hudson (tjh@cryptsoft.com). + + Copyright remains Eric Young's, and as such any Copyright notices in + the code are not to be removed. + If this package is used in a product, Eric Young should be given attribution + as the author of the parts of the library used. + This can be in the form of a textual message at program startup or + in documentation (online or textual) provided with the package. + + Redistribution and use in source and binary forms, with or without + modification, are permitted provided that the following conditions + are met: + 1. Redistributions of source code must retain the copyright + notice, this list of conditions and the following disclaimer. + 2. Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + 3. All advertising materials mentioning features or use of this software + must display the following acknowledgement: + "This product includes cryptographic software written by + Eric Young (eay@cryptsoft.com)" + The word 'cryptographic' can be left out if the rouines from the library + being used are not cryptographic related :-). + 4. If you include any Windows specific code (or a derivative thereof) from + the apps directory (application code) you must include an acknowledgement: + "This product includes software written by Tim Hudson (tjh@cryptsoft.com)" + + THIS SOFTWARE IS PROVIDED BY ERIC YOUNG ``AS IS'' AND + ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE + FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + SUCH DAMAGE. + + The licence and distribution terms for any publically available version or + derivative of this code cannot be changed. i.e. this code cannot simply be + copied and put under another distribution licence + [including the GNU Public Licence.]