Skip to content

Commit

Permalink
fix wrong email when use gitea as OAuth2 provider (#7640)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
renothing authored and lafriks committed Jul 27, 2019
1 parent 700cd34 commit cbf231a
Show file tree
Hide file tree
Showing 12 changed files with 32 additions and 27 deletions.
8 changes: 4 additions & 4 deletions models/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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,
Expand Down Expand Up @@ -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(),
}
}
Expand Down
7 changes: 3 additions & 4 deletions routers/api/v1/admin/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
9 changes: 7 additions & 2 deletions routers/api/v1/convert/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion routers/api/v1/org/member.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions routers/api/v1/org/team.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion routers/api/v1/repo/collaborators.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions routers/api/v1/repo/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion routers/api/v1/repo/star.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
3 changes: 2 additions & 1 deletion routers/api/v1/repo/subscriber.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
3 changes: 2 additions & 1 deletion routers/api/v1/user/follower.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions routers/api/v1/user/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
8 changes: 2 additions & 6 deletions routers/api/v1/user/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit cbf231a

Please sign in to comment.