Skip to content

Commit

Permalink
feat(cookie): remove expiry options
Browse files Browse the repository at this point in the history
Always create session cookies instead of
persistent cookies with expiry.
  • Loading branch information
tronghn committed Dec 19, 2023
1 parent e008320 commit 9c2d1cb
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 56 deletions.
15 changes: 0 additions & 15 deletions pkg/cookie/cookie.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,6 @@ func (in *Cookie) Decrypt(crypter crypto.Crypter) (string, error) {
return string(plaintext), err
}

// UnsetExpiry sets the MaxAge and Expires fields to their 'nil' values to unset them. For most user agents, this means
// that the cookie should expire at the 'end of a session', typically when the browser is closed.
//
// The cookie should still be explicitly cleared/expired whenever it is no longer needed.
func (in *Cookie) UnsetExpiry() {
in.MaxAge = 0
in.Expires = time.Time{}
}

func Clear(w http.ResponseWriter, name string, opts Options) {
expires := time.Unix(0, 0)
maxAge := -1
Expand Down Expand Up @@ -106,13 +97,8 @@ func GetDecrypted(r *http.Request, key string, crypter crypto.Crypter) (string,
}

func Make(name, value string, opts Options) *Cookie {
expires := time.Now().Add(opts.ExpiresIn)
maxAge := int(opts.ExpiresIn.Seconds())

cookie := &http.Cookie{
Expires: expires,
HttpOnly: true,
MaxAge: maxAge,
Name: name,
Path: "/",
SameSite: opts.SameSite,
Expand Down Expand Up @@ -149,7 +135,6 @@ func SetLegacyCookie(w http.ResponseWriter, value string, opts Options) {
c := Make(loginservice, value, opts.
WithSameSite(http.SameSiteLaxMode).
WithPath("/"))
c.UnsetExpiry()
Set(w, c)
}

Expand Down
12 changes: 5 additions & 7 deletions pkg/cookie/cookie_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,15 @@ import (
var encryptionKey = `G8Roe6AcoBpdr5GhO3cs9iORl4XIC8eq` // 256 bits key

func TestMake(t *testing.T) {
expiresIn := 5 * time.Minute
opts := cookie.DefaultOptions().WithExpiresIn(expiresIn)
opts := cookie.DefaultOptions()

name := "some-cookie"
value := "some-value"

result := cookie.Make(name, value, opts)

shouldExpireBefore := time.Now().Add(expiresIn)
assert.True(t, result.Expires.Before(shouldExpireBefore))
assert.Equal(t, int(opts.ExpiresIn.Seconds()), result.MaxAge)
assert.True(t, result.Expires.IsZero())
assert.Equal(t, 0, result.MaxAge)
assert.True(t, result.HttpOnly)
assert.Equal(t, name, result.Name)
assert.Equal(t, value, result.Value)
Expand Down Expand Up @@ -166,7 +164,7 @@ func TestClearWithPath(t *testing.T) {
func TestCookie_Encrypt(t *testing.T) {
crypter := crypto.NewCrypter([]byte(encryptionKey))

opts := cookie.DefaultOptions().WithExpiresIn(1 * time.Minute)
opts := cookie.DefaultOptions()
name := "some-name"
value := "some-value"

Expand All @@ -179,7 +177,7 @@ func TestCookie_Encrypt(t *testing.T) {
func TestCookie_Decrypt(t *testing.T) {
crypter := crypto.NewCrypter([]byte(encryptionKey))

opts := cookie.DefaultOptions().WithExpiresIn(1 * time.Minute)
opts := cookie.DefaultOptions()
name := "some-name"
value := "some-value"

Expand Down
15 changes: 4 additions & 11 deletions pkg/cookie/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,13 @@ package cookie

import (
"net/http"
"time"
)

type Options struct {
ExpiresIn time.Duration
Domain string
Path string
SameSite http.SameSite
Secure bool
Domain string
Path string
SameSite http.SameSite
Secure bool
}

func DefaultOptions() Options {
Expand All @@ -25,11 +23,6 @@ func (o Options) WithDomain(domain string) Options {
return o
}

func (o Options) WithExpiresIn(expiresIn time.Duration) Options {
o.ExpiresIn = expiresIn
return o
}

func (o Options) WithPath(path string) Options {
o.Path = path
return o
Expand Down
17 changes: 0 additions & 17 deletions pkg/cookie/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package cookie_test
import (
"net/http"
"testing"
"time"

"github.com/stretchr/testify/assert"

Expand All @@ -15,7 +14,6 @@ func TestDefaultOptions(t *testing.T) {

assert.Equal(t, http.SameSiteLaxMode, opts.SameSite)
assert.True(t, opts.Secure)
assert.Empty(t, opts.ExpiresIn)
assert.Empty(t, opts.Domain)
assert.Empty(t, opts.Path)
}
Expand All @@ -35,21 +33,6 @@ func TestOptions_WithDomain(t *testing.T) {
assert.Equal(t, ".some.other.domain", newOpts.Domain, "copy of options should have new value")
}

func TestOptions_WithExpiresIn(t *testing.T) {
expiresIn := 1 * time.Minute
opts := cookie.Options{}.WithExpiresIn(expiresIn)

assert.Equal(t, 1*time.Minute, opts.ExpiresIn)

opts = cookie.Options{
ExpiresIn: 2 * time.Minute,
}
newOpts := opts.WithExpiresIn(expiresIn)

assert.Equal(t, 2*time.Minute, opts.ExpiresIn, "original options should be unchanged")
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)
Expand Down
9 changes: 3 additions & 6 deletions pkg/handler/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,7 @@ func (s *Standalone) Login(w http.ResponseWriter, r *http.Request) {
return
}

opts := s.GetCookieOptions(r).
WithExpiresIn(1 * time.Hour).
WithSameSite(http.SameSiteLaxMode)
opts := s.GetCookieOptions(r).WithSameSite(http.SameSiteLaxMode)
err = login.SetCookie(w, opts, s.Crypter, canonicalRedirect)
if err != nil {
s.InternalError(w, r, fmt.Errorf("login: setting cookie: %w", err))
Expand Down Expand Up @@ -220,7 +218,7 @@ func (s *Standalone) LoginCallback(w http.ResponseWriter, r *http.Request) {
return
}

err = sess.SetCookie(w, opts.WithExpiresIn(sessionLifetime), s.Crypter)
err = sess.SetCookie(w, opts, s.Crypter)
if err != nil {
s.InternalError(w, r, fmt.Errorf("callback: setting session cookie: %w", err))
return
Expand Down Expand Up @@ -300,8 +298,7 @@ func (s *Standalone) Logout(w http.ResponseWriter, r *http.Request) {
canonicalRedirect = s.Redirect.Canonical(r)
}

opts := s.CookieOptions.WithExpiresIn(5 * time.Minute)
err = logout.SetCookie(w, opts, s.Crypter, canonicalRedirect)
err = logout.SetCookie(w, s.CookieOptions, s.Crypter, canonicalRedirect)
if err != nil {
s.InternalError(w, r, fmt.Errorf("logout: setting logout cookie: %w", err))
return
Expand Down

0 comments on commit 9c2d1cb

Please sign in to comment.