OAuth/deck: handle callbacks with errors #10048
Comments
k8s-ci-robot
added
kind/bug
petr-muller
added a commit
to petr-muller/test-infra
that referenced
this issue
Nov 9, 2018
Make Prow properly detect and log possible error information when passed by GitHub and differentiate the messaging from the case where token exchange fails for other reasons. Certain error conditions [1] cause GitHub to call back the OAuth redirect with error information present in parameters. Previously, Prow's handler assumed the token to be always present, failing with a generic error message when it was not. [1] https://developer.github.com/apps/managing-oauth-apps/troubleshooting-authorization-request-errors/ Fixes: kubernetes#10048
ibzib
pushed a commit
to ibzib/test-infra
that referenced
this issue
Nov 21, 2018
Make Prow properly detect and log possible error information when passed by GitHub and differentiate the messaging from the case where token exchange fails for other reasons. Certain error conditions [1] cause GitHub to call back the OAuth redirect with error information present in parameters. Previously, Prow's handler assumed the token to be always present, failing with a generic error message when it was not. [1] https://developer.github.com/apps/managing-oauth-apps/troubleshooting-authorization-request-errors/ Fixes: kubernetes#10048
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
There are error cases where the registered OAuth callback is called back by GitHub without the user access token, but with error information passed as parameters.
The handler for the OAuth redirect does not detect this error condition and ultimately fails with 500 when trying to extract the token, showing the user quite unfriendly error message and logging just the generic error message:
500 Internal server error Exchange code for token: oauth2: server response missing access_tokenAt minimum, we should log the errors passed by GitHub - the errors are quite rare, but logging properly makes debugging easier (we just hit this when investigating what turned out to be a configuration mistake).
/kind bug
/area prow
/cc @stevekuznetsov
The text was updated successfully, but these errors were encountered: