Authz: Remove "wrapper" interface and only check feature toggle for grpc mode (#98933)

* Remove "wrapper" interface and only check feature toggle for grpc and cloud mode

* Only set name for update checks

* Set dashboard permissions for admin user
This commit is contained in:
Karl Persson
2025-01-15 09:23:56 +01:00
committed by GitHub
parent 0d302a161a
commit 3f71a72c1a
6 changed files with 32 additions and 37 deletions
@@ -15,7 +15,6 @@ import (
grafanaapiserver "github.com/grafana/grafana/pkg/services/apiserver" grafanaapiserver "github.com/grafana/grafana/pkg/services/apiserver"
"github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/auth"
"github.com/grafana/grafana/pkg/services/authn/authnimpl" "github.com/grafana/grafana/pkg/services/authn/authnimpl"
"github.com/grafana/grafana/pkg/services/authz"
"github.com/grafana/grafana/pkg/services/cleanup" "github.com/grafana/grafana/pkg/services/cleanup"
"github.com/grafana/grafana/pkg/services/cloudmigration" "github.com/grafana/grafana/pkg/services/cloudmigration"
"github.com/grafana/grafana/pkg/services/dashboardsnapshots" "github.com/grafana/grafana/pkg/services/dashboardsnapshots"
@@ -73,7 +72,7 @@ func ProvideBackgroundServiceRegistry(
_ dashboardsnapshots.Service, _ dashboardsnapshots.Service,
_ serviceaccounts.Service, _ *guardian.Provider, _ serviceaccounts.Service, _ *guardian.Provider,
_ *plugindashboardsservice.DashboardUpdater, _ *sanitizer.Provider, _ *plugindashboardsservice.DashboardUpdater, _ *sanitizer.Provider,
_ *grpcserver.HealthService, _ authz.Client, _ *grpcserver.ReflectionService, _ *grpcserver.HealthService, _ *grpcserver.ReflectionService,
_ *ldapapi.Service, _ *apiregistry.Service, _ auth.IDService, _ *teamapi.TeamAPI, _ ssosettings.Service, _ *ldapapi.Service, _ *apiregistry.Service, _ auth.IDService, _ *teamapi.TeamAPI, _ ssosettings.Service,
_ cloudmigration.Service, _ authnimpl.Registration, _ cloudmigration.Service, _ authnimpl.Registration,
) *BackgroundServiceRegistry { ) *BackgroundServiceRegistry {
+11 -26
View File
@@ -2,6 +2,7 @@ package authz
import ( import (
"context" "context"
"errors"
"github.com/fullstorydev/grpchan" "github.com/fullstorydev/grpchan"
"github.com/fullstorydev/grpchan/inprocgrpc" "github.com/fullstorydev/grpchan/inprocgrpc"
@@ -27,56 +28,40 @@ import (
// `authzService` is hardcoded in authz-service // `authzService` is hardcoded in authz-service
const authzServiceAudience = "authzService" const authzServiceAudience = "authzService"
type Client interface {
authzlib.AccessClient
}
// ProvideAuthZClient provides an AuthZ client and creates the AuthZ service. // ProvideAuthZClient provides an AuthZ client and creates the AuthZ service.
func ProvideAuthZClient( func ProvideAuthZClient(
cfg *setting.Cfg, features featuremgmt.FeatureToggles, grpcServer grpcserver.Provider, cfg *setting.Cfg, features featuremgmt.FeatureToggles, grpcServer grpcserver.Provider,
tracer tracing.Tracer, db db.DB, tracer tracing.Tracer, db db.DB,
) (Client, error) { ) (authzlib.AccessClient, error) {
if !features.IsEnabledGlobally(featuremgmt.FlagAuthZGRPCServer) {
return nil, nil
}
authCfg, err := ReadCfg(cfg) authCfg, err := ReadCfg(cfg)
if err != nil { if err != nil {
return nil, err return nil, err
} }
var client Client isRemoteServer := authCfg.mode == ModeCloud || authCfg.mode == ModeGRPC
if !features.IsEnabledGlobally(featuremgmt.FlagAuthZGRPCServer) && isRemoteServer {
return nil, errors.New("authZGRPCServer feature toggle is required for cloud and grpc mode")
}
// Register the server // Register the server
sql := legacysql.NewDatabaseProvider(db) sql := legacysql.NewDatabaseProvider(db)
server := rbac.NewService(sql, legacy.NewLegacySQLStores(sql), log.New("authz-grpc-server"), tracer) server := rbac.NewService(sql, legacy.NewLegacySQLStores(sql), log.New("authz-grpc-server"), tracer)
switch authCfg.mode { switch authCfg.mode {
case ModeInProc:
client, err = newInProcLegacyClient(server, tracer)
if err != nil {
return nil, err
}
case ModeGRPC: case ModeGRPC:
client, err = newGrpcLegacyClient(authCfg, tracer) return newGrpcLegacyClient(authCfg, tracer)
if err != nil {
return nil, err
}
case ModeCloud: case ModeCloud:
client, err = newCloudLegacyClient(authCfg, tracer) return newCloudLegacyClient(authCfg, tracer)
if err != nil { default:
return nil, err return newInProcLegacyClient(server, tracer)
}
} }
return client, err
} }
// ProvideStandaloneAuthZClient provides a standalone AuthZ client, without registering the AuthZ service. // ProvideStandaloneAuthZClient provides a standalone AuthZ client, without registering the AuthZ service.
// You need to provide a remote address in the configuration // You need to provide a remote address in the configuration
func ProvideStandaloneAuthZClient( func ProvideStandaloneAuthZClient(
cfg *setting.Cfg, features featuremgmt.FeatureToggles, tracer tracing.Tracer, cfg *setting.Cfg, features featuremgmt.FeatureToggles, tracer tracing.Tracer,
) (Client, error) { ) (authzlib.AccessClient, error) {
if !features.IsEnabledGlobally(featuremgmt.FlagAuthZGRPCServer) { if !features.IsEnabledGlobally(featuremgmt.FlagAuthZGRPCServer) {
return nil, nil return nil, nil
} }
+3 -3
View File
@@ -12,12 +12,12 @@ import (
"google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/credentials/insecure"
authnlib "github.com/grafana/authlib/authn" authnlib "github.com/grafana/authlib/authn"
"github.com/grafana/authlib/authz"
infraDB "github.com/grafana/grafana/pkg/infra/db" infraDB "github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/services/apiserver/options" "github.com/grafana/grafana/pkg/services/apiserver/options"
"github.com/grafana/grafana/pkg/services/authn/grpcutils" "github.com/grafana/grafana/pkg/services/authn/grpcutils"
"github.com/grafana/grafana/pkg/services/authz"
"github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/storage/legacysql" "github.com/grafana/grafana/pkg/storage/legacysql"
@@ -35,7 +35,7 @@ func ProvideUnifiedStorageClient(
db infraDB.DB, db infraDB.DB,
tracer tracing.Tracer, tracer tracing.Tracer,
reg prometheus.Registerer, reg prometheus.Registerer,
authzc authz.Client, authzc authz.AccessClient,
docs resource.DocumentBuilderSupplier, docs resource.DocumentBuilderSupplier,
) (resource.ResourceClient, error) { ) (resource.ResourceClient, error) {
// See: apiserver.ApplyGrafanaConfig(cfg, features, o) // See: apiserver.ApplyGrafanaConfig(cfg, features, o)
@@ -62,7 +62,7 @@ func newClient(opts options.StorageOptions,
db infraDB.DB, db infraDB.DB,
tracer tracing.Tracer, tracer tracing.Tracer,
reg prometheus.Registerer, reg prometheus.Registerer,
authzc authz.Client, authzc authz.AccessClient,
docs resource.DocumentBuilderSupplier, docs resource.DocumentBuilderSupplier,
) (resource.ResourceClient, error) { ) (resource.ResourceClient, error) {
ctx := context.Background() ctx := context.Background()
+7 -3
View File
@@ -372,7 +372,7 @@ func (s *server) newEvent(ctx context.Context, user claims.AuthInfo, key *Resour
} }
check := authz.CheckRequest{ check := authz.CheckRequest{
Verb: "create", Verb: utils.VerbCreate,
Group: key.Group, Group: key.Group,
Resource: key.Resource, Resource: key.Resource,
Namespace: key.Namespace, Namespace: key.Namespace,
@@ -386,7 +386,7 @@ func (s *server) newEvent(ctx context.Context, user claims.AuthInfo, key *Resour
if oldValue == nil { if oldValue == nil {
event.Type = WatchEvent_ADDED event.Type = WatchEvent_ADDED
} else { } else {
check.Verb = "update" check.Verb = utils.VerbUpdate
temp := &unstructured.Unstructured{} temp := &unstructured.Unstructured{}
err = temp.UnmarshalJSON(oldValue) err = temp.UnmarshalJSON(oldValue)
@@ -437,8 +437,12 @@ func (s *server) newEvent(ctx context.Context, user claims.AuthInfo, key *Resour
return nil, err return nil, err
} }
// We only set name for update checks
if check.Verb == utils.VerbUpdate {
check.Name = key.Name
}
check.Folder = obj.GetFolder() check.Folder = obj.GetFolder()
check.Name = key.Name
a, err := s.access.Check(ctx, user, check) a, err := s.access.Check(ctx, user, check)
if err != nil { if err != nil {
return nil, AsErrorResult(err) return nil, AsErrorResult(err)
+2 -2
View File
@@ -7,13 +7,13 @@ import (
"path/filepath" "path/filepath"
"strings" "strings"
"github.com/grafana/authlib/authz"
"github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus"
"github.com/grafana/grafana/pkg/storage/unified/search" "github.com/grafana/grafana/pkg/storage/unified/search"
infraDB "github.com/grafana/grafana/pkg/infra/db" infraDB "github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/services/authz"
"github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/storage/unified/resource" "github.com/grafana/grafana/pkg/storage/unified/resource"
@@ -23,7 +23,7 @@ import (
// Creates a new ResourceServer // Creates a new ResourceServer
func NewResourceServer(ctx context.Context, db infraDB.DB, cfg *setting.Cfg, func NewResourceServer(ctx context.Context, db infraDB.DB, cfg *setting.Cfg,
features featuremgmt.FeatureToggles, docs resource.DocumentBuilderSupplier, features featuremgmt.FeatureToggles, docs resource.DocumentBuilderSupplier,
tracer tracing.Tracer, reg prometheus.Registerer, ac authz.Client) (resource.ResourceServer, error) { tracer tracing.Tracer, reg prometheus.Registerer, ac authz.AccessClient) (resource.ResourceServer, error) {
apiserverCfg := cfg.SectionWithEnvOverrides("grafana-apiserver") apiserverCfg := cfg.SectionWithEnvOverrides("grafana-apiserver")
opts := resource.ResourceServerOptions{ opts := resource.ResourceServerOptions{
Tracer: tracer, Tracer: tracer,
+8 -1
View File
@@ -442,7 +442,14 @@ func (c *K8sTestHelper) LoadYAMLOrJSON(body string) *unstructured.Unstructured {
func (c *K8sTestHelper) createTestUsers(orgName string) OrgUsers { func (c *K8sTestHelper) createTestUsers(orgName string) OrgUsers {
c.t.Helper() c.t.Helper()
users := OrgUsers{ users := OrgUsers{
Admin: c.CreateUser("admin", orgName, org.RoleAdmin, nil), Admin: c.CreateUser("admin", orgName, org.RoleAdmin, []resourcepermissions.SetResourcePermissionCommand{
{
Actions: []string{"dashboards:read", "dashboards:write", "dashboards:create", "dashboards:delete"},
Resource: "dashboards",
ResourceAttribute: "uid",
ResourceID: "*",
},
}),
Editor: c.CreateUser("editor", orgName, org.RoleEditor, nil), Editor: c.CreateUser("editor", orgName, org.RoleEditor, nil),
Viewer: c.CreateUser("viewer", orgName, org.RoleViewer, nil), Viewer: c.CreateUser("viewer", orgName, org.RoleViewer, nil),
} }