Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API: UserID is missing sometimes #12846

Closed
6543 opened this issue Sep 15, 2020 · 11 comments · Fixed by #12855
Closed

API: UserID is missing sometimes #12846

6543 opened this issue Sep 15, 2020 · 11 comments · Fixed by #12855
Labels
modifies/api This PR adds API routes or modifies them type/bug
Milestone

Comments

@6543
Copy link
Member

6543 commented Sep 15, 2020

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 !!

@6543 6543 added modifies/api This PR adds API routes or modifies them type/bug labels Sep 15, 2020
@a1012112796
Copy link
Member

It's not a bug :)

// 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
Copy link
Member Author

6543 commented Sep 15, 2020

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

@6543
Copy link
Member Author

6543 commented Sep 15, 2020

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
Copy link
Member Author

6543 commented Sep 15, 2020

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

@a1012112796
Copy link
Member

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

@a1012112796
Copy link
Member

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
Copy link
Member

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

@6543 6543 added this to the 1.13.0 milestone Sep 15, 2020
@a1012112796
Copy link
Member

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 :(
tmp

@6543

This comment has been minimized.

@6543
Copy link
Member Author

6543 commented Sep 15, 2020

@a1012112796 I have created #12855 to fix this issue

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

@6543 6543 changed the title API: ListPullReviews API: UserID is missing sometimes Sep 16, 2020
@6543
Copy link
Member Author

6543 commented Sep 27, 2020

@a1012112796 the refactor for User -> API-User convert refactor is ready :)

-> #12856

@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
modifies/api This PR adds API routes or modifies them type/bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants