diff --git a/internal/cmd/auth.go b/internal/cmd/auth.go index b834b06b88..63c6e323aa 100644 --- a/internal/cmd/auth.go +++ b/internal/cmd/auth.go @@ -137,6 +137,7 @@ func registerFunc(ctx context.Context, conn *grpc.ClientConn, fn func(context.Co func authenticationHTTPMount( ctx context.Context, + logger *zap.Logger, cfg config.AuthenticationConfig, r chi.Router, conn *grpc.ClientConn, @@ -172,6 +173,6 @@ func authenticationHTTPMount( r.Group(func(r chi.Router) { r.Use(middleware...) - r.Mount("/auth/v1", gateway.NewGatewayServeMux(muxOpts...)) + r.Mount("/auth/v1", gateway.NewGatewayServeMux(logger, muxOpts...)) }) } diff --git a/internal/cmd/http.go b/internal/cmd/http.go index 81beffbbb9..c60c76df88 100644 --- a/internal/cmd/http.go +++ b/internal/cmd/http.go @@ -55,7 +55,7 @@ func NewHTTPServer( isConsole = cfg.Log.Encoding == config.LogEncodingConsole r = chi.NewRouter() - api = gateway.NewGatewayServeMux() + api = gateway.NewGatewayServeMux(logger) httpPort = cfg.Server.HTTPPort ) @@ -130,7 +130,7 @@ func NewHTTPServer( // mount all authentication related HTTP components // to the chi router. - authenticationHTTPMount(ctx, cfg.Authentication, r, conn) + authenticationHTTPMount(ctx, logger, cfg.Authentication, r, conn) r.Group(func(r chi.Router) { r.Use(func(handler http.Handler) http.Handler { diff --git a/internal/gateway/gateway.go b/internal/gateway/gateway.go index cbbc7bcecf..c25b820c5d 100644 --- a/internal/gateway/gateway.go +++ b/internal/gateway/gateway.go @@ -1,8 +1,11 @@ package gateway import ( + "sync" + "github.com/grpc-ecosystem/grpc-gateway/v2/runtime" "go.flipt.io/flipt/rpc/flipt" + "go.uber.org/zap" "google.golang.org/protobuf/encoding/protojson" ) @@ -13,20 +16,26 @@ import ( // See: rpc/flipt/marshal.go // // See: https://github.com/flipt-io/flipt/issues/664 -var commonMuxOptions = []runtime.ServeMuxOption{ - runtime.WithMarshalerOption(runtime.MIMEWildcard, flipt.NewV1toV2MarshallerAdapter()), - runtime.WithMarshalerOption("application/json+pretty", &runtime.JSONPb{ - MarshalOptions: protojson.MarshalOptions{ - Indent: " ", - Multiline: true, // Optional, implied by presence of "Indent". - }, - UnmarshalOptions: protojson.UnmarshalOptions{ - DiscardUnknown: true, - }, - }), -} +var commonMuxOptions []runtime.ServeMuxOption +var once sync.Once // NewGatewayServeMux builds a new gateway serve mux with common options. -func NewGatewayServeMux(opts ...runtime.ServeMuxOption) *runtime.ServeMux { +func NewGatewayServeMux(logger *zap.Logger, opts ...runtime.ServeMuxOption) *runtime.ServeMux { + once.Do(func() { + commonMuxOptions = []runtime.ServeMuxOption{ + runtime.WithMarshalerOption(runtime.MIMEWildcard, flipt.NewV1toV2MarshallerAdapter(logger)), + runtime.WithMarshalerOption("application/json+pretty", &runtime.JSONPb{ + MarshalOptions: protojson.MarshalOptions{ + Indent: " ", + Multiline: true, // Optional, implied by presence of "Indent". + }, + UnmarshalOptions: protojson.UnmarshalOptions{ + DiscardUnknown: true, + }, + }), + } + + }) + return runtime.NewServeMux(append(commonMuxOptions, opts...)...) } diff --git a/internal/server/auth/method/kubernetes/testing/http.go b/internal/server/auth/method/kubernetes/testing/http.go index 3737a3e03c..1403a794ad 100644 --- a/internal/server/auth/method/kubernetes/testing/http.go +++ b/internal/server/auth/method/kubernetes/testing/http.go @@ -26,7 +26,7 @@ func StartHTTPServer( t.Helper() var ( - mux = gateway.NewGatewayServeMux() + mux = gateway.NewGatewayServeMux(logger) httpServer = &HTTPServer{ GRPCServer: StartGRPCServer(t, ctx, logger, conf), } diff --git a/internal/server/auth/method/oidc/testing/http.go b/internal/server/auth/method/oidc/testing/http.go index 1cdaa0048f..ab4ae20c39 100644 --- a/internal/server/auth/method/oidc/testing/http.go +++ b/internal/server/auth/method/oidc/testing/http.go @@ -34,6 +34,7 @@ func StartHTTPServer( oidcmiddleware = oidc.NewHTTPMiddleware(conf.Session) mux = gateway.NewGatewayServeMux( + logger, runtime.WithMetadata(oidc.ForwardCookies), runtime.WithForwardResponseOption(oidcmiddleware.ForwardResponseOption), ) diff --git a/rpc/flipt/marshaller.go b/rpc/flipt/marshaller.go index 391a83548c..1086411bbe 100644 --- a/rpc/flipt/marshaller.go +++ b/rpc/flipt/marshaller.go @@ -1,10 +1,13 @@ package flipt import ( + "encoding/json" + "errors" "io" grpc_gateway_v1 "github.com/grpc-ecosystem/grpc-gateway/runtime" grpc_gateway_v2 "github.com/grpc-ecosystem/grpc-gateway/v2/runtime" + "go.uber.org/zap" ) var _ grpc_gateway_v2.Marshaler = &V1toV2MarshallerAdapter{} @@ -20,10 +23,11 @@ var _ grpc_gateway_v2.Marshaler = &V1toV2MarshallerAdapter{} // TODO: remove this custom marshaller for Flipt API v2 as we want to use the default v2 marshaller directly. type V1toV2MarshallerAdapter struct { *grpc_gateway_v1.JSONPb + logger *zap.Logger } -func NewV1toV2MarshallerAdapter() *V1toV2MarshallerAdapter { - return &V1toV2MarshallerAdapter{&grpc_gateway_v1.JSONPb{OrigName: false, EmitDefaults: true}} +func NewV1toV2MarshallerAdapter(logger *zap.Logger) *V1toV2MarshallerAdapter { + return &V1toV2MarshallerAdapter{&grpc_gateway_v1.JSONPb{OrigName: false, EmitDefaults: true}, logger} } func (m *V1toV2MarshallerAdapter) ContentType(_ interface{}) string { @@ -34,8 +38,30 @@ func (m *V1toV2MarshallerAdapter) Marshal(v interface{}) ([]byte, error) { return m.JSONPb.Marshal(v) } +// decoderInterceptor intercepts and modifies the outbound error return value for +// inputs that fail to unmarshal against the protobuf. +type decoderInterceptor struct { + grpc_gateway_v1.Decoder + logger *zap.Logger +} + +func (c *decoderInterceptor) Decode(v interface{}) error { + err := c.Decoder.Decode(v) + if err != nil { + c.logger.Debug("JSON decoding failed for inputs", zap.Error(err)) + + if _, ok := err.(*json.UnmarshalTypeError); ok { + return errors.New("invalid values for key(s) in json body") + } + + return err + } + + return nil +} + func (m *V1toV2MarshallerAdapter) NewDecoder(r io.Reader) grpc_gateway_v2.Decoder { - return m.JSONPb.NewDecoder(r) + return &decoderInterceptor{Decoder: m.JSONPb.NewDecoder(r), logger: m.logger} } func (m *V1toV2MarshallerAdapter) NewEncoder(w io.Writer) grpc_gateway_v2.Encoder { diff --git a/test/api.sh b/test/api.sh index 49ce4d3aaa..bd6b07c4b5 100755 --- a/test/api.sh +++ b/test/api.sh @@ -263,6 +263,12 @@ step_5_test_evaluation() status 200 matches "\"flagKey\":\"$flag_key\"" matches "\"match\":false" + + # evaluate returns 400 plus user friendly error message + authedShakedown POST "/api/v1/evaluate" -H 'Content-Type:application/json' -d "{\"flag_key\":\"$flag_key\",\"entity_id\":\"$(uuid_str)\",\"context\":\"hello\"}" + status 400 + matches "\"code\":3" + contains "\"message\":\"invalid values for key(s) in json body\"" } step_6_test_batch_evaluation()