From 147bf017456cfaade09cac80b10a377eff18e435 Mon Sep 17 00:00:00 2001 From: Karl Persson Date: Tue, 23 Jan 2024 12:12:32 +0100 Subject: [PATCH] IDForwarding: Always forward id tokens to plugins (#81041) * Always forward id tokens to plugins --- pkg/api/pluginproxy/ds_proxy.go | 3 +- pkg/services/auth/id.go | 7 --- .../clientmiddleware/forward_id_middleware.go | 12 ----- .../forward_id_middleware_test.go | 44 +++---------------- 4 files changed, 6 insertions(+), 60 deletions(-) diff --git a/pkg/api/pluginproxy/ds_proxy.go b/pkg/api/pluginproxy/ds_proxy.go index f1b2e8e2469..f3bdf7d6467 100644 --- a/pkg/api/pluginproxy/ds_proxy.go +++ b/pkg/api/pluginproxy/ds_proxy.go @@ -19,7 +19,6 @@ 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" @@ -270,7 +269,7 @@ func (proxy *DataSourceProxy) director(req *http.Request) { } } - if proxy.features.IsEnabled(req.Context(), featuremgmt.FlagIdForwarding) && auth.IsIDForwardingEnabledForDataSource(proxy.ds) { + if proxy.features.IsEnabled(req.Context(), featuremgmt.FlagIdForwarding) { proxyutil.ApplyForwardIDHeader(req, proxy.ctx.SignedInUser) } } diff --git a/pkg/services/auth/id.go b/pkg/services/auth/id.go index 92a18a9b8d7..06d302d0f15 100644 --- a/pkg/services/auth/id.go +++ b/pkg/services/auth/id.go @@ -6,7 +6,6 @@ import ( "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 { @@ -22,9 +21,3 @@ type IDClaims struct { jwt.Claims AuthenticatedBy string `json:"authenticatedBy,omitempty"` } - -const settingsKey = "forwardGrafanaIdToken" - -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 ee7cf30738c..7c6ca3ee581 100644 --- a/pkg/services/pluginsintegration/clientmiddleware/forward_id_middleware.go +++ b/pkg/services/pluginsintegration/clientmiddleware/forward_id_middleware.go @@ -5,11 +5,8 @@ import ( "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" @@ -36,15 +33,6 @@ func (m *ForwardIDMiddleware) applyToken(ctx context.Context, pCtx backend.Plugi return nil } - jsonDataBytes, err := simplejson.NewJson(pCtx.DataSourceInstanceSettings.JSONData) - if err != nil { - return err - } - - if !auth.IsIDForwardingEnabledForDataSource(&datasources.DataSource{JsonData: jsonDataBytes}) { - return nil - } - // token will only be present if faeturemgmt.FlagIdForwarding is enabled if token := reqCtx.SignedInUser.GetIDToken(); token != "" { req.SetHTTPHeader(forwardIDHeaderName, token) diff --git a/pkg/services/pluginsintegration/clientmiddleware/forward_id_middleware_test.go b/pkg/services/pluginsintegration/clientmiddleware/forward_id_middleware_test.go index 42c0953cad4..a7397c300aa 100644 --- a/pkg/services/pluginsintegration/clientmiddleware/forward_id_middleware_test.go +++ b/pkg/services/pluginsintegration/clientmiddleware/forward_id_middleware_test.go @@ -2,7 +2,6 @@ package clientmiddleware import ( "context" - "encoding/json" "net/http" "testing" @@ -17,15 +16,9 @@ import ( ) func TestForwardIDMiddleware(t *testing.T) { - settingWithEnabled, err := json.Marshal(map[string]any{ - "forwardGrafanaIdToken": true, - }) - require.NoError(t, err) - - settingWithDisabled, err := json.Marshal(map[string]any{ - "forwardGrafanaIdToken": false, - }) - require.NoError(t, err) + pluginContext := backend.PluginContext{ + DataSourceInstanceSettings: &backend.DataSourceInstanceSettings{}, + } t.Run("Should set forwarded id header if present", func(t *testing.T) { cdt := clienttest.NewClientDecoratorTest(t, clienttest.WithMiddlewares(NewForwardIDMiddleware())) @@ -36,36 +29,13 @@ func TestForwardIDMiddleware(t *testing.T) { }) err := cdt.Decorator.CallResource(ctx, &backend.CallResourceRequest{ - PluginContext: backend.PluginContext{ - DataSourceInstanceSettings: &backend.DataSourceInstanceSettings{ - JSONData: settingWithEnabled, - }, - }, + PluginContext: pluginContext, }, 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())) @@ -75,11 +45,7 @@ func TestForwardIDMiddleware(t *testing.T) { }) err := cdt.Decorator.CallResource(ctx, &backend.CallResourceRequest{ - PluginContext: backend.PluginContext{ - DataSourceInstanceSettings: &backend.DataSourceInstanceSettings{ - JSONData: settingWithEnabled, - }, - }, + PluginContext: pluginContext, }, nopCallResourceSender) require.NoError(t, err)