From 0f5c5eeb6b5aa7b25f622b5ecbef5e2b4e9cb5eb Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 26 Jun 2022 07:49:56 +0800 Subject: [PATCH 01/17] Improve valid user name check --- models/db/name.go | 10 +++++++--- models/db/name_test.go | 14 ++++++++++++++ models/repo/repo.go | 2 +- models/user/user.go | 2 +- 4 files changed, 23 insertions(+), 5 deletions(-) create mode 100644 models/db/name_test.go diff --git a/models/db/name.go b/models/db/name.go index 9c9d18f184748..0b46ec075e6cf 100644 --- a/models/db/name.go +++ b/models/db/name.go @@ -14,10 +14,9 @@ import ( var ( // ErrNameEmpty name is empty error - ErrNameEmpty = errors.New("Name is empty") + ErrNameEmpty = errors.New("name is empty") - // AlphaDashDotPattern characters prohibited in a user name (anything except A-Za-z0-9_.-) - AlphaDashDotPattern = regexp.MustCompile(`[^\w-\.]`) + validUsernamePattern = regexp.MustCompile(`^[\da-zA-Z][-.\w]+$`) ) // ErrNameReserved represents a "reserved name" error. @@ -89,3 +88,8 @@ func IsUsableName(names, patterns []string, name string) error { return nil } + +// IsUsableName checks if name is valid +func IsValidUsername(name string) bool { + return validUsernamePattern.MatchString(name) +} diff --git a/models/db/name_test.go b/models/db/name_test.go new file mode 100644 index 0000000000000..ceb42893fa8e7 --- /dev/null +++ b/models/db/name_test.go @@ -0,0 +1,14 @@ +package db + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestIsValidUsername(t *testing.T) { + assert.True(t, IsValidUsername("abc")) + assert.True(t, IsValidUsername("0.b-c")) + assert.False(t, IsValidUsername(".abc")) + assert.False(t, IsValidUsername("a/bc")) +} diff --git a/models/repo/repo.go b/models/repo/repo.go index bb2a7468ff42a..6903cb8e42e7b 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -48,7 +48,7 @@ var ( // IsUsableRepoName returns true when repository is usable func IsUsableRepoName(name string) error { - if db.AlphaDashDotPattern.MatchString(name) { + if db.IsValidUsername(name) { // Note: usually this error is normally caught up earlier in the UI return db.ErrNameCharsNotAllowed{Name: name} } diff --git a/models/user/user.go b/models/user/user.go index f7d457b91b5a5..7663de2d7fb91 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -608,7 +608,7 @@ var ( // IsUsableUsername returns an error when a username is reserved func IsUsableUsername(name string) error { // Validate username make sure it satisfies requirement. - if db.AlphaDashDotPattern.MatchString(name) { + if db.IsValidUsername(name) { // Note: usually this error is normally caught up earlier in the UI return db.ErrNameCharsNotAllowed{Name: name} } From 70fe59ac9de2ef2fd61f126046497ef727e3e520 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 26 Jun 2022 19:00:29 +0800 Subject: [PATCH 02/17] fix lint --- models/db/name.go | 2 +- models/db/name_test.go | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/models/db/name.go b/models/db/name.go index 0b46ec075e6cf..a0212d6330409 100644 --- a/models/db/name.go +++ b/models/db/name.go @@ -89,7 +89,7 @@ func IsUsableName(names, patterns []string, name string) error { return nil } -// IsUsableName checks if name is valid +// IsValidUsername checks if name is valid func IsValidUsername(name string) bool { return validUsernamePattern.MatchString(name) } diff --git a/models/db/name_test.go b/models/db/name_test.go index ceb42893fa8e7..67fae0cc6120b 100644 --- a/models/db/name_test.go +++ b/models/db/name_test.go @@ -1,3 +1,7 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file.package db + package db import ( @@ -9,6 +13,7 @@ import ( func TestIsValidUsername(t *testing.T) { assert.True(t, IsValidUsername("abc")) assert.True(t, IsValidUsername("0.b-c")) + assert.False(t, IsValidUsername(".abc")) assert.False(t, IsValidUsername("a/bc")) } From 54c673b77b0f6254f2e5b5c1a437bd1cfcaf802a Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 27 Jun 2022 10:03:16 +0800 Subject: [PATCH 03/17] move valid username regexp to user model, fix tests --- models/db/name.go | 8 -------- models/repo/repo.go | 2 +- models/user/name.go | 14 ++++++++++++++ models/{db => user}/name_test.go | 5 ++++- models/user/user.go | 2 +- 5 files changed, 20 insertions(+), 11 deletions(-) create mode 100644 models/user/name.go rename models/{db => user}/name_test.go (77%) diff --git a/models/db/name.go b/models/db/name.go index a0212d6330409..09073a84730d4 100644 --- a/models/db/name.go +++ b/models/db/name.go @@ -7,7 +7,6 @@ package db import ( "errors" "fmt" - "regexp" "strings" "unicode/utf8" ) @@ -15,8 +14,6 @@ import ( var ( // ErrNameEmpty name is empty error ErrNameEmpty = errors.New("name is empty") - - validUsernamePattern = regexp.MustCompile(`^[\da-zA-Z][-.\w]+$`) ) // ErrNameReserved represents a "reserved name" error. @@ -88,8 +85,3 @@ func IsUsableName(names, patterns []string, name string) error { return nil } - -// IsValidUsername checks if name is valid -func IsValidUsername(name string) bool { - return validUsernamePattern.MatchString(name) -} diff --git a/models/repo/repo.go b/models/repo/repo.go index 6903cb8e42e7b..d395633e35bb2 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -48,7 +48,7 @@ var ( // IsUsableRepoName returns true when repository is usable func IsUsableRepoName(name string) error { - if db.IsValidUsername(name) { + if user_model.IsValidUsername(name) { // Note: usually this error is normally caught up earlier in the UI return db.ErrNameCharsNotAllowed{Name: name} } diff --git a/models/user/name.go b/models/user/name.go new file mode 100644 index 0000000000000..3d96c9bff3c52 --- /dev/null +++ b/models/user/name.go @@ -0,0 +1,14 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file.package db + +package user + +import "regexp" + +var validUsernamePattern = regexp.MustCompile(`^[\da-zA-Z][-.\w]*$`) + +// IsValidUsername checks if name is valid +func IsValidUsername(name string) bool { + return validUsernamePattern.MatchString(name) +} diff --git a/models/db/name_test.go b/models/user/name_test.go similarity index 77% rename from models/db/name_test.go rename to models/user/name_test.go index 67fae0cc6120b..6bc51d120f0da 100644 --- a/models/db/name_test.go +++ b/models/user/name_test.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file.package db -package db +package user import ( "testing" @@ -11,9 +11,12 @@ import ( ) func TestIsValidUsername(t *testing.T) { + assert.True(t, IsValidUsername("a")) assert.True(t, IsValidUsername("abc")) assert.True(t, IsValidUsername("0.b-c")) + assert.False(t, IsValidUsername("")) assert.False(t, IsValidUsername(".abc")) assert.False(t, IsValidUsername("a/bc")) + assert.False(t, IsValidUsername("☁️")) } diff --git a/models/user/user.go b/models/user/user.go index 5eb4487200c09..f66b907dcd48c 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -608,7 +608,7 @@ var ( // IsUsableUsername returns an error when a username is reserved func IsUsableUsername(name string) error { // Validate username make sure it satisfies requirement. - if db.IsValidUsername(name) { + if IsValidUsername(name) { // Note: usually this error is normally caught up earlier in the UI return db.ErrNameCharsNotAllowed{Name: name} } From 4b624c7e51f5212688424f838db7906a3f371bef Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 27 Jun 2022 10:14:33 +0800 Subject: [PATCH 04/17] fix fmt --- models/db/name.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/models/db/name.go b/models/db/name.go index 09073a84730d4..5009c369f0b13 100644 --- a/models/db/name.go +++ b/models/db/name.go @@ -11,10 +11,8 @@ import ( "unicode/utf8" ) -var ( - // ErrNameEmpty name is empty error - ErrNameEmpty = errors.New("name is empty") -) +// ErrNameEmpty name is empty error +var ErrNameEmpty = errors.New("name is empty") // ErrNameReserved represents a "reserved name" error. type ErrNameReserved struct { From 94d79cecd3d80477dfaa7369f00204debd48d821 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 27 Jun 2022 10:30:21 +0800 Subject: [PATCH 05/17] fix more checks --- cmd/serv.go | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/cmd/serv.go b/cmd/serv.go index b00c3962f41fb..c18ed68d1d579 100644 --- a/cmd/serv.go +++ b/cmd/serv.go @@ -12,7 +12,6 @@ import ( "net/url" "os" "os/exec" - "regexp" "strconv" "strings" "time" @@ -20,6 +19,7 @@ import ( asymkey_model "code.gitea.io/gitea/models/asymkey" git_model "code.gitea.io/gitea/models/git" "code.gitea.io/gitea/models/perm" + user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" @@ -83,15 +83,12 @@ func setup(logPath string, debug bool) { } } -var ( - allowedCommands = map[string]perm.AccessMode{ - "git-upload-pack": perm.AccessModeRead, - "git-upload-archive": perm.AccessModeRead, - "git-receive-pack": perm.AccessModeWrite, - lfsAuthenticateVerb: perm.AccessModeNone, - } - alphaDashDotPattern = regexp.MustCompile(`[^\w-\.]`) -) +var allowedCommands = map[string]perm.AccessMode{ + "git-upload-pack": perm.AccessModeRead, + "git-upload-archive": perm.AccessModeRead, + "git-receive-pack": perm.AccessModeWrite, + lfsAuthenticateVerb: perm.AccessModeNone, +} func fail(userMessage, logMessage string, args ...interface{}) error { // There appears to be a chance to cause a zombie process and failure to read the Exit status @@ -205,7 +202,7 @@ func runServ(c *cli.Context) error { username := strings.ToLower(rr[0]) reponame := strings.ToLower(strings.TrimSuffix(rr[1], ".git")) - if alphaDashDotPattern.MatchString(reponame) { + if !user_model.IsValidUsername(reponame) { return fail("Invalid repo name", "Invalid repo name: %s", reponame) } From b203f62e7a8eee13966c6aa35b8568d301bb6db6 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Tue, 1 Nov 2022 11:24:07 +0800 Subject: [PATCH 06/17] feat: update username check --- models/user/name.go | 13 ++++++++++--- models/user/name_test.go | 7 +++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/models/user/name.go b/models/user/name.go index 3d96c9bff3c52..be2498cc76407 100644 --- a/models/user/name.go +++ b/models/user/name.go @@ -4,11 +4,18 @@ package user -import "regexp" +import ( + "regexp" +) -var validUsernamePattern = regexp.MustCompile(`^[\da-zA-Z][-.\w]*$`) +var ( + validUsernamePattern = regexp.MustCompile(`^[\da-zA-Z][-.\w]*$`) + invalidUsernamePattern = regexp.MustCompile(`[-._]{2,}|[-._]$`) +) // IsValidUsername checks if name is valid func IsValidUsername(name string) bool { - return validUsernamePattern.MatchString(name) + // 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) } diff --git a/models/user/name_test.go b/models/user/name_test.go index 6bc51d120f0da..7c1d5a7d62d3c 100644 --- a/models/user/name_test.go +++ b/models/user/name_test.go @@ -14,9 +14,16 @@ func TestIsValidUsername(t *testing.T) { assert.True(t, IsValidUsername("a")) assert.True(t, IsValidUsername("abc")) assert.True(t, IsValidUsername("0.b-c")) + assert.True(t, IsValidUsername("a.b-c_d")) assert.False(t, IsValidUsername("")) assert.False(t, IsValidUsername(".abc")) + assert.False(t, IsValidUsername("abc.")) + assert.False(t, IsValidUsername("a..bc")) + assert.False(t, IsValidUsername("a...bc")) + assert.False(t, IsValidUsername("a.-bc")) + assert.False(t, IsValidUsername("a._bc")) + assert.False(t, IsValidUsername("a_-bc")) assert.False(t, IsValidUsername("a/bc")) assert.False(t, IsValidUsername("☁️")) } From 138a2b56234e8cfc5572a83ebc85aa8591f169b2 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Tue, 1 Nov 2022 11:38:42 +0800 Subject: [PATCH 07/17] chore: use old rule for repo name --- cmd/serv.go | 19 +++++++++++-------- models/repo/repo.go | 2 +- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/cmd/serv.go b/cmd/serv.go index 695746b06fd21..06561f348a43e 100644 --- a/cmd/serv.go +++ b/cmd/serv.go @@ -12,6 +12,7 @@ import ( "net/url" "os" "os/exec" + "regexp" "strconv" "strings" "time" @@ -19,7 +20,6 @@ import ( asymkey_model "code.gitea.io/gitea/models/asymkey" git_model "code.gitea.io/gitea/models/git" "code.gitea.io/gitea/models/perm" - user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" @@ -83,12 +83,15 @@ func setup(logPath string, debug bool) { } } -var allowedCommands = map[string]perm.AccessMode{ - "git-upload-pack": perm.AccessModeRead, - "git-upload-archive": perm.AccessModeRead, - "git-receive-pack": perm.AccessModeWrite, - lfsAuthenticateVerb: perm.AccessModeNone, -} +var ( + allowedCommands = map[string]perm.AccessMode{ + "git-upload-pack": perm.AccessModeRead, + "git-upload-archive": perm.AccessModeRead, + "git-receive-pack": perm.AccessModeWrite, + lfsAuthenticateVerb: perm.AccessModeNone, + } + alphaDashDotPattern = regexp.MustCompile(`[^\w-\.]`) +) func fail(userMessage, logMessage string, args ...interface{}) error { // There appears to be a chance to cause a zombie process and failure to read the Exit status @@ -202,7 +205,7 @@ func runServ(c *cli.Context) error { username := strings.ToLower(rr[0]) reponame := strings.ToLower(strings.TrimSuffix(rr[1], ".git")) - if !user_model.IsValidUsername(reponame) { + if alphaDashDotPattern.MatchString(reponame) { return fail("Invalid repo name", "Invalid repo name: %s", reponame) } diff --git a/models/repo/repo.go b/models/repo/repo.go index 7c26eccbc07e6..77e0367a5a5c7 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -54,7 +54,7 @@ var ( // IsUsableRepoName returns true when repository is usable func IsUsableRepoName(name string) error { - if user_model.IsValidUsername(name) { + if db.AlphaDashDotPattern.MatchString(name) { // Note: usually this error is normally caught up earlier in the UI return db.ErrNameCharsNotAllowed{Name: name} } From d2f2ba1dcc0d2548146bb5d5371eaedfe2ef51dd Mon Sep 17 00:00:00 2001 From: Jason Song Date: Tue, 1 Nov 2022 14:33:11 +0800 Subject: [PATCH 08/17] feat: add new validation rule --- models/user/name.go | 21 ------------------ models/user/name_test.go | 29 ------------------------- models/user/user.go | 3 ++- modules/structs/admin_user.go | 2 +- modules/validation/binding.go | 20 +++++++++++++++++ modules/validation/helpers.go | 12 ++++++++++ modules/validation/helpers_test.go | 27 +++++++++++++++++++++++ modules/web/middleware/binding.go | 2 ++ options/locale/locale_en-US.ini | 1 + services/forms/admin.go | 4 ++-- services/forms/user_form.go | 2 +- services/forms/user_form_auth_openid.go | 2 +- 12 files changed, 69 insertions(+), 56 deletions(-) delete mode 100644 models/user/name.go delete mode 100644 models/user/name_test.go diff --git a/models/user/name.go b/models/user/name.go deleted file mode 100644 index be2498cc76407..0000000000000 --- a/models/user/name.go +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. -// Use of this source code is governed by a MIT-style -// license that can be found in the LICENSE file.package db - -package user - -import ( - "regexp" -) - -var ( - validUsernamePattern = regexp.MustCompile(`^[\da-zA-Z][-.\w]*$`) - invalidUsernamePattern = regexp.MustCompile(`[-._]{2,}|[-._]$`) -) - -// IsValidUsername checks if name 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) -} diff --git a/models/user/name_test.go b/models/user/name_test.go deleted file mode 100644 index 7c1d5a7d62d3c..0000000000000 --- a/models/user/name_test.go +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. -// Use of this source code is governed by a MIT-style -// license that can be found in the LICENSE file.package db - -package user - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestIsValidUsername(t *testing.T) { - assert.True(t, IsValidUsername("a")) - assert.True(t, IsValidUsername("abc")) - assert.True(t, IsValidUsername("0.b-c")) - assert.True(t, IsValidUsername("a.b-c_d")) - - assert.False(t, IsValidUsername("")) - assert.False(t, IsValidUsername(".abc")) - assert.False(t, IsValidUsername("abc.")) - assert.False(t, IsValidUsername("a..bc")) - assert.False(t, IsValidUsername("a...bc")) - assert.False(t, IsValidUsername("a.-bc")) - assert.False(t, IsValidUsername("a._bc")) - assert.False(t, IsValidUsername("a_-bc")) - assert.False(t, IsValidUsername("a/bc")) - assert.False(t, IsValidUsername("☁️")) -} diff --git a/models/user/user.go b/models/user/user.go index b4083ec654c6d..84e2c4a9cc6ab 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -29,6 +29,7 @@ import ( "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" + "code.gitea.io/gitea/modules/validation" "golang.org/x/crypto/argon2" "golang.org/x/crypto/bcrypt" @@ -621,7 +622,7 @@ var ( // IsUsableUsername returns an error when a username is reserved func IsUsableUsername(name string) error { // Validate username make sure it satisfies requirement. - if IsValidUsername(name) { + if !validation.IsValidUsername(name) { // Note: usually this error is normally caught up earlier in the UI return db.ErrNameCharsNotAllowed{Name: name} } diff --git a/modules/structs/admin_user.go b/modules/structs/admin_user.go index eccbf29a46f0c..2f6f502af639b 100644 --- a/modules/structs/admin_user.go +++ b/modules/structs/admin_user.go @@ -10,7 +10,7 @@ type CreateUserOption struct { SourceID int64 `json:"source_id"` LoginName string `json:"login_name"` // 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)"` // required: true // swagger:strfmt email diff --git a/modules/validation/binding.go b/modules/validation/binding.go index f08f632426649..c054fbe7b6c0e 100644 --- a/modules/validation/binding.go +++ b/modules/validation/binding.go @@ -24,6 +24,9 @@ const ( // ErrRegexPattern is returned when a regex pattern is invalid ErrRegexPattern = "RegexPattern" + + // ErrUsername is username error + ErrUsername = "UsernameError" ) // AddBindingRules adds additional binding rules @@ -34,6 +37,7 @@ func AddBindingRules() { addGlobPatternRule() addRegexPatternRule() addGlobOrRegexPatternRule() + addUsernamePatternRule() } 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 { colon := strings.IndexByte(hostport, ':') if colon == -1 { diff --git a/modules/validation/helpers.go b/modules/validation/helpers.go index 484b12b2a23c4..af21681d38337 100644 --- a/modules/validation/helpers.go +++ b/modules/validation/helpers.go @@ -91,3 +91,15 @@ func IsValidExternalTrackerURLFormat(uri string) bool { return true } + +var ( + validUsernamePattern = regexp.MustCompile(`^[\da-zA-Z][-.\w]*$`) + invalidUsernamePattern = regexp.MustCompile(`[-._]{2,}|[-._]$`) +) + +// 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) +} diff --git a/modules/validation/helpers_test.go b/modules/validation/helpers_test.go index f6f897e8210da..a558fb3190f37 100644 --- a/modules/validation/helpers_test.go +++ b/modules/validation/helpers_test.go @@ -155,3 +155,30 @@ 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}, + } + for _, tt := range tests { + t.Run(tt.arg, func(t *testing.T) { + assert.Equalf(t, tt.want, IsValidUsername(tt.arg), "IsValidUsername(%v)", tt.arg) + }) + } +} diff --git a/modules/web/middleware/binding.go b/modules/web/middleware/binding.go index 636e655b9e956..cced9717be0ab 100644 --- a/modules/web/middleware/binding.go +++ b/modules/web/middleware/binding.go @@ -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) case validation.ErrRegexPattern: data["ErrorMsg"] = trName + l.Tr("form.regex_pattern_error", errs[0].Message) + case validation.ErrUsername: + data["ErrorMsg"] = trName + l.Tr("form.username_error") default: msg := errs[0].Classification if msg != "" && errs[0].Message != "" { diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 1566dfc97d422..fcb1a527a3809 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -461,6 +461,7 @@ url_error = `'%s' is not a valid URL.` include_error = ` must contain substring '%s'.` glob_pattern_error = ` glob pattern is invalid: %s.` regex_pattern_error = ` regex pattern is invalid: %s.` +username_error = ` should contain only alphanumeric, dash ('-'), underscore ('_') and dot ('.') characters, and cannot begin or end with non-alphanumeric, consecutive non-alphanumerics are also not allowed.` unknown_error = Unknown error: captcha_incorrect = The CAPTCHA code is incorrect. password_not_match = The passwords do not match. diff --git a/services/forms/admin.go b/services/forms/admin.go index 5abef0550e39a..537b9f982cfe1 100644 --- a/services/forms/admin.go +++ b/services/forms/admin.go @@ -18,7 +18,7 @@ import ( type AdminCreateUserForm struct { LoginType string `binding:"Required"` LoginName string - UserName string `binding:"Required;AlphaDashDot;MaxSize(40)"` + UserName string `binding:"Required;Username;MaxSize(40)"` Email string `binding:"Required;Email;MaxSize(254)"` Password string `binding:"MaxSize(255)"` SendNotify bool @@ -35,7 +35,7 @@ func (f *AdminCreateUserForm) Validate(req *http.Request, errs binding.Errors) b // AdminEditUserForm form for admin to create user type AdminEditUserForm struct { LoginType string `binding:"Required"` - UserName string `binding:"AlphaDashDot;MaxSize(40)"` + UserName string `binding:"Username;MaxSize(40)"` LoginName string FullName string `binding:"MaxSize(100)"` Email string `binding:"Required;Email;MaxSize(254)"` diff --git a/services/forms/user_form.go b/services/forms/user_form.go index 036c2ca3ec2c5..080b7c44c6bbf 100644 --- a/services/forms/user_form.go +++ b/services/forms/user_form.go @@ -90,7 +90,7 @@ func (f *InstallForm) Validate(req *http.Request, errs binding.Errors) binding.E // RegisterForm form for registering type RegisterForm struct { - UserName string `binding:"Required;AlphaDashDot;MaxSize(40)"` + UserName string `binding:"Required;Username;MaxSize(40)"` Email string `binding:"Required;MaxSize(254)"` Password string `binding:"MaxSize(255)"` Retype string diff --git a/services/forms/user_form_auth_openid.go b/services/forms/user_form_auth_openid.go index 992517f34f0f0..d1ed0a23c78a7 100644 --- a/services/forms/user_form_auth_openid.go +++ b/services/forms/user_form_auth_openid.go @@ -27,7 +27,7 @@ func (f *SignInOpenIDForm) Validate(req *http.Request, errs binding.Errors) bind // SignUpOpenIDForm form for signin up with OpenID type SignUpOpenIDForm struct { - UserName string `binding:"Required;AlphaDashDot;MaxSize(40)"` + UserName string `binding:"Required;Username;MaxSize(40)"` Email string `binding:"Required;Email;MaxSize(254)"` GRecaptchaResponse string `form:"g-recaptcha-response"` HcaptchaResponse string `form:"h-captcha-response"` From 0bcd62f43bbb2e706654bf9995f87b93b43db835 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Tue, 1 Nov 2022 15:12:32 +0800 Subject: [PATCH 09/17] test: update integration tests --- tests/integration/user_test.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/tests/integration/user_test.go b/tests/integration/user_test.go index 110f5c89bfbd3..cc4088c8c8f71 100644 --- a/tests/integration/user_test.go +++ b/tests/integration/user_test.go @@ -53,6 +53,18 @@ func TestRenameInvalidUsername(t *testing.T) { "%00", "thisHas ASpace", "ptho>lo Date: Tue, 1 Nov 2022 15:54:06 +0800 Subject: [PATCH 10/17] fix: update more form validations --- services/forms/org.go | 4 ++-- services/forms/user_form.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/services/forms/org.go b/services/forms/org.go index dec2dbfa6555a..c7ee91134552f 100644 --- a/services/forms/org.go +++ b/services/forms/org.go @@ -24,7 +24,7 @@ import ( // CreateOrgForm form for creating organization 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 RepoAdminChangeTeamAccess bool } @@ -37,7 +37,7 @@ func (f *CreateOrgForm) Validate(req *http.Request, errs binding.Errors) binding // UpdateOrgSettingForm form for updating organization settings 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)"` Description string `binding:"MaxSize(255)"` Website string `binding:"ValidUrl;MaxSize(255)"` diff --git a/services/forms/user_form.go b/services/forms/user_form.go index 080b7c44c6bbf..0f7f761ed1f85 100644 --- a/services/forms/user_form.go +++ b/services/forms/user_form.go @@ -64,7 +64,7 @@ type InstallForm struct { 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"` AdminConfirmPasswd string AdminEmail string `binding:"OmitEmpty;MinSize(3);MaxSize(254);Include(@)" locale:"install.admin_email"` @@ -242,7 +242,7 @@ func (f *IntrospectTokenForm) Validate(req *http.Request, errs binding.Errors) b // UpdateProfileForm form for updating profile type UpdateProfileForm struct { - Name string `binding:"AlphaDashDot;MaxSize(40)"` + Name string `binding:"Username;MaxSize(40)"` FullName string `binding:"MaxSize(100)"` KeepEmailPrivate bool Website string `binding:"ValidSiteUrl;MaxSize(255)"` From 0ba8593e41566e6b3a3569f3d715ebd0dd3c9284 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Tue, 1 Nov 2022 16:36:47 +0800 Subject: [PATCH 11/17] test: check new error message --- tests/integration/user_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/user_test.go b/tests/integration/user_test.go index cc4088c8c8f71..15e53f8bc8c63 100644 --- a/tests/integration/user_test.go +++ b/tests/integration/user_test.go @@ -80,7 +80,7 @@ func TestRenameInvalidUsername(t *testing.T) { htmlDoc := NewHTMLParser(t, resp.Body) assert.Contains(t, 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}) From 5ef6c022d673af1b1188037190fe75fb1fe50425 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 4 Nov 2022 09:35:19 +0800 Subject: [PATCH 12/17] Update modules/validation/helpers.go Co-authored-by: delvh --- modules/validation/helpers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/validation/helpers.go b/modules/validation/helpers.go index af21681d38337..827f52bb8a7e0 100644 --- a/modules/validation/helpers.go +++ b/modules/validation/helpers.go @@ -94,7 +94,7 @@ func IsValidExternalTrackerURLFormat(uri string) bool { var ( validUsernamePattern = regexp.MustCompile(`^[\da-zA-Z][-.\w]*$`) - invalidUsernamePattern = regexp.MustCompile(`[-._]{2,}|[-._]$`) + invalidUsernamePattern = regexp.MustCompile(`[-._]{2,}|[-._]$`) // No consecutive or trailing non-alphanumeric chars ) // IsValidUsername checks if username is valid From 1f53ec0d81e9d54c355a004a5fb65c4eabc2438c Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 4 Nov 2022 09:38:40 +0800 Subject: [PATCH 13/17] test: add more cases --- modules/validation/helpers_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/modules/validation/helpers_test.go b/modules/validation/helpers_test.go index a558fb3190f37..9bdbdb4a77555 100644 --- a/modules/validation/helpers_test.go +++ b/modules/validation/helpers_test.go @@ -175,6 +175,10 @@ func TestIsValidUsername(t *testing.T) { {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) { From f61c61358713cc640bd0aa28368ac019356aacd0 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 4 Nov 2022 09:47:35 +0800 Subject: [PATCH 14/17] fix: update username error --- options/locale/locale_en-US.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 30fcb5d94bbad..da53a8bb7d9b0 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -463,7 +463,7 @@ url_error = `'%s' is not a valid URL.` include_error = ` must contain substring '%s'.` glob_pattern_error = ` glob pattern is invalid: %s.` regex_pattern_error = ` regex pattern is invalid: %s.` -username_error = ` should contain only alphanumeric, dash ('-'), underscore ('_') and dot ('.') characters, and cannot begin or end with non-alphanumeric, consecutive non-alphanumerics are also not allowed.` +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: captcha_incorrect = The CAPTCHA code is incorrect. password_not_match = The passwords do not match. From 8f904dd57b56cf00d28babd26596c4ade06a5959 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 4 Nov 2022 09:56:38 +0800 Subject: [PATCH 15/17] test: add more cases --- tests/integration/user_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/integration/user_test.go b/tests/integration/user_test.go index 15e53f8bc8c63..017700ad40239 100644 --- a/tests/integration/user_test.go +++ b/tests/integration/user_test.go @@ -65,6 +65,10 @@ func TestRenameInvalidUsername(t *testing.T) { "a_-bc", "a/bc", "☁️", + "-", + "--diff", + "-im-here", + "a space", } session := loginUser(t, "user2") From 4ffb9c8a1fdfa16db128c41e2d6d3595398b212e Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 4 Nov 2022 11:10:44 +0800 Subject: [PATCH 16/17] chore: fmt --- modules/validation/helpers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/validation/helpers.go b/modules/validation/helpers.go index 827f52bb8a7e0..8e49c7855e5dd 100644 --- a/modules/validation/helpers.go +++ b/modules/validation/helpers.go @@ -94,7 +94,7 @@ func IsValidExternalTrackerURLFormat(uri string) bool { var ( validUsernamePattern = regexp.MustCompile(`^[\da-zA-Z][-.\w]*$`) - invalidUsernamePattern = regexp.MustCompile(`[-._]{2,}|[-._]$`) // No consecutive or trailing non-alphanumeric chars + invalidUsernamePattern = regexp.MustCompile(`[-._]{2,}|[-._]$`) // No consecutive or trailing non-alphanumeric chars ) // IsValidUsername checks if username is valid From a5eac5477d7a09edc364b9a1c1c7d0913343e870 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 4 Nov 2022 12:09:37 +0800 Subject: [PATCH 17/17] chore: trigger CI