From 79c60d0e4fcf1fbc9653c1cb13d28e82248cf43c Mon Sep 17 00:00:00 2001
From: Matt Silverlock <matt@eatsleeprepeat.net>
Date: Sun, 26 Apr 2020 10:13:33 -0700
Subject: [PATCH] fix: Set SameSite=Lax by default (#136)

* change: set SameSite=Lax by default
* deps: update errors to v0.9.1
* build: add go 1.13, go 1.14
* docs: update SameSiteDefaultMode godoc
---
 .circleci/config.yml | 12 ++++++++++++
 csrf.go              |  7 ++++---
 go.mod               |  2 +-
 go.sum               |  2 ++
 options.go           |  4 ++++
 store_test.go        | 13 +++++++------
 6 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/.circleci/config.yml b/.circleci/config.yml
index a4cd11e..c2eea3a 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -23,6 +23,16 @@ jobs:
       LATEST: "true"
       GO111MODULE: "on"
 
+  "1.14":
+    <<: *test
+    docker:
+      - image: circleci/golang:1.14
+
+  "1.13":
+    <<: *test
+    docker:
+      - image: circleci/golang:1.13
+
   "1.12":
     <<: *test
     docker:
@@ -58,6 +68,8 @@ workflows:
   build:
     jobs:
       - "latest"
+      - "1.14"
+      - "1.13"
       - "1.12"
       - "1.11"
       - "1.10"
diff --git a/csrf.go b/csrf.go
index 1102b3d..f21e0a2 100644
--- a/csrf.go
+++ b/csrf.go
@@ -62,9 +62,10 @@ type SameSiteMode int
 
 // SameSite options
 const (
-	// SameSiteDefaultMode sets an invalid SameSite header which defaults to
-	// 'Lax' in most browsers, but may cause some browsers to ignore the cookie
-	// entirely.
+	// SameSiteDefaultMode sets the `SameSite` cookie attribute, which is
+	// invalid in some older browsers due to changes in the SameSite spec. These
+	// browsers will not send the cookie to the server.
+	// csrf uses SameSiteLaxMode (SameSite=Lax) as the default as of v1.7.0+
 	SameSiteDefaultMode SameSiteMode = iota + 1
 	SameSiteLaxMode
 	SameSiteStrictMode
diff --git a/go.mod b/go.mod
index ae06dfb..23a5c6e 100644
--- a/go.mod
+++ b/go.mod
@@ -2,7 +2,7 @@ module github.com/gorilla/csrf
 
 require (
 	github.com/gorilla/securecookie v1.1.1
-	github.com/pkg/errors v0.8.0
+	github.com/pkg/errors v0.9.1
 )
 
 go 1.13
diff --git a/go.sum b/go.sum
index 2d11d99..ff4965c 100644
--- a/go.sum
+++ b/go.sum
@@ -2,3 +2,5 @@ github.com/gorilla/securecookie v1.1.1 h1:miw7JPhV+b/lAHSXz4qd/nN9jRiAFV5FwjeKyC
 github.com/gorilla/securecookie v1.1.1/go.mod h1:ra0sb63/xPlUeL+yeDciTfxMRAA+MP+HVt/4epWDjd4=
 github.com/pkg/errors v0.8.0 h1:WdK/asTD0HN+q6hsWO3/vpuAkAr+tw6aNJNDFFf0+qw=
 github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
+github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
+github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
diff --git a/options.go b/options.go
index 0205f5f..c61d301 100644
--- a/options.go
+++ b/options.go
@@ -152,6 +152,10 @@ func parseOptions(h http.Handler, opts ...Option) *csrf {
 	cs.opts.Secure = true
 	cs.opts.HttpOnly = true
 
+	// Set SameSite=Lax by default, allowing the CSRF cookie to only be sent on
+	// top-level navigations.
+	cs.opts.SameSite = SameSiteLaxMode
+
 	// Default; only override this if the package user explicitly calls MaxAge(0)
 	cs.opts.MaxAge = defaultAge
 
diff --git a/store_test.go b/store_test.go
index 7fe6913..27e6157 100644
--- a/store_test.go
+++ b/store_test.go
@@ -160,9 +160,9 @@ func TestSameSizeSet(t *testing.T) {
 	}
 }
 
-// TestSamesiteBackwardsCompat tests that the default set of options do not set
-// any SameSite attribute.
-func TestSamesiteBackwardsCompat(t *testing.T) {
+// TestSameSiteDefault tests that the default set of options
+// set SameSite=Lax on the CSRF cookie.
+func TestSameSiteDefaultLaxMode(t *testing.T) {
 	s := http.NewServeMux()
 	s.HandleFunc("/", testHandler)
 
@@ -182,10 +182,11 @@ func TestSamesiteBackwardsCompat(t *testing.T) {
 
 	cookie := rr.Header().Get("Set-Cookie")
 	if cookie == "" {
-		t.Fatalf("cookie not get set-cookie header: got headers %v", rr.Header())
+		t.Fatalf("cookie not get Set-Cookie header: got headers %v", rr.Header())
 	}
 
-	if strings.Contains(cookie, "SameSite") {
-		t.Fatalf("cookie should not contain the substring 'SameSite' by default, but did: %q", cookie)
+	sameSiteLax := "SameSite=Lax"
+	if !strings.Contains(cookie, sameSiteLax) {
+		t.Fatalf("cookie should contain %q by default: got %s", sameSiteLax, cookie)
 	}
 }