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

file_uploader headers on https deployed streamlit app #10049

Open
3 of 4 tasks
Test-028 opened this issue Dec 19, 2024 · 5 comments
Open
3 of 4 tasks

file_uploader headers on https deployed streamlit app #10049

Test-028 opened this issue Dec 19, 2024 · 5 comments

Comments

@Test-028
Copy link

Checklist

  • I have searched the existing issues for similar issues.
  • I added a very descriptive title to this issue.
  • I have provided sufficient information below to help reproduce this issue.

Summary

When using file_uploader component on a streamlit application that has been deployed behind a load balancer that performs https termination everything works fine.

Let's suppose that my application is accessible through https://myapp.com
enableCorsand enableXsrfProtectionare both on.

Then due to this block in the upload_file_request_handler.py file:

if config.get_option("server.enableXsrfProtection"):
            self.set_header(
                "Access-Control-Allow-Origin",
                server_util.get_url(config.get_option("browser.serverAddress")),
            )
            self.set_header("Access-Control-Allow-Headers", "X-Xsrftoken, Content-Type")
            self.set_header("Vary", "Origin")
            self.set_header("Access-Control-Allow-Credentials", "true")

If I let my configuration unchanged everything works fine , but we will have
Access-Control-Allow-Origin with localhost as value which is something I don't want to have.

If I put myapp.com as browser.serverAddress , I'll get http://myapp.com (and not https://myapp.com) in the Access-Control-Allow-Origin header

and if i enter https://myapp.com as browser.serverAddress it will be http://https://myapp.com in the Access-Control-Allow-Origin header.

If we look at server_util.get_url we can understand this behavior , as I said in the beginning I perform https termination before the application and therefore I don't set any server.sslCertFile that will make the get_url adding https as protocol instead of http.

Finally I don't even understand why we set this Cors header in the file_uploader, in any of the scenario everything works fine (except that we have weird Access-Control-Allow-Origin: the whole point of my issue) , because I guess when using the file_uploader, requests are made from the same origin and aren't cross origin requests.

Reproducible Code Example

No response

Steps To Reproduce

No response

Expected Behavior

No response

Current Behavior

No response

Is this a regression?

  • Yes, this used to work in a previous version.

Debug info

  • Streamlit version:
  • Python version:
  • Operating System:
  • Browser:

Additional Information

No response

@Test-028 Test-028 added status:needs-triage Has not been triaged by the Streamlit team type:bug Something isn't working labels Dec 19, 2024
Copy link

If this issue affects you, please react with a 👍 (thumbs up emoji) to the initial post.

Your feedback helps us prioritize which bugs to investigate and address first.

Visits

@kajarenc kajarenc assigned kajarenc and unassigned kajarenc Dec 19, 2024
@kajarenc
Copy link
Collaborator

Hello @Test-028 , thank you for opening this issue!

I don't fully understand the problem exact problem you facing.

Access-Control-Allow-Origin with localhost as value which is something I don't want to have.
If the infrastructure behind the proxy is under your control, why not allow also requests from "localhost"

Or as an alternative, I think you should be able to disable CORS protection on the Streamlit level and delegate that check to the proxy

Please let us know if you are able to fix the issue on the infrastructure level or if you have any other suggestions/comments related to this issue.

@kajarenc kajarenc added priority:P4 feature:st.file_uploader and removed status:needs-triage Has not been triaged by the Streamlit team labels Dec 20, 2024
@Test-028
Copy link
Author

Test-028 commented Dec 20, 2024

Hello @kajarenc , thank you for your reply !
The problem is that if we disable cors protection , a lot of Access-Control-Allow-Origin: * , will be placed (first in the file_uploader requests , but in other handler as well) , so I'll have to implement a rule in the proxy to remove all these Access-Control-Allow-Origin: * that will be added by streamlit server in the proxy.

My exact problem , is that , with the setup that streamlit suggest for deploying HTTPS application, the file_uploader handler add this Access-Control-Allow-Origin: .......... when we enable xsrf protection, but enabling xsrf protection doesn't mean we necessarily want to perform cross-origin requests.

If you look at other endpoints present on streamlit , the logic when we enable Cors protection is to simply reject CORS request, and no Access-Control-Allow-Origin are set , whereas for the file_uploader there is a different treatment , and I can't understand the underlying reason, why we will necessarily want to make cross origin request to the file uploader ?

Why not simply getting rid of the possibility of doing cross origin request on it , like it is done for other endpoints in streamlit when enableCors is on: that is no cross origin request -> No CORS headers.

Currently if my understanding is correct, in a regular setup you will have Acces-Control-Allow-Origin that has the same value as your origin (so not really needed I guess) , and with people with the same situation as me , a weird Acces-Control-Allow-Origin when using the file_uploader. Whereas in any case we are doing same origin requests.

@kajarenc
Copy link
Collaborator

Yeah, I see; thank you for the explanation

We thinking of improving this experience, especially decoupling CORS and CSRF stremlit configs, and this is also could be point of improvement!

just to check, when we talk about upload endpoint , we mean "/_stcore/upload_file/... endpoint, right?

yeah, for now the workaround with configuring a rule on proxy layer to remove undesired headers should work

@Test-028
Copy link
Author

Test-028 commented Dec 20, 2024

@kajarenc Exactly I'm speaking about this endpoint.

I think it received a different treatment for no obvious reason, its set-header method should have been like the other ones.
only the "if allow_cross_origin_requests() -> Allow-Control-Origin:*" and that's all (today, because if cors is false , you can't have xsrf protection, so no need to setup the cross-origin-headers, basically only having the current elif allow_cross_origin_requests(). But what is inside the if about the xsrf protection in my eyes doesn't make sense )

And certainly a lot of users are impacted without noticing it , since at the end they are doing the request from the same origin, but they have this weird Access-Control-Allow-origin set to localhost for example, which can be a potential important security issue I guess.

When you look at everything, you think that by having enableCors set to true in streamlit , will disable all CORS requests, its true, except for the file_uploader..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants