From e9c34a233712679f4c783b3aa472f5b640e91a37 Mon Sep 17 00:00:00 2001 From: Jan Dittberner Date: Sat, 2 Jan 2021 18:06:03 +0100 Subject: [PATCH] Implement error pages, improve request logging --- active.de.toml | 4 + active.en.toml | 1 + app/handlers/oidc_callback.go | 19 ++--- cmd/app/main.go | 104 ++++++----------------- cmd/idp/main.go | 70 ++++++---------- common/handlers/errors.go | 137 +++++++++++++++++++++++++++++++ common/handlers/observability.go | 31 ++++++- common/handlers/startup.go | 45 ++++++++++ common/services/configuration.go | 65 +++++++++++++++ common/services/i18n.go | 4 + frontend_src/_custom.scss | 5 +- idp/handlers/consent.go | 41 +++++++-- idp/handlers/login.go | 37 +++++++-- templates/app/base.gohtml | 13 ++- templates/app/errors.gohtml | 13 +++ templates/app/index.gohtml | 9 +- templates/idp/base.gohtml | 11 ++- templates/idp/errors.gohtml | 13 +++ translate.de.toml | 7 +- 19 files changed, 462 insertions(+), 167 deletions(-) create mode 100644 common/handlers/errors.go create mode 100644 common/handlers/startup.go create mode 100644 common/services/configuration.go create mode 100644 templates/app/errors.gohtml create mode 100644 templates/idp/errors.gohtml diff --git a/active.de.toml b/active.de.toml index 97cc7e9..f67719d 100644 --- a/active.de.toml +++ b/active.de.toml @@ -22,6 +22,10 @@ other = "Bitte gib ein Passwort ein." hash = "sha1-31632fcec9d22a8463757f459e51c7c0eccd1f28" other = "Dieses Feld wird benötigt." +[ErrorTitle] +hash = "sha1-736aec25a98f5ec5b71400bb0163f891f509b566" +other = "Es ist ein Fehler aufgetreten" + [ErrorUnknown] hash = "sha1-e5fd9aa24c9417e7332e6f25936ae2a6ec8f1524" other = "Unbekannter Fehler" diff --git a/active.en.toml b/active.en.toml index 75769f9..9d7bf69 100644 --- a/active.en.toml +++ b/active.en.toml @@ -4,6 +4,7 @@ ErrorEmail = "Please enter a valid email address." ErrorEmailRequired = "Please enter an email address." ErrorPasswordRequired = "Please enter a password." ErrorRequired = "Please enter a value" +ErrorTitle = "An error has occurred" ErrorUnknown = "Unknown error" IndexGreeting = "Hello {{ .User }}" IndexIntroductionText = "This is an authorization protected resource" diff --git a/app/handlers/oidc_callback.go b/app/handlers/oidc_callback.go index 30c6b05..42dea36 100644 --- a/app/handlers/oidc_callback.go +++ b/app/handlers/oidc_callback.go @@ -10,6 +10,7 @@ import ( "golang.org/x/oauth2" "git.cacert.org/oidc_login/app/services" + "git.cacert.org/oidc_login/common/handlers" commonServices "git.cacert.org/oidc_login/common/services" ) @@ -28,7 +29,7 @@ type oidcCallbackHandler struct { func (c *oidcCallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodGet { - http.Error(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed) + w.WriteHeader(http.StatusMethodNotAllowed) return } if r.URL.Path != "/callback" { @@ -39,7 +40,13 @@ func (c *oidcCallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) errorText := r.URL.Query().Get("error") errorDescription := r.URL.Query().Get("error_description") if errorText != "" { - c.RenderErrorTemplate(w, errorText, errorDescription, http.StatusForbidden) + errorDetails := &handlers.ErrorDetails{ + ErrorMessage: errorText, + } + if errorDescription != "" { + errorDetails.ErrorDetails = []string{errorDescription} + } + handlers.GetErrorBucket(r).AddError(errorDetails) return } @@ -106,14 +113,6 @@ Not valid after: %s w.WriteHeader(http.StatusFound) } -func (c *oidcCallbackHandler) RenderErrorTemplate(w http.ResponseWriter, errorText string, errorDescription string, status int) { - if errorDescription != "" { - http.Error(w, errorDescription, status) - } else { - http.Error(w, errorText, status) - } -} - func NewCallbackHandler(ctx context.Context, logger *log.Logger) *oidcCallbackHandler { return &oidcCallbackHandler{ keySet: commonServices.GetJwkSet(ctx), diff --git a/cmd/app/main.go b/cmd/app/main.go index c936b08..03d7f7b 100644 --- a/cmd/app/main.go +++ b/cmd/app/main.go @@ -6,20 +6,11 @@ import ( "encoding/base64" "fmt" "net/http" - "os" - "os/signal" - "strings" - "sync/atomic" "time" - "github.com/knadh/koanf" "github.com/knadh/koanf/parsers/toml" "github.com/knadh/koanf/providers/confmap" - "github.com/knadh/koanf/providers/env" - "github.com/knadh/koanf/providers/file" - "github.com/knadh/koanf/providers/posflag" log "github.com/sirupsen/logrus" - flag "github.com/spf13/pflag" "git.cacert.org/oidc_login/app/handlers" "git.cacert.org/oidc_login/app/services" @@ -28,48 +19,21 @@ import ( ) func main() { - f := flag.NewFlagSet("config", flag.ContinueOnError) - f.Usage = func() { - fmt.Println(f.FlagUsages()) - os.Exit(0) - } - f.StringSlice("conf", []string{"resource_app.toml"}, "path to one or more .toml files") logger := log.New() - var err error - - if err = f.Parse(os.Args[1:]); err != nil { - logger.Fatal(err) - } - - config := koanf.New(".") - - _ = config.Load(confmap.Provider(map[string]interface{}{ - "server.port": 4000, - "server.name": "app.cacert.localhost", - "server.key": "certs/app.cacert.localhost.key", - "server.certificate": "certs/app.cacert.localhost.crt.pem", - "oidc.server": "https://auth.cacert.localhost:4444/", - "session.path": "sessions/app", - "i18n.languages": []string{"en", "de"}, - }, "."), nil) - cFiles, _ := f.GetStringSlice("conf") - for _, c := range cFiles { - if err := config.Load(file.Provider(c), toml.Parser()); err != nil { - logger.Fatalf("error loading config file: %s", err) - } - } - if err := config.Load(posflag.Provider(f, ".", config), nil); err != nil { - logger.Fatalf("error loading configuration: %s", err) - } - if err := config.Load(file.Provider("resource_app.toml"), toml.Parser()); err != nil && !os.IsNotExist(err) { - log.Fatalf("error loading config: %v", err) - } - const prefix = "RESOURCE_APP_" - if err := config.Load(env.Provider(prefix, ".", func(s string) string { - return strings.Replace(strings.ToLower( - strings.TrimPrefix(s, prefix)), "_", ".", -1) - }), nil); err != nil { - log.Fatalf("error loading config: %v", err) + config, err := commonServices.ConfigureApplication( + logger, + "RESOURCE_APP", + map[string]interface{}{ + "server.port": 4000, + "server.name": "app.cacert.localhost", + "server.key": "certs/app.cacert.localhost.key", + "server.certificate": "certs/app.cacert.localhost.crt.pem", + "oidc.server": "https://auth.cacert.localhost:4444/", + "session.path": "sessions/app", + "i18n.languages": []string{"en", "de"}, + }) + if err != nil { + log.Fatalf("error loading configuration: %v", err) } oidcServer := config.MustString("oidc.server") @@ -151,6 +115,14 @@ func main() { tracing := commonHandlers.Tracing(nextRequestId) logging := commonHandlers.Logging(logger) hsts := commonHandlers.EnableHSTS() + errorMiddleware, err := commonHandlers.ErrorHandling( + ctx, + logger, + "templates/app", + ) + if err != nil { + logger.Fatalf("could not initialize request error handling: %v", err) + } tlsConfig := &tls.Config{ ServerName: config.String("server.name"), @@ -158,40 +130,12 @@ func main() { } server := &http.Server{ Addr: serverAddr, - Handler: tracing(logging(hsts(router))), + Handler: tracing(logging(hsts(errorMiddleware(router)))), ReadTimeout: 5 * time.Second, WriteTimeout: 10 * time.Second, IdleTimeout: 15 * time.Second, TLSConfig: tlsConfig, } - done := make(chan bool) - quit := make(chan os.Signal, 1) - signal.Notify(quit, os.Interrupt) - - go func() { - <-quit - logger.Infoln("Server is shutting down...") - atomic.StoreInt32(&commonHandlers.Healthy, 0) - - ctx, cancel := context.WithTimeout(ctx, 30*time.Second) - defer cancel() - - server.SetKeepAlivesEnabled(false) - if err := server.Shutdown(ctx); err != nil { - logger.Fatalf("Could not gracefully shutdown the server: %v\n", err) - } - close(done) - }() - - logger.Infof("Server is ready to handle requests at https://%s/", server.Addr) - atomic.StoreInt32(&commonHandlers.Healthy, 1) - if err := server.ListenAndServeTLS( - config.String("server.certificate"), config.String("server.key"), - ); err != nil && err != http.ErrServerClosed { - logger.Fatalf("Could not listen on %s: %v\n", server.Addr, err) - } - - <-done - logger.Infoln("Server stopped") + commonHandlers.StartApplication(logger, ctx, server, config) } diff --git a/cmd/idp/main.go b/cmd/idp/main.go index 9ec5aa0..12832b0 100644 --- a/cmd/idp/main.go +++ b/cmd/idp/main.go @@ -11,21 +11,13 @@ import ( "net/url" "os" "os/signal" - "strings" "sync/atomic" "time" "github.com/go-openapi/runtime/client" "github.com/gorilla/csrf" - "github.com/knadh/koanf" - "github.com/knadh/koanf/parsers/toml" - "github.com/knadh/koanf/providers/confmap" - "github.com/knadh/koanf/providers/env" - "github.com/knadh/koanf/providers/file" - "github.com/knadh/koanf/providers/posflag" hydra "github.com/ory/hydra-client-go/client" log "github.com/sirupsen/logrus" - flag "github.com/spf13/pflag" commonHandlers "git.cacert.org/oidc_login/common/handlers" commonServices "git.cacert.org/oidc_login/common/services" @@ -34,45 +26,21 @@ import ( ) func main() { - f := flag.NewFlagSet("config", flag.ContinueOnError) - f.Usage = func() { - fmt.Println(f.FlagUsages()) - os.Exit(0) - } - f.StringSlice("conf", []string{"idp.toml"}, "path to one or more .toml files") logger := log.New() - var err error - - if err = f.Parse(os.Args[1:]); err != nil { - logger.Fatal(err) - } - - config := koanf.New(".") - - _ = config.Load(confmap.Provider(map[string]interface{}{ - "server.port": 3000, - "server.name": "login.cacert.localhost", - "server.key": "certs/idp.cacert.localhost.key", - "server.certificate": "certs/idp.cacert.localhost.crt.pem", - "security.client.ca-file": "certs/client_ca.pem", - "admin.url": "https://hydra.cacert.localhost:4445/", - "i18n.languages": []string{"en", "de"}, - }, "."), nil) - cFiles, _ := f.GetStringSlice("conf") - for _, c := range cFiles { - if err := config.Load(file.Provider(c), toml.Parser()); err != nil { - logger.Fatalf("error loading config file: %s", err) - } - } - if err := config.Load(posflag.Provider(f, ".", config), nil); err != nil { - logger.Fatalf("error loading configuration: %s", err) - } - const prefix = "IDP_" - if err := config.Load(env.Provider(prefix, ".", func(s string) string { - return strings.Replace(strings.ToLower( - strings.TrimPrefix(s, prefix)), "_", ".", -1) - }), nil); err != nil { - log.Fatalf("error loading config: %v", err) + config, err := commonServices.ConfigureApplication( + logger, + "IDP", + map[string]interface{}{ + "server.port": 3000, + "server.name": "login.cacert.localhost", + "server.key": "certs/idp.cacert.localhost.key", + "server.certificate": "certs/idp.cacert.localhost.crt.pem", + "security.client.ca-file": "certs/client_ca.pem", + "admin.url": "https://hydra.cacert.localhost:4445/", + "i18n.languages": []string{"en", "de"}, + }) + if err != nil { + log.Fatalf("error loading configuration: %v", err) } logger.Infoln("Server is starting") @@ -139,6 +107,14 @@ func main() { csrf.Secure(true), csrf.SameSite(csrf.SameSiteStrictMode), csrf.MaxAge(600)) + errorMiddleware, err := commonHandlers.ErrorHandling( + ctx, + logger, + "templates/idp", + ) + if err != nil { + logger.Fatalf("could not initialize request error handling: %v", err) + } clientCertPool := x509.NewCertPool() pemBytes, err := ioutil.ReadFile(config.MustString("security.client.ca-file")) @@ -155,7 +131,7 @@ func main() { } server := &http.Server{ Addr: fmt.Sprintf("%s:%d", config.String("server.name"), config.Int("server.port")), - Handler: tracing(logging(hsts(csrfProtect(router)))), + Handler: tracing(logging(hsts(errorMiddleware(csrfProtect(router))))), ReadTimeout: 20 * time.Second, WriteTimeout: 20 * time.Second, IdleTimeout: 30 * time.Second, diff --git a/common/handlers/errors.go b/common/handlers/errors.go new file mode 100644 index 0000000..0992f20 --- /dev/null +++ b/common/handlers/errors.go @@ -0,0 +1,137 @@ +package handlers + +import ( + "context" + "html/template" + "net/http" + "path" + + "github.com/nicksnyder/go-i18n/v2/i18n" + log "github.com/sirupsen/logrus" + + commonServices "git.cacert.org/oidc_login/common/services" +) + +type errorKey int + +const ( + errorBucketKey errorKey = iota +) + +type ErrorDetails struct { + ErrorMessage string + ErrorDetails []string + ErrorCode string + Error error +} + +type ErrorBucket struct { + errorDetails *ErrorDetails + templates *template.Template + logger *log.Logger + bundle *i18n.Bundle + messageCatalog *commonServices.MessageCatalog +} + +func (b *ErrorBucket) serveHTTP(w http.ResponseWriter, r *http.Request) { + if b.errorDetails != nil { + accept := r.Header.Get("Accept-Language") + localizer := i18n.NewLocalizer(b.bundle, accept) + err := b.templates.Lookup("base").Execute(w, map[string]interface{}{ + "Title": b.messageCatalog.LookupMessage( + "ErrorTitle", + nil, + localizer, + ), + "details": b.errorDetails, + }) + if err != nil { + log.Errorf("error rendering error template: %v", err) + http.Error( + w, + http.StatusText(http.StatusInternalServerError), + http.StatusInternalServerError, + ) + } + } +} + +func GetErrorBucket(r *http.Request) *ErrorBucket { + return r.Context().Value(errorBucketKey).(*ErrorBucket) +} + +// call this from your application's handler +func (b *ErrorBucket) AddError(details *ErrorDetails) { + b.errorDetails = details +} + +type errorResponseWriter struct { + http.ResponseWriter + ctx context.Context + statusCode int +} + +func (w *errorResponseWriter) WriteHeader(code int) { + w.statusCode = code + if code >= 400 { + w.Header().Set("Content-Type", "text/html; charset=utf-8") + errorBucket := w.ctx.Value(errorBucketKey).(*ErrorBucket) + if errorBucket != nil && errorBucket.errorDetails == nil { + errorBucket.AddError(&ErrorDetails{ + ErrorMessage: http.StatusText(code), + }) + } + } + w.ResponseWriter.WriteHeader(code) +} + +func (w *errorResponseWriter) Write(content []byte) (int, error) { + if w.statusCode > 400 { + errorBucket := w.ctx.Value(errorBucketKey).(*ErrorBucket) + if errorBucket != nil { + if errorBucket.errorDetails.ErrorDetails == nil { + errorBucket.errorDetails.ErrorDetails = make([]string, 0) + } + errorBucket.errorDetails.ErrorDetails = append( + errorBucket.errorDetails.ErrorDetails, string(content), + ) + return len(content), nil + } + } + return w.ResponseWriter.Write(content) +} + +func ErrorHandling( + handlerContext context.Context, + logger *log.Logger, + templateBaseDir string, +) (func(http.Handler) http.Handler, error) { + errorTemplates, err := template.ParseFiles( + path.Join(templateBaseDir, "base.gohtml"), + path.Join(templateBaseDir, "errors.gohtml"), + ) + if err != nil { + return nil, err + } + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + errorBucket := &ErrorBucket{ + templates: errorTemplates, + logger: logger, + bundle: commonServices.GetI18nBundle(handlerContext), + messageCatalog: commonServices.GetMessageCatalog(handlerContext), + } + ctx := context.WithValue(r.Context(), errorBucketKey, errorBucket) + interCeptingResponseWriter := &errorResponseWriter{ + w, + ctx, + http.StatusOK, + } + next.ServeHTTP( + interCeptingResponseWriter, + r.WithContext(ctx), + ) + errorBucket.serveHTTP(w, r) + }) + }, nil +} diff --git a/common/handlers/observability.go b/common/handlers/observability.go index 3c34c42..bf72806 100644 --- a/common/handlers/observability.go +++ b/common/handlers/observability.go @@ -14,17 +14,44 @@ const ( requestIdKey key = iota ) +type statusCodeInterceptor struct { + http.ResponseWriter + code int + count int +} + +func (sci *statusCodeInterceptor) WriteHeader(code int) { + sci.code = code + sci.ResponseWriter.WriteHeader(code) +} + +func (sci *statusCodeInterceptor) Write(content []byte) (int, error) { + count, err := sci.ResponseWriter.Write(content) + sci.count += count + return count, err +} + func Logging(logger *log.Logger) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + interceptor := &statusCodeInterceptor{w, http.StatusOK, 0} defer func() { requestId, ok := r.Context().Value(requestIdKey).(string) if !ok { requestId = "unknown" } - logger.Infoln(requestId, r.Method, r.URL.Path, r.RemoteAddr, r.UserAgent()) + logger.Infof( + "%s %s \"%s %s\" %d %d \"%s\"", + requestId, + r.RemoteAddr, + r.Method, + r.URL.Path, + interceptor.code, + interceptor.count, + r.UserAgent(), + ) }() - next.ServeHTTP(w, r) + next.ServeHTTP(interceptor, r) }) } } diff --git a/common/handlers/startup.go b/common/handlers/startup.go new file mode 100644 index 0000000..9be56d6 --- /dev/null +++ b/common/handlers/startup.go @@ -0,0 +1,45 @@ +package handlers + +import ( + "context" + "net/http" + "os" + "os/signal" + "sync/atomic" + "time" + + "github.com/knadh/koanf" + "github.com/sirupsen/logrus" +) + +func StartApplication(logger *logrus.Logger, ctx context.Context, server *http.Server, config *koanf.Koanf) { + done := make(chan bool) + quit := make(chan os.Signal, 1) + signal.Notify(quit, os.Interrupt) + + go func() { + <-quit + logger.Infoln("Server is shutting down...") + atomic.StoreInt32(&Healthy, 0) + + ctx, cancel := context.WithTimeout(ctx, 30*time.Second) + defer cancel() + + server.SetKeepAlivesEnabled(false) + if err := server.Shutdown(ctx); err != nil { + logger.Fatalf("Could not gracefully shutdown the server: %v\n", err) + } + close(done) + }() + + logger.Infof("Server is ready to handle requests at https://%s/", server.Addr) + atomic.StoreInt32(&Healthy, 1) + if err := server.ListenAndServeTLS( + config.String("server.certificate"), config.String("server.key"), + ); err != nil && err != http.ErrServerClosed { + logger.Fatalf("Could not listen on %s: %v\n", server.Addr, err) + } + + <-done + logger.Infoln("Server stopped") +} diff --git a/common/services/configuration.go b/common/services/configuration.go new file mode 100644 index 0000000..bf272b5 --- /dev/null +++ b/common/services/configuration.go @@ -0,0 +1,65 @@ +package services + +import ( + "fmt" + "os" + "strings" + + "github.com/knadh/koanf" + "github.com/knadh/koanf/parsers/toml" + "github.com/knadh/koanf/providers/confmap" + "github.com/knadh/koanf/providers/env" + "github.com/knadh/koanf/providers/file" + "github.com/knadh/koanf/providers/posflag" + "github.com/sirupsen/logrus" + "github.com/spf13/pflag" +) + +func ConfigureApplication( + logger *logrus.Logger, + appName string, + defaultConfig map[string]interface{}, +) (*koanf.Koanf, error) { + f := pflag.NewFlagSet("config", pflag.ContinueOnError) + f.Usage = func() { + fmt.Println(f.FlagUsages()) + os.Exit(0) + } + f.StringSlice( + "conf", + []string{fmt.Sprintf("%s.toml", strings.ToLower(appName))}, + "path to one or more .toml files", + ) + var err error + + if err = f.Parse(os.Args[1:]); err != nil { + logger.Fatal(err) + } + + config := koanf.New(".") + + _ = config.Load(confmap.Provider(defaultConfig, "."), nil) + cFiles, _ := f.GetStringSlice("conf") + for _, c := range cFiles { + if err := config.Load(file.Provider(c), toml.Parser()); err != nil { + logger.Fatalf("error loading config file: %s", err) + } + } + if err := config.Load(posflag.Provider(f, ".", config), nil); err != nil { + logger.Fatalf("error loading configuration: %s", err) + } + if err := config.Load( + file.Provider("resource_app.toml"), + toml.Parser(), + ); err != nil && !os.IsNotExist(err) { + logrus.Fatalf("error loading config: %v", err) + } + prefix := fmt.Sprintf("%s_", strings.ToUpper(appName)) + if err := config.Load(env.Provider(prefix, ".", func(s string) string { + return strings.Replace(strings.ToLower( + strings.TrimPrefix(s, prefix)), "_", ".", -1) + }), nil); err != nil { + logrus.Fatalf("error loading config: %v", err) + } + return config, err +} diff --git a/common/services/i18n.go b/common/services/i18n.go index 1985ff2..65ef9b8 100644 --- a/common/services/i18n.go +++ b/common/services/i18n.go @@ -91,6 +91,10 @@ func InitI18n(ctx context.Context, logger *log.Logger, languages []string) conte func initMessageCatalog(logger *log.Logger) *MessageCatalog { messages := make(map[string]*i18n.Message) + messages["ErrorTitle"] = &i18n.Message{ + ID: "ErrorTitle", + Other: "An error has occurred", + } return &MessageCatalog{messages: messages, logger: logger} } diff --git a/frontend_src/_custom.scss b/frontend_src/_custom.scss index 319f9f9..97072c0 100644 --- a/frontend_src/_custom.scss +++ b/frontend_src/_custom.scss @@ -11,8 +11,9 @@ body { body.idp { display: -ms-flexbox; display: flex; - // -ms-flex-align: center; - // align-items: center; +} + +.error-message, body.idp { padding-top: 40px; padding-bottom: 40px; } diff --git a/idp/handlers/consent.go b/idp/handlers/consent.go index 08f383a..518dd4a 100644 --- a/idp/handlers/consent.go +++ b/idp/handlers/consent.go @@ -6,6 +6,7 @@ import ( "fmt" "html/template" "net/http" + "strconv" "strings" "time" @@ -18,6 +19,7 @@ import ( "github.com/ory/hydra-client-go/models" log "github.com/sirupsen/logrus" + "git.cacert.org/oidc_login/common/handlers" commonServices "git.cacert.org/oidc_login/common/services" "git.cacert.org/oidc_login/idp/services" ) @@ -70,8 +72,25 @@ func (h *consentHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { consentData, err := h.adminClient.GetConsentRequest( admin.NewGetConsentRequestParams().WithConsentChallenge(challenge)) if err != nil { - h.logger.Error("error getting consent information: %v", err) - http.Error(w, err.Error(), http.StatusInternalServerError) + h.logger.Errorf("error getting consent information: %v", err) + var errorDetails *handlers.ErrorDetails + switch v := err.(type) { + case *admin.GetConsentRequestConflict: + errorDetails = &handlers.ErrorDetails{ + ErrorMessage: *v.Payload.Error, + ErrorDetails: []string{v.Payload.ErrorDescription}, + } + if v.Payload.StatusCode != 0 { + errorDetails.ErrorCode = strconv.Itoa(int(v.Payload.StatusCode)) + } + break + default: + errorDetails = &handlers.ErrorDetails{ + ErrorMessage: "could not get consent details", + ErrorDetails: []string{http.StatusText(http.StatusInternalServerError)}, + } + } + handlers.GetErrorBucket(r).AddError(errorDetails) return } @@ -86,7 +105,11 @@ func (h *consentHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { decoder := form.NewDecoder() if err := decoder.Decode(&consentInfo, r.Form); err != nil { h.logger.Error(err) - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + http.Error( + w, + http.StatusText(http.StatusInternalServerError), + http.StatusInternalServerError, + ) return } @@ -99,8 +122,8 @@ func (h *consentHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { r.Context(), `SELECT email, verified, fname, mname, lname, dob, language, modified FROM users -WHERE uniqueID = ? - AND LOCKED = 0`, +WHERE uniqueid = ? + AND locked = 0`, ) if err != nil { h.logger.Errorf("error preparing user information SQL: %v", err) @@ -181,7 +204,13 @@ WHERE uniqueID = ? } } -func (h *consentHandler) renderConsentForm(w http.ResponseWriter, r *http.Request, consentData *admin.GetConsentRequestOK, err error, localizer *i18n.Localizer) { +func (h *consentHandler) renderConsentForm( + w http.ResponseWriter, + r *http.Request, + consentData *admin.GetConsentRequestOK, + err error, + localizer *i18n.Localizer, +) { trans := func(id string, values ...map[string]interface{}) string { if len(values) > 0 { return h.messageCatalog.LookupMessage(id, values[0], localizer) diff --git a/idp/handlers/login.go b/idp/handlers/login.go index ec9e8e4..a735b7d 100644 --- a/idp/handlers/login.go +++ b/idp/handlers/login.go @@ -7,9 +7,9 @@ import ( "encoding/hex" "html/template" "net/http" + "strconv" "time" - "github.com/go-openapi/runtime" "github.com/go-playground/form/v4" "github.com/go-playground/validator/v10" "github.com/gorilla/csrf" @@ -19,6 +19,7 @@ import ( "github.com/ory/hydra-client-go/models" log "github.com/sirupsen/logrus" + "git.cacert.org/oidc_login/common/handlers" commonServices "git.cacert.org/oidc_login/common/services" "git.cacert.org/oidc_login/idp/services" ) @@ -143,15 +144,33 @@ func (h *loginHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // finish login and redirect to target loginRequest, err := h.adminClient.AcceptLoginRequest( - admin.NewAcceptLoginRequestParams().WithLoginChallenge(challenge).WithBody(&models.AcceptLoginRequest{ - Acr: string(authMethod), - Remember: true, - RememberFor: 0, - Subject: userId, - }).WithTimeout(time.Second * 10)) + admin.NewAcceptLoginRequestParams().WithLoginChallenge(challenge).WithBody( + &models.AcceptLoginRequest{ + Acr: string(authMethod), + Remember: true, + RememberFor: 0, + Subject: userId, + }).WithTimeout(time.Second * 10)) if err != nil { h.logger.Errorf("error getting login request: %#v", err) - http.Error(w, err.Error(), err.(*runtime.APIError).Code) + var errorDetails *handlers.ErrorDetails + switch v := err.(type) { + case *admin.AcceptLoginRequestNotFound: + errorDetails = &handlers.ErrorDetails{ + ErrorMessage: *v.Payload.Error, + ErrorDetails: []string{v.Payload.ErrorDescription}, + } + if v.Payload.StatusCode != 0 { + errorDetails.ErrorCode = strconv.Itoa(int(v.Payload.StatusCode)) + } + break + default: + errorDetails = &handlers.ErrorDetails{ + ErrorMessage: "could not accept login", + ErrorDetails: []string{err.Error()}, + } + } + handlers.GetErrorBucket(r).AddError(errorDetails) return } w.Header().Add("Location", *loginRequest.GetPayload().RedirectTo) @@ -255,7 +274,7 @@ func (h *loginHandler) performCertificateLogin(emails []string, r *http.Request) db := services.GetDb(h.context) query, args, err := sqlx.In( - `SELECT DISTINCT u.uniqueID + `SELECT DISTINCT u.uniqueid FROM users u JOIN email e ON e.memid = u.id WHERE e.email IN (?) diff --git a/templates/app/base.gohtml b/templates/app/base.gohtml index 306c370..aa957bf 100644 --- a/templates/app/base.gohtml +++ b/templates/app/base.gohtml @@ -12,6 +12,7 @@ + @@ -22,10 +23,18 @@ + {{ .Title }} - - {{ template "content" . }} + +
+ {{ template "content" . }} +
+ diff --git a/templates/app/errors.gohtml b/templates/app/errors.gohtml new file mode 100644 index 0000000..7bffd54 --- /dev/null +++ b/templates/app/errors.gohtml @@ -0,0 +1,13 @@ +{{ define "content" }} +
+ CAcert +

{{ .Title }}

+

{{ if .details.ErrorCode }} + {{ .details.ErrorCode }} {{ end }}{{ .details.ErrorMessage }}

+ {{ if .details.ErrorDetails }} + {{ range .details.ErrorDetails }} +

{{ . }}

+ {{ end }} + {{ end }} +
+{{ end }} \ No newline at end of file diff --git a/templates/app/index.gohtml b/templates/app/index.gohtml index 79c581e..adba408 100644 --- a/templates/app/index.gohtml +++ b/templates/app/index.gohtml @@ -1,5 +1,8 @@ {{ define "content" }} -

{{ .Greeting }}

-

{{ .IntroductionText }}

- {{ .LogoutLabel }} +
+ CAcert +

{{ .Greeting }}

+

{{ .IntroductionText }}

+ {{ .LogoutLabel }} +
{{ end }} \ No newline at end of file diff --git a/templates/idp/base.gohtml b/templates/idp/base.gohtml index 5c547cf..3b108d4 100644 --- a/templates/idp/base.gohtml +++ b/templates/idp/base.gohtml @@ -26,8 +26,15 @@ {{ .Title }} - - {{ template "content" . }} + +
+ {{ template "content" . }} +
+ diff --git a/templates/idp/errors.gohtml b/templates/idp/errors.gohtml new file mode 100644 index 0000000..aba7fdb --- /dev/null +++ b/templates/idp/errors.gohtml @@ -0,0 +1,13 @@ +{{ define "content" }} +
+ CAcert +

{{ .Title }}

+

{{ if .details.ErrorCode }} + {{ .details.ErrorCode }} {{ end }}{{ .details.ErrorMessage }}

+ {{ if .details.ErrorDetails }} + {{ range .details.ErrorDetails }} +

{{ . }}

+ {{ end }} + {{ end }} +
+{{ end }} \ No newline at end of file diff --git a/translate.de.toml b/translate.de.toml index 3ff4287..7715597 100644 --- a/translate.de.toml +++ b/translate.de.toml @@ -1,4 +1,3 @@ -[LabelAcceptCertLogin] -description = "Label for a button to accept certificate login" -hash = "sha1-95cf27f4bdee62b51ee8bc673d25a46bcceed452" -other = "Ja, bitte nutze das Zertifikat" +[ErrorTitle] +hash = "sha1-736aec25a98f5ec5b71400bb0163f891f509b566" +other = "Es ist ein Fehler aufgetreten"