search

nulab / scala-oauth2-provider

OAuth 2.0 server-side implementation written in Scala
MIT License
537 stars 97 forks source link

Redirect Uri Mismatch error alignment with specification #108

Closed rmmeans closed 7 years ago

rmmeans commented 7 years ago

Currently, the RedirectUriMismatch error is using a non-standard errorType. Google popularized this error, but it is not part of the standard specification.

Section 4.1.2.1 specifically states (https://tools.ietf.org/html/rfc6749#section-4.1.2.1) If the request fails due to a missing, invalid, or mismatching redirection URI … and the valid error types listed that can be returned does not include redirect_uri_mismatch.

I propose of the errors listed, the one that makes the most sense is invalid_request additionally, we can use the error_description field which is optional to provide the existing message of redirect_uri_mismatch so the developer knows what he / she did wrong.

rmmeans commented 7 years ago

I'm realizing now that part of the error responses only applies when you follow the redirect URL and include an error in the query string, which the spec states you must not do in the event the error is a bad redirect URI.

I may be misinterpreting the spec here. I'll re-examine this tomorrow.

rmmeans commented 7 years ago

Sorry for that last comment - I was mistaken and clearly need to go to bed 😪 💤 . I do believe what I have submitted in a PR is correct.

The key here really is this: redirect_uri_mismatch is not a standard error code. Section 5.2 (https://tools.ietf.org/html/rfc6749#section-5.2) is the correct section to discuss these error states and the json body that is returned when attempting to exchange for an auth code etc.

Because generic OAuth clients may be communicating directly with this library, my vote would be the library needs to follow the spec as much as possible for error status return types.

tsuyoshizawa commented 7 years ago

redirect_uri_mismatch is not a standard error code. Section 5.2 (https://tools.ietf.org/html/rfc6749#section-5.2) is the correct section to discuss these error states

Thank you for detail link. I understand.

Let me know if you need new release version soon. I'm going to release new version after released Play 2.6.

rmmeans commented 7 years ago

No, I don't need a new release quite yet. I'm am currently investigating one other spec related issue - if it turns out to be something, I might like a release after that one.

Thanks again!