-
Notifications
You must be signed in to change notification settings - Fork 981
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
oauth2: return error if error field is present in access token response #442
Conversation
…ld is present in token response
This PR (HEAD: 6057702) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/oauth2/+/261220 to see it. Tip: You can toggle comments from me using the |
Message from Gobot Gobot: Patch Set 1: Congratulations on opening your first change. Thank you for your contribution! Next steps: Most changes in the Go project go through a few rounds of revision. This can be Please don’t reply on this GitHub thread. Visit golang.org/cl/261220. |
Message from Go Bot: Patch Set 1: Congratulations on opening your first change. Thank you for your contribution! Next steps: Most changes in the Go project go through a few rounds of revision. This can be Please don’t reply on this GitHub thread. Visit golang.org/cl/261220. |
Related issue #274 (reported by me long ago), I guess this one will get fixed as well. |
Came here because Slack does something similar and there's no way to know exactly what the error is. What's missing from this being merged? |
Hello @codyoss, |
Also find it quite cumbersome not being able to get the error. Can this be merged? |
@bradfitz Can you take a look or assign a new reviewer? https://go-review.googlesource.com/c/oauth2/+/261220 |
Closing due to lack of activity |
Could cfe200d have been an equivalent fix? |
The issue this fork was meant to address (golang/oauth2#441) has been fixed upstream: - [proposed fix that was part of fork](golang/oauth2#442) - [upstream fix](golang/oauth2@cfe200d) (golang/go#58125) Notably, both implementations change the error returned to render more verbose details on `.Error()` if the response is a status 200 _but_ an error (see above issues) A diff against upstream indicates that there are no other changes: sourcegraph/oauth2#3 ## Test plan Test suites pass
The issue this fork was meant to address (golang/oauth2#441) has been fixed upstream: - [proposed fix that was part of fork](golang/oauth2#442) - [upstream fix](golang/oauth2@cfe200d) (golang/go#58125) Notably, both implementations change the error returned to render more verbose details on `.Error()` if the response is a status 200 _but_ an error (see above issues) A diff against upstream indicates that there are no other changes: sourcegraph/oauth2#3 ## Test plan Test suites pass
Return an error containing the error, error_description, and error_uri when the error field is present in the access token response, even if the HTTP response code is 2xx.
Fixes #441