From 1b6d39f8239551afef24d3220792299061e334e2 Mon Sep 17 00:00:00 2001 From: Karl Persson Date: Fri, 27 Oct 2023 08:30:33 +0200 Subject: [PATCH] IDForwarding: Require that id forwarding is enabled for data source (#77131) * Require that id forwarding is enabled for data source * Address feedback --- pkg/api/pluginproxy/ds_proxy.go | 3 +- pkg/services/auth/id.go | 8 ++++ .../clientmiddleware/forward_id_middleware.go | 15 ++++++- .../forward_id_middleware_test.go | 44 +++++++++++++++++-- 4 files changed, 65 insertions(+), 5 deletions(-) diff --git a/pkg/api/pluginproxy/ds_proxy.go b/pkg/api/pluginproxy/ds_proxy.go index 35c1ca2efc5..87c449d13c1 100644 --- a/pkg/api/pluginproxy/ds_proxy.go +++ b/pkg/api/pluginproxy/ds_proxy.go @@ -19,6 +19,7 @@ import ( glog "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/plugins" + "github.com/grafana/grafana/pkg/services/auth" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/featuremgmt" @@ -269,7 +270,7 @@ func (proxy *DataSourceProxy) director(req *http.Request) { } } - if proxy.features.IsEnabled(featuremgmt.FlagIdForwarding) { + if proxy.features.IsEnabled(featuremgmt.FlagIdForwarding) && auth.IsIDForwardingEnabledForDataSource(proxy.ds) { proxyutil.ApplyForwardIDHeader(req, proxy.ctx.SignedInUser) } } diff --git a/pkg/services/auth/id.go b/pkg/services/auth/id.go index 33d505b23f7..4a0364db707 100644 --- a/pkg/services/auth/id.go +++ b/pkg/services/auth/id.go @@ -4,7 +4,9 @@ import ( "context" "github.com/go-jose/go-jose/v3/jwt" + "github.com/grafana/grafana/pkg/services/auth/identity" + "github.com/grafana/grafana/pkg/services/datasources" ) type IDService interface { @@ -19,3 +21,9 @@ type IDSigner interface { type IDClaims struct { jwt.Claims } + +const settingsKey = "forwardIdToken" + +func IsIDForwardingEnabledForDataSource(ds *datasources.DataSource) bool { + return ds.JsonData != nil && ds.JsonData.Get(settingsKey).MustBool() +} diff --git a/pkg/services/pluginsintegration/clientmiddleware/forward_id_middleware.go b/pkg/services/pluginsintegration/clientmiddleware/forward_id_middleware.go index 9e7f9269b3f..ee7cf30738c 100644 --- a/pkg/services/pluginsintegration/clientmiddleware/forward_id_middleware.go +++ b/pkg/services/pluginsintegration/clientmiddleware/forward_id_middleware.go @@ -4,8 +4,12 @@ import ( "context" "github.com/grafana/grafana-plugin-sdk-go/backend" + + "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/plugins" + "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/contexthandler" + "github.com/grafana/grafana/pkg/services/datasources" ) const forwardIDHeaderName = "X-Grafana-Id" @@ -28,7 +32,16 @@ type ForwardIDMiddleware struct { func (m *ForwardIDMiddleware) applyToken(ctx context.Context, pCtx backend.PluginContext, req backend.ForwardHTTPHeaders) error { reqCtx := contexthandler.FromContext(ctx) // if request not for a datasource or no HTTP request context skip middleware - if req == nil || reqCtx == nil || reqCtx.SignedInUser == nil { + if req == nil || reqCtx == nil || reqCtx.SignedInUser == nil || pCtx.DataSourceInstanceSettings == nil { + return nil + } + + jsonDataBytes, err := simplejson.NewJson(pCtx.DataSourceInstanceSettings.JSONData) + if err != nil { + return err + } + + if !auth.IsIDForwardingEnabledForDataSource(&datasources.DataSource{JsonData: jsonDataBytes}) { return nil } diff --git a/pkg/services/pluginsintegration/clientmiddleware/forward_id_middleware_test.go b/pkg/services/pluginsintegration/clientmiddleware/forward_id_middleware_test.go index 6ca5773256e..087bf29729e 100644 --- a/pkg/services/pluginsintegration/clientmiddleware/forward_id_middleware_test.go +++ b/pkg/services/pluginsintegration/clientmiddleware/forward_id_middleware_test.go @@ -2,20 +2,31 @@ package clientmiddleware import ( "context" + "encoding/json" "net/http" "testing" "github.com/grafana/grafana-plugin-sdk-go/backend" - "github.com/grafana/grafana/pkg/web" "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/plugins/manager/client/clienttest" "github.com/grafana/grafana/pkg/services/contexthandler/ctxkey" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/web" ) func TestForwardIDMiddleware(t *testing.T) { + settingWithEnabled, err := json.Marshal(map[string]any{ + "forwardIdToken": true, + }) + require.NoError(t, err) + + settingWithDisabled, err := json.Marshal(map[string]any{ + "forwardIdToken": false, + }) + require.NoError(t, err) + t.Run("Should set forwarded id header if present", func(t *testing.T) { cdt := clienttest.NewClientDecoratorTest(t, clienttest.WithMiddlewares(NewForwardIDMiddleware())) @@ -25,13 +36,36 @@ func TestForwardIDMiddleware(t *testing.T) { }) err := cdt.Decorator.CallResource(ctx, &backend.CallResourceRequest{ - PluginContext: backend.PluginContext{}, + PluginContext: backend.PluginContext{ + DataSourceInstanceSettings: &backend.DataSourceInstanceSettings{ + JSONData: settingWithEnabled, + }, + }, }, nopCallResourceSender) require.NoError(t, err) require.Equal(t, "some-token", cdt.CallResourceReq.Headers[forwardIDHeaderName][0]) }) + t.Run("Should not set forwarded id header if setting is disabled", func(t *testing.T) { + cdt := clienttest.NewClientDecoratorTest(t, clienttest.WithMiddlewares(NewForwardIDMiddleware())) + + ctx := context.WithValue(context.Background(), ctxkey.Key{}, &contextmodel.ReqContext{ + Context: &web.Context{Req: &http.Request{}}, + SignedInUser: &user.SignedInUser{IDToken: "some-token"}, + }) + + err := cdt.Decorator.CallResource(ctx, &backend.CallResourceRequest{ + PluginContext: backend.PluginContext{ + DataSourceInstanceSettings: &backend.DataSourceInstanceSettings{ + JSONData: settingWithDisabled, + }, + }, + }, nopCallResourceSender) + require.NoError(t, err) + require.Len(t, cdt.CallResourceReq.Headers[forwardIDHeaderName], 0) + }) + t.Run("Should not set forwarded id header if not present", func(t *testing.T) { cdt := clienttest.NewClientDecoratorTest(t, clienttest.WithMiddlewares(NewForwardIDMiddleware())) @@ -41,7 +75,11 @@ func TestForwardIDMiddleware(t *testing.T) { }) err := cdt.Decorator.CallResource(ctx, &backend.CallResourceRequest{ - PluginContext: backend.PluginContext{}, + PluginContext: backend.PluginContext{ + DataSourceInstanceSettings: &backend.DataSourceInstanceSettings{ + JSONData: settingWithEnabled, + }, + }, }, nopCallResourceSender) require.NoError(t, err)