From e197eb7590cf46456caf7bb2d18309403a058ce7 Mon Sep 17 00:00:00 2001 From: Hans Hjort Date: Tue, 19 Nov 2019 12:32:44 -0500 Subject: [PATCH 1/3] Set 'Secure' status on cookies --- usersync/cookie.go | 2 ++ usersync/cookie_test.go | 3 +++ 2 files changed, 5 insertions(+) diff --git a/usersync/cookie.go b/usersync/cookie.go index 6ced4b620fb..e10c285f53c 100644 --- a/usersync/cookie.go +++ b/usersync/cookie.go @@ -125,6 +125,7 @@ func (cookie *PBSCookie) ToHTTPCookie(ttl time.Duration) *http.Cookie { Value: b64, Expires: time.Now().Add(ttl), Path: "/", + Secure: true, } } @@ -202,6 +203,7 @@ func (cookie *PBSCookie) SetCookieOnResponse(w http.ResponseWriter, setSiteCooki Value: SameSiteCookieValue, Expires: time.Now().Add(ttl), Path: "/", + Secure: true, } sameSiteCookieStr := sameSiteCookie.String() sameSiteCookieStr += SameSiteAttribute diff --git a/usersync/cookie_test.go b/usersync/cookie_test.go index a59f7def8ea..ef2e9911e46 100644 --- a/usersync/cookie_test.go +++ b/usersync/cookie_test.go @@ -425,6 +425,9 @@ func TestSetCookieOnResponseForSameSiteNone(t *testing.T) { if !strings.Contains(writtenCookie, "SSCookie=1") { t.Error("Set-Cookie should contain SSCookie=1") } + if !strings.Contains(writtenCookie, "; Secure;") { + t.Error("Set-Cookie should contain Secure") + } } func TestSetCookieOnResponseForOlderChromeVersion(t *testing.T) { From 41d2d3ef4e6c16068db491f3b084e55576f8e297 Mon Sep 17 00:00:00 2001 From: Hans Hjort Date: Tue, 19 Nov 2019 12:38:47 -0500 Subject: [PATCH 2/3] Don't set Secure in legacy case --- usersync/cookie.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/usersync/cookie.go b/usersync/cookie.go index e10c285f53c..0ad3b2aa1b6 100644 --- a/usersync/cookie.go +++ b/usersync/cookie.go @@ -125,7 +125,6 @@ func (cookie *PBSCookie) ToHTTPCookie(ttl time.Duration) *http.Cookie { Value: b64, Expires: time.Now().Add(ttl), Path: "/", - Secure: true, } } @@ -194,9 +193,11 @@ func (cookie *PBSCookie) SetCookieOnResponse(w http.ResponseWriter, setSiteCooki currSize = len([]byte(httpCookie.String())) } - uidsCookieStr := httpCookie.String() + var uidsCookieStr string var sameSiteCookie *http.Cookie if setSiteCookie { + httpCookie.Secure = true + uidsCookieStr := httpCookie.String() uidsCookieStr += SameSiteAttribute sameSiteCookie = &http.Cookie{ Name: SameSiteCookieName, @@ -208,6 +209,8 @@ func (cookie *PBSCookie) SetCookieOnResponse(w http.ResponseWriter, setSiteCooki sameSiteCookieStr := sameSiteCookie.String() sameSiteCookieStr += SameSiteAttribute w.Header().Add("Set-Cookie", sameSiteCookieStr) + } else { + uidsCookieStr = httpCookie.String() } w.Header().Add("Set-Cookie", uidsCookieStr) } From 2cfda8c0c1445d3a33a35760e3958ae69fc2cdb2 Mon Sep 17 00:00:00 2001 From: Hans Hjort Date: Wed, 20 Nov 2019 08:57:06 -0500 Subject: [PATCH 3/3] Fixes a scoping bug --- usersync/cookie.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/usersync/cookie.go b/usersync/cookie.go index 0ad3b2aa1b6..13aa7f38dd3 100644 --- a/usersync/cookie.go +++ b/usersync/cookie.go @@ -197,7 +197,7 @@ func (cookie *PBSCookie) SetCookieOnResponse(w http.ResponseWriter, setSiteCooki var sameSiteCookie *http.Cookie if setSiteCookie { httpCookie.Secure = true - uidsCookieStr := httpCookie.String() + uidsCookieStr = httpCookie.String() uidsCookieStr += SameSiteAttribute sameSiteCookie = &http.Cookie{ Name: SameSiteCookieName,