Skip to content

Commit ea385f5

Browse files
JakobDevsilverwind
andauthored
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>
1 parent 7dc2e50 commit ea385f5

File tree

4 files changed

+24
-9
lines changed

4 files changed

+24
-9
lines changed

models/user/user.go

+6-1
Original file line numberDiff line numberDiff line change
@@ -203,11 +203,16 @@ func UpdateUserTheme(u *User, themeName string) error {
203203
return UpdateUserCols(db.DefaultContext, u, "theme")
204204
}
205205

206+
// GetPlaceholderEmail returns an noreply email
207+
func (u *User) GetPlaceholderEmail() string {
208+
return fmt.Sprintf("%s@%s", u.LowerName, setting.Service.NoReplyAddress)
209+
}
210+
206211
// GetEmail returns an noreply email, if the user has set to keep his
207212
// email address private, otherwise the primary email address.
208213
func (u *User) GetEmail() string {
209214
if u.KeepEmailPrivate {
210-
return fmt.Sprintf("%s@%s", u.LowerName, setting.Service.NoReplyAddress)
215+
return u.GetPlaceholderEmail()
211216
}
212217
return u.Email
213218
}

services/convert/user.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func toUser(ctx context.Context, user *user_model.User, signed, authed bool) *ap
5151
ID: user.ID,
5252
UserName: user.Name,
5353
FullName: user.FullName,
54-
Email: user.GetEmail(),
54+
Email: user.GetPlaceholderEmail(),
5555
AvatarURL: user.AvatarLink(ctx),
5656
Created: user.CreatedUnix.AsTime(),
5757
Restricted: user.IsRestricted,

tests/integration/api_user_info_test.go

+16
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99
"testing"
1010

1111
auth_model "code.gitea.io/gitea/models/auth"
12+
"code.gitea.io/gitea/models/unittest"
13+
user_model "code.gitea.io/gitea/models/user"
1214
api "code.gitea.io/gitea/modules/structs"
1315
"code.gitea.io/gitea/tests"
1416

@@ -21,6 +23,8 @@ func TestAPIUserInfo(t *testing.T) {
2123
user := "user1"
2224
user2 := "user31"
2325

26+
user3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user3"})
27+
2428
session := loginUser(t, user)
2529
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadUser)
2630

@@ -36,6 +40,18 @@ func TestAPIUserInfo(t *testing.T) {
3640

3741
req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/users/%s", user2))
3842
MakeRequest(t, req, http.StatusNotFound)
43+
44+
// test if the placaholder Mail is returned if a User is not logged in
45+
req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/users/%s", user3.Name))
46+
resp = MakeRequest(t, req, http.StatusOK)
47+
DecodeJSON(t, resp, &u)
48+
assert.Equal(t, user3.GetPlaceholderEmail(), u.Email)
49+
50+
// Test if the correct Mail is returned if a User is logged in
51+
req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/users/%s?token=%s", user3.Name, token))
52+
resp = MakeRequest(t, req, http.StatusOK)
53+
DecodeJSON(t, resp, &u)
54+
assert.Equal(t, user3.GetEmail(), u.Email)
3955
})
4056

4157
t.Run("GetAuthenticatedUser", func(t *testing.T) {

tests/integration/api_user_search_test.go

+1-7
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,12 @@
44
package integration
55

66
import (
7-
"fmt"
87
"net/http"
98
"testing"
109

1110
auth_model "code.gitea.io/gitea/models/auth"
1211
"code.gitea.io/gitea/models/unittest"
1312
user_model "code.gitea.io/gitea/models/user"
14-
"code.gitea.io/gitea/modules/setting"
1513
api "code.gitea.io/gitea/modules/structs"
1614
"code.gitea.io/gitea/tests"
1715

@@ -54,11 +52,7 @@ func TestAPIUserSearchNotLoggedIn(t *testing.T) {
5452
for _, user := range results.Data {
5553
assert.Contains(t, user.UserName, query)
5654
modelUser = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: user.ID})
57-
if modelUser.KeepEmailPrivate {
58-
assert.EqualValues(t, fmt.Sprintf("%s@%s", modelUser.LowerName, setting.Service.NoReplyAddress), user.Email)
59-
} else {
60-
assert.EqualValues(t, modelUser.Email, user.Email)
61-
}
55+
assert.EqualValues(t, modelUser.GetPlaceholderEmail(), user.Email)
6256
}
6357
}
6458

0 commit comments

Comments
 (0)