tusd icon indicating copy to clipboard operation
tusd copied to clipboard

Add flag to control CORS Origin header

Open sean9999 opened this issue 2 years ago • 6 comments

I added a flag to allow the user to set their own Access-Control-Allow-Origin header. I feel it's proper to be able to set this header because CORS was designed with this use-case in mind. You need to be able explicitly say what origin can access your API. Using a reverse-proxy felt kludgy, because the app already sets CORS headers. It is not offloading that responsibility. This resolves #135 and #386.

I developed this for personal use because I am running tusd in a Kubernetes environment, and because of the way k8s handles network routing via Ingress controller, setting the proper Origin header was infeasible (even with --behind-proxy turned on). Using a reverse proxy felt wrong because k8s already does a lot of network indirection.

I hope you find this useful

sean9999 avatar Sep 04 '21 23:09 sean9999

Thank you very much for this PR, but I have one question: Is it not necessary to validate the the Origin header in the request matches config.CorsOrigin? If they are not equal, we could return an error. Or is this not necessary? Right now, I don't see such a test but only that config.CorsOrigin is always set as the header value.

Acconut avatar Oct 15 '21 20:10 Acconut

: Is it not necessary to validate the the Origin header in the request matches config.CorsOrigin?

Right now, I don't see such a test but only that config.CorsOrigin is always set as the header value.

The most simplistic setup on apache/nginx is also just adding the origin with no further logic applied. I'd say this is okay to go for the very first iteration. refs https://ubiq.co/tech-blog/enable-cors-apache-web-server/ (I just grabbed to first hit on google search :see_no_evil: )

@sean9999 @Acconut is there anything I can help out with to get this PR going? THX a lot

DeepDiver1975 avatar Nov 08 '21 15:11 DeepDiver1975

Currently also experimenting with tusd in a kubernetes setup. Adding this functionality would gladly improve the setup.

@sean9999 @Acconut - is there anything we can do to move this PR forward?

patrickjahns avatar Mar 18 '22 15:03 patrickjahns

Yea adding this would be nice.. For now I use proxy_hide_header Access-Control-Allow-Origin; on nginx which disables their bad header.

G2G2G2G avatar Aug 07 '22 07:08 G2G2G2G

Is it not necessary to validate the the Origin header in the request matches config.CorsOrigin? If they are not equal, we could return an error. Or is this not necessary? Right now, I don't see such a test but only that config.CorsOrigin is always set as the header value.

You're right. I checked the spec, and a CORS compliant server is supposed to refuse a connection or return an error in that case. I'll try adding that in. https://www.w3.org/TR/2020/SPSD-cors-20200602/#resource-requests

sean9999 avatar Aug 10 '22 21:08 sean9999

I'll try adding that in

Sounds good! After that, I think we can merge this :)

Acconut avatar Aug 11 '22 08:08 Acconut

Took the freedom to rebase this PR -> #942

DeepDiver1975 avatar May 09 '23 09:05 DeepDiver1975

Is it not necessary to validate the the Origin header in the request matches config.CorsOrigin? If they are not equal, we could return an error. Or is this not necessary? Right now, I don't see such a test but only that config.CorsOrigin is always set as the header value.

You're right. I checked the spec, and a CORS compliant server is supposed to refuse a connection or return an error in that case. I'll try adding that in. https://www.w3.org/TR/2020/SPSD-cors-20200602/#resource-requests

For anyone interested. This would be the last point that needs to be done. If this check is implement, we can move towards merging such a feature :)

Acconut avatar Jun 12 '23 13:06 Acconut

can be closed as well - merged as part of https://github.com/tus/tusd/pull/987

DeepDiver1975 avatar Sep 05 '23 09:09 DeepDiver1975

This is superseeded by #997. Thank you for all your help!

Acconut avatar Sep 06 '23 08:09 Acconut