Auth: remove id token flag (#92209)

This commit is contained in:
Ryan McKinley 2024-08-21 16:30:17 +03:00 committed by GitHub
parent 302bfe3edf
commit 2e60f28044
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
16 changed files with 12 additions and 63 deletions

View File

@ -151,7 +151,6 @@ Experimental features might be changed or removed without prior notice.
| `wargamesTesting` | Placeholder feature flag for internal testing | | `wargamesTesting` | Placeholder feature flag for internal testing |
| `externalCorePlugins` | Allow core plugins to be loaded as external | | `externalCorePlugins` | Allow core plugins to be loaded as external |
| `pluginsAPIMetrics` | Sends metrics of public grafana packages usage by plugins | | `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 | | `enableNativeHTTPHistogram` | Enables native HTTP Histograms |
| `disableClassicHTTPHistogram` | Disables classic HTTP Histogram (use with enableNativeHTTPHistogram) | | `disableClassicHTTPHistogram` | Disables classic HTTP Histogram (use with enableNativeHTTPHistogram) |
| `kubernetesSnapshots` | Routes snapshot requests from /api to the /apis endpoint | | `kubernetesSnapshots` | Routes snapshot requests from /api to the /apis endpoint |

View File

@ -106,7 +106,6 @@ export interface FeatureToggles {
alertingInsights?: boolean; alertingInsights?: boolean;
externalCorePlugins?: boolean; externalCorePlugins?: boolean;
pluginsAPIMetrics?: boolean; pluginsAPIMetrics?: boolean;
idForwarding?: boolean;
externalServiceAccounts?: boolean; externalServiceAccounts?: boolean;
panelMonitoring?: boolean; panelMonitoring?: boolean;
enableNativeHTTPHistogram?: boolean; enableNativeHTTPHistogram?: boolean;

View File

@ -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 { func (proxy *DataSourceProxy) validateRequest() error {

View File

@ -185,10 +185,7 @@ func (proxy PluginProxy) director(req *http.Request) {
req.Header.Set("X-Grafana-Context", string(ctxJSON)) req.Header.Set("X-Grafana-Context", string(ctxJSON))
proxyutil.ApplyUserHeader(proxy.cfg.SendUserHeader, req, proxy.ctx.SignedInUser) proxyutil.ApplyUserHeader(proxy.cfg.SendUserHeader, req, proxy.ctx.SignedInUser)
proxyutil.ApplyForwardIDHeader(req, proxy.ctx.SignedInUser)
if proxy.features.IsEnabled(req.Context(), featuremgmt.FlagIdForwarding) {
proxyutil.ApplyForwardIDHeader(req, proxy.ctx.SignedInUser)
}
if err := addHeaders(&req.Header, proxy.matchedRoute, data); err != nil { if err := addHeaders(&req.Header, proxy.matchedRoute, data); err != nil {
proxy.ctx.JsonApiErr(500, "Failed to render plugin headers", err) proxy.ctx.JsonApiErr(500, "Failed to render plugin headers", err)

View File

@ -73,7 +73,6 @@ type Requester interface {
// HasUniqueId returns true if the entity has a unique id // HasUniqueId returns true if the entity has a unique id
HasUniqueId() bool HasUniqueId() bool
// GetIDToken returns a signed token representing the identity that can be forwarded to plugins and external services. // 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 GetIDToken() string
} }

View File

@ -18,7 +18,6 @@ import (
"github.com/grafana/grafana/pkg/services/apiserver/endpoints/request" "github.com/grafana/grafana/pkg/services/apiserver/endpoints/request"
"github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/auth"
"github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/authn"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/setting"
) )
@ -31,8 +30,9 @@ const (
var _ auth.IDService = (*Service)(nil) var _ auth.IDService = (*Service)(nil)
func ProvideService( func ProvideService(
cfg *setting.Cfg, signer auth.IDSigner, cache remotecache.CacheStorage, cfg *setting.Cfg, signer auth.IDSigner,
features featuremgmt.FeatureToggles, authnService authn.Service, cache remotecache.CacheStorage,
authnService authn.Service,
reg prometheus.Registerer, reg prometheus.Registerer,
) *Service { ) *Service {
s := &Service{ s := &Service{
@ -42,9 +42,7 @@ func ProvideService(
nsMapper: request.GetNamespaceMapper(cfg), nsMapper: request.GetNamespaceMapper(cfg),
} }
if features.IsEnabledGlobally(featuremgmt.FlagIdForwarding) { authnService.RegisterPostAuthHook(s.hook, 140)
authnService.RegisterPostAuthHook(s.hook, 140)
}
return s return s
} }

View File

@ -15,15 +15,12 @@ import (
"github.com/grafana/grafana/pkg/services/auth/idtest" "github.com/grafana/grafana/pkg/services/auth/idtest"
"github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/authn"
"github.com/grafana/grafana/pkg/services/authn/authntest" "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/services/login"
"github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/setting"
) )
func Test_ProvideService(t *testing.T) { func Test_ProvideService(t *testing.T) {
t.Run("should register post auth hook when feature flag is enabled", func(t *testing.T) { t.Run("should register post auth hook", func(t *testing.T) {
features := featuremgmt.WithFeatures(featuremgmt.FlagIdForwarding)
var hookRegistered bool var hookRegistered bool
authnService := &authntest.MockService{ authnService := &authntest.MockService{
RegisterPostAuthHookFunc: func(_ authn.PostAuthHookFn, _ uint) { 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) 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) { 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) { t.Run("should sign identity", func(t *testing.T) {
s := ProvideService( s := ProvideService(
setting.NewCfg(), signer, remotecache.NewFakeCacheStorage(), setting.NewCfg(), signer, remotecache.NewFakeCacheStorage(),
featuremgmt.WithFeatures(featuremgmt.FlagIdForwarding),
&authntest.FakeService{}, nil, &authntest.FakeService{}, nil,
) )
token, _, err := s.SignIdentity(context.Background(), &authn.Identity{ID: "1", Type: claims.TypeUser}) 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) { t.Run("should sign identity with authenticated by if user is externally authenticated", func(t *testing.T) {
s := ProvideService( s := ProvideService(
setting.NewCfg(), signer, remotecache.NewFakeCacheStorage(), setting.NewCfg(), signer, remotecache.NewFakeCacheStorage(),
featuremgmt.WithFeatures(featuremgmt.FlagIdForwarding),
&authntest.FakeService{}, nil, &authntest.FakeService{}, nil,
) )
token, _, err := s.SignIdentity(context.Background(), &authn.Identity{ 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) { t.Run("should sign identity with authenticated by if user is externally authenticated", func(t *testing.T) {
s := ProvideService( s := ProvideService(
setting.NewCfg(), signer, remotecache.NewFakeCacheStorage(), setting.NewCfg(), signer, remotecache.NewFakeCacheStorage(),
featuremgmt.WithFeatures(featuremgmt.FlagIdForwarding),
&authntest.FakeService{}, nil, &authntest.FakeService{}, nil,
) )
_, gotClaims, err := s.SignIdentity(context.Background(), &authn.Identity{ _, gotClaims, err := s.SignIdentity(context.Background(), &authn.Identity{

View File

@ -28,10 +28,6 @@ type LocalSigner struct {
} }
func (s *LocalSigner) SignIDToken(ctx context.Context, claims *auth.IDClaims) (string, error) { 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) signer, err := s.getSigner(ctx)
if err != nil { if err != nil {
return "", err return "", err

View File

@ -72,7 +72,6 @@ type Identity struct {
// Permissions is the list of permissions the entity has. // Permissions is the list of permissions the entity has.
Permissions map[int64]map[string][]string Permissions map[int64]map[string][]string
// IDToken is a signed token representing the identity that can be forwarded to plugins and external services. // 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 IDToken string
IDTokenClaims *authn.Claims[authn.IDTokenClaims] IDTokenClaims *authn.Claims[authn.IDTokenClaims]
} }

View File

@ -664,12 +664,6 @@ var (
Stage: FeatureStageExperimental, Stage: FeatureStageExperimental,
Owner: grafanaPluginsPlatformSquad, 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", Name: "externalServiceAccounts",
Description: "Automatic service account and token setup for plugins", Description: "Automatic service account and token setup for plugins",

View File

@ -87,7 +87,6 @@ wargamesTesting,experimental,@grafana/hosted-grafana-team,false,false,false
alertingInsights,GA,@grafana/alerting-squad,false,false,true alertingInsights,GA,@grafana/alerting-squad,false,false,true
externalCorePlugins,experimental,@grafana/plugins-platform-backend,false,false,false externalCorePlugins,experimental,@grafana/plugins-platform-backend,false,false,false
pluginsAPIMetrics,experimental,@grafana/plugins-platform-backend,false,false,true 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 externalServiceAccounts,preview,@grafana/identity-access-team,false,false,false
panelMonitoring,GA,@grafana/dataviz-squad,false,false,true panelMonitoring,GA,@grafana/dataviz-squad,false,false,true
enableNativeHTTPHistogram,experimental,@grafana/grafana-backend-services-squad,false,true,false enableNativeHTTPHistogram,experimental,@grafana/grafana-backend-services-squad,false,true,false

1 Name Stage Owner requiresDevMode RequiresRestart FrontendOnly
87 alertingInsights GA @grafana/alerting-squad false false true
88 externalCorePlugins experimental @grafana/plugins-platform-backend false false false
89 pluginsAPIMetrics experimental @grafana/plugins-platform-backend false false true
idForwarding experimental @grafana/identity-access-team false false false
90 externalServiceAccounts preview @grafana/identity-access-team false false false
91 panelMonitoring GA @grafana/dataviz-squad false false true
92 enableNativeHTTPHistogram experimental @grafana/grafana-backend-services-squad false true false

View File

@ -359,10 +359,6 @@ const (
// Sends metrics of public grafana packages usage by plugins // Sends metrics of public grafana packages usage by plugins
FlagPluginsAPIMetrics = "pluginsAPIMetrics" FlagPluginsAPIMetrics = "pluginsAPIMetrics"
// FlagIdForwarding
// Generate signed id token for identity that can be forwarded to plugins and external services
FlagIdForwarding = "idForwarding"
// FlagExternalServiceAccounts // FlagExternalServiceAccounts
// Automatic service account and token setup for plugins // Automatic service account and token setup for plugins
FlagExternalServiceAccounts = "externalServiceAccounts" FlagExternalServiceAccounts = "externalServiceAccounts"

View File

@ -1320,7 +1320,8 @@
"metadata": { "metadata": {
"name": "idForwarding", "name": "idForwarding",
"resourceVersion": "1718727528075", "resourceVersion": "1718727528075",
"creationTimestamp": "2023-09-25T15:21:28Z" "creationTimestamp": "2023-09-25T15:21:28Z",
"deletionTimestamp": "2024-08-21T11:35:56Z"
}, },
"spec": { "spec": {
"description": "Generate signed id token for identity that can be forwarded to plugins and external services", "description": "Generate signed id token for identity that can be forwarded to plugins and external services",

View File

@ -12,8 +12,7 @@ import (
const forwardIDHeaderName = "X-Grafana-Id" const forwardIDHeaderName = "X-Grafana-Id"
// NewForwardIDMiddleware creates a new plugins.ClientMiddleware that will // NewForwardIDMiddleware creates a new plugins.ClientMiddleware that will
// set grafana id header on outgoing plugins.Client requests if the // set grafana id header on outgoing plugins.Client requests
// feature toggle FlagIdForwarding is enabled
func NewForwardIDMiddleware() plugins.ClientMiddleware { func NewForwardIDMiddleware() plugins.ClientMiddleware {
return plugins.ClientMiddlewareFunc(func(next plugins.Client) plugins.Client { return plugins.ClientMiddlewareFunc(func(next plugins.Client) plugins.Client {
return &ForwardIDMiddleware{ return &ForwardIDMiddleware{
@ -35,7 +34,6 @@ func (m *ForwardIDMiddleware) applyToken(ctx context.Context, pCtx backend.Plugi
return nil return nil
} }
// token will only be present if faeturemgmt.FlagIdForwarding is enabled
if token := reqCtx.SignedInUser.GetIDToken(); token != "" { if token := reqCtx.SignedInUser.GetIDToken(); token != "" {
req.SetHTTPHeader(forwardIDHeaderName, token) req.SetHTTPHeader(forwardIDHeaderName, token)
} }

View File

@ -187,12 +187,9 @@ func CreateMiddlewares(cfg *setting.Cfg, oAuthTokenService oauthtoken.OAuthToken
clientmiddleware.NewCookiesMiddleware(skipCookiesNames), clientmiddleware.NewCookiesMiddleware(skipCookiesNames),
clientmiddleware.NewResourceResponseMiddleware(), clientmiddleware.NewResourceResponseMiddleware(),
clientmiddleware.NewCachingMiddlewareWithFeatureManager(cachingService, features), clientmiddleware.NewCachingMiddlewareWithFeatureManager(cachingService, features),
clientmiddleware.NewForwardIDMiddleware(),
) )
if features.IsEnabledGlobally(featuremgmt.FlagIdForwarding) {
middlewares = append(middlewares, clientmiddleware.NewForwardIDMiddleware())
}
if cfg.SendUserHeader { if cfg.SendUserHeader {
middlewares = append(middlewares, clientmiddleware.NewUserHeaderMiddleware()) middlewares = append(middlewares, clientmiddleware.NewUserHeaderMiddleware())
} }

View File

@ -45,7 +45,6 @@ type SignedInUser struct {
Permissions map[int64]map[string][]string `json:"-"` Permissions map[int64]map[string][]string `json:"-"`
// IDToken is a signed token representing the identity that can be forwarded to plugins and external services. // 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:"-"` IDToken string `json:"-" xorm:"-"`
IDTokenClaims *authnlib.Claims[authnlib.IDTokenClaims] `json:"-" xorm:"-"` IDTokenClaims *authnlib.Claims[authnlib.IDTokenClaims] `json:"-" xorm:"-"`