Skip to content

Commit 3d893a7

Browse files
boojackclaude
andcommitted
fix(backend): implement protocol-agnostic header setting for dual gRPC/Connect-RPC support
Problem: The codebase supports both native gRPC and Connect-RPC protocols, but auth service was using grpc.SetHeader() which only works for native gRPC. This caused "failed to set grpc header" errors when using Connect-RPC clients (browsers using nice-grpc-web). Solution: - Created HeaderCarrier pattern for protocol-agnostic header setting - HeaderCarrier stores headers in context for Connect-RPC requests - Falls back to grpc.SetHeader for native gRPC requests - Updated auth service to use SetResponseHeader() instead of grpc.SetHeader() - Refactored Connect wrappers to use withHeaderCarrier() helper to eliminate code duplication Additional fixes: - Allow public methods when gRPC metadata is missing in ACL interceptor - Properly handle ParseSessionCookieValue errors instead of ignoring them - Fix buildSessionCookie to gracefully handle missing metadata Files changed: - server/router/api/v1/header_carrier.go: New protocol-agnostic header carrier - server/router/api/v1/auth_service.go: Use SetResponseHeader, handle missing metadata - server/router/api/v1/connect_services.go: Use withHeaderCarrier helper - server/router/api/v1/acl.go: Allow public methods without metadata - server/router/api/v1/connect_interceptors.go: Handle ParseSessionCookieValue errors 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 8a7e008 commit 3d893a7

File tree

5 files changed

+167
-35
lines changed

5 files changed

+167
-35
lines changed

server/router/api/v1/acl.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,14 +53,23 @@ func NewGRPCAuthInterceptor(store *store.Store, secret string) *GRPCAuthIntercep
5353
func (in *GRPCAuthInterceptor) AuthenticationInterceptor(ctx context.Context, request any, serverInfo *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (any, error) {
5454
md, ok := metadata.FromIncomingContext(ctx)
5555
if !ok {
56+
// If metadata is missing, only allow public methods
57+
if IsPublicMethod(serverInfo.FullMethod) {
58+
return handler(ctx, request)
59+
}
5660
return nil, status.Errorf(codes.Unauthenticated, "failed to parse metadata from incoming context")
5761
}
5862

5963
// Try session cookie authentication
6064
if sessionCookie := extractSessionCookieFromMetadata(md); sessionCookie != "" {
6165
user, err := in.authenticator.AuthenticateBySession(ctx, sessionCookie)
6266
if err == nil && user != nil {
63-
_, sessionID, _ := auth.ParseSessionCookieValue(sessionCookie)
67+
_, sessionID, err := auth.ParseSessionCookieValue(sessionCookie)
68+
if err != nil {
69+
// This should not happen since AuthenticateBySession already validated the cookie
70+
// but handle it gracefully anyway
71+
sessionID = ""
72+
}
6473
ctx, err = in.authenticator.AuthorizeAndSetContext(ctx, serverInfo.FullMethod, user, sessionID, "", IsAdminOnlyMethod)
6574
if err != nil {
6675
return nil, toGRPCError(err, codes.PermissionDenied)

server/router/api/v1/auth_service.go

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010

1111
"github.com/pkg/errors"
1212
"golang.org/x/crypto/bcrypt"
13-
"google.golang.org/grpc"
1413
"google.golang.org/grpc/codes"
1514
"google.golang.org/grpc/metadata"
1615
"google.golang.org/grpc/status"
@@ -237,10 +236,8 @@ func (s *APIV1Service) doSignIn(ctx context.Context, user *store.User, expireTim
237236
if err != nil {
238237
return status.Errorf(codes.Internal, "failed to build session cookie, error: %v", err)
239238
}
240-
if err := grpc.SetHeader(ctx, metadata.New(map[string]string{
241-
"Set-Cookie": sessionCookie,
242-
})); err != nil {
243-
return status.Errorf(codes.Internal, "failed to set grpc header, error: %v", err)
239+
if err := SetResponseHeader(ctx, "Set-Cookie", sessionCookie); err != nil {
240+
return status.Errorf(codes.Internal, "failed to set response header, error: %v", err)
244241
}
245242

246243
return nil
@@ -284,11 +281,9 @@ func (s *APIV1Service) clearAuthCookies(ctx context.Context) error {
284281
return errors.Wrap(err, "failed to build session cookie")
285282
}
286283

287-
// Set both cookies in the response
288-
if err := grpc.SetHeader(ctx, metadata.New(map[string]string{
289-
"Set-Cookie": sessionCookie,
290-
})); err != nil {
291-
return errors.Wrap(err, "failed to set grpc header")
284+
// Set cookie in the response
285+
if err := SetResponseHeader(ctx, "Set-Cookie", sessionCookie); err != nil {
286+
return errors.Wrap(err, "failed to set response header")
292287
}
293288
return nil
294289
}
@@ -305,15 +300,18 @@ func (*APIV1Service) buildSessionCookie(ctx context.Context, sessionCookieValue
305300
attrs = append(attrs, "Expires="+expireTime.Format(time.RFC1123))
306301
}
307302

308-
md, ok := metadata.FromIncomingContext(ctx)
309-
if !ok {
310-
return "", errors.New("failed to get metadata from context")
311-
}
312-
var origin string
313-
for _, v := range md.Get("origin") {
314-
origin = v
303+
// Try to determine if the request is HTTPS by checking the origin header
304+
// Default to non-HTTPS (Strict SameSite) if metadata is not available
305+
isHTTPS := false
306+
if md, ok := metadata.FromIncomingContext(ctx); ok {
307+
for _, v := range md.Get("origin") {
308+
if strings.HasPrefix(v, "https://") {
309+
isHTTPS = true
310+
break
311+
}
312+
}
315313
}
316-
isHTTPS := strings.HasPrefix(origin, "https://")
314+
317315
if isHTTPS {
318316
attrs = append(attrs, "SameSite=None")
319317
attrs = append(attrs, "Secure")

server/router/api/v1/connect_interceptors.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,12 @@ func (in *AuthInterceptor) WrapUnary(next connect.UnaryFunc) connect.UnaryFunc {
150150
if sessionCookie := auth.ExtractSessionCookieFromHeader(header.Get("Cookie")); sessionCookie != "" {
151151
user, err := in.authenticator.AuthenticateBySession(ctx, sessionCookie)
152152
if err == nil && user != nil {
153-
_, sessionID, _ := auth.ParseSessionCookieValue(sessionCookie)
153+
_, sessionID, err := auth.ParseSessionCookieValue(sessionCookie)
154+
if err != nil {
155+
// This should not happen since AuthenticateBySession already validated the cookie
156+
// but handle it gracefully anyway
157+
sessionID = ""
158+
}
154159
ctx, err = in.authenticator.AuthorizeAndSetContext(ctx, procedure, user, sessionID, "", IsAdminOnlyMethod)
155160
if err != nil {
156161
return nil, convertAuthError(err)

server/router/api/v1/connect_services.go

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -40,29 +40,27 @@ func (s *ConnectServiceHandler) UpdateInstanceSetting(ctx context.Context, req *
4040
}
4141

4242
// AuthService
43+
//
44+
// Auth service methods need special handling for response headers (cookies).
45+
// We use withHeaderCarrier helper to inject a header carrier into the context,
46+
// which allows the service to set headers in a protocol-agnostic way.
4347

4448
func (s *ConnectServiceHandler) GetCurrentSession(ctx context.Context, req *connect.Request[v1pb.GetCurrentSessionRequest]) (*connect.Response[v1pb.GetCurrentSessionResponse], error) {
45-
resp, err := s.APIV1Service.GetCurrentSession(ctx, req.Msg)
46-
if err != nil {
47-
return nil, convertGRPCError(err)
48-
}
49-
return connect.NewResponse(resp), nil
49+
return withHeaderCarrier(ctx, func(ctx context.Context) (*v1pb.GetCurrentSessionResponse, error) {
50+
return s.APIV1Service.GetCurrentSession(ctx, req.Msg)
51+
})
5052
}
5153

5254
func (s *ConnectServiceHandler) CreateSession(ctx context.Context, req *connect.Request[v1pb.CreateSessionRequest]) (*connect.Response[v1pb.CreateSessionResponse], error) {
53-
resp, err := s.APIV1Service.CreateSession(ctx, req.Msg)
54-
if err != nil {
55-
return nil, convertGRPCError(err)
56-
}
57-
return connect.NewResponse(resp), nil
55+
return withHeaderCarrier(ctx, func(ctx context.Context) (*v1pb.CreateSessionResponse, error) {
56+
return s.APIV1Service.CreateSession(ctx, req.Msg)
57+
})
5858
}
5959

6060
func (s *ConnectServiceHandler) DeleteSession(ctx context.Context, req *connect.Request[v1pb.DeleteSessionRequest]) (*connect.Response[emptypb.Empty], error) {
61-
resp, err := s.APIV1Service.DeleteSession(ctx, req.Msg)
62-
if err != nil {
63-
return nil, convertGRPCError(err)
64-
}
65-
return connect.NewResponse(resp), nil
61+
return withHeaderCarrier(ctx, func(ctx context.Context) (*emptypb.Empty, error) {
62+
return s.APIV1Service.DeleteSession(ctx, req.Msg)
63+
})
6664
}
6765

6866
// UserService
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
package v1
2+
3+
import (
4+
"context"
5+
6+
"connectrpc.com/connect"
7+
"google.golang.org/grpc"
8+
"google.golang.org/grpc/metadata"
9+
"google.golang.org/protobuf/proto"
10+
)
11+
12+
// headerCarrierKey is the context key for storing headers to be set in the response.
13+
type headerCarrierKey struct{}
14+
15+
// HeaderCarrier stores headers that need to be set in the response.
16+
//
17+
// Problem: The codebase supports two protocols simultaneously:
18+
// - Native gRPC: Uses grpc.SetHeader() to set response headers
19+
// - Connect-RPC: Uses connect.Response.Header().Set() to set response headers
20+
//
21+
// Solution: HeaderCarrier provides a protocol-agnostic way to set headers.
22+
// - Service methods call SetResponseHeader() regardless of protocol
23+
// - For gRPC requests: SetResponseHeader uses grpc.SetHeader directly
24+
// - For Connect requests: SetResponseHeader stores headers in HeaderCarrier
25+
// - Connect wrappers extract headers from HeaderCarrier and apply to response
26+
//
27+
// This allows service methods to work with both protocols without knowing which one is being used.
28+
type HeaderCarrier struct {
29+
headers map[string]string
30+
}
31+
32+
// newHeaderCarrier creates a new header carrier.
33+
func newHeaderCarrier() *HeaderCarrier {
34+
return &HeaderCarrier{
35+
headers: make(map[string]string),
36+
}
37+
}
38+
39+
// Set adds a header to the carrier.
40+
func (h *HeaderCarrier) Set(key, value string) {
41+
h.headers[key] = value
42+
}
43+
44+
// Get retrieves a header from the carrier.
45+
func (h *HeaderCarrier) Get(key string) string {
46+
return h.headers[key]
47+
}
48+
49+
// All returns all headers.
50+
func (h *HeaderCarrier) All() map[string]string {
51+
return h.headers
52+
}
53+
54+
// WithHeaderCarrier adds a header carrier to the context.
55+
func WithHeaderCarrier(ctx context.Context) context.Context {
56+
return context.WithValue(ctx, headerCarrierKey{}, newHeaderCarrier())
57+
}
58+
59+
// GetHeaderCarrier retrieves the header carrier from the context.
60+
// Returns nil if no carrier is present.
61+
func GetHeaderCarrier(ctx context.Context) *HeaderCarrier {
62+
if carrier, ok := ctx.Value(headerCarrierKey{}).(*HeaderCarrier); ok {
63+
return carrier
64+
}
65+
return nil
66+
}
67+
68+
// SetResponseHeader sets a header in the response.
69+
//
70+
// This function works for both gRPC and Connect protocols:
71+
// - For gRPC: Uses grpc.SetHeader to set headers in gRPC metadata
72+
// - For Connect: Stores in HeaderCarrier for Connect wrapper to apply later
73+
//
74+
// The protocol is automatically detected based on whether a HeaderCarrier
75+
// exists in the context (injected by Connect wrappers).
76+
func SetResponseHeader(ctx context.Context, key, value string) error {
77+
// Try Connect first (check if we have a header carrier)
78+
if carrier := GetHeaderCarrier(ctx); carrier != nil {
79+
carrier.Set(key, value)
80+
return nil
81+
}
82+
83+
// Fall back to gRPC
84+
return grpc.SetHeader(ctx, metadata.New(map[string]string{
85+
key: value,
86+
}))
87+
}
88+
89+
// withHeaderCarrier is a helper for Connect service wrappers that need to set response headers.
90+
//
91+
// It injects a HeaderCarrier into the context, calls the service method,
92+
// and applies any headers from the carrier to the Connect response.
93+
//
94+
// Usage in Connect wrappers:
95+
//
96+
// func (s *ConnectServiceHandler) CreateSession(ctx context.Context, req *connect.Request[...]) (*connect.Response[...], error) {
97+
// return withHeaderCarrier(ctx, func(ctx context.Context) (*v1pb.CreateSessionResponse, error) {
98+
// return s.APIV1Service.CreateSession(ctx, req.Msg)
99+
// })
100+
// }
101+
func withHeaderCarrier[T proto.Message](ctx context.Context, fn func(context.Context) (T, error)) (*connect.Response[T], error) {
102+
// Inject header carrier for Connect protocol
103+
ctx = WithHeaderCarrier(ctx)
104+
105+
// Call the service method
106+
resp, err := fn(ctx)
107+
if err != nil {
108+
return nil, convertGRPCError(err)
109+
}
110+
111+
// Create Connect response
112+
connectResp := connect.NewResponse(resp)
113+
114+
// Apply any headers set via the header carrier
115+
if carrier := GetHeaderCarrier(ctx); carrier != nil {
116+
for key, value := range carrier.All() {
117+
connectResp.Header().Set(key, value)
118+
}
119+
}
120+
121+
return connectResp, nil
122+
}

0 commit comments

Comments
 (0)