From 3fe2227c8213aa7e0de8e2bb87055d770517c92e Mon Sep 17 00:00:00 2001 From: Georges Chaudy Date: Tue, 17 Dec 2024 12:28:00 +0100 Subject: [PATCH] [auth] make id-token optional (#97831) make idtoken optional enure there is always an identity in the context fix: update token fix: now it should work fix: now it should work --- pkg/apiserver/rest/dualwriter_syncer.go | 2 +- .../authn/grpcutils/grpc_authenticator.go | 13 +--- pkg/storage/unified/resource/client.go | 61 +------------------ pkg/storage/unified/resource/client_test.go | 27 ++++++++ 4 files changed, 32 insertions(+), 71 deletions(-) create mode 100644 pkg/storage/unified/resource/client_test.go diff --git a/pkg/apiserver/rest/dualwriter_syncer.go b/pkg/apiserver/rest/dualwriter_syncer.go index 8f82e696a30..0ae57464760 100644 --- a/pkg/apiserver/rest/dualwriter_syncer.go +++ b/pkg/apiserver/rest/dualwriter_syncer.go @@ -17,7 +17,6 @@ import ( "k8s.io/klog/v2" "github.com/grafana/authlib/claims" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/apimachinery/utils" ) @@ -160,6 +159,7 @@ func legacyToUnifiedStorageDataSyncer(ctx context.Context, cfg *SyncerConfig) (b log.Info("starting legacyToUnifiedStorageDataSyncer") startSync := time.Now() + // Add a claim to the context to allow the background job to use the underlying access_token permissions. orgId := int64(1) ctx = klog.NewContext(ctx, log) diff --git a/pkg/services/authn/grpcutils/grpc_authenticator.go b/pkg/services/authn/grpcutils/grpc_authenticator.go index 1b326d0d553..421477ecb9d 100644 --- a/pkg/services/authn/grpcutils/grpc_authenticator.go +++ b/pkg/services/authn/grpcutils/grpc_authenticator.go @@ -23,7 +23,7 @@ func NewInProcGrpcAuthenticator() *authnlib.GrpcAuthenticator { return authnlib.NewUnsafeGrpcAuthenticator( &authnlib.GrpcAuthenticatorConfig{}, authnlib.WithDisableAccessTokenAuthOption(), - authnlib.WithIDTokenAuthOption(true), + authnlib.WithIDTokenAuthOption(false), ) } @@ -47,21 +47,14 @@ func NewGrpcAuthenticator(authCfg *GrpcServerConfig, tracer tracing.Tracer) (*au grpcOpts := []authnlib.GrpcAuthenticatorOption{ authnlib.WithKeyRetrieverOption(keyRetriever), authnlib.WithTracerAuthOption(tracer), + authnlib.WithIDTokenAuthOption(false), } - switch authCfg.Mode { - case ModeOnPrem: + if authCfg.Mode == ModeOnPrem { grpcOpts = append(grpcOpts, // Access token are not yet available on-prem authnlib.WithDisableAccessTokenAuthOption(), - authnlib.WithIDTokenAuthOption(true), - ) - case ModeCloud: - grpcOpts = append(grpcOpts, - // ID tokens are enabled but not required in cloud - authnlib.WithIDTokenAuthOption(false), ) } - return authnlib.NewGrpcAuthenticator( &grpcAuthCfg, grpcOpts..., diff --git a/pkg/storage/unified/resource/client.go b/pkg/storage/unified/resource/client.go index 5bbd0d88b9a..dc0b8923e3e 100644 --- a/pkg/storage/unified/resource/client.go +++ b/pkg/storage/unified/resource/client.go @@ -5,20 +5,15 @@ import ( "crypto/tls" "fmt" "net/http" - "time" "github.com/fullstorydev/grpchan" "github.com/fullstorydev/grpchan/inprocgrpc" - "github.com/go-jose/go-jose/v3" - "github.com/go-jose/go-jose/v3/jwt" authnlib "github.com/grafana/authlib/authn" "github.com/grafana/authlib/claims" grpcAuth "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/auth" "google.golang.org/grpc" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/tracing" - "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/authn/grpcutils" grpcUtils "github.com/grafana/grafana/pkg/storage/unified/resource/grpc" ) @@ -139,19 +134,7 @@ func idTokenExtractor(ctx context.Context) (string, error) { return token[0], nil } - // If no token is found, create an internal token. - // This is a workaround for StaticRequester not having a signed ID token. - if staticRequester, ok := authInfo.(*identity.StaticRequester); ok { - token, _, err := createInternalToken(staticRequester) - if err != nil { - return "", fmt.Errorf("failed to create internal token: %w", err) - } - - staticRequester.IDToken = token - return token, nil - } - - return "", fmt.Errorf("id-token not found") + return "", nil } func allowInsecureTransportOpt(grpcClientConfig *authnlib.GrpcClientConfig, opts []authnlib.GrpcClientInterceptorOption) []authnlib.GrpcClientInterceptorOption { @@ -159,45 +142,3 @@ func allowInsecureTransportOpt(grpcClientConfig *authnlib.GrpcClientConfig, opts 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{} - signerOpts.WithType("jwt") // Should be uppercase, but this is what authlib expects - signer, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.HS256, Key: []byte("internal key")}, &signerOpts) - if err != nil { - return "", nil, err - } - - now := time.Now() - tokenTTL := 10 * time.Minute - idClaims := &auth.IDClaims{ - Claims: jwt.Claims{ - Audience: authInfo.GetAudience(), - Subject: authInfo.GetSubject(), - Expiry: jwt.NewNumericDate(now.Add(tokenTTL)), - IssuedAt: jwt.NewNumericDate(now), - }, - Rest: authnlib.IDTokenClaims{ - Namespace: authInfo.GetNamespace(), - Identifier: authInfo.GetIdentifier(), - Type: authInfo.GetIdentityType(), - }, - } - - if claims.IsIdentityType(authInfo.GetIdentityType(), claims.TypeUser) { - idClaims.Rest.Email = authInfo.GetEmail() - idClaims.Rest.EmailVerified = authInfo.GetEmailVerified() - idClaims.Rest.AuthenticatedBy = authInfo.GetAuthenticatedBy() - idClaims.Rest.Username = authInfo.GetUsername() - idClaims.Rest.DisplayName = authInfo.GetName() - } - - builder := jwt.Signed(signer).Claims(&idClaims.Rest).Claims(idClaims.Claims) - token, err := builder.CompactSerialize() - if err != nil { - return "", nil, err - } - - return token, idClaims, nil -} diff --git a/pkg/storage/unified/resource/client_test.go b/pkg/storage/unified/resource/client_test.go new file mode 100644 index 00000000000..819c3821914 --- /dev/null +++ b/pkg/storage/unified/resource/client_test.go @@ -0,0 +1,27 @@ +package resource + +import ( + "context" + "testing" + + "github.com/grafana/authlib/claims" + "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/stretchr/testify/assert" +) + +func TestIDTokenExtractor(t *testing.T) { + t.Run("should return an error when no claims found", func(t *testing.T) { + token, err := idTokenExtractor(context.Background()) + assert.Error(t, err) + assert.Empty(t, token) + }) + t.Run("should return an empty token for static requester of type service account as grafana admin ", func(t *testing.T) { + ctx := identity.WithRequester(context.Background(), &identity.StaticRequester{ + Type: claims.TypeServiceAccount, + IsGrafanaAdmin: true, + }) + token, err := idTokenExtractor(ctx) + assert.NoError(t, err) + assert.Empty(t, token) + }) +}