Skip to content
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

please consider returning more descriptive errors like ErrInvalidRequest #173

Closed
arvenil opened this issue Feb 23, 2016 · 3 comments
Closed

Comments

@arvenil
Copy link

arvenil commented Feb 23, 2016

https://tools.ietf.org/html/rfc6749 defines error responses returned to clients, however this library seems to completely shadow received responses so one can't distinguish between different type of errors.

For example for Resource Owner Password Credentials Grant (https://tools.ietf.org/html/rfc6749#section-4.3) RFC6749 defines error message and codes in https://tools.ietf.org/html/rfc6749#section-5.2 Now we can't distinguish between e.g. invalid_request and invalid_grant as what the func PasswordCredentialsToken returns is basic error message

oauth2/internal/token.go

Lines 174 to 176 in 2cd4472

if code := r.StatusCode; code < 200 || code > 299 {
return nil, fmt.Errorf("oauth2: cannot fetch token: %v\nResponse: %s", r.Status, body)
}

Well, since there is response in the error message I could parse it but I think you realize ugliness of such a hack ;)

What I would expect here is following standard concepts from golang standard library and to introduce package errors, and export them. So instead returning meaningless errors (something went wrong but what?) we could receive something more useful like ErrInvalidRequest or ErrInvalidGrant.

Right now my biggest real life issue is that I can't distinguish that user provided wrong credentials from any other unknown error.

@adayoung
Copy link

adayoung commented Oct 8, 2016

YES! This! Please!! 😍
@arvenil How are you actually doing it at the moment while it's not there?

@arvenil
Copy link
Author

arvenil commented Oct 11, 2016

@adayoung Like I stated in description the only method is to parse whole message for e.g. invalid_request or invalid_grant - ugly hack but "works".

@adayoung
Copy link

@arvenil Oh me too! I ended up writing this. It probably doesn't cover everything but so far it catches things I'm likely to encounter. I'll have to keep a lookout for when it doesn't! I'd really appreciate if you'd point to some code examples of how you're doing it. I'm just learning Go and I may not be doing things right at all 😊

gopherbot pushed a commit that referenced this issue Apr 11, 2023
Parse error response described in https://datatracker.ietf.org/doc/html/rfc6749#section-5.2

Handle unorthodox servers responding 200 in error case.

Implements API changes in accepted proposal golang/go#58125

Fixes #441
Fixes #274
Updates #173

Change-Id: If9399c3f952ac0501edbeefeb3a71ed057ca8d37
GitHub-Last-Rev: 0030e27
GitHub-Pull-Request: #610
Reviewed-on: https://go-review.googlesource.com/c/oauth2/+/451076
Run-TryBot: Matt Hickford <[email protected]>
Run-TryBot: Damien Neil <[email protected]>
Reviewed-by: Matt Hickford <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
Reviewed-by: Cody Oss <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants