Skip to content

Commit 8f2d3ac

Browse files
authored
sessions pkg: omit domain attribute in session cookie (buzzfeed#329)
* pkg: cookie_store: omit domain value unless explicitly set * pkg: cookie_store: small adjustments for the sake of (mostly) logging * update documentation further
1 parent 7018b6f commit 8f2d3ac

File tree

4 files changed

+23
-11
lines changed

4 files changed

+23
-11
lines changed

docs/sso_authenticator_config.md

+3-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ Defaults for the below settings can be found here: https://github.com/buzzfeed/s
1111
```
1212
SESSION_COOKIE_NAME - string - name associated with the session cookie
1313
SESSION_COOKIE_SECRET - string - seed string for secure cookies
14-
SESSION_COOKIE_DOMAIN - string - cookie domain to force cookies to (ie: .yourcompany.com)*
14+
SESSION_COOKIE_DOMAIN - string - cookie domain (and subdomains) to force cookies to (ie: .yourcompany.com). If this is set,
15+
cookies will be valid on subdomains too. To restrict to only the request's domain, leave this unset.
16+
Reference: https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#define_where_cookies_are_sent
1517
SESSION_KEY - string - seed string for secure auth codes
1618
SESSION_COOKIE_SECURE - bool - set secure (HTTPS) cookie flag
1719
SESSION_COOKIE_HTTPONLY - bool - set 'httponly' cookie flag

docs/sso_proxy_config.md

+3-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ Defaults for the below settings can be found here: https://github.com/buzzfeed/s
1111
```
1212
SESSION_COOKIE_NAME - string - name associated with the session cookie
1313
SESSION_COOKIE_SECRET - string - seed string for secure cookies
14-
SESSION_COOKIE_DOMAIN - string - cookie domain to force cookies to (ie: .yourcompany.com)*
14+
SESSION_COOKIE_DOMAIN - string - cookie domain (and subdomains) to force cookies to (ie: .yourcompany.com). If this is set,
15+
cookies will be valid on subdomains too. To restrict to only the request's domain, leave this unset.
16+
Reference: https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#define_where_cookies_are_sent
1517
SESSION_COOKIE_SECURE - bool - set secure (HTTPS) cookie flag
1618
SESSION_COOKIE_HTTPONLY - bool - set 'httponly' cookie flag
1719
SESSION_TTL_LIFETIME - time.Duration - 'time-to-live' of a session lifetime

internal/pkg/sessions/cookie_store.go

+7-7
Original file line numberDiff line numberDiff line change
@@ -81,22 +81,22 @@ func NewCookieStore(cookieName string, optFuncs ...func(*CookieStore) error) (*C
8181

8282
func (s *CookieStore) makeCookie(req *http.Request, name string, value string, expiration time.Duration, now time.Time) *http.Cookie {
8383
logger := log.NewLogEntry()
84-
domain := req.Host
85-
if h, _, err := net.SplitHostPort(domain); err == nil {
86-
domain = h
87-
}
84+
8885
if s.CookieDomain != "" {
86+
domain := req.Host
87+
if h, _, err := net.SplitHostPort(domain); err == nil {
88+
domain = h
89+
}
8990
if !strings.HasSuffix(domain, s.CookieDomain) {
90-
logger.WithRequestHost(domain).WithCookieDomain(s.CookieDomain).Warn("Warning: Using configured cookie domain.")
91+
logger.WithRequestHost(domain).WithCookieDomain(s.CookieDomain).Warn("Warning: Using explicitly configured cookie domain.")
9192
}
92-
domain = s.CookieDomain
9393
}
9494

9595
return &http.Cookie{
9696
Name: name,
9797
Value: value,
9898
Path: "/",
99-
Domain: domain,
99+
Domain: s.CookieDomain,
100100
HttpOnly: s.CookieHTTPOnly,
101101
Secure: s.CookieSecure,
102102
Expires: now.Add(expiration),

internal/pkg/sessions/cookie_store_test.go

+10-2
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,16 @@ func TestMakeSessionCookie(t *testing.T) {
104104
expectedCookie *http.Cookie
105105
}{
106106
{
107+
// In order to prevent subdomains being included, the Domain
108+
// value is left empty by default.
109+
// While empty, it defaults to the URL domain with subdomains _excluded_.
110+
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie
107111
name: "default cookie domain",
108112
expectedCookie: &http.Cookie{
109113
Name: cookieName,
110114
Value: cookieValue,
111115
Path: "/",
112-
Domain: "www.example.com",
116+
Domain: "",
113117
HttpOnly: true,
114118
Secure: true,
115119
Expires: now.Add(expiration),
@@ -159,12 +163,16 @@ func TestMakeSessionCSRFCookie(t *testing.T) {
159163
expectedCookie *http.Cookie
160164
}{
161165
{
166+
// In order to prevent subdomains being included, the Domain
167+
// value is left empty by default.
168+
// While empty, it defaults to the URL domain with subdomains _excluded_.
169+
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie
162170
name: "default cookie domain",
163171
expectedCookie: &http.Cookie{
164172
Name: csrfName,
165173
Value: cookieValue,
166174
Path: "/",
167-
Domain: "www.example.com",
175+
Domain: "",
168176
HttpOnly: true,
169177
Secure: true,
170178
Expires: now.Add(expiration),

0 commit comments

Comments
 (0)