From ac55fad1baa98e939a10fd731f0434ea8a1e99e3 Mon Sep 17 00:00:00 2001 From: Todd Treece <360020+toddtreece@users.noreply.github.com> Date: Wed, 10 Dec 2025 09:12:26 -0500 Subject: [PATCH] Plugins App: Switch to resource authorizer (#115019) --- apps/plugins/go.mod | 4 +- apps/plugins/pkg/app/app.go | 40 ++----------------- apps/plugins/pkg/app/authorizer.go | 32 --------------- pkg/registry/apps/plugins/register.go | 11 ++--- .../backgroundsvcs/adapter/service_test.go | 15 +------ .../apiserver/auth/authorizer/resource.go | 4 +- 6 files changed, 13 insertions(+), 93 deletions(-) delete mode 100644 apps/plugins/pkg/app/authorizer.go diff --git a/apps/plugins/go.mod b/apps/plugins/go.mod index 669c7c46844..287c2ff0bbe 100644 --- a/apps/plugins/go.mod +++ b/apps/plugins/go.mod @@ -10,11 +10,9 @@ replace github.com/grafana/grafana/pkg/apiserver => ../../pkg/apiserver require ( github.com/emicklei/go-restful/v3 v3.13.0 - github.com/grafana/authlib/types v0.0.0-20251119142549-be091cf2f4d4 github.com/grafana/grafana v0.0.0-00010101000000-000000000000 github.com/grafana/grafana-app-sdk v0.48.5 github.com/grafana/grafana-app-sdk/logging v0.48.3 - github.com/grafana/grafana/pkg/apimachinery v0.0.0 github.com/stretchr/testify v1.11.1 k8s.io/apimachinery v0.34.2 k8s.io/apiserver v0.34.2 @@ -78,11 +76,13 @@ require ( github.com/google/uuid v1.6.0 // indirect github.com/grafana/alerting v0.0.0-20251204145817-de8c2bbf9eba // indirect github.com/grafana/authlib v0.0.0-20250930082137-a40e2c2b094f // indirect + github.com/grafana/authlib/types v0.0.0-20251119142549-be091cf2f4d4 // indirect github.com/grafana/dataplane/sdata v0.0.9 // indirect github.com/grafana/dskit v0.0.0-20250908063411-6b6da59b5cc4 // indirect github.com/grafana/grafana-aws-sdk v1.3.0 // indirect github.com/grafana/grafana-azure-sdk-go/v2 v2.3.1 // indirect github.com/grafana/grafana-plugin-sdk-go v0.284.0 // indirect + github.com/grafana/grafana/pkg/apimachinery v0.0.0 // indirect github.com/grafana/grafana/pkg/apiserver v0.0.0 // indirect github.com/grafana/otel-profiling-go v0.5.1 // indirect github.com/grafana/pyroscope-go/godeltaprof v0.1.9 // indirect diff --git a/apps/plugins/pkg/app/app.go b/apps/plugins/pkg/app/app.go index 4c1ce4cfe0f..7c0187dd59d 100644 --- a/apps/plugins/pkg/app/app.go +++ b/apps/plugins/pkg/app/app.go @@ -70,6 +70,7 @@ type PluginAppConfig struct { } func ProvideAppInstaller( + authorizer authorizer.Authorizer, metaProviderManager *meta.ProviderManager, ) (*PluginAppInstaller, error) { specificConfig := &PluginAppConfig{ @@ -88,21 +89,17 @@ func ProvideAppInstaller( appInstaller := &PluginAppInstaller{ AppInstaller: defaultInstaller, + authorizer: authorizer, metaManager: metaProviderManager, ready: make(chan struct{}), } return appInstaller, nil } -func (p *PluginAppInstaller) WithAccessChecker(access authlib.AccessChecker) *PluginAppInstaller { - p.access = access - return p -} - type PluginAppInstaller struct { appsdkapiserver.AppInstaller metaManager *meta.ProviderManager - access authlib.AccessChecker + authorizer authorizer.Authorizer // restConfig is set during InitializeApp and used by the client factory restConfig *restclient.Config @@ -153,34 +150,5 @@ func (p *PluginAppInstaller) InstallAPIs( } func (p *PluginAppInstaller) GetAuthorizer() authorizer.Authorizer { - if p.access == nil { - return nil - } - - return authorizer.AuthorizerFunc( - func(ctx context.Context, a authorizer.Attributes) (decision authorizer.Decision, reason string, err error) { - info, ok := authlib.AuthInfoFrom(ctx) - if !ok { - return authorizer.DecisionDeny, "failed to get auth info", nil - } - - res, err := p.access.Check(ctx, info, authlib.CheckRequest{ - Verb: a.GetVerb(), - Group: a.GetAPIGroup(), - Resource: a.GetResource(), - Name: a.GetName(), - Namespace: a.GetNamespace(), - Subresource: a.GetSubresource(), - Path: a.GetPath(), - }, "") - if err != nil { - return authorizer.DecisionDeny, "failed to perform authorization", err - } - - if !res.Allowed { - return authorizer.DecisionDeny, "permission denied", nil - } - - return authorizer.DecisionAllow, "", nil - }) + return p.authorizer } diff --git a/apps/plugins/pkg/app/authorizer.go b/apps/plugins/pkg/app/authorizer.go deleted file mode 100644 index b6a800694f7..00000000000 --- a/apps/plugins/pkg/app/authorizer.go +++ /dev/null @@ -1,32 +0,0 @@ -package app - -import ( - "context" - - "k8s.io/apiserver/pkg/authorization/authorizer" - - "github.com/grafana/grafana/pkg/apimachinery/identity" -) - -func GetAuthorizer() authorizer.Authorizer { - return authorizer.AuthorizerFunc(func( - ctx context.Context, attr authorizer.Attributes, - ) (authorized authorizer.Decision, reason string, err error) { - if !attr.IsResourceRequest() { - return authorizer.DecisionNoOpinion, "", nil - } - - // require a user - u, err := identity.GetRequester(ctx) - if err != nil { - return authorizer.DecisionDeny, "valid user is required", err - } - - // check if is admin - if u.HasRole(identity.RoleAdmin) { - return authorizer.DecisionAllow, "", nil - } - - return authorizer.DecisionDeny, "forbidden", nil - }) -} diff --git a/pkg/registry/apps/plugins/register.go b/pkg/registry/apps/plugins/register.go index aebb8e98178..9113d927a29 100644 --- a/pkg/registry/apps/plugins/register.go +++ b/pkg/registry/apps/plugins/register.go @@ -6,12 +6,12 @@ import ( authlib "github.com/grafana/authlib/types" appsdkapiserver "github.com/grafana/grafana-app-sdk/k8s/apiserver" - "k8s.io/apiserver/pkg/authorization/authorizer" pluginsapp "github.com/grafana/grafana/apps/plugins/pkg/app" "github.com/grafana/grafana/apps/plugins/pkg/app/meta" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/apiserver/appinstaller" + grafanaauthorizer "github.com/grafana/grafana/pkg/services/apiserver/auth/authorizer" ) var ( @@ -37,18 +37,13 @@ func ProvideAppInstaller(accessControlService accesscontrol.Service, accessClien cloudProvider := meta.NewCatalogProvider(grafanaComAPIURL) metaProviderManager := meta.NewProviderManager(coreProvider, cloudProvider) - i, err := pluginsapp.ProvideAppInstaller(metaProviderManager) + authorizer := grafanaauthorizer.NewResourceAuthorizer(accessClient) + i, err := pluginsapp.ProvideAppInstaller(authorizer, metaProviderManager) if err != nil { return nil, err } - i.WithAccessChecker(accessClient) - return &AppInstaller{ PluginAppInstaller: i, }, nil } - -func (a *AppInstaller) GetAuthorizer() authorizer.Authorizer { - return pluginsapp.GetAuthorizer() -} diff --git a/pkg/registry/backgroundsvcs/adapter/service_test.go b/pkg/registry/backgroundsvcs/adapter/service_test.go index ed357d66443..275068967c0 100644 --- a/pkg/registry/backgroundsvcs/adapter/service_test.go +++ b/pkg/registry/backgroundsvcs/adapter/service_test.go @@ -55,15 +55,9 @@ func TestServiceAdapter_ErrorHandling(t *testing.T) { adapter := asNamedService(mockSvc) - t.Cleanup(func() { - adapter.StopAsync() - err := adapter.AwaitTerminated(context.Background()) - require.ErrorIs(t, err, expectedErr) - }) - err := adapter.StartAsync(context.Background()) require.NoError(t, err) - err = adapter.AwaitRunning(context.Background()) + err = adapter.AwaitTerminated(context.Background()) require.ErrorIs(t, err, expectedErr) require.True(t, mockSvc.runCalled) }) @@ -95,14 +89,9 @@ func TestServiceAdapter_ErrorHandling(t *testing.T) { adapter := asNamedService(mockSvc) - t.Cleanup(func() { - adapter.StopAsync() - err := adapter.AwaitTerminated(context.Background()) - require.ErrorIs(t, err, expectedErr) - }) err := adapter.StartAsync(context.Background()) require.NoError(t, err) - err = adapter.AwaitRunning(context.Background()) + err = adapter.AwaitTerminated(context.Background()) require.ErrorIs(t, err, expectedErr) require.True(t, mockSvc.runCalled) }) diff --git a/pkg/services/apiserver/auth/authorizer/resource.go b/pkg/services/apiserver/auth/authorizer/resource.go index b86f6b40f09..f2f959f9381 100644 --- a/pkg/services/apiserver/auth/authorizer/resource.go +++ b/pkg/services/apiserver/auth/authorizer/resource.go @@ -9,13 +9,13 @@ import ( claims "github.com/grafana/authlib/types" ) -func NewResourceAuthorizer(c claims.AccessClient) authorizer.Authorizer { +func NewResourceAuthorizer(c claims.AccessChecker) authorizer.Authorizer { return ResourceAuthorizer{c} } // ResourceAuthorizer is used to translate authorizer.Authorizer calls to claims.AccessClient calls type ResourceAuthorizer struct { - c claims.AccessClient + c claims.AccessChecker } func (r ResourceAuthorizer) Authorize(ctx context.Context, attr authorizer.Attributes) (authorizer.Decision, string, error) {