follow #30454

Author: wxiaoguang

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
 }

models/user/user_test.go

@@ -506,15 +506,16 @@ func Test_NormalizeUserFromEmail(t *testing.T) {
 		Expected          string
 		IsNormalizedValid bool
 	}{
+		{"[email protected]", "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)

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)))

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

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)

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,