use user id in noreply emails (#36550)

This implements id based hidden emails in format of
`user+id@NoReplyAddress`

resolves: https://github.com/go-gitea/gitea/issues/33471

---

The change is not breaking however it is recommended for users to move
to this newer type of no reply address

---------

Co-authored-by: Lauris B <lauris@nix.lv>

models/user/user.go

@@ -13,6 +13,7 @@ import (
 	"net/url"
 	"path/filepath"
 	"regexp"
+	"strconv"
 	"strings"
 	"sync"
 	"time"
@@ -212,7 +213,7 @@ func (u *User) SetLastLogin() {
 
 // GetPlaceholderEmail returns an noreply email
 func (u *User) GetPlaceholderEmail() string {
-	return fmt.Sprintf("%s@%s", u.LowerName, setting.Service.NoReplyAddress)
+	return fmt.Sprintf("%s+%d@%s", u.LowerName, u.ID, setting.Service.NoReplyAddress)
 }
 
 // GetEmail returns a noreply email, if the user has set to keep his
@@ -1197,14 +1198,18 @@ func GetUsersByEmails(ctx context.Context, emails []string) (*EmailUserMap, erro
 
 	needCheckEmails := make(container.Set[string])
 	needCheckUserNames := make(container.Set[string])
+	needCheckUserIDs := make(container.Set[int64])
 	noReplyAddressSuffix := "@" + strings.ToLower(setting.Service.NoReplyAddress)
 	for _, email := range emails {
 		emailLower := strings.ToLower(email)
-		if noReplyUserNameLower, ok := strings.CutSuffix(emailLower, noReplyAddressSuffix); ok {
-			needCheckUserNames.Add(noReplyUserNameLower)
-			needCheckEmails.Add(emailLower)
-		} else {
-			needCheckEmails.Add(emailLower)
+		needCheckEmails.Add(emailLower)
+		if localPart, ok := strings.CutSuffix(emailLower, noReplyAddressSuffix); ok {
+			name, id := parseLocalPartToNameID(localPart)
+			if id != 0 {
+				needCheckUserIDs.Add(id)
+			} else if name != "" {
+				needCheckUserNames.Add(name)
+			}
 		}
 	}
 
@@ -1234,16 +1239,57 @@ func GetUsersByEmails(ctx context.Context, emails []string) (*EmailUserMap, erro
 		}
 	}
 
-	users := make(map[int64]*User, len(needCheckUserNames))
-	if err := db.GetEngine(ctx).In("lower_name", needCheckUserNames.Values()).Find(&users); err != nil {
-		return nil, err
+	usersByIDs := make(map[int64]*User)
+	if len(needCheckUserIDs) > 0 || len(needCheckUserNames) > 0 {
+		cond := builder.NewCond()
+		if len(needCheckUserIDs) > 0 {
+			cond = cond.Or(builder.In("id", needCheckUserIDs.Values()))
+		}
+		if len(needCheckUserNames) > 0 {
+			cond = cond.Or(builder.In("lower_name", needCheckUserNames.Values()))
+		}
+		if err := db.GetEngine(ctx).Where(cond).Find(&usersByIDs); err != nil {
+			return nil, err
+		}
 	}
-	for _, user := range users {
-		results[strings.ToLower(user.GetPlaceholderEmail())] = user
+
+	usersByName := make(map[string]*User)
+	for _, user := range usersByIDs {
+		usersByName[user.LowerName] = user
 	}
+
+	for _, email := range emails {
+		emailLower := strings.ToLower(email)
+		if _, ok := results[emailLower]; ok {
+			continue
+		}
+
+		localPart, ok := strings.CutSuffix(emailLower, noReplyAddressSuffix)
+		if !ok {
+			continue
+		}
+		name, id := parseLocalPartToNameID(localPart)
+		if user, ok := usersByIDs[id]; ok {
+			results[emailLower] = user
+		} else if user, ok := usersByName[name]; ok {
+			results[emailLower] = user
+		}
+	}
+
 	return &EmailUserMap{results}, nil
 }
 
+// parseLocalPartToNameID attempts to unparse local-part of email that's in format user+id
+// returns user and id if possible
+func parseLocalPartToNameID(localPart string) (string, int64) {
+	var id int64
+	name, idstr, hasPlus := strings.Cut(localPart, "+")
+	if hasPlus {
+		id, _ = strconv.ParseInt(idstr, 10, 64)
+	}
+	return name, id
+}
+
 // GetUserByEmail returns the user object by given e-mail if exists.
 func GetUserByEmail(ctx context.Context, email string) (*User, error) {
 	if len(email) == 0 {
@@ -1262,16 +1308,12 @@ func GetUserByEmail(ctx context.Context, email string) (*User, error) {
 	}
 
 	// Finally, if email address is the protected email address:
-	if before, ok := strings.CutSuffix(email, "@"+setting.Service.NoReplyAddress); ok {
-		username := before
-		user := &User{}
-		has, err := db.GetEngine(ctx).Where("lower_name=?", username).Get(user)
-		if err != nil {
-			return nil, err
-		}
-		if has {
-			return user, nil
+	if localPart, ok := strings.CutSuffix(email, strings.ToLower("@"+setting.Service.NoReplyAddress)); ok {
+		name, id := parseLocalPartToNameID(localPart)
+		if id != 0 {
+			return GetUserByID(ctx, id)
 		}
+		return GetUserByName(ctx, name)
 	}
 
 	return nil, ErrUserNotExist{Name: email}

models/user/user_test.go

@@ -51,12 +51,27 @@ func TestOAuth2Application_LoadUser(t *testing.T) {
 
 func TestUserEmails(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
+	defer test.MockVariableValue(&setting.Service.NoReplyAddress, "NoReply.gitea.internal")()
 	t.Run("GetUserEmailsByNames", func(t *testing.T) {
-		// ignore none active user email
+		// ignore not active user email
 		assert.ElementsMatch(t, []string{"user8@example.com"}, user_model.GetUserEmailsByNames(t.Context(), []string{"user8", "user9"}))
 		assert.ElementsMatch(t, []string{"user8@example.com", "user5@example.com"}, user_model.GetUserEmailsByNames(t.Context(), []string{"user8", "user5"}))
 		assert.ElementsMatch(t, []string{"user8@example.com"}, user_model.GetUserEmailsByNames(t.Context(), []string{"user8", "org7"}))
 	})
+
+	cases := []struct {
+		Email string
+		UID   int64
+	}{
+		{"UseR1@example.com", 1},
+		{"user1-2@example.COM", 1},
+		{"USER2@" + setting.Service.NoReplyAddress, 2},
+		{"user2+2@" + setting.Service.NoReplyAddress, 2},
+		{"oldUser2UsernameWhichDoesNotMatterForQuery+2@" + setting.Service.NoReplyAddress, 2},
+		{"badUser+99999@" + setting.Service.NoReplyAddress, 0},
+		{"user4@example.com", 4},
+		{"no-such", 0},
+	}
 	t.Run("GetUsersByEmails", func(t *testing.T) {
 		defer test.MockVariableValue(&setting.Service.NoReplyAddress, "NoReply.gitea.internal")()
 		testGetUserByEmail := func(t *testing.T, email string, uid int64) {
@@ -70,15 +85,27 @@ func TestUserEmails(t *testing.T) {
 			require.NotNil(t, user)
 			assert.Equal(t, uid, user.ID)
 		}
-		cases := []struct {
-			Email string
-			UID   int64
-		}{
-			{"UseR1@example.com", 1},
-			{"user1-2@example.COM", 1},
-			{"USER2@" + setting.Service.NoReplyAddress, 2},
-			{"user4@example.com", 4},
-			{"no-such", 0},
+		for _, c := range cases {
+			t.Run(c.Email, func(t *testing.T) {
+				testGetUserByEmail(t, c.Email, c.UID)
+			})
+		}
+
+		t.Run("NoReplyConflict", func(t *testing.T) {
+			setting.Service.NoReplyAddress = "example.com"
+			testGetUserByEmail(t, "user1-2@example.COM", 1)
+		})
+	})
+	t.Run("GetUserByEmail", func(t *testing.T) {
+		testGetUserByEmail := func(t *testing.T, email string, uid int64) {
+			user, err := user_model.GetUserByEmail(t.Context(), email)
+			if uid == 0 {
+				require.Error(t, err)
+				assert.Nil(t, user)
+			} else {
+				require.NotNil(t, user)
+				assert.Equal(t, uid, user.ID)
+			}
 		}
 		for _, c := range cases {
 			t.Run(c.Email, func(t *testing.T) {

tests/integration/editor_test.go

@@ -258,7 +258,7 @@ func testEditorWebGitCommitEmail(t *testing.T) {
 		t.Run("DefaultEmailKeepPrivate", func(t *testing.T) {
 			defer tests.PrintCurrentTest(t)()
 			paramsForKeepPrivate["commit_email"] = ""
-			resp1 = makeReq(t, linkForKeepPrivate, paramsForKeepPrivate, "User Two", "user2@noreply.example.org")
+			resp1 = makeReq(t, linkForKeepPrivate, paramsForKeepPrivate, "User Two", "user2+2@noreply.example.org")
 		})
 		t.Run("ChooseEmail", func(t *testing.T) {
 			defer tests.PrintCurrentTest(t)()

tests/integration/repofiles_change_test.go

@@ -132,14 +132,14 @@ func getExpectedFileResponseForRepoFilesCreate(commitID string, lastCommit *git.
 			Author: &api.CommitUser{
 				Identity: api.Identity{
 					Name:  "User Two",
-					Email: "user2@noreply.example.org",
+					Email: "user2+2@noreply.example.org",
 				},
 				Date: time.Now().UTC().Format(time.RFC3339),
 			},
 			Committer: &api.CommitUser{
 				Identity: api.Identity{
 					Name:  "User Two",
-					Email: "user2@noreply.example.org",
+					Email: "user2+2@noreply.example.org",
 				},
 				Date: time.Now().UTC().Format(time.RFC3339),
 			},
@@ -202,14 +202,14 @@ func getExpectedFileResponseForRepoFilesUpdate(commitID, filename, lastCommitSHA
 			Author: &api.CommitUser{
 				Identity: api.Identity{
 					Name:  "User Two",
-					Email: "user2@noreply.example.org",
+					Email: "user2+2@noreply.example.org",
 				},
 				Date: time.Now().UTC().Format(time.RFC3339),
 			},
 			Committer: &api.CommitUser{
 				Identity: api.Identity{
 					Name:  "User Two",
-					Email: "user2@noreply.example.org",
+					Email: "user2+2@noreply.example.org",
 				},
 				Date: time.Now().UTC().Format(time.RFC3339),
 			},
@@ -312,13 +312,13 @@ func getExpectedFileResponseForRepoFilesUpdateRename(commitID, lastCommitSHA str
 			Author: &api.CommitUser{
 				Identity: api.Identity{
 					Name:  "User Two",
-					Email: "user2@noreply.example.org",
+					Email: "user2+2@noreply.example.org",
 				},
 			},
 			Committer: &api.CommitUser{
 				Identity: api.Identity{
 					Name:  "User Two",
-					Email: "user2@noreply.example.org",
+					Email: "user2+2@noreply.example.org",
 				},
 			},
 			Parents: []*api.CommitMeta{