From 7ed496ea371821a05fad535a7a65bef0cfb055fd Mon Sep 17 00:00:00 2001 From: Gusted Date: Fri, 6 Feb 2026 17:47:30 +0100 Subject: [PATCH] fix: reflect allowed username change in profile setting (#11171) - When working forgejo/forgejo!8714 I did not touch the UI to remove the note and `disabled` attribute. This was not intentional, and was likely caused by me straight going for testing (as the backend code would allow the username change). - Slightly refactor the context to a common function, don't hard error if `CanUserRename` fails but does default to that you cannot rename in that case (which is the standard behavior of OAuth2 users anyway). I already was aware that it seems !8714 wasn't working on Codeberg but someone at FOSDEM pointed it out again, thus the reason for this bug fix. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11171 Reviewed-by: Mathieu Fenniak Reviewed-by: Beowulf Co-authored-by: Gusted Co-committed-by: Gusted --- routers/web/user/setting/profile.go | 26 +++++---- services/user/user.go | 44 +++++++++------ templates/user/settings/profile.tmpl | 4 +- .../fixtures/TestUserRename/login_source.yml | 10 ++++ tests/integration/user_settings_test.go | 55 +++++++++++++++++++ 5 files changed, 110 insertions(+), 29 deletions(-) create mode 100644 tests/integration/fixtures/TestUserRename/login_source.yml diff --git a/routers/web/user/setting/profile.go b/routers/web/user/setting/profile.go index 125e387866..466fbff435 100644 --- a/routers/web/user/setting/profile.go +++ b/routers/web/user/setting/profile.go @@ -43,8 +43,8 @@ const ( var commonPronouns = []string{"he/him", "she/her", "they/them", "it/its", "any pronouns"} -// Profile render user's profile page -func Profile(ctx *context.Context) { +// profileContext sets common context for profile settings template. +func profileContext(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("settings.profile") ctx.Data["PageIsSettingsProfile"] = true ctx.Data["AllowedUserVisibilityModes"] = setting.Service.AllowedUserVisibilityModesSlice.ToVisibleTypeSlice() @@ -55,20 +55,24 @@ func Profile(ctx *context.Context) { ctx.Data["MaxAvatarWidth"] = setting.Avatar.MaxWidth ctx.Data["MaxAvatarHeight"] = setting.Avatar.MaxHeight + canUserRename, err := user_service.CanUserRename(ctx, ctx.Doer) + if err != nil { + log.Error("CanUserRename for user[%d]: %v", ctx.Doer.ID, err) + } + + ctx.Data["UserRenameDisabled"] = !canUserRename +} + +// Profile render user's profile page +func Profile(ctx *context.Context) { + profileContext(ctx) + ctx.HTML(http.StatusOK, tplSettingsProfile) } // ProfilePost response for change user's profile func ProfilePost(ctx *context.Context) { - ctx.Data["Title"] = ctx.Tr("settings") - ctx.Data["PageIsSettingsProfile"] = true - ctx.Data["AllowedUserVisibilityModes"] = setting.Service.AllowedUserVisibilityModesSlice.ToVisibleTypeSlice() - ctx.Data["DisableGravatar"] = setting.Config().Picture.DisableGravatar.Value(ctx) - ctx.Data["CooldownPeriod"] = setting.Service.UsernameCooldownPeriod - ctx.Data["CommonPronouns"] = commonPronouns - ctx.Data["MaxAvatarFileSize"] = setting.Avatar.MaxFileSize - ctx.Data["MaxAvatarWidth"] = setting.Avatar.MaxWidth - ctx.Data["MaxAvatarHeight"] = setting.Avatar.MaxHeight + profileContext(ctx) if ctx.HasError() { ctx.HTML(http.StatusOK, tplSettingsProfile) diff --git a/services/user/user.go b/services/user/user.go index 020d68d432..e16ec7c754 100644 --- a/services/user/user.go +++ b/services/user/user.go @@ -38,35 +38,47 @@ func RenameUser(ctx context.Context, u *user_model.User, newUserName string) err return renameUser(ctx, u, newUserName, false) } -// RenameUser renames a user as an admin. +// AdminRenameUser renames a user as an admin. func AdminRenameUser(ctx context.Context, u *user_model.User, newUserName string) error { return renameUser(ctx, u, newUserName, true) } +// CanUserRename returns if the given user can be renamed. +// +// This is merely a precondition, you likely want to use [RenameUser] or [AdminRenameUser] +// which also takes into consideration username cooldown of a new username. +func CanUserRename(ctx context.Context, user *user_model.User) (bool, error) { + // Non-local users are not allowed to change their username. + // Organizations can always be renamed. + if user.IsOrganization() || user.IsLocal() { + return true, nil + } + + // If the user's authentication source is OAuth2 and that source allows for + // username changes then don't make a fuzz about it. + if !user.IsOAuth2() { + return false, nil + } + + source, err := auth.GetSourceByID(ctx, user.LoginSource) + if err != nil { + return false, err + } + return source.Cfg.(*oauth2.Source).AllowUsernameChange, nil +} + func renameUser(ctx context.Context, u *user_model.User, newUserName string, doerIsAdmin bool) error { if newUserName == u.Name { return nil } - // Non-local users are not allowed to change their username. // If the doer is an admin, then allow the rename - they know better. - if !doerIsAdmin && !u.IsOrganization() && !u.IsLocal() { - // If the user's authentication source is OAuth2 and that source allows for - // username changes then don't make a fuzz about it. - - if !u.IsOAuth2() { - return user_model.ErrUserIsNotLocal{ - UID: u.ID, - Name: u.Name, - } - } - - source, err := auth.GetSourceByID(ctx, u.LoginSource) + if !doerIsAdmin { + canRenamed, err := CanUserRename(ctx, u) if err != nil { return err } - sourceCfg := source.Cfg.(*oauth2.Source) - if !sourceCfg.AllowUsernameChange { + if !canRenamed { return user_model.ErrUserIsNotLocal{ UID: u.ID, Name: u.Name, diff --git a/templates/user/settings/profile.tmpl b/templates/user/settings/profile.tmpl index d43a4d4c6e..49a349a4f1 100644 --- a/templates/user/settings/profile.tmpl +++ b/templates/user/settings/profile.tmpl @@ -9,8 +9,8 @@ {{ctx.Locale.Tr "settings.profile_desc"}}