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 #480

Merged
merged 3 commits into from
Jun 15, 2017
Merged

redirect_uri mismatch #480

merged 3 commits into from
Jun 15, 2017

Conversation

orenmazor
Copy link
Contributor

One of our customers came across this issue. they provided the correct redirect_uri on the auth code request, but then the wrong one on the token request.

the error they receive was a 401 "Access Denied", which is correct, but not descriptive. I could start overriding things up in the stack to add a check, but I feel like a more descriptive error can just come back from here, seeing as how there's already an exception specifically for this.

thoughts?

/cc @bjmc @thedrow

@orenmazor
Copy link
Contributor Author

if this is acceptable i will happily adjust the test suite to handle it, obviously

@bjmc
Copy link
Contributor

bjmc commented Jun 14, 2017

This seems sensible to me, unless there's something in the OAuth2 spec that forbids giving more detailed information for security reasons. And this could be potentially a breaking change for clients that look for the old, generic error.

@bjmc
Copy link
Contributor

bjmc commented Jun 14, 2017

Actually, should this be MismatchingRedirectURIError? The context here is that the client sent a different redirect_uri during the authorization request than they sent on the token request.

@orenmazor
Copy link
Contributor Author

Yeah, that actually makes sense to me. updated.

@thedrow thedrow merged commit 1c38f09 into oauthlib:master Jun 15, 2017
@thedrow
Copy link
Collaborator

thedrow commented Jun 15, 2017

Thanks for the contribution. It will be released soon.

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.

4 participants