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>
This commit is contained in:
		
							parent
							
								
									7dc2e50113
								
							
						
					
					
						commit
						ea385f5d39
					
				|  | @ -203,11 +203,16 @@ func UpdateUserTheme(u *User, themeName string) error { | ||||||
| 	return UpdateUserCols(db.DefaultContext, u, "theme") | 	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
 | // GetEmail returns an noreply email, if the user has set to keep his
 | ||||||
| // email address private, otherwise the primary email address.
 | // email address private, otherwise the primary email address.
 | ||||||
| func (u *User) GetEmail() string { | func (u *User) GetEmail() string { | ||||||
| 	if u.KeepEmailPrivate { | 	if u.KeepEmailPrivate { | ||||||
| 		return fmt.Sprintf("%s@%s", u.LowerName, setting.Service.NoReplyAddress) | 		return u.GetPlaceholderEmail() | ||||||
| 	} | 	} | ||||||
| 	return u.Email | 	return u.Email | ||||||
| } | } | ||||||
|  |  | ||||||
|  | @ -51,7 +51,7 @@ func toUser(ctx context.Context, user *user_model.User, signed, authed bool) *ap | ||||||
| 		ID:          user.ID, | 		ID:          user.ID, | ||||||
| 		UserName:    user.Name, | 		UserName:    user.Name, | ||||||
| 		FullName:    user.FullName, | 		FullName:    user.FullName, | ||||||
| 		Email:       user.GetEmail(), | 		Email:       user.GetPlaceholderEmail(), | ||||||
| 		AvatarURL:   user.AvatarLink(ctx), | 		AvatarURL:   user.AvatarLink(ctx), | ||||||
| 		Created:     user.CreatedUnix.AsTime(), | 		Created:     user.CreatedUnix.AsTime(), | ||||||
| 		Restricted:  user.IsRestricted, | 		Restricted:  user.IsRestricted, | ||||||
|  |  | ||||||
|  | @ -9,6 +9,8 @@ import ( | ||||||
| 	"testing" | 	"testing" | ||||||
| 
 | 
 | ||||||
| 	auth_model "code.gitea.io/gitea/models/auth" | 	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" | 	api "code.gitea.io/gitea/modules/structs" | ||||||
| 	"code.gitea.io/gitea/tests" | 	"code.gitea.io/gitea/tests" | ||||||
| 
 | 
 | ||||||
|  | @ -21,6 +23,8 @@ func TestAPIUserInfo(t *testing.T) { | ||||||
| 	user := "user1" | 	user := "user1" | ||||||
| 	user2 := "user31" | 	user2 := "user31" | ||||||
| 
 | 
 | ||||||
|  | 	user3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user3"}) | ||||||
|  | 
 | ||||||
| 	session := loginUser(t, user) | 	session := loginUser(t, user) | ||||||
| 	token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadUser) | 	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)) | 		req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/users/%s", user2)) | ||||||
| 		MakeRequest(t, req, http.StatusNotFound) | 		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) { | 	t.Run("GetAuthenticatedUser", func(t *testing.T) { | ||||||
|  |  | ||||||
|  | @ -4,14 +4,12 @@ | ||||||
| package integration | package integration | ||||||
| 
 | 
 | ||||||
| import ( | import ( | ||||||
| 	"fmt" |  | ||||||
| 	"net/http" | 	"net/http" | ||||||
| 	"testing" | 	"testing" | ||||||
| 
 | 
 | ||||||
| 	auth_model "code.gitea.io/gitea/models/auth" | 	auth_model "code.gitea.io/gitea/models/auth" | ||||||
| 	"code.gitea.io/gitea/models/unittest" | 	"code.gitea.io/gitea/models/unittest" | ||||||
| 	user_model "code.gitea.io/gitea/models/user" | 	user_model "code.gitea.io/gitea/models/user" | ||||||
| 	"code.gitea.io/gitea/modules/setting" |  | ||||||
| 	api "code.gitea.io/gitea/modules/structs" | 	api "code.gitea.io/gitea/modules/structs" | ||||||
| 	"code.gitea.io/gitea/tests" | 	"code.gitea.io/gitea/tests" | ||||||
| 
 | 
 | ||||||
|  | @ -54,11 +52,7 @@ func TestAPIUserSearchNotLoggedIn(t *testing.T) { | ||||||
| 	for _, user := range results.Data { | 	for _, user := range results.Data { | ||||||
| 		assert.Contains(t, user.UserName, query) | 		assert.Contains(t, user.UserName, query) | ||||||
| 		modelUser = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: user.ID}) | 		modelUser = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: user.ID}) | ||||||
| 		if modelUser.KeepEmailPrivate { | 		assert.EqualValues(t, modelUser.GetPlaceholderEmail(), user.Email) | ||||||
| 			assert.EqualValues(t, fmt.Sprintf("%s@%s", modelUser.LowerName, setting.Service.NoReplyAddress), user.Email) |  | ||||||
| 		} else { |  | ||||||
| 			assert.EqualValues(t, modelUser.Email, user.Email) |  | ||||||
| 		} |  | ||||||
| 	} | 	} | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue