Skip to content

Commit

Permalink
add SameSite support
Browse files Browse the repository at this point in the history
Note that upstream support is still pending [0] (and even when merged, this
will require the latest Go version). So this adds a workaround to allow
for setting SameSite on the cookies used, as per the draft [1].

Also changes the options_test TestCookieOptions calls to `t.Errorf`,
since this will run all checks even if one fails. (`t.Fatal[f]` would
stop test execution.)

[0]: golang/go#15867
[1]: https://tools.ietf.org/html/draft-west-first-party-cookies-07

Signed-off-by: Stephan Renatus <srenatus@chef.io>
  • Loading branch information
srenatus committed Nov 27, 2017
1 parent 462b35a commit 0f71f8c
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 22 deletions.
10 changes: 10 additions & 0 deletions manager.go
Expand Up @@ -26,6 +26,7 @@ func NewManager(store Store) *Manager {
path: "/",
persist: false,
secure: false,
sameSite: "",
}

return &Manager{
Expand Down Expand Up @@ -97,6 +98,15 @@ func (m *Manager) Secure(b bool) {
m.opts.secure = b
}

// SameSite sets the 'SameSite' attribute on the session cookie. The default value
// is nil; setting no SameSite attribute. Allowed values are "Lax" and "Strict".
// Note that "" (empty-string) causes SameSite to NOT be set -- don't confuse this
// with the cookie's 'SameSite' attribute (without Lax/Strict), which would default
// to "Strict".
func (m *Manager) SameSite(s string) {
m.opts.sameSite = s
}

// Load returns the session data for the current request.
func (m *Manager) Load(r *http.Request) *Session {
return load(r, m.store, m.opts)
Expand Down
1 change: 1 addition & 0 deletions options.go
Expand Up @@ -17,4 +17,5 @@ type options struct {
path string
persist bool
secure bool
sameSite string
}
46 changes: 34 additions & 12 deletions options_test.go
Expand Up @@ -11,16 +11,19 @@ func TestCookieOptions(t *testing.T) {

_, _, cookie := testRequest(t, testPutString(manager), "")
if strings.Contains(cookie, "Path=/") == false {
t.Fatalf("got %q: expected to contain %q", cookie, "Path=/")
t.Errorf("got %q: expected to contain %q", cookie, "Path=/")
}
if strings.Contains(cookie, "Domain=") == true {
t.Fatalf("got %q: expected to not contain %q", cookie, "Domain=")
t.Errorf("got %q: expected to not contain %q", cookie, "Domain=")
}
if strings.Contains(cookie, "Secure") == true {
t.Fatalf("got %q: expected to not contain %q", cookie, "Secure")
t.Errorf("got %q: expected to not contain %q", cookie, "Secure")
}
if strings.Contains(cookie, "HttpOnly") == false {
t.Fatalf("got %q: expected to contain %q", cookie, "HttpOnly")
t.Errorf("got %q: expected to contain %q", cookie, "HttpOnly")
}
if strings.Contains(cookie, "SameSite") == true {
t.Errorf("got %q: expected to not contain %q", cookie, "SameSite")
}

manager = NewManager(newMockStore())
Expand All @@ -30,36 +33,55 @@ func TestCookieOptions(t *testing.T) {
manager.HttpOnly(false)
manager.Lifetime(time.Hour)
manager.Persist(true)
manager.SameSite("Lax")

_, _, cookie = testRequest(t, testPutString(manager), "")
if strings.Contains(cookie, "Path=/foo") == false {
t.Fatalf("got %q: expected to contain %q", cookie, "Path=/foo")
t.Errorf("got %q: expected to contain %q", cookie, "Path=/foo")
}
if strings.Contains(cookie, "Domain=example.org") == false {
t.Fatalf("got %q: expected to contain %q", cookie, "Domain=example.org")
t.Errorf("got %q: expected to contain %q", cookie, "Domain=example.org")
}
if strings.Contains(cookie, "Secure") == false {
t.Fatalf("got %q: expected to contain %q", cookie, "Secure")
t.Errorf("got %q: expected to contain %q", cookie, "Secure")
}
if strings.Contains(cookie, "HttpOnly") == true {
t.Fatalf("got %q: expected to not contain %q", cookie, "HttpOnly")
t.Errorf("got %q: expected to not contain %q", cookie, "HttpOnly")
}
if strings.Contains(cookie, "Max-Age=3600") == false {
t.Fatalf("got %q: expected to contain %q:", cookie, "Max-Age=86400")
t.Errorf("got %q: expected to contain %q:", cookie, "Max-Age=86400")
}
if strings.Contains(cookie, "Expires=") == false {
t.Fatalf("got %q: expected to contain %q:", cookie, "Expires")
t.Errorf("got %q: expected to contain %q:", cookie, "Expires")
}
if strings.Contains(cookie, "SameSite=Lax") == false {
t.Errorf("got %q: expected to contain %q", cookie, "SameSite=Lax")
}

manager = NewManager(newMockStore())
manager.Lifetime(time.Hour)

_, _, cookie = testRequest(t, testPutString(manager), "")
if strings.Contains(cookie, "Max-Age=") == true {
t.Fatalf("got %q: expected not to contain %q:", cookie, "Max-Age=")
t.Errorf("got %q: expected not to contain %q:", cookie, "Max-Age=")
}
if strings.Contains(cookie, "Expires=") == true {
t.Fatalf("got %q: expected not to contain %q:", cookie, "Expires")
t.Errorf("got %q: expected not to contain %q:", cookie, "Expires")
}

manager = NewManager(newMockStore())
manager.SameSite("Strict")
_, _, cookie = testRequest(t, testPutString(manager), "")
if strings.Contains(cookie, "SameSite=Strict") == false {
t.Errorf("got %q: expected to contain %q", cookie, "SameSite=Strict")
}

manager = NewManager(newMockStore())
// empty string disables
manager.SameSite("")
_, _, cookie = testRequest(t, testPutString(manager), "")
if strings.Contains(cookie, "SameSite") == true {
t.Errorf("got %q: expected to not contain %q", cookie, "SameSite")
}
}

Expand Down
37 changes: 27 additions & 10 deletions session.go
Expand Up @@ -34,6 +34,20 @@ type Session struct {
mu sync.Mutex
}

// cookie wraps http.Cookie, adding SameSite support
type cookie struct {
std *http.Cookie // "stdlib cookie"
sameSite string
}

func (c *cookie) String() string {
v := c.std.String()
if c.sameSite != "" {
v = v + "; SameSite=" + c.sameSite
}
return v
}

func newSession(store Store, opts *options) *Session {
return &Session{
data: make(map[string]interface{}),
Expand Down Expand Up @@ -125,18 +139,21 @@ func (s *Session) write(w http.ResponseWriter) error {
}
}

cookie := &http.Cookie{
Name: s.opts.name,
Value: s.token,
Path: s.opts.path,
Domain: s.opts.domain,
Secure: s.opts.secure,
HttpOnly: s.opts.httpOnly,
cookie := &cookie{
std: &http.Cookie{
Name: s.opts.name,
Value: s.token,
Path: s.opts.path,
Domain: s.opts.domain,
Secure: s.opts.secure,
HttpOnly: s.opts.httpOnly,
},
sameSite: s.opts.sameSite,
}
if s.opts.persist == true {
// Round up expiry time to the nearest second.
cookie.Expires = time.Unix(expiry.Unix()+1, 0)
cookie.MaxAge = int(expiry.Sub(time.Now()).Seconds() + 1)
cookie.std.Expires = time.Unix(expiry.Unix()+1, 0)
cookie.std.MaxAge = int(expiry.Sub(time.Now()).Seconds() + 1)
}

// Overwrite any existing cookie header for the session...
Expand All @@ -150,7 +167,7 @@ func (s *Session) write(w http.ResponseWriter) error {
}
// Or set a new one if necessary.
if !set {
http.SetCookie(w, cookie)
w.Header().Add("Set-Cookie", cookie.String())
}

return nil
Expand Down

0 comments on commit 0f71f8c

Please sign in to comment.