From 03c3083fe9d32a099b19ba325e45d1eac639281a Mon Sep 17 00:00:00 2001 From: "Daniel A.C. Martin" <github@daniel-martin.co.uk> Date: Thu, 6 Jun 2019 17:38:55 +0100 Subject: [PATCH] [KEYCLOAK-10668] Do not set the cookie domain By setting the domain attribute on the cookie we were allowing the cookie to be applied to subdomains where it may not be valid and may interfere with other services protected by keycloak-gatekeeper. (For example, a gatekeeper running on https://domain.com could break a gatekeeper running on https://sub.domain.com .) Instead, we should simply not set the attribute unless there is a specific domain configured. For more information please see section 4.1.2.3 of [RFC 6265]. [RFC 6265]: https://tools.ietf.org/html/rfc6265#section-4.1.2 --- cookies.go | 2 +- cookies_test.go | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/cookies.go b/cookies.go index 64dddc4..d3d2b08 100644 --- a/cookies.go +++ b/cookies.go @@ -28,7 +28,7 @@ import ( // 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 - domain := strings.Split(host, ":")[0] + domain := "" if r.config.CookieDomain != "" { domain = r.config.CookieDomain } diff --git a/cookies_test.go b/cookies_test.go index bbd9cd4..2563f1a 100644 --- a/cookies_test.go +++ b/cookies_test.go @@ -39,7 +39,7 @@ func TestCookieDomainHostHeader(t *testing.T) { defer resp.Body.Close() assert.NotNil(t, cookie) - assert.Equal(t, cookie.Domain, "127.0.0.1") + assert.Equal(t, cookie.Domain, "") } func TestCookieBasePath(t *testing.T) { @@ -113,7 +113,7 @@ func TestDropCookie(t *testing.T) { 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", + "test-cookie=test-value; Path=/", "we have not set the cookie, headers: %v", resp.Header()) req = newFakeHTTPRequest("GET", "/admin") @@ -122,7 +122,7 @@ func TestDropCookie(t *testing.T) { 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", + "test-cookie=test-value; Path=/", "we have not set the cookie, headers: %v", resp.Header()) req = newFakeHTTPRequest("GET", "/admin") @@ -149,7 +149,7 @@ func TestDropRefreshCookie(t *testing.T) { p.dropRefreshTokenCookie(req, resp, "test", 0) assert.Equal(t, resp.Header().Get("Set-Cookie"), - refreshCookie+"=test; Path=/; Domain=127.0.0.1", + refreshCookie+"=test; Path=/", "we have not set the cookie, headers: %v", resp.Header()) } @@ -162,7 +162,7 @@ func TestSessionOnlyCookie(t *testing.T) { p.dropCookie(resp, req.Host, "test-cookie", "test-value", 1*time.Hour) assert.Equal(t, resp.Header().Get("Set-Cookie"), - "test-cookie=test-value; Path=/; Domain=127.0.0.1", + "test-cookie=test-value; Path=/", "we have not set the cookie, headers: %v", resp.Header()) } @@ -174,7 +174,7 @@ func TestHTTPOnlyCookie(t *testing.T) { 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", + "test-cookie=test-value; Path=/", "we have not set the cookie, headers: %v", resp.Header()) req = newFakeHTTPRequest("GET", "/admin") @@ -183,7 +183,7 @@ func TestHTTPOnlyCookie(t *testing.T) { 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; HttpOnly", + "test-cookie=test-value; Path=/; HttpOnly", "we have not set the cookie, headers: %v", resp.Header()) } @@ -194,7 +194,7 @@ func TestClearAccessTokenCookie(t *testing.T) { resp := httptest.NewRecorder() p.clearAccessTokenCookie(req, resp) assert.Contains(t, resp.Header().Get("Set-Cookie"), - accessCookie+"=; Path=/; Domain=127.0.0.1; Expires=", + accessCookie+"=; Path=/; Expires=", "we have not cleared the, headers: %v", resp.Header()) } @@ -204,7 +204,7 @@ func TestClearRefreshAccessTokenCookie(t *testing.T) { resp := httptest.NewRecorder() p.clearRefreshTokenCookie(req, resp) assert.Contains(t, resp.Header().Get("Set-Cookie"), - refreshCookie+"=; Path=/; Domain=127.0.0.1; Expires=", + refreshCookie+"=; Path=/; Expires=", "we have not cleared the, headers: %v", resp.Header()) } @@ -214,7 +214,7 @@ func TestClearAllCookies(t *testing.T) { resp := httptest.NewRecorder() p.clearAllCookies(req, resp) assert.Contains(t, resp.Header().Get("Set-Cookie"), - accessCookie+"=; Path=/; Domain=127.0.0.1; Expires=", + accessCookie+"=; Path=/; Expires=", "we have not cleared the, headers: %v", resp.Header()) } -- GitLab