diff --git a/pkg/cookies/cookies.go b/pkg/cookies/cookies.go index 36ee7554..eefcbfb6 100644 --- a/pkg/cookies/cookies.go +++ b/pkg/cookies/cookies.go @@ -76,7 +76,8 @@ func ParseSameSite(v options.SameSiteMode) http.SameSite { case options.SameSiteNone: return http.SameSiteNoneMode case options.SameSiteDefault: - return 0 + // Default to Lax if not specified, as per https://web.dev/samesite-cookies-explained/#samesite-by-default + return http.SameSiteLaxMode default: panic(fmt.Sprintf("Invalid value for SameSite: %s", v)) } diff --git a/pkg/cookies/cookies_suite_test.go b/pkg/cookies/cookies_suite_test.go index 87ef36c3..0d152b4d 100644 --- a/pkg/cookies/cookies_suite_test.go +++ b/pkg/cookies/cookies_suite_test.go @@ -16,10 +16,6 @@ const ( cookieDomain = "o2p.cookies.test" cookiePath = "/cookie-tests" - sameSiteLax = "lax" - sameSiteStrict = "strict" - sameSiteNone = "none" - nowEpoch = 1609366421 ) diff --git a/pkg/cookies/cookies_test.go b/pkg/cookies/cookies_test.go index e3437829..1f8e9303 100644 --- a/pkg/cookies/cookies_test.go +++ b/pkg/cookies/cookies_test.go @@ -115,7 +115,7 @@ var _ = Describe("Cookie Tests", func() { Secret: validSecret, Domains: domains, Path: "", - Expire: time.Hour, + Expire: 15 * time.Minute, Insecure: ptr.To(false), ScriptAccess: options.ScriptAccessAllowed, SameSite: "", diff --git a/pkg/cookies/csrf_per_request_test.go b/pkg/cookies/csrf_per_request_test.go index 1312aafb..e85d5052 100644 --- a/pkg/cookies/csrf_per_request_test.go +++ b/pkg/cookies/csrf_per_request_test.go @@ -163,7 +163,7 @@ var _ = Describe("CSRF Cookie with non-fixed name Tests", func() { )) Expect(rw.Header().Get("Set-Cookie")).To(ContainSubstring( fmt.Sprintf( - "; Path=%s; Domain=%s; Max-Age=%d; HttpOnly; Secure", + "; Path=%s; Domain=%s; Max-Age=%d; HttpOnly; Secure; SameSite=Lax", cookiePath, cookieDomain, int(cookieOpts.CSRFExpire.Seconds()), @@ -180,7 +180,7 @@ var _ = Describe("CSRF Cookie with non-fixed name Tests", func() { Expect(rw.Header().Get("Set-Cookie")).To(Equal( fmt.Sprintf( - "%s=; Path=%s; Domain=%s; Max-Age=0; HttpOnly; Secure", + "%s=; Path=%s; Domain=%s; Max-Age=0; HttpOnly; Secure; SameSite=Lax", privateCSRF.cookieName(), cookiePath, cookieDomain, @@ -257,7 +257,7 @@ var _ = Describe("CSRF Cookie with non-fixed name Tests", func() { Expect(clearedCookies).To(HaveLen(2)) Expect(clearedCookies[0]).To(Equal( fmt.Sprintf( - "%s=; Path=%s; Domain=%s; Max-Age=0; HttpOnly; Secure", + "%s=; Path=%s; Domain=%s; Max-Age=0; HttpOnly; Secure; SameSite=Lax", privateCSRF1.cookieName(), cookiePath, cookieDomain, @@ -265,7 +265,7 @@ var _ = Describe("CSRF Cookie with non-fixed name Tests", func() { )) Expect(clearedCookies[1]).To(Equal( fmt.Sprintf( - "%s=; Path=%s; Domain=%s; Max-Age=0; HttpOnly; Secure", + "%s=; Path=%s; Domain=%s; Max-Age=0; HttpOnly; Secure; SameSite=Lax", privateCSRF2.cookieName(), cookiePath, cookieDomain, diff --git a/pkg/cookies/csrf_test.go b/pkg/cookies/csrf_test.go index a8693047..daf9323f 100644 --- a/pkg/cookies/csrf_test.go +++ b/pkg/cookies/csrf_test.go @@ -188,7 +188,7 @@ var _ = Describe("CSRF Cookie Tests", func() { )) Expect(rw.Header().Get("Set-Cookie")).To(ContainSubstring( fmt.Sprintf( - "; Path=%s; Domain=%s; Max-Age=%d; HttpOnly; Secure", + "; Path=%s; Domain=%s; Max-Age=%d; HttpOnly; Secure; SameSite=Lax", cookiePath, cookieDomain, int(cookieOpts.CSRFExpire.Seconds()), @@ -266,7 +266,7 @@ var _ = Describe("CSRF Cookie Tests", func() { Expect(rw.Header().Get("Set-Cookie")).To(Equal( fmt.Sprintf( - "%s=; Path=%s; Domain=%s; Max-Age=0; HttpOnly; Secure", + "%s=; Path=%s; Domain=%s; Max-Age=0; HttpOnly; Secure; SameSite=Lax", privateCSRF.cookieName(), cookiePath, cookieDomain, @@ -306,12 +306,12 @@ var _ = Describe("CSRF Cookie Tests", func() { cookieOpts = &options.Cookie{ Name: cookieName, - Secret: options.SecretSource{Value: cookieSecret}, + Secret: &options.SecretSource{Value: cookieSecret}, Domains: []string{cookieDomain}, Path: cookiePath, Expire: time.Hour, Insecure: ptr.To(false), - NotHttpOnly: ptr.To(false), + ScriptAccess: options.ScriptAccessDenied, CSRFPerRequest: ptr.To(false), CSRFExpire: time.Hour, } @@ -319,7 +319,7 @@ var _ = Describe("CSRF Cookie Tests", func() { It("Call SetCookie when CSRF SameSite is not defined. Expected result: CSRF cookie SameSite is the same as session cookie.", func() { // prepare - cookieOpts.SameSite = sameSiteLax + cookieOpts.CSRFSameSite = options.SameSiteLax CSRF, _ := NewCSRF(cookieOpts, "verifier") rw := httptest.NewRecorder() CSRF.(*csrf).clock = func() time.Time { return testNow } @@ -341,7 +341,7 @@ var _ = Describe("CSRF Cookie Tests", func() { It("Call SetCookie when CSRF SameSite is an empty string. Expected result: CSRF cookie SameSite is the same as session cookie.", func() { // prepare - cookieOpts.SameSite = sameSiteLax + cookieOpts.SameSite = options.SameSiteLax cookieOpts.CSRFSameSite = "" CSRF, _ := NewCSRF(cookieOpts, "verifier") rw := httptest.NewRecorder() @@ -364,8 +364,8 @@ var _ = Describe("CSRF Cookie Tests", func() { It("Call SetCookie when CSRF SameSite is 'none'. Expected result: CSRF cookie SameSite is None.", func() { // prepare - cookieOpts.SameSite = sameSiteLax - cookieOpts.CSRFSameSite = sameSiteNone + cookieOpts.SameSite = options.SameSiteLax + cookieOpts.CSRFSameSite = options.SameSiteNone CSRF, _ := NewCSRF(cookieOpts, "verifier") rw := httptest.NewRecorder() CSRF.(*csrf).clock = func() time.Time { return testNow } @@ -387,8 +387,8 @@ var _ = Describe("CSRF Cookie Tests", func() { It("Call SetCookie when CSRF SameSite is 'strict'. Expected result: CSRF cookie SameSite is Strict.", func() { // prepare - cookieOpts.SameSite = sameSiteLax - cookieOpts.CSRFSameSite = sameSiteStrict + cookieOpts.SameSite = options.SameSiteLax + cookieOpts.CSRFSameSite = options.SameSiteStrict CSRF, _ := NewCSRF(cookieOpts, "verifier") rw := httptest.NewRecorder() CSRF.(*csrf).clock = func() time.Time { return testNow } @@ -410,8 +410,8 @@ var _ = Describe("CSRF Cookie Tests", func() { It("Call SetCookie when CSRF SameSite is 'lax'. Expected result: CSRF cookie SameSite is Lax.", func() { // prepare - cookieOpts.SameSite = sameSiteStrict - cookieOpts.CSRFSameSite = sameSiteLax + cookieOpts.SameSite = options.SameSiteStrict + cookieOpts.CSRFSameSite = options.SameSiteLax CSRF, _ := NewCSRF(cookieOpts, "verifier") rw := httptest.NewRecorder() CSRF.(*csrf).clock = func() time.Time { return testNow } @@ -433,7 +433,7 @@ var _ = Describe("CSRF Cookie Tests", func() { It("Call ClearCookie when CSRF SameSite is not defined. Expected result: CSRF cookie SameSite is the same as session cookie.", func() { // prepare - cookieOpts.SameSite = sameSiteLax + cookieOpts.SameSite = options.SameSiteLax CSRF, _ := NewCSRF(cookieOpts, "verifier") rw := httptest.NewRecorder() CSRF.(*csrf).clock = func() time.Time { return testNow } @@ -454,7 +454,7 @@ var _ = Describe("CSRF Cookie Tests", func() { It("Call ClearCookie when CSRF SameSite is an empty string. Expected result: CSRF cookie SameSite is the same as session cookie.", func() { // prepare - cookieOpts.SameSite = sameSiteLax + cookieOpts.SameSite = options.SameSiteLax cookieOpts.CSRFSameSite = "" CSRF, _ := NewCSRF(cookieOpts, "verifier") rw := httptest.NewRecorder() @@ -476,8 +476,8 @@ var _ = Describe("CSRF Cookie Tests", func() { It("Call ClearCookie when CSRF SameSite is 'none'. Expected result: CSRF cookie SameSite is None.", func() { // prepare - cookieOpts.SameSite = sameSiteLax - cookieOpts.CSRFSameSite = sameSiteNone + cookieOpts.SameSite = options.SameSiteLax + cookieOpts.CSRFSameSite = options.SameSiteNone CSRF, _ := NewCSRF(cookieOpts, "verifier") rw := httptest.NewRecorder() CSRF.(*csrf).clock = func() time.Time { return testNow } @@ -498,8 +498,8 @@ var _ = Describe("CSRF Cookie Tests", func() { It("Call ClearCookie when CSRF SameSite is 'strict'. Expected result: CSRF cookie SameSite is Strict.", func() { // prepare - cookieOpts.SameSite = sameSiteLax - cookieOpts.CSRFSameSite = sameSiteStrict + cookieOpts.SameSite = options.SameSiteLax + cookieOpts.CSRFSameSite = options.SameSiteStrict CSRF, _ := NewCSRF(cookieOpts, "verifier") rw := httptest.NewRecorder() CSRF.(*csrf).clock = func() time.Time { return testNow } @@ -520,8 +520,8 @@ var _ = Describe("CSRF Cookie Tests", func() { It("Call ClearCookie when CSRF SameSite is 'lax'. Expected result: CSRF cookie SameSite is Lax.", func() { // prepare - cookieOpts.SameSite = sameSiteStrict - cookieOpts.CSRFSameSite = sameSiteLax + cookieOpts.SameSite = options.SameSiteStrict + cookieOpts.CSRFSameSite = options.SameSiteLax CSRF, _ := NewCSRF(cookieOpts, "verifier") rw := httptest.NewRecorder() CSRF.(*csrf).clock = func() time.Time { return testNow }