diff --git a/pkg/router/cookies.go b/pkg/router/cookies.go index 413d03a..e970648 100644 --- a/pkg/router/cookies.go +++ b/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) diff --git a/pkg/router/handler_callback.go b/pkg/router/handler_callback.go index 6eef211..772b480 100644 --- a/pkg/router/handler_callback.go +++ b/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) } diff --git a/pkg/router/handler_login.go b/pkg/router/handler_login.go index 1967aa7..85d582b 100644 --- a/pkg/router/handler_login.go +++ b/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)) } diff --git a/pkg/router/router_test.go b/pkg/router/router_test.go index 8093ef5..e923d6a 100644 --- a/pkg/router/router_test.go +++ b/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")