From 56a4af87d706087ea42780a79f8043df1b5bc3ea Mon Sep 17 00:00:00 2001 From: "grafana-delivery-bot[bot]" <132647405+grafana-delivery-bot[bot]@users.noreply.github.com> Date: Tue, 4 Jun 2024 18:14:11 +0300 Subject: [PATCH] [v10.4.x] Plugins: Don't forward cookies for app plugins (#88711) Plugins: Don't forward cookies for app plugins (#88663) (cherry picked from commit 0af2931672beca9278acd11e8336033c48e9b1b5) Co-authored-by: Marcus Efraimsson --- .../clientmiddleware/cookies_middleware.go | 33 +++++----- .../cookies_middleware_test.go | 63 +++++++++++++++++++ 2 files changed, 82 insertions(+), 14 deletions(-) diff --git a/pkg/services/pluginsintegration/clientmiddleware/cookies_middleware.go b/pkg/services/pluginsintegration/clientmiddleware/cookies_middleware.go index acff24ce249..a11b2d5e35a 100644 --- a/pkg/services/pluginsintegration/clientmiddleware/cookies_middleware.go +++ b/pkg/services/pluginsintegration/clientmiddleware/cookies_middleware.go @@ -32,25 +32,30 @@ type CookiesMiddleware struct { func (m *CookiesMiddleware) applyCookies(ctx context.Context, pCtx backend.PluginContext, req any) error { reqCtx := contexthandler.FromContext(ctx) - // if request not for a datasource or no HTTP request context skip middleware - if req == nil || pCtx.DataSourceInstanceSettings == nil || reqCtx == nil || reqCtx.Req == nil { + allowedCookies := []string{} + // if no HTTP request context skip middleware + if req == nil || reqCtx == nil || reqCtx.Req == nil { return nil } - settings := pCtx.DataSourceInstanceSettings - jsonDataBytes, err := simplejson.NewJson(settings.JSONData) - if err != nil { - return err + if pCtx.DataSourceInstanceSettings != nil { + settings := pCtx.DataSourceInstanceSettings + jsonDataBytes, err := simplejson.NewJson(settings.JSONData) + if err != nil { + return err + } + + ds := &datasources.DataSource{ + ID: settings.ID, + OrgID: pCtx.OrgID, + JsonData: jsonDataBytes, + Updated: settings.Updated, + } + + allowedCookies = ds.AllowedCookies() } - ds := &datasources.DataSource{ - ID: settings.ID, - OrgID: pCtx.OrgID, - JsonData: jsonDataBytes, - Updated: settings.Updated, - } - - proxyutil.ClearCookieHeader(reqCtx.Req, ds.AllowedCookies(), m.skipCookiesNames) + proxyutil.ClearCookieHeader(reqCtx.Req, allowedCookies, m.skipCookiesNames) cookieStr := reqCtx.Req.Header.Get(cookieHeaderName) switch t := req.(type) { diff --git a/pkg/services/pluginsintegration/clientmiddleware/cookies_middleware_test.go b/pkg/services/pluginsintegration/clientmiddleware/cookies_middleware_test.go index 1ea74a5cd25..41822d6a4ba 100644 --- a/pkg/services/pluginsintegration/clientmiddleware/cookies_middleware_test.go +++ b/pkg/services/pluginsintegration/clientmiddleware/cookies_middleware_test.go @@ -151,4 +151,67 @@ func TestCookiesMiddleware(t *testing.T) { require.EqualValues(t, "cookie2=", cdt.CheckHealthReq.Headers[cookieHeaderName]) }) }) + + t.Run("When app", func(t *testing.T) { + req, err := http.NewRequest(http.MethodGet, "/some/thing", nil) + require.NoError(t, err) + req.AddCookie(&http.Cookie{ + Name: "cookie1", + }) + req.AddCookie(&http.Cookie{ + Name: "cookie2", + }) + req.AddCookie(&http.Cookie{ + Name: "cookie3", + }) + req.Header.Set(otherHeader, "test") + + cdt := clienttest.NewClientDecoratorTest(t, + clienttest.WithReqContext(req, &user.SignedInUser{}), + clienttest.WithMiddlewares(NewCookiesMiddleware([]string{"grafana_session"})), + ) + + pluginCtx := backend.PluginContext{ + AppInstanceSettings: &backend.AppInstanceSettings{}, + } + + t.Run("Should not forward cookies when calling QueryData", func(t *testing.T) { + pReq := &backend.QueryDataRequest{ + PluginContext: pluginCtx, + Headers: map[string]string{otherHeader: "test"}, + } + pReq.Headers[backend.CookiesHeaderName] = req.Header.Get(backend.CookiesHeaderName) + _, err = cdt.Decorator.QueryData(req.Context(), pReq) + require.NoError(t, err) + require.NotNil(t, cdt.QueryDataReq) + require.Len(t, cdt.QueryDataReq.Headers, 1) + require.Equal(t, "test", cdt.QueryDataReq.Headers[otherHeader]) + }) + + t.Run("Should not forward cookies when calling CallResource", func(t *testing.T) { + pReq := &backend.CallResourceRequest{ + PluginContext: pluginCtx, + Headers: map[string][]string{otherHeader: {"test"}}, + } + pReq.Headers[backend.CookiesHeaderName] = []string{req.Header.Get(backend.CookiesHeaderName)} + err = cdt.Decorator.CallResource(req.Context(), pReq, nopCallResourceSender) + require.NoError(t, err) + require.NotNil(t, cdt.CallResourceReq) + require.Len(t, cdt.CallResourceReq.Headers, 1) + require.Equal(t, "test", cdt.CallResourceReq.Headers[otherHeader][0]) + }) + + t.Run("Should not forward cookies when calling CheckHealth", func(t *testing.T) { + pReq := &backend.CheckHealthRequest{ + PluginContext: pluginCtx, + Headers: map[string]string{otherHeader: "test"}, + } + pReq.Headers[backend.CookiesHeaderName] = req.Header.Get(backend.CookiesHeaderName) + _, err = cdt.Decorator.CheckHealth(req.Context(), pReq) + require.NoError(t, err) + require.NotNil(t, cdt.CheckHealthReq) + require.Len(t, cdt.CheckHealthReq.Headers, 1) + require.Equal(t, "test", cdt.CheckHealthReq.Headers[otherHeader]) + }) + }) }