fix(router/login): alleviate SameSite issues for login cookie

A login cookie is set as part of the redirection flow between the RP
and OP, and thus inherently involves cross-site requests. Our client
uses the response_mode=query parameter for authorization requests, which
should work with the SameSite attribute set to Lax. However, there are
certain versions of user agents on certain operating systems (e.g.
Safari 12.2 on iOS<12.2, MacOS<10.14.4, Android WebView<72) that do not
properly handle cookies with the SameSite attribute set.

This commit attempts to alleviate this issue for legacy browsers by
introducing a fallback cookie without the SameSite attribute set.

Additionally, we also set the SameSite value for the original login
cookie to None to ensure that the cookie persists through the
cross-origin redirection requests.

pkg/router/cookies.go

@@ -1,7 +1,6 @@
 package router
 
 import (
-	"fmt"
 	"net/http"
 
 	"github.com/nais/wonderwall/pkg/cookie"
@@ -10,6 +9,7 @@ import (
 const (
 	SessionCookieName     = "io.nais.wonderwall.session"
 	LoginCookieName       = "io.nais.wonderwall.callback"
+	LoginLegacyCookieName = "io.nais.wonderwall.callback.legacy"
 )
 
 func (h *Handler) setEncryptedCookie(w http.ResponseWriter, key string, plaintext string, opts cookie.Options) error {
@@ -25,7 +25,7 @@ func (h *Handler) setEncryptedCookie(w http.ResponseWriter, key string, plaintex
 func (h *Handler) getDecryptedCookie(r *http.Request, key string) (string, error) {
 	encryptedCookie, err := cookie.Get(r, key)
 	if err != nil {
-		return "", fmt.Errorf("no cookie named '%s': %w", key, err)
+		return "", err
 	}
 
 	return encryptedCookie.Decrypt(h.Crypter)

pkg/router/handler_callback.go

@@ -57,7 +57,7 @@ func (h *Handler) Callback(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	h.clearLoginCookie(w)
+	h.clearLoginCookies(w)
 
 	http.Redirect(w, r, loginCookie.Referer, http.StatusTemporaryRedirect)
 }

pkg/router/handler_login.go

@@ -7,6 +7,8 @@ import (
 	"net/http"
 	"time"
 
+	log "github.com/sirupsen/logrus"
+
 	"github.com/nais/wonderwall/pkg/cookie"
 	"github.com/nais/wonderwall/pkg/openid"
 	"github.com/nais/wonderwall/pkg/router/request"
@@ -53,7 +55,12 @@ func (h *Handler) Login(w http.ResponseWriter, r *http.Request) {
 func (h *Handler) getLoginCookie(r *http.Request) (*openid.LoginCookie, error) {
 	loginCookieJson, err := h.getDecryptedCookie(r, LoginCookieName)
 	if err != nil {
-		return nil, err
+		log.Warnf("failed to fetch login cookie; falling back to legacy cookie: %+v", err)
+		log.Debugf("debug: user-agent: %s", r.UserAgent())
+		loginCookieJson, err = h.getDecryptedCookie(r, LoginLegacyCookieName)
+		if err != nil {
+			return nil, err
+		}
 	}
 
 	var loginCookie openid.LoginCookie
@@ -71,7 +78,9 @@ func (h *Handler) setLoginCookies(w http.ResponseWriter, loginCookie *openid.Log
 		return fmt.Errorf("marshalling login cookie: %w", err)
 	}
 
-	opts := h.Cookies.WithExpiresIn(LoginCookieLifetime)
+	opts := h.Cookies.
+		WithExpiresIn(LoginCookieLifetime).
+		WithSameSite(http.SameSiteNoneMode)
 	value := string(loginCookieJson)
 
 	err = h.setEncryptedCookie(w, LoginCookieName, value, opts)
@@ -79,10 +88,17 @@ func (h *Handler) setLoginCookies(w http.ResponseWriter, loginCookie *openid.Log
 		return err
 	}
 
+	// set a duplicate cookie without the SameSite value set for user agents that do not properly handle SameSite
+	err = h.setEncryptedCookie(w, LoginLegacyCookieName, value, opts.WithSameSite(http.SameSiteDefaultMode))
+	if err != nil {
+		return err
+	}
+
 	return nil
 }
 
-func (h *Handler) clearLoginCookie(w http.ResponseWriter) {
+func (h *Handler) clearLoginCookies(w http.ResponseWriter) {
 	opts := h.Cookies
-	cookie.Clear(w, LoginCookieName, opts)
+	cookie.Clear(w, LoginCookieName, opts.WithSameSite(http.SameSiteNoneMode))
+	cookie.Clear(w, LoginLegacyCookieName, opts.WithSameSite(http.SameSiteDefaultMode))
 }

pkg/router/router_test.go

@@ -69,6 +69,8 @@ func TestHandler_Login(t *testing.T) {
 	cookies := client.Jar.Cookies(loginURL)
 	loginCookie := getCookieFromJar(router.LoginCookieName, cookies)
 	assert.NotNil(t, loginCookie)
+	loginLegacyCookie := getCookieFromJar(router.LoginLegacyCookieName, cookies)
+	assert.NotNil(t, loginLegacyCookie)
 
 	location := resp.Header.Get("location")
 	u, err := url.Parse(location)
@@ -126,9 +128,11 @@ func TestHandler_Callback_and_Logout(t *testing.T) {
 	cookies := client.Jar.Cookies(loginURL)
 	sessionCookie := getCookieFromJar(router.SessionCookieName, cookies)
 	loginCookie := getCookieFromJar(router.LoginCookieName, cookies)
+	loginLegacyCookie := getCookieFromJar(router.LoginLegacyCookieName, cookies)
 
 	assert.Nil(t, sessionCookie)
 	assert.NotNil(t, loginCookie)
+	assert.NotNil(t, loginLegacyCookie)
 
 	// Get authorization URL
 	location := resp.Header.Get("location")
@@ -154,9 +158,11 @@ func TestHandler_Callback_and_Logout(t *testing.T) {
 	cookies = client.Jar.Cookies(callbackURL)
 	sessionCookie = getCookieFromJar(router.SessionCookieName, cookies)
 	loginCookie = getCookieFromJar(router.LoginCookieName, cookies)
+	loginLegacyCookie = getCookieFromJar(router.LoginLegacyCookieName, cookies)
 
 	assert.NotNil(t, sessionCookie)
 	assert.Nil(t, loginCookie)
+	assert.Nil(t, loginLegacyCookie)
 
 	// Request self-initiated logout
 	logoutURL, err := url.Parse(server.URL + "/oauth2/logout")