-
Notifications
You must be signed in to change notification settings - Fork 92
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
Authorization issue when running graphql including repository collaborators via this lib #72
Comments
That is unexpected.
Yeah, it's a good idea to confirm the generated query matches what you intended to write. There isn't a public API to do it yet, but in the meantime you can either make changes to // teeRoundTripper copies request bodies to stdout.
type teeRoundTripper struct {
http.RoundTripper
}
func (t teeRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
if req.Body != nil {
req.Body = struct {
io.Reader
io.Closer
}{
Reader: io.TeeReader(req.Body, os.Stdout),
Closer: req.Body,
}
}
return t.RoundTripper.RoundTrip(req)
} (See https://play.golang.org/p/jAKBW46BdPh for a complete snippet.) |
I used mitm proxy and this is what I intercepted. {
"query": "query($owner:String!$repoCursor:String){organization(login: $owner){repositories(first: 100, after: $repoCursor){totalCount,pageInfo{hasNextPage,endCursor},nodes{id,name,description,url,sshUrl,owner{login},isPrivate,createdAt,updatedAt,pushedAt,repositoryTopics(first: 25){nodes{topic{name},resourcePath}},collaborators(first: 15){totalCount,pageInfo{hasNextPage,endCursor},nodes{name,login,avatarUrl}}}}}}",
"variables": {
"owner": "philips-labs",
"repoCursor": null
}
} Cleaning up that graphql with some indentation looks like this. query($owner:String!$repoCursor:String) {
organization(login: $owner) {
repositories(first: 100, after: $repoCursor) {
totalCount,
pageInfo {
hasNextPage,
endCursor
},
nodes {
id,
name,
description,
url,
sshUrl,
owner {
login
},
isPrivate,
createdAt,
updatedAt,
pushedAt,
repositoryTopics(first: 25) {
nodes{
topic{name},
resourcePath
}
},
collaborators(first: 15) {
totalCount,
pageInfo{ hasNextPage, endCursor },
nodes{name,login,avatarUrl}
}
}
}
}
} I can also confirm I'm getting the data back as expected. So it seems to be something in the lib that is doing some check or magic somewhere after getting the response causing that error regarding token push permissions.
{
"data": {
"organization": {
"repositories": {
"nodes": [
{
"collaborators": {
"nodes": [
{
"avatarUrl": "https://avatars3.githubusercontent.com/u/694733?u=6aeb327c48cb88ae31eb88e680b96228f53cae51&v=4",
"login": "marcofranssen",
"name": "Marco Franssen"
}, Any clue if this can be easily fixed? @dmitshur |
I'm quite confident the "Must have push access to view repository collaborators." error is coming from the GitHub API server, it's not in Did you check if the response has an "errors" field set? If so, maybe the GitHub response contains partial data and also an error. The behavior of graphql and githubv4 is to populate any partial data, and return the first error from "errors" field in the GraphQL response. See here. |
Let me check that. How should I deal with that partial response from the lib if that is the case? What would you recommend? |
You where right, "errors": [
{
"locations": [
{
"column": 307,
"line": 1
}
],
"message": "Must have push access to view repository collaborators.",
"path": [
"organization",
"repositories",
"nodes",
34,
"collaborators"
],
"type": "FORBIDDEN"
}
] strange thing is we do receive those collaborators regardless in our response. |
Maybe the results are incomplete? If you think it's a bug on GitHub's side, it would be helpful to report it to them.
I need to add to the API to make it possible to type assert the error into a valid GraphQL response containing non-empty errors, to be able tell it apart from other more serious errors (e.g., unable to make the GraphQL query because can't reach the server). This is tracked in issue #41. Also see #41 (comment) for an idea for a workaround that you can use until a better API is added. |
It would indeed be great if we could do type assertions on the error to make a decision on how to continue. I can confirm it is a bug in the api. It happens when there is a archived repository in the response. For now I filter the archived repositories first to work around this issue. |
FYI: Following is a workarround for the error by skipping archived repositories. query($query:String!$repoCursor:String) {
repos: search(query: $query, type: REPOSITORY first:100 after: $repoCursor) {
repositoryCount
pageInfo {
hasNextPage,
endCursor
},
edges {
node {
... on Repository {
id,
name,
description,
url,
sshUrl,
owner {
login
},
isPrivate,
createdAt,
updatedAt,
pushedAt,
repositoryTopics(first: 25) {
nodes{
topic{name},
resourcePath
}
},
collaborators(first: 15) {
totalCount,
pageInfo{ hasNextPage, endCursor },
nodes{name,login,avatarUrl}
}
}
}
}
}
} Using the arguments as following: {
"query": "org:philips-labs archived:false",
"repoCursor": null
} type RepositorySearch struct {
RepositoryCount int
PageInfo PageInfo
Edges []Edge
}
type Edge struct {
Node Node
}
type Node struct {
Repository Repository `graphql:"... on Repository"`
}
variables := map[string]interface{}{
"query": githubv4.String(fmt.Sprintf("org:%s archived:false", owner)),
"repoCursor": (*githubv4.String)(nil),
}
var q struct {
Repositories RepositorySearch `graphql:"search(query: $query, type: REPOSITORY first:100, after: $repoCursor)""`
}
err := c.Query(ctx, &q, variables) |
Is there any update on this one? Like a revamp on how to deal with the errors from this library. |
Hi I'm facing a very weird issue where I get following error from my code.
I have searched the lib and my code for this message, but it's not in there. So I assume there is something wrong in my graphql query. Is there a way I can inspect the generated graphql?
However when ran from graphql playground using the same authentication token it works.
Also see here the PR on my tool.
philips-labs/tabia#32
The text was updated successfully, but these errors were encountered: