Skip to content

Commit a41bd17

Browse files
committed
Code review suggestions
1 parent 974a5be commit a41bd17

File tree

3 files changed

+16
-11
lines changed

3 files changed

+16
-11
lines changed

models/user/user.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -516,13 +516,13 @@ func GetUserSalt() (string, error) {
516516
return hex.EncodeToString(rBytes), nil
517517
}
518518

519-
var normalizeRE = regexp.MustCompile(`[^\da-zA-Z-.\w]`)
519+
var validUsernameRE = regexp.MustCompile(`[^\w-.]`)
520520

521521
// normalizeUserName returns a string with single-quotes removed,
522522
// and any other non-supported username characters replaced with
523523
// a `-` character
524524
func NormalizeUserName(s string) string {
525-
return normalizeRE.ReplaceAllString(strings.ReplaceAll(s, "'", ""), "-")
525+
return validUsernameRE.ReplaceAllString(strings.ReplaceAll(s, "'", ""), "-")
526526
}
527527

528528
var (

models/user/user_test.go

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -546,19 +546,25 @@ func Test_ValidateUser(t *testing.T) {
546546
}
547547

548548
func Test_NormalizeUserFromEmail(t *testing.T) {
549-
testCases := [][]interface{}{
550-
// input name, expected normalized name, is normalized name valid
549+
testCases := []struct {
550+
Input string
551+
Expected string
552+
IsNormalizedValid bool
553+
}{
551554
{"test", "test", true},
552-
{"Sinéad.O'Connor", "Sin-ad.OConnor", true}, // We should consider allowing custom replacement characters (eg. é -> e)
555+
{"Sinéad.O'Connor", "Sin-ad.OConnor", true},
556+
{"Æsir", "-sir", false},
557+
// \u00e9\u0065\u0301
558+
{"éé", "-e-", false},
553559
{"Awareness Hub", "Awareness-Hub", true},
554560
{"double__underscore", "double__underscore", false}, // We should consider squashing double non-alpha characters
555561
{".bad.", ".bad.", false},
562+
{"new😀user", "new-user", true},
556563
}
557564
for _, testCase := range testCases {
558-
fmt.Println(testCase...)
559-
normalizedName := user_model.NormalizeUserName(testCase[0].(string))
560-
assert.EqualValues(t, testCase[1].(string), normalizedName)
561-
if testCase[2].(bool) {
565+
normalizedName := user_model.NormalizeUserName(testCase.Input)
566+
assert.EqualValues(t, testCase.Expected, normalizedName)
567+
if testCase.IsNormalizedValid {
562568
assert.NoError(t, user_model.IsUsableUsername(normalizedName))
563569
} else {
564570
assert.Error(t, user_model.IsUsableUsername(normalizedName))

routers/web/auth/auth.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212

1313
"code.gitea.io/gitea/models/auth"
1414
"code.gitea.io/gitea/models/db"
15-
"code.gitea.io/gitea/models/user"
1615
user_model "code.gitea.io/gitea/models/user"
1716
"code.gitea.io/gitea/modules/auth/password"
1817
"code.gitea.io/gitea/modules/base"
@@ -374,7 +373,7 @@ func getUserName(gothUser *goth.User) string {
374373
case setting.OAuth2UsernameEmail:
375374
return strings.Split(gothUser.Email, "@")[0]
376375
case setting.OAuth2UsernameEmailNormalized:
377-
return user.NormalizeUserName(strings.Split(gothUser.Email, "@")[0])
376+
return user_model.NormalizeUserName(strings.Split(gothUser.Email, "@")[0])
378377
case setting.OAuth2UsernameNickname:
379378
return gothUser.NickName
380379
default: // OAuth2UsernameUserid

0 commit comments

Comments
 (0)