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 <me@silverwind.io>

diff --git a/models/user/user.go b/models/user/user.go
index 4b19eda67b61c82317f941cd2a1aa9011b7a3cbb..86cf2ad28060f3ed91fa16e3482f2ec3b43f0540 100644 (file)
--- 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 79fcba0176f9a3104659aee1ffd7bdcf806e3059..3521dd2f905c3c9f3dd49503be2585e25d19fe6e 100644 (file)
--- 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 82cd97e904c0dcc9dc1f9dfd777f6d5ce71b5a00..f4edfd8941cf3f19d734195059d2e8c3b097b08a 100644 (file)
--- 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 be14d5a6b2965f79e4ca41757a6408c125bb490d..c5b202b3193f79b6a8fa35a1865e66d13f86076f 100644 (file)
--- 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)
        }
 }