Make cookies HttpOnly and obey COOKIE_SECURE flag (#4707)

SagePtr · techknowlogick · commit 052aa54b2b92 · 2018-08-14T16:19:20.000-04:00
diff --git 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
@@ -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
@@ -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
@@ -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"))

Original file line number	Diff line number	Diff line change
`@@ -44,13 +44,13 @@ func SignInOpenID(ctx *context.Context) {`
`44`	`44`
`45`	`45`	`redirectTo := ctx.Query("redirect_to")`
`46`	`46`	`if len(redirectTo) > 0 {`
`47`		`- ctx.SetCookie("redirect_to", redirectTo, 0, setting.AppSubURL)`
	`47`	`+ ctx.SetCookie("redirect_to", redirectTo, 0, setting.AppSubURL, "", setting.SessionConfig.Secure, true)`
`48`	`48`	`} else {`
`49`	`49`	`redirectTo, _ = url.QueryUnescape(ctx.GetCookie("redirect_to"))`
`50`	`50`	`}`
`51`	`51`
`52`	`52`	`if isSucceed {`
`53`		`- ctx.SetCookie("redirect_to", "", -1, setting.AppSubURL)`
	`53`	`+ ctx.SetCookie("redirect_to", "", -1, setting.AppSubURL, "", setting.SessionConfig.Secure, true)`
`54`	`54`	`ctx.RedirectToFirst(redirectTo)`
`55`	`55`	`return`
`56`	`56`	`}`
Original file line number	Diff line number	Diff line change
`@@ -103,7 +103,7 @@ func ProfilePost(ctx *context.Context, form auth.UpdateProfileForm) {`
`103`	`103`	`}`
`104`	`104`
`105`	`105`	`// Update the language to the one we just set`
`106`		`- ctx.SetCookie("lang", ctx.User.Language, nil, setting.AppSubURL)`
	`106`	`+ ctx.SetCookie("lang", ctx.User.Language, nil, setting.AppSubURL, "", setting.SessionConfig.Secure, true)`
`107`	`107`
`108`	`108`	`log.Trace("User settings updated: %s", ctx.User.Name)`
`109`	`109`	`ctx.Flash.Success(i18n.Tr(ctx.User.Language, "settings.update_profile_success"))`