fix wrong email when use gitea as OAuth2 provider (#7640) (#7647)

renothing · lafriks · commit d789170e315f · 2019-07-27T17:25:16.000+03:00
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.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -91,8 +91,7 @@ func CreateUser(ctx *context.APIContext, form api.CreateUserOption) {`
`91`	`91`	`if form.SendNotify && setting.MailService != nil {`
`92`	`92`	`models.SendRegisterNotifyMail(ctx.Context.Context, u)`
`93`	`93`	`}`
`94`		`-`
`95`		`- ctx.JSON(201, u.APIFormat())`
	`94`	`+ ctx.JSON(201, convert.ToUser(u, ctx.IsSigned, ctx.User.IsAdmin))`
`96`	`95`	`}`
`97`	`96`
`98`	`97`	`// EditUser api for modifying a user's information`
`@@ -181,7 +180,7 @@ func EditUser(ctx *context.APIContext, form api.EditUserOption) {`
`181`	`180`	`}`
`182`	`181`	`log.Trace("Account profile updated by admin (%s): %s", ctx.User.Name, u.Name)`
`183`	`182`
`184`		`- ctx.JSON(200, u.APIFormat())`
	`183`	`+ ctx.JSON(200, convert.ToUser(u, ctx.IsSigned, ctx.User.IsAdmin))`
`185`	`184`	`}`
`186`	`185`
`187`	`186`	`// DeleteUser api for deleting a user`
`@@ -326,7 +325,7 @@ func GetAllUsers(ctx *context.APIContext) {`
`326`	`325`
`327`	`326`	`results := make([]*api.User, len(users))`
`328`	`327`	`for i := range users {`
`329`		`- results[i] = convert.ToUser(users[i], ctx.IsSigned, ctx.User != nil && ctx.User.IsAdmin)`
	`328`	`+ results[i] = convert.ToUser(users[i], ctx.IsSigned, ctx.User.IsAdmin)`
`330`	`329`	`}`
`331`	`330`
`332`	`331`	`ctx.JSON(200, &results)`
Original file line number	Diff line number	Diff line change
`@@ -12,6 +12,7 @@ import (`
`12`	`12`	`"code.gitea.io/gitea/models"`
`13`	`13`	`"code.gitea.io/gitea/modules/context"`
`14`	`14`	`"code.gitea.io/gitea/modules/setting"`
	`15`	`+ "code.gitea.io/gitea/routers/api/v1/convert"`
`15`	`16`	`"code.gitea.io/gitea/routers/api/v1/user"`
`16`	`17`	`)`
`17`	`18`
`@@ -46,7 +47,7 @@ func listMembers(ctx *context.APIContext, publicOnly bool) {`
`46`	`47`
`47`	`48`	`apiMembers := make([]*api.User, len(members))`
`48`	`49`	`for i, member := range members {`
`49`		`- apiMembers[i] = member.APIFormat()`
	`50`	`+ apiMembers[i] = convert.ToUser(member, ctx.IsSigned, ctx.User != nil && ctx.User.IsAdmin)`
`50`	`51`	`}`
`51`	`52`	`ctx.JSON(200, apiMembers)`
`52`	`53`	`}`
Original file line number	Diff line number	Diff line change
`@@ -257,7 +257,7 @@ func GetTeamMembers(ctx *context.APIContext) {`
`257`	`257`	`}`
`258`	`258`	`members := make([]*api.User, len(team.Members))`
`259`	`259`	`for i, member := range team.Members {`
`260`		`- members[i] = member.APIFormat()`
	`260`	`+ members[i] = convert.ToUser(member, ctx.IsSigned, ctx.User.IsAdmin)`
`261`	`261`	`}`
`262`	`262`	`ctx.JSON(200, members)`
`263`	`263`	`}`
`@@ -288,7 +288,7 @@ func GetTeamMember(ctx *context.APIContext) {`
`288`	`288`	`if ctx.Written() {`
`289`	`289`	`return`
`290`	`290`	`}`
`291`		`- ctx.JSON(200, u.APIFormat())`
	`291`	`+ ctx.JSON(200, convert.ToUser(u, ctx.IsSigned, ctx.User.IsAdmin))`
`292`	`292`	`}`
`293`	`293`
`294`	`294`	`// AddTeamMember api for add a member to a team`
Original file line number	Diff line number	Diff line change
`@@ -8,6 +8,7 @@ import (`
`8`	`8`	`"code.gitea.io/gitea/modules/context"`
`9`	`9`
`10`	`10`	`api "code.gitea.io/gitea/modules/structs"`
	`11`	`+ "code.gitea.io/gitea/routers/api/v1/convert"`
`11`	`12`	`)`
`12`	`13`
`13`	`14`	`// ListStargazers list a repository's stargazers`
`@@ -38,7 +39,7 @@ func ListStargazers(ctx *context.APIContext) {`
`38`	`39`	`}`
`39`	`40`	`users := make([]*api.User, len(stargazers))`
`40`	`41`	`for i, stargazer := range stargazers {`
`41`		`- users[i] = stargazer.APIFormat()`
	`42`	`+ users[i] = convert.ToUser(stargazer, ctx.IsSigned, ctx.User != nil && ctx.User.IsAdmin)`
`42`	`43`	`}`
`43`	`44`	`ctx.JSON(200, users)`
`44`	`45`	`}`
Original file line number	Diff line number	Diff line change
`@@ -9,12 +9,13 @@ import (`
`9`	`9`
`10`	`10`	`"code.gitea.io/gitea/models"`
`11`	`11`	`"code.gitea.io/gitea/modules/context"`
	`12`	`+ "code.gitea.io/gitea/routers/api/v1/convert"`
`12`	`13`	`)`
`13`	`14`
`14`	`15`	`func responseAPIUsers(ctx context.APIContext, users []models.User) {`
`15`	`16`	`apiUsers := make([]*api.User, len(users))`
`16`	`17`	`for i := range users {`
`17`		`- apiUsers[i] = users[i].APIFormat()`
	`18`	`+ apiUsers[i] = convert.ToUser(users[i], ctx.IsSigned, ctx.User != nil && ctx.User.IsAdmin)`
`18`	`19`	`}`
`19`	`20`	`ctx.JSON(200, &apiUsers)`
`20`	`21`	`}`