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

HTTPBearer security scheme is returning 403 instead or 401 #10177

Open
Kludex opened this issue Aug 31, 2023 Discussed in #9130 · 7 comments
Open

HTTPBearer security scheme is returning 403 instead or 401 #10177

Kludex opened this issue Aug 31, 2023 Discussed in #9130 · 7 comments
Labels
bug Something isn't working

Comments

@Kludex
Copy link
Sponsor Collaborator

Kludex commented Aug 31, 2023

Discussed in #9130

Originally posted by aaaaahaaaaa September 8, 2020
HTTPBearer security scheme enabled as a dependency is returning a 403 when a request is unauthenticated because of a missing or a malformed authorization header. In those scenarios, a 401 should be returned instead.

Related PRs

@Kludex Kludex added question Question or problem investigate question-migrate bug Something isn't working and removed question Question or problem investigate question-migrate labels Aug 31, 2023
@ezeparziale
Copy link

I use the auto_error=False and control the correct http error code manually.

@ordinary-jamie
Copy link

ordinary-jamie commented Sep 12, 2023

Hey @Kludex, what do you think about raising a HTTP 400 BAD REQUEST for cases where the authentication scheme is not correctly 'bearer'?

From RFC 6750 Section 3.1:

invalid_request
         The request is missing a required parameter, includes an
         unsupported parameter or parameter value, repeats the same
         parameter, uses more than one method for including an access
         token, or is otherwise malformed.  The resource server SHOULD
         respond with the HTTP 400 (Bad Request) status code.

invalid_token
         The access token provided is expired, revoked, malformed, or
         invalid for other reasons.  The resource SHOULD respond with
         the HTTP 401 (Unauthorized) status code.  The client MAY
         request a new access token and retry the protected resource
         request.

My reading of this text is that:

  • when the server understands the request (i.e. 'bearer' scheme is correct) but the token itself is malformed we should return a 401
  • when the server does not understand the request (i.e. 'bearer' scheme is wrong), then we should return a 400

@Kludex
Copy link
Sponsor Collaborator Author

Kludex commented Sep 12, 2023

Can you show me the diff you are talking about?

@ordinary-jamie
Copy link

ordinary-jamie commented Sep 12, 2023

fastapi/security/http.py L#125

EDIT: Wrong line sorry

@Kludex
Copy link
Sponsor Collaborator Author

Kludex commented Sep 12, 2023

You want 400 instead of 403? That's why I asked the diff, not the lines in the code source. 👀

@ordinary-jamie
Copy link

Sorry, my misunderstanding. At any rate, I don't think it is necessary after reading section 3 again I realised they offered an example for this scenario.

tldr; 401 seems like the appropriate error code 👍

 If the request lacks any authentication information (e.g., the client
   was unaware that authentication is necessary or attempted using an
   unsupported authentication method), the resource server SHOULD NOT
   include an error code or other error information.

   For example:

     HTTP/1.1 401 Unauthorized
     WWW-Authenticate: Bearer realm="example"

@bootrecords
Copy link

This not only affects the HTTPBearer, but a lot of other classes in the security scope also return 403 in "Not authenticated" cases where they should return 401 instead, such as the OAuth2 class (though strangely enough not the subclasses OAuth2PasswordBearer and OAuth2AuthorizationCodeBearer), the HTTPBase class and its sublasses (except for HTTPBasic for some reason), all classes in api_key.py and OpenIdConnect. Would probably be good if that could be straightened out for all occurrences there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants