Allow admins to rename non-local users

models/fixtures/login_source.yml

@@ -1 +1,5 @@
-[] # empty
+-
+  id: 1
+  type: 6
+  is_active: true
+  cfg: "{\"Source\":{\"A\":\"string\",\"B\":1}}"

models/fixtures/user.yml

@@ -1556,3 +1556,79 @@
   repo_admin_change_team_access: false
   theme: ""
   keep_activity_private: false
+
+-
+  id: 43
+  lower_name: nonlocaluser1
+  name: nonlocaluser1
+  full_name: User One
+  email: nonlocaluser1@example.com
+  keep_email_private: false
+  email_notifications_preference: enabled
+  passwd: ZogKvWdyEx:password
+  passwd_hash_algo: dummy
+  must_change_password: false
+  login_source: 1
+  login_name: nonlocaluser1
+  type: 0
+  salt: ZogKvWdyEx
+  max_repo_creation: -1
+  is_active: true
+  is_admin: true
+  is_restricted: false
+  allow_git_hook: false
+  allow_import_local: false
+  allow_create_organization: true
+  prohibit_login: false
+  avatar: ""
+  avatar_email: nonlocaluser1@example.com
+  use_custom_avatar: true
+  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
+
+-
+  id: 44
+  lower_name: nonlocaluser2
+  name: nonlocaluser2
+  full_name: User Two
+  email: nonlocaluser2@example.com
+  keep_email_private: false
+  email_notifications_preference: enabled
+  passwd: ZogKvWdyEx:password
+  passwd_hash_algo: dummy
+  must_change_password: false
+  login_source: 1
+  login_name: nonlocaluser2
+  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: false
+  avatar: ""
+  avatar_email: nonlocaluser2@example.com
+  use_custom_avatar: true
+  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
+
+

routers/api/v1/admin/user.go

@@ -480,7 +480,7 @@ func RenameUser(ctx *context.APIContext) {
 	newName := web.GetForm(ctx).(*api.RenameUserOption).NewName
 
 	// Check if username has been changed
-	if err := user_service.RenameUser(ctx, ctx.ContextUser, newName); err != nil {
+	if err := user_service.RenameUser(ctx, ctx.ContextUser, newName, ctx.Doer); err != nil {
 		if user_model.IsErrUserAlreadyExist(err) || db.IsErrNameReserved(err) || db.IsErrNamePatternNotAllowed(err) || db.IsErrNameCharsNotAllowed(err) {
 			ctx.APIError(http.StatusUnprocessableEntity, err)
 		} else {

routers/api/v1/org/org.go

@@ -340,7 +340,7 @@ func Rename(ctx *context.APIContext) {
 
 	form := web.GetForm(ctx).(*api.RenameOrgOption)
 	orgUser := ctx.Org.Organization.AsUser()
-	if err := user_service.RenameUser(ctx, orgUser, form.NewName); err != nil {
+	if err := user_service.RenameUser(ctx, orgUser, form.NewName, ctx.Doer); err != nil {
 		if user_model.IsErrUserAlreadyExist(err) || db.IsErrNameReserved(err) || db.IsErrNamePatternNotAllowed(err) || db.IsErrNameCharsNotAllowed(err) {
 			ctx.APIError(http.StatusUnprocessableEntity, err)
 		} else {

routers/web/admin/users.go

@@ -345,7 +345,7 @@ func EditUserPost(ctx *context.Context) {
 	}
 
 	if form.UserName != "" {
-		if err := user_service.RenameUser(ctx, u, form.UserName); err != nil {
+		if err := user_service.RenameUser(ctx, u, form.UserName, ctx.Doer); err != nil {
 			switch {
 			case user_model.IsErrUserIsNotLocal(err):
 				ctx.Data["Err_UserName"] = true

routers/web/org/setting.go

@@ -213,7 +213,7 @@ func SettingsRenamePost(ctx *context.Context) {
 		return
 	}
 
-	if err := user_service.RenameUser(ctx, ctx.Org.Organization.AsUser(), newOrgName); err != nil {
+	if err := user_service.RenameUser(ctx, ctx.Org.Organization.AsUser(), newOrgName, ctx.Doer); err != nil {
 		if user_model.IsErrUserAlreadyExist(err) {
 			ctx.JSONError(ctx.Tr("org.form.name_been_taken", newOrgName))
 		} else if db.IsErrNameReserved(err) {

routers/web/user/setting/profile.go

@@ -75,7 +75,7 @@ func ProfilePost(ctx *context.Context) {
 			ctx.Redirect(setting.AppSubURL + "/user/settings")
 			return
 		}
-		if err := user_service.RenameUser(ctx, ctx.Doer, form.Name); err != nil {
+		if err := user_service.RenameUser(ctx, ctx.Doer, form.Name, ctx.Doer); err != nil {
 			switch {
 			case user_model.IsErrUserIsNotLocal(err):
 				ctx.Flash.Error(ctx.Tr("form.username_change_not_local_user"))

services/user/user.go

@@ -31,13 +31,13 @@ import (
 )
 
 // RenameUser renames a user
-func RenameUser(ctx context.Context, u *user_model.User, newUserName string) error {
+func RenameUser(ctx context.Context, u *user_model.User, newUserName string, doer *user_model.User) error {
 	if newUserName == u.Name {
 		return nil
 	}
 
-	// Non-local users are not allowed to change their username.
-	if !u.IsOrganization() && !u.IsLocal() {
+	// Non-local users are not allowed to change their own username, but admins are
+	if !u.IsOrganization() && !u.IsLocal() && !doer.IsAdmin {
 		return user_model.ErrUserIsNotLocal{
 			UID:  u.ID,
 			Name: u.Name,

services/user/user_test.go

@@ -100,24 +100,34 @@ func TestCreateUser(t *testing.T) {
 func TestRenameUser(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 21})
-
-	t.Run("Non-Local", func(t *testing.T) {
-		u := &user_model.User{
-			Type:      user_model.UserTypeIndividual,
-			LoginType: auth.OAuth2,
-		}
-		assert.ErrorIs(t, RenameUser(t.Context(), u, "user_rename"), user_model.ErrUserIsNotLocal{})
+	adminUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1, IsAdmin: true})
+	nonLocalAdminUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 43, IsAdmin: true}) //, LoginType: auth.OAuth2}) ???
+	nonLocalAdminUser.LoginType = auth.OAuth2
+	nonLocalUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 44, IsAdmin: false}) // , LoginType: auth.OAuth2}) ???
+	nonLocalUser.LoginType = auth.OAuth2
+
+	t.Run("Non-Local, Self, Non-admin", func(t *testing.T) {
+		assert.ErrorIs(t, RenameUser(t.Context(), nonLocalUser, "nonlocal_user_rename", nonLocalUser), user_model.ErrUserIsNotLocal{UID: nonLocalUser.ID, Name: nonLocalUser.Name})
+	})
+	t.Run("Non-Local, Self, Admin", func(t *testing.T) {
+		assert.NoError(t, RenameUser(t.Context(), nonLocalAdminUser, "nonlocal_user_user_rename", nonLocalAdminUser))
+	})
+	t.Run("Non-Local, Other, Non-admin", func(t *testing.T) {
+		assert.ErrorIs(t, RenameUser(t.Context(), nonLocalUser, "user_rename", user), user_model.ErrUserIsNotLocal{UID: nonLocalUser.ID, Name: nonLocalUser.Name})
+	})
+	t.Run("Non-Local, Other, Admin", func(t *testing.T) {
+		assert.NoError(t, RenameUser(t.Context(), nonLocalAdminUser, "nonlocal_user_rename_2", adminUser))
 	})
 
 	t.Run("Same username", func(t *testing.T) {
-		assert.NoError(t, RenameUser(t.Context(), user, user.Name))
+		assert.NoError(t, RenameUser(t.Context(), user, user.Name, user))
 	})
 
 	t.Run("Non usable username", func(t *testing.T) {
 		usernames := []string{"--diff", ".well-known", "gitea-actions", "aaa.atom", "aa.png"}
 		for _, username := range usernames {
 			assert.Error(t, user_model.IsUsableUsername(username), "non-usable username: %s", username)
-			assert.Error(t, RenameUser(t.Context(), user, username), "non-usable username: %s", username)
+			assert.Error(t, RenameUser(t.Context(), user, username, user), "non-usable username: %s", username)
 		}
 	})
 
@@ -126,7 +136,7 @@ func TestRenameUser(t *testing.T) {
 		unittest.AssertNotExistsBean(t, &user_model.User{ID: user.ID, Name: caps})
 		unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: user.ID, OwnerName: user.Name})
 
-		assert.NoError(t, RenameUser(t.Context(), user, caps))
+		assert.NoError(t, RenameUser(t.Context(), user, caps, user))
 
 		unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: user.ID, Name: caps})
 		unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: user.ID, OwnerName: caps})
@@ -135,17 +145,17 @@ func TestRenameUser(t *testing.T) {
 	t.Run("Already exists", func(t *testing.T) {
 		existUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
 
-		assert.ErrorIs(t, RenameUser(t.Context(), user, existUser.Name), user_model.ErrUserAlreadyExist{Name: existUser.Name})
-		assert.ErrorIs(t, RenameUser(t.Context(), user, existUser.LowerName), user_model.ErrUserAlreadyExist{Name: existUser.LowerName})
+		assert.ErrorIs(t, RenameUser(t.Context(), user, existUser.Name, user), user_model.ErrUserAlreadyExist{Name: existUser.Name})
+		assert.ErrorIs(t, RenameUser(t.Context(), user, existUser.LowerName, user), user_model.ErrUserAlreadyExist{Name: existUser.LowerName})
 		newUsername := fmt.Sprintf("uSEr%d", existUser.ID)
-		assert.ErrorIs(t, RenameUser(t.Context(), user, newUsername), user_model.ErrUserAlreadyExist{Name: newUsername})
+		assert.ErrorIs(t, RenameUser(t.Context(), user, newUsername, user), user_model.ErrUserAlreadyExist{Name: newUsername})
 	})
 
 	t.Run("Normal", func(t *testing.T) {
 		oldUsername := user.Name
 		newUsername := "User_Rename"
 
-		assert.NoError(t, RenameUser(t.Context(), user, newUsername))
+		assert.NoError(t, RenameUser(t.Context(), user, newUsername, user))
 		unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: user.ID, Name: newUsername, LowerName: strings.ToLower(newUsername)})
 
 		redirectUID, err := user_model.LookupUserRedirect(t.Context(), oldUsername)

templates/admin/user/edit.tmpl

@@ -9,7 +9,7 @@
 				{{.CsrfTokenHtml}}
 				<div class="field {{if .Err_UserName}}error{{end}}">
 					<label for="user_name">{{ctx.Locale.Tr "username"}}</label>
-					<input id="user_name" name="user_name" value="{{.User.Name}}" {{if not .User.IsLocal}}disabled{{end}} maxlength="40">
+					<input id="user_name" name="user_name" value="{{.User.Name}}" maxlength="40">
 				</div>
 				<!-- Types and name -->
 				<div class="inline required field {{if .Err_LoginType}}error{{end}}">