diff --git a/pkg/cookie/cookie.go b/pkg/cookie/cookie.go index 3e31dc3..1d127c8 100644 --- a/pkg/cookie/cookie.go +++ b/pkg/cookie/cookie.go @@ -60,6 +60,10 @@ func Clear(w http.ResponseWriter, name string, opts Options) { cookie.Domain = opts.Domain } + if len(opts.Path) > 0 { + cookie.Path = opts.Path + } + http.SetCookie(w, cookie) } @@ -91,6 +95,10 @@ func Make(name, value string, opts Options) *Cookie { cookie.Domain = opts.Domain } + if len(opts.Path) > 0 { + cookie.Path = opts.Path + } + return &Cookie{cookie} } diff --git a/pkg/cookie/cookie_test.go b/pkg/cookie/cookie_test.go index eba90aa..d8f01ab 100644 --- a/pkg/cookie/cookie_test.go +++ b/pkg/cookie/cookie_test.go @@ -43,6 +43,36 @@ func TestMakeWithDomain(t *testing.T) { assert.Equal(t, ".some.domain", result.Domain) } +func TestMakeWithPath(t *testing.T) { + for _, test := range []struct { + name string + path string + want string + }{ + { + name: "path with multiple subpaths", + path: "/some/path", + want: "/some/path", + }, + { + name: "empty path", + path: "", + want: "/", + }, + { + name: "root path", + path: "/", + want: "/", + }, + } { + t.Run(test.name, func(t *testing.T) { + opts := cookie.DefaultOptions().WithPath(test.path) + result := cookie.Make("some-cookie", "some-value", opts) + assert.Equal(t, test.want, result.Path) + }) + } +} + func TestClear(t *testing.T) { opts := cookie.DefaultOptions() name := "some-name" @@ -91,6 +121,50 @@ func TestClearWithDomain(t *testing.T) { assert.Equal(t, "some.domain", result.Domain) } +func TestClearWithPath(t *testing.T) { + for _, test := range []struct { + name string + path string + want string + }{ + { + name: "path with multiple subpaths", + path: "/some/path", + want: "/some/path", + }, + { + name: "empty path", + path: "", + want: "/", + }, + { + name: "root path", + path: "/", + want: "/", + }, + } { + t.Run(test.name, func(t *testing.T) { + opts := cookie.DefaultOptions().WithPath(test.path) + name := "some-cookie" + + writer := httptest.NewRecorder() + cookie.Clear(writer, name, opts) + + cookies := writer.Result().Cookies() + + var result *http.Cookie + for _, c := range cookies { + if c.Name == name { + result = c + } + } + + assert.NotNil(t, result) + assert.Equal(t, test.want, result.Path) + }) + } +} + func TestCookie_Encrypt(t *testing.T) { crypter := crypto.NewCrypter([]byte(encryptionKey)) diff --git a/pkg/cookie/options.go b/pkg/cookie/options.go index 0aa2ab7..1ea69f8 100644 --- a/pkg/cookie/options.go +++ b/pkg/cookie/options.go @@ -8,6 +8,7 @@ import ( type Options struct { ExpiresIn time.Duration Domain string + Path string SameSite http.SameSite Secure bool } @@ -24,13 +25,18 @@ func (o Options) WithDomain(domain string) Options { return o } -func (o Options) WithSameSite(sameSite http.SameSite) Options { - o.SameSite = sameSite +func (o Options) WithExpiresIn(expiresIn time.Duration) Options { + o.ExpiresIn = expiresIn return o } -func (o Options) WithExpiresIn(expiresIn time.Duration) Options { - o.ExpiresIn = expiresIn +func (o Options) WithPath(path string) Options { + o.Path = path + return o +} + +func (o Options) WithSameSite(sameSite http.SameSite) Options { + o.SameSite = sameSite return o } diff --git a/pkg/cookie/options_test.go b/pkg/cookie/options_test.go index 5c92886..1b1bbc9 100644 --- a/pkg/cookie/options_test.go +++ b/pkg/cookie/options_test.go @@ -17,6 +17,22 @@ func TestDefaultOptions(t *testing.T) { assert.True(t, opts.Secure) assert.Empty(t, opts.ExpiresIn) assert.Empty(t, opts.Domain) + assert.Empty(t, opts.Path) +} + +func TestOptions_WithDomain(t *testing.T) { + domain := ".some.domain" + opts := cookie.Options{}.WithDomain(domain) + + assert.Equal(t, ".some.domain", opts.Domain) + + opts = cookie.Options{ + Domain: ".domain", + } + newOpts := opts.WithDomain(".some.other.domain") + + assert.Equal(t, ".domain", opts.Domain, "original options should be unchanged") + assert.Equal(t, ".some.other.domain", newOpts.Domain, "copy of options should have new value") } func TestOptions_WithExpiresIn(t *testing.T) { @@ -34,6 +50,21 @@ func TestOptions_WithExpiresIn(t *testing.T) { assert.Equal(t, 1*time.Minute, newOpts.ExpiresIn, "copy of options should have new value") } +func TestOptions_WithPath(t *testing.T) { + path := "/some/path" + opts := cookie.Options{}.WithPath(path) + + assert.Equal(t, "/some/path", opts.Path) + + opts = cookie.Options{ + Path: "/some/path", + } + newOpts := opts.WithPath("/some/other/path") + + assert.Equal(t, "/some/path", opts.Path, "original options should be unchanged") + assert.Equal(t, "/some/other/path", newOpts.Path, "copy of options should have new value") +} + func TestOptions_WithSameSite(t *testing.T) { sameSite := http.SameSiteDefaultMode opts := cookie.Options{}.WithSameSite(sameSite) @@ -62,18 +93,3 @@ func TestOptions_WithSecure(t *testing.T) { assert.False(t, opts.Secure, "original options should be unchanged") assert.True(t, newOpts.Secure, "copy of options should have new value") } - -func TestOptions_WithDomain(t *testing.T) { - domain := ".some.domain" - opts := cookie.Options{}.WithDomain(domain) - - assert.Equal(t, ".some.domain", opts.Domain) - - opts = cookie.Options{ - Domain: ".domain", - } - newOpts := opts.WithDomain(".some.other.domain") - - assert.Equal(t, ".domain", opts.Domain, "original options should be unchanged") - assert.Equal(t, ".some.other.domain", newOpts.Domain, "copy of options should have new value") -} diff --git a/pkg/loginstatus/loginstatus.go b/pkg/loginstatus/loginstatus.go index 77c5db1..d6918b5 100644 --- a/pkg/loginstatus/loginstatus.go +++ b/pkg/loginstatus/loginstatus.go @@ -23,6 +23,7 @@ type Client interface { SetCookie(w http.ResponseWriter, token *TokenResponse, opts cookie.Options) HasCookie(r *http.Request) bool ClearCookie(w http.ResponseWriter, opts cookie.Options) + CookieOptions(opts cookie.Options) cookie.Options } func NewClient(config config.Loginstatus, httpClient *http.Client) Client { @@ -71,7 +72,7 @@ func (c client) SetCookie(w http.ResponseWriter, token *TokenResponse, opts cook name := c.config.CookieName expiresIn := time.Duration(token.ExpiresIn) * time.Second - opts = c.cookieOptions(opts). + opts = c.CookieOptions(opts). WithExpiresIn(expiresIn) newCookie := cookie.Make(name, token.AccessToken, opts) @@ -88,15 +89,16 @@ func (c client) HasCookie(r *http.Request) bool { func (c client) ClearCookie(w http.ResponseWriter, opts cookie.Options) { cookieName := c.config.CookieName - opts = c.cookieOptions(opts) + opts = c.CookieOptions(opts) cookie.Clear(w, cookieName, opts) } -func (c client) cookieOptions(opts cookie.Options) cookie.Options { +func (c client) CookieOptions(opts cookie.Options) cookie.Options { domain := c.config.CookieDomain return opts.WithDomain(domain). - WithSameSite(SameSiteMode) + WithSameSite(SameSiteMode). + WithPath("/") } func request(ctx context.Context, url string, token *jwt.AccessToken) (*http.Request, error) { diff --git a/pkg/loginstatus/loginstatus_test.go b/pkg/loginstatus/loginstatus_test.go index d28ba2c..41d77cd 100644 --- a/pkg/loginstatus/loginstatus_test.go +++ b/pkg/loginstatus/loginstatus_test.go @@ -17,6 +17,8 @@ import ( "github.com/nais/wonderwall/pkg/loginstatus" ) +var cookieOpts = cookie.DefaultOptions().WithPath("/some/path") + func TestClient_ExchangeToken(t *testing.T) { server := httptest.NewServer(loginstatusHandler()) httpclient := server.Client() @@ -58,10 +60,11 @@ func TestClient_SetCookie(t *testing.T) { ExpiresIn: 3599, } cfg := newCfg("https://some-server") - opts := cookie.DefaultOptions() + + client := loginstatus.NewClient(cfg, http.DefaultClient) + opts := client.CookieOptions(cookieOpts) writer := httptest.NewRecorder() - client := loginstatus.NewClient(cfg, http.DefaultClient) client.SetCookie(writer, tokenResponse, opts) cookies := writer.Result().Cookies() @@ -88,10 +91,10 @@ func TestClient_SetCookie(t *testing.T) { func TestClient_ClearCookie(t *testing.T) { cfg := newCfg("https://some-server") - opts := cookie.DefaultOptions() + client := loginstatus.NewClient(cfg, http.DefaultClient) + opts := client.CookieOptions(cookieOpts) writer := httptest.NewRecorder() - client := loginstatus.NewClient(cfg, http.DefaultClient) client.ClearCookie(writer, opts) cookies := writer.Result().Cookies() @@ -118,13 +121,13 @@ func TestClient_ClearCookie(t *testing.T) { func TestClient_HasCookie(t *testing.T) { cfg := newCfg("https://some-server") - opts := cookie.DefaultOptions() + client := loginstatus.NewClient(cfg, http.DefaultClient) + opts := client.CookieOptions(cookieOpts) c := cookie.Make(cfg.CookieName, "some-value", opts) r := httptest.NewRequest(http.MethodGet, "/", nil) r.AddCookie(c.Cookie) - client := loginstatus.NewClient(cfg, http.DefaultClient) actual := client.HasCookie(r) assert.True(t, actual) @@ -133,6 +136,45 @@ func TestClient_HasCookie(t *testing.T) { assert.False(t, actual) } +func TestClient_CookieOptions(t *testing.T) { + cfg := newCfg("https://some-server") + client := loginstatus.NewClient(cfg, http.DefaultClient) + + for _, test := range []struct { + name string + opts cookie.Options + }{ + { + name: "default cookie options", + opts: cookie.DefaultOptions(), + }, + { + name: "override domain", + opts: cookie.DefaultOptions().WithDomain(".some.other.domain"), + }, + { + name: "override path", + opts: cookie.DefaultOptions().WithPath("/some/path"), + }, + { + name: "override samesite", + opts: cookie.DefaultOptions().WithSameSite(http.SameSiteStrictMode), + }, + } { + t.Run(test.name, func(t *testing.T) { + opts := client.CookieOptions(test.opts) + + assert.Empty(t, opts.ExpiresIn) + assert.True(t, opts.Secure) + + // options below should never be overridden regardless of input + assert.Equal(t, cfg.CookieDomain, opts.Domain) + assert.Equal(t, "/", opts.Path) + assert.Equal(t, http.SameSiteDefaultMode, opts.SameSite) + }) + } +} + func newCfg(serverURL string) config.Loginstatus { return config.Loginstatus{ Enabled: true, diff --git a/pkg/router/handler.go b/pkg/router/handler.go index 028aa73..912ee11 100644 --- a/pkg/router/handler.go +++ b/pkg/router/handler.go @@ -45,9 +45,12 @@ func NewHandler( } loginstatusClient := loginstatus.NewClient(cfg.Loginstatus, http.DefaultClient) + cookiePath := config.ParseIngress(cfg.Ingress) + cookieOpts := cookie.DefaultOptions().WithPath(cookiePath) + return &Handler{ Config: cfg, - CookieOptions: cookie.DefaultOptions(), + CookieOptions: cookieOpts, Crypter: crypter, Httplogger: httplogger, lock: sync.Mutex{},