Skip to content

🐛 Bug Report - config.allow_http used for two apparently unrelated purposes #27

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

Closed
vokimon opened this issue Oct 11, 2023 · 1 comment · Fixed by #29
Closed

🐛 Bug Report - config.allow_http used for two apparently unrelated purposes #27

vokimon opened this issue Oct 11, 2023 · 1 comment · Fixed by #29
Assignees
Labels
bug Something isn't working hacktoberfest Participating in Hacktoberfest

Comments

@vokimon
Copy link
Contributor

vokimon commented Oct 11, 2023

Bug description

OAuth2Backend uses config.allow_http to set Auth.http. Which is later used to set two features that are unrelated and should be independently for security reasons.

First, is used to set the httponly parameter for the cookie. This is set true for security. The browser should be able to send the cookie back but Javascript should be unable to access it.

Then in token_data, auth.http is used like it meant "use http protocol instead of https for the authorization response". Indeed it sets OAUTHLIB_INSECURE_TRANSPORT=1 if auth.http is true. While developing in http://localhost, this what you want. But in a server using http for authentication is not secure.

So:

  • if config.allow_http=True, we are using http protocol in production which is bad.
  • if config.allow_http=False, we are allowing any javascript in the browser to read the auth cookie which is also bad.

I guess that a different parameter should be used to set the cookie parameter.

Besides, i think that the examples, should not set an insecure setup without warning since the examples are usually copied as is. We noticed just because that gave us some problems in a different place.

Reproduction URL

No response

Reproduction steps

  • Run the example as is in an https server and the authentication will be using http protocol.
  • Add the following javascript code to the template:
console.log("Cookies", document.cookie)
  • If you run it with allow_http to true as in the current example, you won't see the Authorization cookie which is a secure behaviour.
  • But if you change allow_http to false, as you would like to have in production, then you will see the Authorization cookie on the console.

Screenshots

No response

Logs

No response

Browsers

Firefox, Chrome

OS

Linux

@vokimon vokimon added the bug Something isn't working label Oct 11, 2023
@ArtyomVancyan
Copy link
Member

ArtyomVancyan commented Oct 11, 2023

Hi @vokimon, first of all, thanks for the detailed report. I have read the docs carefully and came to such conclusion:

If the Secure attribute is never sent with unsecured HTTP (except on localhost), then setting it to not request.auth.http will not be wrong. And the HttpOnly attribute can be strictly set to True, because it only refers to the Authorization cookie. I do not see any reason to allow users to access or change it from JavaScript, even for development purposes (there are tons of other ways for sending requests with desired cookies: Python, Postman, etc.).

I would like to hear your opinion before I will change something. Also, you can open a PR if you have any other ideas.

@ArtyomVancyan ArtyomVancyan moved this to Todo in fastapi-oauth2 Oct 12, 2023
@ArtyomVancyan ArtyomVancyan added the hacktoberfest Participating in Hacktoberfest label Oct 13, 2023
@ArtyomVancyan ArtyomVancyan moved this from Todo to In Progress in fastapi-oauth2 Oct 13, 2023
@ArtyomVancyan ArtyomVancyan self-assigned this Oct 13, 2023
ArtyomVancyan added a commit to ArtyomVancyan/fastapi-oauth2 that referenced this issue Oct 13, 2023
ArtyomVancyan added a commit to ArtyomVancyan/fastapi-oauth2 that referenced this issue Oct 13, 2023
@github-project-automation github-project-automation bot moved this from In Progress to Done in fastapi-oauth2 Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working hacktoberfest Participating in Hacktoberfest
Projects
Status: Done
2 participants