mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2025-08-19 08:51:10 +00:00
feat: add option to allow non-local users to change usernames (#8714)
Add a new config option for OAuth2 authentication sources: allow users to change their username. In the case where OAuth2 is more like a social OAuth2 login there's no need to not allow users to change their username. The information how the user is linked to the authentication source is stored in different fields. Resolves forgejo/forgejo#687 Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8714 Reviewed-by: 0ko <0ko@noreply.codeberg.org> Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org> Co-authored-by: Gusted <postmaster@gusted.xyz> Co-committed-by: Gusted <postmaster@gusted.xyz>
This commit is contained in:
parent
16a0c97fbf
commit
b51f97e97d
13 changed files with 119 additions and 17 deletions
|
@ -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{}
|
||||
|
||||
|
|
|
@ -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,
|
||||
},
|
||||
},
|
||||
},
|
||||
|
|
8
models/user/fixtures/login_source.yml
Normal file
8
models/user/fixtures/login_source.yml
Normal file
|
@ -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
|
|
@ -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
|
||||
|
|
|
@ -106,6 +106,8 @@
|
|||
"discussion.sidebar.reference": "Reference",
|
||||
"editor.textarea.tab_hint": "Line already indented. Press <kbd>Tab</kbd> again or <kbd>Escape</kbd> to leave the editor.",
|
||||
"editor.textarea.shift_tab_hint": "No indentation on this line. Press <kbd>Shift</kbd> + <kbd>Tab</kbd> again or <kbd>Escape</kbd> 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. <a href=\"%s\" target=\"_blank\">Learn more</a>.",
|
||||
|
|
|
@ -190,6 +190,7 @@ func parseOAuth2Config(form forms.AuthenticationForm) *oauth2.Source {
|
|||
AdminGroup: form.Oauth2AdminGroup,
|
||||
GroupTeamMap: form.Oauth2GroupTeamMap,
|
||||
GroupTeamMapRemoval: form.Oauth2GroupTeamMapRemoval,
|
||||
AllowUsernameChange: form.AllowUsernameChange,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -29,6 +29,7 @@ type Source struct {
|
|||
GroupTeamMapRemoval bool
|
||||
RestrictedGroup string
|
||||
SkipLocalTwoFA bool `json:",omitempty"`
|
||||
AllowUsernameChange bool
|
||||
|
||||
// reference to the authSource
|
||||
authSource *auth.Source
|
||||
|
|
|
@ -79,6 +79,7 @@ type AuthenticationForm struct {
|
|||
SkipLocalTwoFA bool
|
||||
GroupTeamMap string `binding:"ValidGroupTeamMap"`
|
||||
GroupTeamMapRemoval bool
|
||||
AllowUsernameChange bool
|
||||
}
|
||||
|
||||
// Validate validates fields
|
||||
|
|
|
@ -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,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -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) {
|
||||
|
|
|
@ -299,6 +299,13 @@
|
|||
<p class="help">{{ctx.Locale.Tr "admin.auths.skip_local_two_fa_helper"}}</p>
|
||||
</div>
|
||||
</div>
|
||||
<div class="optional field">
|
||||
<div class="ui checkbox">
|
||||
<label for="allow_username_change"><strong>{{ctx.Locale.Tr "admin.auths.allow_username_change"}}</strong></label>
|
||||
<input id="allow_username_change" name="allow_username_change" type="checkbox" {{if $cfg.AllowUsernameChange}}checked{{end}}>
|
||||
<p class="help">{{ctx.Locale.Tr "admin.auths.allow_username_change.description"}}</p>
|
||||
</div>
|
||||
</div>
|
||||
<div class="oauth2_use_custom_url inline field">
|
||||
<div class="ui checkbox">
|
||||
<label><strong>{{ctx.Locale.Tr "admin.auths.oauth2_use_custom_url"}}</strong></label>
|
||||
|
|
|
@ -35,6 +35,13 @@
|
|||
<p class="help">{{ctx.Locale.Tr "admin.auths.skip_local_two_fa_helper"}}</p>
|
||||
</div>
|
||||
</div>
|
||||
<div class="optional field">
|
||||
<div class="ui checkbox">
|
||||
<label for="allow_username_change"><strong>{{ctx.Locale.Tr "admin.auths.allow_username_change"}}</strong></label>
|
||||
<input id="allow_username_change" name="allow_username_change" type="checkbox" {{if .allow_username_change}}checked{{end}}>
|
||||
<p class="help">{{ctx.Locale.Tr "admin.auths.allow_username_change.description"}}</p>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<div class="oauth2_use_custom_url inline field">
|
||||
<div class="ui checkbox">
|
||||
|
|
32
tests/integration/admin_auth_source_test.go
Normal file
32
tests/integration/admin_auth_source_test.go
Normal file
|
@ -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)
|
||||
}
|
Loading…
Add table
Add a link
Reference in a new issue