From ef050c428a0a893607314a4d5d8d441e445e630a Mon Sep 17 00:00:00 2001 From: mo khan Date: Wed, 7 May 2025 09:06:48 -0700 Subject: refactor: move cookie to web package --- app/controllers/sessions/controller.go | 14 +++++----- app/controllers/sessions/controller_test.go | 8 +++--- app/controllers/sessions/service_test.go | 8 +++--- app/middleware/id_token.go | 4 +-- app/middleware/id_token_test.go | 5 ++-- pkg/web/cookie.go | 33 +++++++++++++++++++++++ pkg/web/cookie/expire.go | 17 ------------ pkg/web/cookie/new.go | 24 ----------------- pkg/web/cookie/new_test.go | 19 ------------- pkg/web/cookie/reset.go | 19 ------------- pkg/web/cookie/reset_test.go | 25 ------------------ pkg/web/cookie_test.go | 41 +++++++++++++++++++++++++++++ 12 files changed, 93 insertions(+), 124 deletions(-) create mode 100644 pkg/web/cookie.go delete mode 100644 pkg/web/cookie/expire.go delete mode 100644 pkg/web/cookie/new.go delete mode 100644 pkg/web/cookie/new_test.go delete mode 100644 pkg/web/cookie/reset.go delete mode 100644 pkg/web/cookie/reset_test.go create mode 100644 pkg/web/cookie_test.go diff --git a/app/controllers/sessions/controller.go b/app/controllers/sessions/controller.go index 13cb2de..9bbc2b4 100644 --- a/app/controllers/sessions/controller.go +++ b/app/controllers/sessions/controller.go @@ -4,11 +4,11 @@ import ( "net/http" "time" - xcookie "github.com/xlgmokha/x/pkg/cookie" + "github.com/xlgmokha/x/pkg/cookie" "github.com/xlgmokha/x/pkg/log" "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/web/cookie" + "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/web" ) type Controller struct { @@ -35,11 +35,11 @@ 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 - http.SetCookie(w, cookie.New( + http.SetCookie(w, web.NewCookie( "oauth_state", nonce, - xcookie.WithSameSite(http.SameSiteLaxMode), - xcookie.WithExpiration(time.Now().Add(10*time.Minute)), + cookie.WithSameSite(http.SameSiteLaxMode), + cookie.WithExpiration(time.Now().Add(10*time.Minute)), )) http.Redirect(w, r, url, http.StatusFound) } @@ -139,11 +139,11 @@ func (c *Controller) Create(w http.ResponseWriter, r *http.Request) { return } - xcookie.Write(w, cookie.New("session", encoded, xcookie.WithExpiration(tokens.Expiry))) + cookie.Write(w, web.NewCookie("session", encoded, cookie.WithExpiration(tokens.Expiry))) http.Redirect(w, r, "/dashboard", http.StatusFound) } func (c *Controller) Destroy(w http.ResponseWriter, r *http.Request) { - cookie.Expire(w, "session") + web.ExpireCookie(w, "session") http.Redirect(w, r, "/", http.StatusFound) } diff --git a/app/controllers/sessions/controller_test.go b/app/controllers/sessions/controller_test.go index 8efc813..c86f2f8 100644 --- a/app/controllers/sessions/controller_test.go +++ b/app/controllers/sessions/controller_test.go @@ -17,7 +17,7 @@ import ( "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/test" - "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/web/cookie" + "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/web" ) func TestSessions(t *testing.T) { @@ -95,7 +95,7 @@ func TestSessions(t *testing.T) { r, w := test.RequestResponse( "GET", "/session/callback?code="+code+"&state=invalid", - test.WithCookie(cookie.New("oauth_state", nonce)), + test.WithCookie(web.NewCookie("oauth_state", 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(cookie.New("oauth_state", nonce)), + test.WithCookie(web.NewCookie("oauth_state", nonce)), ) mux.ServeHTTP(w, r) @@ -185,7 +185,7 @@ 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 := cookie.New("session", "value") + cookie := web.NewCookie("session", "value") r, w := test.RequestResponse("POST", "/session/destroy", test.WithCookie(cookie)) mux.ServeHTTP(w, r) diff --git a/app/controllers/sessions/service_test.go b/app/controllers/sessions/service_test.go index c2de6f4..e5e08fa 100644 --- a/app/controllers/sessions/service_test.go +++ b/app/controllers/sessions/service_test.go @@ -10,7 +10,7 @@ import ( "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/test" - "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/web/cookie" + "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/web" ) func TestService(t *testing.T) { @@ -46,7 +46,7 @@ func TestService(t *testing.T) { r := test.Request( "GET", "/session/callback?code="+code+"&state=invalid", - test.WithCookie(cookie.New("oauth_state", nonce)), + test.WithCookie(web.NewCookie("oauth_state", nonce)), ) tokens, err := svc.Exchange(r) @@ -59,7 +59,7 @@ func TestService(t *testing.T) { r := test.Request( "GET", "/session/callback?code=invalid", - test.WithCookie(cookie.New("oauth_state", nonce)), + test.WithCookie(web.NewCookie("oauth_state", nonce)), ) tokens, err := svc.Exchange(r) @@ -76,7 +76,7 @@ func TestService(t *testing.T) { r := test.Request( "GET", "/session/callback?code="+code+"&state="+nonce, - test.WithCookie(cookie.New("oauth_state", nonce)), + test.WithCookie(web.NewCookie("oauth_state", nonce)), ) tokens, err := svc.Exchange(r) diff --git a/app/middleware/id_token.go b/app/middleware/id_token.go index 2bba8ee..bb874e2 100644 --- a/app/middleware/id_token.go +++ b/app/middleware/id_token.go @@ -7,7 +7,7 @@ import ( "github.com/xlgmokha/x/pkg/x" 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/web/cookie" + "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/web" ) func IDToken(cfg *oidc.OpenID, parsers ...TokenParser) func(http.Handler) http.Handler { @@ -20,7 +20,7 @@ func IDToken(cfg *oidc.OpenID, parsers ...TokenParser) func(http.Handler) http.H if err != nil { log.WithFields(r.Context(), log.Fields{"error": err}) - cookie.Expire(w, "session") + web.ExpireCookie(w, "session") } 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 02c2901..06e9c96 100644 --- a/app/middleware/id_token_test.go +++ b/app/middleware/id_token_test.go @@ -15,7 +15,6 @@ import ( "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/oidc" "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/test" "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/web" - "gitlab.com/gitlab-org/software-supply-chain-security/authorization/sparkled/pkg/web/cookie" "golang.org/x/oauth2" ) @@ -56,7 +55,7 @@ func TestIDToken(t *testing.T) { r, w := test.RequestResponse( "GET", "/example", - test.WithCookie(cookie.New("session", encoded)), + test.WithCookie(web.NewCookie("session", encoded)), ) server.ServeHTTP(w, r) @@ -75,7 +74,7 @@ func TestIDToken(t *testing.T) { r, w := test.RequestResponse( "GET", "/example", - test.WithCookie(cookie.New("session", "invalid")), + test.WithCookie(web.NewCookie("session", "invalid")), ) server.ServeHTTP(w, r) diff --git a/pkg/web/cookie.go b/pkg/web/cookie.go new file mode 100644 index 0000000..d4b1555 --- /dev/null +++ b/pkg/web/cookie.go @@ -0,0 +1,33 @@ +package web + +import ( + "net/http" + + "github.com/xlgmokha/x/pkg/cookie" + "github.com/xlgmokha/x/pkg/env" + "github.com/xlgmokha/x/pkg/x" +) + +func NewCookie(name, value string, options ...x.Option[*http.Cookie]) *http.Cookie { + options = x.Prepend[x.Option[*http.Cookie]]( + options, + cookie.WithName(name), + cookie.WithValue(value), // TODO:: digitally sign the value + cookie.WithPath("/"), + cookie.WithHttpOnly(true), + cookie.WithSecure(true), + cookie.WithSameSite(http.SameSiteDefaultMode), + cookie.WithDomain(env.Fetch("HOST", "localhost")), + ) + + return x.New[*http.Cookie](options...) +} + +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/expire.go b/pkg/web/cookie/expire.go deleted file mode 100644 index 7d90274..0000000 --- a/pkg/web/cookie/expire.go +++ /dev/null @@ -1,17 +0,0 @@ -package cookie - -import ( - "net/http" - - "github.com/xlgmokha/x/pkg/cookie" - "github.com/xlgmokha/x/pkg/env" -) - -func Expire(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/new.go b/pkg/web/cookie/new.go deleted file mode 100644 index be0241d..0000000 --- a/pkg/web/cookie/new.go +++ /dev/null @@ -1,24 +0,0 @@ -package cookie - -import ( - "net/http" - - "github.com/xlgmokha/x/pkg/cookie" - "github.com/xlgmokha/x/pkg/env" - "github.com/xlgmokha/x/pkg/x" -) - -func New(name, value string, options ...x.Option[*http.Cookie]) *http.Cookie { - options = x.Prepend[x.Option[*http.Cookie]]( - options, - cookie.WithName(name), - cookie.WithValue(value), // TODO:: digitally sign the value - cookie.WithPath("/"), - cookie.WithHttpOnly(true), - cookie.WithSecure(true), - cookie.WithSameSite(http.SameSiteDefaultMode), - cookie.WithDomain(env.Fetch("HOST", "localhost")), - ) - - return x.New[*http.Cookie](options...) -} diff --git a/pkg/web/cookie/new_test.go b/pkg/web/cookie/new_test.go deleted file mode 100644 index 5c9e92c..0000000 --- a/pkg/web/cookie/new_test.go +++ /dev/null @@ -1,19 +0,0 @@ -package cookie - -import ( - "net/http" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/xlgmokha/x/pkg/env" -) - -func TestNew(t *testing.T) { - env.With(env.Vars{"HOST": "sparkle.example.com"}, func() { - cookie := New("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) - }) -} diff --git a/pkg/web/cookie/reset.go b/pkg/web/cookie/reset.go deleted file mode 100644 index 39625e6..0000000 --- a/pkg/web/cookie/reset.go +++ /dev/null @@ -1,19 +0,0 @@ -package cookie - -import ( - "net/http" - - "github.com/xlgmokha/x/pkg/cookie" - "github.com/xlgmokha/x/pkg/env" -) - -func Reset(name string) *http.Cookie { - return cookie.Reset( - name, - cookie.WithPath("/"), - cookie.WithHttpOnly(true), - cookie.WithSecure(true), - cookie.WithSameSite(http.SameSiteDefaultMode), - cookie.WithDomain(env.Fetch("HOST", "localhost")), - ) -} diff --git a/pkg/web/cookie/reset_test.go b/pkg/web/cookie/reset_test.go deleted file mode 100644 index 6291eac..0000000 --- a/pkg/web/cookie/reset_test.go +++ /dev/null @@ -1,25 +0,0 @@ -package cookie - -import ( - "net/http" - "testing" - "time" - - "github.com/stretchr/testify/assert" - "github.com/xlgmokha/x/pkg/env" -) - -func TestReset(t *testing.T) { - env.With(env.Vars{"HOST": "sparkle.example.com"}, func() { - result := Reset("example") - - assert.Equal(t, -1, result.MaxAge) - assert.Equal(t, time.Unix(0, 0), result.Expires) - assert.Empty(t, result.Value) - assert.Equal(t, time.Unix(0, 0), result.Expires) - assert.True(t, result.HttpOnly) - assert.True(t, result.Secure) - assert.Equal(t, http.SameSiteDefaultMode, result.SameSite) - assert.Equal(t, "sparkle.example.com", result.Domain) - }) -} diff --git a/pkg/web/cookie_test.go b/pkg/web/cookie_test.go new file mode 100644 index 0000000..60cf7cf --- /dev/null +++ b/pkg/web/cookie_test.go @@ -0,0 +1,41 @@ +package web + +import ( + "net/http" + "net/http/httptest" + "testing" + "time" + + "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) + }) +} + +func TestExpireCookie(t *testing.T) { + env.With(env.Vars{"HOST": "sparkle.example.com"}, func() { + w := httptest.NewRecorder() + + ExpireCookie(w, "example") + + 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) + }) +} -- cgit v1.2.3