From 3a589743141fb98aa09a9f0d8112aa73c821dec7 Mon Sep 17 00:00:00 2001
From: Sofia Papagiannaki <papagian@users.noreply.github.com>
Date: Fri, 9 Aug 2019 09:10:08 +0300
Subject: [PATCH] Backend: Do not set SameSite cookie attribute if
 cookie_samesite is none (#18462)

* Do not set SameSite login_error cookie attribute if cookie_samesite is none

* Do not set SameSite grafana_session cookie attribute if cookie_samesite is none

* Update middleware tests

(cherry picked from commit 4e29357d15dc1ece6ddda0d43fc5139d5c7c27c0)
---
 pkg/api/login.go                  |  9 ++++--
 pkg/middleware/middleware.go      |  4 ++-
 pkg/middleware/middleware_test.go | 46 +++++++++++++++++++------------
 3 files changed, 37 insertions(+), 22 deletions(-)

diff --git a/pkg/api/login.go b/pkg/api/login.go
index 75fefd30232..d561c6b3680 100644
--- a/pkg/api/login.go
+++ b/pkg/api/login.go
@@ -199,15 +199,18 @@ func (hs *HTTPServer) trySetEncryptedCookie(ctx *models.ReqContext, cookieName s
 		return err
 	}
 
-	http.SetCookie(ctx.Resp, &http.Cookie{
+	cookie := http.Cookie{
 		Name:     cookieName,
 		MaxAge:   60,
 		Value:    hex.EncodeToString(encryptedError),
 		HttpOnly: true,
 		Path:     setting.AppSubUrl + "/",
 		Secure:   hs.Cfg.CookieSecure,
-		SameSite: hs.Cfg.CookieSameSite,
-	})
+	}
+	if hs.Cfg.CookieSameSite != http.SameSiteDefaultMode {
+		cookie.SameSite = hs.Cfg.CookieSameSite
+	}
+	http.SetCookie(ctx.Resp, &cookie)
 
 	return nil
 }
diff --git a/pkg/middleware/middleware.go b/pkg/middleware/middleware.go
index 2bb0f8a49d5..ded7bc4018f 100644
--- a/pkg/middleware/middleware.go
+++ b/pkg/middleware/middleware.go
@@ -267,7 +267,9 @@ func WriteSessionCookie(ctx *models.ReqContext, value string, maxLifetimeDays in
 		Path:     setting.AppSubUrl + "/",
 		Secure:   setting.CookieSecure,
 		MaxAge:   maxAge,
-		SameSite: setting.CookieSameSite,
+	}
+	if setting.CookieSameSite != http.SameSiteDefaultMode {
+		cookie.SameSite = setting.CookieSameSite
 	}
 
 	http.SetCookie(ctx.Resp, &cookie)
diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go
index c8213ff2e6b..c40d73cb02a 100644
--- a/pkg/middleware/middleware_test.go
+++ b/pkg/middleware/middleware_test.go
@@ -306,28 +306,38 @@ func TestMiddlewareContext(t *testing.T) {
 			maxAgeHours := (time.Duration(setting.LoginMaxLifetimeDays) * 24 * time.Hour)
 			maxAge := (maxAgeHours + time.Hour).Seconds()
 
-			expectedCookie := &http.Cookie{
-				Name:     setting.LoginCookieName,
-				Value:    "rotated",
-				Path:     setting.AppSubUrl + "/",
-				HttpOnly: true,
-				MaxAge:   int(maxAge),
-				Secure:   setting.CookieSecure,
-				SameSite: setting.CookieSameSite,
+			sameSitePolicies := []http.SameSite{
+				http.SameSiteDefaultMode,
+				http.SameSiteLaxMode,
+				http.SameSiteStrictMode,
 			}
+			for _, sameSitePolicy := range sameSitePolicies {
+				setting.CookieSameSite = sameSitePolicy
+				expectedCookie := &http.Cookie{
+					Name:     setting.LoginCookieName,
+					Value:    "rotated",
+					Path:     setting.AppSubUrl + "/",
+					HttpOnly: true,
+					MaxAge:   int(maxAge),
+					Secure:   setting.CookieSecure,
+				}
+				if sameSitePolicy != http.SameSiteDefaultMode {
+					expectedCookie.SameSite = sameSitePolicy
+				}
 
-			sc.fakeReq("GET", "/").exec()
+				sc.fakeReq("GET", "/").exec()
 
-			Convey("should init context with user info", func() {
-				So(sc.context.IsSignedIn, ShouldBeTrue)
-				So(sc.context.UserId, ShouldEqual, 12)
-				So(sc.context.UserToken.UserId, ShouldEqual, 12)
-				So(sc.context.UserToken.UnhashedToken, ShouldEqual, "rotated")
-			})
+				Convey(fmt.Sprintf("Should init context with user info and setting.SameSite=%v", sameSitePolicy), func() {
+					So(sc.context.IsSignedIn, ShouldBeTrue)
+					So(sc.context.UserId, ShouldEqual, 12)
+					So(sc.context.UserToken.UserId, ShouldEqual, 12)
+					So(sc.context.UserToken.UnhashedToken, ShouldEqual, "rotated")
+				})
 
-			Convey("should set cookie", func() {
-				So(sc.resp.Header().Get("Set-Cookie"), ShouldEqual, expectedCookie.String())
-			})
+				Convey(fmt.Sprintf("Should set cookie with setting.SameSite=%v", sameSitePolicy), func() {
+					So(sc.resp.Header().Get("Set-Cookie"), ShouldEqual, expectedCookie.String())
+				})
+			}
 		})
 
 		middlewareScenario(t, "Invalid/expired auth token in cookie", func(sc *scenarioContext) {