From 437dee03159d1f6b39d9634e92604e904e5faa9d Mon Sep 17 00:00:00 2001 From: Bobby DeSimone Date: Fri, 5 Jul 2019 12:32:35 -0700 Subject: [PATCH] internal/sessions: allow manual session scope --- CHANGELOG.md | 1 + authenticate/authenticate.go | 1 + internal/sessions/cookie_store.go | 17 +++++---- internal/sessions/cookie_store_test.go | 49 ++++++++++++++------------ 4 files changed, 37 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 22978c19d..f2c8d0811 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ - Fixed Azure group lookups [GH-190] - If a session is too large (over 4096 bytes) Pomerium will no longer fail silently. [GH-211] - Internal URLs like dashboard now start auth process to login a user if no session is found [GH-205]. +- When set,`CookieDomain` lets a user set the scope of the user session. CSRF cookies will still always be scoped at the individual route level. [GH-181] ## v0.0.5 diff --git a/authenticate/authenticate.go b/authenticate/authenticate.go index 3d42dd377..66b23f27f 100644 --- a/authenticate/authenticate.go +++ b/authenticate/authenticate.go @@ -66,6 +66,7 @@ func New(opts config.Options) (*Authenticate, error) { cookieStore, err := sessions.NewCookieStore( &sessions.CookieStoreOptions{ Name: opts.CookieName, + CookieDomain: opts.CookieDomain, CookieSecure: opts.CookieSecure, CookieHTTPOnly: opts.CookieHTTPOnly, CookieExpire: opts.CookieExpire, diff --git a/internal/sessions/cookie_store.go b/internal/sessions/cookie_store.go index f999dc7f4..aeceb07d5 100644 --- a/internal/sessions/cookie_store.go +++ b/internal/sessions/cookie_store.go @@ -68,20 +68,19 @@ func NewCookieStore(opts *CookieStoreOptions) (*CookieStore, error) { } func (s *CookieStore) makeCookie(req *http.Request, name string, value string, expiration time.Duration, now time.Time) *http.Cookie { - // if csrf, scope cookie to the route or service specific domain domain := req.Host - if h, _, err := net.SplitHostPort(domain); err == nil { - domain = h - } - if s.CookieDomain != "" { - domain = s.CookieDomain - } - // Non-CSRF sessions can shared, and set domain-wide - if !strings.Contains(name, "csrf") { + if name == s.csrfName() { + domain = req.Host + } else if s.CookieDomain != "" { + domain = s.CookieDomain + } else { domain = splitDomain(domain) } + if h, _, err := net.SplitHostPort(domain); err == nil { + domain = h + } c := &http.Cookie{ Name: name, Value: value, diff --git a/internal/sessions/cookie_store_test.go b/internal/sessions/cookie_store_test.go index 32e808cec..377d34643 100644 --- a/internal/sessions/cookie_store_test.go +++ b/internal/sessions/cookie_store_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" "github.com/pomerium/pomerium/internal/cryptutil" ) @@ -110,35 +111,39 @@ func TestCookieStore_makeCookie(t *testing.T) { name string domain string - cookieName string - value string - expiration time.Duration - want *http.Cookie - wantCSRF *http.Cookie + cookieDomain string + cookieName string + value string + expiration time.Duration + want *http.Cookie + wantCSRF *http.Cookie }{ - {"good", "http://httpbin.corp.pomerium.io", "_pomerium", "value", 0, &http.Cookie{Name: "_pomerium", Value: "value", Path: "/", Domain: "corp.pomerium.io", Secure: true, HttpOnly: true}, &http.Cookie{Name: "_pomerium_csrf", Value: "value", Path: "/", Domain: "httpbin.corp.pomerium.io", Secure: true, HttpOnly: true}}, - {"domains with https", "https://httpbin.corp.pomerium.io", "_pomerium", "value", 0, &http.Cookie{Name: "_pomerium", Value: "value", Path: "/", Domain: "corp.pomerium.io", Secure: true, HttpOnly: true}, &http.Cookie{Name: "_pomerium_csrf", Value: "value", Path: "/", Domain: "httpbin.corp.pomerium.io", Secure: true, HttpOnly: true}}, - {"domain with port", "http://httpbin.corp.pomerium.io:443", "_pomerium", "value", 0, &http.Cookie{Name: "_pomerium", Value: "value", Path: "/", Domain: "corp.pomerium.io", Secure: true, HttpOnly: true}, &http.Cookie{Name: "_pomerium_csrf", Value: "value", Path: "/", Domain: "httpbin.corp.pomerium.io", Secure: true, HttpOnly: true}}, - {"expiration set", "http://httpbin.corp.pomerium.io:443", "_pomerium", "value", 10 * time.Second, &http.Cookie{Expires: now.Add(10 * time.Second), Name: "_pomerium", Value: "value", Path: "/", Domain: "corp.pomerium.io", Secure: true, HttpOnly: true}, &http.Cookie{Expires: now.Add(10 * time.Second), Name: "_pomerium_csrf", Value: "value", Path: "/", Domain: "httpbin.corp.pomerium.io", Secure: true, HttpOnly: true}}, + {"good", "http://httpbin.corp.pomerium.io", "", "_pomerium", "value", 0, &http.Cookie{Name: "_pomerium", Value: "value", Path: "/", Domain: "corp.pomerium.io", Secure: true, HttpOnly: true}, &http.Cookie{Name: "_pomerium_csrf", Value: "value", Path: "/", Domain: "httpbin.corp.pomerium.io", Secure: true, HttpOnly: true}}, + {"domains with https", "https://httpbin.corp.pomerium.io", "", "_pomerium", "value", 0, &http.Cookie{Name: "_pomerium", Value: "value", Path: "/", Domain: "corp.pomerium.io", Secure: true, HttpOnly: true}, &http.Cookie{Name: "_pomerium_csrf", Value: "value", Path: "/", Domain: "httpbin.corp.pomerium.io", Secure: true, HttpOnly: true}}, + {"domain with port", "http://httpbin.corp.pomerium.io:443", "", "_pomerium", "value", 0, &http.Cookie{Name: "_pomerium", Value: "value", Path: "/", Domain: "corp.pomerium.io", Secure: true, HttpOnly: true}, &http.Cookie{Name: "_pomerium_csrf", Value: "value", Path: "/", Domain: "httpbin.corp.pomerium.io", Secure: true, HttpOnly: true}}, + {"expiration set", "http://httpbin.corp.pomerium.io:443", "", "_pomerium", "value", 10 * time.Second, &http.Cookie{Expires: now.Add(10 * time.Second), Name: "_pomerium", Value: "value", Path: "/", Domain: "corp.pomerium.io", Secure: true, HttpOnly: true}, &http.Cookie{Expires: now.Add(10 * time.Second), Name: "_pomerium_csrf", Value: "value", Path: "/", Domain: "httpbin.corp.pomerium.io", Secure: true, HttpOnly: true}}, + {"good", "http://httpbin.corp.pomerium.io", "pomerium.io", "_pomerium", "value", 0, &http.Cookie{Name: "_pomerium", Value: "value", Path: "/", Domain: "pomerium.io", Secure: true, HttpOnly: true}, &http.Cookie{Name: "_pomerium_csrf", Value: "value", Path: "/", Domain: "httpbin.corp.pomerium.io", Secure: true, HttpOnly: true}}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { r := httptest.NewRequest("GET", tt.domain, nil) - o := &CookieStoreOptions{ - Name: "_pomerium", - CookieSecure: true, - CookieHTTPOnly: true, - CookieDomain: "httpbin.corp.pomerium.io", - CookieExpire: 10 * time.Second, - CookieCipher: cipher} - - s, _ := NewCookieStore(o) - if got := s.makeCookie(r, tt.cookieName, tt.value, tt.expiration, now); !reflect.DeepEqual(got, tt.want) { - t.Errorf("CookieStore.makeCookie() = \n%#v, \nwant\n%#v", got, tt.want) + s, err := NewCookieStore( + &CookieStoreOptions{ + Name: "_pomerium", + CookieSecure: true, + CookieHTTPOnly: true, + CookieDomain: tt.cookieDomain, + CookieExpire: 10 * time.Second, + CookieCipher: cipher}) + if err != nil { + t.Fatal(err) } - if got := s.makeSessionCookie(r, tt.value, tt.expiration, now); !reflect.DeepEqual(got, tt.want) { - t.Errorf("CookieStore.makeCookie() = \n%#v, \nwant\n%#v", got, tt.want) + if diff := cmp.Diff(s.makeCookie(r, tt.cookieName, tt.value, tt.expiration, now), tt.want); diff != "" { + t.Errorf("CookieStore.makeCookie() = \n%s", diff) + } + if diff := cmp.Diff(s.makeSessionCookie(r, tt.value, tt.expiration, now), tt.want); diff != "" { + t.Errorf("CookieStore.makeSessionCookie() = \n%s", diff) } got := s.makeCSRFCookie(r, tt.value, tt.expiration, now) tt.wantCSRF.Name = "_pomerium_csrf"