Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix API leaking Usermail if not logged in #25097

Merged
merged 7 commits into from
Jul 31, 2023
Merged

Fix API leaking Usermail if not logged in #25097

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0220f5e
Fix API leaking Usermail if not logged in
JakobDev Jun 6, 2023
585483f
Merge branch 'main' into usermail
silverwind Jul 26, 2023
41f5c43
Fix TestAPIUserSearchNotLoggedIn()
JakobDev Jul 27, 2023
c60354f
Remove imports
JakobDev Jul 27, 2023
8dc13c6
Remove if
JakobDev Jul 27, 2023
96dcb6e
Merge branch 'main' into usermail
GiteaBot Jul 30, 2023
16245ec
Merge branch 'main' into usermail
GiteaBot Jul 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion models/user/user.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion services/convert/user.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we need to check if signed is true?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file already cntians this code down below which does the check:

// hide primary email if API caller is anonymous or user keep email private
if signed && (!user.KeepEmailPrivate || authed) {
	result.Email = user.Email
}

As user.GetMail() only checks if the Mail is private, but not if someone is logged in, it always returns the Mail if it is not private, so this Check hasn't worked. Now that Email is set to the Placeholder before this Check, it is now working.

AvatarURL: user.AvatarLink(ctx),
Created: user.CreatedUnix.AsTime(),
Restricted: user.IsRestricted,
Expand Down
16 changes: 16 additions & 0 deletions tests/integration/api_user_info_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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)

Expand All @@ -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) {
Expand Down
8 changes: 1 addition & 7 deletions tests/integration/api_user_search_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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)
}
}

Expand Down