From 54acf7b0d4b57907a6d8a2ac470744f185adf0b5 Mon Sep 17 00:00:00 2001 From: Kyle D Date: Wed, 3 Jan 2024 16:48:20 -0800 Subject: [PATCH] Normalize oauth email username (#28561) --- custom/conf/app.example.ini | 4 +++ .../config-cheat-sheet.en-us.md | 10 +++++-- models/user/user.go | 25 +++++++++++++++++ models/user/user_test.go | 28 +++++++++++++++++++ modules/setting/oauth2.go | 2 +- routers/web/auth/auth.go | 8 +++--- routers/web/auth/linkaccount.go | 6 +++- routers/web/auth/oauth.go | 7 ++++- 8 files changed, 80 insertions(+), 10 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index d58309f141bd..d1dd0a141b96 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1529,6 +1529,10 @@ LEVEL = Info ;; userid = use the userid / sub attribute ;; nickname = use the nickname attribute ;; email = use the username part of the email attribute +;; Note: `nickname` and `email` options will normalize input strings using the following criteria: +;; - diacritics are removed +;; - the characters in the set `['´\x60]` are removed +;; - the characters in the set `[\s~+]` are replaced with `-` ;USERNAME = nickname ;; ;; Update avatar if available from oauth2 provider. diff --git a/docs/content/administration/config-cheat-sheet.en-us.md b/docs/content/administration/config-cheat-sheet.en-us.md index e111ff6db62e..c11b4012a551 100644 --- a/docs/content/administration/config-cheat-sheet.en-us.md +++ b/docs/content/administration/config-cheat-sheet.en-us.md @@ -596,9 +596,13 @@ And the following unique queues: - `OPENID_CONNECT_SCOPES`: **_empty_**: List of additional openid connect scopes. (`openid` is implicitly added) - `ENABLE_AUTO_REGISTRATION`: **false**: Automatically create user accounts for new oauth2 users. - `USERNAME`: **nickname**: The source of the username for new oauth2 accounts: - - userid - use the userid / sub attribute - - nickname - use the nickname attribute - - email - use the username part of the email attribute + - `userid` - use the userid / sub attribute + - `nickname` - use the nickname attribute + - `email` - use the username part of the email attribute + - Note: `nickname` and `email` options will normalize input strings using the following criteria: + - diacritics are removed + - the characters in the set `['´\x60]` are removed + - the characters in the set `[\s~+]` are replaced with `-` - `UPDATE_AVATAR`: **false**: Update avatar if available from oauth2 provider. Update will be performed on each login. - `ACCOUNT_LINKING`: **login**: How to handle if an account / email already exists: - disabled - show an error diff --git a/models/user/user.go b/models/user/user.go index ce0e055b15c6..d828f3d65dcc 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -10,8 +10,10 @@ import ( "fmt" "net/url" "path/filepath" + "regexp" "strings" "time" + "unicode" _ "image/jpeg" // Needed for jpeg support @@ -29,6 +31,9 @@ import ( "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/validation" + "golang.org/x/text/runes" + "golang.org/x/text/transform" + "golang.org/x/text/unicode/norm" "xorm.io/builder" ) @@ -515,6 +520,26 @@ func GetUserSalt() (string, error) { return hex.EncodeToString(rBytes), nil } +// Note: The set of characters here can safely expand without a breaking change, +// but characters removed from this set can cause user account linking to break +var ( + customCharsReplacement = strings.NewReplacer("Æ", "AE") + removeCharsRE = regexp.MustCompile(`['´\x60]`) + removeDiacriticsTransform = transform.Chain(norm.NFD, runes.Remove(runes.In(unicode.Mn)), norm.NFC) + replaceCharsHyphenRE = regexp.MustCompile(`[\s~+]`) +) + +// normalizeUserName returns a string with single-quotes and diacritics +// removed, and any other non-supported username characters replaced with +// a `-` character +func NormalizeUserName(s string) (string, error) { + strDiacriticsRemoved, n, err := transform.String(removeDiacriticsTransform, customCharsReplacement.Replace(s)) + if err != nil { + return "", fmt.Errorf("Failed to normalize character `%v` in provided username `%v`", s[n], s) + } + return replaceCharsHyphenRE.ReplaceAllLiteralString(removeCharsRE.ReplaceAllLiteralString(strDiacriticsRemoved, ""), "-"), nil +} + var ( reservedUsernames = []string{ ".", diff --git a/models/user/user_test.go b/models/user/user_test.go index 971117482c2d..65aebea43a95 100644 --- a/models/user/user_test.go +++ b/models/user/user_test.go @@ -544,3 +544,31 @@ func Test_ValidateUser(t *testing.T) { assert.EqualValues(t, expected, err == nil, fmt.Sprintf("case: %+v", kase)) } } + +func Test_NormalizeUserFromEmail(t *testing.T) { + testCases := []struct { + Input string + Expected string + IsNormalizedValid bool + }{ + {"test", "test", true}, + {"Sinéad.O'Connor", "Sinead.OConnor", true}, + {"Æsir", "AEsir", true}, + // \u00e9\u0065\u0301 + {"éé", "ee", true}, + {"Awareness Hub", "Awareness-Hub", true}, + {"double__underscore", "double__underscore", false}, // We should consider squashing double non-alpha characters + {".bad.", ".bad.", false}, + {"new😀user", "new😀user", false}, // No plans to support + } + for _, testCase := range testCases { + normalizedName, err := user_model.NormalizeUserName(testCase.Input) + assert.NoError(t, err) + assert.EqualValues(t, testCase.Expected, normalizedName) + if testCase.IsNormalizedValid { + assert.NoError(t, user_model.IsUsableUsername(normalizedName)) + } else { + assert.Error(t, user_model.IsUsableUsername(normalizedName)) + } + } +} diff --git a/modules/setting/oauth2.go b/modules/setting/oauth2.go index aea76b989ce6..10cadf03dd32 100644 --- a/modules/setting/oauth2.go +++ b/modules/setting/oauth2.go @@ -21,7 +21,7 @@ const ( OAuth2UsernameUserid OAuth2UsernameType = "userid" // OAuth2UsernameNickname oauth2 nickname field will be used as gitea name OAuth2UsernameNickname OAuth2UsernameType = "nickname" - // OAuth2UsernameEmail username of oauth2 email filed will be used as gitea name + // OAuth2UsernameEmail username of oauth2 email field will be used as gitea name OAuth2UsernameEmail OAuth2UsernameType = "email" ) diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index 21a72a952153..474bae98e4a4 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -368,14 +368,14 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe return setting.AppSubURL + "/" } -func getUserName(gothUser *goth.User) string { +func getUserName(gothUser *goth.User) (string, error) { switch setting.OAuth2Client.Username { case setting.OAuth2UsernameEmail: - return strings.Split(gothUser.Email, "@")[0] + return user_model.NormalizeUserName(strings.Split(gothUser.Email, "@")[0]) case setting.OAuth2UsernameNickname: - return gothUser.NickName + return user_model.NormalizeUserName(gothUser.NickName) default: // OAuth2UsernameUserid - return gothUser.UserID + return gothUser.UserID, nil } } diff --git a/routers/web/auth/linkaccount.go b/routers/web/auth/linkaccount.go index f41590dc1358..1d94e52fe3e4 100644 --- a/routers/web/auth/linkaccount.go +++ b/routers/web/auth/linkaccount.go @@ -55,7 +55,11 @@ func LinkAccount(ctx *context.Context) { } gu, _ := gothUser.(goth.User) - uname := getUserName(&gu) + uname, err := getUserName(&gu) + if err != nil { + ctx.ServerError("UserSignIn", err) + return + } email := gu.Email ctx.Data["user_name"] = uname ctx.Data["email"] = email diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index 21d82cea4503..00305a36ee2b 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -970,8 +970,13 @@ func SignInOAuthCallback(ctx *context.Context) { ctx.ServerError("CreateUser", err) return } + uname, err := getUserName(&gothUser) + if err != nil { + ctx.ServerError("UserSignIn", err) + return + } u = &user_model.User{ - Name: getUserName(&gothUser), + Name: uname, FullName: gothUser.Name, Email: gothUser.Email, LoginType: auth.OAuth2,