From 2e60f28044f39c6f54353063e0650a075058592d Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Wed, 21 Aug 2024 16:30:17 +0300 Subject: [PATCH] Auth: remove id token flag (#92209) --- .../feature-toggles/index.md | 1 - .../src/types/featureToggles.gen.ts | 1 - pkg/api/pluginproxy/ds_proxy.go | 4 +--- pkg/api/pluginproxy/pluginproxy.go | 5 +--- pkg/apimachinery/identity/requester.go | 1 - pkg/services/auth/idimpl/service.go | 10 ++++---- pkg/services/auth/idimpl/service_test.go | 24 ++----------------- pkg/services/auth/idimpl/signer.go | 4 ---- pkg/services/authn/identity.go | 1 - pkg/services/featuremgmt/registry.go | 6 ----- pkg/services/featuremgmt/toggles_gen.csv | 1 - pkg/services/featuremgmt/toggles_gen.go | 4 ---- pkg/services/featuremgmt/toggles_gen.json | 3 ++- .../clientmiddleware/forward_id_middleware.go | 4 +--- .../pluginsintegration/pluginsintegration.go | 5 +--- pkg/services/user/identity.go | 1 - 16 files changed, 12 insertions(+), 63 deletions(-) diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index 3dcb474dd6c..ce404a688f2 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -151,7 +151,6 @@ Experimental features might be changed or removed without prior notice. | `wargamesTesting` | Placeholder feature flag for internal testing | | `externalCorePlugins` | Allow core plugins to be loaded as external | | `pluginsAPIMetrics` | Sends metrics of public grafana packages usage by plugins | -| `idForwarding` | Generate signed id token for identity that can be forwarded to plugins and external services | | `enableNativeHTTPHistogram` | Enables native HTTP Histograms | | `disableClassicHTTPHistogram` | Disables classic HTTP Histogram (use with enableNativeHTTPHistogram) | | `kubernetesSnapshots` | Routes snapshot requests from /api to the /apis endpoint | diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 1c577afa5f0..0d26cd877e6 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -106,7 +106,6 @@ export interface FeatureToggles { alertingInsights?: boolean; externalCorePlugins?: boolean; pluginsAPIMetrics?: boolean; - idForwarding?: boolean; externalServiceAccounts?: boolean; panelMonitoring?: boolean; enableNativeHTTPHistogram?: boolean; diff --git a/pkg/api/pluginproxy/ds_proxy.go b/pkg/api/pluginproxy/ds_proxy.go index 70d77d68bb0..746b3c9a3f4 100644 --- a/pkg/api/pluginproxy/ds_proxy.go +++ b/pkg/api/pluginproxy/ds_proxy.go @@ -271,9 +271,7 @@ func (proxy *DataSourceProxy) director(req *http.Request) { } } - if proxy.features.IsEnabled(req.Context(), featuremgmt.FlagIdForwarding) { - proxyutil.ApplyForwardIDHeader(req, proxy.ctx.SignedInUser) - } + proxyutil.ApplyForwardIDHeader(req, proxy.ctx.SignedInUser) } func (proxy *DataSourceProxy) validateRequest() error { diff --git a/pkg/api/pluginproxy/pluginproxy.go b/pkg/api/pluginproxy/pluginproxy.go index dd5f36a3cb9..8288c15276f 100644 --- a/pkg/api/pluginproxy/pluginproxy.go +++ b/pkg/api/pluginproxy/pluginproxy.go @@ -185,10 +185,7 @@ func (proxy PluginProxy) director(req *http.Request) { req.Header.Set("X-Grafana-Context", string(ctxJSON)) proxyutil.ApplyUserHeader(proxy.cfg.SendUserHeader, req, proxy.ctx.SignedInUser) - - if proxy.features.IsEnabled(req.Context(), featuremgmt.FlagIdForwarding) { - proxyutil.ApplyForwardIDHeader(req, proxy.ctx.SignedInUser) - } + proxyutil.ApplyForwardIDHeader(req, proxy.ctx.SignedInUser) if err := addHeaders(&req.Header, proxy.matchedRoute, data); err != nil { proxy.ctx.JsonApiErr(500, "Failed to render plugin headers", err) diff --git a/pkg/apimachinery/identity/requester.go b/pkg/apimachinery/identity/requester.go index ac44d236b29..5105a9dfb5f 100644 --- a/pkg/apimachinery/identity/requester.go +++ b/pkg/apimachinery/identity/requester.go @@ -73,7 +73,6 @@ type Requester interface { // HasUniqueId returns true if the entity has a unique id HasUniqueId() bool // GetIDToken returns a signed token representing the identity that can be forwarded to plugins and external services. - // Will only be set when featuremgmt.FlagIdForwarding is enabled. GetIDToken() string } diff --git a/pkg/services/auth/idimpl/service.go b/pkg/services/auth/idimpl/service.go index 7c75c86dc7f..3a9baa8b22d 100644 --- a/pkg/services/auth/idimpl/service.go +++ b/pkg/services/auth/idimpl/service.go @@ -18,7 +18,6 @@ import ( "github.com/grafana/grafana/pkg/services/apiserver/endpoints/request" "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/authn" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/setting" ) @@ -31,8 +30,9 @@ const ( var _ auth.IDService = (*Service)(nil) func ProvideService( - cfg *setting.Cfg, signer auth.IDSigner, cache remotecache.CacheStorage, - features featuremgmt.FeatureToggles, authnService authn.Service, + cfg *setting.Cfg, signer auth.IDSigner, + cache remotecache.CacheStorage, + authnService authn.Service, reg prometheus.Registerer, ) *Service { s := &Service{ @@ -42,9 +42,7 @@ func ProvideService( nsMapper: request.GetNamespaceMapper(cfg), } - if features.IsEnabledGlobally(featuremgmt.FlagIdForwarding) { - authnService.RegisterPostAuthHook(s.hook, 140) - } + authnService.RegisterPostAuthHook(s.hook, 140) return s } diff --git a/pkg/services/auth/idimpl/service_test.go b/pkg/services/auth/idimpl/service_test.go index 61f5dbf6d66..20b1e1c0832 100644 --- a/pkg/services/auth/idimpl/service_test.go +++ b/pkg/services/auth/idimpl/service_test.go @@ -15,15 +15,12 @@ import ( "github.com/grafana/grafana/pkg/services/auth/idtest" "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/authn/authntest" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/setting" ) func Test_ProvideService(t *testing.T) { - t.Run("should register post auth hook when feature flag is enabled", func(t *testing.T) { - features := featuremgmt.WithFeatures(featuremgmt.FlagIdForwarding) - + t.Run("should register post auth hook", func(t *testing.T) { var hookRegistered bool authnService := &authntest.MockService{ RegisterPostAuthHookFunc: func(_ authn.PostAuthHookFn, _ uint) { @@ -31,23 +28,9 @@ func Test_ProvideService(t *testing.T) { }, } - _ = ProvideService(setting.NewCfg(), nil, nil, features, authnService, nil) + _ = ProvideService(setting.NewCfg(), nil, nil, authnService, nil) assert.True(t, hookRegistered) }) - - t.Run("should not register post auth hook when feature flag is disabled", func(t *testing.T) { - features := featuremgmt.WithFeatures() - - var hookRegistered bool - authnService := &authntest.MockService{ - RegisterPostAuthHookFunc: func(_ authn.PostAuthHookFn, _ uint) { - hookRegistered = true - }, - } - - _ = ProvideService(setting.NewCfg(), nil, nil, features, authnService, nil) - assert.False(t, hookRegistered) - }) } func TestService_SignIdentity(t *testing.T) { @@ -67,7 +50,6 @@ func TestService_SignIdentity(t *testing.T) { t.Run("should sign identity", func(t *testing.T) { s := ProvideService( setting.NewCfg(), signer, remotecache.NewFakeCacheStorage(), - featuremgmt.WithFeatures(featuremgmt.FlagIdForwarding), &authntest.FakeService{}, nil, ) token, _, err := s.SignIdentity(context.Background(), &authn.Identity{ID: "1", Type: claims.TypeUser}) @@ -78,7 +60,6 @@ func TestService_SignIdentity(t *testing.T) { t.Run("should sign identity with authenticated by if user is externally authenticated", func(t *testing.T) { s := ProvideService( setting.NewCfg(), signer, remotecache.NewFakeCacheStorage(), - featuremgmt.WithFeatures(featuremgmt.FlagIdForwarding), &authntest.FakeService{}, nil, ) token, _, err := s.SignIdentity(context.Background(), &authn.Identity{ @@ -104,7 +85,6 @@ func TestService_SignIdentity(t *testing.T) { t.Run("should sign identity with authenticated by if user is externally authenticated", func(t *testing.T) { s := ProvideService( setting.NewCfg(), signer, remotecache.NewFakeCacheStorage(), - featuremgmt.WithFeatures(featuremgmt.FlagIdForwarding), &authntest.FakeService{}, nil, ) _, gotClaims, err := s.SignIdentity(context.Background(), &authn.Identity{ diff --git a/pkg/services/auth/idimpl/signer.go b/pkg/services/auth/idimpl/signer.go index 29d664e99b2..0a37b263f45 100644 --- a/pkg/services/auth/idimpl/signer.go +++ b/pkg/services/auth/idimpl/signer.go @@ -28,10 +28,6 @@ type LocalSigner struct { } func (s *LocalSigner) SignIDToken(ctx context.Context, claims *auth.IDClaims) (string, error) { - if !s.features.IsEnabled(ctx, featuremgmt.FlagIdForwarding) { - return "", nil - } - signer, err := s.getSigner(ctx) if err != nil { return "", err diff --git a/pkg/services/authn/identity.go b/pkg/services/authn/identity.go index e2afc31647a..4a0da892d8d 100644 --- a/pkg/services/authn/identity.go +++ b/pkg/services/authn/identity.go @@ -72,7 +72,6 @@ type Identity struct { // Permissions is the list of permissions the entity has. Permissions map[int64]map[string][]string // IDToken is a signed token representing the identity that can be forwarded to plugins and external services. - // Will only be set when featuremgmt.FlagIdForwarding is enabled. IDToken string IDTokenClaims *authn.Claims[authn.IDTokenClaims] } diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index db598c1d193..ca00201458c 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -664,12 +664,6 @@ var ( Stage: FeatureStageExperimental, Owner: grafanaPluginsPlatformSquad, }, - { - Name: "idForwarding", - Description: "Generate signed id token for identity that can be forwarded to plugins and external services", - Stage: FeatureStageExperimental, - Owner: identityAccessTeam, - }, { Name: "externalServiceAccounts", Description: "Automatic service account and token setup for plugins", diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index c9315a75b12..355fa7d25da 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -87,7 +87,6 @@ wargamesTesting,experimental,@grafana/hosted-grafana-team,false,false,false alertingInsights,GA,@grafana/alerting-squad,false,false,true externalCorePlugins,experimental,@grafana/plugins-platform-backend,false,false,false pluginsAPIMetrics,experimental,@grafana/plugins-platform-backend,false,false,true -idForwarding,experimental,@grafana/identity-access-team,false,false,false externalServiceAccounts,preview,@grafana/identity-access-team,false,false,false panelMonitoring,GA,@grafana/dataviz-squad,false,false,true enableNativeHTTPHistogram,experimental,@grafana/grafana-backend-services-squad,false,true,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index bd81612d795..89c2c2c1909 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -359,10 +359,6 @@ const ( // Sends metrics of public grafana packages usage by plugins FlagPluginsAPIMetrics = "pluginsAPIMetrics" - // FlagIdForwarding - // Generate signed id token for identity that can be forwarded to plugins and external services - FlagIdForwarding = "idForwarding" - // FlagExternalServiceAccounts // Automatic service account and token setup for plugins FlagExternalServiceAccounts = "externalServiceAccounts" diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index 40b7aad2501..a8a0ac0e195 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -1320,7 +1320,8 @@ "metadata": { "name": "idForwarding", "resourceVersion": "1718727528075", - "creationTimestamp": "2023-09-25T15:21:28Z" + "creationTimestamp": "2023-09-25T15:21:28Z", + "deletionTimestamp": "2024-08-21T11:35:56Z" }, "spec": { "description": "Generate signed id token for identity that can be forwarded to plugins and external services", diff --git a/pkg/services/pluginsintegration/clientmiddleware/forward_id_middleware.go b/pkg/services/pluginsintegration/clientmiddleware/forward_id_middleware.go index c43d6e5e62a..9e799759157 100644 --- a/pkg/services/pluginsintegration/clientmiddleware/forward_id_middleware.go +++ b/pkg/services/pluginsintegration/clientmiddleware/forward_id_middleware.go @@ -12,8 +12,7 @@ import ( const forwardIDHeaderName = "X-Grafana-Id" // NewForwardIDMiddleware creates a new plugins.ClientMiddleware that will -// set grafana id header on outgoing plugins.Client requests if the -// feature toggle FlagIdForwarding is enabled +// set grafana id header on outgoing plugins.Client requests func NewForwardIDMiddleware() plugins.ClientMiddleware { return plugins.ClientMiddlewareFunc(func(next plugins.Client) plugins.Client { return &ForwardIDMiddleware{ @@ -35,7 +34,6 @@ func (m *ForwardIDMiddleware) applyToken(ctx context.Context, pCtx backend.Plugi 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/pluginsintegration.go b/pkg/services/pluginsintegration/pluginsintegration.go index bf0e7705fc0..e411e6c8552 100644 --- a/pkg/services/pluginsintegration/pluginsintegration.go +++ b/pkg/services/pluginsintegration/pluginsintegration.go @@ -187,12 +187,9 @@ func CreateMiddlewares(cfg *setting.Cfg, oAuthTokenService oauthtoken.OAuthToken clientmiddleware.NewCookiesMiddleware(skipCookiesNames), clientmiddleware.NewResourceResponseMiddleware(), clientmiddleware.NewCachingMiddlewareWithFeatureManager(cachingService, features), + clientmiddleware.NewForwardIDMiddleware(), ) - if features.IsEnabledGlobally(featuremgmt.FlagIdForwarding) { - middlewares = append(middlewares, clientmiddleware.NewForwardIDMiddleware()) - } - if cfg.SendUserHeader { middlewares = append(middlewares, clientmiddleware.NewUserHeaderMiddleware()) } diff --git a/pkg/services/user/identity.go b/pkg/services/user/identity.go index 83a303b28c8..c6939b47b0d 100644 --- a/pkg/services/user/identity.go +++ b/pkg/services/user/identity.go @@ -45,7 +45,6 @@ type SignedInUser struct { Permissions map[int64]map[string][]string `json:"-"` // IDToken is a signed token representing the identity that can be forwarded to plugins and external services. - // Will only be set when featuremgmt.FlagIdForwarding is enabled. IDToken string `json:"-" xorm:"-"` IDTokenClaims *authnlib.Claims[authnlib.IDTokenClaims] `json:"-" xorm:"-"`