cli / go-gh

A Go module for interacting with gh and the GitHub API from the command line.
https://pkg.go.dev/github.com/cli/go-gh/v2
MIT License
322 stars 45 forks source link

API Authentication Fails When Using OAuth or App Tokens #59

Closed samueljmello closed 1 year ago

samueljmello commented 1 year ago

I've noticed what seems like some unexpected behavior when using OAuth (start with gho_) or App (start with ghs_) tokens. I wrote an extension that utilizes the ClientOptions struct and when I provide ClientOptions.AuthToken for either OAuth or App generated tokens, I get a 401.

Upon analyzing what's taking place, it appears that the authorization header that go-gh is using has token instead of bearer. This works for PATs, but not for the aforementioned token types.

To get around this I had to set ClientOptions.AuthToken and then also override ClientOptions.Headers providing the Authorization:bearer <token> key-value manually.

If I try to just add the header key-value override, the lib falls back to using built-in authentication (gh auth login) instead of the provided token.

At least, this is the functionality I'm seeing. Help?

samcoe commented 1 year ago

@samueljmello Thanks for writing in. You are correct that currently we hardcode token when automatically resolving the authorization header. That is odd behavior you are seeing though, if you specified the Authorization header the library should respect that and not override it. Are you using v0.0.3? If so, could you try using the HEAD of the trunk branch and seeing if this changes the behavior you are seeing?

mislav commented 1 year ago

Upon analyzing what's taking place, it appears that the authorization header that go-gh is using has token instead of bearer. This works for PATs, but not for the aforementioned token types.

The token <TOKEN> header value should be working not just for PATs, but for OAuth and App-generated tokens too. There should be no need for you to supply specific tokens with bearer keyword instead of token. https://docs.github.com/en/rest/overview/resources-in-the-rest-api#oauth2-token-sent-in-a-header

Could you give an example of an endpoint that is not accessible using the token keyword? Thanks

samueljmello commented 1 year ago

Thanks @samcoe and @mislav! I appreciate the help.

  1. I am using 0.0.3
  2. Yeah, it's strange. It just rejects the App token with 401
  3. The resource that first gets queried is the GraphQL API for listing repositories in an organization.

My GH extension: https://github.com/mona-actions/gh-migrate-webhook-secrets

samueljmello commented 1 year ago

Update:

If I set up the ClientOptions.Headers without providing ClientOptions.AuthToken I get an error when I try to instantiate the client stating "not found".

samcoe commented 1 year ago

@samueljmello That is the expected behavior. Without specifying ClientOptions.AuthToken go-gh tries to resolve a valid auth token, and returns the not found error if none can be resolved from the environment or local config file. If the ClientOptions.Headers "Authorization" header is also specified it will take precedence over ClientOptions.AuthToken so you can pass an invalid value such as "none" to skip the resolution step.

samueljmello commented 1 year ago

@samcoe It took a while to figure things out, as we were working with a client that had specific use cases. It turns out that my initial report is incorrect: you don't need to pass ClientOptions.Headers with Bearer for tokens to work. I will try my best to surmise what the actual problem was:

What works:

  1. Using PAT works against regular API endpoints
  2. Using GH App Tokens work against regular API endpoints (except specific endpoints like /user which aren't supported)
  3. Using OAUTH Tokens also work, just like a PAT

What doesn't work:

  1. GH App Tokens on App API endpoints (GitHub Apps API). These expect a JWT

So essentially, when I was testing, I had a request going to /user which was failing when I used an App token, and additionally I tried to use the App API endpoints which require a JWT. This combined with code changes flying back-and-forth lead to a lot of confusion. So now I can concretely say that this is a non-issue and had more to do with how specific tokens work and what they can be used for.

Thanks for all your support and patience.