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

invalid_grant status code should be 400 #561

Conversation

freeduerinckx
Copy link
Contributor

@freeduerinckx freeduerinckx commented Jul 4, 2018

According to section 5.2 of rfc 6749
(https://tools.ietf.org/html/rfc6749#section-5.2)

A server should respond with 400 in case of an invalid grant. The
given grant is invalid and the client should give other data.

A 401 is not applicable here because the client is required to give
suitable Authorization header field which doesn't make any sense if
you are trying to acquire a grant authentication.

According to sections 10.4.1 and 10.4.2 of rfc 2616
(https://tools.ietf.org/html/rfc2616#section-10.4.1)

WIP: these changes breaks downstream libs tests:

  • django
  • flask

@freeduerinckx freeduerinckx force-pushed the invalid-grant-should-respond-with-400 branch from 0595bad to f00c7a0 Compare July 4, 2018 12:36
According to section 5.2 of rfc 6749
(https://tools.ietf.org/html/rfc6749#section-5.2)

A server should respond with 400 in case of an invalid grant. The
given grant is invalid and the client should give other data.

A 401 is not applicable here because the client is required to give
a suitable Authorization header field which doesn't make any sense if
you are trying to acquire a grant authentication.

According to sections 10.4.1 and 10.4.2 of rfc 2616
(https://tools.ietf.org/html/rfc2616#section-10.4.1)
@freeduerinckx freeduerinckx force-pushed the invalid-grant-should-respond-with-400 branch from f00c7a0 to a4f39fc Compare July 4, 2018 12:42
@JonathanHuot
Copy link
Member

Thanks for the PR! This issue was highlighted at the beginning of #264 but no PR has been proposed since. Few more 401 must be 400 also as mentionned in the #264 .

We should release that as part of the 3.0.0 branch.

@JonathanHuot JonathanHuot added this to the 3.0.0 milestone Jul 5, 2018
@JonathanHuot JonathanHuot self-assigned this Aug 12, 2018
@jvanasco
Copy link
Contributor

The docstring on the error should reference the RFC sections mentioned above.

Copy link
Collaborator

@thedrow thedrow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@JonathanHuot JonathanHuot merged commit 7fb3bd4 into oauthlib:master Sep 20, 2018
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.

None yet

4 participants