-
-
Notifications
You must be signed in to change notification settings - Fork 393
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
Google authentication just started failing with the passport strategy #125
Comments
Hi @morungos, this is 100% unrelated to a passport strategy (apart from the error handler, which i'll address below) and I am unable to reproduce with a standard implementation.
I'm familiar with this google payload envelope. is_standard_body_error shouldn't return true for such envelope. This is actually further masking the response for you since i'm guessing there's also a top level member Providing state to the token endpoint (draft-ietf-oauth-mix-up-mitigation) was removed in v2.4.0 as it's no longer a BCP. To aid in debugging this on your end you might look at #90 (comment) and if you're getting an OpenIdConnectError instance as a result of a backend call you can inspect the |
``` { error: { code: 400, message: 'Request contains an invalid argument.', status: 'INVALID_ARGUMENT' } } ``` This is not an OpenIdConnectError and will no longer be returned as such, this will now fall under throwing HTTPError instances instead. See #125
|
and here's your problem :) https://twitter.com/busymac/status/1045112552626298880 |
Wow, thanks @panva -- I'd had a long day and didn't have the processing power to pursue it further. Despite your guess (mine too, I added some extra logging to poke inside the response) the body did not contain any other fields, the entire body was the single error object above. That was a huge part of the problem: the total lack of meaningful error information in the body. Fortunately the Twitter thread does point to a Google snafu, and all is now functioning normally again. Many thanks for the error handling fix, and I'll put a little more effort into making sure I handle weird error cases at my end. |
I've been using the module and the passport strategy happily for a while now, but I think it just started failing, and the issue is in the POST to
/token
, so it's deeper in the innards than I'm used to. I'll document what I've discovered so far, in case anyone else comes across this.As far as I can tell, this is a new issue that manifested in the last 24 hours, on a server that was not touched. It's also reproduced in my development environment. It's showing in versions from 2.4.1 back to 2.2.1.
The initial symptom is an error in a catch error handler:
But I think this may simply be masking the issue, by the error returned from
/token
not being in the right place in the response code logic for the passport handlers.As far as I can tell, all goes well in the redirect and back, until it gets to the POST to
/token
. The error returned from Google is:The POST request for /token contains approximately the following:
I was suspicious about the state, because it doesn't seem to be a standard part of the authorization code flow, which as far as I can tell is what is happening here. So I hacked the code briefly to remove that one field
state
, but it made no difference.A suspicion? Maybe the client key and secret need to be added (based on this: https://aaronparecki.com/oauth-2-simplified/#web-server-apps) and this is what's showing. The Google error is desperately unhelpful, and this is where some logging would have made a big difference to debugging this.
The text was updated successfully, but these errors were encountered: