Backend plugins: Prepare and clean request headers before resource calls (#22321)

Moves common request proxy utilities to proxyutil package with
support for removing X-Forwarded-Host, X-Forwarded-Port,
X-Forwarded-Proto headers, setting X-Forwarded-For header
and cleaning Cookie header.
Using the proxyutil package to prepare and clean request
headers before resource calls.

Closes #21512

pkg/api/api.go

@@ -252,8 +252,8 @@ func (hs *HTTPServer) registerRoutes() {
 		apiRoute.Get("/plugins/:pluginId/settings", Wrap(GetPluginSettingByID))
 		apiRoute.Get("/plugins/:pluginId/markdown/:name", Wrap(GetPluginMarkdown))
 		apiRoute.Get("/plugins/:pluginId/health", Wrap(hs.CheckHealth))
-		apiRoute.Any("/plugins/:pluginId/resources", Wrap(hs.CallResource))
+		apiRoute.Any("/plugins/:pluginId/resources", hs.CallResource)
-		apiRoute.Any("/plugins/:pluginId/resources/*", Wrap(hs.CallResource))
+		apiRoute.Any("/plugins/:pluginId/resources/*", hs.CallResource)
 
 		apiRoute.Group("/plugins", func(pluginRoute routing.RouteRegister) {
 			pluginRoute.Get("/:pluginId/dashboards/", Wrap(GetPluginDashboards))
@@ -263,8 +263,8 @@ func (hs *HTTPServer) registerRoutes() {
 		apiRoute.Get("/frontend/settings/", hs.GetFrontendSettings)
 		apiRoute.Any("/datasources/proxy/:id/*", reqSignedIn, hs.ProxyDataSourceRequest)
 		apiRoute.Any("/datasources/proxy/:id", reqSignedIn, hs.ProxyDataSourceRequest)
-		apiRoute.Any("/datasources/:id/resources", Wrap(hs.CallDatasourceResource))
+		apiRoute.Any("/datasources/:id/resources", hs.CallDatasourceResource)
-		apiRoute.Any("/datasources/:id/resources/*", Wrap(hs.CallDatasourceResource))
+		apiRoute.Any("/datasources/:id/resources/*", hs.CallDatasourceResource)
 
 		// Folders
 		apiRoute.Group("/folders", func(folderRoute routing.RouteRegister) {

pkg/api/datasources.go

@@ -255,38 +255,30 @@ func GetDataSourceIdByName(c *m.ReqContext) Response {
 }
 
 // /api/datasources/:id/resources/*
-func (hs *HTTPServer) CallDatasourceResource(c *m.ReqContext) Response {
+func (hs *HTTPServer) CallDatasourceResource(c *m.ReqContext) {
 	datasourceID := c.ParamsInt64(":id")
 	ds, err := hs.DatasourceCache.GetDatasource(datasourceID, c.SignedInUser, c.SkipCache)
 	if err != nil {
 		if err == m.ErrDataSourceAccessDenied {
-			return Error(403, "Access denied to datasource", err)
+			c.JsonApiErr(403, "Access denied to datasource", err)
+			return
 		}
-		return Error(500, "Unable to load datasource meta data", err)
+		c.JsonApiErr(500, "Unable to load datasource meta data", err)
+		return
 	}
 
 	// find plugin
 	plugin, ok := plugins.DataSources[ds.Type]
 	if !ok {
-		return Error(500, "Unable to find datasource plugin", err)
+		c.JsonApiErr(500, "Unable to find datasource plugin", err)
+		return
 	}
 
-	body, err := c.Req.Body().Bytes()
+	config := backendplugin.PluginConfig{
-	if err != nil {
-		return Error(500, "Failed to read request body", err)
-	}
-
-	jsonDataBytes, err := ds.JsonData.MarshalJSON()
-	if err != nil {
-		return Error(500, "Failed to marshal JSON data to bytes", err)
-	}
-
-	req := backendplugin.CallResourceRequest{
-		Config: backendplugin.PluginConfig{
 		OrgID:                   c.OrgId,
 		PluginID:                plugin.Id,
 		PluginType:              plugin.Type,
-			JSONData:                jsonDataBytes,
+		JSONData:                ds.JsonData,
 		DecryptedSecureJSONData: ds.DecryptedValues(),
 		Updated:                 ds.Updated,
 		DataSourceConfig: &backendplugin.DataSourceConfig{
@@ -298,27 +290,8 @@ func (hs *HTTPServer) CallDatasourceResource(c *m.ReqContext) Response {
 			BasicAuthEnabled: ds.BasicAuth,
 			BasicAuthUser:    ds.BasicAuthUser,
 		},
-		},
-		Path:    c.Params("*"),
-		Method:  c.Req.Method,
-		URL:     c.Req.URL.String(),
-		Headers: c.Req.Header.Clone(),
-		Body:    body,
-	}
-	resp, err := hs.BackendPluginManager.CallResource(c.Req.Context(), req)
-	if err != nil {
-		return Error(500, "Failed to call datasource resource", err)
-	}
-
-	if resp.Status >= 400 {
-		return Error(resp.Status, "", nil)
-	}
-
-	return &NormalResponse{
-		body:   resp.Body,
-		status: resp.Status,
-		header: resp.Headers,
 	}
+	hs.BackendPluginManager.CallResource(config, c, c.Params("*"))
 }
 
 func convertModelToDtos(ds *m.DataSource) dtos.DataSource {

pkg/api/pluginproxy/ds_proxy.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"io/ioutil"
 	"log"
-	"net"
 	"net/http"
 	"net/http/httputil"
 	"net/url"
@@ -24,6 +23,7 @@ import (
 	"github.com/grafana/grafana/pkg/plugins"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/util"
+	"github.com/grafana/grafana/pkg/util/proxyutil"
 )
 
 var (
@@ -185,48 +185,22 @@ func (proxy *DataSourceProxy) getDirector() func(req *http.Request) {
 			req.Header.Add("X-Grafana-User", proxy.ctx.SignedInUser.Login)
 		}
 
-		// clear cookie header, except for whitelisted cookies
+		keepCookieNames := []string{}
-		var keptCookies []*http.Cookie
 		if proxy.ds.JsonData != nil {
 			if keepCookies := proxy.ds.JsonData.Get("keepCookies"); keepCookies != nil {
-				keepCookieNames := keepCookies.MustStringArray()
+				keepCookieNames = keepCookies.MustStringArray()
-				for _, c := range req.Cookies() {
-					for _, v := range keepCookieNames {
-						if c.Name == v {
-							keptCookies = append(keptCookies, c)
 			}
 		}
-				}
-			}
-		}
-		req.Header.Del("Cookie")
-		for _, c := range keptCookies {
-			req.AddCookie(c)
-		}
 
-		// clear X-Forwarded Host/Port/Proto headers
+		proxyutil.ClearCookieHeader(req, keepCookieNames)
-		req.Header.Del("X-Forwarded-Host")
+		proxyutil.PrepareProxyRequest(req)
-		req.Header.Del("X-Forwarded-Port")
+
-		req.Header.Del("X-Forwarded-Proto")
 		req.Header.Set("User-Agent", fmt.Sprintf("Grafana/%s", setting.BuildVersion))
 
 		// Clear Origin and Referer to avoir CORS issues
 		req.Header.Del("Origin")
 		req.Header.Del("Referer")
 
-		// set X-Forwarded-For header
-		if req.RemoteAddr != "" {
-			remoteAddr, _, err := net.SplitHostPort(req.RemoteAddr)
-			if err != nil {
-				remoteAddr = req.RemoteAddr
-			}
-			if req.Header.Get("X-Forwarded-For") != "" {
-				req.Header.Set("X-Forwarded-For", req.Header.Get("X-Forwarded-For")+", "+remoteAddr)
-			} else {
-				req.Header.Set("X-Forwarded-For", remoteAddr)
-			}
-		}
-
 		if proxy.route != nil {
 			ApplyRoute(proxy.ctx.Req.Context(), req, proxy.proxyPath, proxy.route, proxy.ds)
 		}

pkg/api/pluginproxy/pluginproxy.go

@@ -2,18 +2,17 @@ package pluginproxy
 
 import (
 	"encoding/json"
-	"net"
 	"net/http"
 	"net/http/httputil"
 	"net/url"
 
-	"github.com/grafana/grafana/pkg/setting"
-
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/infra/log"
 	m "github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/plugins"
+	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/util"
+	"github.com/grafana/grafana/pkg/util/proxyutil"
 )
 
 type templateData struct {
@@ -71,23 +70,7 @@ func NewApiPluginProxy(ctx *m.ReqContext, proxyPath string, route *plugins.AppPl
 		req.Header.Del("Cookie")
 		req.Header.Del("Set-Cookie")
 
-		// clear X-Forwarded Host/Port/Proto headers
+		proxyutil.PrepareProxyRequest(req)
-		req.Header.Del("X-Forwarded-Host")
-		req.Header.Del("X-Forwarded-Port")
-		req.Header.Del("X-Forwarded-Proto")
-
-		// set X-Forwarded-For header
-		if req.RemoteAddr != "" {
-			remoteAddr, _, err := net.SplitHostPort(req.RemoteAddr)
-			if err != nil {
-				remoteAddr = req.RemoteAddr
-			}
-			if req.Header.Get("X-Forwarded-For") != "" {
-				req.Header.Set("X-Forwarded-For", req.Header.Get("X-Forwarded-For")+", "+remoteAddr)
-			} else {
-				req.Header.Set("X-Forwarded-For", remoteAddr)
-			}
-		}
 
 		// Create a HTTP header with the context in it.
 		ctxJSON, err := json.Marshal(ctx.SignedInUser)

pkg/api/plugins.go

@@ -1,16 +1,16 @@
 package api
 
 import (
-	"encoding/json"
 	"sort"
 	"time"
 
-	"github.com/grafana/grafana/pkg/plugins/backendplugin"
+	"github.com/grafana/grafana/pkg/components/simplejson"
 
 	"github.com/grafana/grafana/pkg/api/dtos"
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/plugins"
+	"github.com/grafana/grafana/pkg/plugins/backendplugin"
 	"github.com/grafana/grafana/pkg/setting"
 )
 
@@ -238,66 +238,40 @@ func (hs *HTTPServer) CheckHealth(c *models.ReqContext) Response {
 }
 
 // /api/plugins/:pluginId/resources/*
-func (hs *HTTPServer) CallResource(c *models.ReqContext) Response {
+func (hs *HTTPServer) CallResource(c *models.ReqContext) {
 	pluginID := c.Params("pluginId")
 	plugin, exists := plugins.Plugins[pluginID]
 	if !exists {
-		return Error(404, "Plugin not found, no installed plugin with that id", nil)
+		c.JsonApiErr(404, "Plugin not found, no installed plugin with that id", nil)
+		return
 	}
 
-	var jsonDataBytes []byte
+	var jsonData *simplejson.Json
 	var decryptedSecureJSONData map[string]string
 	var updated time.Time
 
 	ps, err := hs.getCachedPluginSettings(pluginID, c.SignedInUser)
 	if err != nil {
 		if err != models.ErrPluginSettingNotFound {
-			return Error(500, "Failed to get plugin settings", err)
+			c.JsonApiErr(500, "Failed to get plugin settings", err)
+			return
 		}
+		jsonData = simplejson.New()
+		decryptedSecureJSONData = make(map[string]string)
 	} else {
-		jsonDataBytes, err = json.Marshal(&ps.JsonData)
-		if err != nil {
-			return Error(500, "Failed to marshal JSON data to bytes", err)
-		}
-
 		decryptedSecureJSONData = ps.DecryptedValues()
 		updated = ps.Updated
 	}
 
-	body, err := c.Req.Body().Bytes()
+	config := backendplugin.PluginConfig{
-	if err != nil {
-		return Error(500, "Failed to read request body", err)
-	}
-
-	req := backendplugin.CallResourceRequest{
-		Config: backendplugin.PluginConfig{
 		OrgID:                   c.OrgId,
 		PluginID:                plugin.Id,
 		PluginType:              plugin.Type,
-			JSONData:                jsonDataBytes,
+		JSONData:                jsonData,
 		DecryptedSecureJSONData: decryptedSecureJSONData,
 		Updated:                 updated,
-		},
-		Path:    c.Params("*"),
-		Method:  c.Req.Method,
-		URL:     c.Req.URL.String(),
-		Headers: c.Req.Header.Clone(),
-		Body:    body,
-	}
-	resp, err := hs.BackendPluginManager.CallResource(c.Req.Context(), req)
-	if err != nil {
-		return Error(500, "Failed to call resource", err)
-	}
-
-	if resp.Status >= 400 {
-		return Error(resp.Status, "", nil)
-	}
-
-	return &NormalResponse{
-		body:   resp.Body,
-		status: resp.Status,
-		header: resp.Headers,
 	}
+	hs.BackendPluginManager.CallResource(config, c, c.Params("*"))
 }
 
 func (hs *HTTPServer) getCachedPluginSettings(pluginID string, user *models.SignedInUser) (*models.PluginSetting, error) {

pkg/plugins/backendplugin/backend_plugin.go

@@ -205,12 +205,17 @@ func (p *BackendPlugin) callResource(ctx context.Context, req CallResourceReques
 		reqHeaders[k] = &pluginv2.CallResource_StringList{Values: v}
 	}
 
+	jsonDataBytes, err := req.Config.JSONData.ToDB()
+	if err != nil {
+		return nil, err
+	}
+
 	protoReq := &pluginv2.CallResource_Request{
 		Config: &pluginv2.PluginConfig{
 			OrgId:                   req.Config.OrgID,
 			PluginId:                req.Config.PluginID,
 			PluginType:              req.Config.PluginType,
-			JsonData:                req.Config.JSONData,
+			JsonData:                jsonDataBytes,
 			DecryptedSecureJsonData: req.Config.DecryptedSecureJSONData,
 			UpdatedMS:               req.Config.Updated.UnixNano() / int64(time.Millisecond),
 		},

pkg/plugins/backendplugin/contracts.go

@@ -1,10 +1,11 @@
 package backendplugin
 
 import (
-	"encoding/json"
 	"strconv"
 	"time"
 
+	"github.com/grafana/grafana/pkg/components/simplejson"
+
 	"github.com/grafana/grafana-plugin-sdk-go/genproto/pluginv2"
 )
 
@@ -69,7 +70,7 @@ type PluginConfig struct {
 	OrgID                   int64
 	PluginID                string
 	PluginType              string
-	JSONData                json.RawMessage
+	JSONData                *simplejson.Json
 	DecryptedSecureJSONData map[string]string
 	Updated                 time.Time
 	DataSourceConfig        *DataSourceConfig

pkg/plugins/backendplugin/manager.go

@@ -6,6 +6,9 @@ import (
 	"sync"
 	"time"
 
+	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/util/proxyutil"
+
 	"github.com/prometheus/client_golang/prometheus"
 
 	"github.com/grafana/grafana/pkg/infra/log"
@@ -41,7 +44,7 @@ type Manager interface {
 	// CheckHealth checks the health of a registered backend plugin.
 	CheckHealth(ctx context.Context, pluginID string) (*CheckHealthResult, error)
 	// CallResource calls a plugin resource.
-	CallResource(ctx context.Context, req CallResourceRequest) (*CallResourceResult, error)
+	CallResource(pluginConfig PluginConfig, ctx *models.ReqContext, path string)
 }
 
 type manager struct {
@@ -170,18 +173,46 @@ func (m *manager) CheckHealth(ctx context.Context, pluginID string) (*CheckHealt
 }
 
 // CallResource calls a plugin resource.
-func (m *manager) CallResource(ctx context.Context, req CallResourceRequest) (*CallResourceResult, error) {
+func (m *manager) CallResource(config PluginConfig, c *models.ReqContext, path string) {
 	m.pluginsMu.RLock()
-	p, registered := m.plugins[req.Config.PluginID]
+	p, registered := m.plugins[config.PluginID]
 	m.pluginsMu.RUnlock()
 
 	if !registered {
-		return nil, ErrPluginNotRegistered
+		c.JsonApiErr(404, "Plugin not registered", nil)
+		return
 	}
 
-	res, err := p.callResource(ctx, req)
+	clonedReq := c.Req.Clone(c.Req.Context())
+	keepCookieNames := []string{}
+	if config.JSONData != nil {
+		if keepCookies := config.JSONData.Get("keepCookies"); keepCookies != nil {
+			keepCookieNames = keepCookies.MustStringArray()
+		}
+	}
+
+	proxyutil.ClearCookieHeader(clonedReq, keepCookieNames)
+	proxyutil.PrepareProxyRequest(clonedReq)
+
+	body, err := c.Req.Body().Bytes()
 	if err != nil {
-		return nil, err
+		c.JsonApiErr(500, "Failed to read request body", err)
+		return
+	}
+
+	req := CallResourceRequest{
+		Config:  config,
+		Path:    path,
+		Method:  clonedReq.Method,
+		URL:     clonedReq.URL.String(),
+		Headers: clonedReq.Header,
+		Body:    body,
+	}
+
+	res, err := p.callResource(clonedReq.Context(), req)
+	if err != nil {
+		c.JsonApiErr(500, "Failed to call resource", err)
+		return
 	}
 
 	// Make sure a content type always is returned in response
@@ -189,7 +220,20 @@ func (m *manager) CallResource(ctx context.Context, req CallResourceRequest) (*C
 		res.Headers["Content-Type"] = []string{"application/json"}
 	}
 
-	return res, nil
+	for k, values := range res.Headers {
+		if k == "Set-Cookie" {
+			continue
+		}
+
+		for _, v := range values {
+			c.Resp.Header().Add(k, v)
+		}
+	}
+
+	c.WriteHeader(res.Status)
+	if _, err := c.Write(res.Body); err != nil {
+		p.logger.Error("Failed to write resource response", "error", err)
+	}
 }
 
 func startPluginAndRestartKilledProcesses(ctx context.Context, p *BackendPlugin) error {

pkg/util/proxyutil/proxyutil.go

@@ -0,0 +1,44 @@
+package proxyutil
+
+import (
+	"net"
+	"net/http"
+)
+
+// PrepareProxyRequest prepares a request for being proxied.
+// Removes X-Forwarded-Host, X-Forwarded-Port, X-Forwarded-Proto headers.
+// Set X-Forwarded-For headers.
+func PrepareProxyRequest(req *http.Request) {
+	req.Header.Del("X-Forwarded-Host")
+	req.Header.Del("X-Forwarded-Port")
+	req.Header.Del("X-Forwarded-Proto")
+
+	if req.RemoteAddr != "" {
+		remoteAddr, _, err := net.SplitHostPort(req.RemoteAddr)
+		if err != nil {
+			remoteAddr = req.RemoteAddr
+		}
+		if req.Header.Get("X-Forwarded-For") != "" {
+			req.Header.Set("X-Forwarded-For", req.Header.Get("X-Forwarded-For")+", "+remoteAddr)
+		} else {
+			req.Header.Set("X-Forwarded-For", remoteAddr)
+		}
+	}
+}
+
+// ClearCookieHeader clear cookie header, except for cookies specified to be kept.
+func ClearCookieHeader(req *http.Request, keepCookiesNames []string) {
+	var keepCookies []*http.Cookie
+	for _, c := range req.Cookies() {
+		for _, v := range keepCookiesNames {
+			if c.Name == v {
+				keepCookies = append(keepCookies, c)
+			}
+		}
+	}
+
+	req.Header.Del("Cookie")
+	for _, c := range keepCookies {
+		req.AddCookie(c)
+	}
+}

pkg/util/proxyutil/proxyutil_test.go

@@ -0,0 +1,67 @@
+package proxyutil
+
+import (
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestPrepareProxyRequest(t *testing.T) {
+	t.Run("Prepare proxy request should clear X-Forwarded headers", func(t *testing.T) {
+		req, err := http.NewRequest(http.MethodGet, "/", nil)
+		require.NoError(t, err)
+		req.Header.Add("X-Forwarded-Host", "host")
+		req.Header.Add("X-Forwarded-Port", "123")
+		req.Header.Add("X-Forwarded-Proto", "http1")
+
+		PrepareProxyRequest(req)
+		require.NotContains(t, req.Header, "X-Forwarded-Host")
+		require.NotContains(t, req.Header, "X-Forwarded-Port")
+		require.NotContains(t, req.Header, "X-Forwarded-Proto")
+	})
+
+	t.Run("Prepare proxy request should set X-Forwarded-For", func(t *testing.T) {
+		req, err := http.NewRequest(http.MethodGet, "/", nil)
+		req.RemoteAddr = "127.0.0.1:1234"
+		require.NoError(t, err)
+
+		PrepareProxyRequest(req)
+		require.Contains(t, req.Header, "X-Forwarded-For")
+		require.Equal(t, "127.0.0.1", req.Header.Get("X-Forwarded-For"))
+	})
+
+	t.Run("Prepare proxy request should appent client ip at the end of X-Forwarded-For", func(t *testing.T) {
+		req, err := http.NewRequest(http.MethodGet, "/", nil)
+		req.RemoteAddr = "127.0.0.1:1234"
+		req.Header.Add("X-Forwarded-For", "192.168.0.1")
+		require.NoError(t, err)
+
+		PrepareProxyRequest(req)
+		require.Contains(t, req.Header, "X-Forwarded-For")
+		require.Equal(t, "192.168.0.1, 127.0.0.1", req.Header.Get("X-Forwarded-For"))
+	})
+}
+
+func TestClearCookieHeader(t *testing.T) {
+	t.Run("Clear cookie header should clear Cookie header", func(t *testing.T) {
+		req, err := http.NewRequest(http.MethodGet, "/", nil)
+		require.NoError(t, err)
+		req.AddCookie(&http.Cookie{Name: "cookie"})
+
+		ClearCookieHeader(req, nil)
+		require.NotContains(t, req.Header, "Cookie")
+	})
+
+	t.Run("Clear cookie header with cookies to keep should clear Cookie header and keep cookies", func(t *testing.T) {
+		req, err := http.NewRequest(http.MethodGet, "/", nil)
+		require.NoError(t, err)
+		req.AddCookie(&http.Cookie{Name: "cookie1"})
+		req.AddCookie(&http.Cookie{Name: "cookie2"})
+		req.AddCookie(&http.Cookie{Name: "cookie3"})
+
+		ClearCookieHeader(req, []string{"cookie1", "cookie3"})
+		require.Contains(t, req.Header, "Cookie")
+		require.Equal(t, "cookie1=; cookie3=", req.Header.Get("Cookie"))
+	})
+}