From 9590312d47d5ac9ec787df89ac22905a4a684113 Mon Sep 17 00:00:00 2001 From: Sean Macdonald Date: Sat, 4 Sep 2021 19:13:55 -0400 Subject: [PATCH 1/4] added flags for CORS header --- cmd/tusd/cli/flags.go | 2 ++ cmd/tusd/cli/serve.go | 5 +++++ pkg/handler/config.go | 11 +++++++++++ pkg/handler/unrouted_handler.go | 5 +++++ 4 files changed, 23 insertions(+) diff --git a/cmd/tusd/cli/flags.go b/cmd/tusd/cli/flags.go index 1cc1e4b45..e9d46824d 100644 --- a/cmd/tusd/cli/flags.go +++ b/cmd/tusd/cli/flags.go @@ -53,6 +53,7 @@ var Flags struct { ShowVersion bool ExposeMetrics bool MetricsPath string + CorsOrigin string BehindProxy bool VerboseOutput bool S3TransferAcceleration bool @@ -103,6 +104,7 @@ func ParseFlags() { flag.BoolVar(&Flags.ShowVersion, "version", false, "Print tusd version information") flag.BoolVar(&Flags.ExposeMetrics, "expose-metrics", true, "Expose metrics about tusd usage") flag.StringVar(&Flags.MetricsPath, "metrics-path", "/metrics", "Path under which the metrics endpoint will be accessible") + flag.StringVar(&Flags.CorsOrigin, "cors-origin", "", "Explicitly set Access-Control-Allow-Origin header") flag.BoolVar(&Flags.BehindProxy, "behind-proxy", false, "Respect X-Forwarded-* and similar headers which may be set by proxies") flag.BoolVar(&Flags.VerboseOutput, "verbose", true, "Enable verbose logging output") flag.BoolVar(&Flags.S3TransferAcceleration, "s3-transfer-acceleration", false, "Use AWS S3 transfer acceleration endpoint (requires -s3-bucket option and Transfer Acceleration property on S3 bucket to be set)") diff --git a/cmd/tusd/cli/serve.go b/cmd/tusd/cli/serve.go index b4619ccbc..01357af69 100644 --- a/cmd/tusd/cli/serve.go +++ b/cmd/tusd/cli/serve.go @@ -26,6 +26,7 @@ func Serve() { config := handler.Config{ MaxSize: Flags.MaxSize, BasePath: Flags.Basepath, + CorsOrigin: Flags.CorsOrigin, RespectForwardedHeaders: Flags.BehindProxy, EnableExperimentalProtocol: Flags.ExperimentalProtocol, DisableDownload: Flags.DisableDownload, @@ -106,6 +107,10 @@ func Serve() { protocol = "https" } + if Flags.CorsOrigin != "" { + stdout.Printf("CORS origin header is %s", Flags.CorsOrigin) + } + if Flags.HttpSock == "" { stdout.Printf("You can now upload files to: %s://%s%s", protocol, address, basepath) } diff --git a/pkg/handler/config.go b/pkg/handler/config.go index b53b9fc75..433be45bf 100644 --- a/pkg/handler/config.go +++ b/pkg/handler/config.go @@ -49,6 +49,10 @@ type Config struct { NotifyCreatedUploads bool // Logger is the logger to use internally, mostly for printing requests. Logger *log.Logger + // Explicitly set Access-Control-Allow-Origin in cases where RespectForwardedHeaders + // doesn't give you the desired result. This can be the case with some reverse proxies + // or a kubernetes setup with complex network routing rules + CorsOrigin string // Respect the X-Forwarded-Host, X-Forwarded-Proto and Forwarded headers // potentially set by proxies when generating an absolute URL in the // response to POST requests. @@ -95,5 +99,12 @@ func (config *Config) validate() error { return errors.New("tusd: StoreComposer in Config needs to contain a non-nil core") } + if config.CorsOrigin != "" && config.CorsOrigin != "*" && config.CorsOrigin != "null" { + _, err := url.ParseRequestURI(config.CorsOrigin) + if err != nil { + errors.New("tusd: CorsOrigin is not a valid URL") + } + } + return nil } diff --git a/pkg/handler/unrouted_handler.go b/pkg/handler/unrouted_handler.go index e48757e19..b9a142520 100644 --- a/pkg/handler/unrouted_handler.go +++ b/pkg/handler/unrouted_handler.go @@ -226,7 +226,12 @@ func (handler *UnroutedHandler) Middleware(h http.Handler) http.Handler { header := w.Header() if origin := r.Header.Get("Origin"); !handler.config.DisableCors && origin != "" { + var configuredOrigin = handler.config.CorsOrigin + if configuredOrigin != "" { + origin = configuredOrigin + } header.Set("Access-Control-Allow-Origin", origin) + header.Set("Vary", "Origin") if r.Method == "OPTIONS" { allowedMethods := "POST, HEAD, PATCH, OPTIONS" From f5ac00bbdaeed5cb7a242ec6dd753ecae5fb74f2 Mon Sep 17 00:00:00 2001 From: Sean Macdonald Date: Sat, 4 Sep 2021 19:34:03 -0400 Subject: [PATCH 2/4] go fmt From 49607de10b6a090f44cea837963930284639fd04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Mon, 8 May 2023 11:16:11 +0200 Subject: [PATCH 3/4] fix: go vet --- pkg/handler/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/handler/config.go b/pkg/handler/config.go index 433be45bf..841bb8941 100644 --- a/pkg/handler/config.go +++ b/pkg/handler/config.go @@ -102,7 +102,7 @@ func (config *Config) validate() error { if config.CorsOrigin != "" && config.CorsOrigin != "*" && config.CorsOrigin != "null" { _, err := url.ParseRequestURI(config.CorsOrigin) if err != nil { - errors.New("tusd: CorsOrigin is not a valid URL") + return errors.New("tusd: CorsOrigin is not a valid URL") } } From af6cca68cbf2b08bff9ef1608a3ec526ede197c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Tue, 13 Jun 2023 15:58:42 +0200 Subject: [PATCH 4/4] fix: check Origin to match configured CORS Origin --- pkg/handler/cors_test.go | 185 +++++++++++++++++++++++++------- pkg/handler/unrouted_handler.go | 41 +++---- pkg/handler/utils_test.go | 15 ++- 3 files changed, 184 insertions(+), 57 deletions(-) diff --git a/pkg/handler/cors_test.go b/pkg/handler/cors_test.go index aad2021e6..08045d4db 100644 --- a/pkg/handler/cors_test.go +++ b/pkg/handler/cors_test.go @@ -9,67 +9,196 @@ import ( ) func TestCORS(t *testing.T) { - SubTest(t, "Preflight", func(t *testing.T, store *MockFullDataStore, composer *StoreComposer) { + SubTest(t, "PreFlight - Conditional allow methods", func(t *testing.T, store *MockFullDataStore, composer *StoreComposer) { handler, _ := NewHandler(Config{ - StoreComposer: composer, + StoreComposer: composer, + CorsOrigin: "https://tus.io", + DisableTermination: true, + DisableDownload: true, }) (&httpTest{ Method: "OPTIONS", ReqHeader: map[string]string{ - "Origin": "tus.io", + "Origin": "https://tus.io", }, Code: http.StatusOK, ResHeader: map[string]string{ "Access-Control-Allow-Headers": "Authorization, Origin, X-Requested-With, X-Request-ID, X-HTTP-Method-Override, Content-Type, Upload-Length, Upload-Offset, Tus-Resumable, Upload-Metadata, Upload-Defer-Length, Upload-Concat, Upload-Incomplete, Upload-Draft-Interop-Version", - "Access-Control-Allow-Methods": "POST, HEAD, PATCH, OPTIONS, GET, DELETE", + "Access-Control-Allow-Methods": "POST, HEAD, PATCH, OPTIONS", "Access-Control-Max-Age": "86400", - "Access-Control-Allow-Origin": "tus.io", + "Access-Control-Allow-Origin": "https://tus.io", }, }).Run(handler, t) }) + SubTest(t, "PreFlight - No Origin configured", func(t *testing.T, store *MockFullDataStore, composer *StoreComposer) { + composer = NewStoreComposer() + composer.UseCore(store) - SubTest(t, "Conditional allow methods", func(t *testing.T, store *MockFullDataStore, composer *StoreComposer) { handler, _ := NewHandler(Config{ - StoreComposer: composer, - DisableTermination: true, - DisableDownload: true, + StoreComposer: composer, + CorsOrigin: "", }) (&httpTest{ Method: "OPTIONS", + DisallowedResHeader: []string{ + "Access-Control-Allow-Origin", + "Access-Control-Allow-Methods", + "Access-Control-Allow-Headers", + "Access-Control-Max-Age", + }, + Code: http.StatusOK, ReqHeader: map[string]string{ - "Origin": "tus.io", + "Origin": "https://tus.io", + }, + }).Run(handler, t) + }) + SubTest(t, "PreFlight - Disabled CORS", func(t *testing.T, store *MockFullDataStore, composer *StoreComposer) { + composer = NewStoreComposer() + composer.UseCore(store) + + handler, _ := NewHandler(Config{ + StoreComposer: composer, + CorsOrigin: "", + DisableCors: true, + }) + + (&httpTest{ + Method: "OPTIONS", + DisallowedResHeader: []string{ + "Access-Control-Allow-Origin", + "Access-Control-Allow-Methods", + "Access-Control-Allow-Headers", + "Access-Control-Max-Age", }, Code: http.StatusOK, + ReqHeader: map[string]string{ + "Origin": "https://tus.io", + }, + }).Run(handler, t) + }) + SubTest(t, "PreFlight - Wildcard Origin", func(t *testing.T, store *MockFullDataStore, composer *StoreComposer) { + composer = NewStoreComposer() + composer.UseCore(store) + + handler, _ := NewHandler(Config{ + StoreComposer: composer, + CorsOrigin: "*", + }) + + (&httpTest{ + Method: "OPTIONS", ResHeader: map[string]string{ + "Access-Control-Allow-Origin": "*", + "Access-Control-Allow-Methods": "POST, HEAD, PATCH, OPTIONS, GET, DELETE", + "Access-Control-Allow-Headers": "Authorization, Origin, X-Requested-With, X-Request-ID, X-HTTP-Method-Override, Content-Type, Upload-Length, Upload-Offset, Tus-Resumable, Upload-Metadata, Upload-Defer-Length, Upload-Concat, Upload-Incomplete, Upload-Draft-Interop-Version", + "Access-Control-Max-Age": "86400", + }, + Code: http.StatusOK, + ReqHeader: map[string]string{ + "Origin": "https://tus.io", + }, + }).Run(handler, t) + }) + SubTest(t, "PreFlight - Matching Origin", func(t *testing.T, store *MockFullDataStore, composer *StoreComposer) { + composer = NewStoreComposer() + composer.UseCore(store) + + handler, _ := NewHandler(Config{ + StoreComposer: composer, + CorsOrigin: "https://tus.io", + }) + + (&httpTest{ + Method: "OPTIONS", + ResHeader: map[string]string{ + "Access-Control-Allow-Origin": "https://tus.io", + "Access-Control-Allow-Methods": "POST, HEAD, PATCH, OPTIONS, GET, DELETE", "Access-Control-Allow-Headers": "Authorization, Origin, X-Requested-With, X-Request-ID, X-HTTP-Method-Override, Content-Type, Upload-Length, Upload-Offset, Tus-Resumable, Upload-Metadata, Upload-Defer-Length, Upload-Concat, Upload-Incomplete, Upload-Draft-Interop-Version", - "Access-Control-Allow-Methods": "POST, HEAD, PATCH, OPTIONS", "Access-Control-Max-Age": "86400", - "Access-Control-Allow-Origin": "tus.io", + }, + Code: http.StatusOK, + ReqHeader: map[string]string{ + "Origin": "https://tus.io", }, }).Run(handler, t) }) + SubTest(t, "PreFlight - Not Matching Origin", func(t *testing.T, store *MockFullDataStore, composer *StoreComposer) { + composer = NewStoreComposer() + composer.UseCore(store) - SubTest(t, "Request", func(t *testing.T, store *MockFullDataStore, composer *StoreComposer) { handler, _ := NewHandler(Config{ StoreComposer: composer, + CorsOrigin: "https://tus.net", }) (&httpTest{ - Name: "Actual request", - Method: "GET", + Method: "OPTIONS", + DisallowedResHeader: []string{ + "Access-Control-Allow-Origin", + "Access-Control-Allow-Methods", + "Access-Control-Allow-Headers", + "Access-Control-Max-Age", + }, + Code: http.StatusOK, ReqHeader: map[string]string{ - "Origin": "tus.io", + "Origin": "https://tus.io", }, - Code: http.StatusMethodNotAllowed, + }).Run(handler, t) + }) + SubTest(t, "Actual Request - Wildcard Origin", func(t *testing.T, store *MockFullDataStore, composer *StoreComposer) { + composer = NewStoreComposer() + composer.UseCore(store) + + handler, _ := NewHandler(Config{ + StoreComposer: composer, + CorsOrigin: "*", + }) + + (&httpTest{ + Method: "POST", ResHeader: map[string]string{ - "Access-Control-Expose-Headers": "Upload-Offset, Location, Upload-Length, Tus-Version, Tus-Resumable, Tus-Max-Size, Tus-Extension, Upload-Metadata, Upload-Defer-Length, Upload-Concat, Upload-Incomplete, Upload-Draft-Interop-Version", - "Access-Control-Allow-Origin": "tus.io", + "Access-Control-Allow-Origin": "*", + "Access-Control-Expose-Headers": "Upload-Offset, Location, Upload-Length, Tus-Version, Tus-Resumable, Tus-Max-Size, Tus-Extension, Upload-Metadata, Upload-Defer-Length, Upload-Concat", + }, + DisallowedResHeader: []string{ + "Access-Control-Allow-Methods", + "Access-Control-Allow-Headers", + "Access-Control-Max-Age", + }, + Code: http.StatusPreconditionFailed, + ReqHeader: map[string]string{ + "Origin": "https://tus.io", }, }).Run(handler, t) }) + SubTest(t, "Actual Request - Matching Origin", func(t *testing.T, store *MockFullDataStore, composer *StoreComposer) { + composer = NewStoreComposer() + composer.UseCore(store) + handler, _ := NewHandler(Config{ + StoreComposer: composer, + CorsOrigin: "https://tus.io", + }) + + (&httpTest{ + Method: "POST", + ResHeader: map[string]string{ + "Access-Control-Allow-Origin": "https://tus.io", + "Access-Control-Expose-Headers": "Upload-Offset, Location, Upload-Length, Tus-Version, Tus-Resumable, Tus-Max-Size, Tus-Extension, Upload-Metadata, Upload-Defer-Length, Upload-Concat", + }, + DisallowedResHeader: []string{ + "Access-Control-Allow-Methods", + "Access-Control-Allow-Headers", + "Access-Control-Max-Age", + }, + Code: http.StatusPreconditionFailed, + ReqHeader: map[string]string{ + "Origin": "https://tus.io", + }, + }).Run(handler, t) + }) SubTest(t, "AppendHeaders", func(t *testing.T, store *MockFullDataStore, composer *StoreComposer) { handler, _ := NewHandler(Config{ StoreComposer: composer, @@ -77,7 +206,7 @@ func TestCORS(t *testing.T) { req, _ := http.NewRequest("OPTIONS", "", nil) req.Header.Set("Tus-Resumable", "1.0.0") - req.Header.Set("Origin", "tus.io") + req.Header.Set("Origin", "https://tus.io") req.Host = "tus.io" res := httptest.NewRecorder() @@ -96,20 +225,4 @@ func TestCORS(t *testing.T) { t.Errorf("expected header to contain METHOD but got: %#v", methods) } }) - - SubTest(t, "Disable CORS", func(t *testing.T, store *MockFullDataStore, composer *StoreComposer) { - handler, _ := NewHandler(Config{ - StoreComposer: composer, - DisableCors: true, - }) - - (&httpTest{ - Method: "OPTIONS", - ReqHeader: map[string]string{ - "Origin": "tus.io", - }, - Code: http.StatusOK, - ResHeader: map[string]string{}, - }).Run(handler, t) - }) } diff --git a/pkg/handler/unrouted_handler.go b/pkg/handler/unrouted_handler.go index b9a142520..9f9ac5c36 100644 --- a/pkg/handler/unrouted_handler.go +++ b/pkg/handler/unrouted_handler.go @@ -227,27 +227,32 @@ func (handler *UnroutedHandler) Middleware(h http.Handler) http.Handler { if origin := r.Header.Get("Origin"); !handler.config.DisableCors && origin != "" { var configuredOrigin = handler.config.CorsOrigin - if configuredOrigin != "" { - origin = configuredOrigin + if configuredOrigin == "*" { + origin = "*" } - header.Set("Access-Control-Allow-Origin", origin) - header.Set("Vary", "Origin") - - if r.Method == "OPTIONS" { - allowedMethods := "POST, HEAD, PATCH, OPTIONS" - if !handler.config.DisableDownload { - allowedMethods += ", GET" - } - - if !handler.config.DisableTermination { - allowedMethods += ", DELETE" + if configuredOrigin == origin { + header.Set("Access-Control-Allow-Origin", origin) + header.Set("Vary", "Origin") + + if r.Method == "OPTIONS" { + allowedMethods := "POST, HEAD, PATCH, OPTIONS" + if !handler.config.DisableDownload { + allowedMethods += ", GET" + } + + if !handler.config.DisableTermination { + allowedMethods += ", DELETE" + } + + // Preflight request + header.Add("Access-Control-Allow-Methods", allowedMethods) + header.Add("Access-Control-Allow-Headers", "Authorization, Origin, X-Requested-With, X-Request-ID, X-HTTP-Method-Override, Content-Type, Upload-Length, Upload-Offset, Tus-Resumable, Upload-Metadata, Upload-Defer-Length, Upload-Concat, Upload-Incomplete, Upload-Draft-Interop-Version") + header.Set("Access-Control-Max-Age", "86400") + } else { + // Actual request + header.Add("Access-Control-Expose-Headers", "Upload-Offset, Location, Upload-Length, Tus-Version, Tus-Resumable, Tus-Max-Size, Tus-Extension, Upload-Metadata, Upload-Defer-Length, Upload-Concat") } - // Preflight request - header.Add("Access-Control-Allow-Methods", allowedMethods) - header.Add("Access-Control-Allow-Headers", "Authorization, Origin, X-Requested-With, X-Request-ID, X-HTTP-Method-Override, Content-Type, Upload-Length, Upload-Offset, Tus-Resumable, Upload-Metadata, Upload-Defer-Length, Upload-Concat, Upload-Incomplete, Upload-Draft-Interop-Version") - header.Set("Access-Control-Max-Age", "86400") - } else { // Actual request header.Add("Access-Control-Expose-Headers", "Upload-Offset, Location, Upload-Length, Tus-Version, Tus-Resumable, Tus-Max-Size, Tus-Extension, Upload-Metadata, Upload-Defer-Length, Upload-Concat, Upload-Incomplete, Upload-Draft-Interop-Version") diff --git a/pkg/handler/utils_test.go b/pkg/handler/utils_test.go index c2e6356d3..6df2a9488 100644 --- a/pkg/handler/utils_test.go +++ b/pkg/handler/utils_test.go @@ -52,9 +52,10 @@ type httpTest struct { ReqBody io.Reader ReqHeader map[string]string - Code int - ResBody string - ResHeader map[string]string + Code int + ResBody string + ResHeader map[string]string + DisallowedResHeader []string } func (test *httpTest) Run(handler http.Handler, t *testing.T) *httptest.ResponseRecorder { @@ -82,6 +83,14 @@ func (test *httpTest) Run(handler http.Handler, t *testing.T) *httptest.Response } } + for _, key := range test.DisallowedResHeader { + header := w.Header().Get(key) + + if header != "" { + t.Errorf("Not Expected '%s' (got '%s')", key, header) + } + } + if test.ResBody != "" && w.Body.String() != test.ResBody { t.Errorf("Expected '%s' as body (got '%s'", test.ResBody, w.Body.String()) }