From 5ae37e0dec7082710b6e07e6453ce68ecaa67eba Mon Sep 17 00:00:00 2001 From: Huiren Jiang Date: Tue, 10 Dec 2024 14:31:51 +0800 Subject: [PATCH 1/3] [bearertokenauthextension] Load token lazily for gRPC AUTH to fix token refresh issue --- .../bearertokenauthextension/bearertokenauth.go | 6 +++--- .../bearertokenauth_test.go | 14 +++++++++----- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/extension/bearertokenauthextension/bearertokenauth.go b/extension/bearertokenauthextension/bearertokenauth.go index e7a8ad3e4212..28ecbf965e08 100644 --- a/extension/bearertokenauthextension/bearertokenauth.go +++ b/extension/bearertokenauthextension/bearertokenauth.go @@ -23,12 +23,12 @@ var _ credentials.PerRPCCredentials = (*PerRPCAuth)(nil) // PerRPCAuth is a gRPC credentials.PerRPCCredentials implementation that returns an 'authorization' header. type PerRPCAuth struct { - metadata map[string]string + auth *BearerTokenAuth } // GetRequestMetadata returns the request metadata to be used with the RPC. func (c *PerRPCAuth) GetRequestMetadata(context.Context, ...string) (map[string]string, error) { - return c.metadata, nil + return map[string]string{"authorization": c.auth.authorizationValue()}, nil } // RequireTransportSecurity always returns true for this implementation. Passing bearer tokens in plain-text connections is a bad idea. @@ -171,7 +171,7 @@ func (b *BearerTokenAuth) Shutdown(_ context.Context) error { // PerRPCCredentials returns PerRPCAuth an implementation of credentials.PerRPCCredentials that func (b *BearerTokenAuth) PerRPCCredentials() (credentials.PerRPCCredentials, error) { return &PerRPCAuth{ - metadata: map[string]string{"authorization": b.authorizationValue()}, + auth: b, }, nil } diff --git a/extension/bearertokenauthextension/bearertokenauth_test.go b/extension/bearertokenauthextension/bearertokenauth_test.go index a6257a8511bd..2e3ea2663691 100644 --- a/extension/bearertokenauthextension/bearertokenauth_test.go +++ b/extension/bearertokenauthextension/bearertokenauth_test.go @@ -18,15 +18,19 @@ import ( ) func TestPerRPCAuth(t *testing.T) { - metadata := map[string]string{ - "authorization": "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9...", - } + cfg := createDefaultConfig().(*Config) + cfg.BearerToken = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9..." // test meta data is properly - perRPCAuth := &PerRPCAuth{metadata: metadata} + bauth := newBearerTokenAuth(cfg, nil) + assert.NotNil(t, bauth) + perRPCAuth := &PerRPCAuth{auth: bauth} md, err := perRPCAuth.GetRequestMetadata(context.Background()) assert.NoError(t, err) - assert.Equal(t, md, metadata) + expectedMetadata := map[string]string{ + "authorization": "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9...", + } + assert.Equal(t, md, expectedMetadata) // always true ok := perRPCAuth.RequireTransportSecurity() From 6a7e656ad13cfdcc8eaf8aa66e7f56e619af4365 Mon Sep 17 00:00:00 2001 From: Huiren Jiang Date: Tue, 10 Dec 2024 17:05:43 +0800 Subject: [PATCH 2/3] fix for PR comment --- .../bearertokenauth_test.go | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/extension/bearertokenauthextension/bearertokenauth_test.go b/extension/bearertokenauthextension/bearertokenauth_test.go index 2e3ea2663691..2517eb258178 100644 --- a/extension/bearertokenauthextension/bearertokenauth_test.go +++ b/extension/bearertokenauthextension/bearertokenauth_test.go @@ -206,6 +206,34 @@ func TestBearerTokenFileContentUpdate(t *testing.T) { assert.Equal(t, authHeaderValue, fmt.Sprintf("%s %s", scheme, string(token))) } +func TestBearerTokenUpdateForGrpc(t *testing.T) { + // prepare + cfg := createDefaultConfig().(*Config) + cfg.BearerToken = "1234" + + bauth := newBearerTokenAuth(cfg, zaptest.NewLogger(t)) + assert.NotNil(t, bauth) + + perRPCAuth, err := bauth.PerRPCCredentials() + assert.NoError(t, err) + + ctx := context.Background() + assert.NoError(t, bauth.Start(ctx, componenttest.NewNopHost())) + + // initial token, OK + md, err := perRPCAuth.GetRequestMetadata(context.Background()) + assert.NoError(t, err) + assert.Equal(t, map[string]string{"authorization": "Bearer " + "1234"}, md) + + // update the token + bauth.setAuthorizationValue("5678") + md, err = perRPCAuth.GetRequestMetadata(context.Background()) + assert.NoError(t, err) + assert.Equal(t, map[string]string{"authorization": "Bearer " + "5678"}, md) + + assert.NoError(t, bauth.Shutdown(context.Background())) +} + func TestBearerServerAuthenticateWithScheme(t *testing.T) { const token = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9..." // #nosec cfg := createDefaultConfig().(*Config) From ee169d8390703923d3fecbcefe9c88bc37f26fad Mon Sep 17 00:00:00 2001 From: Huiren Jiang Date: Wed, 11 Dec 2024 09:45:14 +0800 Subject: [PATCH 3/3] add changelog --- ...nauthextension-fix-grpc-token-refresh.yaml | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 .chloggen/bearertokenauthextension-fix-grpc-token-refresh.yaml diff --git a/.chloggen/bearertokenauthextension-fix-grpc-token-refresh.yaml b/.chloggen/bearertokenauthextension-fix-grpc-token-refresh.yaml new file mode 100644 index 000000000000..45dfd929d294 --- /dev/null +++ b/.chloggen/bearertokenauthextension-fix-grpc-token-refresh.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: bug_fix + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: bearertokenauthextension + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Load token lazily for gRPC AUTH to fix token refresh issue + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [36749] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: []