From cc0c06f225e21911e935111af06a67a67a010a71 Mon Sep 17 00:00:00 2001 From: Livio Spring Date: Tue, 23 Apr 2024 10:35:25 +0200 Subject: [PATCH] fix: exclude db connection error details (#7785) * fix: exclude db connection error details * remove potential recursive error --- internal/api/assets/asset.go | 28 ++++++++++++++----- internal/api/grpc/gerrors/zitadel_errors.go | 5 ++++ .../server/middleware/instance_interceptor.go | 7 +++-- .../http/middleware/instance_interceptor.go | 9 +++--- 4 files changed, 35 insertions(+), 14 deletions(-) diff --git a/internal/api/assets/asset.go b/internal/api/assets/asset.go index 5836d66e0b..5f30c94a29 100644 --- a/internal/api/assets/asset.go +++ b/internal/api/assets/asset.go @@ -2,6 +2,7 @@ package assets import ( "context" + "errors" "fmt" "io" "net/http" @@ -12,14 +13,17 @@ import ( "github.com/gabriel-vasile/mimetype" "github.com/gorilla/mux" "github.com/zitadel/logging" + "golang.org/x/text/language" "github.com/zitadel/zitadel/internal/api/authz" http_util "github.com/zitadel/zitadel/internal/api/http" http_mw "github.com/zitadel/zitadel/internal/api/http/middleware" "github.com/zitadel/zitadel/internal/command" + "github.com/zitadel/zitadel/internal/i18n" "github.com/zitadel/zitadel/internal/id" "github.com/zitadel/zitadel/internal/query" "github.com/zitadel/zitadel/internal/static" + "github.com/zitadel/zitadel/internal/zerrors" ) const ( @@ -73,19 +77,29 @@ type Downloader interface { type ErrorHandler func(w http.ResponseWriter, r *http.Request, err error, defaultCode int) -func DefaultErrorHandler(w http.ResponseWriter, r *http.Request, err error, defaultCode int) { - logging.WithFields("uri", r.RequestURI).WithError(err).Warn("error occurred on asset api") - code, ok := http_util.ZitadelErrorToHTTPStatusCode(err) - if !ok { - code = defaultCode +func DefaultErrorHandler(translator *i18n.Translator) func(w http.ResponseWriter, r *http.Request, err error, defaultCode int) { + return func(w http.ResponseWriter, r *http.Request, err error, defaultCode int) { + logging.WithFields("uri", r.RequestURI).WithError(err).Warn("error occurred on asset api") + code, ok := http_util.ZitadelErrorToHTTPStatusCode(err) + if !ok { + code = defaultCode + } + zErr := new(zerrors.ZitadelError) + if errors.As(err, &zErr) { + zErr.SetMessage(translator.LocalizeFromCtx(r.Context(), zErr.GetMessage(), nil)) + zErr.Parent = nil // ensuring we don't leak any unwanted information + err = zErr + } + http.Error(w, err.Error(), code) } - http.Error(w, err.Error(), code) } func NewHandler(commands *command.Commands, verifier authz.APITokenVerifier, authConfig authz.Config, idGenerator id.Generator, storage static.Storage, queries *query.Queries, callDurationInterceptor, instanceInterceptor, assetCacheInterceptor, accessInterceptor func(handler http.Handler) http.Handler) http.Handler { + translator, err := i18n.NewZitadelTranslator(language.English) + logging.OnError(err).Panic("unable to get translator") h := &Handler{ commands: commands, - errorHandler: DefaultErrorHandler, + errorHandler: DefaultErrorHandler(translator), authInterceptor: http_mw.AuthorizationInterceptor(verifier, authConfig), idGenerator: idGenerator, storage: storage, diff --git a/internal/api/grpc/gerrors/zitadel_errors.go b/internal/api/grpc/gerrors/zitadel_errors.go index 60e8473898..8e131c2d35 100644 --- a/internal/api/grpc/gerrors/zitadel_errors.go +++ b/internal/api/grpc/gerrors/zitadel_errors.go @@ -3,6 +3,7 @@ package gerrors import ( "errors" + "github.com/jackc/pgx/v5/pgconn" "github.com/zitadel/logging" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -35,6 +36,10 @@ func ExtractZITADELError(err error) (c codes.Code, msg, id string, ok bool) { if err == nil { return codes.OK, "", "", false } + connErr := new(pgconn.ConnectError) + if ok := errors.As(err, &connErr); ok { + return codes.Internal, "db connection error", "", true + } zitadelErr := new(zerrors.ZitadelError) if ok := errors.As(err, &zitadelErr); !ok { return codes.Unknown, err.Error(), "", false diff --git a/internal/api/grpc/server/middleware/instance_interceptor.go b/internal/api/grpc/server/middleware/instance_interceptor.go index f5965beb35..ba637d59fa 100644 --- a/internal/api/grpc/server/middleware/instance_interceptor.go +++ b/internal/api/grpc/server/middleware/instance_interceptor.go @@ -62,14 +62,15 @@ func setInstance(ctx context.Context, req interface{}, info *grpc.UnaryServerInf } instance, err := verifier.InstanceByHost(interceptorCtx, host) if err != nil { - err = fmt.Errorf("unable to set instance using origin %s (ExternalDomain is %s): %w", zitadel_http.ComposedOrigin(ctx), externalDomain, err) + origin := zitadel_http.ComposedOrigin(ctx) + logging.WithFields("origin", origin, "externalDomain", externalDomain).WithError(err).Error("unable to set instance") zErr := new(zerrors.ZitadelError) if errors.As(err, &zErr) { zErr.SetMessage(translator.LocalizeFromCtx(ctx, zErr.GetMessage(), nil)) zErr.Parent = err - err = zErr + return nil, status.Error(codes.NotFound, fmt.Sprintf("unable to set instance using origin %s (ExternalDomain is %s): %s", origin, externalDomain, zErr)) } - return nil, status.Error(codes.NotFound, err.Error()) + return nil, status.Error(codes.NotFound, fmt.Sprintf("unable to set instance using origin %s (ExternalDomain is %s)", origin, externalDomain)) } span.End() return handler(authz.WithInstance(ctx, instance), req) diff --git a/internal/api/http/middleware/instance_interceptor.go b/internal/api/http/middleware/instance_interceptor.go index 4b315d5a7b..2117b98d30 100644 --- a/internal/api/http/middleware/instance_interceptor.go +++ b/internal/api/http/middleware/instance_interceptor.go @@ -56,14 +56,15 @@ func (a *instanceInterceptor) handleInstance(w http.ResponseWriter, r *http.Requ } ctx, err := setInstance(r, a.verifier, a.headerName) if err != nil { - err = fmt.Errorf("unable to set instance using origin %s (ExternalDomain is %s): %w", zitadel_http.ComposedOrigin(r.Context()), a.externalDomain, err) + origin := zitadel_http.ComposedOrigin(r.Context()) + logging.WithFields("origin", origin, "externalDomain", a.externalDomain).WithError(err).Error("unable to set instance") zErr := new(zerrors.ZitadelError) if errors.As(err, &zErr) { zErr.SetMessage(a.translator.LocalizeFromRequest(r, zErr.GetMessage(), nil)) - zErr.Parent = err - err = zErr + http.Error(w, fmt.Sprintf("unable to set instance using origin %s (ExternalDomain is %s): %s", origin, a.externalDomain, zErr), http.StatusNotFound) + return } - http.Error(w, err.Error(), http.StatusNotFound) + http.Error(w, fmt.Sprintf("unable to set instance using origin %s (ExternalDomain is %s)", origin, a.externalDomain), http.StatusNotFound) return } r = r.WithContext(ctx)