From 0b0186b4c33a6ce5057141bb0a64046fe1cc6cae Mon Sep 17 00:00:00 2001 From: Matt Silverlock Date: Sun, 12 Jun 2016 13:39:42 -0700 Subject: [PATCH] [bugfix] Support a cookie MaxAge of 0. - As per #38 - we now support a MaxAge of 0 to allow for session cookie support. gorilla/csrf's CSRF tokens are designed to be reasonably long lived (12 hours), but there are some applications that require this. - Note that setting a MaxAge < 0 will default to 12 hours, so you must explcitly set csrf.MaxAge(0) to invoke this behaviour. --- csrf.go | 2 +- store.go | 4 ++-- store_test.go | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/csrf.go b/csrf.go index 6d12cc0..ff54833 100644 --- a/csrf.go +++ b/csrf.go @@ -127,7 +127,7 @@ func Protect(authKey []byte, opts ...Option) func(http.Handler) http.Handler { cs.opts.ErrorHandler = http.HandlerFunc(unauthorizedHandler) } - if cs.opts.MaxAge < 1 { + if cs.opts.MaxAge < 0 { // Default of 12 hours cs.opts.MaxAge = defaultAge } diff --git a/store.go b/store.go index 6638219..39f47ad 100644 --- a/store.go +++ b/store.go @@ -68,11 +68,11 @@ func (cs *cookieStore) Save(token []byte, w http.ResponseWriter) error { } // Set the Expires field on the cookie based on the MaxAge + // If MaxAge <= 0, we don't set the Expires attribute, making the cookie + // session-only. if cs.maxAge > 0 { cookie.Expires = time.Now().Add( time.Duration(cs.maxAge) * time.Second) - } else { - cookie.Expires = time.Unix(1, 0) } // Write the authenticated cookie to the response. diff --git a/store_test.go b/store_test.go index 4345990..311a1f7 100644 --- a/store_test.go +++ b/store_test.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "strings" "testing" "github.com/gorilla/securecookie" @@ -93,3 +94,35 @@ func TestCookieEncode(t *testing.T) { t.Fatal("cookiestore did not report an invalid hashkey on encode") } } + +// TestMaxAgeZero tests that setting MaxAge(0) does not set the Expires +// attribute on the cookie. +func TestMaxAgeZero(t *testing.T) { + var age = 0 + + s := http.NewServeMux() + s.HandleFunc("/", testHandler) + + r, err := http.NewRequest("GET", "/", nil) + if err != nil { + t.Fatal(err) + } + + rr := httptest.NewRecorder() + p := Protect(testKey, MaxAge(age))(s) + p.ServeHTTP(rr, r) + + if rr.Code != http.StatusOK { + t.Fatalf("middleware failed to pass to the next handler: got %v want %v", + rr.Code, http.StatusOK) + } + + if rr.Header().Get("Set-Cookie") == "" { + t.Fatalf("cookie not set: got %q", rr.Header().Get("Set-Cookie")) + } + + cookie := rr.Header().Get("Set-Cookie") + if !strings.Contains(cookie, "HttpOnly") || strings.Contains(cookie, "Expires") { + t.Fatalf("cookie incorrectly has the Expires attribute set: got %q", cookie) + } +}