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

Option to disable CORS headers #386

Closed
sripberger opened this issue May 19, 2020 · 7 comments · Fixed by #899
Closed

Option to disable CORS headers #386

sripberger opened this issue May 19, 2020 · 7 comments · Fixed by #899

Comments

@sripberger
Copy link

Is your feature request related to a problem? Please describe.
I'm seeing that you guys are blindly setting Access-Control-Allow-Origin to match the Origin header of the request, regardless of what that Origin header is. That's nice for ease of use, but it isn't exactly a good idea on the public internet.

You don't provide any options for changing CORS behavior, and I'm seeing in #135 that the response to this is that in production you're going to want to put tusd behind a proxy of some kind, anyway, and should probably have the proxy handle the CORS headers instead. This is totally reasonable, but it can present some challenges and pitfalls.

In my case I'm using haproxy, and I can have it conditionally set Access-Control-Origin only for Origins that match a Regex contained in a file with something like this:

capture request header origin len 128
http-response add-header Access-Control-Allow-Origin %[capture.req.hdr(0)] if { capture.req.hdr(0) -m reg -f /etc/haproxy/cors-origins.lst 

The problem here is that add-header appends to what is already there, so this header will be set to ${Origin}, ${Origin} for requests that hit tusd, which is obviously will cause the request to be blocked by the browser.

If I change this configuration to use set-header instead, which overwrites what may or may not already be there, it will get rid of this problem. However, having this collision go undetected creates a more serious problem, which is that tusd is still setting the header for non-matching origins, and the proxy is not clearing this. So even though haproxy is checking against the regexes, I am still allowing requests from all origins if they terminate at tusd.

To fix this, I have to actively delete the header in my configuration for non-matching origins. This isn't too complicated, since it's just a matter of starting by deleting whatever might be there:

capture request header origin len 128
http-response delete-header Access-Control-Allow-Origin
http-response set-header Access-Control-Allow-Origin %[capture.req.hdr(0)] if { capture.req.hdr(0) -m reg -f /etc/haproxy/cors-origins.lst
http-

It would be nice if it were cleaner, though. Having to explicitly delete something that shouldn't even be there in the first place is a bit of a kludge, to the point where I feel the need to leave a comment in the config file making note of tusd's behavior that causes this delete header line to be required.

Describe the solution you'd like
To handle this problem-- and other confusion arising from multiple processes being responsible for CORS headers-- it would be nice to have an option of disabling CORS headers completely in tusd. That way I can easily just say that CORS is handled completely by my proxy.

Something like tusd --no-cors.

Describe alternatives you've considered
I've described the alternative of just deleting these headers in my proxy configuration, and while it works it feels a bit kludgy and requires explaining commments.

Can you provide help with implementing this feature?
I would be interested, but I'm not familiar with Go and I have very little time to spend on it at the moment. If I do end up pitching it in, it will have to wait for a month or two, I imagine.

@Acconut
Copy link
Member

Acconut commented May 24, 2020

Thanks for the feedback, it's very helpful. We have been getting these comments a few times already now so we are open to adding a --cors-origin-pattern option, so users can configure what origins they want to allow. Since my times is also very restricted, I would be happy to assist with opening a PR.

@sripberger
Copy link
Author

I appreciate the response, though I will note that --cors-origin-pattern wouldn't be quite what I'm looking for. This request is really more about preventing tusd from doing anything whatsoever with CORS headers. I've got the origin pattern check implemented in HAProxy already and do not need or want tusd to repeat it.

It is understandable if you guys don't have time at the moment. It's not a huge priority for me, either. Just something that would slightly clean up my HAProxy configuration. 👍

@Acconut
Copy link
Member

Acconut commented May 29, 2020

Ok, so if I understand your proposal of --no-cors correctly, the flag would allow you to get rid of the http-response delete-header Access-Control-Allow-Origin (etc) instructions in HAProxy. Is this correct? If so, then it would be a little benefit for adding this flag, making it a low priority (but not useless) for us as well.

@thedevhaider
Copy link

Hey Guys, Any update on this issue as I am looking for a way to whitelist the origins. Usually, we have a tus server running on a separate machine. So, It becomes necessary to whitelist some domains.

@Acconut
Copy link
Member

Acconut commented Sep 16, 2020

Unfortunately, there is no update. You still have to use a proxy if you want custom CORS headers. We are open for PRs, though :)

@akkie
Copy link
Contributor

akkie commented Feb 7, 2023

Hi @Acconut, I also stumbled over this issue. I have created a PR with a new flag --disable-cors which completely disables all CORS headers. Would be great if you could have a look.

@Acconut
Copy link
Member

Acconut commented Feb 11, 2023

Thank you, @akkie! I will have a look soon.

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

Successfully merging a pull request may close this issue.

4 participants