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

Authorization issue when running graphql including repository collaborators via this lib #72

Open
marcofranssen opened this issue Jul 24, 2020 · 9 comments

Comments

@marcofranssen
Copy link

marcofranssen commented Jul 24, 2020

Hi I'm facing a very weird issue where I get following error from my code.

$ bin/tabia github repositories -O philips-labs -F json
2020/07/24 15:15:19 Must have push access to view repository collaborators.

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.

# Write your query or mutation here
query organizationRepositories($owner: String!) {
        organization(login:$owner) {
          repositories(first: 100) {
            totalCount
            nodes {
              owner {
                login
              }
              collaborators(first: 15) {
                nodes {
                  name
                  login
                  avatarUrl
                }
              }
              repositoryTopics(first: 25) {
                nodes {
                  topic {
                    name
                  }
                }
              }
              name
              id
              url
              description
              isPrivate
            }
          }
        }
      }

Also see here the PR on my tool.
philips-labs/tabia#32

@dmitshur
Copy link
Member

However when ran from graphql playground using the same authentication token it works.

That is unexpected.

Is there a way I can inspect the generated graphql?

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 graphql package or use an http.RoundTripper that prints the request bodies:

// 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.)

@marcofranssen
Copy link
Author

marcofranssen commented Jul 24, 2020

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.

Date: Fri, 24 Jul 2020 18:33:20 GMT
Content-Type: application/json; charset=utf-8
Transfer-Encoding: chunked
Server: GitHub.com
Status: 200 OK
Cache-Control: no-cache
X-OAuth-Scopes: admin:enterprise, admin:org, repo, user
X-Accepted-OAuth-Scopes: repo
X-GitHub-Media-Type: github.v4; format=json
X-RateLimit-Limit: 5000
X-RateLimit-Remaining: 4996
X-RateLimit-Reset: 1595619115
Access-Control-Expose-Headers: ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, Deprecation, Sunset
Access-Control-Allow-Origin: *
Strict-Transport-Security: max-age=31536000; includeSubdomains; preload
X-Frame-Options: deny
X-Content-Type-Options: nosniff
X-XSS-Protection: 1; mode=block
Referrer-Policy: origin-when-cross-origin, strict-origin-when-cross-origin
Content-Security-Policy: default-src 'none'
Vary: Accept-Encoding, Accept, X-Requested-With
Content-Encoding: gzip
Vary: Accept-Encoding
X-GitHub-Request-Id: CB29:AA75:836BB:9FAED:5F1B296F
{
    "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

@dmitshur
Copy link
Member

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.

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 githubv4 or any of its dependencies.

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.

@marcofranssen
Copy link
Author

Let me check that. How should I deal with that partial response from the lib if that is the case? What would you recommend?

@marcofranssen
Copy link
Author

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.

@dmitshur
Copy link
Member

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.

How should I deal with that partial response from the lib if that is the case? What would you recommend?

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.

@marcofranssen
Copy link
Author

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.

@marcofranssen
Copy link
Author

marcofranssen commented Jul 29, 2020

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)

@marcofranssen
Copy link
Author

Is there any update on this one?

Like a revamp on how to deal with the errors from this library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants