From ae20d03aece78fb44dc1caaacfa40c3aa40c7949 Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Sun, 27 Nov 2022 19:36:10 +0800 Subject: [PATCH] refactor(db): migrate `UpdateUser` off `user.go` (#7267) --- CHANGELOG.md | 1 + internal/db/repo.go | 26 ++++-- internal/db/user.go | 45 --------- internal/db/user_mail.go | 22 +---- internal/db/users.go | 136 +++++++++++++++++++++++----- internal/db/users_test.go | 107 +++++++++++++++++----- internal/email/message.go | 4 + internal/form/user.go | 4 +- internal/route/admin/users.go | 49 +++++----- internal/route/api/v1/admin/user.go | 57 ++++++------ internal/route/api/v1/org/org.go | 23 +++-- internal/route/org/setting.go | 15 ++- internal/route/repo/setting.go | 2 +- internal/route/user/auth.go | 48 +++++----- internal/route/user/setting.go | 45 +++++---- 15 files changed, 350 insertions(+), 234 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9603806bf..17abb9f84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ All notable changes to Gogs are documented in this file. ### Fixed - Unable to use LDAP authentication on ARM machines. [#6761](https://github.com/gogs/gogs/issues/6761) +- Unable to choose "Lookup Avatar by mail" in user settings without deleting custom avatar. [#7267](https://github.com/gogs/gogs/pull/7267) ### Removed diff --git a/internal/db/repo.go b/internal/db/repo.go index 8abe47edd..896c39b8c 100644 --- a/internal/db/repo.go +++ b/internal/db/repo.go @@ -34,6 +34,7 @@ import ( "gogs.io/gogs/internal/avatar" "gogs.io/gogs/internal/conf" dberrors "gogs.io/gogs/internal/db/errors" + "gogs.io/gogs/internal/dbutil" "gogs.io/gogs/internal/errutil" "gogs.io/gogs/internal/markup" "gogs.io/gogs/internal/osutil" @@ -1112,11 +1113,9 @@ func createRepository(e *xorm.Session, doer, owner *User, repo *Repository) (err return err } - owner.NumRepos++ - // Remember visibility preference. - owner.LastRepoVisibility = repo.IsPrivate - if err = updateUser(e, owner); err != nil { - return fmt.Errorf("updateUser: %v", err) + _, err = e.Exec(dbutil.Quote("UPDATE %s SET num_repos = num_repos + 1 WHERE id = ?", "user"), owner.ID) + if err != nil { + return errors.Wrap(err, "increase owned repository count") } // Give access to all members in owner team. @@ -1222,8 +1221,17 @@ func CreateRepository(doer, owner *User, opts CreateRepoOptionsLegacy) (_ *Repos return nil, fmt.Errorf("CreateRepository 'git update-server-info': %s", stderr) } } + if err = sess.Commit(); err != nil { + return nil, err + } - return repo, sess.Commit() + // Remember visibility preference + err = Users.Update(context.TODO(), owner.ID, UpdateUserOptions{LastRepoVisibility: &repo.IsPrivate}) + if err != nil { + return nil, errors.Wrap(err, "update user") + } + + return repo, nil } func countRepositories(userID int64, private bool) int64 { @@ -2544,6 +2552,12 @@ func ForkRepository(doer, owner *User, baseRepo *Repository, name, desc string) return nil, fmt.Errorf("Commit: %v", err) } + // Remember visibility preference + err = Users.Update(context.TODO(), owner.ID, UpdateUserOptions{LastRepoVisibility: &repo.IsPrivate}) + if err != nil { + return nil, errors.Wrap(err, "update user") + } + if err = repo.UpdateSize(); err != nil { log.Error("UpdateSize [repo_id: %d]: %v", repo.ID, err) } diff --git a/internal/db/user.go b/internal/db/user.go index cc2de95b5..c2091e6f4 100644 --- a/internal/db/user.go +++ b/internal/db/user.go @@ -19,10 +19,7 @@ import ( "gogs.io/gogs/internal/conf" "gogs.io/gogs/internal/db/errors" - "gogs.io/gogs/internal/errutil" "gogs.io/gogs/internal/repoutil" - "gogs.io/gogs/internal/strutil" - "gogs.io/gogs/internal/tool" "gogs.io/gogs/internal/userutil" ) @@ -42,48 +39,6 @@ func (u *User) AfterSet(colName string, _ xorm.Cell) { } } -// TODO(unknwon): Update call sites to use refactored methods and delete this one. -func updateUser(e Engine, u *User) error { - // Organization does not need email - if !u.IsOrganization() { - u.Email = strings.ToLower(u.Email) - has, err := e.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{args: errutil.Args{"email": u.Email}} - } - - if u.AvatarEmail == "" { - u.AvatarEmail = u.Email - } - u.Avatar = tool.HashEmail(u.AvatarEmail) - } - - u.LowerName = strings.ToLower(u.Name) - u.Location = strutil.Truncate(u.Location, 255) - u.Website = strutil.Truncate(u.Website, 255) - u.Description = strutil.Truncate(u.Description, 255) - - _, err := e.ID(u.ID).AllCols().Update(u) - return err -} - -// TODO(unknwon): Refactoring together with methods that do updates. -func (u *User) BeforeUpdate() { - if u.MaxRepoCreation < -1 { - u.MaxRepoCreation = -1 - } - u.UpdatedUnix = time.Now().Unix() -} - -// UpdateUser updates user's information. -// -// TODO(unknwon): Update call sites to use refactored methods and delete this one. -func UpdateUser(u *User) error { - return updateUser(x, u) -} - // deleteBeans deletes all given beans, beans should contain delete conditions. func deleteBeans(e Engine, beans ...interface{}) (err error) { for i := range beans { diff --git a/internal/db/user_mail.go b/internal/db/user_mail.go index eae79af71..d657faa0e 100644 --- a/internal/db/user_mail.go +++ b/internal/db/user_mail.go @@ -11,7 +11,6 @@ import ( "gogs.io/gogs/internal/db/errors" "gogs.io/gogs/internal/errutil" - "gogs.io/gogs/internal/userutil" ) // EmailAddresses is the list of all email addresses of a user. Can contain the @@ -120,28 +119,11 @@ func AddEmailAddresses(emails []*EmailAddress) error { } func (email *EmailAddress) Activate() error { - user, err := Users.GetByID(context.TODO(), email.UserID) - if err != nil { - return err - } - if user.Rands, err = userutil.RandomSalt(); err != nil { - return err - } - - sess := x.NewSession() - defer sess.Close() - if err = sess.Begin(); err != nil { - return err - } - email.IsActivated = true - if _, err := sess.ID(email.ID).AllCols().Update(email); err != nil { - return err - } else if err = updateUser(sess, user); err != nil { + if _, err := x.ID(email.ID).AllCols().Update(email); err != nil { return err } - - return sess.Commit() + return Users.Update(context.TODO(), email.UserID, UpdateUserOptions{GenerateNewRands: true}) } func DeleteEmailAddress(email *EmailAddress) (err error) { diff --git a/internal/db/users.go b/internal/db/users.go index 9e5687d43..65326a950 100644 --- a/internal/db/users.go +++ b/internal/db/users.go @@ -87,8 +87,7 @@ type UsersStore interface { // Results are paginated by given page and page size, and sorted by the time of // follow in descending order. ListFollowings(ctx context.Context, userID int64, page, pageSize int) ([]*User, error) - // Update updates all fields for the given user, all values are persisted as-is - // (i.e. empty values would overwrite/wipe out existing values). + // Update updates fields for the given user. Update(ctx context.Context, userID int64, opts UpdateUserOptions) error // UseCustomAvatar uses the given avatar as the user custom avatar. UseCustomAvatar(ctx context.Context, userID int64, avatar []byte) error @@ -324,8 +323,10 @@ type ErrEmailAlreadyUsed struct { args errutil.Args } +// IsErrEmailAlreadyUsed returns true if the underlying error has the type +// ErrEmailAlreadyUsed. func IsErrEmailAlreadyUsed(err error) bool { - _, ok := err.(ErrEmailAlreadyUsed) + _, ok := errors.Cause(err).(ErrEmailAlreadyUsed) return ok } @@ -415,8 +416,10 @@ type ErrUserNotExist struct { args errutil.Args } +// IsErrUserNotExist returns true if the underlying error has the type +// ErrUserNotExist. func IsErrUserNotExist(err error) bool { - _, ok := err.(ErrUserNotExist) + _, ok := errors.Cause(err).(ErrUserNotExist) return ok } @@ -549,30 +552,117 @@ func (db *users) ListFollowings(ctx context.Context, userID int64, page, pageSiz } type UpdateUserOptions struct { - FullName string - Website string - Location string - Description string + LoginSource *int64 + LoginName *string - MaxRepoCreation int + Password *string + // GenerateNewRands indicates whether to force generate new rands for the user. + GenerateNewRands bool + + FullName *string + Email *string + Website *string + Location *string + Description *string + + MaxRepoCreation *int + LastRepoVisibility *bool + + IsActivated *bool + IsAdmin *bool + AllowGitHook *bool + AllowImportLocal *bool + ProhibitLogin *bool + + Avatar *string + AvatarEmail *string } func (db *users) Update(ctx context.Context, userID int64, opts UpdateUserOptions) error { - if opts.MaxRepoCreation < -1 { - opts.MaxRepoCreation = -1 + updates := map[string]any{ + "updated_unix": db.NowFunc().Unix(), } - return db.WithContext(ctx). - Model(&User{}). - Where("id = ?", userID). - Updates(map[string]any{ - "full_name": strutil.Truncate(opts.FullName, 255), - "website": strutil.Truncate(opts.Website, 255), - "location": strutil.Truncate(opts.Location, 255), - "description": strutil.Truncate(opts.Description, 255), - "max_repo_creation": opts.MaxRepoCreation, - "updated_unix": db.NowFunc().Unix(), - }). - Error + + if opts.LoginSource != nil { + updates["login_source"] = *opts.LoginSource + } + if opts.LoginName != nil { + updates["login_name"] = *opts.LoginName + } + + if opts.Password != nil { + salt, err := userutil.RandomSalt() + if err != nil { + return errors.Wrap(err, "generate salt") + } + updates["salt"] = salt + updates["passwd"] = userutil.EncodePassword(*opts.Password, salt) + opts.GenerateNewRands = true + } + if opts.GenerateNewRands { + rands, err := userutil.RandomSalt() + if err != nil { + return errors.Wrap(err, "generate rands") + } + updates["rands"] = rands + } + + if opts.FullName != nil { + updates["full_name"] = strutil.Truncate(*opts.FullName, 255) + } + if opts.Email != nil { + _, err := db.GetByEmail(ctx, *opts.Email) + if err == nil { + return ErrEmailAlreadyUsed{args: errutil.Args{"email": *opts.Email}} + } else if !IsErrUserNotExist(err) { + return errors.Wrap(err, "check email") + } + updates["email"] = *opts.Email + } + if opts.Website != nil { + updates["website"] = strutil.Truncate(*opts.Website, 255) + } + if opts.Location != nil { + updates["location"] = strutil.Truncate(*opts.Location, 255) + } + if opts.Description != nil { + updates["description"] = strutil.Truncate(*opts.Description, 255) + } + + if opts.MaxRepoCreation != nil { + if *opts.MaxRepoCreation < -1 { + *opts.MaxRepoCreation = -1 + } + updates["max_repo_creation"] = *opts.MaxRepoCreation + } + if opts.LastRepoVisibility != nil { + updates["last_repo_visibility"] = *opts.LastRepoVisibility + } + + if opts.IsActivated != nil { + updates["is_active"] = *opts.IsActivated + } + if opts.IsAdmin != nil { + updates["is_admin"] = *opts.IsAdmin + } + if opts.AllowGitHook != nil { + updates["allow_git_hook"] = *opts.AllowGitHook + } + if opts.AllowImportLocal != nil { + updates["allow_import_local"] = *opts.AllowImportLocal + } + if opts.ProhibitLogin != nil { + updates["prohibit_login"] = *opts.ProhibitLogin + } + + if opts.Avatar != nil { + updates["avatar"] = strutil.Truncate(*opts.Avatar, 2048) + } + if opts.AvatarEmail != nil { + updates["avatar_email"] = strutil.Truncate(*opts.AvatarEmail, 255) + } + + return db.WithContext(ctx).Model(&User{}).Where("id = ?", userID).Updates(updates).Error } func (db *users) UseCustomAvatar(ctx context.Context, userID int64, avatar []byte) error { diff --git a/internal/db/users_test.go b/internal/db/users_test.go index 2ce0efd9a..4cf5aaf49 100644 --- a/internal/db/users_test.go +++ b/internal/db/users_test.go @@ -731,16 +731,69 @@ func usersListFollowings(t *testing.T, db *users) { func usersUpdate(t *testing.T, db *users) { ctx := context.Background() - alice, err := db.Create(ctx, "alice", "alice@example.com", CreateUserOptions{}) + const oldPassword = "Password" + alice, err := db.Create( + ctx, + "alice", + "alice@example.com", + CreateUserOptions{ + FullName: "FullName", + Password: oldPassword, + LoginSource: 9, + LoginName: "LoginName", + Location: "Location", + Website: "Website", + Activated: false, + Admin: false, + }, + ) require.NoError(t, err) - overLimitStr := strings.Repeat("a", 300) + t.Run("update password", func(t *testing.T) { + got := userutil.ValidatePassword(alice.Password, alice.Salt, oldPassword) + require.True(t, got) + + newPassword := "NewPassword" + err = db.Update(ctx, alice.ID, UpdateUserOptions{Password: &newPassword}) + require.NoError(t, err) + alice, err = db.GetByID(ctx, alice.ID) + require.NoError(t, err) + + got = userutil.ValidatePassword(alice.Password, alice.Salt, oldPassword) + assert.False(t, got, "Old password should stop working") + + got = userutil.ValidatePassword(alice.Password, alice.Salt, newPassword) + assert.True(t, got, "New password should work") + }) + + t.Run("update email but already used", func(t *testing.T) { + // todo + }) + + loginSource := int64(1) + maxRepoCreation := 99 + lastRepoVisibility := true + overLimitStr := strings.Repeat("a", 2050) opts := UpdateUserOptions{ - FullName: overLimitStr, - Website: overLimitStr, - Location: overLimitStr, - Description: overLimitStr, - MaxRepoCreation: 1, + LoginSource: &loginSource, + LoginName: &alice.Name, + + FullName: &overLimitStr, + Website: &overLimitStr, + Location: &overLimitStr, + Description: &overLimitStr, + + MaxRepoCreation: &maxRepoCreation, + LastRepoVisibility: &lastRepoVisibility, + + IsActivated: &lastRepoVisibility, + IsAdmin: &lastRepoVisibility, + AllowGitHook: &lastRepoVisibility, + AllowImportLocal: &lastRepoVisibility, + ProhibitLogin: &lastRepoVisibility, + + Avatar: &overLimitStr, + AvatarEmail: &overLimitStr, } err = db.Update(ctx, alice.ID, opts) require.NoError(t, err) @@ -748,28 +801,34 @@ func usersUpdate(t *testing.T, db *users) { alice, err = db.GetByID(ctx, alice.ID) require.NoError(t, err) - wantStr := strings.Repeat("a", 255) - assert.Equal(t, wantStr, alice.FullName) - assert.Equal(t, wantStr, alice.Website) - assert.Equal(t, wantStr, alice.Location) - assert.Equal(t, wantStr, alice.Description) - assert.Equal(t, 1, alice.MaxRepoCreation) - - // Test empty values - opts = UpdateUserOptions{ - FullName: "Alice John", - Website: "https://gogs.io", + assertValues := func() { + assert.Equal(t, loginSource, alice.LoginSource) + assert.Equal(t, alice.Name, alice.LoginName) + wantStr255 := strings.Repeat("a", 255) + assert.Equal(t, wantStr255, alice.FullName) + assert.Equal(t, wantStr255, alice.Website) + assert.Equal(t, wantStr255, alice.Location) + assert.Equal(t, wantStr255, alice.Description) + assert.Equal(t, maxRepoCreation, alice.MaxRepoCreation) + assert.Equal(t, lastRepoVisibility, alice.LastRepoVisibility) + assert.Equal(t, lastRepoVisibility, alice.IsActive) + assert.Equal(t, lastRepoVisibility, alice.IsAdmin) + assert.Equal(t, lastRepoVisibility, alice.AllowGitHook) + assert.Equal(t, lastRepoVisibility, alice.AllowImportLocal) + assert.Equal(t, lastRepoVisibility, alice.ProhibitLogin) + wantStr2048 := strings.Repeat("a", 2048) + assert.Equal(t, wantStr2048, alice.Avatar) + assert.Equal(t, wantStr255, alice.AvatarEmail) } - err = db.Update(ctx, alice.ID, opts) + assertValues() + + // Test ignored values + err = db.Update(ctx, alice.ID, UpdateUserOptions{}) require.NoError(t, err) alice, err = db.GetByID(ctx, alice.ID) require.NoError(t, err) - assert.Equal(t, opts.FullName, alice.FullName) - assert.Equal(t, opts.Website, alice.Website) - assert.Empty(t, alice.Location) - assert.Empty(t, alice.Description) - assert.Empty(t, alice.MaxRepoCreation) + assertValues() } func usersUseCustomAvatar(t *testing.T, db *users) { diff --git a/internal/email/message.go b/internal/email/message.go index f7266cf7c..4537e7eaf 100644 --- a/internal/email/message.go +++ b/internal/email/message.go @@ -232,6 +232,10 @@ func NewContext() { // It returns without confirmation (mail processed asynchronously) in normal cases, // but waits/blocks under hook mode to make sure mail has been sent. func Send(msg *Message) { + if !conf.Email.Enabled { + return + } + mailQueue <- msg if conf.HookMode { diff --git a/internal/form/user.go b/internal/form/user.go index 539dfd98a..fee0569df 100644 --- a/internal/form/user.go +++ b/internal/form/user.go @@ -104,8 +104,8 @@ func (f *UpdateProfile) Validate(ctx *macaron.Context, errs binding.Errors) bind } const ( - AVATAR_LOCAL string = "local" - AVATAR_BYMAIL string = "bymail" + AvatarLocal string = "local" + AvatarLookup string = "lookup" ) type Avatar struct { diff --git a/internal/route/admin/users.go b/internal/route/admin/users.go index 592f3feea..9b47b5fcd 100644 --- a/internal/route/admin/users.go +++ b/internal/route/admin/users.go @@ -8,7 +8,6 @@ import ( "strconv" "strings" - "github.com/unknwon/com" log "unknwon.dev/clog/v2" "gogs.io/gogs/internal/conf" @@ -17,7 +16,6 @@ import ( "gogs.io/gogs/internal/email" "gogs.io/gogs/internal/form" "gogs.io/gogs/internal/route" - "gogs.io/gogs/internal/userutil" ) const ( @@ -176,38 +174,37 @@ func EditUserPost(c *context.Context, f form.AdminEditUser) { return } + opts := db.UpdateUserOptions{ + LoginName: &f.LoginName, + FullName: &f.FullName, + Website: &f.Website, + Location: &f.Location, + MaxRepoCreation: &f.MaxRepoCreation, + IsActivated: &f.Active, + IsAdmin: &f.Admin, + AllowGitHook: &f.AllowGitHook, + AllowImportLocal: &f.AllowImportLocal, + ProhibitLogin: &f.ProhibitLogin, + } + fields := strings.Split(f.LoginType, "-") if len(fields) == 2 { - loginSource := com.StrTo(fields[1]).MustInt64() - + loginSource, _ := strconv.ParseInt(fields[1], 10, 64) if u.LoginSource != loginSource { - u.LoginSource = loginSource + opts.LoginSource = &loginSource } } - if len(f.Password) > 0 { - u.Password = f.Password - var err error - if u.Salt, err = userutil.RandomSalt(); err != nil { - c.Error(err, "get user salt") - return - } - u.Password = userutil.EncodePassword(u.Password, u.Salt) + if f.Password != "" { + opts.Password = &f.Password } - u.LoginName = f.LoginName - u.FullName = f.FullName - u.Email = f.Email - u.Website = f.Website - u.Location = f.Location - u.MaxRepoCreation = f.MaxRepoCreation - u.IsActive = f.Active - u.IsAdmin = f.Admin - u.AllowGitHook = f.AllowGitHook - u.AllowImportLocal = f.AllowImportLocal - u.ProhibitLogin = f.ProhibitLogin + if u.Email != f.Email { + opts.Email = &f.Email + } - if err := db.UpdateUser(u); err != nil { + err := db.Users.Update(c.Req.Context(), u.ID, opts) + if err != nil { if db.IsErrEmailAlreadyUsed(err) { c.Data["Err_Email"] = true c.RenderWithErr(c.Tr("form.email_been_used"), USER_EDIT, &f) @@ -216,7 +213,7 @@ func EditUserPost(c *context.Context, f form.AdminEditUser) { } return } - log.Trace("Account profile updated by admin (%s): %s", c.User.Name, u.Name) + log.Trace("Account updated by admin %q: %s", c.User.Name, u.Name) c.Flash.Success(c.Tr("admin.users.update_profile_success")) c.Redirect(conf.Server.Subpath + "/admin/users/" + c.Params(":userid")) diff --git a/internal/route/api/v1/admin/user.go b/internal/route/api/v1/admin/user.go index c7c207af8..97f7a0bb7 100644 --- a/internal/route/api/v1/admin/user.go +++ b/internal/route/api/v1/admin/user.go @@ -15,7 +15,6 @@ import ( "gogs.io/gogs/internal/db" "gogs.io/gogs/internal/email" "gogs.io/gogs/internal/route/api/v1/user" - "gogs.io/gogs/internal/userutil" ) func parseLoginSource(c *context.APIContext, sourceID int64) { @@ -83,39 +82,30 @@ func EditUser(c *context.APIContext, form api.EditUserOption) { return } - if len(form.Password) > 0 { - u.Password = form.Password - var err error - if u.Salt, err = userutil.RandomSalt(); err != nil { - c.Error(err, "get user salt") - return - } - u.Password = userutil.EncodePassword(u.Password, u.Salt) + opts := db.UpdateUserOptions{ + LoginSource: &form.SourceID, + LoginName: &form.LoginName, + FullName: &form.FullName, + Website: &form.Website, + Location: &form.Location, + MaxRepoCreation: form.MaxRepoCreation, + IsActivated: form.Active, + IsAdmin: form.Admin, + AllowGitHook: form.AllowGitHook, + AllowImportLocal: form.AllowImportLocal, + ProhibitLogin: nil, // TODO: Add this option to API } - u.LoginSource = form.SourceID - u.LoginName = form.LoginName - u.FullName = form.FullName - u.Email = form.Email - u.Website = form.Website - u.Location = form.Location - if form.Active != nil { - u.IsActive = *form.Active - } - if form.Admin != nil { - u.IsAdmin = *form.Admin - } - if form.AllowGitHook != nil { - u.AllowGitHook = *form.AllowGitHook - } - if form.AllowImportLocal != nil { - u.AllowImportLocal = *form.AllowImportLocal - } - if form.MaxRepoCreation != nil { - u.MaxRepoCreation = *form.MaxRepoCreation + if form.Password != "" { + opts.Password = &form.Password } - if err := db.UpdateUser(u); err != nil { + if u.Email != form.Email { + opts.Email = &form.Email + } + + err := db.Users.Update(c.Req.Context(), u.ID, opts) + if err != nil { if db.IsErrEmailAlreadyUsed(err) { c.ErrorStatus(http.StatusUnprocessableEntity, err) } else { @@ -123,8 +113,13 @@ func EditUser(c *context.APIContext, form api.EditUserOption) { } return } - log.Trace("Account profile updated by admin %q: %s", c.User.Name, u.Name) + log.Trace("Account updated by admin %q: %s", c.User.Name, u.Name) + u, err = db.Users.GetByID(c.Req.Context(), u.ID) + if err != nil { + c.Error(err, "get user") + return + } c.JSONSuccess(u.APIFormat()) } diff --git a/internal/route/api/v1/org/org.go b/internal/route/api/v1/org/org.go index 2f6b4b70b..9b99bead7 100644 --- a/internal/route/api/v1/org/org.go +++ b/internal/route/api/v1/org/org.go @@ -89,14 +89,25 @@ func Edit(c *context.APIContext, form api.EditOrgOption) { return } - org.FullName = form.FullName - org.Description = form.Description - org.Website = form.Website - org.Location = form.Location - if err := db.UpdateUser(org); err != nil { - c.Error(err, "update user") + err := db.Users.Update( + c.Req.Context(), + c.Org.Organization.ID, + db.UpdateUserOptions{ + FullName: &form.FullName, + Website: &form.Website, + Location: &form.Location, + Description: &form.Description, + }, + ) + if err != nil { + c.Error(err, "update organization") return } + org, err = db.GetOrgByName(org.Name) + if err != nil { + c.Error(err, "get organization") + return + } c.JSONSuccess(convert.ToOrganization(org)) } diff --git a/internal/route/org/setting.go b/internal/route/org/setting.go index 0cfc24546..e632e8a7a 100644 --- a/internal/route/org/setting.go +++ b/internal/route/org/setting.go @@ -63,16 +63,15 @@ func SettingsPost(c *context.Context, f form.UpdateOrgSetting) { } opts := db.UpdateUserOptions{ - FullName: f.FullName, - Website: f.Website, - Location: f.Location, - Description: f.Description, - MaxRepoCreation: org.MaxRepoCreation, + FullName: &f.FullName, + Website: &f.Website, + Location: &f.Location, + Description: &f.Description, } if c.User.IsAdmin { - opts.MaxRepoCreation = f.MaxRepoCreation + opts.MaxRepoCreation = &f.MaxRepoCreation } - err := db.Users.Update(c.Req.Context(), c.User.ID, opts) + err := db.Users.Update(c.Req.Context(), c.Org.Organization.ID, opts) if err != nil { c.Error(err, "update organization") return @@ -83,7 +82,7 @@ func SettingsPost(c *context.Context, f form.UpdateOrgSetting) { } func SettingsAvatar(c *context.Context, f form.Avatar) { - f.Source = form.AVATAR_LOCAL + f.Source = form.AvatarLocal if err := user.UpdateAvatarSetting(c, f, c.Org.Organization); err != nil { c.Flash.Error(err.Error()) } else { diff --git a/internal/route/repo/setting.go b/internal/route/repo/setting.go index e5e4fe6f7..c02254eeb 100644 --- a/internal/route/repo/setting.go +++ b/internal/route/repo/setting.go @@ -309,7 +309,7 @@ func SettingsAvatar(c *context.Context) { } func SettingsAvatarPost(c *context.Context, f form.Avatar) { - f.Source = form.AVATAR_LOCAL + f.Source = form.AvatarLocal if err := UpdateAvatarSetting(c, f, c.Repo.Repository); err != nil { c.Flash.Error(err.Error()) } else { diff --git a/internal/route/user/auth.go b/internal/route/user/auth.go index 8e63d9147..ff0febb9c 100644 --- a/internal/route/user/auth.go +++ b/internal/route/user/auth.go @@ -367,9 +367,16 @@ func SignUpPost(c *context.Context, cpt *captcha.Captcha, f form.Register) { // // Auto-set admin for the only user. if db.Users.Count(c.Req.Context()) == 1 { - user.IsAdmin = true - user.IsActive = true - if err := db.UpdateUser(user); err != nil { + v := true + err := db.Users.Update( + c.Req.Context(), + user.ID, + db.UpdateUserOptions{ + IsActivated: &v, + IsAdmin: &v, + }, + ) + if err != nil { c.Error(err, "update user") return } @@ -476,13 +483,16 @@ func Activate(c *context.Context) { // Verify code. if user := verifyUserActiveCode(code); user != nil { - user.IsActive = true - var err error - if user.Rands, err = userutil.RandomSalt(); err != nil { - c.Error(err, "get user salt") - return - } - if err := db.UpdateUser(user); err != nil { + v := true + err := db.Users.Update( + c.Req.Context(), + user.ID, + db.UpdateUserOptions{ + GenerateNewRands: true, + IsActivated: &v, + }, + ) + if err != nil { c.Error(err, "update user") return } @@ -601,26 +611,16 @@ func ResetPasswdPost(c *context.Context) { if u := verifyUserActiveCode(code); u != nil { // Validate password length. - passwd := c.Query("password") - if len(passwd) < 6 { + password := c.Query("password") + if len(password) < 6 { c.Data["IsResetForm"] = true c.Data["Err_Password"] = true c.RenderWithErr(c.Tr("auth.password_too_short"), RESET_PASSWORD, nil) return } - u.Password = passwd - var err error - if u.Rands, err = userutil.RandomSalt(); err != nil { - c.Error(err, "get user salt") - return - } - if u.Salt, err = userutil.RandomSalt(); err != nil { - c.Error(err, "get user salt") - return - } - u.Password = userutil.EncodePassword(u.Password, u.Salt) - if err := db.UpdateUser(u); err != nil { + err := db.Users.Update(c.Req.Context(), u.ID, db.UpdateUserOptions{Password: &password}) + if err != nil { c.Error(err, "update user") return } diff --git a/internal/route/user/setting.go b/internal/route/user/setting.go index fbb7c9ae3..0f5a62bf5 100644 --- a/internal/route/user/setting.go +++ b/internal/route/user/setting.go @@ -96,10 +96,9 @@ func SettingsPost(c *context.Context, f form.UpdateProfile) { c.Req.Context(), c.User.ID, db.UpdateUserOptions{ - FullName: f.FullName, - Website: f.Website, - Location: f.Location, - MaxRepoCreation: c.User.MaxRepoCreation, + FullName: &f.FullName, + Website: &f.Website, + Location: &f.Location, }, ) if err != nil { @@ -113,13 +112,23 @@ func SettingsPost(c *context.Context, f form.UpdateProfile) { // FIXME: limit upload size func UpdateAvatarSetting(c *context.Context, f form.Avatar, ctxUser *db.User) error { - if f.Source == form.AVATAR_BYMAIL && len(f.Gravatar) > 0 { - ctxUser.UseCustomAvatar = false - ctxUser.Avatar = cryptoutil.MD5(f.Gravatar) - ctxUser.AvatarEmail = f.Gravatar + if f.Source == form.AvatarLookup && f.Gravatar != "" { + avatar := cryptoutil.MD5(f.Gravatar) + err := db.Users.Update( + c.Req.Context(), + ctxUser.ID, + db.UpdateUserOptions{ + Avatar: &avatar, + AvatarEmail: &f.Gravatar, + }, + ) + if err != nil { + return errors.Wrap(err, "update user") + } - if err := db.UpdateUser(ctxUser); err != nil { - return fmt.Errorf("update user: %v", err) + err = db.Users.DeleteCustomAvatar(c.Req.Context(), c.User.ID) + if err != nil { + return errors.Wrap(err, "delete custom avatar") } return nil } @@ -193,14 +202,14 @@ func SettingsPasswordPost(c *context.Context, f form.ChangePassword) { } else if f.Password != f.Retype { c.Flash.Error(c.Tr("form.password_not_match")) } else { - c.User.Password = f.Password - var err error - if c.User.Salt, err = userutil.RandomSalt(); err != nil { - c.Errorf(err, "get user salt") - return - } - c.User.Password = userutil.EncodePassword(c.User.Password, c.User.Salt) - if err := db.UpdateUser(c.User); err != nil { + err := db.Users.Update( + c.Req.Context(), + c.User.ID, + db.UpdateUserOptions{ + Password: &f.Password, + }, + ) + if err != nil { c.Errorf(err, "update user") return }