From 4805151f56d49a3f67e5c9f192c7322fe813a4ab Mon Sep 17 00:00:00 2001 From: TheFox0x7 Date: Sat, 14 Feb 2026 17:51:03 +0100 Subject: [PATCH] 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 --- models/user/user.go | 82 ++++++++++++++++------ models/user/user_test.go | 47 ++++++++++--- tests/integration/editor_test.go | 2 +- tests/integration/repofiles_change_test.go | 12 ++-- 4 files changed, 106 insertions(+), 37 deletions(-) diff --git a/models/user/user.go b/models/user/user.go index 1797d3eefc..59a0b4e5d1 100644 --- a/models/user/user.go +++ b/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} diff --git a/models/user/user_test.go b/models/user/user_test.go index 923f2cd40e..378acc4180 100644 --- a/models/user/user_test.go +++ b/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) { diff --git a/tests/integration/editor_test.go b/tests/integration/editor_test.go index c70bb061c9..8368c91258 100644 --- a/tests/integration/editor_test.go +++ b/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)() diff --git a/tests/integration/repofiles_change_test.go b/tests/integration/repofiles_change_test.go index 442959b8a5..390ec2ebeb 100644 --- a/tests/integration/repofiles_change_test.go +++ b/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{