From cbf231a67595feb36fccb68dc00b2bc7607fa882 Mon Sep 17 00:00:00 2001
From: renothing <261274+renothing@users.noreply.github.com>
Date: Sat, 27 Jul 2019 21:15:30 +0800
Subject: [PATCH] fix wrong email when use gitea as OAuth2 provider (#7640)

when you use gitea as OAuth2 provider, the /api/v1/user should return
user primary email as identifier, which is unique in OAuth2 clients.

this patch use convert.ToUser replace all u.APIFormat in api requests,
return primary email when caller is yourself or admin.
---
 models/user.go                       | 8 ++++----
 routers/api/v1/admin/user.go         | 7 +++----
 routers/api/v1/convert/convert.go    | 9 +++++++--
 routers/api/v1/org/member.go         | 3 ++-
 routers/api/v1/org/team.go           | 4 ++--
 routers/api/v1/repo/collaborators.go | 3 ++-
 routers/api/v1/repo/hook.go          | 4 ++--
 routers/api/v1/repo/star.go          | 3 ++-
 routers/api/v1/repo/subscriber.go    | 3 ++-
 routers/api/v1/user/follower.go      | 3 ++-
 routers/api/v1/user/key.go           | 4 ++--
 routers/api/v1/user/user.go          | 8 ++------
 12 files changed, 32 insertions(+), 27 deletions(-)

diff --git a/models/user.go b/models/user.go
index aa3e527cd5..1f684a5940 100644
--- a/models/user.go
+++ b/models/user.go
@@ -204,9 +204,9 @@ func (u *User) UpdateTheme(themeName string) error {
 	return UpdateUserCols(u, "theme")
 }
 
-// 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.
-func (u *User) getEmail() string {
+func (u *User) GetEmail() string {
 	if u.KeepEmailPrivate {
 		return fmt.Sprintf("%s@%s", u.LowerName, setting.Service.NoReplyAddress)
 	}
@@ -219,7 +219,7 @@ func (u *User) APIFormat() *api.User {
 		ID:        u.ID,
 		UserName:  u.Name,
 		FullName:  u.FullName,
-		Email:     u.getEmail(),
+		Email:     u.GetEmail(),
 		AvatarURL: u.AvatarLink(),
 		Language:  u.Language,
 		IsAdmin:   u.IsAdmin,
@@ -434,7 +434,7 @@ func (u *User) GetFollowing(page int) ([]*User, error) {
 func (u *User) NewGitSig() *git.Signature {
 	return &git.Signature{
 		Name:  u.GitName(),
-		Email: u.getEmail(),
+		Email: u.GetEmail(),
 		When:  time.Now(),
 	}
 }
diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go
index 6dc3b0325b..f4b694aa22 100644
--- a/routers/api/v1/admin/user.go
+++ b/routers/api/v1/admin/user.go
@@ -91,8 +91,7 @@ func CreateUser(ctx *context.APIContext, form api.CreateUserOption) {
 	if form.SendNotify && setting.MailService != nil {
 		models.SendRegisterNotifyMail(ctx.Context.Context, u)
 	}
-
-	ctx.JSON(201, u.APIFormat())
+	ctx.JSON(201, convert.ToUser(u, ctx.IsSigned, ctx.User.IsAdmin))
 }
 
 // EditUser api for modifying a user's information
@@ -181,7 +180,7 @@ func EditUser(ctx *context.APIContext, form api.EditUserOption) {
 	}
 	log.Trace("Account profile updated by admin (%s): %s", ctx.User.Name, u.Name)
 
-	ctx.JSON(200, u.APIFormat())
+	ctx.JSON(200, convert.ToUser(u, ctx.IsSigned, ctx.User.IsAdmin))
 }
 
 // DeleteUser api for deleting a user
@@ -326,7 +325,7 @@ func GetAllUsers(ctx *context.APIContext) {
 
 	results := make([]*api.User, len(users))
 	for i := range users {
-		results[i] = convert.ToUser(users[i], ctx.IsSigned, ctx.User != nil && ctx.User.IsAdmin)
+		results[i] = convert.ToUser(users[i], ctx.IsSigned, ctx.User.IsAdmin)
 	}
 
 	ctx.JSON(200, &results)
diff --git a/routers/api/v1/convert/convert.go b/routers/api/v1/convert/convert.go
index f1cb23de43..d2691f8238 100644
--- a/routers/api/v1/convert/convert.go
+++ b/routers/api/v1/convert/convert.go
@@ -229,7 +229,7 @@ func ToTeam(team *models.Team) *api.Team {
 }
 
 // ToUser convert models.User to api.User
-func ToUser(user *models.User, signed, admin bool) *api.User {
+func ToUser(user *models.User, signed, authed bool) *api.User {
 	result := &api.User{
 		ID:        user.ID,
 		UserName:  user.Name,
@@ -239,7 +239,12 @@ func ToUser(user *models.User, signed, admin bool) *api.User {
 		LastLogin: user.LastLoginUnix.AsTime(),
 		Created:   user.CreatedUnix.AsTime(),
 	}
-	if signed && (!user.KeepEmailPrivate || admin) {
+	// hide primary email if API caller isn't user itself or an admin
+	if !signed {
+		result.Email = ""
+	} else if user.KeepEmailPrivate && !authed {
+		result.Email = user.GetEmail()
+	} else {
 		result.Email = user.Email
 	}
 	return result
diff --git a/routers/api/v1/org/member.go b/routers/api/v1/org/member.go
index ad60dfbda5..4ada2d6ef6 100644
--- a/routers/api/v1/org/member.go
+++ b/routers/api/v1/org/member.go
@@ -12,6 +12,7 @@ import (
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/context"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/routers/api/v1/convert"
 	"code.gitea.io/gitea/routers/api/v1/user"
 )
 
@@ -46,7 +47,7 @@ func listMembers(ctx *context.APIContext, publicOnly bool) {
 
 	apiMembers := make([]*api.User, len(members))
 	for i, member := range members {
-		apiMembers[i] = member.APIFormat()
+		apiMembers[i] = convert.ToUser(member, ctx.IsSigned, ctx.User != nil && ctx.User.IsAdmin)
 	}
 	ctx.JSON(200, apiMembers)
 }
diff --git a/routers/api/v1/org/team.go b/routers/api/v1/org/team.go
index 40b6e008b9..59a000b670 100644
--- a/routers/api/v1/org/team.go
+++ b/routers/api/v1/org/team.go
@@ -257,7 +257,7 @@ func GetTeamMembers(ctx *context.APIContext) {
 	}
 	members := make([]*api.User, len(team.Members))
 	for i, member := range team.Members {
-		members[i] = member.APIFormat()
+		members[i] = convert.ToUser(member, ctx.IsSigned, ctx.User.IsAdmin)
 	}
 	ctx.JSON(200, members)
 }
@@ -288,7 +288,7 @@ func GetTeamMember(ctx *context.APIContext) {
 	if ctx.Written() {
 		return
 	}
-	ctx.JSON(200, u.APIFormat())
+	ctx.JSON(200, convert.ToUser(u, ctx.IsSigned, ctx.User.IsAdmin))
 }
 
 // AddTeamMember api for add a member to a team
diff --git a/routers/api/v1/repo/collaborators.go b/routers/api/v1/repo/collaborators.go
index 98daf2c9d5..3ba03e054c 100644
--- a/routers/api/v1/repo/collaborators.go
+++ b/routers/api/v1/repo/collaborators.go
@@ -12,6 +12,7 @@ import (
 	"code.gitea.io/gitea/modules/context"
 
 	api "code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/routers/api/v1/convert"
 )
 
 // ListCollaborators list a repository's collaborators
@@ -42,7 +43,7 @@ func ListCollaborators(ctx *context.APIContext) {
 	}
 	users := make([]*api.User, len(collaborators))
 	for i, collaborator := range collaborators {
-		users[i] = collaborator.APIFormat()
+		users[i] = convert.ToUser(collaborator.User, ctx.IsSigned, ctx.User != nil && ctx.User.IsAdmin)
 	}
 	ctx.JSON(200, users)
 }
diff --git a/routers/api/v1/repo/hook.go b/routers/api/v1/repo/hook.go
index b35a10ac3d..5c71262560 100644
--- a/routers/api/v1/repo/hook.go
+++ b/routers/api/v1/repo/hook.go
@@ -130,8 +130,8 @@ func TestHook(ctx *context.APIContext) {
 			convert.ToCommit(ctx.Repo.Repository, ctx.Repo.Commit),
 		},
 		Repo:   ctx.Repo.Repository.APIFormat(models.AccessModeNone),
-		Pusher: ctx.User.APIFormat(),
-		Sender: ctx.User.APIFormat(),
+		Pusher: convert.ToUser(ctx.User, ctx.IsSigned, false),
+		Sender: convert.ToUser(ctx.User, ctx.IsSigned, false),
 	}); err != nil {
 		ctx.Error(500, "PrepareWebhook: ", err)
 		return
diff --git a/routers/api/v1/repo/star.go b/routers/api/v1/repo/star.go
index 046142252b..1b2ef0b027 100644
--- a/routers/api/v1/repo/star.go
+++ b/routers/api/v1/repo/star.go
@@ -8,6 +8,7 @@ import (
 	"code.gitea.io/gitea/modules/context"
 
 	api "code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/routers/api/v1/convert"
 )
 
 // ListStargazers list a repository's stargazers
@@ -38,7 +39,7 @@ func ListStargazers(ctx *context.APIContext) {
 	}
 	users := make([]*api.User, len(stargazers))
 	for i, stargazer := range stargazers {
-		users[i] = stargazer.APIFormat()
+		users[i] = convert.ToUser(stargazer, ctx.IsSigned, ctx.User != nil && ctx.User.IsAdmin)
 	}
 	ctx.JSON(200, users)
 }
diff --git a/routers/api/v1/repo/subscriber.go b/routers/api/v1/repo/subscriber.go
index b7d329dc78..ec0ea6dada 100644
--- a/routers/api/v1/repo/subscriber.go
+++ b/routers/api/v1/repo/subscriber.go
@@ -8,6 +8,7 @@ import (
 	"code.gitea.io/gitea/modules/context"
 
 	api "code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/routers/api/v1/convert"
 )
 
 // ListSubscribers list a repo's subscribers (i.e. watchers)
@@ -38,7 +39,7 @@ func ListSubscribers(ctx *context.APIContext) {
 	}
 	users := make([]*api.User, len(subscribers))
 	for i, subscriber := range subscribers {
-		users[i] = subscriber.APIFormat()
+		users[i] = convert.ToUser(subscriber, ctx.IsSigned, ctx.User != nil && ctx.User.IsAdmin)
 	}
 	ctx.JSON(200, users)
 }
diff --git a/routers/api/v1/user/follower.go b/routers/api/v1/user/follower.go
index 453f73137d..078f30af3c 100644
--- a/routers/api/v1/user/follower.go
+++ b/routers/api/v1/user/follower.go
@@ -9,12 +9,13 @@ import (
 
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/context"
+	"code.gitea.io/gitea/routers/api/v1/convert"
 )
 
 func responseAPIUsers(ctx *context.APIContext, users []*models.User) {
 	apiUsers := make([]*api.User, len(users))
 	for i := range users {
-		apiUsers[i] = users[i].APIFormat()
+		apiUsers[i] = convert.ToUser(users[i], ctx.IsSigned, ctx.User != nil && ctx.User.IsAdmin)
 	}
 	ctx.JSON(200, &apiUsers)
 }
diff --git a/routers/api/v1/user/key.go b/routers/api/v1/user/key.go
index 286f9ae4c3..e3d7aa4b3e 100644
--- a/routers/api/v1/user/key.go
+++ b/routers/api/v1/user/key.go
@@ -22,13 +22,13 @@ func appendPrivateInformation(apiKey *api.PublicKey, key *models.PublicKey, defa
 		apiKey.KeyType = "user"
 
 		if defaultUser.ID == key.OwnerID {
-			apiKey.Owner = defaultUser.APIFormat()
+			apiKey.Owner = convert.ToUser(defaultUser, true, true)
 		} else {
 			user, err := models.GetUserByID(key.OwnerID)
 			if err != nil {
 				return apiKey, err
 			}
-			apiKey.Owner = user.APIFormat()
+			apiKey.Owner = convert.ToUser(user, true, true)
 		}
 	} else {
 		apiKey.KeyType = "unknown"
diff --git a/routers/api/v1/user/user.go b/routers/api/v1/user/user.go
index 8d05a67185..fc3b7a8160 100644
--- a/routers/api/v1/user/user.go
+++ b/routers/api/v1/user/user.go
@@ -104,11 +104,7 @@ func GetInfo(ctx *context.APIContext) {
 		return
 	}
 
-	// Hide user e-mail when API caller isn't signed in.
-	if !ctx.IsSigned {
-		u.Email = ""
-	}
-	ctx.JSON(200, u.APIFormat())
+	ctx.JSON(200, convert.ToUser(u, ctx.IsSigned, ctx.User.ID == u.ID || ctx.User.IsAdmin))
 }
 
 // GetAuthenticatedUser get current user's information
@@ -121,7 +117,7 @@ func GetAuthenticatedUser(ctx *context.APIContext) {
 	// responses:
 	//   "200":
 	//     "$ref": "#/responses/User"
-	ctx.JSON(200, ctx.User.APIFormat())
+	ctx.JSON(200, convert.ToUser(ctx.User, ctx.IsSigned, ctx.User != nil))
 }
 
 // GetUserHeatmapData is the handler to get a users heatmap