Skip to content
This repository has been archived by the owner on Dec 7, 2020. It is now read-only.

Commit

Permalink
[KEYCLOAK-10938] Support SameSite cookie attribute
Browse files Browse the repository at this point in the history
Adds an option to the config to enable the SameSite attribute for
cookies set by gatekeeper.

SameSiteCookie can be Strict|Lax|None
None will not set the SameSite flag.
  • Loading branch information
fiji-flo authored and Bruno Oliveira da Silva committed Oct 2, 2019
1 parent 03c3083 commit 9c5c77d
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 1 deletion.
4 changes: 4 additions & 0 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ func newDefaultConfig() *Config {
SelfSignedTLSHostnames: hostnames,
RequestIDHeader: "X-Request-ID",
ResponseHeaders: make(map[string]string),
SameSiteCookie: SameSiteLax,
SecureCookie: true,
ServerIdleTimeout: 120 * time.Second,
ServerReadTimeout: 10 * time.Second,
Expand Down Expand Up @@ -93,6 +94,9 @@ func (r *Config) isValid() error {
if r.MaxIdleConnsPerHost < 0 || r.MaxIdleConnsPerHost > r.MaxIdleConns {
return errors.New("maxi-idle-connections-per-host must be a number > 0 and <= max-idle-connections")
}
if r.SameSiteCookie != "" && r.SameSiteCookie != SameSiteStrict && r.SameSiteCookie != SameSiteLax && r.SameSiteCookie != SameSiteNone {
return errors.New("same-site-cookie must be one of Strict|Lax|None")
}
if r.TLSCertificate != "" && r.TLSPrivateKey == "" {
return errors.New("you have not provided a private key")
}
Expand Down
20 changes: 20 additions & 0 deletions cookies.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ import (
uuid "github.com/satori/go.uuid"
)

// SameSite cookie config options
const (
SameSiteStrict = "Strict"
SameSiteLax = "Lax"
SameSiteNone = "None"
)

// dropCookie drops a cookie into the response
func (r *oauthProxy) dropCookie(w http.ResponseWriter, host, name, value string, duration time.Duration) {
// step: default to the host header, else the config domain
Expand All @@ -48,6 +55,13 @@ func (r *oauthProxy) dropCookie(w http.ResponseWriter, host, name, value string,
cookie.Expires = time.Now().Add(duration)
}

switch r.config.SameSiteCookie {
case SameSiteStrict:
cookie.SameSite = http.SameSiteStrictMode
case SameSiteLax:
cookie.SameSite = http.SameSiteLaxMode
}

http.SetCookie(w, cookie)
}

Expand All @@ -65,6 +79,12 @@ func (r *oauthProxy) getMaxCookieChunkLength(req *http.Request, cookieName strin
if !r.config.EnableSessionCookies {
maxCookieChunkLength -= len("Expires=Mon, 02 Jan 2006 03:04:05 MST; ")
}
switch r.config.SameSiteCookie {
case SameSiteStrict:
maxCookieChunkLength -= len("SameSite=Strict ")
case SameSiteLax:
maxCookieChunkLength -= len("SameSite=Lax ")
}
if r.config.SecureCookie {
maxCookieChunkLength -= len("Secure")
}
Expand Down
47 changes: 46 additions & 1 deletion cookies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,45 @@ func TestSessionOnlyCookie(t *testing.T) {
"we have not set the cookie, headers: %v", resp.Header())
}

func TestSameSiteCookie(t *testing.T) {
p, _, _ := newTestProxyService(nil)

req := newFakeHTTPRequest("GET", "/admin")
resp := httptest.NewRecorder()
p.dropCookie(resp, req.Host, "test-cookie", "test-value", 0)

assert.Equal(t, resp.Header().Get("Set-Cookie"),
"test-cookie=test-value; Path=/; Domain=127.0.0.1",
"we have not set the cookie, headers: %v", resp.Header())

req = newFakeHTTPRequest("GET", "/admin")
resp = httptest.NewRecorder()
p.config.SameSiteCookie = SameSiteStrict
p.dropCookie(resp, req.Host, "test-cookie", "test-value", 0)

assert.Equal(t, resp.Header().Get("Set-Cookie"),
"test-cookie=test-value; Path=/; Domain=127.0.0.1; SameSite=Strict",
"we have not set the cookie, headers: %v", resp.Header())

req = newFakeHTTPRequest("GET", "/admin")
resp = httptest.NewRecorder()
p.config.SameSiteCookie = SameSiteLax
p.dropCookie(resp, req.Host, "test-cookie", "test-value", 0)

assert.Equal(t, resp.Header().Get("Set-Cookie"),
"test-cookie=test-value; Path=/; Domain=127.0.0.1; SameSite=Lax",
"we have not set the cookie, headers: %v", resp.Header())

req = newFakeHTTPRequest("GET", "/admin")
resp = httptest.NewRecorder()
p.config.SameSiteCookie = SameSiteNone
p.dropCookie(resp, req.Host, "test-cookie", "test-value", 0)

assert.Equal(t, resp.Header().Get("Set-Cookie"),
"test-cookie=test-value; Path=/; Domain=127.0.0.1",
"we have not set the cookie, headers: %v", resp.Header())
}

func TestHTTPOnlyCookie(t *testing.T) {
p, _, _ := newTestProxyService(nil)

Expand Down Expand Up @@ -225,13 +264,19 @@ func TestGetMaxCookieChunkLength(t *testing.T) {
p.config.HTTPOnlyCookie = true
p.config.EnableSessionCookies = true
p.config.SecureCookie = true
p.config.SameSiteCookie = "Strict"
p.config.CookieDomain = "1234567890"
assert.Equal(t, p.getMaxCookieChunkLength(req, "1234567890"), 4033,
assert.Equal(t, p.getMaxCookieChunkLength(req, "1234567890"), 4017,
"cookie chunk calculation is not correct")

p.config.SameSiteCookie = "Lax"
assert.Equal(t, p.getMaxCookieChunkLength(req, "1234567890"), 4020,
"cookie chunk calculation is not correct")

p.config.HTTPOnlyCookie = false
p.config.EnableSessionCookies = false
p.config.SecureCookie = false
p.config.SameSiteCookie = "None"
p.config.CookieDomain = ""
assert.Equal(t, p.getMaxCookieChunkLength(req, ""), 4021,
"cookie chunk calculation is not correct")
Expand Down
2 changes: 2 additions & 0 deletions doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,8 @@ type Config struct {
SecureCookie bool `json:"secure-cookie" yaml:"secure-cookie" usage:"enforces the cookie to be secure"`
// HTTPOnlyCookie enforces the cookie as http only
HTTPOnlyCookie bool `json:"http-only-cookie" yaml:"http-only-cookie" usage:"enforces the cookie is in http only mode"`
// SameSiteCookie enforces cookies to be send only to same site requests.
SameSiteCookie string `json:"same-site-cookie" yaml:"same-site-cookie" usage:"enforces cookies to be send only to same site requests according to the policy (can be Strict|Lax|None)"`

// MatchClaims is a series of checks, the claims in the token must match those here
MatchClaims map[string]string `json:"match-claims" yaml:"match-claims" usage:"keypair values for matching access token claims e.g. aud=myapp, iss=http://example.*"`
Expand Down

0 comments on commit 9c5c77d

Please sign in to comment.