From 49ea32b28914107f97d8d819921a2fe2b0bde9b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juraci=20Paix=C3=A3o=20Kr=C3=B6hling?= Date: Wed, 24 Jul 2024 16:53:35 +0200 Subject: [PATCH] [configgrpc] Send UNAUTHENTICATED on auth failure (#10670) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #7646 Signed-off-by: Juraci Paixão Kröhling Signed-off-by: Juraci Paixão Kröhling --- .chloggen/jpkroehling-grpc-statuscode.yaml | 12 ++++++++++++ config/configgrpc/configgrpc.go | 6 ++++-- config/configgrpc/configgrpc_test.go | 8 ++++++-- 3 files changed, 22 insertions(+), 4 deletions(-) create mode 100644 .chloggen/jpkroehling-grpc-statuscode.yaml diff --git a/.chloggen/jpkroehling-grpc-statuscode.yaml b/.chloggen/jpkroehling-grpc-statuscode.yaml new file mode 100644 index 00000000000..d78ef4c7a9a --- /dev/null +++ b/.chloggen/jpkroehling-grpc-statuscode.yaml @@ -0,0 +1,12 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: 'enhancement' + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: configgrpc + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: gRPC auth errors now return gRPC status code UNAUTHENTICATED (16) + +# One or more tracking issues or pull requests related to the change +issues: [7646] + diff --git a/config/configgrpc/configgrpc.go b/config/configgrpc/configgrpc.go index f718f25913e..951aa93a87c 100644 --- a/config/configgrpc/configgrpc.go +++ b/config/configgrpc/configgrpc.go @@ -17,12 +17,14 @@ import ( "go.opentelemetry.io/otel" "google.golang.org/grpc" "google.golang.org/grpc/balancer" + "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials" "google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/encoding/gzip" "google.golang.org/grpc/keepalive" "google.golang.org/grpc/metadata" "google.golang.org/grpc/peer" + "google.golang.org/grpc/status" "go.opentelemetry.io/collector/client" "go.opentelemetry.io/collector/component" @@ -478,7 +480,7 @@ func authUnaryServerInterceptor(ctx context.Context, req any, _ *grpc.UnaryServe ctx, err := server.Authenticate(ctx, headers) if err != nil { - return nil, err + return nil, status.Error(codes.Unauthenticated, err.Error()) } return handler(ctx, req) @@ -493,7 +495,7 @@ func authStreamServerInterceptor(srv any, stream grpc.ServerStream, _ *grpc.Stre ctx, err := server.Authenticate(ctx, headers) if err != nil { - return err + return status.Error(codes.Unauthenticated, err.Error()) } return handler(srv, wrapServerStream(ctx, stream)) diff --git a/config/configgrpc/configgrpc_test.go b/config/configgrpc/configgrpc_test.go index 0b2b1cf4cd2..1769733dea6 100644 --- a/config/configgrpc/configgrpc_test.go +++ b/config/configgrpc/configgrpc_test.go @@ -19,8 +19,10 @@ import ( "go.uber.org/zap/zaptest/observer" "google.golang.org/grpc" "google.golang.org/grpc/balancer" + "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" "google.golang.org/grpc/peer" + "google.golang.org/grpc/status" "go.opentelemetry.io/collector/client" "go.opentelemetry.io/collector/component" @@ -1022,7 +1024,8 @@ func TestDefaultUnaryInterceptorAuthFailure(t *testing.T) { // verify assert.Nil(t, res) - assert.Equal(t, expectedErr, err) + assert.ErrorContains(t, err, expectedErr.Error()) + assert.Equal(t, codes.Unauthenticated, status.Code(err)) assert.True(t, authCalled) } @@ -1098,7 +1101,8 @@ func TestDefaultStreamInterceptorAuthFailure(t *testing.T) { err := authStreamServerInterceptor(nil, streamServer, &grpc.StreamServerInfo{}, handler, auth.NewServer(auth.WithServerAuthenticate(authFunc))) // verify - assert.Equal(t, expectedErr, err) + assert.ErrorContains(t, err, expectedErr.Error()) // unfortunately, grpc errors don't wrap the original ones + assert.Equal(t, codes.Unauthenticated, status.Code(err)) assert.True(t, authCalled) }