From 37656cec801f6c4cec5f7c041a53de3043a5e3ad 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 793cebbf561ae..e48accf02daba 100644
--- a/models/user.go
+++ b/models/user.go
@@ -195,9 +195,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)
 	}
@@ -210,7 +210,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,
@@ -425,7 +425,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 6dc3b0325b1e9..f4b694aa22879 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 f1cb23de4378f..d2691f8238086 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 ad60dfbda5a32..4ada2d6ef6885 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 40b6e008b9db4..59a000b67096d 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 98daf2c9d5ca2..3ba03e054cf5f 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 b35a10ac3d07e..5c71262560bcb 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 046142252b4ad..1b2ef0b027d25 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 b7d329dc7808b..ec0ea6dadab01 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 453f73137d96d..078f30af3ccb8 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 286f9ae4c3493..e3d7aa4b3e10b 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 8d05a671850ab..fc3b7a816036c 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