API: UserID is missing sometimes

[6543]

sometimes UserID is missing!!

https://gitea.com/api/swagger#/repository/repoListPullReviews

example:

GET https://gitea.com/api/v1/repos/gitea/test_repo/pulls/7/reviews -> techknowlogick has ID 0 !!

[a1012112796]

It's not a bug :)

gitea/modules/convert/convert.go

Lines 347 to 354 in 08a905f

	// only site admin will get these information and possibly user himself
	if authed {
	result.ID = user.ID
	result.IsAdmin = user.IsAdmin
	result.LastLogin = user.LastLoginUnix.AsTime()
	result.Language = user.Language
	}
	return result

[6543]

other endpoint's return it and it is needed for migration - inconsistency is a bug

[6543]

I already know where the issue is - we have 2 differend function to convert a user into an api-user -> bad thing!!!

but didn't had time to refactor all of it

[6543]

@a1012112796 just use https://gitea.com/api/swagger#/repository/repoListPullRequests and you have the ID (9)

[a1012112796]

Hmm, Maybe the id of user is not an secret message.

[a1012112796]

found it :)

gitea/models/user.go

Lines 241 to 257 in 08a905f

	// APIFormat converts a User to api.User
	func (u *User) APIFormat() *api.User {
	if u == nil {
	return nil
	}
	return &api.User{
	ID: u.ID,
	UserName: u.Name,
	FullName: u.FullName,
	Email: u.GetEmail(),
	AvatarURL: u.AvatarLink(),
	Language: u.Language,
	IsAdmin: u.IsAdmin,
	LastLogin: u.LastLoginUnix.AsTime(),
	Created: u.CreatedUnix.AsTime(),
	}
	}

[a1012112796]

So the main problem is whether the id of user should be protected?

[a1012112796]

Chage Idea:

diff --git a/models/user.go b/models/user.go
index c7b3f0981..08237b2cb 100644
--- a/models/user.go
+++ b/models/user.go
@@ -239,21 +239,34 @@ func (u *User) GetEmail() string {
 }
 
 // APIFormat converts a User to api.User
-func (u *User) APIFormat() *api.User {
+func (u *User) APIFormat(doer *User) *api.User {
 	if u == nil {
 		return nil
 	}
-	return &api.User{
-		ID:        u.ID,
+
+	result := &api.User{
 		UserName:  u.Name,
 		FullName:  u.FullName,
-		Email:     u.GetEmail(),
 		AvatarURL: u.AvatarLink(),
 		Language:  u.Language,
-		IsAdmin:   u.IsAdmin,
-		LastLogin: u.LastLoginUnix.AsTime(),
 		Created:   u.CreatedUnix.AsTime(),
 	}
+
+	signed := doer != nil
+	authed := doer != nil && (doer.IsAdmin || u.ID == doer.ID)
+
+	// hide primary email if API caller is anonymous or user keep email private
+	if signed && (!u.KeepEmailPrivate || authed) {
+		result.Email = u.Email
+	}
+	// only site admin will get these information and possibly user himself
+	if authed {
+		result.ID = u.ID
+		result.IsAdmin = u.IsAdmin
+		result.LastLogin = u.LastLoginUnix.AsTime()
+		result.Language = u.Language
+	}
+	return result
 }
 
 // IsLocal returns true if user login type is LoginPlain.

Then will face a big work :(

[6543]

there are more .APIFormat() but only around 64 for user :D
and yes it has to be refactored to use convert package

[6543]

@a1012112796 I have created #12855 to fix this issue

and #12856 witch wont get into v1.13 who will remove APIFormat ...