From 197ef4ee79e7c4881c5c612115326f4e874c9415 Mon Sep 17 00:00:00 2001
From: mo khan <mo@mokhan.ca>
Date: Wed, 30 Apr 2025 12:02:14 -0600
Subject: fix: the CSRF cookie needs to have a same site lax mode

---
 app/controllers/sessions/controller.go |  5 ++++-
 app/controllers/sessions/service.go    |  3 ++-
 pkg/web/cookie/cookie_test.go          | 10 +---------
 pkg/web/cookie/new.go                  |  3 +--
 4 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/app/controllers/sessions/controller.go b/app/controllers/sessions/controller.go
index 8d0e858..5babe7d 100644
--- a/app/controllers/sessions/controller.go
+++ b/app/controllers/sessions/controller.go
@@ -33,7 +33,10 @@ func (c *Controller) New(w http.ResponseWriter, r *http.Request) {
 	}
 
 	url, nonce := c.svc.GenerateRedirectURL()
-	http.SetCookie(w, cookie.New("oauth_state", nonce, time.Now().Add(10*time.Minute)))
+	cookie := cookie.New("oauth_state", nonce, time.Now().Add(10*time.Minute))
+	// This cookie must be sent as part of a redirect that originates from the OIDC Provider
+	cookie.SameSite = http.SameSiteLaxMode
+	http.SetCookie(w, cookie)
 	http.Redirect(w, r, url, http.StatusFound)
 }
 
diff --git a/app/controllers/sessions/service.go b/app/controllers/sessions/service.go
index 0ee692a..0fd7692 100644
--- a/app/controllers/sessions/service.go
+++ b/app/controllers/sessions/service.go
@@ -3,6 +3,7 @@ package sessions
 import (
 	"context"
 	"errors"
+	"fmt"
 	"net/http"
 
 	"gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/oidc"
@@ -34,7 +35,7 @@ func (svc *Service) GenerateRedirectURL() (string, string) {
 func (svc *Service) Exchange(r *http.Request) (*oidc.Tokens, error) {
 	cookies := r.CookiesNamed("oauth_state")
 	if len(cookies) != 1 {
-		return nil, errors.New("Missing CSRF token")
+		return nil, errors.New(fmt.Sprintf("Missing CSRF token: %v, cookies: %v", len(cookies), r.Cookies()))
 	}
 
 	state := r.URL.Query().Get("state")
diff --git a/pkg/web/cookie/cookie_test.go b/pkg/web/cookie/cookie_test.go
index 523e496..2f600f4 100644
--- a/pkg/web/cookie/cookie_test.go
+++ b/pkg/web/cookie/cookie_test.go
@@ -12,7 +12,7 @@ import (
 func TestCookie(t *testing.T) {
 	t.Run("New", func(t *testing.T) {
 		t.Run("returns a cookie pinned to the HOST", func(t *testing.T) {
-			env.With(env.Vars{"HOST": "sparkle.example.com", "APP_ENV": "production"}, func() {
+			env.With(env.Vars{"HOST": "sparkle.example.com"}, func() {
 				cookie := New("name", "value", time.Now().Add(1*time.Minute))
 				assert.Equal(t, "sparkle.example.com", cookie.Domain)
 				assert.True(t, cookie.HttpOnly)
@@ -20,14 +20,6 @@ func TestCookie(t *testing.T) {
 				assert.Equal(t, http.SameSiteStrictMode, cookie.SameSite)
 			})
 		})
-
-		t.Run("disables the secure flag for development", func(t *testing.T) {
-			env.With(env.Vars{"HOST": "sparkle.example.com", "APP_ENV": "development"}, func() {
-				cookie := New("name", "value", time.Now().Add(1*time.Minute))
-				assert.True(t, cookie.HttpOnly)
-				assert.False(t, cookie.Secure)
-			})
-		})
 	})
 
 	t.Run("Reset", func(t *testing.T) {
diff --git a/pkg/web/cookie/new.go b/pkg/web/cookie/new.go
index 6dc9a2e..d4d0700 100644
--- a/pkg/web/cookie/new.go
+++ b/pkg/web/cookie/new.go
@@ -8,7 +8,6 @@ import (
 )
 
 func New(name, value string, expires time.Time) *http.Cookie {
-	appEnv := env.Fetch("APP_ENV", "development")
 	return &http.Cookie{
 		Name:     name,
 		Value:    value, // TODO:: digitally sign the value
@@ -16,7 +15,7 @@ func New(name, value string, expires time.Time) *http.Cookie {
 		MaxAge:   int(time.Until(expires).Seconds()),
 		Path:     "/",
 		HttpOnly: true,
-		Secure:   appEnv == "production",
+		Secure:   true,
 		SameSite: http.SameSiteStrictMode,
 		Domain:   env.Fetch("HOST", "localhost"),
 	}
-- 
cgit v1.2.3