feat: use a cookie prefix to lock down the session cookie

> __Host-: If a cookie name has this prefix, it's accepted in a
> Set-Cookie header only if it's also marked with the Secure attribute,
> was sent from a secure origin, does not include a Domain attribute,
> and has the Path attribute set to /. In other words, the cookie is
> domain-locked.

https://developer.mozilla.org/en-US/docs/Web/HTTP/Guides/Cookies#cookie_prefixes

10 files changed, 35 insertions, 40 deletions

diff --git a/app/cfg/cfg.go b/app/cfg/cfg.go
index ee6fffe..5b3025e 100644
--- a/app/cfg/cfg.go
+++ b/app/cfg/cfg.go
@@ -13,3 +13,6 @@ var OIDCIssuer string = env.Fetch("OIDC_ISSUER", "https://gitlab.com")
 var OAuthClientID string = env.Fetch("OAUTH_CLIENT_ID", "client_id")
 var OAuthClientSecret string = env.Fetch("OAUTH_CLIENT_SECRET", "client_secret")
 var OAuthRedirectURL string = env.Fetch("OAUTH_REDIRECT_URL", "http://localhost:8080/session/callback")
+
+var SessionCookie string = "__Host-session"
+var CSRFCookie string = "__Host-csrf"
diff --git a/app/controllers/sessions/controller.go b/app/controllers/sessions/controller.go
index 7860ed5..61fdaf8 100644
--- a/app/controllers/sessions/controller.go
+++ b/app/controllers/sessions/controller.go
@@ -6,6 +6,7 @@ import (
 
 	"github.com/xlgmokha/x/pkg/cookie"
 	"github.com/xlgmokha/x/pkg/log"
+	"gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/cfg"
 	"gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/middleware"
 	"gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/oidc"
 	"gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/pls"
@@ -37,7 +38,7 @@ func (c *Controller) New(w http.ResponseWriter, r *http.Request) {
 	url, nonce := c.svc.GenerateRedirectURL()
 	// This cookie must be sent as part of a redirect that originates from the OIDC Provider
 	cookie.Write(w, web.NewCookie(
-		"oauth_state",
+		cfg.CSRFCookie,
 		nonce,
 		cookie.WithSameSite(http.SameSiteLaxMode),
 		cookie.WithExpiration(time.Now().Add(10*time.Minute)),
@@ -140,7 +141,7 @@ func (c *Controller) Create(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	ck := web.NewCookie("session", encoded,
+	ck := web.NewCookie(cfg.SessionCookie, encoded,
 		cookie.WithSameSite(http.SameSiteLaxMode),
 		cookie.WithExpiration(tokens.Expiry),
 	)
diff --git a/app/controllers/sessions/controller_test.go b/app/controllers/sessions/controller_test.go
index 4b68c7a..82c56d5 100644
--- a/app/controllers/sessions/controller_test.go
+++ b/app/controllers/sessions/controller_test.go
@@ -95,7 +95,7 @@ func TestSessions(t *testing.T) {
 			r, w := test.RequestResponse(
 				"GET",
 				"/session/callback?code="+code+"&state=invalid",
-				test.WithCookie(web.NewCookie("oauth_state", nonce)),
+				test.WithCookie(web.NewCookie(xcfg.CSRFCookie, nonce)),
 			)
 
 			mux.ServeHTTP(w, r)
@@ -119,7 +119,7 @@ func TestSessions(t *testing.T) {
 			r, w := test.RequestResponse(
 				"GET",
 				"/session/callback?code="+code+"&state="+nonce,
-				test.WithCookie(web.NewCookie("oauth_state", nonce)),
+				test.WithCookie(web.NewCookie(xcfg.CSRFCookie, nonce)),
 			)
 
 			mux.ServeHTTP(w, r)
@@ -176,8 +176,7 @@ func TestSessions(t *testing.T) {
 
 			t.Run("applies the appropriate cookie settings", func(t *testing.T) {
 				assert.Equal(t, "/", cookie.Path)
-				assert.Equal(t, "localhost", cookie.Domain)
-				assert.Equal(t, "session", cookie.Name)
+				assert.Equal(t, xcfg.SessionCookie, cookie.Name)
 				assert.Equal(t, http.SameSiteLaxMode, cookie.SameSite)
 				assert.Equal(t, x.Must(time.Parse(time.RFC3339, tokens["expiry"].(string))).Unix(), cookie.Expires.Unix())
 				assert.True(t, cookie.HttpOnly)
@@ -189,14 +188,14 @@ func TestSessions(t *testing.T) {
 
 	t.Run("POST /session/destroy", func(t *testing.T) {
 		t.Run("clears the session cookie", func(t *testing.T) {
-			cookie := web.NewCookie("session", "value")
+			cookie := web.NewCookie(xcfg.SessionCookie, "value")
 			r, w := test.RequestResponse("POST", "/session/destroy", test.WithCookie(cookie))
 
 			mux.ServeHTTP(w, r)
 
 			require.Equal(t, http.StatusFound, w.Code)
 			assert.Equal(t, "/", w.Header().Get("Location"))
-			assert.Equal(t, "session=; Path=/; Domain=localhost; Expires=Thu, 01 Jan 1970 00:00:00 GMT; Max-Age=0; HttpOnly; Secure", w.Header().Get("Set-Cookie"))
+			assert.Equal(t, "session=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; Max-Age=0; HttpOnly; Secure", w.Header().Get("Set-Cookie"))
 		})
 	})
 }
diff --git a/app/controllers/sessions/service.go b/app/controllers/sessions/service.go
index af1512c..2dec9e3 100644
--- a/app/controllers/sessions/service.go
+++ b/app/controllers/sessions/service.go
@@ -5,6 +5,7 @@ import (
 	"errors"
 	"net/http"
 
+	"gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/cfg"
 	"gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/oidc"
 	"gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/pls"
 	"gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/web"
@@ -33,7 +34,7 @@ func (svc *Service) GenerateRedirectURL() (string, string) {
 }
 
 func (svc *Service) Exchange(r *http.Request) (*oidc.Tokens, error) {
-	cookies := r.CookiesNamed("oauth_state")
+	cookies := r.CookiesNamed(cfg.CSRFCookie)
 	if len(cookies) != 1 {
 		return nil, errors.New("Missing CSRF token")
 	}
diff --git a/app/controllers/sessions/service_test.go b/app/controllers/sessions/service_test.go
index 81248ca..4bea1dd 100644
--- a/app/controllers/sessions/service_test.go
+++ b/app/controllers/sessions/service_test.go
@@ -8,6 +8,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/xlgmokha/x/pkg/test"
+	xcfg "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/cfg"
 	"gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/oidc"
 	"gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/pls"
 	"gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/web"
@@ -46,7 +47,7 @@ func TestService(t *testing.T) {
 			r := test.Request(
 				"GET",
 				"/session/callback?code="+code+"&state=invalid",
-				test.WithCookie(web.NewCookie("oauth_state", nonce)),
+				test.WithCookie(web.NewCookie(xcfg.CSRFCookie, nonce)),
 			)
 			tokens, err := svc.Exchange(r)
 
@@ -59,7 +60,7 @@ func TestService(t *testing.T) {
 
 			r := test.Request(
 				"GET", "/session/callback?code=invalid",
-				test.WithCookie(web.NewCookie("oauth_state", nonce)),
+				test.WithCookie(web.NewCookie(xcfg.CSRFCookie, nonce)),
 			)
 
 			tokens, err := svc.Exchange(r)
@@ -76,7 +77,7 @@ func TestService(t *testing.T) {
 			r := test.Request(
 				"GET",
 				"/session/callback?code="+code+"&state="+nonce,
-				test.WithCookie(web.NewCookie("oauth_state", nonce)),
+				test.WithCookie(web.NewCookie(xcfg.CSRFCookie, nonce)),
 			)
 
 			tokens, err := svc.Exchange(r)
diff --git a/app/middleware/id_token.go b/app/middleware/id_token.go
index 5a44f49..e0b5b0d 100644
--- a/app/middleware/id_token.go
+++ b/app/middleware/id_token.go
@@ -21,7 +21,7 @@ func IDToken(cfg *oidc.OpenID, parsers ...TokenParser) func(http.Handler) http.H
 
 					if err != nil {
 						pls.LogError(r.Context(), err)
-						web.ExpireCookie(w, "session")
+						web.ExpireCookie(w, xcfg.SessionCookie)
 					} else {
 						log.WithFields(r.Context(), log.Fields{"id_token": idToken})
 						next.ServeHTTP(
diff --git a/app/middleware/id_token_test.go b/app/middleware/id_token_test.go
index 95d9b40..31a4333 100644
--- a/app/middleware/id_token_test.go
+++ b/app/middleware/id_token_test.go
@@ -55,7 +55,7 @@ func TestIDToken(t *testing.T) {
 			r, w := test.RequestResponse(
 				"GET",
 				"/example",
-				test.WithCookie(web.NewCookie("session", encoded)),
+				test.WithCookie(web.NewCookie(xcfg.SessionCookie, encoded)),
 			)
 			server.ServeHTTP(w, r)
 
@@ -74,7 +74,7 @@ func TestIDToken(t *testing.T) {
 			r, w := test.RequestResponse(
 				"GET",
 				"/example",
-				test.WithCookie(web.NewCookie("session", "invalid")),
+				test.WithCookie(web.NewCookie(xcfg.SessionCookie, "invalid")),
 			)
 			server.ServeHTTP(w, r)
 
diff --git a/app/middleware/token_parser.go b/app/middleware/token_parser.go
index f64522b..08219b4 100644
--- a/app/middleware/token_parser.go
+++ b/app/middleware/token_parser.go
@@ -4,6 +4,7 @@ import (
 	"net/http"
 
 	"github.com/xlgmokha/x/pkg/x"
+	"gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/app/cfg"
 	"gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/oidc"
 	"gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/pls"
 	"gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/web"
@@ -12,7 +13,7 @@ import (
 type TokenParser x.Mapper[*http.Request, oidc.RawToken]
 
 func IDTokenFromSessionCookie(r *http.Request) oidc.RawToken {
-	cookies := r.CookiesNamed("session")
+	cookies := r.CookiesNamed(cfg.SessionCookie)
 
 	if len(cookies) != 1 {
 		return ""
diff --git a/pkg/web/cookie.go b/pkg/web/cookie.go
index c60121f..fd81c1d 100644
--- a/pkg/web/cookie.go
+++ b/pkg/web/cookie.go
@@ -26,8 +26,6 @@ func NewCookie(name, value string, options ...x.Option[*http.Cookie]) *http.Cook
 		cookie.WithPath("/"),
 		cookie.WithHttpOnly(true),
 		cookie.WithSecure(true),
-		cookie.WithSameSite(http.SameSiteDefaultMode),
-		cookie.WithDomain(env.Fetch("HOST", "localhost")),
 	)...)
 }
 
@@ -51,7 +49,6 @@ func withSignedValue(value string) x.Option[*http.Cookie] {
 func ExpireCookie(w http.ResponseWriter, name string) {
 	cookie.Expire(w, name,
 		cookie.WithPath("/"),
-		cookie.WithDomain(env.Fetch("HOST", "localhost")),
 		cookie.WithHttpOnly(true),
 		cookie.WithSecure(true),
 	)
diff --git a/pkg/web/cookie_test.go b/pkg/web/cookie_test.go
index 60cf7cf..1a3bfb0 100644
--- a/pkg/web/cookie_test.go
+++ b/pkg/web/cookie_test.go
@@ -8,34 +8,26 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"github.com/xlgmokha/x/pkg/env"
 )
 
 func TestNewCookie(t *testing.T) {
-	env.With(env.Vars{"HOST": "sparkle.example.com"}, func() {
-		cookie := NewCookie("name", "value")
-		assert.Equal(t, "sparkle.example.com", cookie.Domain)
-		assert.True(t, cookie.HttpOnly)
-		assert.True(t, cookie.Secure)
-		assert.Equal(t, http.SameSiteDefaultMode, cookie.SameSite)
-	})
+	cookie := NewCookie("name", "value")
+	assert.True(t, cookie.HttpOnly)
+	assert.True(t, cookie.Secure)
 }
 
 func TestExpireCookie(t *testing.T) {
-	env.With(env.Vars{"HOST": "sparkle.example.com"}, func() {
-		w := httptest.NewRecorder()
+	w := httptest.NewRecorder()
 
-		ExpireCookie(w, "example")
+	ExpireCookie(w, "example")
 
-		result, err := http.ParseSetCookie(w.Header().Get("Set-Cookie"))
-		require.NoError(t, err)
+	result, err := http.ParseSetCookie(w.Header().Get("Set-Cookie"))
+	require.NoError(t, err)
 
-		assert.Empty(t, result.Value)
-		assert.Equal(t, "sparkle.example.com", result.Domain)
-		assert.Equal(t, -1, result.MaxAge)
-		assert.Equal(t, time.Unix(0, 0).Unix(), result.Expires.Unix())
-		assert.True(t, result.HttpOnly)
-		assert.True(t, result.Secure)
-		assert.Zero(t, result.SameSite)
-	})
+	assert.Empty(t, result.Value)
+	assert.Equal(t, -1, result.MaxAge)
+	assert.Equal(t, time.Unix(0, 0).Unix(), result.Expires.Unix())
+	assert.True(t, result.HttpOnly)
+	assert.True(t, result.Secure)
+	assert.Zero(t, result.SameSite)
 }