diff --git a/integrations/admin_user_test.go b/integrations/admin_user_test.go new file mode 100644 index 000000000000..f6b4590d2724 --- /dev/null +++ b/integrations/admin_user_test.go @@ -0,0 +1,82 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package integrations + +import ( + "net/http" + "strconv" + "testing" + + "code.gitea.io/gitea/models" + + "github.com/stretchr/testify/assert" +) + +func TestAdminViewUsers(t *testing.T) { + prepareTestEnv(t) + + session := loginUser(t, "user1") + req := NewRequest(t, "GET", "/admin/users") + session.MakeRequest(t, req, http.StatusOK) + + session = loginUser(t, "user2") + req = NewRequest(t, "GET", "/admin/users") + session.MakeRequest(t, req, http.StatusForbidden) +} + +func TestAdminViewUser(t *testing.T) { + prepareTestEnv(t) + + session := loginUser(t, "user1") + req := NewRequest(t, "GET", "/admin/users/1") + session.MakeRequest(t, req, http.StatusOK) + + session = loginUser(t, "user2") + req = NewRequest(t, "GET", "/admin/users/1") + session.MakeRequest(t, req, http.StatusForbidden) +} + +func TestAdminEditUser(t *testing.T) { + prepareTestEnv(t) + + testSuccessfullEdit(t, models.User{ID: 2, Name: "newusername", LoginName: "otherlogin", Email: "new@e-mail.gitea"}) +} + +func testSuccessfullEdit(t *testing.T, formData models.User) { + makeRequest(t, formData, http.StatusFound) +} + +func makeRequest(t *testing.T, formData models.User, headerCode int) { + session := loginUser(t, "user1") + csrf := GetCSRF(t, session, "/admin/users/"+strconv.Itoa(int(formData.ID))) + req := NewRequestWithValues(t, "POST", "/admin/users/"+strconv.Itoa(int(formData.ID)), map[string]string{ + "_csrf": csrf, + "user_name": formData.Name, + "login_name": formData.LoginName, + "login_type": "0-0", + "email": formData.Email, + }) + + session.MakeRequest(t, req, headerCode) + user := models.AssertExistsAndLoadBean(t, &models.User{ID: formData.ID}).(*models.User) + assert.Equal(t, formData.Name, user.Name) + assert.Equal(t, formData.LoginName, user.LoginName) + assert.Equal(t, formData.Email, user.Email) +} + +func TestAdminDeleteUser(t *testing.T) { + defer prepareTestEnv(t)() + + session := loginUser(t, "user1") + + csrf := GetCSRF(t, session, "/admin/users/8") + req := NewRequestWithValues(t, "POST", "/admin/users/8/delete", map[string]string{ + "_csrf": csrf, + }) + session.MakeRequest(t, req, http.StatusOK) + + assertUserDeleted(t, 8) + models.CheckConsistencyFor(t, &models.User{}) +} diff --git a/integrations/delete_user_test.go b/integrations/delete_user_test.go index 32032b3ddf45..86be6f22ad78 100644 --- a/integrations/delete_user_test.go +++ b/integrations/delete_user_test.go @@ -24,21 +24,6 @@ func assertUserDeleted(t *testing.T, userID int64) { models.AssertNotExistsBean(t, &models.Star{UID: userID}) } -func TestAdminDeleteUser(t *testing.T) { - defer prepareTestEnv(t)() - - session := loginUser(t, "user1") - - csrf := GetCSRF(t, session, "/admin/users/8") - req := NewRequestWithValues(t, "POST", "/admin/users/8/delete", map[string]string{ - "_csrf": csrf, - }) - session.MakeRequest(t, req, http.StatusOK) - - assertUserDeleted(t, 8) - models.CheckConsistencyFor(t, &models.User{}) -} - func TestUserDeleteAccount(t *testing.T) { defer prepareTestEnv(t)() diff --git a/models/user.go b/models/user.go index 73178f256c1b..d3f1b16c2e7f 100644 --- a/models/user.go +++ b/models/user.go @@ -913,19 +913,19 @@ func ChangeUserName(u *User, newUserName string) (err error) { return err } - isExist, err := IsUserExist(0, newUserName) - if err != nil { - return err - } else if isExist { - return ErrUserAlreadyExist{newUserName} - } - sess := x.NewSession() defer sess.Close() if err = sess.Begin(); err != nil { return err } + isExist, err := isUserExist(sess, 0, newUserName) + if err != nil { + return err + } else if isExist { + return ErrUserAlreadyExist{newUserName} + } + if _, err = sess.Exec("UPDATE `repository` SET owner_name=? WHERE owner_name=?", newUserName, u.Name); err != nil { return fmt.Errorf("Change repo owner name: %v", err) } diff --git a/modules/auth/admin.go b/modules/auth/admin.go index 1f840251c7bc..f2d02635517a 100644 --- a/modules/auth/admin.go +++ b/modules/auth/admin.go @@ -28,6 +28,7 @@ func (f *AdminCreateUserForm) Validate(ctx *macaron.Context, errs binding.Errors // AdminEditUserForm form for admin to create user type AdminEditUserForm struct { LoginType string `binding:"Required"` + UserName string `binding:"AlphaDashDot;MaxSize(40)"` LoginName string FullName string `binding:"MaxSize(100)"` Email string `binding:"Required;Email;MaxSize(254)"` diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index de4dcde3f0bb..5f21c75f76d2 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -359,6 +359,7 @@ password_not_match = The passwords do not match. lang_select_error = Select a language from the list. username_been_taken = The username is already taken. +username_change_not_local_user = Non-local users are not allowed to change their username. repo_name_been_taken = The repository name is already used. repository_files_already_exist = Files already exist for this repository. Contact the system administrator. repository_files_already_exist.adopt = Files already exist for this repository and can only be Adopted. diff --git a/routers/admin/users.go b/routers/admin/users.go index 1dc6d5bbe204..8a848b1f9ddc 100644 --- a/routers/admin/users.go +++ b/routers/admin/users.go @@ -18,6 +18,7 @@ import ( "code.gitea.io/gitea/modules/password" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/routers" + router_user_setting "code.gitea.io/gitea/routers/user/setting" "code.gitea.io/gitea/services/mailer" ) @@ -269,6 +270,15 @@ func EditUserPost(ctx *context.Context, form auth.AdminEditUserForm) { u.HashPassword(form.Password) } + if len(form.UserName) != 0 && u.Name != form.UserName { + if err := router_user_setting.HandleUsernameChange(ctx, u, form.UserName); err != nil { + ctx.Redirect(setting.AppSubURL + "/admin/users") + return + } + u.Name = form.UserName + u.LowerName = strings.ToLower(form.UserName) + } + if form.Reset2FA { tf, err := models.GetTwoFactorByUID(u.ID) if err != nil && !models.IsErrTwoFactorNotEnrolled(err) { diff --git a/routers/user/setting/profile.go b/routers/user/setting/profile.go index fe9ce098fef7..c935b56230d8 100644 --- a/routers/user/setting/profile.go +++ b/routers/user/setting/profile.go @@ -38,42 +38,36 @@ func Profile(ctx *context.Context) { ctx.HTML(200, tplSettingsProfile) } -func handleUsernameChange(ctx *context.Context, newName string) { +// HandleUsernameChange handle username changes from user settings and admin interface +func HandleUsernameChange(ctx *context.Context, user *models.User, newName string) error { // Non-local users are not allowed to change their username. - if len(newName) == 0 || !ctx.User.IsLocal() { - return + if !user.IsLocal() { + ctx.Flash.Error(ctx.Tr("form.username_change_not_local_user")) + return fmt.Errorf(ctx.Tr("form.username_change_not_local_user")) } // Check if user name has been changed - if ctx.User.LowerName != strings.ToLower(newName) { - if err := models.ChangeUserName(ctx.User, newName); err != nil { + if user.LowerName != strings.ToLower(newName) { + if err := models.ChangeUserName(user, newName); err != nil { switch { case models.IsErrUserAlreadyExist(err): ctx.Flash.Error(ctx.Tr("form.username_been_taken")) - ctx.Redirect(setting.AppSubURL + "/user/settings") case models.IsErrEmailAlreadyUsed(err): ctx.Flash.Error(ctx.Tr("form.email_been_used")) - ctx.Redirect(setting.AppSubURL + "/user/settings") case models.IsErrNameReserved(err): ctx.Flash.Error(ctx.Tr("user.form.name_reserved", newName)) - ctx.Redirect(setting.AppSubURL + "/user/settings") case models.IsErrNamePatternNotAllowed(err): ctx.Flash.Error(ctx.Tr("user.form.name_pattern_not_allowed", newName)) - ctx.Redirect(setting.AppSubURL + "/user/settings") case models.IsErrNameCharsNotAllowed(err): ctx.Flash.Error(ctx.Tr("user.form.name_chars_not_allowed", newName)) - ctx.Redirect(setting.AppSubURL + "/user/settings") default: ctx.ServerError("ChangeUserName", err) } - return + return err } - log.Trace("User name changed: %s -> %s", ctx.User.Name, newName) + log.Trace("User name changed: %s -> %s", user.Name, newName) } - - // In case it's just a case change - ctx.User.Name = newName - ctx.User.LowerName = strings.ToLower(newName) + return nil } // ProfilePost response for change user's profile @@ -86,9 +80,13 @@ func ProfilePost(ctx *context.Context, form auth.UpdateProfileForm) { return } - handleUsernameChange(ctx, form.Name) - if ctx.Written() { - return + if len(form.Name) != 0 && ctx.User.Name != form.Name { + if err := HandleUsernameChange(ctx, ctx.User, form.Name); err != nil { + ctx.Redirect(setting.AppSubURL + "/user/settings") + return + } + ctx.User.Name = form.Name + ctx.User.LowerName = strings.ToLower(form.Name) } ctx.User.FullName = form.FullName diff --git a/templates/admin/user/edit.tmpl b/templates/admin/user/edit.tmpl index 9edf337f04c0..a07f3a105417 100644 --- a/templates/admin/user/edit.tmpl +++ b/templates/admin/user/edit.tmpl @@ -9,9 +9,9 @@
{{.CsrfTokenHtml}} -
+
- {{.User.Name}} +
diff --git a/web_src/js/index.js b/web_src/js/index.js index 37c31538afb4..07d5b99e3116 100644 --- a/web_src/js/index.js +++ b/web_src/js/index.js @@ -1796,6 +1796,7 @@ function initAdmin() { if ($('.admin.new.user').length > 0 || $('.admin.edit.user').length > 0) { $('#login_type').on('change', function () { if ($(this).val().substring(0, 1) === '0') { + $('#user_name').removeAttr('disabled'); $('#login_name').removeAttr('required'); $('.non-local').hide(); $('.local').show(); @@ -1805,6 +1806,7 @@ function initAdmin() { $('#password').attr('required', 'required'); } } else { + $('#user_name').attr('disabled', 'disabled'); $('#login_name').attr('required', 'required'); $('.non-local').show(); $('.local').hide();