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

how to set/overwrite cors in the tusd code provided in the doc #450

Closed
scratchoo opened this issue Jan 16, 2021 · 11 comments · Fixed by #997
Closed

how to set/overwrite cors in the tusd code provided in the doc #450

scratchoo opened this issue Jan 16, 2021 · 11 comments · Fixed by #997
Labels

Comments

@scratchoo
Copy link

Question
I'm not a GO developer (I even did my best to learn GO basics just in the hope to make it works) but after wasting days without luck, I will do my last try to find an answer, otherwise, I'm giving up all the GO story because it just hurts to burn all the energy without finding the simplest answer: can just someone tell how to set cors to a specific host (i.e: example.com) instead of the default * without using a proxy

Setup details

Here is the example from the doc (we just need a way to overwrite the "Access-Control-Allow-Origin", I browsed the whole internet and can't find someone answering it !!!

package main

import (
	"fmt"
	"net/http"

	"github.com/tus/tusd/pkg/filestore"
	tusd "github.com/tus/tusd/pkg/handler"
)

func main() {
	// Create a new FileStore instance which is responsible for
	// storing the uploaded file on disk in the specified directory.
	// This path _must_ exist before tusd will store uploads in it.
	// If you want to save them on a different medium, for example
	// a remote FTP server, you can implement your own storage backend
	// by implementing the tusd.DataStore interface.
	store := filestore.FileStore{
		Path: "./uploads",
	}

	// A storage backend for tusd may consist of multiple different parts which
	// handle upload creation, locking, termination and so on. The composer is a
	// place where all those separated pieces are joined together. In this example
	// we only use the file store but you may plug in multiple.
	composer := tusd.NewStoreComposer()
	store.UseIn(composer)

	// Create a new HTTP handler for the tusd server by providing a configuration.
	// The StoreComposer property must be set to allow the handler to function.
	handler, err := tusd.NewHandler(tusd.Config{
		BasePath:              "/files/",
		StoreComposer:         composer,
		NotifyCompleteUploads: true,
	})
	if err != nil {
		panic(fmt.Errorf("Unable to create handler: %s", err))
	}

	// Start another goroutine for receiving events from the handler whenever
	// an upload is completed. The event will contains details about the upload
	// itself and the relevant HTTP request.
	go func() {
		for {
			event := <-handler.CompleteUploads
			fmt.Printf("Upload %s finished\n", event.Upload.ID)
		}
	}()

	// Right now, nothing has happened since we need to start the HTTP server on
	// our own. In the end, tusd will start listening on and accept request at
	// http://localhost:8080/files
	http.Handle("/files/", http.StripPrefix("/files/", handler))
	err = http.ListenAndServe(":8080", nil)
	if err != nil {
		panic(fmt.Errorf("Unable to listen: %s", err))
	}
}
@Acconut
Copy link
Member

Acconut commented Jan 17, 2021

I am sorry to hear that you are struggling so much with tusd. However, there is currently no way to change or overwrite the CORS setup from tusd. Neither programmatically nor using the tusd binary. The only option right now it to use an external proxy, such as nginx. Since this is an often requested feature, we are always open for PRs to improve in this regard :)

@scratchoo
Copy link
Author

scratchoo commented Jan 17, 2021

@Acconut if only this was written in the doc, you would save me tons of time and struggles I was able to set CORS via Nginx but the upload is not resumable even though I added:

    # Disable request and response buffering
    proxy_request_buffering  off;
    proxy_buffering          off;
    proxy_http_version       1.1;

    # Add X-Forwarded-* headers
    proxy_set_header X-Forwarded-Host $host;
    proxy_set_header X-Forwarded-Proto $scheme;

here is a live demo: ********* (demo link deleted) I use uppy with resume: true, when I pause and I resume, it starts again from the very beginning.

@Acconut
Copy link
Member

Acconut commented Jan 17, 2021

Could it be that there is another proxy between the browser and your nginx before tusd? Maybe from your hosting provider? It looks like this proxy buffers the request from a brief look.

@scratchoo
Copy link
Author

scratchoo commented Jan 18, 2021

@Acconut yeah you are right, it seems that the url they provided me with was using some kind of proxy/load-balancer, once I added an IPv4 it started working finally :) I'm ready now to upload files, but most importantly I will be sure to write a blog post about all the challenges I faced during the implementation, hopefully, I can save people like me some time...

just one last piece, as I'm using tusd programmatically, any method I can verify the upload is authorized?

@Acconut
Copy link
Member

Acconut commented Jan 18, 2021

You can use the PreCreateCallback in the config struct: https://godoc.org/github.com/tus/tusd/pkg/handler#Config

I will be sure to write a blog post about all the challenges I faced during the implementation, hopefully, I can save people like me some time...

Feel free to send us the link as well, so we can try to improve in these areas :)

@scratchoo
Copy link
Author

@Acconut yeah I see the PreUploadCreateCallback... as I'm totally new to GO I have to take a little time to figure out how to use it with the necessary verifications.

I will make sure to send the URL once I get it done. Thank you very much for the guidance, you just saved me an extra 3 days (lol) 👍

@wodCZ
Copy link

wodCZ commented Jan 22, 2021

Could you please consider adding a tusd flag to enable Access-Control-Allow-Credentials CORS header?

Following this issue we enabled withCredentials with the example provided in tus-js-client docs on our client, so we can forward loadbalancer (AWS's ALB) session cookie, but now we're getting this error:

Access to XMLHttpRequest at 'https://example.com:1080/files/' from origin 'https://app.com' has been blocked by CORS policy: Response to preflight request doesn't pass access control check: The value of the 'Access-Control-Allow-Credentials' header in the response is '' which must be 'true' when the request's credentials mode is 'include'. The credentials mode of requests initiated by the XMLHttpRequest is controlled by the withCredentials attribute.

Without cookies tus requests are distributed randomly to our tusd instances, which caused the corruption mentioned here.

We're not running, nor planning to run another reverse-proxy between the loadbalancer and tusd instance, adding nginx just to add this header feels like trying to kill a fly with a bazooka, especially given that this exact use-case seems to be recommended thorough the tus projects :)

I'm not a gopher, but adding a flag and conditionally adding the allow-credentials header here doesn't sound that hard? Would that be something you'd consider merging if I prepared a PR?

Thanks!

@Acconut
Copy link
Member

Acconut commented Jan 22, 2021

Following this issue we enabled withCredentials with the example provided in tus-js-client docs on our client, so we can forward loadbalancer (AWS's ALB) session cookie, but now we're getting this error:

That's a good reason, I never came across before.

I am open to adding options for customizing the CORS header sent by tusd since this is a popular issue. So feel free to open a PR. However, it would be helpful if we have a concept for not only customizing Access-Control-Allow-Credentials but also the other headers. So, if you make a PR please put some thoughts into how other headers could be customized in the future as well :)

@wodCZ
Copy link

wodCZ commented Jan 22, 2021

@Acconut noted, I'll consider.

This seems like a bigger issue though. In meanwhile I got to the same issue with cookies in our react-native app, which goes deep through react-native-tus-client, tus-android-client down to this tus-java-client PR here, pointing that cookies are not passed and the related discussion about changing http stack/client.
I'm considering avoiding the issue (and adding support in 2 languages I don't know) by just moving the instance out of the scaled service to a single node.

@Acconut
Copy link
Member

Acconut commented Jan 22, 2021

Oh right, thanks for letting us know that the Java client does not support Cookies. As I said, people are apparently rarely using Cookies with tusd, so this is a bit of untested territory.

Akkarine added a commit to Akkarine/tusd that referenced this issue Mar 21, 2022
chore/fix tus#333:
- s3store - lowercase metadata keys, so client can send them in any case
- fix possible errors for empty Basepath ("/")
sec/fix tus#450 - allow accepting credentials (for example, Cookie, so forwarding in hooks will work as described in docs)
@Acconut
Copy link
Member

Acconut commented Sep 6, 2023

This is implemented in #997 and will be included in the next release.

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

Successfully merging a pull request may close this issue.

3 participants