diff --git a/models/user/user.go b/models/user/user.go index d459ec239e978..6129819403f33 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -501,19 +501,19 @@ func GetUserSalt() (string, error) { // 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~+]`) + customCharsReplacement = strings.NewReplacer("Æ", "AE") + removeCharsRE = regexp.MustCompile(`['´\x60]`) + transformDiacritics = 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 +// NormalizeUserName only takes the name part if it is an email address, transforms it diacritics to ASCII characters. +// It returns a string with all quotes removed, and any other non-supported username characters are replaced with a `-` character func NormalizeUserName(s string) (string, error) { - strDiacriticsRemoved, n, err := transform.String(removeDiacriticsTransform, customCharsReplacement.Replace(s)) + s, _, _ = strings.Cut(s, "@") + strDiacriticsRemoved, n, err := transform.String(transformDiacritics, customCharsReplacement.Replace(s)) if err != nil { - return "", fmt.Errorf("Failed to normalize character `%v` in provided username `%v`", s[n], s) + return "", fmt.Errorf("failed to normalize the string of provided username %q at position %d", s, n) } return replaceCharsHyphenRE.ReplaceAllLiteralString(removeCharsRE.ReplaceAllLiteralString(strDiacriticsRemoved, ""), "-"), nil } diff --git a/models/user/user_test.go b/models/user/user_test.go index a4550fa655d4b..d7bb339c867be 100644 --- a/models/user/user_test.go +++ b/models/user/user_test.go @@ -506,15 +506,16 @@ func Test_NormalizeUserFromEmail(t *testing.T) { Expected string IsNormalizedValid bool }{ + {"name@example.com", "name", true}, {"test", "test", true}, {"Sinéad.O'Connor", "Sinead.OConnor", true}, {"Æsir", "AEsir", true}, - // \u00e9\u0065\u0301 - {"éé", "ee", true}, + {"éé", "ee", true}, // \u00e9\u0065\u0301 {"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 + {`"quoted"`, `"quoted"`, false}, // No plans to support } for _, testCase := range testCases { normalizedName, err := user_model.NormalizeUserName(testCase.Input) diff --git a/modules/setting/oauth2.go b/modules/setting/oauth2.go index 6930197b22a7e..bf551c6aef6b5 100644 --- a/modules/setting/oauth2.go +++ b/modules/setting/oauth2.go @@ -16,14 +16,10 @@ import ( type OAuth2UsernameType string const ( - // OAuth2UsernameUserid oauth2 userid field will be used as gitea name - OAuth2UsernameUserid OAuth2UsernameType = "userid" - // OAuth2UsernameNickname oauth2 nickname field will be used as gitea name - OAuth2UsernameNickname OAuth2UsernameType = "nickname" - // OAuth2UsernameEmail username of oauth2 email field will be used as gitea name - OAuth2UsernameEmail OAuth2UsernameType = "email" - // OAuth2UsernameEmail username of oauth2 preferred_username field will be used as gitea name - OAuth2UsernamePreferredUsername OAuth2UsernameType = "preferred_username" + OAuth2UsernameUserid OAuth2UsernameType = "userid" // use user id (sub) field as gitea's username + OAuth2UsernameNickname OAuth2UsernameType = "nickname" // use nickname field + OAuth2UsernameEmail OAuth2UsernameType = "email" // use email field + OAuth2UsernamePreferredUsername OAuth2UsernameType = "preferred_username" // use preferred_username field ) func (username OAuth2UsernameType) isValid() bool { @@ -71,8 +67,8 @@ func loadOAuth2ClientFrom(rootCfg ConfigProvider) { OAuth2Client.EnableAutoRegistration = sec.Key("ENABLE_AUTO_REGISTRATION").MustBool() OAuth2Client.Username = OAuth2UsernameType(sec.Key("USERNAME").MustString(string(OAuth2UsernameNickname))) if !OAuth2Client.Username.isValid() { - log.Warn("Username setting is not valid: '%s', will fallback to '%s'", OAuth2Client.Username, OAuth2UsernameNickname) OAuth2Client.Username = OAuth2UsernameNickname + log.Warn("[oauth2_client].USERNAME setting is invalid, falls back to %q", OAuth2Client.Username) } OAuth2Client.UpdateAvatar = sec.Key("UPDATE_AVATAR").MustBool() OAuth2Client.AccountLinking = OAuth2AccountLinkingType(sec.Key("ACCOUNT_LINKING").MustString(string(OAuth2AccountLinkingLogin))) diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index 9ef32ebdb13cc..7c873796fe40e 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -382,17 +382,17 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe return setting.AppSubURL + "/" } -func getUserName(gothUser *goth.User) (string, error) { +// extractUserNameFromOAuth2 tries to extract a normalized username from the given OAuth2 user. +// It returns ("", nil) if the required field doesn't exist. +func extractUserNameFromOAuth2(gothUser *goth.User) (string, error) { switch setting.OAuth2Client.Username { case setting.OAuth2UsernameEmail: - return user_model.NormalizeUserName(strings.Split(gothUser.Email, "@")[0]) + return user_model.NormalizeUserName(gothUser.Email) case setting.OAuth2UsernamePreferredUsername: - preferredUsername, exists := gothUser.RawData["preferred_username"] - if exists { - return user_model.NormalizeUserName(preferredUsername.(string)) - } else { - return "", fmt.Errorf("preferred_username is missing in received user data but configured as username source for user_id %q. Check if OPENID_CONNECT_SCOPES contains profile", gothUser.UserID) + if preferredUsername, ok := gothUser.RawData["preferred_username"].(string); ok { + return user_model.NormalizeUserName(preferredUsername) } + return "", nil case setting.OAuth2UsernameNickname: return user_model.NormalizeUserName(gothUser.NickName) default: // OAuth2UsernameUserid diff --git a/routers/web/auth/linkaccount.go b/routers/web/auth/linkaccount.go index f744a57a43f94..4d9959dd0177c 100644 --- a/routers/web/auth/linkaccount.go +++ b/routers/web/auth/linkaccount.go @@ -48,23 +48,22 @@ func LinkAccount(ctx *context.Context) { ctx.Data["SignInLink"] = setting.AppSubURL + "/user/link_account_signin" ctx.Data["SignUpLink"] = setting.AppSubURL + "/user/link_account_signup" - gothUser := ctx.Session.Get("linkAccountGothUser") - if gothUser == nil { + gothUser, ok := ctx.Session.Get("linkAccountGothUser").(goth.User) + if !ok { ctx.ServerError("UserSignIn", errors.New("not in LinkAccount session")) return } - gu, _ := gothUser.(goth.User) - uname, err := getUserName(&gu) + uname, err := extractUserNameFromOAuth2(&gothUser) if err != nil { ctx.ServerError("UserSignIn", err) return } - email := gu.Email + email := gothUser.Email ctx.Data["user_name"] = uname ctx.Data["email"] = email - if len(email) != 0 { + if email != "" { u, err := user_model.GetUserByEmail(ctx, email) if err != nil && !user_model.IsErrUserNotExist(err) { ctx.ServerError("UserSignIn", err) @@ -73,7 +72,7 @@ func LinkAccount(ctx *context.Context) { if u != nil { ctx.Data["user_exists"] = true } - } else if len(uname) != 0 { + } else if uname != "" { u, err := user_model.GetUserByName(ctx, uname) if err != nil && !user_model.IsErrUserNotExist(err) { ctx.ServerError("UserSignIn", err) diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index 3189d1372e259..23624a503d096 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -952,23 +952,27 @@ func SignInOAuthCallback(ctx *context.Context) { if gothUser.Email == "" { missingFields = append(missingFields, "email") } - if setting.OAuth2Client.Username == setting.OAuth2UsernameNickname && gothUser.NickName == "" { - missingFields = append(missingFields, "nickname") + uname, err := extractUserNameFromOAuth2(&gothUser) + if err != nil { + ctx.ServerError("UserSignIn", err) + return + } + if uname == "" { + if setting.OAuth2Client.Username == setting.OAuth2UsernameNickname { + missingFields = append(missingFields, "nickname") + } else if setting.OAuth2Client.Username == setting.OAuth2UsernamePreferredUsername { + missingFields = append(missingFields, "preferred_username") + } // else: "UserID" and "Email" have been handled above separately } if len(missingFields) > 0 { - log.Error("OAuth2 Provider %s returned empty or missing fields: %s", authSource.Name, missingFields) + err := fmt.Errorf("OAuth2 Provider %q returned empty or missing fields: %s", authSource.Name, strings.Join(missingFields, ",")) if authSource.IsOAuth2() && authSource.Cfg.(*oauth2.Source).Provider == "openidConnect" { - log.Error("You may need to change the 'OPENID_CONNECT_SCOPES' setting to request all required fields") + err = fmt.Errorf("you may need to change the '[oauth2_client].OPENID_CONNECT_SCOPES' setting to request all required fields: %w", err) } - err = fmt.Errorf("OAuth2 Provider %s returned empty or missing fields: %s", authSource.Name, missingFields) + ctx.Flash.Error(err.Error(), true) ctx.ServerError("CreateUser", err) return } - uname, err := getUserName(&gothUser) - if err != nil { - ctx.ServerError("UserSignIn", err) - return - } u = &user_model.User{ Name: uname, FullName: gothUser.Name,