From ea385f5d39a286ec36f3735fc7c5211b141f6165 Mon Sep 17 00:00:00 2001 From: JakobDev Date: Mon, 31 Jul 2023 03:18:38 +0200 Subject: [PATCH] Fix API leaking Usermail if not logged in (#25097) The API should only return the real Mail of a User, if the caller is logged in. The check do to this don't work. This PR fixes this. This not really a security issue, but can lead to Spam. --------- Co-authored-by: silverwind --- models/user/user.go | 7 ++++++- services/convert/user.go | 2 +- tests/integration/api_user_info_test.go | 16 ++++++++++++++++ tests/integration/api_user_search_test.go | 8 +------- 4 files changed, 24 insertions(+), 9 deletions(-) diff --git a/models/user/user.go b/models/user/user.go index 4b19eda67..86cf2ad28 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -203,11 +203,16 @@ func UpdateUserTheme(u *User, themeName string) error { return UpdateUserCols(db.DefaultContext, u, "theme") } +// GetPlaceholderEmail returns an noreply email +func (u *User) GetPlaceholderEmail() string { + return fmt.Sprintf("%s@%s", u.LowerName, setting.Service.NoReplyAddress) +} + // GetEmail returns an noreply email, if the user has set to keep his // email address private, otherwise the primary email address. func (u *User) GetEmail() string { if u.KeepEmailPrivate { - return fmt.Sprintf("%s@%s", u.LowerName, setting.Service.NoReplyAddress) + return u.GetPlaceholderEmail() } return u.Email } diff --git a/services/convert/user.go b/services/convert/user.go index 79fcba017..3521dd2f9 100644 --- a/services/convert/user.go +++ b/services/convert/user.go @@ -51,7 +51,7 @@ func toUser(ctx context.Context, user *user_model.User, signed, authed bool) *ap ID: user.ID, UserName: user.Name, FullName: user.FullName, - Email: user.GetEmail(), + Email: user.GetPlaceholderEmail(), AvatarURL: user.AvatarLink(ctx), Created: user.CreatedUnix.AsTime(), Restricted: user.IsRestricted, diff --git a/tests/integration/api_user_info_test.go b/tests/integration/api_user_info_test.go index 82cd97e90..f4edfd894 100644 --- a/tests/integration/api_user_info_test.go +++ b/tests/integration/api_user_info_test.go @@ -9,6 +9,8 @@ import ( "testing" auth_model "code.gitea.io/gitea/models/auth" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/tests" @@ -21,6 +23,8 @@ func TestAPIUserInfo(t *testing.T) { user := "user1" user2 := "user31" + user3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user3"}) + session := loginUser(t, user) token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadUser) @@ -36,6 +40,18 @@ func TestAPIUserInfo(t *testing.T) { req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/users/%s", user2)) MakeRequest(t, req, http.StatusNotFound) + + // test if the placaholder Mail is returned if a User is not logged in + req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/users/%s", user3.Name)) + resp = MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &u) + assert.Equal(t, user3.GetPlaceholderEmail(), u.Email) + + // Test if the correct Mail is returned if a User is logged in + req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/users/%s?token=%s", user3.Name, token)) + resp = MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &u) + assert.Equal(t, user3.GetEmail(), u.Email) }) t.Run("GetAuthenticatedUser", func(t *testing.T) { diff --git a/tests/integration/api_user_search_test.go b/tests/integration/api_user_search_test.go index be14d5a6b..c5b202b31 100644 --- a/tests/integration/api_user_search_test.go +++ b/tests/integration/api_user_search_test.go @@ -4,14 +4,12 @@ package integration import ( - "fmt" "net/http" "testing" auth_model "code.gitea.io/gitea/models/auth" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/tests" @@ -54,11 +52,7 @@ func TestAPIUserSearchNotLoggedIn(t *testing.T) { for _, user := range results.Data { assert.Contains(t, user.UserName, query) modelUser = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: user.ID}) - if modelUser.KeepEmailPrivate { - assert.EqualValues(t, fmt.Sprintf("%s@%s", modelUser.LowerName, setting.Service.NoReplyAddress), user.Email) - } else { - assert.EqualValues(t, modelUser.Email, user.Email) - } + assert.EqualValues(t, modelUser.GetPlaceholderEmail(), user.Email) } }