Enable CORS only when origin is defined

config/confighttp/README.md

@@ -36,12 +36,13 @@ exporter:
 [Receivers](https://github.com/open-telemetry/opentelemetry-collector/blob/main/receiver/README.md)
 leverage server configuration.
 
-- [`cors_allowed_origins`](https://github.com/rs/cors): An empty list here and
-  in `cors_allowed_headers` means that CORS is not enabled at all. 
-  A wildcard can be used to match any origin or one or more characters of an origin.
-- [`cors_allowed_headers`](https://github.com/rs/cors): An empty list here and 
-  in `cors_allowed_origins` means that CORS is not enabled at all. 
-  A wildcard can be used to match any header or one or more characters in the header.
+- [`cors_allowed_origins`](https://github.com/rs/cors): An empty list means
+  that CORS is not enabled at all. A wildcard can be used to match any origin
+  or one or more characters of an origin.
+- [`cors_allowed_headers`](https://github.com/rs/cors): When CORS is enabled,
+  can be used to specify an optional list of allowed headers. By default, it includes `Accept`, 
+  `Content-Type`, `X-Requested-With`. `Origin` is also always
+  added to the list. A wildcard (`*`) can be used to match any header.
 - `endpoint`: Valid value syntax available [here](https://github.com/grpc/grpc/blob/master/doc/naming.md)
 - [`tls_settings`](../configtls/README.md)
 

config/confighttp/confighttp.go

@@ -21,6 +21,7 @@ import (
 	"time"
 
 	"github.com/rs/cors"
+	"go.uber.org/zap"
 
 	"go.opentelemetry.io/collector/config/configtls"
 	"go.opentelemetry.io/collector/internal/middleware"
@@ -111,14 +112,14 @@ type HTTPServerSettings struct {
 
 	// CorsOrigins are the allowed CORS origins for HTTP/JSON requests to grpc-gateway adapter
 	// for the OTLP receiver. See github.com/rs/cors
-	// An empty CorsOrigins and CorsHeaders means that CORS is not enabled at all.
-	// A wildcard (*) can be used to match any origin or one or more characters of an origin.
+	// An empty list means that CORS is not enabled at all. A wildcard (*) can be
+	// used to match any origin or one or more characters of an origin.
 	CorsOrigins []string `mapstructure:"cors_allowed_origins"`
 
 	// CorsHeaders are the allowed CORS headers for HTTP/JSON requests to grpc-gateway adapter
 	// for the OTLP receiver. See github.com/rs/cors
-	// An empty CorsOrigins and CorsHeaders means that CORS is not enabled at all.
-	// A wildcard (*) can be used to match any header or one or more characters of a header.
+	// CORS needs to be enabled first by providing a non-empty list in CorsOrigins
+	// A wildcard (*) can be used to match any header.
 	CorsHeaders []string `mapstructure:"cors_allowed_headers"`
 }
 
@@ -143,10 +144,18 @@ func (hss *HTTPServerSettings) ToListener() (net.Listener, error) {
 // returned by HTTPServerSettings.ToServer().
 type toServerOptions struct {
 	errorHandler middleware.ErrorHandler
+	logger       *zap.Logger
 }
 
 type ToServerOption func(opts *toServerOptions)
 
+// WithLogger allows to specify optional logger used during initialization.
+func WithLogger(logger *zap.Logger) ToServerOption {
+	return func(opts *toServerOptions) {
+		opts.logger = logger
+	}
+}
+
 // WithErrorHandler overrides the HTTP error handler that gets invoked
 // when there is a failure inside middleware.HTTPContentDecompressor.
 func WithErrorHandler(e middleware.ErrorHandler) ToServerOption {
@@ -160,10 +169,14 @@ func (hss *HTTPServerSettings) ToServer(handler http.Handler, opts ...ToServerOp
 	for _, o := range opts {
 		o(serverOpts)
 	}
-	if len(hss.CorsOrigins) > 0 || len(hss.CorsHeaders) > 0 {
+	if len(hss.CorsOrigins) > 0 {
 		co := cors.Options{AllowedOrigins: hss.CorsOrigins, AllowedHeaders: hss.CorsHeaders}
 		handler = cors.New(co).Handler(handler)
+	} else if len(hss.CorsHeaders) > 0 && serverOpts.logger != nil {
+		serverOpts.logger.Warn(
+			"CORS needs to be enabled via `cors_allowed_origins` for `cors_allowed_headers` to have an effect")
 	}
+
 	handler = middleware.HTTPContentDecompressor(
 		handler,
 		middleware.WithErrorHandler(serverOpts.errorHandler),

config/confighttp/confighttp_test.go

@@ -25,6 +25,8 @@ import (
 	"testing"
 	"time"
 
+	"go.uber.org/zap"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -316,37 +318,93 @@ func TestHttpReception(t *testing.T) {
 }
 
 func TestHttpCors(t *testing.T) {
-	hss := &HTTPServerSettings{
-		Endpoint:    "localhost:0",
-		CorsOrigins: []string{"allowed-*.com"},
+	tests := []struct {
+		name             string
+		CorsOrigins      []string
+		CorsHeaders      []string
+		allowedWorks     bool
+		disallowedWorks  bool
+		extraHeaderWorks bool
+	}{
+		{
+			name:             "noCORS",
+			allowedWorks:     false,
+			disallowedWorks:  false,
+			extraHeaderWorks: false,
+		},
+		{
+			name:             "OriginCORS",
+			CorsOrigins:      []string{"allowed-*.com"},
+			CorsHeaders:      []string{},
+			allowedWorks:     true,
+			disallowedWorks:  false,
+			extraHeaderWorks: false,
+		},
+		{
+			name:             "HeaderCORS",
+			CorsOrigins:      []string{"allowed-*.com"},
+			CorsHeaders:      []string{"ExtraHeader"},
+			allowedWorks:     true,
+			disallowedWorks:  false,
+			extraHeaderWorks: true,
+		},
 	}
 
-	ln, err := hss.ToListener()
-	assert.NoError(t, err)
-	s := hss.ToServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
-	go func() {
-		_ = s.Serve(ln)
-	}()
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			hss := &HTTPServerSettings{
+				Endpoint:    "localhost:0",
+				CorsOrigins: tt.CorsOrigins,
+				CorsHeaders: tt.CorsHeaders,
+			}
+
+			ln, err := hss.ToListener()
+			assert.NoError(t, err)
+			s := hss.ToServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
+			go func() {
+				_ = s.Serve(ln)
+			}()
+
+			// TODO: make starting server deterministic
+			// Wait for the servers to start
+			<-time.After(10 * time.Millisecond)
 
-	// TODO: make starting server deterministic
-	// Wait for the servers to start
-	<-time.After(10 * time.Millisecond)
+			url := fmt.Sprintf("http://%s", ln.Addr().String())
 
-	url := fmt.Sprintf("http://%s", ln.Addr().String())
+			// Verify allowed domain gets responses that allow CORS.
+			verifyCorsResp(t, url, "allowed-origin.com", false, 200, tt.allowedWorks)
 
-	// Verify allowed domain gets responses that allow CORS.
-	verifyCorsResp(t, url, "allowed-origin.com", 200, true)
+			// Verify allowed domain and extra headers gets responses that allow CORS.
+			verifyCorsResp(t, url, "allowed-origin.com", true, 200, tt.extraHeaderWorks)
 
-	// Verify disallowed domain gets responses that disallow CORS.
-	verifyCorsResp(t, url, "disallowed-origin.com", 200, false)
+			// Verify disallowed domain gets responses that disallow CORS.
+			verifyCorsResp(t, url, "disallowed-origin.com", false, 200, tt.disallowedWorks)
 
+			require.NoError(t, s.Close())
+		})
+	}
+}
+
+func TestHttpCorsInvalidSettings(t *testing.T) {
+	hss := &HTTPServerSettings{
+		Endpoint:    "localhost:0",
+		CorsHeaders: []string{"some-header"},
+	}
+
+	// This effectively does not enable CORS but should also not cause an error
+	s := hss.ToServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}), WithLogger(zap.NewNop()))
+	require.NotNil(t, s)
 	require.NoError(t, s.Close())
 }
 
-func verifyCorsResp(t *testing.T, url string, origin string, wantStatus int, wantAllowed bool) {
+func verifyCorsResp(t *testing.T, url string, origin string, extraHeader bool, wantStatus int, wantAllowed bool) {
 	req, err := http.NewRequest("OPTIONS", url, nil)
 	require.NoError(t, err, "Error creating trace OPTIONS request: %v", err)
 	req.Header.Set("Origin", origin)
+	if extraHeader {
+		req.Header.Set("ExtraHeader", "foo")
+		req.Header.Set("Access-Control-Request-Headers", "ExtraHeader")
+	}
 	req.Header.Set("Access-Control-Request-Method", "POST")
 
 	resp, err := http.DefaultClient.Do(req)

receiver/otlpreceiver/README.md

@@ -52,8 +52,8 @@ port is `55681`.
 
 The HTTP/JSON endpoint can also optionally configure
 [CORS](https://fetch.spec.whatwg.org/#cors-protocol), which is enabled by
-specifying a list of allowed CORS origins in the `cors_allowed_origins` and/or
-`cors_allowed_headers` fields:
+specifying a list of allowed CORS origins in the `cors_allowed_origins`
+and optionally headers in `cors_allowed_headers`:
 
 ```yaml
 receivers:

receiver/otlpreceiver/config_test.go

@@ -185,6 +185,7 @@ func TestLoadConfig(t *testing.T) {
 			Protocols: Protocols{
 				HTTP: &confighttp.HTTPServerSettings{
 					Endpoint:    "0.0.0.0:55681",
+					CorsOrigins: []string{"https://*.test.com", "https://test.com"},
 					CorsHeaders: []string{"ExampleHeader"},
 				},
 			},

receiver/otlpreceiver/otlp.go

@@ -143,6 +143,7 @@ func (r *otlpReceiver) startProtocolServers(host component.Host) error {
 		r.serverHTTP = r.cfg.HTTP.ToServer(
 			r.gatewayMux,
 			confighttp.WithErrorHandler(errorHandler),
+			confighttp.WithLogger(r.logger),
 		)
 		err = r.startHTTPServer(r.cfg.HTTP, host)
 		if err != nil {

receiver/otlpreceiver/testdata/config.yaml

@@ -82,6 +82,9 @@ receivers:
   otlp/corsheader:
     protocols:
       http:
+        cors_allowed_origins:
+          - https://*.test.com # Wildcard subdomain. Allows domains like https://www.test.com and https://foo.test.com but not https://wwwtest.com.
+          - https://test.com # Fully qualified domain name. Allows https://test.com only.
         cors_allowed_headers:
           - ExampleHeader
 processors: