diff --git a/cmd/admin_auth_oauth.go b/cmd/admin_auth_oauth.go index 8a756480cd..ef5d9116e3 100644 --- a/cmd/admin_auth_oauth.go +++ b/cmd/admin_auth_oauth.go @@ -125,6 +125,10 @@ func oauthCLIFlags() []cli.Flag { Name: "group-team-map-removal", Usage: "Activate automatic team membership removal depending on groups", }, + &cli.BoolFlag{ + Name: "allow-username-change", + Usage: "Allow users to change their username", + }, } } @@ -176,6 +180,7 @@ func parseOAuth2Config(_ context.Context, c *cli.Command) *oauth2.Source { RestrictedGroup: c.String("restricted-group"), GroupTeamMap: c.String("group-team-map"), GroupTeamMapRemoval: c.Bool("group-team-map-removal"), + AllowUsernameChange: c.Bool("allow-username-change"), } } @@ -277,6 +282,10 @@ func (a *authService) updateOauth(ctx context.Context, c *cli.Command) error { oAuth2Config.GroupTeamMapRemoval = c.Bool("group-team-map-removal") } + if c.IsSet("allow-username-change") { + oAuth2Config.AllowUsernameChange = c.Bool("allow-username-change") + } + // update custom URL mapping customURLMapping := &oauth2.CustomURLMapping{} diff --git a/cmd/admin_auth_oauth_test.go b/cmd/admin_auth_oauth_test.go index ea5442e62d..3430ad1f56 100644 --- a/cmd/admin_auth_oauth_test.go +++ b/cmd/admin_auth_oauth_test.go @@ -55,6 +55,7 @@ func TestAddOauth(t *testing.T) { "--restricted-group", "restricted", "--group-team-map", `{"org_a_team_1": {"organization-a": ["Team 1"]}, "org_a_all_teams": {"organization-a": ["Team 1", "Team 2", "Team 3"]}}`, "--group-team-map-removal", + "--allow-username-change", }, source: &auth.Source{ Type: auth.OAuth2, @@ -83,6 +84,7 @@ func TestAddOauth(t *testing.T) { GroupTeamMapRemoval: true, RestrictedGroup: "restricted", SkipLocalTwoFA: true, + AllowUsernameChange: true, }, }, }, diff --git a/models/user/fixtures/login_source.yml b/models/user/fixtures/login_source.yml new file mode 100644 index 0000000000..3950f85964 --- /dev/null +++ b/models/user/fixtures/login_source.yml @@ -0,0 +1,8 @@ +- + id: 1001 + type: 6 # OAuth2 + name: OAuth2 authentication source + is_active: 1 + cfg: '{"Provider":"invalid","ClientID":"invalid","ClientSecret":"invalid","AllowUsernameChange":true}' + created_unix: 1753740851 + updated_unix: 1753740851 diff --git a/models/user/fixtures/user.yml b/models/user/fixtures/user.yml index b1892f331b..137064a368 100644 --- a/models/user/fixtures/user.yml +++ b/models/user/fixtures/user.yml @@ -11,6 +11,7 @@ must_change_password: false login_source: 1001 login_name: 123 + login_type: 6 type: 5 salt: ZogKvWdyEx max_repo_creation: -1 diff --git a/options/locale_next/locale_en-US.json b/options/locale_next/locale_en-US.json index e730c84726..aa8d811bd3 100644 --- a/options/locale_next/locale_en-US.json +++ b/options/locale_next/locale_en-US.json @@ -106,6 +106,8 @@ "discussion.sidebar.reference": "Reference", "editor.textarea.tab_hint": "Line already indented. Press Tab again or Escape to leave the editor.", "editor.textarea.shift_tab_hint": "No indentation on this line. Press Shift + Tab again or Escape to leave the editor.", + "admin.auths.allow_username_change": "Allow username change", + "admin.auths.allow_username_change.description": "Allow users to change their username in the profile settings", "admin.dashboard.cleanup_offline_runners": "Cleanup offline runners", "admin.dashboard.remove_resolved_reports": "Remove resolved reports", "settings.visibility.description": "Profile visibility affects others' ability to access your non-private repositories. Learn more.", diff --git a/routers/web/admin/auths.go b/routers/web/admin/auths.go index 2c6dc76305..c352b6ad1a 100644 --- a/routers/web/admin/auths.go +++ b/routers/web/admin/auths.go @@ -190,6 +190,7 @@ func parseOAuth2Config(form forms.AuthenticationForm) *oauth2.Source { AdminGroup: form.Oauth2AdminGroup, GroupTeamMap: form.Oauth2GroupTeamMap, GroupTeamMapRemoval: form.Oauth2GroupTeamMapRemoval, + AllowUsernameChange: form.AllowUsernameChange, } } diff --git a/services/auth/source/oauth2/source.go b/services/auth/source/oauth2/source.go index 5245f88270..a3126cf353 100644 --- a/services/auth/source/oauth2/source.go +++ b/services/auth/source/oauth2/source.go @@ -29,6 +29,7 @@ type Source struct { GroupTeamMapRemoval bool RestrictedGroup string SkipLocalTwoFA bool `json:",omitempty"` + AllowUsernameChange bool // reference to the authSource authSource *auth.Source diff --git a/services/forms/auth_form.go b/services/forms/auth_form.go index e665ca0d19..b89e87f749 100644 --- a/services/forms/auth_form.go +++ b/services/forms/auth_form.go @@ -79,6 +79,7 @@ type AuthenticationForm struct { SkipLocalTwoFA bool GroupTeamMap string `binding:"ValidGroupTeamMap"` GroupTeamMapRemoval bool + AllowUsernameChange bool } // Validate validates fields diff --git a/services/user/user.go b/services/user/user.go index d682d5a434..9cb6858f0c 100644 --- a/services/user/user.go +++ b/services/user/user.go @@ -13,6 +13,7 @@ import ( "forgejo.org/models" asymkey_model "forgejo.org/models/asymkey" + "forgejo.org/models/auth" "forgejo.org/models/db" "forgejo.org/models/organization" packages_model "forgejo.org/models/packages" @@ -25,6 +26,7 @@ import ( "forgejo.org/modules/storage" "forgejo.org/modules/util" "forgejo.org/services/agit" + "forgejo.org/services/auth/source/oauth2" org_service "forgejo.org/services/org" "forgejo.org/services/packages" container_service "forgejo.org/services/packages/container" @@ -49,9 +51,26 @@ func renameUser(ctx context.Context, u *user_model.User, newUserName string, doe // 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() { - return user_model.ErrUserIsNotLocal{ - UID: u.ID, - Name: u.Name, + // 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 err != nil { + return err + } + sourceCfg := source.Cfg.(*oauth2.Source) + if !sourceCfg.AllowUsernameChange { + 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 f1cab60a6d..0747833557 100644 --- a/services/user/user_test.go +++ b/services/user/user_test.go @@ -25,6 +25,7 @@ import ( "forgejo.org/modules/setting" "forgejo.org/modules/test" "forgejo.org/modules/timeutil" + "forgejo.org/services/auth/source/oauth2" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -140,23 +141,10 @@ func TestCreateUser(t *testing.T) { } func TestRenameUser(t *testing.T) { + defer unittest.OverrideFixtures("models/user/fixtures/")() require.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{ - ID: 2, - Name: "old-name", - Type: user_model.UserTypeIndividual, - LoginType: auth.OAuth2, - } - require.ErrorIs(t, RenameUser(db.DefaultContext, u, "user_rename2"), user_model.ErrUserIsNotLocal{UID: 2, Name: "old-name"}) - - t.Run("Admin", func(t *testing.T) { - require.NoError(t, AdminRenameUser(t.Context(), u, "user_rename2")) - }) - }) - t.Run("Same username", func(t *testing.T) { require.NoError(t, RenameUser(db.DefaultContext, user, user.Name)) }) @@ -225,6 +213,30 @@ func TestRenameUser(t *testing.T) { unittest.AssertExistsIf(t, true, &user_model.Redirect{LowerName: "redirect-1"}) unittest.AssertExistsIf(t, true, &user_model.Redirect{LowerName: "redirect-2"}) }) + + t.Run("Non-local", func(t *testing.T) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1041, LoginSource: 1001}) + authSource := unittest.AssertExistsAndLoadBean(t, &auth.Source{ID: user.LoginSource}) + assert.False(t, user.IsLocal()) + assert.True(t, user.IsOAuth2()) + + t.Run("Allowed", func(t *testing.T) { + require.NoError(t, RenameUser(t.Context(), user, "I-am-a-local-username")) + }) + + t.Run("Not allowed", func(t *testing.T) { + authSourceCfg := authSource.Cfg.(*oauth2.Source) + authSourceCfg.AllowUsernameChange = false + authSource.Cfg = authSourceCfg + _, err := db.GetEngine(t.Context()).Cols("cfg").ID(authSource.ID).Update(authSource) + require.NoError(t, err) + + require.ErrorIs(t, RenameUser(t.Context(), user, "Another-username-change"), user_model.ErrUserIsNotLocal{UID: user.ID, Name: user.Name}) + t.Run("Admin", func(t *testing.T) { + require.NoError(t, AdminRenameUser(t.Context(), user, "Another-username-change")) + }) + }) + }) } func TestCreateUser_Issue5882(t *testing.T) { diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl index 1ca5573cae..11be03acc5 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -299,6 +299,13 @@

{{ctx.Locale.Tr "admin.auths.skip_local_two_fa_helper"}}

+
+
+ + +

{{ctx.Locale.Tr "admin.auths.allow_username_change.description"}}

+
+
diff --git a/templates/admin/auth/source/oauth.tmpl b/templates/admin/auth/source/oauth.tmpl index 7d0a64d269..0cb9ea56bc 100644 --- a/templates/admin/auth/source/oauth.tmpl +++ b/templates/admin/auth/source/oauth.tmpl @@ -35,6 +35,13 @@

{{ctx.Locale.Tr "admin.auths.skip_local_two_fa_helper"}}

+
+
+ + +

{{ctx.Locale.Tr "admin.auths.allow_username_change.description"}}

+
+
diff --git a/tests/integration/admin_auth_source_test.go b/tests/integration/admin_auth_source_test.go new file mode 100644 index 0000000000..4b46541318 --- /dev/null +++ b/tests/integration/admin_auth_source_test.go @@ -0,0 +1,32 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later. + +package integration + +import ( + "fmt" + "net/http" + "testing" + + "forgejo.org/models/auth" + "forgejo.org/tests" +) + +func TestAdminAuthAllowUsernameChangeSetting(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + session := loginUser(t, "user1") + + source := addAuthSource(t, map[string]string{ + "type": fmt.Sprintf("%d", auth.OAuth2), + "name": "some-name", + "is_active": "on", + "allow_username_change": "on", + "oauth2_provider": "gitlab", + }) + + response := session.MakeRequest(t, NewRequestf(t, "GET", "/admin/auths/%d", source.ID), http.StatusOK) + htmlDoc := NewHTMLParser(t, response.Body) + + htmlDoc.AssertElement(t, "#allow_username_change[checked]", true) +}