Skip to content

Commit 627bbaf

Browse files
cnvergencecnvergence
committed
use grpc status errors in jwtinterceptor
Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>
1 parent 9aaac88 commit 627bbaf

2 files changed

Lines changed: 55 additions & 40 deletions

File tree

‎internal/xds/server/kubejwt/jwtinterceptor.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,13 @@ package kubejwt
77

88
import (
99
"context"
10-
"fmt"
1110
"strings"
1211

1312
discoveryv3 "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v3"
1413
"google.golang.org/grpc"
14+
"google.golang.org/grpc/codes"
1515
"google.golang.org/grpc/metadata"
16+
"google.golang.org/grpc/status"
1617
"k8s.io/client-go/kubernetes"
1718

1819
"github.com/envoyproxy/gateway/internal/logging"
@@ -51,18 +52,17 @@ func (i *JWTAuthInterceptor) authenticate(ctx context.Context, msg any) error {
5152

5253
md, ok := metadata.FromIncomingContext(ctx)
5354
if !ok {
54-
return fmt.Errorf("missing metadata")
55+
return status.Errorf(codes.Unauthenticated, "missing metadata for node %s", nodeID)
5556
}
5657

5758
authHeader := md.Get("authorization")
5859
if len(authHeader) == 0 {
59-
return fmt.Errorf("missing authorization token in metadata: %s", md)
60+
return status.Errorf(codes.Unauthenticated, "missing authorization header for node %s", nodeID)
6061
}
6162
token := strings.TrimPrefix(authHeader[0], "Bearer ")
6263

6364
if err := i.validateKubeJWT(ctx, token, nodeID); err != nil {
64-
i.logger.Error(err, "failed to validate token")
65-
return fmt.Errorf("failed to validate token: %w", err)
65+
return status.Errorf(codes.Unauthenticated, "failed to validate the token for node %s: %v", nodeID, err)
6666
}
6767

6868
return nil
@@ -109,15 +109,15 @@ func extractNodeID(m any) (string, error) {
109109
switch req := m.(type) {
110110
case *discoveryv3.DeltaDiscoveryRequest:
111111
if req.Node == nil || req.Node.Id == "" {
112-
return "", fmt.Errorf("missing node ID")
112+
return "", status.Error(codes.InvalidArgument, "missing node ID")
113113
}
114114
return req.Node.Id, nil
115115
case *discoveryv3.DiscoveryRequest:
116116
if req.Node == nil || req.Node.Id == "" {
117-
return "", fmt.Errorf("missing node ID")
117+
return "", status.Error(codes.InvalidArgument, "missing node ID")
118118
}
119119
return req.Node.Id, nil
120120
default:
121-
return "", fmt.Errorf("unexpected message type: %T", m)
121+
return "", status.Errorf(codes.InvalidArgument, "unexpected message type: %T", m)
122122
}
123123
}

‎internal/xds/server/kubejwt/jwtinterceptor_test.go‎

Lines changed: 47 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@ import (
1616
"github.com/stretchr/testify/assert"
1717
"github.com/stretchr/testify/require"
1818
"google.golang.org/grpc"
19+
"google.golang.org/grpc/codes"
1920
"google.golang.org/grpc/metadata"
21+
"google.golang.org/grpc/status"
2022

2123
egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
2224
"github.com/envoyproxy/gateway/internal/logging"
@@ -42,85 +44,96 @@ func (m *mockServerStream) RecvMsg(_ any) error { return nil }
4244
// authTestCase defines a single test case for the authenticate method.
4345
// The same cases are run against both the stream (RecvMsg) and unary interceptor paths.
4446
type authTestCase struct {
45-
name string
46-
msg any
47-
ctx context.Context
48-
wantErr string
47+
name string
48+
msg any
49+
ctx context.Context
50+
wantErr string
51+
wantCode codes.Code
4952
}
5053

5154
func authTestCases() []authTestCase {
5255
return []authTestCase{
5356
{
54-
name: "unknown message type is rejected (fail-closed)",
55-
msg: &discoveryv3.DiscoveryResponse{},
56-
ctx: context.Background(),
57-
wantErr: "unexpected message type",
57+
name: "unknown message type is rejected (fail-closed)",
58+
msg: &discoveryv3.DiscoveryResponse{},
59+
ctx: context.Background(),
60+
wantErr: "unexpected message type",
61+
wantCode: codes.InvalidArgument,
5862
},
5963
{
60-
name: "nil message is rejected",
61-
msg: nil,
62-
ctx: context.Background(),
63-
wantErr: "unexpected message type",
64+
name: "nil message is rejected",
65+
msg: nil,
66+
ctx: context.Background(),
67+
wantErr: "unexpected message type",
68+
wantCode: codes.InvalidArgument,
6469
},
6570
{
66-
name: "DeltaDiscoveryRequest with nil node is rejected",
67-
msg: &discoveryv3.DeltaDiscoveryRequest{},
68-
ctx: context.Background(),
69-
wantErr: "missing node ID",
71+
name: "DeltaDiscoveryRequest with nil node is rejected",
72+
msg: &discoveryv3.DeltaDiscoveryRequest{},
73+
ctx: context.Background(),
74+
wantErr: "missing node ID",
75+
wantCode: codes.InvalidArgument,
7076
},
7177
{
72-
name: "DiscoveryRequest with nil node is rejected",
73-
msg: &discoveryv3.DiscoveryRequest{},
74-
ctx: context.Background(),
75-
wantErr: "missing node ID",
78+
name: "DiscoveryRequest with nil node is rejected",
79+
msg: &discoveryv3.DiscoveryRequest{},
80+
ctx: context.Background(),
81+
wantErr: "missing node ID",
82+
wantCode: codes.InvalidArgument,
7683
},
7784
{
7885
name: "DeltaDiscoveryRequest with empty node ID is rejected",
7986
msg: &discoveryv3.DeltaDiscoveryRequest{
8087
Node: &corev3.Node{Id: ""},
8188
},
82-
ctx: context.Background(),
83-
wantErr: "missing node ID",
89+
ctx: context.Background(),
90+
wantErr: "missing node ID",
91+
wantCode: codes.InvalidArgument,
8492
},
8593
{
8694
name: "DiscoveryRequest with empty node ID is rejected",
8795
msg: &discoveryv3.DiscoveryRequest{
8896
Node: &corev3.Node{Id: ""},
8997
},
90-
ctx: context.Background(),
91-
wantErr: "missing node ID",
98+
ctx: context.Background(),
99+
wantErr: "missing node ID",
100+
wantCode: codes.InvalidArgument,
92101
},
93102
{
94103
name: "DeltaDiscoveryRequest without metadata is rejected",
95104
msg: &discoveryv3.DeltaDiscoveryRequest{
96105
Node: &corev3.Node{Id: "pod-1"},
97106
},
98-
ctx: context.Background(),
99-
wantErr: "missing metadata",
107+
ctx: context.Background(),
108+
wantErr: "missing metadata",
109+
wantCode: codes.Unauthenticated,
100110
},
101111
{
102112
name: "DiscoveryRequest without metadata is rejected",
103113
msg: &discoveryv3.DiscoveryRequest{
104114
Node: &corev3.Node{Id: "pod-1"},
105115
},
106-
ctx: context.Background(),
107-
wantErr: "missing metadata",
116+
ctx: context.Background(),
117+
wantErr: "missing metadata",
118+
wantCode: codes.Unauthenticated,
108119
},
109120
{
110121
name: "DeltaDiscoveryRequest without auth header is rejected",
111122
msg: &discoveryv3.DeltaDiscoveryRequest{
112123
Node: &corev3.Node{Id: "pod-1"},
113124
},
114-
ctx: metadata.NewIncomingContext(context.Background(), metadata.MD{}),
115-
wantErr: "missing authorization token",
125+
ctx: metadata.NewIncomingContext(context.Background(), metadata.MD{}),
126+
wantErr: "missing authorization header",
127+
wantCode: codes.Unauthenticated,
116128
},
117129
{
118130
name: "DiscoveryRequest without auth header is rejected",
119131
msg: &discoveryv3.DiscoveryRequest{
120132
Node: &corev3.Node{Id: "pod-1"},
121133
},
122-
ctx: metadata.NewIncomingContext(context.Background(), metadata.MD{}),
123-
wantErr: "missing authorization token",
134+
ctx: metadata.NewIncomingContext(context.Background(), metadata.MD{}),
135+
wantErr: "missing authorization header",
136+
wantCode: codes.Unauthenticated,
124137
},
125138
}
126139
}
@@ -141,6 +154,7 @@ func TestAuthenticate_Stream(t *testing.T) {
141154
err := ws.RecvMsg(tt.msg)
142155
require.Error(t, err)
143156
assert.Contains(t, err.Error(), tt.wantErr)
157+
assert.Equal(t, tt.wantCode, status.Code(err), "unexpected gRPC status code")
144158
})
145159
}
146160
}
@@ -157,6 +171,7 @@ func TestAuthenticate_Unary(t *testing.T) {
157171
_, err := unary(tt.ctx, tt.msg, &grpc.UnaryServerInfo{}, handler)
158172
require.Error(t, err)
159173
assert.Contains(t, err.Error(), tt.wantErr)
174+
assert.Equal(t, tt.wantCode, status.Code(err), "unexpected gRPC status code")
160175
})
161176
}
162177
}

0 commit comments

Comments
 (0)