diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go index 6afa651448..abc4e52877 100644 --- a/routers/api/v1/admin/user.go +++ b/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 { diff --git a/routers/api/v1/org/org.go b/routers/api/v1/org/org.go index 08e37e8df4..0c108a933c 100644 --- a/routers/api/v1/org/org.go +++ b/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 { diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index 1f22d800a9..a338936151 100644 --- a/routers/web/admin/users.go +++ b/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 diff --git a/routers/web/org/setting.go b/routers/web/org/setting.go index fe585b3a00..0e4dab8fb6 100644 --- a/routers/web/org/setting.go +++ b/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) { diff --git a/routers/web/user/setting/profile.go b/routers/web/user/setting/profile.go index 27b0c83a38..45a6c64f7b 100644 --- a/routers/web/user/setting/profile.go +++ b/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")) diff --git a/services/user/user.go b/services/user/user.go index d8abf199c3..8e42fa3ccd 100644 --- a/services/user/user.go +++ b/services/user/user.go @@ -31,17 +31,15 @@ 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() { - return user_model.ErrUserIsNotLocal{ - UID: u.ID, - Name: u.Name, - } + // Non-local users are not allowed to change their own username, but admins are + isExternalUser := !u.IsOrganization() && !u.IsLocal() + if isExternalUser && !doer.IsAdmin { + return user_model.ErrUserIsNotLocal{UID: u.ID, Name: u.Name} } if err := user_model.IsUsableUsername(newUserName); err != nil { diff --git a/services/user/user_test.go b/services/user/user_test.go index 48852b4cb9..25e8ee7b2f 100644 --- a/services/user/user_test.go +++ b/services/user/user_test.go @@ -20,6 +20,7 @@ import ( org_service "code.gitea.io/gitea/services/org" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestMain(m *testing.M) { @@ -101,23 +102,31 @@ 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, + t.Run("External user", func(t *testing.T) { + adminUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1, IsAdmin: true}) + externalUser := &user_model.User{ + Name: "external_user", + Email: "external_user@gitea.io", + LoginType: auth.LDAP, } - assert.ErrorIs(t, RenameUser(t.Context(), u, "user_rename"), user_model.ErrUserIsNotLocal{}) + require.NoError(t, user_model.CreateUser(t.Context(), externalUser, &user_model.Meta{})) + + err := RenameUser(t.Context(), externalUser, externalUser.Name+"_changed", externalUser) + assert.True(t, user_model.IsErrUserIsNotLocal(err), "external user is not allowed to rename themselves") + + err = RenameUser(t.Context(), externalUser, externalUser.Name+"_changed", adminUser) + assert.NoError(t, err, "admin can rename external user") }) 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 +135,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 +144,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) diff --git a/templates/admin/user/edit.tmpl b/templates/admin/user/edit.tmpl index 879b5cb550..5a11d5bb72 100644 --- a/templates/admin/user/edit.tmpl +++ b/templates/admin/user/edit.tmpl @@ -9,7 +9,7 @@ {{.CsrfTokenHtml}}
- +