Improve valid user name check (#20136)

Close https://github.com/go-gitea/gitea/issues/21640

Before: Gitea can create users like ".xxx" or "x..y", which is not
ideal, it's already a consensus that dot filenames have special
meanings, and `a..b` is a confusing name when doing cross repo compare.

After: stricter

Co-authored-by: Jason Song <i@wolfogre.com>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: delvh <dev.lh@web.de>
This commit is contained in:
wxiaoguang 2022-11-04 17:04:08 +08:00 committed by GitHub
parent 4c6b4a67d9
commit 2900dc90a7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 95 additions and 14 deletions

View file

@ -29,6 +29,7 @@ import (
"code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/timeutil"
"code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/modules/validation"
"golang.org/x/crypto/argon2" "golang.org/x/crypto/argon2"
"golang.org/x/crypto/bcrypt" "golang.org/x/crypto/bcrypt"
@ -621,7 +622,7 @@ var (
// IsUsableUsername returns an error when a username is reserved // IsUsableUsername returns an error when a username is reserved
func IsUsableUsername(name string) error { func IsUsableUsername(name string) error {
// Validate username make sure it satisfies requirement. // Validate username make sure it satisfies requirement.
if db.AlphaDashDotPattern.MatchString(name) { if !validation.IsValidUsername(name) {
// Note: usually this error is normally caught up earlier in the UI // Note: usually this error is normally caught up earlier in the UI
return db.ErrNameCharsNotAllowed{Name: name} return db.ErrNameCharsNotAllowed{Name: name}
} }

View file

@ -10,7 +10,7 @@ type CreateUserOption struct {
SourceID int64 `json:"source_id"` SourceID int64 `json:"source_id"`
LoginName string `json:"login_name"` LoginName string `json:"login_name"`
// required: true // required: true
Username string `json:"username" binding:"Required;AlphaDashDot;MaxSize(40)"` Username string `json:"username" binding:"Required;Username;MaxSize(40)"`
FullName string `json:"full_name" binding:"MaxSize(100)"` FullName string `json:"full_name" binding:"MaxSize(100)"`
// required: true // required: true
// swagger:strfmt email // swagger:strfmt email

View file

@ -24,6 +24,9 @@ const (
// ErrRegexPattern is returned when a regex pattern is invalid // ErrRegexPattern is returned when a regex pattern is invalid
ErrRegexPattern = "RegexPattern" ErrRegexPattern = "RegexPattern"
// ErrUsername is username error
ErrUsername = "UsernameError"
) )
// AddBindingRules adds additional binding rules // AddBindingRules adds additional binding rules
@ -34,6 +37,7 @@ func AddBindingRules() {
addGlobPatternRule() addGlobPatternRule()
addRegexPatternRule() addRegexPatternRule()
addGlobOrRegexPatternRule() addGlobOrRegexPatternRule()
addUsernamePatternRule()
} }
func addGitRefNameBindingRule() { func addGitRefNameBindingRule() {
@ -148,6 +152,22 @@ func addGlobOrRegexPatternRule() {
}) })
} }
func addUsernamePatternRule() {
binding.AddRule(&binding.Rule{
IsMatch: func(rule string) bool {
return rule == "Username"
},
IsValid: func(errs binding.Errors, name string, val interface{}) (bool, binding.Errors) {
str := fmt.Sprintf("%v", val)
if !IsValidUsername(str) {
errs.Add([]string{name}, ErrUsername, "invalid username")
return false, errs
}
return true, errs
},
})
}
func portOnly(hostport string) string { func portOnly(hostport string) string {
colon := strings.IndexByte(hostport, ':') colon := strings.IndexByte(hostport, ':')
if colon == -1 { if colon == -1 {

View file

@ -91,3 +91,15 @@ func IsValidExternalTrackerURLFormat(uri string) bool {
return true return true
} }
var (
validUsernamePattern = regexp.MustCompile(`^[\da-zA-Z][-.\w]*$`)
invalidUsernamePattern = regexp.MustCompile(`[-._]{2,}|[-._]$`) // No consecutive or trailing non-alphanumeric chars
)
// IsValidUsername checks if username is valid
func IsValidUsername(name string) bool {
// It is difficult to find a single pattern that is both readable and effective,
// but it's easier to use positive and negative checks.
return validUsernamePattern.MatchString(name) && !invalidUsernamePattern.MatchString(name)
}

View file

@ -155,3 +155,34 @@ func Test_IsValidExternalTrackerURLFormat(t *testing.T) {
}) })
} }
} }
func TestIsValidUsername(t *testing.T) {
tests := []struct {
arg string
want bool
}{
{arg: "a", want: true},
{arg: "abc", want: true},
{arg: "0.b-c", want: true},
{arg: "a.b-c_d", want: true},
{arg: "", want: false},
{arg: ".abc", want: false},
{arg: "abc.", want: false},
{arg: "a..bc", want: false},
{arg: "a...bc", want: false},
{arg: "a.-bc", want: false},
{arg: "a._bc", want: false},
{arg: "a_-bc", want: false},
{arg: "a/bc", want: false},
{arg: "☁️", want: false},
{arg: "-", want: false},
{arg: "--diff", want: false},
{arg: "-im-here", want: false},
{arg: "a space", want: false},
}
for _, tt := range tests {
t.Run(tt.arg, func(t *testing.T) {
assert.Equalf(t, tt.want, IsValidUsername(tt.arg), "IsValidUsername(%v)", tt.arg)
})
}
}

View file

@ -135,6 +135,8 @@ func Validate(errs binding.Errors, data map[string]interface{}, f Form, l transl
data["ErrorMsg"] = trName + l.Tr("form.glob_pattern_error", errs[0].Message) data["ErrorMsg"] = trName + l.Tr("form.glob_pattern_error", errs[0].Message)
case validation.ErrRegexPattern: case validation.ErrRegexPattern:
data["ErrorMsg"] = trName + l.Tr("form.regex_pattern_error", errs[0].Message) data["ErrorMsg"] = trName + l.Tr("form.regex_pattern_error", errs[0].Message)
case validation.ErrUsername:
data["ErrorMsg"] = trName + l.Tr("form.username_error")
default: default:
msg := errs[0].Classification msg := errs[0].Classification
if msg != "" && errs[0].Message != "" { if msg != "" && errs[0].Message != "" {

View file

@ -463,6 +463,7 @@ url_error = `'%s' is not a valid URL.`
include_error = ` must contain substring '%s'.` include_error = ` must contain substring '%s'.`
glob_pattern_error = ` glob pattern is invalid: %s.` glob_pattern_error = ` glob pattern is invalid: %s.`
regex_pattern_error = ` regex pattern is invalid: %s.` regex_pattern_error = ` regex pattern is invalid: %s.`
username_error = ` can only contain alphanumeric chars ('0-9','a-z','A-Z'), dash ('-'), underscore ('_') and dot ('.'). It cannot begin or end with non-alphanumeric chars, and consecutive non-alphanumeric chars are also forbidden.`
unknown_error = Unknown error: unknown_error = Unknown error:
captcha_incorrect = The CAPTCHA code is incorrect. captcha_incorrect = The CAPTCHA code is incorrect.
password_not_match = The passwords do not match. password_not_match = The passwords do not match.

View file

@ -18,7 +18,7 @@ import (
type AdminCreateUserForm struct { type AdminCreateUserForm struct {
LoginType string `binding:"Required"` LoginType string `binding:"Required"`
LoginName string LoginName string
UserName string `binding:"Required;AlphaDashDot;MaxSize(40)"` UserName string `binding:"Required;Username;MaxSize(40)"`
Email string `binding:"Required;Email;MaxSize(254)"` Email string `binding:"Required;Email;MaxSize(254)"`
Password string `binding:"MaxSize(255)"` Password string `binding:"MaxSize(255)"`
SendNotify bool SendNotify bool
@ -35,7 +35,7 @@ func (f *AdminCreateUserForm) Validate(req *http.Request, errs binding.Errors) b
// AdminEditUserForm form for admin to create user // AdminEditUserForm form for admin to create user
type AdminEditUserForm struct { type AdminEditUserForm struct {
LoginType string `binding:"Required"` LoginType string `binding:"Required"`
UserName string `binding:"AlphaDashDot;MaxSize(40)"` UserName string `binding:"Username;MaxSize(40)"`
LoginName string LoginName string
FullName string `binding:"MaxSize(100)"` FullName string `binding:"MaxSize(100)"`
Email string `binding:"Required;Email;MaxSize(254)"` Email string `binding:"Required;Email;MaxSize(254)"`

View file

@ -24,7 +24,7 @@ import (
// CreateOrgForm form for creating organization // CreateOrgForm form for creating organization
type CreateOrgForm struct { type CreateOrgForm struct {
OrgName string `binding:"Required;AlphaDashDot;MaxSize(40)" locale:"org.org_name_holder"` OrgName string `binding:"Required;Username;MaxSize(40)" locale:"org.org_name_holder"`
Visibility structs.VisibleType Visibility structs.VisibleType
RepoAdminChangeTeamAccess bool RepoAdminChangeTeamAccess bool
} }
@ -37,7 +37,7 @@ func (f *CreateOrgForm) Validate(req *http.Request, errs binding.Errors) binding
// UpdateOrgSettingForm form for updating organization settings // UpdateOrgSettingForm form for updating organization settings
type UpdateOrgSettingForm struct { type UpdateOrgSettingForm struct {
Name string `binding:"Required;AlphaDashDot;MaxSize(40)" locale:"org.org_name_holder"` Name string `binding:"Required;Username;MaxSize(40)" locale:"org.org_name_holder"`
FullName string `binding:"MaxSize(100)"` FullName string `binding:"MaxSize(100)"`
Description string `binding:"MaxSize(255)"` Description string `binding:"MaxSize(255)"`
Website string `binding:"ValidUrl;MaxSize(255)"` Website string `binding:"ValidUrl;MaxSize(255)"`

View file

@ -65,7 +65,7 @@ type InstallForm struct {
PasswordAlgorithm string PasswordAlgorithm string
AdminName string `binding:"OmitEmpty;AlphaDashDot;MaxSize(30)" locale:"install.admin_name"` AdminName string `binding:"OmitEmpty;Username;MaxSize(30)" locale:"install.admin_name"`
AdminPasswd string `binding:"OmitEmpty;MaxSize(255)" locale:"install.admin_password"` AdminPasswd string `binding:"OmitEmpty;MaxSize(255)" locale:"install.admin_password"`
AdminConfirmPasswd string AdminConfirmPasswd string
AdminEmail string `binding:"OmitEmpty;MinSize(3);MaxSize(254);Include(@)" locale:"install.admin_email"` AdminEmail string `binding:"OmitEmpty;MinSize(3);MaxSize(254);Include(@)" locale:"install.admin_email"`
@ -91,7 +91,7 @@ func (f *InstallForm) Validate(req *http.Request, errs binding.Errors) binding.E
// RegisterForm form for registering // RegisterForm form for registering
type RegisterForm struct { type RegisterForm struct {
UserName string `binding:"Required;AlphaDashDot;MaxSize(40)"` UserName string `binding:"Required;Username;MaxSize(40)"`
Email string `binding:"Required;MaxSize(254)"` Email string `binding:"Required;MaxSize(254)"`
Password string `binding:"MaxSize(255)"` Password string `binding:"MaxSize(255)"`
Retype string Retype string
@ -243,7 +243,7 @@ func (f *IntrospectTokenForm) Validate(req *http.Request, errs binding.Errors) b
// UpdateProfileForm form for updating profile // UpdateProfileForm form for updating profile
type UpdateProfileForm struct { type UpdateProfileForm struct {
Name string `binding:"AlphaDashDot;MaxSize(40)"` Name string `binding:"Username;MaxSize(40)"`
FullName string `binding:"MaxSize(100)"` FullName string `binding:"MaxSize(100)"`
KeepEmailPrivate bool KeepEmailPrivate bool
Website string `binding:"ValidSiteUrl;MaxSize(255)"` Website string `binding:"ValidSiteUrl;MaxSize(255)"`

View file

@ -27,7 +27,7 @@ func (f *SignInOpenIDForm) Validate(req *http.Request, errs binding.Errors) bind
// SignUpOpenIDForm form for signin up with OpenID // SignUpOpenIDForm form for signin up with OpenID
type SignUpOpenIDForm struct { type SignUpOpenIDForm struct {
UserName string `binding:"Required;AlphaDashDot;MaxSize(40)"` UserName string `binding:"Required;Username;MaxSize(40)"`
Email string `binding:"Required;Email;MaxSize(254)"` Email string `binding:"Required;Email;MaxSize(254)"`
GRecaptchaResponse string `form:"g-recaptcha-response"` GRecaptchaResponse string `form:"g-recaptcha-response"`
HcaptchaResponse string `form:"h-captcha-response"` HcaptchaResponse string `form:"h-captcha-response"`

View file

@ -53,6 +53,22 @@ func TestRenameInvalidUsername(t *testing.T) {
"%00", "%00",
"thisHas ASpace", "thisHas ASpace",
"p<A>tho>lo<gical", "p<A>tho>lo<gical",
".",
"..",
".well-known",
".abc",
"abc.",
"a..bc",
"a...bc",
"a.-bc",
"a._bc",
"a_-bc",
"a/bc",
"☁️",
"-",
"--diff",
"-im-here",
"a space",
} }
session := loginUser(t, "user2") session := loginUser(t, "user2")
@ -68,7 +84,7 @@ func TestRenameInvalidUsername(t *testing.T) {
htmlDoc := NewHTMLParser(t, resp.Body) htmlDoc := NewHTMLParser(t, resp.Body)
assert.Contains(t, assert.Contains(t,
htmlDoc.doc.Find(".ui.negative.message").Text(), htmlDoc.doc.Find(".ui.negative.message").Text(),
translation.NewLocale("en-US").Tr("form.alpha_dash_dot_error"), translation.NewLocale("en-US").Tr("form.username_error"),
) )
unittest.AssertNotExistsBean(t, &user_model.User{Name: invalidUsername}) unittest.AssertNotExistsBean(t, &user_model.User{Name: invalidUsername})
@ -79,9 +95,7 @@ func TestRenameReservedUsername(t *testing.T) {
defer tests.PrepareTestEnv(t)() defer tests.PrepareTestEnv(t)()
reservedUsernames := []string{ reservedUsernames := []string{
".", // ".", "..", ".well-known", // The names are not only reserved but also invalid
"..",
".well-known",
"admin", "admin",
"api", "api",
"assets", "assets",