Security: Fix do not forward login cookie in outgoing requests

(cherry picked from commit 54a32fc83b233f5910495b5fcca0b4f881221538)

Author: marefr

pkg/api/metrics_test.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
+	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/web/webtest"
 
 	"golang.org/x/oauth2"
@@ -498,7 +499,7 @@ func TestAPIEndpoint_Metrics_ParseDashboardQueryParams(t *testing.T) {
 // `/ds/query` endpoint test
 func TestAPIEndpoint_Metrics_QueryMetricsV2(t *testing.T) {
 	qds := query.ProvideService(
-		nil,
+		setting.NewCfg(),
 		nil,
 		nil,
 		&fakePluginRequestValidator{},

pkg/api/pluginproxy/ds_proxy.go

@@ -214,7 +214,7 @@ func (proxy *DataSourceProxy) director(req *http.Request) {
 		}
 	}
 
-	proxyutil.ClearCookieHeader(req, keepCookieNames)
+	proxyutil.ClearCookieHeader(req, keepCookieNames, []string{proxy.cfg.LoginCookieName})
 	req.Header.Set("User-Agent", fmt.Sprintf("Grafana/%s", setting.BuildVersion))
 
 	jsonData := make(map[string]interface{})

pkg/api/plugins.go

@@ -555,7 +555,7 @@ func (hs *HTTPServer) makePluginResourceRequest(w http.ResponseWriter, req *http
 		}
 	}
 
-	proxyutil.ClearCookieHeader(req, keepCookieModel.KeepCookies)
+	proxyutil.ClearCookieHeader(req, keepCookieModel.KeepCookies, []string{hs.Cfg.LoginCookieName})
 	proxyutil.PrepareProxyRequest(req)
 
 	body, err := ioutil.ReadAll(req.Body)

pkg/services/query/query_test.go

@@ -5,9 +5,9 @@ import (
 	"net/http"
 	"testing"
 
+	"github.com/grafana/grafana-plugin-sdk-go/backend"
 	"golang.org/x/oauth2"
 
-	"github.com/grafana/grafana-plugin-sdk-go/backend"
 	"github.com/grafana/grafana/pkg/api/dtos"
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/models"

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)
 	}
 }

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"))
+	})
 }