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 HTTP statuses on error response #264
Comments
The spec mention that 401 may be used with invalid client errors and must be used when the authentication was done via the authorization header field. Although oauthlib currently does nothing to suggest that the 'WWW-Authenticate' header field should be included in the response as the spec require. I reckon keeping 401 for this error is ok but we should add a header suggestion to the exception. Regarding the other the spec does not explicitly say that 401 may be used and thus we should probably consider changing to 400. However it does seem more appropriate with a 401 in both invalid grant and especially unauthorized client errors. @asteinlein, @lepture, @masci thoughts? |
@ib-lundgren Hi - just stumbled into this very issue and would like to chime in. From reading the spec and looking around other answers, IMO a 400 is the correct response. Firstly, the spec:
There's no MUST in the 400 requirements, but there's no MAY either. Secondly, and the reason I stumbled into this was through a bug of sorts in the sun JDK on my client (see here: http://stackoverflow.com/a/7524681/1137254 ) Essentially, my client's HTTP stack would close the connection early for a 401, but the OAuth stack above (google-http-oauth-client) would attempt to continue reading and parse the results. I think this was written into OpenJDK per the definition and expectations around a 401 (UPDATE: it's actually to do with 'streaming' mode). IMO OpenJDK is wrong too and I think I'd hit this same issue if I were to get an "invalid_client" 401 during /token endpoint requests. |
Adding another vote for 400 being the correct error code here. We used to use django-oauth2-provider for oauth2 authentication from mobile apps, and it returned 400 in this case. When switching to django-oauth-toolkit (which uses oauthlib) in order to upgrade to newer Django versions, we had to hack the token request view to return a 400 instead to keep the mobile apps working correctly. Here's one of the authors of the spec specifically stating that a 400 is the intended response for invalid authentication credentials when requesting a token, and why: https://www.ietf.org/mail-archive/web/oauth/current/msg02127.html And here's another example of a project that's having trouble coping with oauthlib using a 401 in these circumstances. |
+1 for 400 |
Vote for 400 |
I'm reviving this old thread & start a PR with the changes suggested above. |
Before making progress on this, I'd like to summarize the discussion which happen in the past.
In any cases, if Other OAuth2.0 implementationsA quick overview of the most common OAuth2.0 implementations are similar to the overall consensus:
ConlusionWe have to align with the RFC, however to not break any implementations, we must not include these changes in 2.x.x version, but wait until 3.x.x Please, feel free to chim in. |
I cannot remember the reason/the client(s) that made me push PR #247, but I see and understand the reasons for reverting this. Thanks for the awesome research @JonathanHuot, and doing this in 3.x sounds like the best approach. |
Fixed in #561 / oauthlib>= 3.0.0 |
according to this RFC that mentioned above invalid client also should return status 400 but in package still is 401 |
Hi @esfandiaryfard, could you point the exact text of the RFC ? Thanks! |
https://github.com/edx-olive/edx-platform/compare/38577488cb8c9335f73456939d480b19fdd92f3c...239cb23502a8616d826acdde50bc4dad70aa7b2b And uses upstream (constrained) versions of requests-oauthlib and oauthlib Bumping requests-oauthlib to 1.2 updates oauthlib, which changes a response code in certain cases, and causes a test to fail. See oauthlib/oauthlib#264 for more context
Updated `oauthlib` now returns a 400 error instead of 401 for invalid grants (ref: oauthlib/oauthlib#264). For our purposes, we don't care which type of error it is as long as it is a definitive error.
Previously, a 401 was returned when invalid credentials were used with the Resource Owner Password Credentials Grant flow. See oauthlib/oauthlib#264 and oauthlib/oauthlib#619 Versions 3+ return 400 making it tougher to distinguish between incorrect passwords and other errors. This patch reverts the status code back to 401, but only in the case where credentials were incorrect.
Previously, a 401 was returned when invalid credentials were used with the Resource Owner Password Credentials Grant flow. See oauthlib/oauthlib#264 and oauthlib/oauthlib#619 Versions 3+ return 400 making it tougher to distinguish between incorrect passwords and other errors. This patch reverts the status code back to 401, but only in the case where credentials were incorrect.
RFC 6749 Section 5.2 (http://tools.ietf.org/html/rfc6749#section-5.2) describes error response. It says that server should respond with HTTP 400 (Bad request). But oauthlib uses HTTP 401 (Unauthorized) instead for errors such as
InvalidClientError
,InvalidGrantError
,UnauthorizedClientError
and other related errors.For example https://github.com/idan/oauthlib/blob/master/oauthlib/oauth2/rfc6749/errors.py#L181
The text was updated successfully, but these errors were encountered: