From 4fd8ac0653b95fc204eade4471734b23039bca13 Mon Sep 17 00:00:00 2001
From: wxiaoguang <wxiaoguang@gmail.com>
Date: Sun, 6 Aug 2023 20:11:39 +0800
Subject: [PATCH] Fix API leaking Usermail if not logged in (#25097) (#26350)
Backport #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: JakobDev <jakobdev@gmx.de>
Co-authored-by: silverwind <me@silverwind.io>
---
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 2077d55f51..b6dc05ddd3 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 79fcba0176..3521dd2f90 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 82cd97e904..f4edfd8941 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 be14d5a6b2..c5b202b319 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)
}
}