From b718922d97174e1cb498c951f22bf7628ce288f4 Mon Sep 17 00:00:00 2001
From: Dalton Hubble <dghubble@gmail.com>
Date: Fri, 25 Nov 2022 21:44:17 -0800
Subject: [PATCH] Add SameSite attribute to the OAuth2 state cookie

* Add a SameSite field to the CookieConfig to allow configuring the
SameSite attribute on the temporary state cookie used by OAuth2
* Set lax mode on the DefaultCookieConfig and DebugOnlyCookieConfig
* Generally, we can't use SameSite strict for the OAuth2 temporary
state cookie. Strict cookies are not sent in the requests to the
callback handler because the redirect chain originated with a redirect
to another domain (e.g. github, google), which is considered the
referrer. Instead, lax mode must be used
* Increase the temporary cookie max age from 1m to 10m to allow users
longer to complete the authorization flow (e.g. login to a provider,
maybe complete 2FA, review permissions, grant permission)

Rel:

* https://www.nogginbox.co.uk/blog/strict-cookies-not-sent-by-request
* https://bugzilla.mozilla.org/show_bug.cgi?id=1453814
---
 CHANGES.md         |  5 +++++
 config.go          | 11 +++++++++--
 internal/cookie.go |  1 +
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/CHANGES.md b/CHANGES.md
index 95cf5fa..2e1c25c 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -4,6 +4,11 @@ Notable changes between releases.
 
 ## Latest
 
+* Add `SameSite` field to the oauth2 state `CookieConfig` ([#112](https://github.com/dghubble/gologin/pull/112))
+  * Set `SameSiteLaxMode` in `DefaultCookieConfig` and `DebugOnlyCookieConfig`
+* Raise the `MaxAge` in `DefaultCookieConfig` and `DebugOnlyCookieConfig`
+  * Allow 10 min for users to complete the authorization flow
+
 ## v2.3.1
 
 * Update minimum Go version from v1.17 to v1.18 ([#116](https://github.com/dghubble/gologin/pull/116))
diff --git a/config.go b/config.go
index 7ca73de..d982964 100644
--- a/config.go
+++ b/config.go
@@ -1,5 +1,7 @@
 package gologin
 
+import "net/http"
+
 // CookieConfig configures http.Cookie creation.
 type CookieConfig struct {
 	// Name is the desired cookie name.
@@ -21,15 +23,19 @@ type CookieConfig struct {
 	// Secure flag indicating to the browser that the cookie should only be
 	// transmitted over a TLS HTTPS connection. Recommended true in production.
 	Secure bool
+	// SameSite attribute modes indicates that a browser not send a cookie in
+	// cross-site requests.
+	SameSite http.SameSite
 }
 
 // DefaultCookieConfig configures short-lived temporary http.Cookie creation.
 var DefaultCookieConfig = CookieConfig{
 	Name:     "gologin-temporary-cookie",
 	Path:     "/",
-	MaxAge:   60, // 60 seconds
+	MaxAge:   600, // 10 min
 	HTTPOnly: true,
 	Secure:   true, // HTTPS only
+	SameSite: http.SameSiteLaxMode,
 }
 
 // DebugOnlyCookieConfig configures creation of short-lived temporary
@@ -38,7 +44,8 @@ var DefaultCookieConfig = CookieConfig{
 var DebugOnlyCookieConfig = CookieConfig{
 	Name:     "gologin-temporary-cookie",
 	Path:     "/",
-	MaxAge:   60, // 60 seconds
+	MaxAge:   600, // 10 min
 	HTTPOnly: true,
 	Secure:   false, // allows cookies to be send over HTTP
+	SameSite: http.SameSiteLaxMode,
 }
diff --git a/internal/cookie.go b/internal/cookie.go
index fd36657..f0c627a 100644
--- a/internal/cookie.go
+++ b/internal/cookie.go
@@ -21,6 +21,7 @@ func NewCookie(config gologin.CookieConfig, value string) *http.Cookie {
 		MaxAge:   config.MaxAge,
 		HttpOnly: config.HTTPOnly,
 		Secure:   config.Secure,
+		SameSite: config.SameSite,
 	}
 	// IE <9 does not understand MaxAge, set Expires if MaxAge is non-zero.
 	if expires, ok := expiresTime(config.MaxAge); ok {