diff --git a/app/routes.go b/app/routes.go index c604f16..9e4a20b 100644 --- a/app/routes.go +++ b/app/routes.go @@ -92,11 +92,8 @@ func (a *App) SetupRoutes() error { manage.POST("/users/:id/update", handlers.UpdateUser()) manage.POST("/users/:id/delete", handlers.DeleteUser()) - e.GET("/logout", handlers.Logout()) + e.GET("/logout", handlers.Logout(), xsrf) e.POST("/logout", handlers.Logout(), handlers.MiddlewareSession, xsrf) - // administrative endpoints. - e.GET("/admin/*", handlers.Admin()) - return nil } diff --git a/app/server.go b/app/server.go index 0213af4..55af6f1 100644 --- a/app/server.go +++ b/app/server.go @@ -130,19 +130,13 @@ func (a *App) SetServerSettings() { } store.Options.Path = "/" - store.Options.Domain = a.setting.HTTPDomain() + // let the domain be set automatically based on where the app is running. + // store.Options.Domain = a.setting.HTTPDomain() store.Options.HttpOnly = true store.Options.SameSite = http.SameSiteStrictMode store.Options.Secure = a.setting.HTTPSecure() store.Options.MaxAge = a.setting.SessionMaxAge() - if a.setting.HTTPSecure() { - // https://www.sjoerdlangkemper.nl/2017/02/09/cookie-prefixes/ - // https://scotthelme.co.uk/tough-cookies/ - // https://check-your-website.server-daten.de/prefix-cookies.html - store.Options.Domain = "__Host-" + store.Options.Domain - } - e.Use(session.Middleware(store)) e.Use(middleware.Secure()) diff --git a/app/settings/helper.go b/app/settings/helper.go index 60570b6..8d7ecb8 100644 --- a/app/settings/helper.go +++ b/app/settings/helper.go @@ -3,7 +3,11 @@ package settings -import "flag" +import ( + "flag" + + "git.dotya.ml/mirre-mt/pcmt/config" +) // as per https://stackoverflow.com/a/54747682. func isFlagPassed(name string) bool { @@ -17,3 +21,35 @@ func isFlagPassed(name string) bool { return found } + +// sortOutFlags checks whether any flag overrides were passed and their validity. +func (s *Settings) sortOutFlags(conf *config.Config, hostFlag *string, portFlag *int, develFlag *bool) { + log.Debug("checking flag overrides") + + overrideMsg := "overriding '%s' based on a flag: %+v" + + if isFlagPassed("host") { + if h := *hostFlag; h != "unset" && h != conf.Host { + log.Debugf(overrideMsg, "host", h) + s.SetHost(h) + } + } + + if isFlagPassed("port") { + if p := *portFlag; p > 0 && p < 65536 { + if p != conf.Port { + log.Debugf(overrideMsg, "port", p) + s.SetPort(p) + } + } else { + log.Warnf("flag-supplied port '%d' outside of bounds, ignoring", p) + } + } + + if isFlagPassed("devel") { + if d := *develFlag; d != conf.DevelMode { + log.Debugf(overrideMsg, "develMode", d) + s.SetIsDevel(d) + } + } +} diff --git a/app/settings/settings.go b/app/settings/settings.go index cbaf0a2..287ae7d 100644 --- a/app/settings/settings.go +++ b/app/settings/settings.go @@ -114,8 +114,17 @@ func (s *Settings) Consolidate(conf *config.Config, host *string, port *int, dev s.SetIsLive(conf.LiveMode) s.SetIsDevel(conf.DevelMode) s.SetLoggerIsJSON(conf.Logger.JSON) + + if conf.HTTP.Secure { + // https://www.sjoerdlangkemper.nl/2017/02/09/cookie-prefixes/ + // https://scotthelme.co.uk/tough-cookies/ + // https://check-your-website.server-daten.de/prefix-cookies.html + s.SetSessionCookieName("__Host-" + conf.Session.CookieName) + } else { + s.SetSessionCookieName(conf.Session.CookieName) + } + s.SetSessionMaxAge(conf.Session.MaxAge) - s.SetSessionCookieName(conf.Session.CookieName) s.SetSessionCookieAuthSecret(conf.Session.CookieAuthSecret) s.SetSessionCookieEncrSecret(conf.Session.CookieEncrSecret) @@ -144,44 +153,14 @@ func (s *Settings) Consolidate(conf *config.Config, host *string, port *int, dev s.SetInitAdminPassword(conf.Init.AdminPassword) } - s.SetHTTPDomain(conf.HTTP.Domain) - s.SetHTTPSecure(conf.HTTP.Secure) - - log.Debug("checking flag overrides") - - overrideMsg := "overriding '%s' based on a flag: %+v" - - if isFlagPassed("host") { - if h := *host; h != "unset" && h != conf.Host { - log.Debugf(overrideMsg, "host", h) - s.SetHost(h) - } - } - - if isFlagPassed("port") { - if p := *port; p > 0 && p < 65536 { - if p != conf.Port { - log.Debugf(overrideMsg, "port", p) - s.SetPort(p) - } - } else { - log.Warnf("flag-supplied port '%d' outside of bounds, ignoring", p) - } - } - - if isFlagPassed("devel") { - if d := *devel; d != conf.DevelMode { - log.Debugf(overrideMsg, "develMode", d) - s.SetIsDevel(d) - } - } - if conf.Registration.Allowed { s.RegistrationAllowed = true } + s.SetHTTPDomain(conf.HTTP.Domain) + s.SetHTTPSecure(conf.HTTP.Secure) s.setAPIKeys() - + s.sortOutFlags(conf, host, port, devel) s.SetVersion(version) } diff --git a/handlers/error.go b/handlers/error.go index fd0dfa6..b7729b1 100644 --- a/handlers/error.go +++ b/handlers/error.go @@ -18,7 +18,7 @@ var ( ) func renderErrorPage(c echo.Context, status int, statusText, error string) error { - addHeaders(c) + defer addHeaders(c) strStatus := strconv.Itoa(status) diff --git a/handlers/handlers.go b/handlers/handlers.go index 3ccfdb6..f64651c 100644 --- a/handlers/handlers.go +++ b/handlers/handlers.go @@ -10,20 +10,15 @@ import ( "github.com/labstack/echo/v4" ) -func Admin() echo.HandlerFunc { - return func(c echo.Context) error { - return echo.NewHTTPError(http.StatusUnauthorized, "Invalid credentials") - } -} - func Index() echo.HandlerFunc { return func(c echo.Context) error { - addHeaders(c) + defer addHeaders(c) if sess, _ := session.Get(setting.SessionCookieName(), c); sess != nil { - username := sess.Values["username"] - if username != nil { - return c.Redirect(http.StatusFound, "/home") + if uname, ok := sess.Values["username"].(string); ok { + if uname != "" { + return c.Redirect(http.StatusFound, "/home") + } } } @@ -62,7 +57,7 @@ func Healthz() echo.HandlerFunc { } func addHeaders(c echo.Context) { - c.Response().Writer.Header().Set("Cross-Origin-Opener-Policy", "same-origin") + c.Response().Header().Set("Cross-Origin-Opener-Policy", "same-origin") } // experimental global redirect handler? diff --git a/handlers/home.go b/handlers/home.go index 6b4b5ed..bdb08af 100644 --- a/handlers/home.go +++ b/handlers/home.go @@ -15,9 +15,7 @@ import ( func Home(client *ent.Client) echo.HandlerFunc { return func(c echo.Context) error { - var username string - - addHeaders(c) + defer addHeaders(c) sess, _ := session.Get(setting.SessionCookieName(), c) if sess == nil { @@ -25,25 +23,15 @@ func Home(client *ent.Client) echo.HandlerFunc { return c.Redirect(http.StatusSeeOther, "/signin") } - if sess.Values["foo"] != nil { - log.Info("gorilla session", "custom field test", sess.Values["foo"].(string)) - } + var username string - uname := sess.Values["username"] - if uname == nil { + username, ok := sess.Values["username"].(string) + if !ok { log.Info("session cookie found but username invalid, redirecting to signin", "endpoint", "/home") - return c.Redirect(http.StatusSeeOther, "/signin") } - log.Debug("session", "username", sess.Values["username"].(string), "endpoint", "/home") - username = sess.Values["username"].(string) - - // example denial. - // if _, err := c.Cookie("aha"); err != nil { - // log.Printf("error: %q", err) - // return echo.NewHTTPError(http.StatusUnauthorized, http.StatusText(http.StatusUnauthorized)) - // } + log.Debug("session", "username", username, "endpoint", "/home") var u moduser.User @@ -102,13 +90,21 @@ func Home(client *ent.Client) echo.HandlerFunc { if _, ok := sess.Values["reauthFlash"].(string); ok { p.Data["info"] = "First time after changing the password, yay!" - // if this is the first login after the initial password change, delete // the cookie value. delete(sess.Values, "reauthFlash") } - _ = sess.Save(c.Request(), c.Response()) + if err := sess.Save(c.Request(), c.Response()); err != nil { + log.Error("Failed to save session", "module", "handlers/home") + + return renderErrorPage( + c, + http.StatusInternalServerError, + http.StatusText(http.StatusInternalServerError)+" (make sure you've got cookies enabled)", + err.Error(), + ) + } err := c.Render(http.StatusOK, "home.tmpl", p) if err != nil { diff --git a/handlers/logout.go b/handlers/logout.go index 23f49be..51550c4 100644 --- a/handlers/logout.go +++ b/handlers/logout.go @@ -12,7 +12,7 @@ import ( func Logout() echo.HandlerFunc { return func(c echo.Context) error { - addHeaders(c) + defer addHeaders(c) switch { case c.Request().Method == "POST": // nolint:goconst @@ -21,16 +21,21 @@ func Logout() echo.HandlerFunc { log.Infof("max-age before logout: %d", sess.Options.MaxAge) sess.Options.MaxAge = -1 - c.Response().Writer.Header().Set("Cache-Control", "no-store") - - if username := sess.Values["username"]; username != nil && username.(string) != "" { - sess.Values["username"] = "" - } + delete(sess.Values, "username") err := sess.Save(c.Request(), c.Response()) if err != nil { c.Logger().Error("could not delete session cookie") + + return renderErrorPage( + c, + http.StatusInternalServerError, + http.StatusText(http.StatusInternalServerError), + err.Error(), + ) } + + c.Response().Header().Set(echo.HeaderCacheControl, "no-store") } return c.Redirect(http.StatusMovedPermanently, "/logout") @@ -38,8 +43,10 @@ func Logout() echo.HandlerFunc { case c.Request().Method == "GET": // nolint:goconst sess, _ := session.Get(setting.SessionCookieName(), c) if sess != nil { - if username := sess.Values["username"]; username != nil { - return c.Redirect(http.StatusSeeOther, "/home") + if uname, ok := sess.Values["username"].(string); ok { + if uname != "" { + return c.Redirect(http.StatusSeeOther, "/home") + } } } diff --git a/handlers/middleware.go b/handlers/middleware.go index 07ee48c..aca9cc0 100644 --- a/handlers/middleware.go +++ b/handlers/middleware.go @@ -18,6 +18,11 @@ import ( func MiddlewareSession(next echo.HandlerFunc) echo.HandlerFunc { return func(c echo.Context) error { + if c.Request().URL.Path == "/logout" && c.Request().Method == "POST" { + log.Debug("skipping auth middleware on /logout POST", "module", "handlers/middleware") + return next(c) + } + sess, _ := session.Get(setting.SessionCookieName(), c) if sess == nil { @@ -29,27 +34,21 @@ func MiddlewareSession(next echo.HandlerFunc) echo.HandlerFunc { ) } - var username string - - // uname, ok := sess.Values["username"].(string) - uname := sess.Values["username"] - - if uname != nil { - username = uname.(string) - - // nolint:goconst - log.Info("Refreshing session cookie", "username", username, + if username, ok := sess.Values["username"].(string); ok { + log.Info("Refreshing session cookie", + "username", username, "module", "middleware", "maxAge", setting.SessionMaxAge(), - "secure", c.Request().URL.Scheme == "https", - "domain", setting.HTTPDomain) + "secure", setting.HTTPSecure(), + "domain", setting.HTTPDomain(), + ) refreshSession( sess, "/", setting.SessionMaxAge(), true, - c.Request().URL.Scheme == "https", // nolint:goconst + setting.HTTPSecure(), http.SameSiteStrictMode, ) @@ -95,10 +94,24 @@ func MiddlewareSession(next echo.HandlerFunc) echo.HandlerFunc { return next(c) } - log.Warn("Could not get username from the cookie") + log.Warn("Could not get username from the cookie", "module", "handlers/middleware") if !sess.IsNew { - log.Errorf("%d - %s", http.StatusUnauthorized, "you need to re-login") + log.Warn("Expired session cookie (without a username) found, redirecting to sign in", "module", "handlers/middleware") + + sess.Values["info"] = "Log in again, please." + + if err := sess.Save(c.Request(), c.Response()); err != nil { + log.Error("Failed to save session", "module", "middleware") + + return renderErrorPage( + c, + http.StatusInternalServerError, + http.StatusText(http.StatusInternalServerError)+" could not save the session cookie", + err.Error(), + ) + } + return c.Redirect(http.StatusTemporaryRedirect, "/signin") } diff --git a/handlers/signin.go b/handlers/signin.go index f668530..7bafed2 100644 --- a/handlers/signin.go +++ b/handlers/signin.go @@ -19,7 +19,7 @@ import ( func Signin() echo.HandlerFunc { return func(c echo.Context) error { - addHeaders(c) + defer addHeaders(c) reauth := false r := c.QueryParam("reauth") @@ -28,23 +28,7 @@ func Signin() echo.HandlerFunc { reauth = true } - // slog.Info("XXX session cookie name:", setting.SessionCookieName()) - sess, _ := session.Get(setting.SessionCookieName(), c) - - var uf *userSignin - - if sess != nil { - username := sess.Values["username"] - if username != nil { - if !reauth { - return c.Redirect(http.StatusPermanentRedirect, "/home") - } - - uf = &userSignin{Username: username.(string)} - } - } - csrf := c.Get("csrf").(string) p := newPage() @@ -52,16 +36,45 @@ func Signin() echo.HandlerFunc { p.Current = "signin" p.CSRF = csrf + var uf *userSignin + + if sess != nil { + if uname, ok := sess.Values["username"].(string); ok && uname != "" { + if !reauth { + return c.Redirect(http.StatusPermanentRedirect, "/home") + } + + uf = &userSignin{Username: uname} + } + } + if reauth { fl := sess.Values["reauthFlash"] - if _, ok := fl.(string); !ok { - p.Data["flash"] = "re-login please" + if reFl, ok := fl.(string); !ok { + p.Data["info"] = "re-login, please." } else { - p.Data["reauthFlash"] = fl.(string) + p.Data["reauthFlash"] = reFl if uf != nil { p.Data["form"] = uf } } + } else { + if i, ok := sess.Values["info"].(string); ok { + p.Data["infoFlash"] = i + + delete(sess.Values, "info") + + if err := sess.Save(c.Request(), c.Response()); err != nil { + log.Error("Failed to save session", "module", "middleware") + + return renderErrorPage( + c, + http.StatusInternalServerError, + http.StatusText(http.StatusInternalServerError)+" could not save the session cookie", + err.Error(), + ) + } + } } return c.Render( @@ -74,7 +87,7 @@ func Signin() echo.HandlerFunc { func SigninPost(client *ent.Client) echo.HandlerFunc { return func(c echo.Context) error { - addHeaders(c) + defer addHeaders(c) cu := new(userSignin) if err := c.Bind(cu); err != nil { @@ -154,18 +167,15 @@ func SigninPost(client *ent.Client) echo.HandlerFunc { ) } - secure := c.Request().URL.Scheme == "https" //nolint:goconst - sess, _ := session.Get(setting.SessionCookieName(), c) if sess != nil { sess.Options = &sessions.Options{ Path: "/", MaxAge: setting.SessionMaxAge(), HttpOnly: true, - Secure: secure, + Secure: setting.HTTPSecure(), SameSite: http.SameSiteStrictMode, } - sess.Values["foo"] = "bar" c.Logger().Debug("saving username to the session cookie") diff --git a/handlers/signup.go b/handlers/signup.go index 11a3993..6248168 100644 --- a/handlers/signup.go +++ b/handlers/signup.go @@ -17,34 +17,16 @@ import ( func Signup() echo.HandlerFunc { return func(c echo.Context) error { - addHeaders(c) + defer addHeaders(c) sess, _ := session.Get(setting.SessionCookieName(), c) if sess != nil { - log.Info("gorilla session", "endpoint", "signup") - - username := sess.Values["username"] - if username != nil { + if uname, ok := sess.Values["username"].(string); ok && uname != "" { return c.Redirect(http.StatusFound, "/home") } } - // tpl := getTmpl("signup.tmpl") - csrf := c.Get("csrf").(string) - - // secure := c.Request().URL.Scheme == "https" - // cookieCSRF := &http.Cookie{ - // Name: "_csrf", - // Value: csrf, - // // SameSite: http.SameSiteStrictMode, - // SameSite: http.SameSiteLaxMode, - // MaxAge: 3600, - // Secure: secure, - // HttpOnly: true, - // } - // c.SetCookie(cookieCSRF) - p := newPage() p.Title = "Sign up" @@ -73,7 +55,7 @@ func Signup() echo.HandlerFunc { func SignupPost(client *ent.Client) echo.HandlerFunc { return func(c echo.Context) error { - addHeaders(c) + defer addHeaders(c) cu := new(userSignup) if err := c.Bind(cu); err != nil { @@ -140,17 +122,14 @@ func SignupPost(client *ent.Client) echo.HandlerFunc { log.Infof("successfully registered user '%s'", u.Username) log.Debug("user details", "id", u.ID, "email", u.Email, "isAdmin", u.IsAdmin) - secure := c.Request().URL.Scheme == "https" //nolint:goconst - sess, _ := session.Get(setting.SessionCookieName(), c) sess.Options = &sessions.Options{ Path: "/", MaxAge: setting.SessionMaxAge(), HttpOnly: true, - Secure: secure, + Secure: setting.HTTPSecure(), SameSite: http.SameSiteStrictMode, } - sess.Values["foo"] = "bar" sess.Values["username"] = username err = sess.Save(c.Request(), c.Response()) diff --git a/templates/signin.tmpl b/templates/signin.tmpl index 1c0b7e8..0457beb 100644 --- a/templates/signin.tmpl +++ b/templates/signin.tmpl @@ -25,6 +25,13 @@

Success: {{.Data.reauthFlash}}

{{- else -}}{{end}} + {{ if and .Data .Data.infoFlash }} +
+

+ {{.Data.infoFlash}} +

+
+ {{- else -}}{{end}}