pallets-eco / flask-jwt

JWT (JSON Web Tokens) for Flask applications
MIT License
564 stars 177 forks source link

Auth endpoint returns http 400 on bad credentials instead of 401 #34

Closed ghost closed 9 years ago

ghost commented 10 years ago

I think that if user provides bad credentials, return code should be 401. 400 would mean that the request is malformed:

http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html

flask_jwt/init.py:168

lyschoening commented 10 years ago

+1

This seems to have already been fixed in master several months ago. All we need now is a new release. https://github.com/mattupstate/flask-jwt/commit/6eb2f4aef6653cc78d30e0a1d15da5134427f914

TimotheeJeannin commented 9 years ago

+1

It would be great to have this fix released. :)

mattupstate commented 9 years ago

I can't agree with this. The authorization endpoint does not require authorization, which is what a 401 response is meant to indicate. In other words, 401 indicates to the client that the Authorization must be set. That header is never necessary when making a request to the authorization endpoint. Therefore, 400 Bad Request is still the most appropriate response when invalid credentials are supplied.

ghost commented 9 years ago

"The authorization endpoint does not require authorization"? Well of course it does, you have to provide username and password to get a token.

Here is some explanation found on stackoverflow: http://stackoverflow.com/a/1960453

mattupstate commented 9 years ago

@bolimir I appreciate the link to inform me of HTTP error codes, but I'm already well aware of their meaning(s) as specified by rfc2616. Which states the following reagarding 401 responses:

The request requires user authentication. The response MUST include a WWW-Authenticate header field (section 14.47) containing a challenge applicable to the requested resource. The client MAY repeat the request with a suitable Authorization header field (section 14.8). If the request already included Authorization credentials, then the 401 response indicates that authorization has been refused for those credentials. If the 401 response contains the same challenge as the prior response, and the user agent has already attempted authentication at least once, then the user SHOULD be presented the entity that was given in the response, since that entity might include relevant diagnostic information. HTTP access authentication is explained in "HTTP Authentication: Basic and Digest Access Authentication" [43].

This means that a 401 response should only be returned when the Authorization header MUST be set for the request to be fulfilled. The authorization endpoint for Flask-JWT does not require the Authorization header to be included in the request.

lyschoening commented 9 years ago

Perhaps it should be 403. From this year's RFC7231:

The 403 (Forbidden) status code indicates that the server understood the request but refuses to authorize it. A server that wishes to make public why the request has been forbidden can describe that reason in the response payload (if any).

If authentication credentials were provided in the request, the server considers them insufficient to grant access. The client SHOULD NOT automatically repeat the request with the same credentials. The client MAY repeat the request with new or different credentials. However, a request might be forbidden for reasons unrelated to the credentials.

An origin server that wishes to "hide" the current existence of a forbidden target resource MAY instead respond with a status code of 404 (Not Found).

mattupstate commented 9 years ago

@lyschoening Nope. A 403 is to indicate to the client that the credentials are insufficient. This does not mean that the credentials are incorrect, but the user identified by the credentials does not have the required access level to make the request and get a successful response.

lyschoening commented 9 years ago

@mattupstate "insufficient" is really ambiguous. I would have understood it differently. (Also, wouldn't it be conceivable that someone simply might not wish to give out JWTs to a certain user despite their credentials being correct?)

In any case, 400 also does not seem correct. Invalid credentials aren't necessarily a client error (without further qualifications).

mattupstate commented 9 years ago

@lyschoening It is not ambiguous, that is the purpose of the specification. Consider this scenario:

Given that users with the admin role can create articles And Mary is an admin user with a valid authentication token And Mary makes a POST request to /articles And with the authentication token in the Authorization header And with a valid JSON formatted request body The API will return a 200 response code

Then consider this scenario:

Given that users with the editor role cannot create articles And Bob is an editor and not an admin with a valid authentication token And Bob makes a POST request to /articles And with the authentication token in the Authorization header And with a valid JSON formatted request body The API will return a 403 response code

In the both scenarios "the server understood the request" because it contained valid parameters and a valid authentication token. However, in the second scenario the server "refuses to authorize it" because the user identified by the token does not have sufficient privileges to create articles.

Also, wouldn't it be conceivable that someone simply might not wish to give out JWTs to a certain user despite their credentials being correct?

What use would this be unless you're providing a public, unauthenticated API?

Invalid credentials aren't necessarily a client error

It depends on the requirements of the request. It is the client's responsibility to provide an interface to collect the username/password pair to be passed in the body of a request to the authorization endpoint. This endpoint does not require the Authorization header to be supplied in order to get a 200 response. 401 and 403 response codes are tailored to indicate an issue with the presence or value of the Authorization header. Consider this scenario:

Given a user named Janet without an authentication token And Janet makes a POST request to /articles And without the Authorization header And with a valid JSON formatted request body The API will return a 401 response code

If this scenario happened within the context of your client application, the client would then display an interface to collect the user's username/password and make a request to the authorization endpoint:

Given a user named Janet without an authentication token And Janet makes a POST request to /authorize And with a valid JSON formatted request body containing her username and password The API will return a 200 response code And the body will contain a valid authentication token

Now the client application Janet is using can take that token and retry the previous scenario and it would result in a 200 is she had sufficient access privileges or a 403 if she did not.