diff --git a/internal/db/email_addresses.go b/internal/db/email_addresses.go index 2929ffd04..8cef705e0 100644 --- a/internal/db/email_addresses.go +++ b/internal/db/email_addresses.go @@ -8,6 +8,7 @@ import ( "context" "fmt" + "github.com/pkg/errors" "gorm.io/gorm" "gogs.io/gogs/internal/errutil" @@ -17,9 +18,11 @@ import ( // // NOTE: All methods are sorted in alphabetical order. type EmailAddressesStore interface { - // GetByEmail returns the email address with given email. It may return - // unverified email addresses and returns ErrEmailNotExist when not found. - GetByEmail(ctx context.Context, email string) (*EmailAddress, error) + // GetByEmail returns the email address with given email. If `needsActivated` is + // true, only activated email will be returned, otherwise, it may return + // inactivated email addresses. It returns ErrEmailNotExist when no qualified + // email is not found. + GetByEmail(ctx context.Context, email string, needsActivated bool) (*EmailAddress, error) } var EmailAddresses EmailAddressesStore @@ -43,7 +46,7 @@ type ErrEmailNotExist struct { } func IsErrEmailAddressNotExist(err error) bool { - _, ok := err.(ErrEmailNotExist) + _, ok := errors.Cause(err).(ErrEmailNotExist) return ok } @@ -55,9 +58,14 @@ func (ErrEmailNotExist) NotFound() bool { return true } -func (db *emailAddresses) GetByEmail(ctx context.Context, email string) (*EmailAddress, error) { +func (db *emailAddresses) GetByEmail(ctx context.Context, email string, needsActivated bool) (*EmailAddress, error) { + tx := db.WithContext(ctx).Where("email = ?", email) + if needsActivated { + tx = tx.Where("is_activated = ?", true) + } + emailAddress := new(EmailAddress) - err := db.WithContext(ctx).Where("email = ?", email).First(emailAddress).Error + err := tx.First(emailAddress).Error if err != nil { if err == gorm.ErrRecordNotFound { return nil, ErrEmailNotExist{ diff --git a/internal/db/email_addresses_test.go b/internal/db/email_addresses_test.go index b54ffbadc..f55db5495 100644 --- a/internal/db/email_addresses_test.go +++ b/internal/db/email_addresses_test.go @@ -49,7 +49,7 @@ func emailAddressesGetByEmail(t *testing.T, db *emailAddresses) { ctx := context.Background() const testEmail = "alice@example.com" - _, err := db.GetByEmail(ctx, testEmail) + _, err := db.GetByEmail(ctx, testEmail, false) wantErr := ErrEmailNotExist{ args: errutil.Args{ "email": testEmail, @@ -58,9 +58,20 @@ func emailAddressesGetByEmail(t *testing.T, db *emailAddresses) { assert.Equal(t, wantErr, err) // TODO: Use EmailAddresses.Create to replace SQL hack when the method is available. - err = db.Exec(`INSERT INTO email_address (uid, email) VALUES (1, ?)`, testEmail).Error + err = db.Exec(`INSERT INTO email_address (uid, email, is_activated) VALUES (1, ?, FALSE)`, testEmail).Error require.NoError(t, err) - got, err := db.GetByEmail(ctx, testEmail) + got, err := db.GetByEmail(ctx, testEmail, false) + require.NoError(t, err) + assert.Equal(t, testEmail, got.Email) + + // Should not return if we only want activated emails + _, err = db.GetByEmail(ctx, testEmail, true) + assert.Equal(t, wantErr, err) + + // TODO: Use EmailAddresses.MarkActivated to replace SQL hack when the method is available. + err = db.Exec(`UPDATE email_address SET is_activated = TRUE WHERE email = ?`, testEmail).Error + require.NoError(t, err) + got, err = db.GetByEmail(ctx, testEmail, true) require.NoError(t, err) assert.Equal(t, testEmail, got.Email) } diff --git a/internal/db/org.go b/internal/db/org.go index b11123c60..cb68451a2 100644 --- a/internal/db/org.go +++ b/internal/db/org.go @@ -106,7 +106,7 @@ func CreateOrganization(org, owner *User) (err error) { return err } - if Users.IsUsernameUsed(context.TODO(), org.Name) { + if Users.IsUsernameUsed(context.TODO(), org.Name, 0) { return ErrUserAlreadyExist{ args: errutil.Args{ "name": org.Name, diff --git a/internal/db/user.go b/internal/db/user.go index 24af0ec18..cc2de95b5 100644 --- a/internal/db/user.go +++ b/internal/db/user.go @@ -21,6 +21,7 @@ import ( "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" ) @@ -41,6 +42,7 @@ 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() { @@ -59,9 +61,9 @@ func updateUser(e Engine, u *User) error { } u.LowerName = strings.ToLower(u.Name) - u.Location = tool.TruncateString(u.Location, 255) - u.Website = tool.TruncateString(u.Website, 255) - u.Description = tool.TruncateString(u.Description, 255) + 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 @@ -76,6 +78,8 @@ func (u *User) BeforeUpdate() { } // 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) } diff --git a/internal/db/users.go b/internal/db/users.go index 01688dab5..2b599597e 100644 --- a/internal/db/users.go +++ b/internal/db/users.go @@ -72,8 +72,10 @@ type UsersStore interface { GetByUsername(ctx context.Context, username string) (*User, error) // HasForkedRepository returns true if the user has forked given repository. HasForkedRepository(ctx context.Context, userID, repoID int64) bool - // IsUsernameUsed returns true if the given username has been used. - IsUsernameUsed(ctx context.Context, username string) bool + // IsUsernameUsed returns true if the given username has been used other than + // the excluded user (a non-positive ID effectively meaning check against all + // users). + IsUsernameUsed(ctx context.Context, username string, excludeUserId int64) bool // List returns a list of users. Results are paginated by given page and page // size, and sorted by primary key (id) in ascending order. List(ctx context.Context, page, pageSize int) ([]*User, error) @@ -85,6 +87,9 @@ 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(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 } @@ -201,7 +206,7 @@ func (db *users) ChangeUsername(ctx context.Context, userID int64, newUsername s return err } - if db.IsUsernameUsed(ctx, newUsername) { + if db.IsUsernameUsed(ctx, newUsername, userID) { return ErrUserAlreadyExist{ args: errutil.Args{ "name": newUsername, @@ -226,6 +231,11 @@ func (db *users) ChangeUsername(ctx context.Context, userID int64, newUsername s return errors.Wrap(err, "update user name") } + // Stop here if it's just a case-change of the username + if strings.EqualFold(user.Name, newUsername) { + return nil + } + // Update all references to the user name in pull requests err = tx.Model(&PullRequest{}). Where("head_user_name = ?", user.LowerName). @@ -328,7 +338,7 @@ func (db *users) Create(ctx context.Context, username, email string, opts Create return nil, err } - if db.IsUsernameUsed(ctx, username) { + if db.IsUsernameUsed(ctx, username, 0) { return nil, ErrUserAlreadyExist{ args: errutil.Args{ "name": username, @@ -428,18 +438,13 @@ func (db *users) GetByEmail(ctx context.Context, email string) (*User, error) { } // Otherwise, check activated email addresses - emailAddress := new(EmailAddress) - err = db.WithContext(ctx). - Where("email = ? AND is_activated = ?", email, true). - First(emailAddress). - Error + emailAddress, err := NewEmailAddressesStore(db.DB).GetByEmail(ctx, email, true) if err != nil { - if err == gorm.ErrRecordNotFound { + if IsErrEmailAddressNotExist(err) { return nil, ErrUserNotExist{args: errutil.Args{"email": email}} } return nil, err } - return db.GetByID(ctx, emailAddress.UserID) } @@ -473,13 +478,13 @@ func (db *users) HasForkedRepository(ctx context.Context, userID, repoID int64) return count > 0 } -func (db *users) IsUsernameUsed(ctx context.Context, username string) bool { +func (db *users) IsUsernameUsed(ctx context.Context, username string, excludeUserId int64) bool { if username == "" { return false } return db.WithContext(ctx). Select("id"). - Where("lower_name = ?", strings.ToLower(username)). + Where("lower_name = ? AND id != ?", strings.ToLower(username), excludeUserId). First(&User{}). Error != gorm.ErrRecordNotFound } @@ -534,6 +539,33 @@ func (db *users) ListFollowings(ctx context.Context, userID int64, page, pageSiz Error } +type UpdateUserOptions struct { + FullName string + Website string + Location string + Description string + + MaxRepoCreation int +} + +func (db *users) Update(ctx context.Context, userID int64, opts UpdateUserOptions) error { + if opts.MaxRepoCreation < -1 { + opts.MaxRepoCreation = -1 + } + 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 +} + func (db *users) UseCustomAvatar(ctx context.Context, userID int64, avatar []byte) error { err := userutil.SaveAvatar(userID, avatar) if err != nil { diff --git a/internal/db/users_test.go b/internal/db/users_test.go index 26d5ec7c5..2ce0efd9a 100644 --- a/internal/db/users_test.go +++ b/internal/db/users_test.go @@ -104,6 +104,7 @@ func TestUsers(t *testing.T) { {"List", usersList}, {"ListFollowers", usersListFollowers}, {"ListFollowings", usersListFollowings}, + {"Update", usersUpdate}, {"UseCustomAvatar", usersUseCustomAvatar}, } { t.Run(tc.name, func(t *testing.T) { @@ -241,10 +242,20 @@ func usersChangeUsername(t *testing.T, db *users) { }) t.Run("name already exists", func(t *testing.T) { - err := db.ChangeUsername(ctx, alice.ID, alice.Name) + bob, err := db.Create( + ctx, + "bob", + "bob@example.com", + CreateUserOptions{ + Activated: true, + }, + ) + require.NoError(t, err) + + err = db.ChangeUsername(ctx, alice.ID, bob.Name) wantErr := ErrUserAlreadyExist{ args: errutil.Args{ - "name": alice.Name, + "name": bob.Name, }, } assert.Equal(t, wantErr, err) @@ -303,7 +314,7 @@ func usersChangeUsername(t *testing.T, db *users) { assert.Equal(t, headUserName, alice.Name) var updatedUnix int64 - err = db.Model(&User{}).Select("updated_unix").Row().Scan(&updatedUnix) + err = db.Model(&User{}).Select("updated_unix").Where("id = ?", alice.ID).Row().Scan(&updatedUnix) require.NoError(t, err) assert.Equal(t, int64(0), updatedUnix) @@ -329,6 +340,13 @@ func usersChangeUsername(t *testing.T, db *users) { require.NoError(t, err) assert.Equal(t, newUsername, alice.Name) assert.Equal(t, db.NowFunc().Unix(), alice.UpdatedUnix) + + // Change the cases of the username should just be fine + err = db.ChangeUsername(ctx, alice.ID, strings.ToUpper(newUsername)) + require.NoError(t, err) + alice, err = db.GetByID(ctx, alice.ID) + require.NoError(t, err) + assert.Equal(t, strings.ToUpper(newUsername), alice.Name) } func usersCount(t *testing.T, db *users) { @@ -561,10 +579,50 @@ func usersIsUsernameUsed(t *testing.T, db *users) { alice, err := db.Create(ctx, "alice", "alice@example.com", CreateUserOptions{}) require.NoError(t, err) - got := db.IsUsernameUsed(ctx, alice.Name) - assert.True(t, got) - got = db.IsUsernameUsed(ctx, "bob") - assert.False(t, got) + tests := []struct { + name string + username string + excludeUserID int64 + want bool + }{ + { + name: "no change", + username: alice.Name, + excludeUserID: alice.ID, + want: false, + }, + { + name: "change case", + username: strings.ToUpper(alice.Name), + excludeUserID: alice.ID, + want: false, + }, + { + name: "not used", + username: "bob", + excludeUserID: alice.ID, + want: false, + }, + { + name: "not used when not excluded", + username: "bob", + excludeUserID: 0, + want: false, + }, + + { + name: "used when not excluded", + username: alice.Name, + excludeUserID: 0, + want: true, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := db.IsUsernameUsed(ctx, test.username, test.excludeUserID) + assert.Equal(t, test.want, got) + }) + } } func usersList(t *testing.T, db *users) { @@ -670,6 +728,50 @@ func usersListFollowings(t *testing.T, db *users) { assert.Equal(t, alice.ID, got[0].ID) } +func usersUpdate(t *testing.T, db *users) { + ctx := context.Background() + + alice, err := db.Create(ctx, "alice", "alice@example.com", CreateUserOptions{}) + require.NoError(t, err) + + overLimitStr := strings.Repeat("a", 300) + opts := UpdateUserOptions{ + FullName: overLimitStr, + Website: overLimitStr, + Location: overLimitStr, + Description: overLimitStr, + MaxRepoCreation: 1, + } + err = db.Update(ctx, alice.ID, opts) + require.NoError(t, err) + + 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", + } + err = db.Update(ctx, alice.ID, opts) + 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) +} + func usersUseCustomAvatar(t *testing.T, db *users) { ctx := context.Background() diff --git a/internal/form/user.go b/internal/form/user.go index d8d73ca45..539dfd98a 100644 --- a/internal/form/user.go +++ b/internal/form/user.go @@ -95,7 +95,6 @@ func (f *SignIn) Validate(ctx *macaron.Context, errs binding.Errors) binding.Err type UpdateProfile struct { Name string `binding:"Required;AlphaDashDot;MaxSize(35)"` FullName string `binding:"MaxSize(100)"` - Email string `binding:"Required;Email;MaxSize(254)"` Website string `binding:"Url;MaxSize(100)"` Location string `binding:"MaxSize(50)"` } diff --git a/internal/route/lfs/mocks_test.go b/internal/route/lfs/mocks_test.go index a1e89f85f..07dfbecb5 100644 --- a/internal/route/lfs/mocks_test.go +++ b/internal/route/lfs/mocks_test.go @@ -2331,6 +2331,9 @@ type MockUsersStore struct { // ListFollowingsFunc is an instance of a mock function object // controlling the behavior of the method ListFollowings. ListFollowingsFunc *UsersStoreListFollowingsFunc + // UpdateFunc is an instance of a mock function object controlling the + // behavior of the method Update. + UpdateFunc *UsersStoreUpdateFunc // UseCustomAvatarFunc is an instance of a mock function object // controlling the behavior of the method UseCustomAvatar. UseCustomAvatarFunc *UsersStoreUseCustomAvatarFunc @@ -2386,7 +2389,7 @@ func NewMockUsersStore() *MockUsersStore { }, }, IsUsernameUsedFunc: &UsersStoreIsUsernameUsedFunc{ - defaultHook: func(context.Context, string) (r0 bool) { + defaultHook: func(context.Context, string, int64) (r0 bool) { return }, }, @@ -2405,6 +2408,11 @@ func NewMockUsersStore() *MockUsersStore { return }, }, + UpdateFunc: &UsersStoreUpdateFunc{ + defaultHook: func(context.Context, int64, db.UpdateUserOptions) (r0 error) { + return + }, + }, UseCustomAvatarFunc: &UsersStoreUseCustomAvatarFunc{ defaultHook: func(context.Context, int64, []byte) (r0 error) { return @@ -2463,7 +2471,7 @@ func NewStrictMockUsersStore() *MockUsersStore { }, }, IsUsernameUsedFunc: &UsersStoreIsUsernameUsedFunc{ - defaultHook: func(context.Context, string) bool { + defaultHook: func(context.Context, string, int64) bool { panic("unexpected invocation of MockUsersStore.IsUsernameUsed") }, }, @@ -2482,6 +2490,11 @@ func NewStrictMockUsersStore() *MockUsersStore { panic("unexpected invocation of MockUsersStore.ListFollowings") }, }, + UpdateFunc: &UsersStoreUpdateFunc{ + defaultHook: func(context.Context, int64, db.UpdateUserOptions) error { + panic("unexpected invocation of MockUsersStore.Update") + }, + }, UseCustomAvatarFunc: &UsersStoreUseCustomAvatarFunc{ defaultHook: func(context.Context, int64, []byte) error { panic("unexpected invocation of MockUsersStore.UseCustomAvatar") @@ -2533,6 +2546,9 @@ func NewMockUsersStoreFrom(i db.UsersStore) *MockUsersStore { ListFollowingsFunc: &UsersStoreListFollowingsFunc{ defaultHook: i.ListFollowings, }, + UpdateFunc: &UsersStoreUpdateFunc{ + defaultHook: i.Update, + }, UseCustomAvatarFunc: &UsersStoreUseCustomAvatarFunc{ defaultHook: i.UseCustomAvatar, }, @@ -3518,24 +3534,24 @@ func (c UsersStoreHasForkedRepositoryFuncCall) Results() []interface{} { // UsersStoreIsUsernameUsedFunc describes the behavior when the // IsUsernameUsed method of the parent MockUsersStore instance is invoked. type UsersStoreIsUsernameUsedFunc struct { - defaultHook func(context.Context, string) bool - hooks []func(context.Context, string) bool + defaultHook func(context.Context, string, int64) bool + hooks []func(context.Context, string, int64) bool history []UsersStoreIsUsernameUsedFuncCall mutex sync.Mutex } // IsUsernameUsed delegates to the next hook function in the queue and // stores the parameter and result values of this invocation. -func (m *MockUsersStore) IsUsernameUsed(v0 context.Context, v1 string) bool { - r0 := m.IsUsernameUsedFunc.nextHook()(v0, v1) - m.IsUsernameUsedFunc.appendCall(UsersStoreIsUsernameUsedFuncCall{v0, v1, r0}) +func (m *MockUsersStore) IsUsernameUsed(v0 context.Context, v1 string, v2 int64) bool { + r0 := m.IsUsernameUsedFunc.nextHook()(v0, v1, v2) + m.IsUsernameUsedFunc.appendCall(UsersStoreIsUsernameUsedFuncCall{v0, v1, v2, r0}) return r0 } // SetDefaultHook sets function that is called when the IsUsernameUsed // method of the parent MockUsersStore instance is invoked and the hook // queue is empty. -func (f *UsersStoreIsUsernameUsedFunc) SetDefaultHook(hook func(context.Context, string) bool) { +func (f *UsersStoreIsUsernameUsedFunc) SetDefaultHook(hook func(context.Context, string, int64) bool) { f.defaultHook = hook } @@ -3543,7 +3559,7 @@ func (f *UsersStoreIsUsernameUsedFunc) SetDefaultHook(hook func(context.Context, // IsUsernameUsed method of the parent MockUsersStore instance invokes the // hook at the front of the queue and discards it. After the queue is empty, // the default hook function is invoked for any future action. -func (f *UsersStoreIsUsernameUsedFunc) PushHook(hook func(context.Context, string) bool) { +func (f *UsersStoreIsUsernameUsedFunc) PushHook(hook func(context.Context, string, int64) bool) { f.mutex.Lock() f.hooks = append(f.hooks, hook) f.mutex.Unlock() @@ -3552,19 +3568,19 @@ func (f *UsersStoreIsUsernameUsedFunc) PushHook(hook func(context.Context, strin // SetDefaultReturn calls SetDefaultHook with a function that returns the // given values. func (f *UsersStoreIsUsernameUsedFunc) SetDefaultReturn(r0 bool) { - f.SetDefaultHook(func(context.Context, string) bool { + f.SetDefaultHook(func(context.Context, string, int64) bool { return r0 }) } // PushReturn calls PushHook with a function that returns the given values. func (f *UsersStoreIsUsernameUsedFunc) PushReturn(r0 bool) { - f.PushHook(func(context.Context, string) bool { + f.PushHook(func(context.Context, string, int64) bool { return r0 }) } -func (f *UsersStoreIsUsernameUsedFunc) nextHook() func(context.Context, string) bool { +func (f *UsersStoreIsUsernameUsedFunc) nextHook() func(context.Context, string, int64) bool { f.mutex.Lock() defer f.mutex.Unlock() @@ -3603,6 +3619,9 @@ type UsersStoreIsUsernameUsedFuncCall struct { // Arg1 is the value of the 2nd argument passed to this method // invocation. Arg1 string + // Arg2 is the value of the 3rd argument passed to this method + // invocation. + Arg2 int64 // Result0 is the value of the 1st result returned from this method // invocation. Result0 bool @@ -3611,7 +3630,7 @@ type UsersStoreIsUsernameUsedFuncCall struct { // Args returns an interface slice containing the arguments of this // invocation. func (c UsersStoreIsUsernameUsedFuncCall) Args() []interface{} { - return []interface{}{c.Arg0, c.Arg1} + return []interface{}{c.Arg0, c.Arg1, c.Arg2} } // Results returns an interface slice containing the results of this @@ -3958,6 +3977,113 @@ func (c UsersStoreListFollowingsFuncCall) Results() []interface{} { return []interface{}{c.Result0, c.Result1} } +// UsersStoreUpdateFunc describes the behavior when the Update method of the +// parent MockUsersStore instance is invoked. +type UsersStoreUpdateFunc struct { + defaultHook func(context.Context, int64, db.UpdateUserOptions) error + hooks []func(context.Context, int64, db.UpdateUserOptions) error + history []UsersStoreUpdateFuncCall + mutex sync.Mutex +} + +// Update delegates to the next hook function in the queue and stores the +// parameter and result values of this invocation. +func (m *MockUsersStore) Update(v0 context.Context, v1 int64, v2 db.UpdateUserOptions) error { + r0 := m.UpdateFunc.nextHook()(v0, v1, v2) + m.UpdateFunc.appendCall(UsersStoreUpdateFuncCall{v0, v1, v2, r0}) + return r0 +} + +// SetDefaultHook sets function that is called when the Update method of the +// parent MockUsersStore instance is invoked and the hook queue is empty. +func (f *UsersStoreUpdateFunc) SetDefaultHook(hook func(context.Context, int64, db.UpdateUserOptions) error) { + f.defaultHook = hook +} + +// PushHook adds a function to the end of hook queue. Each invocation of the +// Update method of the parent MockUsersStore instance invokes the hook at +// the front of the queue and discards it. After the queue is empty, the +// default hook function is invoked for any future action. +func (f *UsersStoreUpdateFunc) PushHook(hook func(context.Context, int64, db.UpdateUserOptions) error) { + f.mutex.Lock() + f.hooks = append(f.hooks, hook) + f.mutex.Unlock() +} + +// SetDefaultReturn calls SetDefaultHook with a function that returns the +// given values. +func (f *UsersStoreUpdateFunc) SetDefaultReturn(r0 error) { + f.SetDefaultHook(func(context.Context, int64, db.UpdateUserOptions) error { + return r0 + }) +} + +// PushReturn calls PushHook with a function that returns the given values. +func (f *UsersStoreUpdateFunc) PushReturn(r0 error) { + f.PushHook(func(context.Context, int64, db.UpdateUserOptions) error { + return r0 + }) +} + +func (f *UsersStoreUpdateFunc) nextHook() func(context.Context, int64, db.UpdateUserOptions) error { + f.mutex.Lock() + defer f.mutex.Unlock() + + if len(f.hooks) == 0 { + return f.defaultHook + } + + hook := f.hooks[0] + f.hooks = f.hooks[1:] + return hook +} + +func (f *UsersStoreUpdateFunc) appendCall(r0 UsersStoreUpdateFuncCall) { + f.mutex.Lock() + f.history = append(f.history, r0) + f.mutex.Unlock() +} + +// History returns a sequence of UsersStoreUpdateFuncCall objects describing +// the invocations of this function. +func (f *UsersStoreUpdateFunc) History() []UsersStoreUpdateFuncCall { + f.mutex.Lock() + history := make([]UsersStoreUpdateFuncCall, len(f.history)) + copy(history, f.history) + f.mutex.Unlock() + + return history +} + +// UsersStoreUpdateFuncCall is an object that describes an invocation of +// method Update on an instance of MockUsersStore. +type UsersStoreUpdateFuncCall struct { + // Arg0 is the value of the 1st argument passed to this method + // invocation. + Arg0 context.Context + // Arg1 is the value of the 2nd argument passed to this method + // invocation. + Arg1 int64 + // Arg2 is the value of the 3rd argument passed to this method + // invocation. + Arg2 db.UpdateUserOptions + // Result0 is the value of the 1st result returned from this method + // invocation. + Result0 error +} + +// Args returns an interface slice containing the arguments of this +// invocation. +func (c UsersStoreUpdateFuncCall) Args() []interface{} { + return []interface{}{c.Arg0, c.Arg1, c.Arg2} +} + +// Results returns an interface slice containing the results of this +// invocation. +func (c UsersStoreUpdateFuncCall) Results() []interface{} { + return []interface{}{c.Result0} +} + // UsersStoreUseCustomAvatarFunc describes the behavior when the // UseCustomAvatar method of the parent MockUsersStore instance is invoked. type UsersStoreUseCustomAvatarFunc struct { diff --git a/internal/route/org/setting.go b/internal/route/org/setting.go index f3e65b9b9..bcef4ea1b 100644 --- a/internal/route/org/setting.go +++ b/internal/route/org/setting.go @@ -5,8 +5,6 @@ package org import ( - "strings" - "github.com/pkg/errors" log "unknwon.dev/clog/v2" @@ -40,43 +38,47 @@ func SettingsPost(c *context.Context, f form.UpdateOrgSetting) { org := c.Org.Organization - // Check if organization name has been changed. - if org.LowerName != strings.ToLower(f.Name) { - if db.Users.IsUsernameUsed(c.Req.Context(), f.Name) { - c.Data["OrgName"] = true - c.RenderWithErr(c.Tr("form.username_been_taken"), SETTINGS_OPTIONS, &f) - return - } else if err := db.Users.ChangeUsername(c.Req.Context(), org.ID, f.Name); err != nil { + // Check if the organization username (including cases) had been changed + if org.Name != f.Name { + err := db.Users.ChangeUsername(c.Req.Context(), c.Org.Organization.ID, f.Name) + if err != nil { c.Data["OrgName"] = true + var msg string switch { + case db.IsErrUserAlreadyExist(errors.Cause(err)): + msg = c.Tr("form.username_been_taken") case db.IsErrNameNotAllowed(errors.Cause(err)): - c.RenderWithErr(c.Tr("user.form.name_not_allowed", err.(db.ErrNameNotAllowed).Value()), SETTINGS_OPTIONS, &f) + msg = c.Tr("user.form.name_not_allowed", err.(db.ErrNameNotAllowed).Value()) default: c.Error(err, "change organization name") + return } + + c.RenderWithErr(msg, SETTINGS_OPTIONS, &f) return } + // reset c.org.OrgLink with new name c.Org.OrgLink = conf.Server.Subpath + "/org/" + f.Name log.Trace("Organization name changed: %s -> %s", org.Name, f.Name) } - // In case it's just a case change. - org.Name = f.Name - org.LowerName = strings.ToLower(f.Name) - if c.User.IsAdmin { - org.MaxRepoCreation = f.MaxRepoCreation + opts := db.UpdateUserOptions{ + FullName: f.FullName, + Website: f.Website, + Location: f.Location, + Description: f.Description, + MaxRepoCreation: org.MaxRepoCreation, } - - org.FullName = f.FullName - org.Description = f.Description - org.Website = f.Website - org.Location = f.Location - if err := db.UpdateUser(org); err != nil { - c.Error(err, "update user") + if c.User.IsAdmin { + opts.MaxRepoCreation = f.MaxRepoCreation + } + err := db.Users.Update(c.Req.Context(), c.User.ID, opts) + if err != nil { + c.Error(err, "update organization") return } - log.Trace("Organization setting updated: %s", org.Name) + c.Flash.Success(c.Tr("org.settings.update_setting_success")) c.Redirect(c.Org.OrgLink + "/settings") } diff --git a/internal/route/repo/setting.go b/internal/route/repo/setting.go index cd87769ac..e5e4fe6f7 100644 --- a/internal/route/repo/setting.go +++ b/internal/route/repo/setting.go @@ -225,7 +225,7 @@ func SettingsPost(c *context.Context, f form.RepoSetting) { } newOwner := c.Query("new_owner_name") - if !db.Users.IsUsernameUsed(c.Req.Context(), newOwner) { + if !db.Users.IsUsernameUsed(c.Req.Context(), newOwner, c.Repo.Owner.ID) { c.RenderWithErr(c.Tr("form.enterred_invalid_owner_name"), SETTINGS_OPTIONS, nil) return } diff --git a/internal/route/user/auth.go b/internal/route/user/auth.go index 86cf1f2e6..1977fe177 100644 --- a/internal/route/user/auth.go +++ b/internal/route/user/auth.go @@ -439,7 +439,7 @@ func verifyActiveEmailCode(code, email string) *db.EmailAddress { data := com.ToStr(user.ID) + email + user.LowerName + user.Password + user.Rands if tool.VerifyTimeLimitCode(data, minutes, prefix) { - emailAddress, err := db.EmailAddresses.GetByEmail(gocontext.TODO(), email) + emailAddress, err := db.EmailAddresses.GetByEmail(gocontext.TODO(), email, false) if err == nil { return emailAddress } diff --git a/internal/route/user/setting.go b/internal/route/user/setting.go index 2924a8f02..fbb7c9ae3 100644 --- a/internal/route/user/setting.go +++ b/internal/route/user/setting.go @@ -11,7 +11,6 @@ import ( "html/template" "image/png" "io" - "strings" "github.com/pkg/errors" "github.com/pquerna/otp" @@ -69,9 +68,10 @@ func SettingsPost(c *context.Context, f form.UpdateProfile) { // Non-local users are not allowed to change their username if c.User.IsLocal() { - // Check if username characters have been changed - if c.User.LowerName != strings.ToLower(f.Name) { - if err := db.Users.ChangeUsername(c.Req.Context(), c.User.ID, f.Name); err != nil { + // Check if the username (including cases) had been changed + if c.User.Name != f.Name { + err := db.Users.ChangeUsername(c.Req.Context(), c.User.ID, f.Name) + if err != nil { c.FormErr("Name") var msg string switch { @@ -90,23 +90,20 @@ func SettingsPost(c *context.Context, f form.UpdateProfile) { log.Trace("Username changed: %s -> %s", c.User.Name, f.Name) } - - // In case it's just a case change - c.User.Name = f.Name - c.User.LowerName = strings.ToLower(f.Name) } - c.User.FullName = f.FullName - c.User.Email = f.Email - c.User.Website = f.Website - c.User.Location = f.Location - if err := db.UpdateUser(c.User); err != nil { - if db.IsErrEmailAlreadyUsed(err) { - msg := c.Tr("form.email_been_used") - c.RenderWithErr(msg, SETTINGS_PROFILE, &f) - return - } - c.Errorf(err, "update user") + err := db.Users.Update( + c.Req.Context(), + c.User.ID, + db.UpdateUserOptions{ + FullName: f.FullName, + Website: f.Website, + Location: f.Location, + MaxRepoCreation: c.User.MaxRepoCreation, + }, + ) + if err != nil { + c.Error(err, "update user") return } diff --git a/internal/strutil/strutil.go b/internal/strutil/strutil.go index 30fa260a0..c10139591 100644 --- a/internal/strutil/strutil.go +++ b/internal/strutil/strutil.go @@ -54,3 +54,12 @@ func Ellipsis(str string, threshold int) string { } return str[:threshold] + "..." } + +// Truncate returns a truncated string if its length is over the limit. +// Otherwise, it returns the original string. +func Truncate(str string, limit int) string { + if len(str) < limit { + return str + } + return str[:limit] +} diff --git a/internal/strutil/strutil_test.go b/internal/strutil/strutil_test.go index 8a2fed758..d22e1f72e 100644 --- a/internal/strutil/strutil_test.go +++ b/internal/strutil/strutil_test.go @@ -95,3 +95,43 @@ func TestEllipsis(t *testing.T) { }) } } + +func TestTruncate(t *testing.T) { + tests := []struct { + name string + str string + limit int + want string + }{ + { + name: "empty string with zero limit", + str: "", + limit: 0, + want: "", + }, + { + name: "smaller length than limit", + str: "ab", + limit: 3, + want: "ab", + }, + { + name: "same length as limit", + str: "abc", + limit: 3, + want: "abc", + }, + { + name: "greater length than limit", + str: "ab", + limit: 1, + want: "a", + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := Truncate(test.str, test.limit) + assert.Equal(t, test.want, got) + }) + } +} diff --git a/internal/tool/tool.go b/internal/tool/tool.go index e4280b2a3..bc6dda11d 100644 --- a/internal/tool/tool.go +++ b/internal/tool/tool.go @@ -5,7 +5,6 @@ package tool import ( - "crypto/md5" "crypto/sha1" "encoding/base64" "encoding/hex" @@ -23,6 +22,7 @@ import ( "github.com/gogs/chardet" "gogs.io/gogs/internal/conf" + "gogs.io/gogs/internal/cryptoutil" ) // ShortSHA1 truncates SHA1 string length to at most 10. @@ -125,10 +125,7 @@ func CreateTimeLimitCode(data string, minutes int, startInf interface{}) string // HashEmail hashes email address to MD5 string. // https://en.gravatar.com/site/implement/hash/ func HashEmail(email string) string { - email = strings.ToLower(strings.TrimSpace(email)) - h := md5.New() - _, _ = h.Write([]byte(email)) - return hex.EncodeToString(h.Sum(nil)) + return cryptoutil.MD5(strings.ToLower(strings.TrimSpace(email))) } // AvatarLink returns relative avatar link to the site domain by given email, @@ -358,15 +355,6 @@ func Subtract(left, right interface{}) interface{} { } } -// TruncateString returns a truncated string with given limit, -// it returns input string if length is not reached limit. -func TruncateString(str string, limit int) string { - if len(str) < limit { - return str - } - return str[:limit] -} - // StringsToInt64s converts a slice of string to a slice of int64. func StringsToInt64s(strs []string) []int64 { ints := make([]int64, len(strs)) diff --git a/templates/user/settings/profile.tmpl b/templates/user/settings/profile.tmpl index 12f21c8f7..cc1eeb174 100644 --- a/templates/user/settings/profile.tmpl +++ b/templates/user/settings/profile.tmpl @@ -23,10 +23,6 @@ -
- - -