From 830600dab0026cd290fcf2da357667feb6569404 Mon Sep 17 00:00:00 2001 From: Claudiu Dragalina-Paraipan Date: Thu, 24 Oct 2024 10:12:37 +0300 Subject: [PATCH] AuthN: Optionally use tokens for unified storage client authentication (#91665) * extracted in-proc mode to #93124 * allow insecure conns in dev mode + refactoring * removed ModeCloud, relying on ModeGrpc and stackID instead to discover if we're running in Cloud * remove the NamespaceAuthorizer would fail in legacy mode. It will be added back in the future. * use FlagAppPlatformGrpcClientAuth to enable new behavior, instead of legacy * extracted authz package changes in #95120 * extracted server side changes in #95086 --------- Co-authored-by: Gabriel MABILLE Co-authored-by: gamab Co-authored-by: Dan Cech --- pkg/server/test_env.go | 4 ++ pkg/services/authn/grpcutils/config.go | 16 ++++++ pkg/services/grpcserver/service.go | 6 +-- pkg/storage/unified/client.go | 54 ++++++++++++++++--- pkg/storage/unified/resource/client.go | 53 +++++++++++++++++- .../unified/sql/test/integration_test.go | 4 +- pkg/tests/apis/helper.go | 6 ++- 7 files changed, 127 insertions(+), 16 deletions(-) diff --git a/pkg/server/test_env.go b/pkg/server/test_env.go index 51a54dde56c..f60f9c6703e 100644 --- a/pkg/server/test_env.go +++ b/pkg/server/test_env.go @@ -4,6 +4,7 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/httpclient" "github.com/grafana/grafana/pkg/plugins/manager/registry" + "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/grpcserver" "github.com/grafana/grafana/pkg/services/notifications" @@ -24,6 +25,7 @@ func ProvideTestEnv( oAuthTokenService *oauthtokentest.Service, featureMgmt featuremgmt.FeatureToggles, resourceClient resource.ResourceClient, + idService auth.IDService, ) (*TestEnv, error) { return &TestEnv{ Server: server, @@ -36,6 +38,7 @@ func ProvideTestEnv( OAuthTokenService: oAuthTokenService, FeatureToggles: featureMgmt, ResourceClient: resourceClient, + IDService: idService, }, nil } @@ -51,4 +54,5 @@ type TestEnv struct { RequestMiddleware web.Middleware FeatureToggles featuremgmt.FeatureToggles ResourceClient resource.ResourceClient + IDService auth.IDService } diff --git a/pkg/services/authn/grpcutils/config.go b/pkg/services/authn/grpcutils/config.go index 253ba6fe8a7..91929a0d1c5 100644 --- a/pkg/services/authn/grpcutils/config.go +++ b/pkg/services/authn/grpcutils/config.go @@ -43,3 +43,19 @@ func ReadGrpcServerConfig(cfg *setting.Cfg) (*GrpcServerConfig, error) { LegacyFallback: section.Key("legacy_fallback").MustBool(true), }, nil } + +type GrpcClientConfig struct { + Token string + TokenExchangeURL string + TokenNamespace string +} + +func ReadGrpcClientConfig(cfg *setting.Cfg) *GrpcClientConfig { + section := cfg.SectionWithEnvOverrides("grpc_client_authentication") + + return &GrpcClientConfig{ + Token: section.Key("token").MustString(""), + TokenExchangeURL: section.Key("token_exchange_url").MustString(""), + TokenNamespace: section.Key("token_namespace").MustString("stacks-" + cfg.StackID), + } +} diff --git a/pkg/services/grpcserver/service.go b/pkg/services/grpcserver/service.go index 611746ee88b..d94ad836874 100644 --- a/pkg/services/grpcserver/service.go +++ b/pkg/services/grpcserver/service.go @@ -69,12 +69,10 @@ func ProvideService(cfg *setting.Cfg, features featuremgmt.FeatureToggles, authe } } - var opts []grpc.ServerOption - // Default auth is admin token check, but this can be overridden by // services which implement ServiceAuthFuncOverride interface. // See https://github.com/grpc-ecosystem/go-grpc-middleware/blob/main/interceptors/auth/auth.go#L30. - opts = append(opts, []grpc.ServerOption{ + opts := []grpc.ServerOption{ grpc.StatsHandler(otelgrpc.NewServerHandler()), grpc.ChainUnaryInterceptor( grpcAuth.UnaryServerInterceptor(authenticator.Authenticate), @@ -86,7 +84,7 @@ func ProvideService(cfg *setting.Cfg, features featuremgmt.FeatureToggles, authe grpcAuth.StreamServerInterceptor(authenticator.Authenticate), middleware.StreamServerInstrumentInterceptor(grpcRequestDuration), ), - }...) + } if s.cfg.GRPCServerTLSConfig != nil { opts = append(opts, grpc.Creds(credentials.NewTLS(cfg.GRPCServerTLSConfig))) diff --git a/pkg/storage/unified/client.go b/pkg/storage/unified/client.go index f7041599c3a..4d1b225c038 100644 --- a/pkg/storage/unified/client.go +++ b/pkg/storage/unified/client.go @@ -5,20 +5,26 @@ import ( "fmt" "path/filepath" - infraDB "github.com/grafana/grafana/pkg/infra/db" - "github.com/grafana/grafana/pkg/infra/tracing" - "github.com/grafana/grafana/pkg/services/apiserver/options" - "github.com/grafana/grafana/pkg/services/featuremgmt" - "github.com/grafana/grafana/pkg/setting" - "github.com/grafana/grafana/pkg/storage/unified/resource" - "github.com/grafana/grafana/pkg/storage/unified/sql" "github.com/prometheus/client_golang/prometheus" "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" "gocloud.dev/blob/fileblob" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" + + authnlib "github.com/grafana/authlib/authn" + + infraDB "github.com/grafana/grafana/pkg/infra/db" + "github.com/grafana/grafana/pkg/infra/tracing" + "github.com/grafana/grafana/pkg/services/apiserver/options" + "github.com/grafana/grafana/pkg/services/authn/grpcutils" + "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/storage/unified/resource" + "github.com/grafana/grafana/pkg/storage/unified/sql" ) +const resourceStoreAudience = "resourceStore" + // This adds a UnifiedStorage client into the wire dependency tree func ProvideUnifiedStorageClient( cfg *setting.Cfg, @@ -79,7 +85,13 @@ func ProvideUnifiedStorageClient( if err != nil { return nil, err } - return resource.NewResourceClient(conn), nil + + // Create a client instance + client, err := newResourceClient(conn, cfg, features) + if err != nil { + return nil, err + } + return client, nil // Use the local SQL default: @@ -90,3 +102,29 @@ func ProvideUnifiedStorageClient( return resource.NewLocalResourceClient(server), nil } } + +func clientCfgMapping(clientCfg *grpcutils.GrpcClientConfig) authnlib.GrpcClientConfig { + return authnlib.GrpcClientConfig{ + TokenClientConfig: &authnlib.TokenExchangeConfig{ + Token: clientCfg.Token, + TokenExchangeURL: clientCfg.TokenExchangeURL, + }, + TokenRequest: &authnlib.TokenExchangeRequest{ + Namespace: clientCfg.TokenNamespace, + Audiences: []string{resourceStoreAudience}, + }, + } +} + +func newResourceClient(conn *grpc.ClientConn, cfg *setting.Cfg, features featuremgmt.FeatureToggles) (resource.ResourceClient, error) { + if !features.IsEnabledGlobally(featuremgmt.FlagAppPlatformGrpcClientAuth) { + return resource.NewLegacyResourceClient(conn), nil + } + if cfg.StackID == "" { + return resource.NewGRPCResourceClient(conn) + } + + grpcClientCfg := grpcutils.ReadGrpcClientConfig(cfg) + + return resource.NewCloudResourceClient(conn, clientCfgMapping(grpcClientCfg), cfg.Env == setting.Dev) +} diff --git a/pkg/storage/unified/resource/client.go b/pkg/storage/unified/resource/client.go index cecbcfcb3b8..46c3c5c3605 100644 --- a/pkg/storage/unified/resource/client.go +++ b/pkg/storage/unified/resource/client.go @@ -2,7 +2,9 @@ package resource import ( "context" + "crypto/tls" "fmt" + "net/http" "time" "github.com/fullstorydev/grpchan" @@ -35,7 +37,7 @@ type resourceClient struct { DiagnosticsClient } -func NewResourceClient(channel *grpc.ClientConn) ResourceClient { +func NewLegacyResourceClient(channel *grpc.ClientConn) ResourceClient { cc := grpchan.InterceptClientConn(channel, grpcUtils.UnaryClientInterceptor, grpcUtils.StreamClientInterceptor) return &resourceClient{ ResourceStoreClient: NewResourceStoreClient(cc), @@ -46,6 +48,7 @@ func NewResourceClient(channel *grpc.ClientConn) ResourceClient { } func NewLocalResourceClient(server ResourceServer) ResourceClient { + // scenario: local in-proc channel := &inprocgrpc.Channel{} grpcAuthInt := grpcutils.NewInProcGrpcAuthenticator() @@ -80,6 +83,48 @@ func NewLocalResourceClient(server ResourceServer) ResourceClient { } } +func NewGRPCResourceClient(conn *grpc.ClientConn) (ResourceClient, error) { + // scenario: remote on-prem + clientInt, err := authnlib.NewGrpcClientInterceptor( + &authnlib.GrpcClientConfig{}, + authnlib.WithDisableAccessTokenOption(), + authnlib.WithIDTokenExtractorOption(idTokenExtractor), + ) + if err != nil { + return nil, err + } + + cc := grpchan.InterceptClientConn(conn, clientInt.UnaryClientInterceptor, clientInt.StreamClientInterceptor) + return &resourceClient{ + ResourceStoreClient: NewResourceStoreClient(cc), + ResourceIndexClient: NewResourceIndexClient(cc), + DiagnosticsClient: NewDiagnosticsClient(cc), + }, nil +} + +func NewCloudResourceClient(conn *grpc.ClientConn, cfg authnlib.GrpcClientConfig, allowInsecure bool) (ResourceClient, error) { + // scenario: remote cloud + opts := []authnlib.GrpcClientInterceptorOption{ + authnlib.WithIDTokenExtractorOption(idTokenExtractor), + } + + if allowInsecure { + opts = allowInsecureTransportOpt(&cfg, opts) + } + + clientInt, err := authnlib.NewGrpcClientInterceptor(&cfg, opts...) + if err != nil { + return nil, err + } + + cc := grpchan.InterceptClientConn(conn, clientInt.UnaryClientInterceptor, clientInt.StreamClientInterceptor) + return &resourceClient{ + ResourceStoreClient: NewResourceStoreClient(cc), + ResourceIndexClient: NewResourceIndexClient(cc), + DiagnosticsClient: NewDiagnosticsClient(cc), + }, nil +} + func idTokenExtractor(ctx context.Context) (string, error) { authInfo, ok := claims.From(ctx) if !ok { @@ -107,6 +152,12 @@ func idTokenExtractor(ctx context.Context) (string, error) { return "", fmt.Errorf("id-token not found") } +func allowInsecureTransportOpt(grpcClientConfig *authnlib.GrpcClientConfig, opts []authnlib.GrpcClientInterceptorOption) []authnlib.GrpcClientInterceptorOption { + client := &http.Client{Transport: &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}}} + tokenClient, _ := authnlib.NewTokenExchangeClient(*grpcClientConfig.TokenClientConfig, authnlib.WithHTTPClient(client)) + return append(opts, authnlib.WithTokenClientOption(tokenClient)) +} + // createInternalToken creates a symmetrically signed token for using in in-proc mode only. func createInternalToken(authInfo claims.AuthInfo) (string, *authnlib.Claims[authnlib.IDTokenClaims], error) { signerOpts := jose.SignerOptions{} diff --git a/pkg/storage/unified/sql/test/integration_test.go b/pkg/storage/unified/sql/test/integration_test.go index 96c0a397c4b..30414aed449 100644 --- a/pkg/storage/unified/sql/test/integration_test.go +++ b/pkg/storage/unified/sql/test/integration_test.go @@ -12,6 +12,7 @@ import ( "github.com/grafana/authlib/claims" "github.com/grafana/dskit/services" + "github.com/grafana/grafana/pkg/apimachinery/identity" infraDB "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/services/featuremgmt" @@ -374,7 +375,8 @@ func TestClientServer(t *testing.T) { t.Run("Create a client", func(t *testing.T) { conn, err := grpc.NewClient(svc.GetAddress(), grpc.WithTransportCredentials(insecure.NewCredentials())) require.NoError(t, err) - client = resource.NewResourceClient(conn) + client, err = resource.NewGRPCResourceClient(conn) + require.NoError(t, err) }) t.Run("Create a resource", func(t *testing.T) { diff --git a/pkg/tests/apis/helper.go b/pkg/tests/apis/helper.go index 2d27caf9ab1..c039bb26d31 100644 --- a/pkg/tests/apis/helper.go +++ b/pkg/tests/apis/helper.go @@ -33,7 +33,6 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/resourcepermissions" "github.com/grafana/grafana/pkg/services/apiserver/endpoints/request" - "github.com/grafana/grafana/pkg/services/auth/idtest" "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/org" @@ -64,6 +63,9 @@ type K8sTestHelper struct { func NewK8sTestHelper(t *testing.T, opts testinfra.GrafanaOpts) *K8sTestHelper { t.Helper() + // Always enable `FlagAppPlatformGrpcClientAuth` for k8s integration tests, as this is the desired behavior. + // The flag only exists to support the transition from the old to the new behavior in dev/ops/prod. + opts.EnableFeatureToggles = append(opts.EnableFeatureToggles, featuremgmt.FlagAppPlatformGrpcClientAuth) dir, path := testinfra.CreateGrafDir(t, opts) _, env := testinfra.StartGrafanaEnv(t, dir, path) @@ -497,7 +499,7 @@ func (c *K8sTestHelper) CreateUser(name string, orgName string, basicRole org.Ro require.Equal(c.t, orgId, s.OrgID) require.Equal(c.t, basicRole, s.OrgRole) // make sure the role was set properly - idToken, idClaims, err := idtest.CreateInternalToken(s, []byte("secret")) + idToken, idClaims, err := c.env.IDService.SignIdentity(context.Background(), s) require.NoError(c.t, err) s.IDToken = idToken s.IDTokenClaims = idClaims