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

Redirect Uri Mismatch error alignment with specification #108

Merged
merged 1 commit into from
Jan 16, 2017

Conversation

rmmeans
Copy link
Contributor

@rmmeans rmmeans commented Jan 13, 2017

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.

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
` 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 did wrong.
@rmmeans
Copy link
Contributor Author

rmmeans commented Jan 13, 2017

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
Copy link
Contributor Author

rmmeans commented Jan 13, 2017

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
Copy link
Member

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.

@tsuyoshizawa tsuyoshizawa merged commit 7f32072 into nulab:master Jan 16, 2017
@rmmeans
Copy link
Contributor Author

rmmeans commented Jan 16, 2017

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!

@rmmeans rmmeans deleted the redirect-uri-alignement branch January 18, 2017 17:42
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

Successfully merging this pull request may close these issues.

2 participants