[configgrpc] Send UNAUTHENTICATED on auth failure (open-telemetry#10670)

Fixes open-telemetry#7646 Signed-off-by: Juraci Paixão Kröhling <[email protected]> Signed-off-by: Juraci Paixão Kröhling <[email protected]>
jackgopack4 · Jul 24, 2024 · 49ea32b · 49ea32b
1 parent 68fb07b
commit 49ea32b
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 4 deletions.
diff --git a/.chloggen/jpkroehling-grpc-statuscode.yaml 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
@@ -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
@@ -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)
 }