Merge pull request #182 from k-wall/samesite

Add configuration for cookie 'SameSite' value.

Author: openshift-merge-robot

README.md

@@ -231,6 +231,7 @@ Usage of oauth-proxy:
   -cookie-httponly: set HttpOnly cookie flag (default true)
   -cookie-name string: the name of the cookie that the oauth_proxy creates (default "_oauth2_proxy")
   -cookie-refresh duration: refresh the cookie after this duration; 0 to disable
+  -cookie-samesite string | set SameSite cookie attribute (ie: `"lax"`, `"strict"`, `"none"`, or `""`)
   -cookie-secret string: the seed string for secure cookies (optionally base64 encoded)
   -cookie-secret-file string: same as "-cookie-secret" but read it from a file
   -cookie-secure: set secure (HTTPS) cookie flag (default true)
@@ -285,6 +286,7 @@ The following environment variables can be used in place of the corresponding co
 - `OAUTH2_PROXY_CLIENT_ID`
 - `OAUTH2_PROXY_CLIENT_SECRET`
 - `OAUTH2_PROXY_COOKIE_NAME`
+- `OAUTH2_PROXY_COOKIE_SAMESITE`
 - `OAUTH2_PROXY_COOKIE_SECRET`
 - `OAUTH2_PROXY_COOKIE_DOMAIN`
 - `OAUTH2_PROXY_COOKIE_EXPIRE`

main.go

@@ -84,6 +84,7 @@ func main() {
 	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")
 	flagSet.Bool("cookie-httponly", true, "set HttpOnly cookie flag")
+	flagSet.String("cookie-samesite", "", "set SameSite cookie attribute (ie: \"lax\", \"strict\", \"none\", or \"\"). ")
 
 	flagSet.Bool("request-logging", false, "Log requests to stdout")
 

oauthproxy.go

@@ -51,6 +51,7 @@ type OAuthProxy struct {
 	CookieHttpOnly bool
 	CookieExpire   time.Duration
 	CookieRefresh  time.Duration
+	CookieSameSite string
 	Validator      func(string) bool
 
 	RobotsPath        string
@@ -236,7 +237,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, domain, refresh)
+	log.Printf("Cookie settings: name:%s secure(https):%v httponly:%v expiry:%s domain:%s samesite:%s refresh:%s", opts.CookieName, opts.CookieSecure, opts.CookieHttpOnly, opts.CookieExpire, domain, opts.CookieSameSite, refresh)
 
 	var cipher *cookie.Cipher
 	if opts.PassAccessToken || (opts.CookieRefresh != time.Duration(0)) {
@@ -260,6 +261,7 @@ func NewOAuthProxy(opts *Options, validator func(string) bool) *OAuthProxy {
 		CookieHttpOnly: opts.CookieHttpOnly,
 		CookieExpire:   opts.CookieExpire,
 		CookieRefresh:  opts.CookieRefresh,
+		CookieSameSite: opts.CookieSameSite,
 		Validator:      validator,
 
 		RobotsPath:        "/robots.txt",
@@ -379,6 +381,7 @@ func (p *OAuthProxy) makeCookie(req *http.Request, name string, value string, ex
 		HttpOnly: p.CookieHttpOnly,
 		Secure:   p.CookieSecure,
 		Expires:  now.Add(expiration),
+		SameSite: parseSameSite(p.CookieSameSite),
 	}
 }
 
@@ -858,3 +861,19 @@ func (p *OAuthProxy) CheckRequestAuth(req *http.Request) (*providers.SessionStat
 	// handle advanced validation
 	return p.provider.ValidateRequest(req)
 }
+
+// Parse a valid http.SameSite value from a user supplied string for use of making cookies.
+func parseSameSite(v string) http.SameSite {
+	switch v {
+	case "lax":
+		return http.SameSiteLaxMode
+	case "strict":
+		return http.SameSiteStrictMode
+	case "none":
+		return http.SameSiteNoneMode
+	case "":
+		return http.SameSiteDefaultMode
+	default:
+		panic(fmt.Sprintf("Invalid value for SameSite: %s", v))
+	}
+}

options.go

@@ -58,6 +58,7 @@ type Options struct {
 	CookieRefresh    time.Duration `flag:"cookie-refresh" cfg:"cookie_refresh" env:"OAUTH2_PROXY_COOKIE_REFRESH"`
 	CookieSecure     bool          `flag:"cookie-secure" cfg:"cookie_secure"`
 	CookieHttpOnly   bool          `flag:"cookie-httponly" cfg:"cookie_httponly"`
+	CookieSameSite   string        `flag:"cookie-samesite" cfg:"cookie_samesite" env:"OAUTH2_PROXY_COOKIE_SAMESITE"`
 
 	Upstreams             []string `flag:"upstream" cfg:"upstreams"`
 	BypassAuthExceptRegex []string `flag:"bypass-auth-except-for" cfg:"bypass_auth_except_for"`
@@ -299,6 +300,12 @@ func (o *Options) Validate(p providers.Provider) error {
 		msgs = append(msgs, "tls-client-ca requires tls-key-file or tls-cert-file to be set to listen on tls")
 	}
 
+	switch o.CookieSameSite {
+	case "", "none", "lax", "strict":
+	default:
+		msgs = append(msgs, fmt.Sprintf("cookie_samesite (%q) must be one of ['', 'lax', 'strict', 'none']", o.CookieSameSite))
+	}
+
 	msgs = parseSignatureKey(o, msgs)
 	msgs = validateCookieName(o, msgs)
 

options_test.go

@@ -210,3 +210,22 @@ func TestValidateCookieBadName(t *testing.T) {
 	assert.Equal(t, err.Error(), "Invalid configuration:\n"+
 		fmt.Sprintf("  invalid cookie name: %q", o.CookieName))
 }
+
+func TestValidateCookieSameSiteUnknown(t *testing.T) {
+	o := testOptions()
+	o.CookieSameSite = "foo"
+	err := o.Validate(&testProvider{})
+	assert.Equal(t, err.Error(), "Invalid configuration:\n"+
+		fmt.Sprintf("  cookie_samesite (%q) must be one of ['', 'lax', 'strict', 'none']", o.CookieSameSite))
+}
+
+func TestValidateCookieSameSite(t *testing.T) {
+	testCases := []string{"", "lax", "strict", "none"}
+	for _, tc := range testCases {
+		t.Run(tc, func(t *testing.T) {
+			o := testOptions()
+			o.CookieSameSite = tc
+			assert.Equal(t, nil, o.Validate(&testProvider{}))
+		})
+	}
+}