summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorGusted <postmaster@gusted.xyz>2025-01-24 05:16:56 +0100
committerGusted <gusted@noreply.codeberg.org>2025-01-24 05:16:56 +0100
commita9c97110f9da75776d6f9f86adcca712367fb052 (patch)
tree63622738e22357587e988f7738ebd56c8d64ccfa
parentUpdate module google.golang.org/grpc to v1.69.4 (forgejo) (#6560) (diff)
downloadforgejo-a9c97110f9da75776d6f9f86adcca712367fb052.tar.xz
forgejo-a9c97110f9da75776d6f9f86adcca712367fb052.zip
feat: add configurable cooldown to claim usernames (#6422)
Add a new option that allows instances to set a cooldown period to claim old usernames. In the context of public instances this can be used to prevent old usernames to be claimed after they are free and allow graceful migration (by making use of the redirect feature) to a new username. The granularity of this cooldown is a day. By default this feature is disabled and thus no cooldown period. The `CreatedUnix` column is added the `user_redirect` table, for existing redirects the timestamp is simply zero as we simply do not know when they were created and are likely already over the cooldown period if the instance configures one. Users can always reclaim their 'old' user name again within the cooldown period. Users can also always reclaim 'old' names of organization they currently own within the cooldown period. Creating and renaming users as an admin user are not affected by the cooldown period for moderation and user support reasons. To avoid abuse of the cooldown feature, such that a user holds a lot of usernames, a new option is added `MAX_USER_REDIRECTS` which sets a limit to the amount of user redirects a user may have, by default this is disabled. If a cooldown period is set then the default is 5. This feature operates independently of the cooldown period feature. Added integration and unit testing. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6422 Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org> Reviewed-by: 0ko <0ko@noreply.codeberg.org> Reviewed-by: Otto <otto@codeberg.org> Co-authored-by: Gusted <postmaster@gusted.xyz> Co-committed-by: Gusted <postmaster@gusted.xyz>
-rw-r--r--models/fixtures/user_redirect.yml1
-rw-r--r--models/forgejo_migrations/migrate.go2
-rw-r--r--models/forgejo_migrations/v27.go18
-rw-r--r--models/user/redirect.go101
-rw-r--r--models/user/user.go12
-rw-r--r--models/user/user_test.go25
-rw-r--r--modules/setting/server_test.go39
-rw-r--r--modules/setting/service.go10
-rw-r--r--options/locale/locale_en-US.ini5
-rw-r--r--routers/api/v1/admin/user.go2
-rw-r--r--routers/web/admin/users.go2
-rw-r--r--routers/web/auth/auth.go4
-rw-r--r--routers/web/org/setting.go6
-rw-r--r--routers/web/user/setting/profile.go5
-rw-r--r--services/user/user.go26
-rw-r--r--services/user/user_test.go23
-rw-r--r--templates/org/settings/options.tmpl18
-rw-r--r--templates/user/settings/profile.tmpl4
-rw-r--r--tests/integration/user_redirect_test.go274
19 files changed, 561 insertions, 16 deletions
diff --git a/models/fixtures/user_redirect.yml b/models/fixtures/user_redirect.yml
index 8ff7993398..f471e94511 100644
--- a/models/fixtures/user_redirect.yml
+++ b/models/fixtures/user_redirect.yml
@@ -2,3 +2,4 @@
id: 1
lower_name: olduser1
redirect_user_id: 1
+ created_unix: 1730000000
diff --git a/models/forgejo_migrations/migrate.go b/models/forgejo_migrations/migrate.go
index 3bf7843211..1450ad3c54 100644
--- a/models/forgejo_migrations/migrate.go
+++ b/models/forgejo_migrations/migrate.go
@@ -90,6 +90,8 @@ var migrations = []*Migration{
NewMigration("Migrate `secret` column to store keying material", MigrateTwoFactorToKeying),
// v26 -> v27
NewMigration("Add `hash_blake2b` column to `package_blob` table", AddHashBlake2bToPackageBlob),
+ // v27 -> v28
+ NewMigration("Add `created_unix` column to `user_redirect` table", AddCreatedUnixToRedirect),
}
// GetCurrentDBVersion returns the current Forgejo database version.
diff --git a/models/forgejo_migrations/v27.go b/models/forgejo_migrations/v27.go
new file mode 100644
index 0000000000..b3a93a9aad
--- /dev/null
+++ b/models/forgejo_migrations/v27.go
@@ -0,0 +1,18 @@
+// Copyright 2024 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package forgejo_migrations //nolint:revive
+
+import (
+ "code.gitea.io/gitea/modules/timeutil"
+
+ "xorm.io/xorm"
+)
+
+func AddCreatedUnixToRedirect(x *xorm.Engine) error {
+ type UserRedirect struct {
+ ID int64 `xorm:"pk autoincr"`
+ CreatedUnix timeutil.TimeStamp `xorm:"created NOT NULL DEFAULT 0"`
+ }
+ return x.Sync(new(UserRedirect))
+}
diff --git a/models/user/redirect.go b/models/user/redirect.go
index 5a40d4df3b..0ea959e99d 100644
--- a/models/user/redirect.go
+++ b/models/user/redirect.go
@@ -6,10 +6,17 @@ package user
import (
"context"
"fmt"
+ "slices"
+ "strconv"
"strings"
+ "time"
"code.gitea.io/gitea/models/db"
+ "code.gitea.io/gitea/modules/setting"
+ "code.gitea.io/gitea/modules/timeutil"
"code.gitea.io/gitea/modules/util"
+
+ "xorm.io/builder"
)
// ErrUserRedirectNotExist represents a "UserRedirectNotExist" kind of error.
@@ -31,11 +38,25 @@ func (err ErrUserRedirectNotExist) Unwrap() error {
return util.ErrNotExist
}
+type ErrCooldownPeriod struct {
+ ExpireTime time.Time
+}
+
+func IsErrCooldownPeriod(err error) bool {
+ _, ok := err.(ErrCooldownPeriod)
+ return ok
+}
+
+func (err ErrCooldownPeriod) Error() string {
+ return fmt.Sprintf("cooldown period for claiming this username has not yet expired: the cooldown period ends at %s", err.ExpireTime)
+}
+
// Redirect represents that a user name should be redirected to another
type Redirect struct {
- ID int64 `xorm:"pk autoincr"`
- LowerName string `xorm:"UNIQUE(s) INDEX NOT NULL"`
- RedirectUserID int64 // userID to redirect to
+ ID int64 `xorm:"pk autoincr"`
+ LowerName string `xorm:"UNIQUE(s) INDEX NOT NULL"`
+ RedirectUserID int64 // userID to redirect to
+ CreatedUnix timeutil.TimeStamp `xorm:"created NOT NULL DEFAULT 0"`
}
// TableName provides the real table name
@@ -47,14 +68,24 @@ func init() {
db.RegisterModel(new(Redirect))
}
-// LookupUserRedirect look up userID if a user has a redirect name
-func LookupUserRedirect(ctx context.Context, userName string) (int64, error) {
+// GetUserRedirect returns the redirect for a given username, this is a
+// case-insenstive operation.
+func GetUserRedirect(ctx context.Context, userName string) (*Redirect, error) {
userName = strings.ToLower(userName)
redirect := &Redirect{LowerName: userName}
if has, err := db.GetEngine(ctx).Get(redirect); err != nil {
- return 0, err
+ return nil, err
} else if !has {
- return 0, ErrUserRedirectNotExist{Name: userName}
+ return nil, ErrUserRedirectNotExist{Name: userName}
+ }
+ return redirect, nil
+}
+
+// LookupUserRedirect look up userID if a user has a redirect name
+func LookupUserRedirect(ctx context.Context, userName string) (int64, error) {
+ redirect, err := GetUserRedirect(ctx, userName)
+ if err != nil {
+ return 0, err
}
return redirect.RedirectUserID, nil
}
@@ -78,6 +109,19 @@ func NewUserRedirect(ctx context.Context, ID int64, oldUserName, newUserName str
})
}
+// LimitUserRedirects deletes the oldest entries in user_redirect of the user,
+// such that the amount of user_redirects is at most `n` amount of entries.
+func LimitUserRedirects(ctx context.Context, userID, n int64) error {
+ // NOTE: It's not possible to combine these two queries into one due to a limitation of MySQL.
+ keepIDs := make([]int64, n)
+ if err := db.GetEngine(ctx).SQL("SELECT id FROM user_redirect WHERE redirect_user_id = ? ORDER BY created_unix DESC LIMIT "+strconv.FormatInt(n, 10), userID).Find(&keepIDs); err != nil {
+ return err
+ }
+
+ _, err := db.GetEngine(ctx).Exec(builder.Delete(builder.And(builder.Eq{"redirect_user_id": userID}, builder.NotIn("id", keepIDs))).From("user_redirect"))
+ return err
+}
+
// DeleteUserRedirect delete any redirect from the specified user name to
// anything else
func DeleteUserRedirect(ctx context.Context, userName string) error {
@@ -85,3 +129,46 @@ func DeleteUserRedirect(ctx context.Context, userName string) error {
_, err := db.GetEngine(ctx).Delete(&Redirect{LowerName: userName})
return err
}
+
+// CanClaimUsername returns if its possible to claim the given username,
+// it checks if the cooldown period for claiming an existing username is over.
+// If there's a cooldown period, the second argument returns the time when
+// that cooldown period is over.
+// In the scenario of renaming, the doerID can be specified to allow the original
+// user of the username to reclaim it within the cooldown period.
+func CanClaimUsername(ctx context.Context, username string, doerID int64) (bool, time.Time, error) {
+ // Only check for a cooldown period if UsernameCooldownPeriod is a positive number.
+ if setting.Service.UsernameCooldownPeriod <= 0 {
+ return true, time.Time{}, nil
+ }
+
+ userRedirect, err := GetUserRedirect(ctx, username)
+ if err != nil {
+ if IsErrUserRedirectNotExist(err) {
+ return true, time.Time{}, nil
+ }
+ return false, time.Time{}, err
+ }
+
+ // Allow reclaiming of user's own username.
+ if userRedirect.RedirectUserID == doerID {
+ return true, time.Time{}, nil
+ }
+
+ // We do not know if the redirect user id was for an organization, so
+ // unconditionally execute the following query to retrieve all users that
+ // are part of the "Owner" team. If the redirect user ID is not an organization
+ // the returned list would be empty.
+ ownerTeamUIDs := []int64{}
+ if err := db.GetEngine(ctx).SQL("SELECT uid FROM team_user INNER JOIN team ON team_user.`team_id` = team.`id` WHERE team.`org_id` = ? AND team.`name` = 'Owners'", userRedirect.RedirectUserID).Find(&ownerTeamUIDs); err != nil {
+ return false, time.Time{}, err
+ }
+
+ if slices.Contains(ownerTeamUIDs, doerID) {
+ return true, time.Time{}, nil
+ }
+
+ // Multiply the value of UsernameCooldownPeriod by the amount of seconds in a day.
+ expireTime := userRedirect.CreatedUnix.Add(86400 * setting.Service.UsernameCooldownPeriod).AsLocalTime()
+ return time.Until(expireTime) <= 0, expireTime, nil
+}
diff --git a/models/user/user.go b/models/user/user.go
index ec9b35964d..423a26c8d3 100644
--- a/models/user/user.go
+++ b/models/user/user.go
@@ -669,6 +669,18 @@ func createUser(ctx context.Context, u *User, createdByAdmin bool, overwriteDefa
return err
}
+ // Check if the new username can be claimed.
+ // Skip this check if done by an admin.
+ if !createdByAdmin {
+ if ok, expireTime, err := CanClaimUsername(ctx, u.Name, -1); err != nil {
+ return err
+ } else if !ok {
+ return ErrCooldownPeriod{
+ ExpireTime: expireTime,
+ }
+ }
+ }
+
// set system defaults
u.KeepEmailPrivate = setting.Service.DefaultKeepEmailPrivate
u.Visibility = setting.Service.DefaultUserVisibilityMode
diff --git a/models/user/user_test.go b/models/user/user_test.go
index 1c734fa926..df0c3856e9 100644
--- a/models/user/user_test.go
+++ b/models/user/user_test.go
@@ -393,6 +393,31 @@ func TestCreateUserWithoutCustomTimestamps(t *testing.T) {
assert.LessOrEqual(t, fetched.UpdatedUnix, timestampEnd)
}
+func TestCreateUserClaimingUsername(t *testing.T) {
+ require.NoError(t, unittest.PrepareTestDatabase())
+ defer test.MockVariableValue(&setting.Service.UsernameCooldownPeriod, 1)()
+
+ _, err := db.GetEngine(db.DefaultContext).NoAutoTime().Insert(&user_model.Redirect{RedirectUserID: 1, LowerName: "redirecting", CreatedUnix: timeutil.TimeStampNow()})
+ require.NoError(t, err)
+
+ user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+
+ user.Name = "redirecting"
+ user.LowerName = strings.ToLower(user.Name)
+ user.ID = 0
+ user.Email = "unique@example.com"
+
+ t.Run("Normal creation", func(t *testing.T) {
+ err = user_model.CreateUser(db.DefaultContext, user)
+ assert.True(t, user_model.IsErrCooldownPeriod(err))
+ })
+
+ t.Run("Creation as admin", func(t *testing.T) {
+ err = user_model.AdminCreateUser(db.DefaultContext, user)
+ require.NoError(t, err)
+ })
+}
+
func TestGetUserIDsByNames(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
diff --git a/modules/setting/server_test.go b/modules/setting/server_test.go
index 8db8168854..7054c474ff 100644
--- a/modules/setting/server_test.go
+++ b/modules/setting/server_test.go
@@ -9,6 +9,7 @@ import (
"code.gitea.io/gitea/modules/test"
"github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
)
func TestDisplayNameDefault(t *testing.T) {
@@ -34,3 +35,41 @@ func TestDisplayNameCustomFormat(t *testing.T) {
displayName := generateDisplayName()
assert.Equal(t, "Forgejo - Beyond coding. We Forge.", displayName)
}
+
+func TestMaxUserRedirectsDefault(t *testing.T) {
+ iniStr := ``
+ cfg, err := NewConfigProviderFromData(iniStr)
+ require.NoError(t, err)
+ loadServiceFrom(cfg)
+
+ assert.EqualValues(t, 0, Service.UsernameCooldownPeriod)
+ assert.EqualValues(t, 0, Service.MaxUserRedirects)
+
+ iniStr = `[service]
+MAX_USER_REDIRECTS = 8`
+ cfg, err = NewConfigProviderFromData(iniStr)
+ require.NoError(t, err)
+ loadServiceFrom(cfg)
+
+ assert.EqualValues(t, 0, Service.UsernameCooldownPeriod)
+ assert.EqualValues(t, 8, Service.MaxUserRedirects)
+
+ iniStr = `[service]
+USERNAME_COOLDOWN_PERIOD = 3`
+ cfg, err = NewConfigProviderFromData(iniStr)
+ require.NoError(t, err)
+ loadServiceFrom(cfg)
+
+ assert.EqualValues(t, 3, Service.UsernameCooldownPeriod)
+ assert.EqualValues(t, 5, Service.MaxUserRedirects)
+
+ iniStr = `[service]
+USERNAME_COOLDOWN_PERIOD = 3
+MAX_USER_REDIRECTS = 8`
+ cfg, err = NewConfigProviderFromData(iniStr)
+ require.NoError(t, err)
+ loadServiceFrom(cfg)
+
+ assert.EqualValues(t, 3, Service.UsernameCooldownPeriod)
+ assert.EqualValues(t, 8, Service.MaxUserRedirects)
+}
diff --git a/modules/setting/service.go b/modules/setting/service.go
index 5a6cc254e0..9807f33352 100644
--- a/modules/setting/service.go
+++ b/modules/setting/service.go
@@ -85,6 +85,8 @@ var Service = struct {
DefaultOrgMemberVisible bool
UserDeleteWithCommentsMaxTime time.Duration
ValidSiteURLSchemes []string
+ UsernameCooldownPeriod int64
+ MaxUserRedirects int64
// OpenID settings
EnableOpenIDSignIn bool
@@ -257,6 +259,14 @@ func loadServiceFrom(rootCfg ConfigProvider) {
}
}
Service.ValidSiteURLSchemes = schemes
+ Service.UsernameCooldownPeriod = sec.Key("USERNAME_COOLDOWN_PERIOD").MustInt64(0)
+
+ // Only set a default if USERNAME_COOLDOWN_PERIOD's feature is active.
+ maxUserRedirectsDefault := int64(0)
+ if Service.UsernameCooldownPeriod > 0 {
+ maxUserRedirectsDefault = 5
+ }
+ Service.MaxUserRedirects = sec.Key("MAX_USER_REDIRECTS").MustInt64(maxUserRedirectsDefault)
mustMapSetting(rootCfg, "service.explore", &Service.Explore)
diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
index 2e01ade7a6..da2fa60e81 100644
--- a/options/locale/locale_en-US.ini
+++ b/options/locale/locale_en-US.ini
@@ -630,6 +630,7 @@ lang_select_error = Select a language from the list.
username_been_taken = The username is already taken.
username_change_not_local_user = Non-local users are not allowed to change their username.
+username_claiming_cooldown = The username cannot be claimed, because its cooldown period is not yet over. It can be claimed on %[1]s.
repo_name_been_taken = The repository name is already used.
repository_force_private = Force Private is enabled: private repositories cannot be made public.
repository_files_already_exist = Files already exist for this repository. Contact the system administrator.
@@ -765,6 +766,8 @@ update_profile_success = Your profile has been updated.
change_username = Your username has been changed.
change_username_prompt = Note: Changing your username also changes your account URL.
change_username_redirect_prompt = The old username will redirect until someone claims it.
+change_username_redirect_prompt.with_cooldown.one = The old username will be available to everyone after a cooldown period of %[1]d day, you can still reclaim the old username during the cooldown period.
+change_username_redirect_prompt.with_cooldown.few = The old username will be available to everyone after a cooldown period of %[1]d days, you can still reclaim the old username during the cooldown period.
continue = Continue
cancel = Cancel
language = Language
@@ -2883,6 +2886,8 @@ settings.update_settings = Update settings
settings.update_setting_success = Organization settings have been updated.
settings.change_orgname_prompt = Note: Changing the organization name will also change your organization's URL and free the old name.
settings.change_orgname_redirect_prompt = The old name will redirect until it is claimed.
+settings.change_orgname_redirect_prompt.with_cooldown.one = The old username will be available to everyone after a cooldown period of %[1]d day, you can still reclaim the old username during the cooldown period.
+settings.change_orgname_redirect_prompt.with_cooldown.few = The old username will be available to everyone after a cooldown period of %[1]d days, you can still reclaim the old username during the cooldown period.
settings.update_avatar_success = The organization's avatar has been updated.
settings.delete = Delete organization
settings.delete_account = Delete this organization
diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go
index acab883107..db8f6627e6 100644
--- a/routers/api/v1/admin/user.go
+++ b/routers/api/v1/admin/user.go
@@ -523,7 +523,7 @@ func RenameUser(ctx *context.APIContext) {
newName := web.GetForm(ctx).(*api.RenameUserOption).NewName
// Check if user name has been changed
- if err := user_service.RenameUser(ctx, ctx.ContextUser, newName); err != nil {
+ if err := user_service.AdminRenameUser(ctx, ctx.ContextUser, newName); err != nil {
switch {
case user_model.IsErrUserAlreadyExist(err):
ctx.Error(http.StatusUnprocessableEntity, "", ctx.Tr("form.username_been_taken"))
diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go
index fb4a1ffa61..6bfc35cb99 100644
--- a/routers/web/admin/users.go
+++ b/routers/web/admin/users.go
@@ -349,7 +349,7 @@ func EditUserPost(ctx *context.Context) {
}
if form.UserName != "" {
- if err := user_service.RenameUser(ctx, u, form.UserName); err != nil {
+ if err := user_service.AdminRenameUser(ctx, u, form.UserName); err != nil {
switch {
case user_model.IsErrUserIsNotLocal(err):
ctx.Data["Err_UserName"] = true
diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go
index ccab47a9a2..5cb4ebb440 100644
--- a/routers/web/auth/auth.go
+++ b/routers/web/auth/auth.go
@@ -9,6 +9,7 @@ import (
"fmt"
"net/http"
"strings"
+ "time"
"code.gitea.io/gitea/models/auth"
"code.gitea.io/gitea/models/db"
@@ -555,6 +556,9 @@ func createUserInContext(ctx *context.Context, tpl base.TplName, form any, u *us
case user_model.IsErrEmailAlreadyUsed(err):
ctx.Data["Err_Email"] = true
ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tpl, form)
+ case user_model.IsErrCooldownPeriod(err):
+ ctx.Data["Err_UserName"] = true
+ ctx.RenderWithErr(ctx.Locale.Tr("form.username_claiming_cooldown", err.(user_model.ErrCooldownPeriod).ExpireTime.Format(time.RFC1123Z)), tpl, form)
case validation.IsErrEmailCharIsNotSupported(err):
ctx.Data["Err_Email"] = true
ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tpl, form)
diff --git a/routers/web/org/setting.go b/routers/web/org/setting.go
index 8bd8ae6126..1683728a8e 100644
--- a/routers/web/org/setting.go
+++ b/routers/web/org/setting.go
@@ -7,6 +7,7 @@ package org
import (
"net/http"
"net/url"
+ "time"
"code.gitea.io/gitea/models"
"code.gitea.io/gitea/models/db"
@@ -48,6 +49,7 @@ func Settings(ctx *context.Context) {
ctx.Data["CurrentVisibility"] = ctx.Org.Organization.Visibility
ctx.Data["RepoAdminChangeTeamAccess"] = ctx.Org.Organization.RepoAdminChangeTeamAccess
ctx.Data["ContextUser"] = ctx.ContextUser
+ ctx.Data["CooldownPeriod"] = setting.Service.UsernameCooldownPeriod
err := shared_user.LoadHeaderCount(ctx)
if err != nil {
@@ -65,6 +67,7 @@ func SettingsPost(ctx *context.Context) {
ctx.Data["PageIsOrgSettings"] = true
ctx.Data["PageIsSettingsOptions"] = true
ctx.Data["CurrentVisibility"] = ctx.Org.Organization.Visibility
+ ctx.Data["CooldownPeriod"] = setting.Service.UsernameCooldownPeriod
if ctx.HasError() {
ctx.HTML(http.StatusOK, tplSettingsOptions)
@@ -78,6 +81,9 @@ func SettingsPost(ctx *context.Context) {
if user_model.IsErrUserAlreadyExist(err) {
ctx.Data["Err_Name"] = true
ctx.RenderWithErr(ctx.Tr("form.username_been_taken"), tplSettingsOptions, &form)
+ } else if user_model.IsErrCooldownPeriod(err) {
+ ctx.Data["Err_UserName"] = true
+ ctx.RenderWithErr(ctx.Locale.Tr("form.username_claiming_cooldown", err.(user_model.ErrCooldownPeriod).ExpireTime.Format(time.RFC1123Z)), tplSettingsOptions, form)
} else if db.IsErrNameReserved(err) {
ctx.Data["Err_Name"] = true
ctx.RenderWithErr(ctx.Tr("repo.form.name_reserved", err.(db.ErrNameReserved).Name), tplSettingsOptions, &form)
diff --git a/routers/web/user/setting/profile.go b/routers/web/user/setting/profile.go
index 907f0f5061..818da9e3fa 100644
--- a/routers/web/user/setting/profile.go
+++ b/routers/web/user/setting/profile.go
@@ -14,6 +14,7 @@ import (
"path/filepath"
"slices"
"strings"
+ "time"
"code.gitea.io/gitea/models/avatars"
"code.gitea.io/gitea/models/db"
@@ -51,6 +52,7 @@ func Profile(ctx *context.Context) {
ctx.Data["AllowedUserVisibilityModes"] = setting.Service.AllowedUserVisibilityModesSlice.ToVisibleTypeSlice()
ctx.Data["DisableGravatar"] = setting.Config().Picture.DisableGravatar.Value(ctx)
ctx.Data["PronounsAreCustom"] = !slices.Contains(recognisedPronouns, ctx.Doer.Pronouns)
+ ctx.Data["CooldownPeriod"] = setting.Service.UsernameCooldownPeriod
ctx.HTML(http.StatusOK, tplSettingsProfile)
}
@@ -62,6 +64,7 @@ func ProfilePost(ctx *context.Context) {
ctx.Data["AllowedUserVisibilityModes"] = setting.Service.AllowedUserVisibilityModesSlice.ToVisibleTypeSlice()
ctx.Data["DisableGravatar"] = setting.Config().Picture.DisableGravatar.Value(ctx)
ctx.Data["PronounsAreCustom"] = !slices.Contains(recognisedPronouns, ctx.Doer.Pronouns)
+ ctx.Data["CooldownPeriod"] = setting.Service.UsernameCooldownPeriod
if ctx.HasError() {
ctx.HTML(http.StatusOK, tplSettingsProfile)
@@ -77,6 +80,8 @@ func ProfilePost(ctx *context.Context) {
ctx.Flash.Error(ctx.Tr("form.username_change_not_local_user"))
case user_model.IsErrUserAlreadyExist(err):
ctx.Flash.Error(ctx.Tr("form.username_been_taken"))
+ case user_model.IsErrCooldownPeriod(err):
+ ctx.Flash.Error(ctx.Tr("form.username_claiming_cooldown", err.(user_model.ErrCooldownPeriod).ExpireTime.Format(time.RFC1123Z)))
case db.IsErrNameReserved(err):
ctx.Flash.Error(ctx.Tr("user.form.name_reserved", form.Name))
case db.IsErrNamePatternNotAllowed(err):
diff --git a/services/user/user.go b/services/user/user.go
index 7610a3fbdd..62fe44ca27 100644
--- a/services/user/user.go
+++ b/services/user/user.go
@@ -33,6 +33,15 @@ import (
// RenameUser renames a user
func RenameUser(ctx context.Context, u *user_model.User, newUserName string) error {
+ return renameUser(ctx, u, newUserName, false)
+}
+
+// RenameUser renames a user as an admin.
+func AdminRenameUser(ctx context.Context, u *user_model.User, newUserName string) error {
+ return renameUser(ctx, u, newUserName, true)
+}
+
+func renameUser(ctx context.Context, u *user_model.User, newUserName string, doerIsAdmin bool) error {
if newUserName == u.Name {
return nil
}
@@ -49,6 +58,17 @@ func RenameUser(ctx context.Context, u *user_model.User, newUserName string) err
return err
}
+ // Check if the new username can be claimed.
+ if !doerIsAdmin {
+ if ok, expireTime, err := user_model.CanClaimUsername(ctx, newUserName, u.ID); err != nil {
+ return err
+ } else if !ok {
+ return user_model.ErrCooldownPeriod{
+ ExpireTime: expireTime,
+ }
+ }
+ }
+
onlyCapitalization := strings.EqualFold(newUserName, u.Name)
oldUserName := u.Name
@@ -85,6 +105,12 @@ func RenameUser(ctx context.Context, u *user_model.User, newUserName string) err
return err
}
+ if setting.Service.MaxUserRedirects > 0 {
+ if err := user_model.LimitUserRedirects(ctx, u.ID, setting.Service.MaxUserRedirects); err != nil {
+ return err
+ }
+ }
+
if err := agit.UserNameChanged(ctx, u, newUserName); err != nil {
return err
}
diff --git a/services/user/user_test.go b/services/user/user_test.go
index 719c9d733c..058ff7b6ed 100644
--- a/services/user/user_test.go
+++ b/services/user/user_test.go
@@ -199,6 +199,29 @@ func TestRenameUser(t *testing.T) {
unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: user.ID, OwnerName: user.Name})
})
+
+ t.Run("Keep N redirects", func(t *testing.T) {
+ defer test.MockProtect(&setting.Service.MaxUserRedirects)()
+ // Start clean
+ unittest.AssertSuccessfulDelete(t, &user_model.Redirect{RedirectUserID: user.ID})
+
+ setting.Service.MaxUserRedirects = 1
+
+ require.NoError(t, RenameUser(db.DefaultContext, user, "redirect-1"))
+ unittest.AssertExistsIf(t, true, &user_model.Redirect{LowerName: "user_rename"})
+
+ // The granularity of created_unix is a second.
+ time.Sleep(time.Second)
+ require.NoError(t, RenameUser(db.DefaultContext, user, "redirect-2"))
+ unittest.AssertExistsIf(t, false, &user_model.Redirect{LowerName: "user_rename"})
+ unittest.AssertExistsIf(t, true, &user_model.Redirect{LowerName: "redirect-1"})
+
+ setting.Service.MaxUserRedirects = 2
+ time.Sleep(time.Second)
+ require.NoError(t, RenameUser(db.DefaultContext, user, "redirect-3"))
+ unittest.AssertExistsIf(t, true, &user_model.Redirect{LowerName: "redirect-1"})
+ unittest.AssertExistsIf(t, true, &user_model.Redirect{LowerName: "redirect-2"})
+ })
}
func TestCreateUser_Issue5882(t *testing.T) {
diff --git a/templates/org/settings/options.tmpl b/templates/org/settings/options.tmpl
index 62debfc0ae..2ef7031aef 100644
--- a/templates/org/settings/options.tmpl
+++ b/templates/org/settings/options.tmpl
@@ -6,14 +6,18 @@
<div class="ui attached segment">
<form class="ui form" action="{{.Link}}" method="post">
{{.CsrfTokenHtml}}
- <div class="required field {{if .Err_Name}}error{{end}}">
- <label for="org_name">{{ctx.Locale.Tr "org.org_name_holder"}}
- <span class="text red tw-hidden" id="org-name-change-prompt">
- <br>{{ctx.Locale.Tr "org.settings.change_orgname_prompt"}}<br>{{ctx.Locale.Tr "org.settings.change_orgname_redirect_prompt"}}
- </span>
- </label>
+ <label {{if .Err_Name}}class="field error"{{end}}>
+ {{ctx.Locale.Tr "org.org_name_holder"}}
<input id="org_name" name="name" value="{{.Org.Name}}" data-org-name="{{.Org.Name}}" autofocus required maxlength="40">
- </div>
+ <span class="help">
+ {{ctx.Locale.Tr "org.settings.change_orgname_prompt"}}
+ {{if gt .CooldownPeriod 0}}
+ {{ctx.Locale.TrN .CooldownPeriod "org.settings.change_orgname_redirect_prompt.with_cooldown.one" "org.settings.change_orgname_redirect_prompt.with_cooldown.few" .CooldownPeriod}}</span>
+ {{else}}
+ {{ctx.Locale.Tr "org.settings.change_orgname_redirect_prompt"}}
+ {{end}}
+ </span>
+ </label>
<div class="field {{if .Err_FullName}}error{{end}}">
<label for="full_name">{{ctx.Locale.Tr "org.org_full_name_holder"}}</label>
<input id="full_name" name="full_name" value="{{.Org.FullName}}" maxlength="100">
diff --git a/templates/user/settings/profile.tmpl b/templates/user/settings/profile.tmpl
index 5ecaada4e2..4cc30d66cf 100644
--- a/templates/user/settings/profile.tmpl
+++ b/templates/user/settings/profile.tmpl
@@ -16,7 +16,11 @@
{{else}}
<span class="help">
{{ctx.Locale.Tr "settings.change_username_prompt"}}
+ {{if gt .CooldownPeriod 0}}
+ {{ctx.Locale.TrN .CooldownPeriod "settings.change_username_redirect_prompt.with_cooldown.one" "settings.change_username_redirect_prompt.with_cooldown.few" .CooldownPeriod}}</span>
+ {{else}}
{{ctx.Locale.Tr "settings.change_username_redirect_prompt"}}
+ {{end}}
</span>
{{end}}
</label>
diff --git a/tests/integration/user_redirect_test.go b/tests/integration/user_redirect_test.go
new file mode 100644
index 0000000000..fef0c7cb67
--- /dev/null
+++ b/tests/integration/user_redirect_test.go
@@ -0,0 +1,274 @@
+// Copyright 2024 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package integration
+
+import (
+ "net/http"
+ "testing"
+ "time"
+
+ "code.gitea.io/gitea/models/db"
+ "code.gitea.io/gitea/models/unittest"
+ user_model "code.gitea.io/gitea/models/user"
+ "code.gitea.io/gitea/modules/setting"
+ "code.gitea.io/gitea/modules/test"
+ "code.gitea.io/gitea/modules/timeutil"
+ forgejo_context "code.gitea.io/gitea/services/context"
+ "code.gitea.io/gitea/tests"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+func TestUserRedirect(t *testing.T) {
+ defer tests.PrepareTestEnv(t)()
+ defer test.MockVariableValue(&setting.Service.UsernameCooldownPeriod, 1)()
+
+ session := loginUser(t, "user2")
+
+ t.Run("Rename user normally", func(t *testing.T) {
+ defer tests.PrintCurrentTest(t)()
+
+ req := NewRequestWithValues(t, "POST", "/user/settings", map[string]string{
+ "_csrf": GetCSRF(t, session, "/user/settings"),
+ "name": "user2-new",
+ })
+ session.MakeRequest(t, req, http.StatusSeeOther)
+
+ flashCookie := session.GetCookie(forgejo_context.CookieNameFlash)
+ assert.NotNil(t, flashCookie)
+ assert.EqualValues(t, "success%3DYour%2Bprofile%2Bhas%2Bbeen%2Bupdated.", flashCookie.Value)
+
+ unittest.AssertExistsIf(t, true, &user_model.Redirect{LowerName: "user2", RedirectUserID: 2})
+ })
+
+ t.Run("Create new user", func(t *testing.T) {
+ defer tests.PrintCurrentTest(t)()
+
+ req := NewRequestWithValues(t, "POST", "/user/sign_up", map[string]string{
+ "_csrf": GetCSRF(t, emptyTestSession(t), "/user/sign_up"),
+ "user_name": "user2",
+ "email": "doesnotexist@example.com",
+ "password": "examplePassword!1",
+ "retype": "examplePassword!1",
+ })
+ resp := MakeRequest(t, req, http.StatusOK)
+
+ htmlDoc := NewHTMLParser(t, resp.Body)
+ flashMessage := htmlDoc.Find(`.flash-message`).Text()
+ assert.Contains(t, flashMessage, "The username cannot be claimed, because its cooldown period is not yet over. It can be claimed on")
+ })
+
+ t.Run("Rename another user", func(t *testing.T) {
+ defer tests.PrintCurrentTest(t)()
+
+ session := loginUser(t, "user4")
+ req := NewRequestWithValues(t, "POST", "/user/settings", map[string]string{
+ "_csrf": GetCSRF(t, session, "/user/settings"),
+ "name": "user2",
+ })
+ session.MakeRequest(t, req, http.StatusSeeOther)
+
+ flashCookie := session.GetCookie(forgejo_context.CookieNameFlash)
+ assert.NotNil(t, flashCookie)
+ assert.Contains(t, flashCookie.Value, "error%3DThe%2Busername%2Bcannot%2Bbe%2Bclaimed%252C%2Bbecause%2Bits%2Bcooldown%2Bperiod%2Bis%2Bnot%2Byet%2Bover.%2BIt%2Bcan%2Bbe%2Bclaimed%2Bon")
+ })
+
+ t.Run("Admin rename user", func(t *testing.T) {
+ defer tests.PrintCurrentTest(t)()
+
+ session := loginUser(t, "user1")
+ req := NewRequestWithValues(t, "POST", "/admin/users/4/edit", map[string]string{
+ "_csrf": GetCSRF(t, session, "/admin/users/4/edit"),
+ "user_name": "user2",
+ "email": "user4@example.com",
+ "login_type": "0-0",
+ })
+ session.MakeRequest(t, req, http.StatusSeeOther)
+ flashCookie := session.GetCookie(forgejo_context.CookieNameFlash)
+ assert.NotNil(t, flashCookie)
+ assert.EqualValues(t, "success%3DThe%2Buser%2Baccount%2Bhas%2Bbeen%2Bupdated.", flashCookie.Value)
+
+ unittest.AssertExistsIf(t, true, &user_model.User{LowerName: "user2"})
+ unittest.AssertExistsIf(t, false, &user_model.Redirect{LowerName: "user2", RedirectUserID: 2})
+ })
+
+ t.Run("Reclaim username", func(t *testing.T) {
+ defer tests.PrintCurrentTest(t)()
+
+ req := NewRequestWithValues(t, "POST", "/user/settings", map[string]string{
+ "_csrf": GetCSRF(t, session, "/user/settings"),
+ "name": "user2-new-2",
+ })
+ session.MakeRequest(t, req, http.StatusSeeOther)
+
+ flashCookie := session.GetCookie(forgejo_context.CookieNameFlash)
+ assert.NotNil(t, flashCookie)
+ assert.EqualValues(t, "success%3DYour%2Bprofile%2Bhas%2Bbeen%2Bupdated.", flashCookie.Value)
+
+ unittest.AssertExistsIf(t, true, &user_model.Redirect{LowerName: "user2-new", RedirectUserID: 2})
+
+ req = NewRequestWithValues(t, "POST", "/user/settings", map[string]string{
+ "_csrf": GetCSRF(t, session, "/user/settings"),
+ "name": "user2-new",
+ })
+ session.MakeRequest(t, req, http.StatusSeeOther)
+
+ flashCookie = session.GetCookie(forgejo_context.CookieNameFlash)
+ assert.NotNil(t, flashCookie)
+ assert.EqualValues(t, "success%3DYour%2Bprofile%2Bhas%2Bbeen%2Bupdated.", flashCookie.Value)
+
+ unittest.AssertExistsIf(t, false, &user_model.Redirect{LowerName: "user2-new", RedirectUserID: 2})
+ unittest.AssertExistsIf(t, true, &user_model.Redirect{LowerName: "user2-new-2", RedirectUserID: 2})
+ })
+
+ t.Run("Profile note", func(t *testing.T) {
+ getPrompt := func(t *testing.T) string {
+ req := NewRequest(t, "GET", "/user/settings")
+ resp := session.MakeRequest(t, req, http.StatusOK)
+ htmlDoc := NewHTMLParser(t, resp.Body)
+
+ return htmlDoc.Find("input[name='name'] + .help").Text()
+ }
+
+ t.Run("No cooldown", func(t *testing.T) {
+ defer test.MockVariableValue(&setting.Service.UsernameCooldownPeriod, 0)()
+ defer tests.PrintCurrentTest(t)()
+
+ assert.Contains(t, getPrompt(t), "The old username will redirect until someone claims it.")
+ })
+
+ t.Run("With cooldown", func(t *testing.T) {
+ defer test.MockVariableValue(&setting.Service.UsernameCooldownPeriod, 8)()
+ defer tests.PrintCurrentTest(t)()
+
+ assert.Contains(t, getPrompt(t), "The old username will be available to everyone after a cooldown period of 8 days, you can still reclaim the old username during the cooldown period.")
+ })
+ })
+
+ t.Run("Org settings note", func(t *testing.T) {
+ getPrompt := func(t *testing.T) string {
+ req := NewRequest(t, "GET", "/org/org3/settings")
+ resp := session.MakeRequest(t, req, http.StatusOK)
+ htmlDoc := NewHTMLParser(t, resp.Body)
+
+ return htmlDoc.Find("#org_name + .help").Text()
+ }
+
+ t.Run("No cooldown", func(t *testing.T) {
+ defer test.MockVariableValue(&setting.Service.UsernameCooldownPeriod, 0)()
+ defer tests.PrintCurrentTest(t)()
+
+ assert.Contains(t, getPrompt(t), "The old name will redirect until it is claimed.")
+ })
+
+ t.Run("With cooldown", func(t *testing.T) {
+ defer test.MockVariableValue(&setting.Service.UsernameCooldownPeriod, 8)()
+ defer tests.PrintCurrentTest(t)()
+
+ assert.Contains(t, getPrompt(t), "The old username will be available to everyone after a cooldown period of 8 days, you can still reclaim the old username during the cooldown period.")
+ })
+ })
+}
+
+// NOTE: This is a unit test but written in the integration test to ensure this runs on all databases.
+func TestLimitUserRedirects(t *testing.T) {
+ defer tests.PrepareTestEnv(t)()
+
+ _, err := db.GetEngine(db.DefaultContext).NoAutoTime().Insert(&user_model.Redirect{RedirectUserID: 1, LowerName: "legacy", CreatedUnix: 0},
+ &user_model.Redirect{RedirectUserID: 1, LowerName: "past", CreatedUnix: timeutil.TimeStampNow().AddDuration(-48 * time.Hour)},
+ &user_model.Redirect{RedirectUserID: 1, LowerName: "recent", CreatedUnix: timeutil.TimeStampNow().AddDuration(-12 * time.Hour)},
+ &user_model.Redirect{RedirectUserID: 1, LowerName: "future", CreatedUnix: timeutil.TimeStampNow().AddDuration(time.Hour)})
+ require.NoError(t, err)
+
+ require.NoError(t, user_model.LimitUserRedirects(db.DefaultContext, 1, 3))
+
+ unittest.AssertExistsIf(t, false, &user_model.Redirect{LowerName: "legacy"})
+ unittest.AssertExistsIf(t, true, &user_model.Redirect{LowerName: "past"})
+ unittest.AssertExistsIf(t, true, &user_model.Redirect{LowerName: "recent"})
+ unittest.AssertExistsIf(t, true, &user_model.Redirect{LowerName: "future"})
+
+ require.NoError(t, user_model.LimitUserRedirects(db.DefaultContext, 1, 1))
+
+ unittest.AssertExistsIf(t, false, &user_model.Redirect{LowerName: "legacy"})
+ unittest.AssertExistsIf(t, false, &user_model.Redirect{LowerName: "past"})
+ unittest.AssertExistsIf(t, false, &user_model.Redirect{LowerName: "recent"})
+ unittest.AssertExistsIf(t, true, &user_model.Redirect{LowerName: "future"})
+}
+
+// NOTE: This is a unit test but written in the integration test to ensure this runs on all databases.
+func TestCanClaimUsername(t *testing.T) {
+ require.NoError(t, unittest.PrepareTestDatabase())
+
+ _, err := db.GetEngine(db.DefaultContext).NoAutoTime().Insert(&user_model.Redirect{RedirectUserID: 1, LowerName: "legacy", CreatedUnix: 0},
+ &user_model.Redirect{RedirectUserID: 1, LowerName: "past", CreatedUnix: timeutil.TimeStampNow().AddDuration(-48 * time.Hour)},
+ &user_model.Redirect{RedirectUserID: 1, LowerName: "recent", CreatedUnix: timeutil.TimeStampNow().AddDuration(-12 * time.Hour)},
+ &user_model.Redirect{RedirectUserID: 1, LowerName: "future", CreatedUnix: timeutil.TimeStampNow().AddDuration(time.Hour)},
+ &user_model.Redirect{RedirectUserID: 3, LowerName: "recent-org", CreatedUnix: timeutil.TimeStampNow().AddDuration(-12 * time.Hour)})
+ require.NoError(t, err)
+
+ testCase := func(t *testing.T, legacy, past, recent, future bool, doerID int64) {
+ t.Helper()
+
+ ok, _, err := user_model.CanClaimUsername(db.DefaultContext, "legacy", doerID)
+ require.NoError(t, err)
+ assert.Equal(t, legacy, ok)
+
+ ok, _, err = user_model.CanClaimUsername(db.DefaultContext, "past", doerID)
+ require.NoError(t, err)
+ assert.Equal(t, past, ok)
+
+ ok, _, err = user_model.CanClaimUsername(db.DefaultContext, "recent", doerID)
+ require.NoError(t, err)
+ assert.Equal(t, recent, ok)
+
+ ok, _, err = user_model.CanClaimUsername(db.DefaultContext, "future", doerID)
+ require.NoError(t, err)
+ assert.Equal(t, future, ok)
+ }
+
+ t.Run("No cooldown", func(t *testing.T) {
+ defer test.MockVariableValue(&setting.Service.UsernameCooldownPeriod, 0)()
+
+ testCase(t, true, true, true, true, -1)
+ })
+
+ t.Run("1 day cooldown", func(t *testing.T) {
+ defer tests.PrintCurrentTest(t)()
+ defer test.MockVariableValue(&setting.Service.UsernameCooldownPeriod, 1)()
+
+ testCase(t, true, true, false, false, -1)
+ })
+
+ t.Run("1 week cooldown", func(t *testing.T) {
+ defer tests.PrintCurrentTest(t)()
+ defer test.MockVariableValue(&setting.Service.UsernameCooldownPeriod, 7)()
+
+ testCase(t, true, false, false, false, -1)
+
+ t.Run("Own username", func(t *testing.T) {
+ defer tests.PrintCurrentTest(t)()
+ defer test.MockVariableValue(&setting.Service.UsernameCooldownPeriod, 7)()
+
+ testCase(t, true, true, true, true, 1)
+ })
+ })
+
+ t.Run("Organisation", func(t *testing.T) {
+ defer test.MockVariableValue(&setting.Service.UsernameCooldownPeriod, 1)()
+
+ t.Run("Not owner", func(t *testing.T) {
+ defer tests.PrintCurrentTest(t)()
+ ok, _, err := user_model.CanClaimUsername(db.DefaultContext, "recent-org", -1)
+ require.NoError(t, err)
+ assert.False(t, ok)
+ })
+ t.Run("Owner", func(t *testing.T) {
+ defer tests.PrintCurrentTest(t)()
+ ok, _, err := user_model.CanClaimUsername(db.DefaultContext, "recent-org", 2)
+ require.NoError(t, err)
+ assert.True(t, ok)
+ })
+ })
+}