Merge pull request #339 from clubhouse/pgroudas/add-samesite-cookie-options
Add SameSite cookie configuration value for session cookie
This commit is contained in:
		
						commit
						ec72ee8bf1
					
				|  | @ -5,6 +5,8 @@ | |||
| ## Breaking Changes | ||||
| 
 | ||||
| ## Changes since v4.1.0 | ||||
| 
 | ||||
| - [#339](https://github.com/pusher/oauth2_proxy/pull/339) Add configuration for cookie 'SameSite' value. (@pgroudas) | ||||
| - [#347](https://github.com/pusher/oauth2_proxy/pull/347) Update keycloak provider configuration documentation | ||||
| - [#325](https://github.com/pusher/oauth2_proxy/pull/325) dist.sh: use sha256sum (@syscll) | ||||
| - [#179](https://github.com/pusher/oauth2_proxy/pull/179) Add Nextcloud provider (@Ramblurr) | ||||
|  |  | |||
|  | @ -20,7 +20,7 @@ _oauth2_proxy() { | |||
| 			COMPREPLY=( $(compgen -W "google azure facebook github keycloak gitlab linkedin login.gov digitalocean" -- ${cur}) ) | ||||
| 			return 0 | ||||
| 			;; | ||||
| 		-@(http-address|https-address|redirect-url|upstream|basic-auth-password|skip-auth-regex|flush-interval|extra-jwt-issuers|email-domain|whitelist-domain|keycloak-group|azure-tenant|bitbucket-team|bitbucket-repository|github-org|github-team|gitlab-group|google-group|google-admin-email|google-service-account-json|client-id|client_secret|banner|footer|proxy-prefix|ping-path|cookie-name|cookie-secret|cookie-domain|cookie-path|cookie-expire|cookie-refresh|redist-sentinel-master-name|redist-sentinel-connection-urls|logging-max-size|logging-max-age|logging-max-backups|standard-logging-format|request-logging-format|exclude-logging-paths|auth-logging-format|oidc-issuer-url|oidc-jwks-url|login-url|redeem-url|profile-url|resource|validate-url|scope|approval-prompt|signature-key|acr-values|jwt-key|pubjwk-url)) | ||||
| 		-@(http-address|https-address|redirect-url|upstream|basic-auth-password|skip-auth-regex|flush-interval|extra-jwt-issuers|email-domain|whitelist-domain|keycloak-group|azure-tenant|bitbucket-team|bitbucket-repository|github-org|github-team|gitlab-group|google-group|google-admin-email|google-service-account-json|client-id|client_secret|banner|footer|proxy-prefix|ping-path|cookie-name|cookie-secret|cookie-domain|cookie-path|cookie-expire|cookie-refresh|cookie-samesite|redist-sentinel-master-name|redist-sentinel-connection-urls|logging-max-size|logging-max-age|logging-max-backups|standard-logging-format|request-logging-format|exclude-logging-paths|auth-logging-format|oidc-issuer-url|oidc-jwks-url|login-url|redeem-url|profile-url|resource|validate-url|scope|approval-prompt|signature-key|acr-values|jwt-key|pubjwk-url)) | ||||
| 			return 0 | ||||
| 			;; | ||||
| 	esac | ||||
|  |  | |||
|  | @ -38,6 +38,7 @@ An example [oauth2_proxy.cfg]({{ site.gitweb }}/contrib/oauth2_proxy.cfg.example | |||
| | `-cookie-refresh` | duration | refresh the cookie after this duration; `0` to disable | | | ||||
| | `-cookie-secret` | string | the seed string for secure cookies (optionally base64 encoded) | | | ||||
| | `-cookie-secure` | bool | set secure (HTTPS) cookie flag | true | | ||||
| | `-cookie-samesite` | string | set SameSite cookie attribute (ie: `"lax"`, `"strict"`, `"none"`, or `""`). | `""` | | ||||
| | `-custom-templates-dir` | string | path to custom html templates | | | ||||
| | `-display-htpasswd-form` | bool | display username / password login form if an htpasswd file is provided | true | | ||||
| | `-email-domain` | string | authenticate emails with the specified domain (may be given multiple times). Use `*` to authenticate any email | | | ||||
|  |  | |||
							
								
								
									
										1
									
								
								main.go
								
								
								
								
							
							
						
						
									
										1
									
								
								main.go
								
								
								
								
							|  | @ -86,6 +86,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.String("session-store-type", "cookie", "the session storage provider to use") | ||||
| 	flagSet.String("redis-connection-url", "", "URL of redis server for redis session storage (eg: redis://HOST[:PORT])") | ||||
|  |  | |||
|  | @ -20,6 +20,7 @@ import ( | |||
| 	"github.com/coreos/go-oidc" | ||||
| 	"github.com/mbland/hmacauth" | ||||
| 	sessionsapi "github.com/pusher/oauth2_proxy/pkg/apis/sessions" | ||||
| 	"github.com/pusher/oauth2_proxy/pkg/cookies" | ||||
| 	"github.com/pusher/oauth2_proxy/pkg/encryption" | ||||
| 	"github.com/pusher/oauth2_proxy/pkg/logger" | ||||
| 	"github.com/pusher/oauth2_proxy/providers" | ||||
|  | @ -68,6 +69,7 @@ type OAuthProxy struct { | |||
| 	CookieHTTPOnly bool | ||||
| 	CookieExpire   time.Duration | ||||
| 	CookieRefresh  time.Duration | ||||
| 	CookieSameSite string | ||||
| 	Validator      func(string) bool | ||||
| 
 | ||||
| 	RobotsPath        string | ||||
|  | @ -195,7 +197,7 @@ func NewWebSocketOrRestReverseProxy(u *url.URL, opts *Options, auth hmacauth.Hma | |||
| 	} | ||||
| } | ||||
| 
 | ||||
| // NewOAuthProxy creates a new instance of OOuthProxy from the options provided
 | ||||
| // NewOAuthProxy creates a new instance of OAuthProxy from the options provided
 | ||||
| func NewOAuthProxy(opts *Options, validator func(string) bool) *OAuthProxy { | ||||
| 	serveMux := http.NewServeMux() | ||||
| 	var auth hmacauth.HmacAuth | ||||
|  | @ -260,7 +262,7 @@ func NewOAuthProxy(opts *Options, validator func(string) bool) *OAuthProxy { | |||
| 		refresh = fmt.Sprintf("after %s", opts.CookieRefresh) | ||||
| 	} | ||||
| 
 | ||||
| 	logger.Printf("Cookie settings: name:%s secure(https):%v httponly:%v expiry:%s domain:%s path:%s refresh:%s", opts.CookieName, opts.CookieSecure, opts.CookieHTTPOnly, opts.CookieExpire, opts.CookieDomain, opts.CookiePath, refresh) | ||||
| 	logger.Printf("Cookie settings: name:%s secure(https):%v httponly:%v expiry:%s domain:%s path:%s samesite:%s refresh:%s", opts.CookieName, opts.CookieSecure, opts.CookieHTTPOnly, opts.CookieExpire, opts.CookieDomain, opts.CookiePath, opts.CookieSameSite, refresh) | ||||
| 
 | ||||
| 	return &OAuthProxy{ | ||||
| 		CookieName:     opts.CookieName, | ||||
|  | @ -272,6 +274,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", | ||||
|  | @ -380,6 +383,7 @@ func (p *OAuthProxy) makeCookie(req *http.Request, name string, value string, ex | |||
| 		HttpOnly: p.CookieHTTPOnly, | ||||
| 		Secure:   p.CookieSecure, | ||||
| 		Expires:  now.Add(expiration), | ||||
| 		SameSite: cookies.ParseSameSite(p.CookieSameSite), | ||||
| 	} | ||||
| } | ||||
| 
 | ||||
|  |  | |||
|  | @ -372,6 +372,12 @@ func (o *Options) Validate() error { | |||
| 		} | ||||
| 	} | ||||
| 
 | ||||
| 	switch o.CookieSameSite { | ||||
| 	case "", "none", "lax", "strict": | ||||
| 	default: | ||||
| 		msgs = append(msgs, fmt.Sprintf("cookie_samesite (%s) must be one of ['', 'lax', 'strict', 'none']", o.CookieSameSite)) | ||||
| 	} | ||||
| 
 | ||||
| 	msgs = parseSignatureKey(o, msgs) | ||||
| 	msgs = validateCookieName(o, msgs) | ||||
| 	msgs = setupLogger(o, msgs) | ||||
|  |  | |||
|  | @ -12,4 +12,5 @@ type CookieOptions struct { | |||
| 	CookieRefresh  time.Duration `flag:"cookie-refresh" cfg:"cookie_refresh" env:"OAUTH2_PROXY_COOKIE_REFRESH"` | ||||
| 	CookieSecure   bool          `flag:"cookie-secure" cfg:"cookie_secure" env:"OAUTH2_PROXY_COOKIE_SECURE"` | ||||
| 	CookieHTTPOnly bool          `flag:"cookie-httponly" cfg:"cookie_httponly" env:"OAUTH2_PROXY_COOKIE_HTTPONLY"` | ||||
| 	CookieSameSite string        `flag:"cookie-samesite" cfg:"cookie_samesite" env:"OAUTH2_PROXY_COOKIE_SAMESITE"` | ||||
| } | ||||
|  |  | |||
|  | @ -1,6 +1,7 @@ | |||
| package cookies | ||||
| 
 | ||||
| import ( | ||||
| 	"fmt" | ||||
| 	"net" | ||||
| 	"net/http" | ||||
| 	"strings" | ||||
|  | @ -12,7 +13,7 @@ import ( | |||
| 
 | ||||
| // MakeCookie constructs a cookie from the given parameters,
 | ||||
| // discovering the domain from the request if not specified.
 | ||||
| func MakeCookie(req *http.Request, name string, value string, path string, domain string, httpOnly bool, secure bool, expiration time.Duration, now time.Time) *http.Cookie { | ||||
| func MakeCookie(req *http.Request, name string, value string, path string, domain string, httpOnly bool, secure bool, expiration time.Duration, now time.Time, sameSite http.SameSite) *http.Cookie { | ||||
| 	if domain != "" { | ||||
| 		host := req.Host | ||||
| 		if h, _, err := net.SplitHostPort(host); err == nil { | ||||
|  | @ -31,11 +32,28 @@ func MakeCookie(req *http.Request, name string, value string, path string, domai | |||
| 		HttpOnly: httpOnly, | ||||
| 		Secure:   secure, | ||||
| 		Expires:  now.Add(expiration), | ||||
| 		SameSite: sameSite, | ||||
| 	} | ||||
| } | ||||
| 
 | ||||
| // MakeCookieFromOptions constructs a cookie based on the givemn *options.CookieOptions,
 | ||||
| // MakeCookieFromOptions constructs a cookie based on the given *options.CookieOptions,
 | ||||
| // value and creation time
 | ||||
| func MakeCookieFromOptions(req *http.Request, name string, value string, opts *options.CookieOptions, expiration time.Duration, now time.Time) *http.Cookie { | ||||
| 	return MakeCookie(req, name, value, opts.CookiePath, opts.CookieDomain, opts.CookieHTTPOnly, opts.CookieSecure, expiration, now) | ||||
| 	return MakeCookie(req, name, value, opts.CookiePath, opts.CookieDomain, opts.CookieHTTPOnly, opts.CookieSecure, expiration, now, ParseSameSite(opts.CookieSameSite)) | ||||
| } | ||||
| 
 | ||||
| // 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)) | ||||
| 	} | ||||
| } | ||||
|  |  | |||
|  | @ -15,7 +15,7 @@ import ( | |||
| 	. "github.com/onsi/gomega" | ||||
| 	"github.com/pusher/oauth2_proxy/pkg/apis/options" | ||||
| 	sessionsapi "github.com/pusher/oauth2_proxy/pkg/apis/sessions" | ||||
| 	"github.com/pusher/oauth2_proxy/pkg/cookies" | ||||
| 	cookiesapi "github.com/pusher/oauth2_proxy/pkg/cookies" | ||||
| 	"github.com/pusher/oauth2_proxy/pkg/encryption" | ||||
| 	"github.com/pusher/oauth2_proxy/pkg/sessions" | ||||
| 	sessionscookie "github.com/pusher/oauth2_proxy/pkg/sessions/cookie" | ||||
|  | @ -79,6 +79,12 @@ var _ = Describe("NewSessionStore", func() { | |||
| 				} | ||||
| 			}) | ||||
| 
 | ||||
| 			It("have the correct SameSite set", func() { | ||||
| 				for _, cookie := range cookies { | ||||
| 					Expect(cookie.SameSite).To(Equal(cookiesapi.ParseSameSite(cookieOpts.CookieSameSite))) | ||||
| 				} | ||||
| 			}) | ||||
| 
 | ||||
| 			It("have a signature timestamp matching session.CreatedAt", func() { | ||||
| 				for _, cookie := range cookies { | ||||
| 					if cookie.Value != "" { | ||||
|  | @ -159,7 +165,7 @@ var _ = Describe("NewSessionStore", func() { | |||
| 					By("Using a valid cookie with a different providers session encoding") | ||||
| 					broken := "BrokenSessionFromADifferentSessionImplementation" | ||||
| 					value := encryption.SignedValue(cookieOpts.CookieSecret, cookieOpts.CookieName, broken, time.Now()) | ||||
| 					cookie := cookies.MakeCookieFromOptions(request, cookieOpts.CookieName, value, cookieOpts, cookieOpts.CookieExpire, time.Now()) | ||||
| 					cookie := cookiesapi.MakeCookieFromOptions(request, cookieOpts.CookieName, value, cookieOpts, cookieOpts.CookieExpire, time.Now()) | ||||
| 					request.AddCookie(cookie) | ||||
| 
 | ||||
| 					err := ss.Save(response, request, session) | ||||
|  | @ -338,6 +344,7 @@ var _ = Describe("NewSessionStore", func() { | |||
| 					CookieSecure:   false, | ||||
| 					CookieHTTPOnly: false, | ||||
| 					CookieDomain:   "example.com", | ||||
| 					CookieSameSite: "strict", | ||||
| 				} | ||||
| 
 | ||||
| 				var err error | ||||
|  | @ -379,6 +386,7 @@ var _ = Describe("NewSessionStore", func() { | |||
| 			CookieRefresh:  time.Duration(1) * time.Hour, | ||||
| 			CookieSecure:   true, | ||||
| 			CookieHTTPOnly: true, | ||||
| 			CookieSameSite: "", | ||||
| 		} | ||||
| 
 | ||||
| 		session = &sessionsapi.SessionState{ | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue