diff --git a/models/fixtures/login_source.yml b/models/fixtures/login_source.yml
index ca780a73aa0c1..fe51488c7066f 100644
--- a/models/fixtures/login_source.yml
+++ b/models/fixtures/login_source.yml
@@ -1 +1 @@
-[] # empty
+[]
diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml
index 976a236011cc9..43f3c6f83f4a6 100644
--- a/models/fixtures/user.yml
+++ b/models/fixtures/user.yml
@@ -1556,3 +1556,77 @@
repo_admin_change_team_access: false
theme: ""
keep_activity_private: false
+
+-
+ id: 43
+ lower_name: user43
+ name: user43
+ full_name: Non-Local User One
+ email: user43@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: user43
+ 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: user43@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: user44
+ name: user44
+ full_name: Non-Local User Two
+ email: user44@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: user44
+ 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: user44@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
diff --git a/models/user/user_test.go b/models/user/user_test.go
index 923f2cd40e956..0213cb402c310 100644
--- a/models/user/user_test.go
+++ b/models/user/user_test.go
@@ -155,13 +155,13 @@ func TestSearchUsers(t *testing.T) {
}
testUserSuccess(user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}},
- []int64{1, 2, 4, 5, 8, 9, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34, 37, 38, 39, 40})
+ []int64{1, 2, 4, 5, 8, 9, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34, 37, 38, 39, 40, 43, 44})
testUserSuccess(user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(false)},
[]int64{9})
testUserSuccess(user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(true)},
- []int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34, 37, 38, 39, 40})
+ []int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34, 37, 38, 39, 40, 43, 44})
testUserSuccess(user_model.SearchUserOptions{Keyword: "user1", OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(true)},
[]int64{1, 10, 11, 12, 13, 14, 15, 16, 18})
@@ -171,7 +171,7 @@ func TestSearchUsers(t *testing.T) {
[]int64{1, 10, 11, 12, 13, 14, 15, 16, 18})
testUserSuccess(user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsAdmin: optional.Some(true)},
- []int64{1})
+ []int64{1, 43})
testUserSuccess(user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsRestricted: optional.Some(true)},
[]int64{29})
diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go
index 6afa651448d26..abc4e52877ea0 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 08e37e8df4286..0c108a933c12a 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 1f22d800a9069..a3389361513a5 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 fe585b3a00bd2..0e4dab8fb685b 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 27b0c83a38373..45a6c64f7bff0 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/update_test.go b/services/user/update_test.go
index b81ac95783d88..983c67340b56f 100644
--- a/services/user/update_test.go
+++ b/services/user/update_test.go
@@ -19,6 +19,12 @@ func TestUpdateUser(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
admin := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
+ nonLocalAdmin := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 43})
+
+ // Need to make the nonlocal admin not an admin for the following checks to make sense
+ assert.NoError(t, UpdateUser(t.Context(), nonLocalAdmin, &UpdateOptions{
+ IsAdmin: UpdateOptionFieldFromValue(false),
+ }))
assert.Error(t, UpdateUser(t.Context(), admin, &UpdateOptions{
IsAdmin: UpdateOptionFieldFromValue(false),
diff --git a/services/user/user.go b/services/user/user.go
index d8abf199c3ffa..49cea61a0497c 100644
--- a/services/user/user.go
+++ b/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,
diff --git a/services/user/user_test.go b/services/user/user_test.go
index 48852b4cb9a39..d5f69ad8301ec 100644
--- a/services/user/user_test.go
+++ b/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)
diff --git a/templates/admin/user/edit.tmpl b/templates/admin/user/edit.tmpl
index 879b5cb550d30..5a11d5bb72f20 100644
--- a/templates/admin/user/edit.tmpl
+++ b/templates/admin/user/edit.tmpl
@@ -9,7 +9,7 @@
{{.CsrfTokenHtml}}
-
+
diff --git a/tests/integration/api_nodeinfo_test.go b/tests/integration/api_nodeinfo_test.go
index 916c2f1723102..2bbe051094335 100644
--- a/tests/integration/api_nodeinfo_test.go
+++ b/tests/integration/api_nodeinfo_test.go
@@ -29,7 +29,7 @@ func TestNodeinfo(t *testing.T) {
DecodeJSON(t, resp, &nodeinfo)
assert.True(t, nodeinfo.OpenRegistrations)
assert.Equal(t, "gitea", nodeinfo.Software.Name)
- assert.Equal(t, 29, nodeinfo.Usage.Users.Total)
+ assert.Equal(t, 31, nodeinfo.Usage.Users.Total)
assert.Equal(t, 22, nodeinfo.Usage.LocalPosts)
assert.Equal(t, 3, nodeinfo.Usage.LocalComments)
}