search

atmos / warden-github

:lock: warden strategy for github oauth
MIT License
54 stars 41 forks source link

Hard to handle GitHub API errors #45

Open rpavlik opened 9 years ago

rpavlik commented 9 years ago

I'm using this with your Sinatra gem.

After many debug prints, I found a bug here on this line: https://github.com/atmos/warden-github/blob/master/lib/warden/github/oauth.rb#L50

That's basically assuming that any time we fail to get exactly what we want in response to authentication, we claim it's a Bad Verification Code, when really, any of the GitHub OAUTH errors will get pushed through there.

My workaround to figure out my problem was to split the line before the rescue into

params = decode_params(response.body)
params.fetch('access_token')

I could then look at the params in the rescue section, which revealed the problem I was having and the easiest way to test this:

I had my GitHub client secret wrong.

Specifically, I was getting the Invalid Client Credentials error from this page: https://developer.github.com/v3/oauth/#common-errors-for-the-access-token-request

atmos commented 9 years ago

I wonder if it'd be advantageous to check these creds at startup and atleast warn that they're invalid. I'm a bit confused why the IndexError exception was raised for this.

rpavlik commented 9 years ago

Well, in the case that authentication fails, there is no "access_token" field in the json from the api (hence index error when trying to access it, i would suppose) but an error object instead - you can test this with something like postman.