From 978449c0b58e791d9fdd18889f7aada01cdf1b21 Mon Sep 17 00:00:00 2001 From: Kush Mansingh <12158241+kushmansingh@users.noreply.github.com> Date: Wed, 7 Oct 2020 15:04:54 -0400 Subject: [PATCH] cookie: add option to set samesite property --- main.go | 1 + oauthproxy.go | 7 ++++++- oauthproxy_test.go | 10 ++++++++-- options.go | 18 ++++++++++++++++++ options_test.go | 2 +- 5 files changed, 34 insertions(+), 4 deletions(-) diff --git a/main.go b/main.go index e3d4445cd..738258cde 100644 --- a/main.go +++ b/main.go @@ -61,6 +61,7 @@ func main() { flagSet.String("cookie-name", "_oauth2_proxy", "the name of the cookie that the oauth_proxy creates") flagSet.String("cookie-secret", "", "the seed string for secure cookies (optionally base64 encoded)") flagSet.String("cookie-domain", "", "an optional cookie domain to force cookies to (ie: .yourcompany.com)*") + flagSet.String("cookie-samesite", "None", "whether the cookie can be accessed by other domains (None, Lax or Strict)") flagSet.Duration("cookie-expire", time.Duration(168)*time.Hour, "expire timeframe for cookie") flagSet.Duration("cookie-refresh", time.Duration(0), "refresh the cookie after this duration; 0 to disable") flagSet.Bool("cookie-secure", true, "set secure (HTTPS) cookie flag") diff --git a/oauthproxy.go b/oauthproxy.go index 7ce1777c0..e5cd6422f 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -41,6 +41,7 @@ type OAuthProxy struct { CookieName string CSRFCookieName string CookieDomain string + CookieSameSite http.SameSite CookieSecure bool CookieHttpOnly bool CookieExpire time.Duration @@ -201,7 +202,7 @@ func NewOAuthProxy(opts *Options, validator func(string) bool) *OAuthProxy { refresh = fmt.Sprintf("after %s", opts.CookieRefresh) } - log.Printf("Cookie settings: name:%s secure(https):%v httponly:%v expiry:%s domain:%s refresh:%s", opts.CookieName, opts.CookieSecure, opts.CookieHttpOnly, opts.CookieExpire, opts.CookieDomain, refresh) + log.Printf("Cookie settings: name:%s samesite: %v secure(https):%v httponly:%v expiry:%s domain:%s refresh:%s", opts.CookieName, opts.CookieSameSite, opts.CookieSecure, opts.CookieHttpOnly, opts.CookieExpire, opts.CookieDomain, refresh) var cipher *cookie.Cipher if opts.PassAccessToken || (opts.CookieRefresh != time.Duration(0)) { @@ -212,11 +213,14 @@ func NewOAuthProxy(opts *Options, validator func(string) bool) *OAuthProxy { } } + sameSite, _ := validateCookieSameSite(opts.CookieSameSite) + return &OAuthProxy{ CookieName: opts.CookieName, CSRFCookieName: fmt.Sprintf("%v_%v", opts.CookieName, "csrf"), CookieSeed: opts.CookieSecret, CookieDomain: opts.CookieDomain, + CookieSameSite: sameSite, CookieSecure: opts.CookieSecure, CookieHttpOnly: opts.CookieHttpOnly, CookieExpire: opts.CookieExpire, @@ -330,6 +334,7 @@ func (p *OAuthProxy) makeCookie(req *http.Request, name string, value string, ex Domain: p.CookieDomain, HttpOnly: p.CookieHttpOnly, Secure: p.CookieSecure, + SameSite: p.CookieSameSite, Expires: now.Add(expiration), } } diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 4c00fa341..2a93c60ab 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -15,8 +15,6 @@ import ( "testing" "time" - "github.com/bitly/oauth2_proxy/providers" - "github.com/bmizerany/assert" "github.com/mbland/hmacauth" "github.com/samsarahq/oauth2_proxy/providers" "github.com/stretchr/testify/assert" @@ -590,6 +588,14 @@ func TestProcessCookieNoCookieError(t *testing.T) { } } +func TestProcessCookieSameSiteSet(t *testing.T) { + pc_test := NewProcessCookieTestWithDefaults() + pc_test.proxy.CookieSameSite = http.SameSiteStrictMode + + cookie := pc_test.MakeCookie("", time.Now()) + assert.Equal(t, cookie.SameSite, http.SameSiteStrictMode) +} + func TestProcessCookieRefreshNotSet(t *testing.T) { pc_test := NewProcessCookieTestWithDefaults() pc_test.proxy.CookieExpire = time.Duration(23) * time.Hour diff --git a/options.go b/options.go index 50a5a8f41..5b6ea35de 100644 --- a/options.go +++ b/options.go @@ -47,6 +47,7 @@ type Options struct { CookieName string `flag:"cookie-name" cfg:"cookie_name" env:"OAUTH2_PROXY_COOKIE_NAME"` CookieSecret string `flag:"cookie-secret" cfg:"cookie_secret" env:"OAUTH2_PROXY_COOKIE_SECRET"` CookieDomain string `flag:"cookie-domain" cfg:"cookie_domain" env:"OAUTH2_PROXY_COOKIE_DOMAIN"` + CookieSameSite string `flag:"cookie-samesite" cfg:"cookie_samesite" env:"OAUTH2_PROXY_COOKIE_SAMESITE"` CookieExpire time.Duration `flag:"cookie-expire" cfg:"cookie_expire" env:"OAUTH2_PROXY_COOKIE_EXPIRE"` CookieRefresh time.Duration `flag:"cookie-refresh" cfg:"cookie_refresh" env:"OAUTH2_PROXY_COOKIE_REFRESH"` CookieSecure bool `flag:"cookie-secure" cfg:"cookie_secure"` @@ -103,6 +104,7 @@ func NewOptions() *Options { DisplayHtpasswdForm: true, CookieName: "_oauth2_proxy", CookieSecure: true, + CookieSameSite: "None", CookieHttpOnly: true, CookieExpire: time.Duration(168) * time.Hour, CookieRefresh: time.Duration(0), @@ -239,6 +241,10 @@ func (o *Options) Validate() error { msgs = parseSignatureKey(o, msgs) msgs = validateCookieName(o, msgs) + if _, ok := validateCookieSameSite(o.CookieSameSite); !ok { + msgs = append(msgs, "cookie same site must be strict, lax or none") + } + if len(msgs) != 0 { return fmt.Errorf("Invalid configuration:\n %s", strings.Join(msgs, "\n ")) @@ -313,6 +319,18 @@ func validateCookieName(o *Options, msgs []string) []string { return msgs } +func validateCookieSameSite(o string) (http.SameSite, bool) { + switch strings.ToLower(o) { + case "strict": + return http.SameSiteStrictMode, true + case "lax": + return http.SameSiteLaxMode, true + case "none": + return http.SameSiteNoneMode, true + } + return http.SameSiteDefaultMode, false +} + func addPadding(secret string) string { padding := len(secret) % 4 switch padding { diff --git a/options_test.go b/options_test.go index 23ae9fb89..5c8bf35a9 100644 --- a/options_test.go +++ b/options_test.go @@ -102,7 +102,7 @@ func TestProxyURLsError(t *testing.T) { assert.NotEqual(t, nil, err) expected := errorMsg([]string{ - "error parsing upstream: parse 127.0.0.1:8081: " + + "error parsing upstream: parse \"127.0.0.1:8081\": " + "first path segment in URL cannot contain colon"}) assert.Equal(t, expected, err.Error()) }