diff --git a/backend/v3/instrumentation/slog.go b/backend/v3/instrumentation/slog.go index c4847908b..51c73123e 100644 --- a/backend/v3/instrumentation/slog.go +++ b/backend/v3/instrumentation/slog.go @@ -2,6 +2,7 @@ package instrumentation import ( "context" + "errors" "log/slog" "os" "slices" @@ -159,6 +160,7 @@ func setLogger(provider *log.LoggerProvider, cfg LogConfig) { Prependers: []slogctx.AttrExtractor{ instanceExtractor, requestIDExtractor, + causeExtractor, slogotel.ExtractTraceSpanID, slogctx.ExtractPrepended, }, @@ -224,6 +226,17 @@ func requestIDExtractor(ctx context.Context, _ time.Time, _ slog.Level, _ string return nil } +// causeExtractor sets the cause of a canceled context to a log entry. +func causeExtractor(ctx context.Context, _ time.Time, _ slog.Level, _ string) []slog.Attr { + err := context.Cause(ctx) + if err != nil && !errors.Is(err, context.Canceled) && !errors.Is(err, context.DeadlineExceeded) { + return []slog.Attr{ + slog.String("cause", err.Error()), + } + } + return nil +} + type replacer func(groups []string, a slog.Attr) slog.Attr func chainReplacers(replacers ...replacer) replacer { diff --git a/backend/v3/instrumentation/slog_test.go b/backend/v3/instrumentation/slog_test.go index 077fa8279..9f86a5fe5 100644 --- a/backend/v3/instrumentation/slog_test.go +++ b/backend/v3/instrumentation/slog_test.go @@ -1,8 +1,11 @@ package instrumentation import ( + "context" + "errors" "log/slog" "testing" + "time" "github.com/stretchr/testify/assert" old_logging "github.com/zitadel/logging" //nolint:staticcheck @@ -164,3 +167,48 @@ func TestLogConfig_replacer(t *testing.T) { }) } } + +func Test_causeExtractor(t *testing.T) { + background := context.Background() + canceled, cancel := context.WithCancel(background) + cancel() + timedOut, cancel := context.WithTimeout(background, -1) + defer cancel() + canceledCause, cause := context.WithCancelCause(background) + cause(errors.New("oops")) + + tests := []struct { + name string // description of this test case + ctx context.Context + want []slog.Attr + }{ + { + name: "valid background, no attributes", + ctx: background, + want: nil, + }, + { + name: "regular canceled context", + ctx: canceled, + want: nil, + }, + { + name: "regular timed out context", + ctx: timedOut, + want: nil, + }, + { + name: "canceled context with cause", + ctx: canceledCause, + want: []slog.Attr{ + slog.String("cause", "oops"), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := causeExtractor(tt.ctx, time.Time{}, 0, "") + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/cmd/start/start.go b/cmd/start/start.go index bb06ac8a1..e6d25ac8b 100644 --- a/cmd/start/start.go +++ b/cmd/start/start.go @@ -434,8 +434,11 @@ func startAPIs( queries, } oidcPrefixes := []string{"/.well-known/openid-configuration", "/oidc/v1", "/oauth/v2"} - // always set the origin in the context if available in the http headers, no matter for what protocol - router.Use(middleware.WithOrigin(config.ExternalSecure, config.HTTP1HostHeader, config.HTTP2HostHeader, config.InstanceHostHeaders, config.PublicHostHeaders)) + router.Use( + middleware.FallbackRecoverHandler(), + // always set the origin in the context if available in the http headers, no matter for what protocol + middleware.WithOrigin(config.ExternalSecure, config.HTTP1HostHeader, config.HTTP2HostHeader, config.InstanceHostHeaders, config.PublicHostHeaders), + ) systemTokenVerifier, err := internal_authz.StartSystemTokenVerifierFromConfig(http_util.BuildHTTP(config.ExternalDomain, config.ExternalPort, config.ExternalSecure), config.SystemAPIUsers) if err != nil { return nil, err diff --git a/internal/api/grpc/gerrors/zitadel_errors.go b/internal/api/grpc/gerrors/zitadel_errors.go index 3a9c6d7a2..d0bddf17b 100644 --- a/internal/api/grpc/gerrors/zitadel_errors.go +++ b/internal/api/grpc/gerrors/zitadel_errors.go @@ -7,6 +7,7 @@ import ( "connectrpc.com/connect" "github.com/jackc/pgx/v5/pgconn" + "github.com/zitadel/sloggcp" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "google.golang.org/protobuf/proto" @@ -119,6 +120,9 @@ func extractError(err error) (c codes.Code, msg, id string, lvl slog.Level) { default: c, lvl = codes.Unknown, slog.LevelError } + if id == zerrors.IDRecover { + lvl = sloggcp.LevelAlert + } return c, msg, id, lvl } diff --git a/internal/api/grpc/server/connect_middleware/error_interceptor.go b/internal/api/grpc/server/connect_middleware/error_interceptor.go index 43d61c2d1..fe5fe983d 100644 --- a/internal/api/grpc/server/connect_middleware/error_interceptor.go +++ b/internal/api/grpc/server/connect_middleware/error_interceptor.go @@ -2,11 +2,13 @@ package connect_middleware import ( "context" + "fmt" "connectrpc.com/connect" "github.com/zitadel/zitadel/internal/api/grpc/gerrors" _ "github.com/zitadel/zitadel/internal/statik" + "github.com/zitadel/zitadel/internal/zerrors" ) func ErrorHandler() connect.UnaryInterceptorFunc { @@ -17,7 +19,21 @@ func ErrorHandler() connect.UnaryInterceptorFunc { } } -func toConnectError(ctx context.Context, req connect.AnyRequest, handler connect.UnaryFunc) (connect.AnyResponse, error) { - resp, err := handler(ctx, req) - return resp, gerrors.ZITADELToConnectError(ctx, err) +func toConnectError(ctx context.Context, req connect.AnyRequest, handler connect.UnaryFunc) (_ connect.AnyResponse, err error) { + ctx, cancel := context.WithCancelCause(ctx) + defer func() { + if rec := recover(); rec != nil { + recErr, ok := rec.(error) + if !ok { + recErr = fmt.Errorf("%v", rec) + } + if recErr != nil { + err = zerrors.ThrowInternal(recErr, zerrors.IDRecover, "Errors.Internal") + } + } + cause := err // avoid passing the transport error as cancel cause. + err = gerrors.ZITADELToConnectError(ctx, err) + cancel(cause) + }() + return handler(ctx, req) } diff --git a/internal/api/grpc/server/connect_middleware/error_interceptor_test.go b/internal/api/grpc/server/connect_middleware/error_interceptor_test.go index 954f2fd58..28a8b259e 100644 --- a/internal/api/grpc/server/connect_middleware/error_interceptor_test.go +++ b/internal/api/grpc/server/connect_middleware/error_interceptor_test.go @@ -2,64 +2,80 @@ package connect_middleware import ( "context" - "reflect" + "errors" "testing" "connectrpc.com/connect" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/zitadel/zitadel/internal/api/authz" ) -func Test_toGRPCError(t *testing.T) { +func Test_toConnectError(t *testing.T) { type args struct { ctx context.Context req connect.AnyRequest handler func(t *testing.T) connect.UnaryFunc } - type res struct { - want interface{} - wantErr bool - } tests := []struct { - name string - args args - res res + name string + args args + want any + wantCode connect.Code }{ { - "no error", - args{ + name: "no error", + args: args{ ctx: context.Background(), req: &mockReq[struct{}]{}, handler: emptyMockHandler(&connect.Response[struct{}]{}, authz.CtxData{}), }, - res{ - &connect.Response[struct{}]{}, - false, - }, + want: &connect.Response[struct{}]{}, }, { - "error", - args{ + name: "error", + args: args{ ctx: context.Background(), req: &mockReq[struct{}]{}, handler: errorMockHandler(), }, - res{ - nil, - true, + want: nil, + wantCode: connect.CodeFailedPrecondition, + }, + { + name: "panic with string", + args: args{ + ctx: context.Background(), + req: &mockReq[struct{}]{}, + handler: panicMockHandler("test panic"), }, + want: nil, + wantCode: connect.CodeInternal, + }, + { + name: "panic with error", + args: args{ + ctx: context.Background(), + req: &mockReq[struct{}]{}, + handler: panicMockHandler(errors.New("oops")), + }, + want: nil, + wantCode: connect.CodeInternal, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got, err := toConnectError(tt.args.ctx, tt.args.req, tt.args.handler(t)) - if (err != nil) != tt.res.wantErr { - t.Errorf("toGRPCError() error = %v, wantErr %v", err, tt.res.wantErr) - return - } - if !reflect.DeepEqual(got, tt.res.want) { - t.Errorf("toGRPCError() got = %v, want %v", got, tt.res.want) + if tt.wantCode != 0 { + var connectErr *connect.Error + require.ErrorAs(t, err, &connectErr) + assert.Equal(t, tt.wantCode, connectErr.Code()) + } else { + require.NoError(t, err) } + assert.Equal(t, tt.want, got) }) } } diff --git a/internal/api/grpc/server/connect_middleware/mock_test.go b/internal/api/grpc/server/connect_middleware/mock_test.go index abd996b01..a691129fb 100644 --- a/internal/api/grpc/server/connect_middleware/mock_test.go +++ b/internal/api/grpc/server/connect_middleware/mock_test.go @@ -24,7 +24,15 @@ func emptyMockHandler(resp connect.AnyResponse, expectedCtxData authz.CtxData) f func errorMockHandler() func(*testing.T) connect.UnaryFunc { return func(t *testing.T) connect.UnaryFunc { return func(ctx context.Context, req connect.AnyRequest) (connect.AnyResponse, error) { - return nil, zerrors.ThrowInternal(nil, "test", "error") + return nil, zerrors.ThrowPreconditionFailed(nil, "test", "error") + } + } +} + +func panicMockHandler(payload any) func(*testing.T) connect.UnaryFunc { + return func(t *testing.T) connect.UnaryFunc { + return func(ctx context.Context, req connect.AnyRequest) (connect.AnyResponse, error) { + panic(payload) } } } diff --git a/internal/api/grpc/server/middleware/error_interceptor.go b/internal/api/grpc/server/middleware/error_interceptor.go index 9c4c86526..70b050319 100644 --- a/internal/api/grpc/server/middleware/error_interceptor.go +++ b/internal/api/grpc/server/middleware/error_interceptor.go @@ -2,11 +2,13 @@ package middleware import ( "context" + "fmt" "google.golang.org/grpc" "github.com/zitadel/zitadel/internal/api/grpc/gerrors" _ "github.com/zitadel/zitadel/internal/statik" + "github.com/zitadel/zitadel/internal/zerrors" ) func ErrorHandler() grpc.UnaryServerInterceptor { @@ -15,7 +17,21 @@ func ErrorHandler() grpc.UnaryServerInterceptor { } } -func toGRPCError(ctx context.Context, req interface{}, handler grpc.UnaryHandler) (interface{}, error) { - resp, err := handler(ctx, req) - return resp, gerrors.ZITADELToGRPCError(ctx, err) +func toGRPCError(ctx context.Context, req interface{}, handler grpc.UnaryHandler) (_ interface{}, err error) { + ctx, cancel := context.WithCancelCause(ctx) + defer func() { + if rec := recover(); rec != nil { + recErr, ok := rec.(error) + if !ok { + recErr = fmt.Errorf("%v", rec) + } + if recErr != nil { + err = zerrors.ThrowInternal(recErr, zerrors.IDRecover, "Errors.Internal") + } + } + cause := err // avoid passing the transport error as cancel cause. + err = gerrors.ZITADELToGRPCError(ctx, err) + cancel(cause) + }() + return handler(ctx, req) } diff --git a/internal/api/grpc/server/middleware/error_interceptor_test.go b/internal/api/grpc/server/middleware/error_interceptor_test.go index 82dc35f64..04fc8e35f 100644 --- a/internal/api/grpc/server/middleware/error_interceptor_test.go +++ b/internal/api/grpc/server/middleware/error_interceptor_test.go @@ -2,62 +2,79 @@ package middleware import ( "context" - "reflect" + "errors" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) func Test_toGRPCError(t *testing.T) { type args struct { ctx context.Context - req interface{} + req any handler grpc.UnaryHandler } - type res struct { - want interface{} - wantErr bool - } tests := []struct { - name string - args args - res res + name string + args args + want any + wantCode codes.Code }{ { - "no error", - args{ + name: "no error", + args: args{ ctx: context.Background(), req: &mockReq{}, handler: emptyMockHandler, }, - res{ - &mockReq{}, - false, - }, + want: &mockReq{}, }, { - "error", - args{ + name: "error", + args: args{ ctx: context.Background(), req: &mockReq{}, handler: errorMockHandler, }, - res{ - nil, - true, + want: nil, + wantCode: codes.FailedPrecondition, + }, + { + name: "panic with string", + args: args{ + ctx: context.Background(), + req: &mockReq{}, + handler: panicMockHandler("test panic"), }, + want: nil, + wantCode: codes.Internal, + }, + { + name: "panic with error", + args: args{ + ctx: context.Background(), + req: &mockReq{}, + handler: panicMockHandler(errors.New("oops")), + }, + want: nil, + wantCode: codes.Internal, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got, err := toGRPCError(tt.args.ctx, tt.args.req, tt.args.handler) - if (err != nil) != tt.res.wantErr { - t.Errorf("toGRPCError() error = %v, wantErr %v", err, tt.res.wantErr) - return - } - if !reflect.DeepEqual(got, tt.res.want) { - t.Errorf("toGRPCError() got = %v, want %v", got, tt.res.want) + if tt.wantCode != 0 { + status, ok := status.FromError(err) + require.True(t, ok) + assert.Equal(t, tt.wantCode, status.Code()) + } else { + require.NoError(t, err) } + assert.Equal(t, tt.want, got) }) } } diff --git a/internal/api/grpc/server/middleware/mock_test.go b/internal/api/grpc/server/middleware/mock_test.go index 498f45082..e3424531d 100644 --- a/internal/api/grpc/server/middleware/mock_test.go +++ b/internal/api/grpc/server/middleware/mock_test.go @@ -8,12 +8,18 @@ import ( "github.com/zitadel/zitadel/internal/zerrors" ) -func emptyMockHandler(_ context.Context, req interface{}) (interface{}, error) { +func emptyMockHandler(_ context.Context, req any) (any, error) { return req, nil } -func errorMockHandler(_ context.Context, req interface{}) (interface{}, error) { - return nil, zerrors.ThrowInternal(nil, "test", "error") +func errorMockHandler(_ context.Context, req any) (any, error) { + return nil, zerrors.ThrowPreconditionFailed(nil, "test", "error") +} + +func panicMockHandler(payload any) func(context.Context, any) (any, error) { + return func(context.Context, any) (any, error) { + panic(payload) + } } type mockReq struct{} diff --git a/internal/api/http/middleware/recover_interceptor.go b/internal/api/http/middleware/recover_interceptor.go new file mode 100644 index 000000000..2f730f9ea --- /dev/null +++ b/internal/api/http/middleware/recover_interceptor.go @@ -0,0 +1,82 @@ +package middleware + +import ( + "context" + "fmt" + "net/http" + + "github.com/zitadel/sloggcp" + + "github.com/zitadel/zitadel/backend/v3/instrumentation/logging" + "github.com/zitadel/zitadel/internal/zerrors" +) + +// RecoverHandler recovers from panics in the HTTP handler chain +// and calls the provided writeResponse function to write an appropriate response to the client. +// +// The request context is canceled with the panic error as the cause. +func RecoverHandler(writeResponse func(w http.ResponseWriter, r *http.Request, err error)) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx, cancel := context.WithCancelCause(r.Context()) + r = r.WithContext(ctx) + + defer func() { + var err error + if rec := recover(); rec != nil { + recErr, ok := rec.(error) + if !ok { + recErr = fmt.Errorf("%v", rec) + } + err = zerrors.ThrowInternal(recErr, zerrors.IDRecover, "Errors.Internal") + logRecovered(ctx, err) + writeResponse(w, r, err) + } + cancel(err) + }() + + next.ServeHTTP(w, r) + }) + } +} + +// FallbackRecoverHandler recovers from panics in the HTTP handler chain +// and returns a 500 Internal Server Error response. +// The request context is canceled with the panic error as the cause, +// so that any ongoing operations can be stopped and cleaned up. +// +// The response is sent as a text/plain response. +// It is used as a last line of defense to prevent the server from crashing +// due to panics in the handlers. +// Protocols (OIDC, SAML, HTML etc.) should use [RecoverHandler] to write +// properly formatted error responses to the clients. +func FallbackRecoverHandler() func(http.Handler) http.Handler { + return RecoverHandler(func(w http.ResponseWriter, r *http.Request, _ error) { + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + }) +} + +// RecoverHandlerWithError is similar to [RecoverHandler] but returns an error instead of writing a response directly. +func RecoverHandlerWithError(next HandlerFuncWithError) HandlerFuncWithError { + return func(w http.ResponseWriter, r *http.Request) (err error) { + ctx, cancel := context.WithCancelCause(r.Context()) + r = r.WithContext(ctx) + defer func() { + if rec := recover(); rec != nil { + recErr, ok := rec.(error) + if !ok { + recErr = fmt.Errorf("%v", rec) + } + err = zerrors.ThrowInternal(recErr, zerrors.IDRecover, "Errors.Internal") + logRecovered(ctx, err) + } + cancel(err) + }() + return next(w, r) + } +} + +func logRecovered(ctx context.Context, err error) { + logger := logging.FromCtx(ctx) + logger.Log(ctx, sloggcp.LevelAlert, "recovered from panic", "err", err) +} diff --git a/internal/api/http/middleware/recover_interceptor_test.go b/internal/api/http/middleware/recover_interceptor_test.go new file mode 100644 index 000000000..3a636f561 --- /dev/null +++ b/internal/api/http/middleware/recover_interceptor_test.go @@ -0,0 +1,91 @@ +package middleware + +import ( + "errors" + "io" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/zitadel/zitadel/internal/zerrors" +) + +func TestRecoverHandler(t *testing.T) { + tests := []struct { + name string + handler http.HandlerFunc + wantStatus int + wantBody string + }{ + { + name: "no panic", + handler: func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + io.WriteString(w, "ok") //nolint:errcheck + }, + wantStatus: http.StatusOK, + wantBody: "ok", + }, + { + name: "panic with error", + handler: func(w http.ResponseWriter, r *http.Request) { + panic(errors.New("oops")) + }, + wantStatus: http.StatusInternalServerError, + wantBody: "ID=RECOVER Message=Errors.Internal Parent=(oops)", + }, + { + name: "panic with string", + handler: func(w http.ResponseWriter, r *http.Request) { + panic("something went wrong") + }, + wantStatus: http.StatusInternalServerError, + wantBody: "ID=RECOVER Message=Errors.Internal Parent=(something went wrong)", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + handler := RecoverHandler(writeResponse)(tt.handler) + w := httptest.NewRecorder() + r := httptest.NewRequest(http.MethodGet, "/", nil) + handler.ServeHTTP(w, r) + + res := w.Result() + defer res.Body.Close() + body, _ := io.ReadAll(res.Body) + assert.Equal(t, tt.wantStatus, res.StatusCode) + assert.Equal(t, tt.wantBody, string(body)) + }) + } +} + +func TestFallbackRecoverHandler(t *testing.T) { + handler := FallbackRecoverHandler()(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + panic("unexpected error") + })) + + w := httptest.NewRecorder() + r := httptest.NewRequest(http.MethodGet, "/", nil) + handler.ServeHTTP(w, r) + + res := w.Result() + defer res.Body.Close() + body, _ := io.ReadAll(res.Body) + assert.Equal(t, http.StatusInternalServerError, res.StatusCode) + assert.Equal(t, "Internal Server Error\n", string(body)) +} + +func TestRecoverHandlerWithError(t *testing.T) { + w := httptest.NewRecorder() + r := httptest.NewRequest(http.MethodGet, "/", nil) + // Panic on nil handler. + err := RecoverHandlerWithError(nil)(w, r) + assert.ErrorIs(t, err, zerrors.ThrowInternal(nil, zerrors.IDRecover, "Errors.Internal")) +} + +func writeResponse(w http.ResponseWriter, _ *http.Request, err error) { + w.WriteHeader(http.StatusInternalServerError) + io.WriteString(w, err.Error()) //nolint:errcheck +} diff --git a/internal/api/oidc/error.go b/internal/api/oidc/error.go index 5cba33044..7ed7644dc 100644 --- a/internal/api/oidc/error.go +++ b/internal/api/oidc/error.go @@ -3,10 +3,12 @@ package oidc import ( "context" "errors" + "net/http" "github.com/zitadel/oidc/v3/pkg/oidc" "github.com/zitadel/oidc/v3/pkg/op" + "github.com/zitadel/zitadel/backend/v3/instrumentation/logging" http_util "github.com/zitadel/zitadel/internal/api/http" "github.com/zitadel/zitadel/internal/zerrors" ) @@ -47,3 +49,7 @@ func oidcError(ctx context.Context, err error) error { oidcErr.Description = zError.GetMessage() return op.NewStatusError(oidcErr, statusCode) } + +func writeRecoverError(w http.ResponseWriter, r *http.Request, err error) { + op.WriteError(w, r, err, logging.FromCtx(r.Context())) +} diff --git a/internal/api/oidc/op.go b/internal/api/oidc/op.go index dae87964d..35576774c 100644 --- a/internal/api/oidc/op.go +++ b/internal/api/oidc/op.go @@ -186,6 +186,7 @@ func NewServer( middleware.MetricsHandler(metricTypes), middleware.TraceHandler(), middleware.LogHandler("oidc"), + middleware.RecoverHandler(writeRecoverError), middleware.NoCacheInterceptor().Handler, instanceHandler, userAgentCookie, diff --git a/internal/api/scim/server.go b/internal/api/scim/server.go index 38b7128e8..ae2228f36 100644 --- a/internal/api/scim/server.go +++ b/internal/api/scim/server.go @@ -70,7 +70,10 @@ func buildMiddleware( middlewares []zhttp_middlware.MiddlewareWithErrorFunc, ) zhttp_middlware.ErrorHandlerFunc { // content type middleware needs to run at the very beginning to correctly set content types of errors - middlewares = append([]zhttp_middlware.MiddlewareWithErrorFunc{smiddleware.ContentTypeMiddleware}, middlewares...) + middlewares = append([]zhttp_middlware.MiddlewareWithErrorFunc{ + smiddleware.ContentTypeMiddleware, + zhttp_middlware.RecoverHandlerWithError, + }, middlewares...) middlewares = append(middlewares, smiddleware.ScimContextMiddleware(query)) scimMiddleware := zhttp_middlware.ChainedWithErrorHandler(serrors.ErrorHandler(translator), middlewares...) return func(handler zhttp_middlware.HandlerFuncWithError) http.Handler { diff --git a/internal/api/ui/login/renderer.go b/internal/api/ui/login/renderer.go index d0238ce6e..edd733a41 100644 --- a/internal/api/ui/login/renderer.go +++ b/internal/api/ui/login/renderer.go @@ -11,8 +11,9 @@ import ( "time" "github.com/gorilla/csrf" - "github.com/zitadel/logging" + old_logging "github.com/zitadel/logging" //nolint:staticcheck + "github.com/zitadel/zitadel/backend/v3/instrumentation/logging" "github.com/zitadel/zitadel/cmd/build" "github.com/zitadel/zitadel/internal/api/authz" http_mw "github.com/zitadel/zitadel/internal/api/http/middleware" @@ -249,7 +250,7 @@ func CreateRenderer(pathPrefix string, staticStorage static.Storage, cookieName tmplMapping, funcs, cookieName, ) - logging.New().OnError(err).WithError(err).Panic("error creating renderer") + old_logging.New().OnError(err).WithError(err).Panic("error creating renderer") return r } @@ -342,18 +343,20 @@ func (l *Login) chooseNextStep(w http.ResponseWriter, r *http.Request, authReq * } func (l *Login) renderInternalError(w http.ResponseWriter, r *http.Request, authReq *domain.AuthRequest, err error) { + ctx := r.Context() if err != nil { - log := logging.WithError(err) + log := logging.FromCtx(ctx).With("err", err) if authReq != nil { - log = log.WithField("auth_req_id", authReq.ID) + log = log.With("auth_req_id", authReq.ID) } + const msg = "render internal error" if zerrors.IsInternal(err) { - log.Error() + log.ErrorContext(ctx, msg) } else { - log.Info() + log.InfoContext(ctx, msg) } } - translator := l.getTranslator(r.Context(), authReq) + translator := l.getTranslator(ctx, authReq) data := l.getBaseData(r, authReq, translator, "Errors.Internal", "", err) l.renderer.RenderTemplate(w, r, translator, l.renderer.Templates[tmplError], data, nil) } @@ -568,7 +571,7 @@ func (l *Login) getOrgPrimaryDomain(r *http.Request, authReq *domain.AuthRequest } org, err := l.query.OrgByID(r.Context(), orgID) if err != nil { - logging.New().WithError(err).Error("cannot get default org") + old_logging.New().WithError(err).Error("cannot get default org") return "" } return org.Domain @@ -591,7 +594,7 @@ func (l *Login) addLoginTranslations(translator *i18n.Translator, customTexts [] Text: text.Text, } err := l.renderer.AddMessages(translator, text.Language, msg) - logging.OnError(err).Warn("could no add message to translator") + old_logging.OnError(err).Warn("could no add message to translator") } } @@ -599,7 +602,7 @@ func (l *Login) customTexts(ctx context.Context, translator *i18n.Translator, or instanceID := authz.GetInstance(ctx).InstanceID() instanceTexts, err := l.query.CustomTextListByTemplate(ctx, instanceID, domain.LoginCustomText, false) if err != nil { - logging.WithFields("instanceID", instanceID).Warn("unable to load custom texts for instance") + old_logging.WithFields("instanceID", instanceID).Warn("unable to load custom texts for instance") return } l.addLoginTranslations(translator, query.CustomTextsToDomain(instanceTexts)) @@ -608,7 +611,7 @@ func (l *Login) customTexts(ctx context.Context, translator *i18n.Translator, or } orgTexts, err := l.query.CustomTextListByTemplate(ctx, orgID, domain.LoginCustomText, false) if err != nil { - logging.WithFields("instanceID", instanceID, "org", orgID).Warn("unable to load custom texts for org") + old_logging.WithFields("instanceID", instanceID, "org", orgID).Warn("unable to load custom texts for org") return } l.addLoginTranslations(translator, query.CustomTextsToDomain(orgTexts)) diff --git a/internal/api/ui/login/router.go b/internal/api/ui/login/router.go index 459a0aab5..6f8ab5537 100644 --- a/internal/api/ui/login/router.go +++ b/internal/api/ui/login/router.go @@ -4,6 +4,7 @@ import ( "net/http" "github.com/gorilla/mux" + "github.com/zitadel/zitadel/internal/api/http/middleware" ) const ( @@ -71,6 +72,7 @@ var ( func CreateRouter(login *Login, interceptors ...mux.MiddlewareFunc) *mux.Router { router := mux.NewRouter() router.Use(interceptors...) + router.Use(middleware.RecoverHandler(login.writeRecoverError)) router.HandleFunc(EndpointRoot, login.handleLogin).Methods(http.MethodGet) router.HandleFunc(EndpointHealthz, login.handleHealthz).Methods(http.MethodGet) router.HandleFunc(EndpointReadiness, login.handleReadiness).Methods(http.MethodGet) @@ -131,3 +133,7 @@ func CreateRouter(login *Login, interceptors ...mux.MiddlewareFunc) *mux.Router router.HandleFunc(EndpointDeviceAuthAction, login.handleDeviceAuthAction).Methods(http.MethodGet, http.MethodPost) return router } + +func (l *Login) writeRecoverError(w http.ResponseWriter, r *http.Request, err error) { + l.renderError(w, r, nil, err) +} diff --git a/internal/zerrors/zerror.go b/internal/zerrors/zerror.go index bb3b6ba58..040e27025 100644 --- a/internal/zerrors/zerror.go +++ b/internal/zerrors/zerror.go @@ -80,6 +80,9 @@ const ( KindUnauthenticated Kind = 16 ) +// IDRecover is the error ID used for errors created after recovery from panics. +const IDRecover = "RECOVER" + // Because errors are created through singletons, config is global. var ( enableReportLocation atomic.Bool