From 052aa54b2b92a67639c6f152a811af5a36fbcb6e Mon Sep 17 00:00:00 2001 From: SagePtr Date: Tue, 14 Aug 2018 22:19:20 +0200 Subject: [PATCH] Make cookies HttpOnly and obey COOKIE_SECURE flag (#4707) --- routers/routes/routes.go | 13 +++++++------ routers/user/auth.go | 32 ++++++++++++++++---------------- routers/user/auth_openid.go | 4 ++-- routers/user/setting/profile.go | 2 +- 4 files changed, 26 insertions(+), 25 deletions(-) diff --git a/routers/routes/routes.go b/routers/routes/routes.go index 1eefbf1b6..48813dc2a 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -116,12 +116,13 @@ func NewMacaron() *macaron.Macaron { })) m.Use(session.Sessioner(setting.SessionConfig)) m.Use(csrf.Csrfer(csrf.Options{ - Secret: setting.SecretKey, - Cookie: setting.CSRFCookieName, - SetCookie: true, - Secure: setting.SessionConfig.Secure, - Header: "X-Csrf-Token", - CookiePath: setting.AppSubURL, + Secret: setting.SecretKey, + Cookie: setting.CSRFCookieName, + SetCookie: true, + Secure: setting.SessionConfig.Secure, + CookieHttpOnly: true, + Header: "X-Csrf-Token", + CookiePath: setting.AppSubURL, })) m.Use(toolbox.Toolboxer(m, toolbox.Options{ HealthCheckFuncs: []*toolbox.HealthCheckFuncDesc{ diff --git a/routers/user/auth.go b/routers/user/auth.go index f7dbaca32..706dc6c14 100644 --- a/routers/user/auth.go +++ b/routers/user/auth.go @@ -56,8 +56,8 @@ func AutoSignIn(ctx *context.Context) (bool, error) { defer func() { if !isSucceed { log.Trace("auto-login cookie cleared: %s", uname) - ctx.SetCookie(setting.CookieUserName, "", -1, setting.AppSubURL) - ctx.SetCookie(setting.CookieRememberName, "", -1, setting.AppSubURL) + ctx.SetCookie(setting.CookieUserName, "", -1, setting.AppSubURL, "", setting.SessionConfig.Secure, true) + ctx.SetCookie(setting.CookieRememberName, "", -1, setting.AppSubURL, "", setting.SessionConfig.Secure, true) } }() @@ -77,7 +77,7 @@ func AutoSignIn(ctx *context.Context) (bool, error) { isSucceed = true ctx.Session.Set("uid", u.ID) ctx.Session.Set("uname", u.Name) - ctx.SetCookie(setting.CSRFCookieName, "", -1, setting.AppSubURL) + ctx.SetCookie(setting.CSRFCookieName, "", -1, setting.AppSubURL, "", setting.SessionConfig.Secure, true) return true, nil } @@ -91,13 +91,13 @@ func checkAutoLogin(ctx *context.Context) bool { redirectTo := ctx.Query("redirect_to") if len(redirectTo) > 0 { - ctx.SetCookie("redirect_to", redirectTo, 0, setting.AppSubURL) + ctx.SetCookie("redirect_to", redirectTo, 0, setting.AppSubURL, "", setting.SessionConfig.Secure, true) } else { redirectTo, _ = url.QueryUnescape(ctx.GetCookie("redirect_to")) } if isSucceed { - ctx.SetCookie("redirect_to", "", -1, setting.AppSubURL) + ctx.SetCookie("redirect_to", "", -1, setting.AppSubURL, "", setting.SessionConfig.Secure, true) ctx.RedirectToFirst(redirectTo, setting.AppSubURL+string(setting.LandingPageURL)) return true } @@ -438,9 +438,9 @@ func handleSignIn(ctx *context.Context, u *models.User, remember bool) { func handleSignInFull(ctx *context.Context, u *models.User, remember bool, obeyRedirect bool) string { if remember { days := 86400 * setting.LogInRememberDays - ctx.SetCookie(setting.CookieUserName, u.Name, days, setting.AppSubURL) + ctx.SetCookie(setting.CookieUserName, u.Name, days, setting.AppSubURL, "", setting.SessionConfig.Secure, true) ctx.SetSuperSecureCookie(base.EncodeMD5(u.Rands+u.Passwd), - setting.CookieRememberName, u.Name, days, setting.AppSubURL) + setting.CookieRememberName, u.Name, days, setting.AppSubURL, "", setting.SessionConfig.Secure, true) } ctx.Session.Delete("openid_verified_uri") @@ -464,10 +464,10 @@ func handleSignInFull(ctx *context.Context, u *models.User, remember bool, obeyR } } - ctx.SetCookie("lang", u.Language, nil, setting.AppSubURL) + ctx.SetCookie("lang", u.Language, nil, setting.AppSubURL, "", setting.SessionConfig.Secure, true) // Clear whatever CSRF has right now, force to generate a new one - ctx.SetCookie(setting.CSRFCookieName, "", -1, setting.AppSubURL) + ctx.SetCookie(setting.CSRFCookieName, "", -1, setting.AppSubURL, "", setting.SessionConfig.Secure, true) // Register last login u.SetLastLogin() @@ -477,7 +477,7 @@ func handleSignInFull(ctx *context.Context, u *models.User, remember bool, obeyR } if redirectTo, _ := url.QueryUnescape(ctx.GetCookie("redirect_to")); len(redirectTo) > 0 && !util.IsExternalURL(redirectTo) { - ctx.SetCookie("redirect_to", "", -1, setting.AppSubURL) + ctx.SetCookie("redirect_to", "", -1, setting.AppSubURL, "", setting.SessionConfig.Secure, true) if obeyRedirect { ctx.RedirectToFirst(redirectTo) } @@ -558,7 +558,7 @@ func handleOAuth2SignIn(u *models.User, gothUser goth.User, ctx *context.Context ctx.Session.Set("uname", u.Name) // Clear whatever CSRF has right now, force to generate a new one - ctx.SetCookie(setting.CSRFCookieName, "", -1, setting.AppSubURL) + ctx.SetCookie(setting.CSRFCookieName, "", -1, setting.AppSubURL, "", setting.SessionConfig.Secure, true) // Register last login u.SetLastLogin() @@ -568,7 +568,7 @@ func handleOAuth2SignIn(u *models.User, gothUser goth.User, ctx *context.Context } if redirectTo, _ := url.QueryUnescape(ctx.GetCookie("redirect_to")); len(redirectTo) > 0 { - ctx.SetCookie("redirect_to", "", -1, setting.AppSubURL) + ctx.SetCookie("redirect_to", "", -1, setting.AppSubURL, "", setting.SessionConfig.Secure, true) ctx.RedirectToFirst(redirectTo) return } @@ -844,10 +844,10 @@ func SignOut(ctx *context.Context) { ctx.Session.Delete("socialId") ctx.Session.Delete("socialName") ctx.Session.Delete("socialEmail") - ctx.SetCookie(setting.CookieUserName, "", -1, setting.AppSubURL) - ctx.SetCookie(setting.CookieRememberName, "", -1, setting.AppSubURL) - ctx.SetCookie(setting.CSRFCookieName, "", -1, setting.AppSubURL) - ctx.SetCookie("lang", "", -1, setting.AppSubURL) // Setting the lang cookie will trigger the middleware to reset the language ot previous state. + ctx.SetCookie(setting.CookieUserName, "", -1, setting.AppSubURL, "", setting.SessionConfig.Secure, true) + ctx.SetCookie(setting.CookieRememberName, "", -1, setting.AppSubURL, "", setting.SessionConfig.Secure, true) + ctx.SetCookie(setting.CSRFCookieName, "", -1, setting.AppSubURL, "", setting.SessionConfig.Secure, true) + ctx.SetCookie("lang", "", -1, setting.AppSubURL, "", setting.SessionConfig.Secure, true) // Setting the lang cookie will trigger the middleware to reset the language ot previous state. ctx.Redirect(setting.AppSubURL + "/") } diff --git a/routers/user/auth_openid.go b/routers/user/auth_openid.go index 9fe3424aa..f697c16e7 100644 --- a/routers/user/auth_openid.go +++ b/routers/user/auth_openid.go @@ -44,13 +44,13 @@ func SignInOpenID(ctx *context.Context) { redirectTo := ctx.Query("redirect_to") if len(redirectTo) > 0 { - ctx.SetCookie("redirect_to", redirectTo, 0, setting.AppSubURL) + ctx.SetCookie("redirect_to", redirectTo, 0, setting.AppSubURL, "", setting.SessionConfig.Secure, true) } else { redirectTo, _ = url.QueryUnescape(ctx.GetCookie("redirect_to")) } if isSucceed { - ctx.SetCookie("redirect_to", "", -1, setting.AppSubURL) + ctx.SetCookie("redirect_to", "", -1, setting.AppSubURL, "", setting.SessionConfig.Secure, true) ctx.RedirectToFirst(redirectTo) return } diff --git a/routers/user/setting/profile.go b/routers/user/setting/profile.go index 6a2eacd09..09073498f 100644 --- a/routers/user/setting/profile.go +++ b/routers/user/setting/profile.go @@ -103,7 +103,7 @@ func ProfilePost(ctx *context.Context, form auth.UpdateProfileForm) { } // Update the language to the one we just set - ctx.SetCookie("lang", ctx.User.Language, nil, setting.AppSubURL) + ctx.SetCookie("lang", ctx.User.Language, nil, setting.AppSubURL, "", setting.SessionConfig.Secure, true) log.Trace("User settings updated: %s", ctx.User.Name) ctx.Flash.Success(i18n.Tr(ctx.User.Language, "settings.update_profile_success"))