httpclient: Don't forward HTTP headers by default (#679)

grafana · May 4, 2023 · 3e2ff58 · 3e2ff58
1 parent 691f934
commit 3e2ff58
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 19 deletions.
diff --git a/backend/data_adapter.go b/backend/data_adapter.go
@@ -23,6 +23,10 @@ func withHeaderMiddleware(ctx context.Context, headers http.Header) context.Cont
 	if len(headers) > 0 {
 		ctx = httpclient.WithContextualMiddleware(ctx,
 			httpclient.MiddlewareFunc(func(opts httpclient.Options, next http.RoundTripper) http.RoundTripper {
+				if !opts.ForwardHTTPHeaders {
+					return next
+				}
+
 				return httpclient.RoundTripperFunc(func(qreq *http.Request) (*http.Response, error) {
 					// Only set a header if it is not already set.
 					for k, v := range headers {

diff --git a/backend/data_adapter_test.go b/backend/data_adapter_test.go
@@ -64,26 +64,53 @@ func (f *fakeDataHandlerWithOAuth) QueryData(ctx context.Context, req *QueryData
 func TestQueryData(t *testing.T) {
 	handler := newFakeDataHandlerWithOAuth()
 	adapter := newDataSDKAdapter(handler)
-	ctx := context.Background()
-	_, err := adapter.QueryData(ctx, &pluginv2.QueryDataRequest{
-		Headers: map[string]string{
-			"Authorization": "Bearer 123",
-		},
-		PluginContext: &pluginv2.PluginContext{},
+
+	t.Run("When forward HTTP headers enabled should forward headers", func(t *testing.T) {
+		ctx := context.Background()
+		_, err := adapter.QueryData(ctx, &pluginv2.QueryDataRequest{
+			Headers: map[string]string{
+				"Authorization": "Bearer 123",
+			},
+			PluginContext: &pluginv2.PluginContext{},
+		})
+		require.NoError(t, err)
+
+		middlewares := httpclient.ContextualMiddlewareFromContext(handler.lastReq.Context())
+		require.Len(t, middlewares, 1)
+
+		reqClone := handler.lastReq.Clone(handler.lastReq.Context())
+		// clean up headers to be sure they are injected
+		reqClone.Header = http.Header{}
+
+		res, err := middlewares[0].CreateMiddleware(httpclient.Options{ForwardHTTPHeaders: true}, finalRoundTripper).RoundTrip(reqClone)
+		require.NoError(t, err)
+		require.NoError(t, res.Body.Close())
+		require.Len(t, reqClone.Header, 1)
+		require.Equal(t, "Bearer 123", reqClone.Header.Get("Authorization"))
+	})
+
+	t.Run("When forward HTTP headers disable should not forward headers", func(t *testing.T) {
+		ctx := context.Background()
+		_, err := adapter.QueryData(ctx, &pluginv2.QueryDataRequest{
+			Headers: map[string]string{
+				"Authorization": "Bearer 123",
+			},
+			PluginContext: &pluginv2.PluginContext{},
+		})
+		require.NoError(t, err)
+
+		middlewares := httpclient.ContextualMiddlewareFromContext(handler.lastReq.Context())
+		require.Len(t, middlewares, 1)
+
+		reqClone := handler.lastReq.Clone(handler.lastReq.Context())
+		// clean up headers to be sure they are injected
+		reqClone.Header = http.Header{}
+
+		res, err := middlewares[0].CreateMiddleware(httpclient.Options{ForwardHTTPHeaders: false}, finalRoundTripper).RoundTrip(reqClone)
+		require.NoError(t, err)
+		require.NoError(t, res.Body.Close())
+		require.Empty(t, reqClone.Header)
 	})
-	require.NoError(t, err)
-
-	middlewares := httpclient.ContextualMiddlewareFromContext(handler.lastReq.Context())
-	require.Len(t, middlewares, 1)
-
-	reqClone := handler.lastReq.Clone(handler.lastReq.Context())
-	// clean up headers to be sure they are injected
-	reqClone.Header = http.Header{}
-	res, err := middlewares[0].CreateMiddleware(httpclient.Options{}, finalRoundTripper).RoundTrip(reqClone)
-	require.NoError(t, err)
-	require.NoError(t, res.Body.Close())
-	require.Len(t, reqClone.Header, 1)
-	require.Equal(t, "Bearer 123", reqClone.Header.Get("Authorization"))
 }
 
 var finalRoundTripper = httpclient.RoundTripperFunc(func(req *http.Request) (*http.Response, error) {

diff --git a/backend/httpclient/options.go b/backend/httpclient/options.go
@@ -62,6 +62,13 @@ type Options struct {
 	// ConfigureTLSConfig optionally provide a ConfigureTLSConfigFunc
 	// to modify the created http.Client.
 	ConfigureTLSConfig ConfigureTLSConfigFunc
+
+	// ForwardHTTPHeaders enable forwarding of all HTTP headers
+	// included in backend.QueryDataRequest, backend.CallResourceRequest,
+	// backend.CheckHealthRequest, e.g. based on if Allowed cookies or
+	// Forward OAuth Identity is configured for the datasource or any
+	// other forwarded HTTP header from Grafana.
+	ForwardHTTPHeaders bool
 }
 
 // BasicAuthOptions basic authentication options.