diff --git a/pkg/api/datasources.go b/pkg/api/datasources.go index 8f181f32dcf..ecc0fdc0108 100644 --- a/pkg/api/datasources.go +++ b/pkg/api/datasources.go @@ -826,7 +826,7 @@ func (hs *HTTPServer) checkDatasourceHealth(c *models.ReqContext, ds *datasource } } - proxyutil.ClearCookieHeader(c.Req, ds.AllowedCookies()) + proxyutil.ClearCookieHeader(c.Req, ds.AllowedCookies(), []string{hs.Cfg.LoginCookieName}) if cookieStr := c.Req.Header.Get("Cookie"); cookieStr != "" { req.Headers["Cookie"] = cookieStr } diff --git a/pkg/api/metrics_test.go b/pkg/api/metrics_test.go index 95c6fb8040d..97ca79dffb2 100644 --- a/pkg/api/metrics_test.go +++ b/pkg/api/metrics_test.go @@ -15,6 +15,7 @@ import ( "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/quota/quotatest" + "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/web/webtest" "golang.org/x/oauth2" @@ -68,7 +69,7 @@ func (ts *fakeOAuthTokenService) IsOAuthPassThruEnabled(*datasources.DataSource) // `/ds/query` endpoint test func TestAPIEndpoint_Metrics_QueryMetricsV2(t *testing.T) { qds := query.ProvideService( - nil, + setting.NewCfg(), nil, nil, &fakePluginRequestValidator{}, @@ -117,7 +118,7 @@ func TestAPIEndpoint_Metrics_QueryMetricsV2(t *testing.T) { func TestAPIEndpoint_Metrics_PluginDecryptionFailure(t *testing.T) { qds := query.ProvideService( - nil, + setting.NewCfg(), nil, nil, &fakePluginRequestValidator{}, diff --git a/pkg/api/plugin_resource.go b/pkg/api/plugin_resource.go index d6e818d3e94..537e9c01b82 100644 --- a/pkg/api/plugin_resource.go +++ b/pkg/api/plugin_resource.go @@ -15,7 +15,6 @@ import ( "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/plugins/backendplugin" - "github.com/grafana/grafana/pkg/services/contexthandler" "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/util/proxyutil" "github.com/grafana/grafana/pkg/web" @@ -119,15 +118,7 @@ func (hs *HTTPServer) makePluginResourceRequest(w http.ResponseWriter, req *http hs.log.Warn("failed to unpack JSONData in datasource instance settings", "err", err) } } - - list := contexthandler.AuthHTTPHeaderListFromContext(req.Context()) - if list != nil { - for _, name := range list.Items { - req.Header.Del(name) - } - } - - proxyutil.ClearCookieHeader(req, keepCookieModel.KeepCookies) + proxyutil.ClearCookieHeader(req, keepCookieModel.KeepCookies, []string{hs.Cfg.LoginCookieName}) proxyutil.PrepareProxyRequest(req) body, err := ioutil.ReadAll(req.Body) diff --git a/pkg/api/pluginproxy/ds_proxy.go b/pkg/api/pluginproxy/ds_proxy.go index 368bb858f06..7d025c23200 100644 --- a/pkg/api/pluginproxy/ds_proxy.go +++ b/pkg/api/pluginproxy/ds_proxy.go @@ -223,7 +223,7 @@ func (proxy *DataSourceProxy) director(req *http.Request) { applyUserHeader(proxy.cfg.SendUserHeader, req, proxy.ctx.SignedInUser) - proxyutil.ClearCookieHeader(req, proxy.ds.AllowedCookies()) + proxyutil.ClearCookieHeader(req, proxy.ds.AllowedCookies(), []string{proxy.cfg.LoginCookieName}) req.Header.Set("User-Agent", fmt.Sprintf("Grafana/%s", setting.BuildVersion)) jsonData := make(map[string]interface{}) diff --git a/pkg/infra/httpclient/httpclientprovider/forwarded_cookie_middleware_test.go b/pkg/infra/httpclient/httpclientprovider/forwarded_cookie_middleware_test.go index 07753e0c0f5..c9eb1767e01 100644 --- a/pkg/infra/httpclient/httpclientprovider/forwarded_cookie_middleware_test.go +++ b/pkg/infra/httpclient/httpclientprovider/forwarded_cookie_middleware_test.go @@ -13,6 +13,7 @@ func TestForwardedCookiesMiddleware(t *testing.T) { tcs := []struct { desc string allowedCookies []string + disallowedCookies []string expectedCookieHeader string }{ { @@ -30,6 +31,12 @@ func TestForwardedCookiesMiddleware(t *testing.T) { allowedCookies: []string{"c1", "c3"}, expectedCookieHeader: "c1=1; c3=3", }, + { + desc: "When provided with allowed and not allowed cookies should populate Cookie header", + allowedCookies: []string{"c1", "c3"}, + disallowedCookies: []string{"c1"}, + expectedCookieHeader: "c3=3", + }, } for _, tc := range tcs { @@ -41,7 +48,7 @@ func TestForwardedCookiesMiddleware(t *testing.T) { {Name: "c2", Value: "2"}, {Name: "c3", Value: "3"}, } - mw := httpclientprovider.ForwardedCookiesMiddleware(forwarded, tc.allowedCookies) + mw := httpclientprovider.ForwardedCookiesMiddleware(forwarded, tc.allowedCookies, tc.disallowedCookies) opts := httpclient.Options{} rt := mw.CreateMiddleware(opts, finalRoundTripper) require.NotNil(t, rt) diff --git a/pkg/infra/httpclient/httpclientprovider/forwarded_cookies_middleware.go b/pkg/infra/httpclient/httpclientprovider/forwarded_cookies_middleware.go index 7d0ef6f2ef5..90c8314bcad 100644 --- a/pkg/infra/httpclient/httpclientprovider/forwarded_cookies_middleware.go +++ b/pkg/infra/httpclient/httpclientprovider/forwarded_cookies_middleware.go @@ -11,13 +11,13 @@ const ForwardedCookiesMiddlewareName = "forwarded-cookies" // ForwardedCookiesMiddleware middleware that sets Cookie header on the // outgoing request, if forwarded cookies configured/provided. -func ForwardedCookiesMiddleware(forwardedCookies []*http.Cookie, allowedCookies []string) httpclient.Middleware { +func ForwardedCookiesMiddleware(forwardedCookies []*http.Cookie, allowedCookies []string, disallowedCookies []string) httpclient.Middleware { return httpclient.NamedMiddlewareFunc(ForwardedCookiesMiddlewareName, func(opts httpclient.Options, next http.RoundTripper) http.RoundTripper { return httpclient.RoundTripperFunc(func(req *http.Request) (*http.Response, error) { for _, cookie := range forwardedCookies { req.AddCookie(cookie) } - proxyutil.ClearCookieHeader(req, allowedCookies) + proxyutil.ClearCookieHeader(req, allowedCookies, disallowedCookies) return next.RoundTrip(req) }) }) diff --git a/pkg/services/query/query.go b/pkg/services/query/query.go index 5f92a121492..e7c0afb3546 100644 --- a/pkg/services/query/query.go +++ b/pkg/services/query/query.go @@ -162,7 +162,7 @@ func (s *Service) handleQueryData(ctx context.Context, user *models.SignedInUser middlewares := []httpclient.Middleware{} if parsedReq.httpRequest != nil { middlewares = append(middlewares, - httpclientprovider.ForwardedCookiesMiddleware(parsedReq.httpRequest.Cookies(), ds.AllowedCookies()), + httpclientprovider.ForwardedCookiesMiddleware(parsedReq.httpRequest.Cookies(), ds.AllowedCookies(), []string{s.cfg.LoginCookieName}), ) } @@ -179,7 +179,7 @@ func (s *Service) handleQueryData(ctx context.Context, user *models.SignedInUser } if parsedReq.httpRequest != nil { - proxyutil.ClearCookieHeader(parsedReq.httpRequest, ds.AllowedCookies()) + proxyutil.ClearCookieHeader(parsedReq.httpRequest, ds.AllowedCookies(), []string{s.cfg.LoginCookieName}) if cookieStr := parsedReq.httpRequest.Header.Get("Cookie"); cookieStr != "" { req.Headers["Cookie"] = cookieStr } diff --git a/pkg/services/query/query_test.go b/pkg/services/query/query_test.go index 09dba33770c..3955642b9ff 100644 --- a/pkg/services/query/query_test.go +++ b/pkg/services/query/query_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/grafana/grafana-plugin-sdk-go/backend" + "github.com/grafana/grafana/pkg/setting" "github.com/stretchr/testify/require" "golang.org/x/oauth2" @@ -99,7 +100,7 @@ func setup(t *testing.T) *testContext { dataSourceCache: dc, oauthTokenService: tc, pluginRequestValidator: rv, - queryService: query.ProvideService(nil, dc, nil, rv, ds, pc, tc), + queryService: query.ProvideService(setting.NewCfg(), dc, nil, rv, ds, pc, tc), } } diff --git a/pkg/util/proxyutil/proxyutil.go b/pkg/util/proxyutil/proxyutil.go index 3db22a1426e..ee56120cc6f 100644 --- a/pkg/util/proxyutil/proxyutil.go +++ b/pkg/util/proxyutil/proxyutil.go @@ -3,6 +3,7 @@ package proxyutil import ( "net" "net/http" + "sort" ) // PrepareProxyRequest prepares a request for being proxied. @@ -26,19 +27,31 @@ func PrepareProxyRequest(req *http.Request) { } } -// ClearCookieHeader clear cookie header, except for cookies specified to be kept. -func ClearCookieHeader(req *http.Request, keepCookiesNames []string) { - var keepCookies []*http.Cookie +// ClearCookieHeader clear cookie header, except for cookies specified to be kept (keepCookiesNames) if not in skipCookiesNames. +func ClearCookieHeader(req *http.Request, keepCookiesNames []string, skipCookiesNames []string) { + keepCookies := map[string]*http.Cookie{} for _, c := range req.Cookies() { for _, v := range keepCookiesNames { if c.Name == v { - keepCookies = append(keepCookies, c) + keepCookies[c.Name] = c } } } + for _, v := range skipCookiesNames { + delete(keepCookies, v) + } + req.Header.Del("Cookie") - for _, c := range keepCookies { + + sortedCookies := []string{} + for name := range keepCookies { + sortedCookies = append(sortedCookies, name) + } + sort.Strings(sortedCookies) + + for _, name := range sortedCookies { + c := keepCookies[name] req.AddCookie(c) } } diff --git a/pkg/util/proxyutil/proxyutil_test.go b/pkg/util/proxyutil/proxyutil_test.go index 5ff61ec1d29..03d816bbcd8 100644 --- a/pkg/util/proxyutil/proxyutil_test.go +++ b/pkg/util/proxyutil/proxyutil_test.go @@ -49,7 +49,7 @@ func TestClearCookieHeader(t *testing.T) { require.NoError(t, err) req.AddCookie(&http.Cookie{Name: "cookie"}) - ClearCookieHeader(req, nil) + ClearCookieHeader(req, nil, nil) require.NotContains(t, req.Header, "Cookie") }) @@ -60,8 +60,20 @@ func TestClearCookieHeader(t *testing.T) { req.AddCookie(&http.Cookie{Name: "cookie2"}) req.AddCookie(&http.Cookie{Name: "cookie3"}) - ClearCookieHeader(req, []string{"cookie1", "cookie3"}) + ClearCookieHeader(req, []string{"cookie1", "cookie3"}, nil) require.Contains(t, req.Header, "Cookie") require.Equal(t, "cookie1=; cookie3=", req.Header.Get("Cookie")) }) + + t.Run("Clear cookie header with cookies to keep and skip 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"}, []string{"cookie3"}) + require.Contains(t, req.Header, "Cookie") + require.Equal(t, "cookie1=", req.Header.Get("Cookie")) + }) }