From 3dd1a177e2daa6d2bab78fe8b9d6508d60d42eae Mon Sep 17 00:00:00 2001 From: shalmali-patil Date: Fri, 26 Jul 2019 11:16:02 +0200 Subject: [PATCH 1/5] committing changes for setting SameSite cookie attribute --- adapters/adform/adform_test.go | 4 +-- adapters/appnexus/appnexus_test.go | 4 +-- adapters/audienceNetwork/facebook_test.go | 4 +-- adapters/lifestreet/lifestreet_test.go | 4 +-- adapters/pubmatic/pubmatic_test.go | 4 +-- adapters/pulsepoint/pulsepoint_test.go | 4 +-- adapters/rubicon/rubicon_test.go | 4 +-- adapters/sovrn/sovrn_test.go | 4 +-- endpoints/setuid.go | 4 +-- pbs/usersync.go | 6 ++-- usersync/cookie.go | 35 +++++++++++++++++++++-- usersync/cookie_test.go | 3 +- 12 files changed, 56 insertions(+), 24 deletions(-) diff --git a/adapters/adform/adform_test.go b/adapters/adform/adform_test.go index f94e7178578..66f3aa75daf 100644 --- a/adapters/adform/adform_test.go +++ b/adapters/adform/adform_test.go @@ -17,10 +17,10 @@ import ( "fmt" - "github.com/mxmCherry/openrtb" "github.com/PubMatic-OpenWrap/prebid-server/adapters" "github.com/PubMatic-OpenWrap/prebid-server/config" "github.com/PubMatic-OpenWrap/prebid-server/openrtb_ext" + "github.com/mxmCherry/openrtb" ) func TestJsonSamples(t *testing.T) { @@ -198,7 +198,7 @@ func preparePrebidRequest(serverUrl string, t *testing.T) *pbs.PBSRequest { pbsCookie := usersync.ParsePBSCookieFromRequest(prebidHttpRequest, &config.HostCookie{}) pbsCookie.TrySync("adform", adformTestData.buyerUID) fakeWriter := httptest.NewRecorder() - pbsCookie.SetCookieOnResponse(fakeWriter, "", time.Minute) + pbsCookie.SetCookieOnResponse(fakeWriter, prebidHttpRequest, "", time.Minute) prebidHttpRequest.Header.Add("Cookie", fakeWriter.Header().Get("Set-Cookie")) cacheClient, _ := dummycache.New() diff --git a/adapters/appnexus/appnexus_test.go b/adapters/appnexus/appnexus_test.go index 9306fd6af8c..05908a649f1 100644 --- a/adapters/appnexus/appnexus_test.go +++ b/adapters/appnexus/appnexus_test.go @@ -16,10 +16,10 @@ import ( "fmt" - "github.com/mxmCherry/openrtb" "github.com/PubMatic-OpenWrap/prebid-server/adapters" "github.com/PubMatic-OpenWrap/prebid-server/adapters/adapterstest" "github.com/PubMatic-OpenWrap/prebid-server/config" + "github.com/mxmCherry/openrtb" ) func TestJsonSamples(t *testing.T) { @@ -363,7 +363,7 @@ func TestAppNexusBasicResponse(t *testing.T) { pc := usersync.ParsePBSCookieFromRequest(req, &config.HostCookie{}) pc.TrySync("adnxs", andata.buyerUID) fakewriter := httptest.NewRecorder() - pc.SetCookieOnResponse(fakewriter, "", 90*24*time.Hour) + pc.SetCookieOnResponse(fakewriter, req, "", 90*24*time.Hour) req.Header.Add("Cookie", fakewriter.Header().Get("Set-Cookie")) cacheClient, _ := dummycache.New() diff --git a/adapters/audienceNetwork/facebook_test.go b/adapters/audienceNetwork/facebook_test.go index eb681cf27a8..1c36fa8f2f5 100644 --- a/adapters/audienceNetwork/facebook_test.go +++ b/adapters/audienceNetwork/facebook_test.go @@ -16,9 +16,9 @@ import ( "fmt" - "github.com/mxmCherry/openrtb" "github.com/PubMatic-OpenWrap/prebid-server/adapters" "github.com/PubMatic-OpenWrap/prebid-server/config" + "github.com/mxmCherry/openrtb" ) type tagInfo struct { @@ -209,7 +209,7 @@ func GenerateBidRequestForTestData(fbdata bidInfo, url string) (*pbs.PBSRequest, pc := usersync.ParsePBSCookieFromRequest(req, &config.HostCookie{}) pc.TrySync("audienceNetwork", fbdata.buyerUID) fakewriter := httptest.NewRecorder() - pc.SetCookieOnResponse(fakewriter, "", 90*24*time.Hour) + pc.SetCookieOnResponse(fakewriter, req, "", 90*24*time.Hour) req.Header.Add("Cookie", fakewriter.Header().Get("Set-Cookie")) cacheClient, _ := dummycache.New() diff --git a/adapters/lifestreet/lifestreet_test.go b/adapters/lifestreet/lifestreet_test.go index f86936cbbfc..05f2192c2a5 100644 --- a/adapters/lifestreet/lifestreet_test.go +++ b/adapters/lifestreet/lifestreet_test.go @@ -16,9 +16,9 @@ import ( "fmt" - "github.com/mxmCherry/openrtb" "github.com/PubMatic-OpenWrap/prebid-server/adapters" "github.com/PubMatic-OpenWrap/prebid-server/config" + "github.com/mxmCherry/openrtb" ) type lsTagInfo struct { @@ -227,7 +227,7 @@ func TestLifestreetBasicResponse(t *testing.T) { pc := usersync.ParsePBSCookieFromRequest(req, &config.HostCookie{}) fakewriter := httptest.NewRecorder() - pc.SetCookieOnResponse(fakewriter, "", 90*24*time.Hour) + pc.SetCookieOnResponse(fakewriter, req, "", 90*24*time.Hour) req.Header.Add("Cookie", fakewriter.Header().Get("Set-Cookie")) cacheClient, _ := dummycache.New() diff --git a/adapters/pubmatic/pubmatic_test.go b/adapters/pubmatic/pubmatic_test.go index 015a7ff2dd5..69375513126 100644 --- a/adapters/pubmatic/pubmatic_test.go +++ b/adapters/pubmatic/pubmatic_test.go @@ -12,13 +12,13 @@ import ( "testing" "time" - "github.com/mxmCherry/openrtb" "github.com/PubMatic-OpenWrap/prebid-server/adapters" "github.com/PubMatic-OpenWrap/prebid-server/adapters/adapterstest" "github.com/PubMatic-OpenWrap/prebid-server/cache/dummycache" "github.com/PubMatic-OpenWrap/prebid-server/config" "github.com/PubMatic-OpenWrap/prebid-server/pbs" "github.com/PubMatic-OpenWrap/prebid-server/usersync" + "github.com/mxmCherry/openrtb" ) func TestJsonSamples(t *testing.T) { @@ -656,7 +656,7 @@ func TestPubmaticSampleRequest(t *testing.T) { pc := usersync.ParsePBSCookieFromRequest(httpReq, &config.HostCookie{}) pc.TrySync("pubmatic", "12345") fakewriter := httptest.NewRecorder() - pc.SetCookieOnResponse(fakewriter, "", 90*24*time.Hour) + pc.SetCookieOnResponse(fakewriter, httpReq, "", 90*24*time.Hour) httpReq.Header.Add("Cookie", fakewriter.Header().Get("Set-Cookie")) cacheClient, _ := dummycache.New() diff --git a/adapters/pulsepoint/pulsepoint_test.go b/adapters/pulsepoint/pulsepoint_test.go index 70c909a8e24..8249d08a3f4 100644 --- a/adapters/pulsepoint/pulsepoint_test.go +++ b/adapters/pulsepoint/pulsepoint_test.go @@ -11,7 +11,6 @@ import ( "testing" "time" - "github.com/mxmCherry/openrtb" "github.com/PubMatic-OpenWrap/prebid-server/adapters" "github.com/PubMatic-OpenWrap/prebid-server/adapters/adapterstest" "github.com/PubMatic-OpenWrap/prebid-server/cache/dummycache" @@ -19,6 +18,7 @@ import ( "github.com/PubMatic-OpenWrap/prebid-server/openrtb_ext" "github.com/PubMatic-OpenWrap/prebid-server/pbs" "github.com/PubMatic-OpenWrap/prebid-server/usersync" + "github.com/mxmCherry/openrtb" ) /** @@ -226,7 +226,7 @@ func SampleRequest(numberOfImpressions int, t *testing.T) *pbs.PBSRequest { pc := usersync.ParsePBSCookieFromRequest(httpReq, &config.HostCookie{}) pc.TrySync("pulsepoint", "pulsepointUser123") fakewriter := httptest.NewRecorder() - pc.SetCookieOnResponse(fakewriter, "", 90*24*time.Hour) + pc.SetCookieOnResponse(fakewriter, httpReq, "", 90*24*time.Hour) httpReq.Header.Add("Cookie", fakewriter.Header().Get("Set-Cookie")) // parse the http request cacheClient, _ := dummycache.New() diff --git a/adapters/rubicon/rubicon_test.go b/adapters/rubicon/rubicon_test.go index 79a6082f1ef..83f5dd3e831 100644 --- a/adapters/rubicon/rubicon_test.go +++ b/adapters/rubicon/rubicon_test.go @@ -20,10 +20,10 @@ import ( "strings" - "github.com/mxmCherry/openrtb" "github.com/PubMatic-OpenWrap/prebid-server/adapters" "github.com/PubMatic-OpenWrap/prebid-server/config" "github.com/PubMatic-OpenWrap/prebid-server/openrtb_ext" + "github.com/mxmCherry/openrtb" ) type rubiAppendTrackerUrlTestScenario struct { @@ -945,7 +945,7 @@ func CreatePrebidRequest(server *httptest.Server, t *testing.T) (an *RubiconAdap pc := usersync.ParsePBSCookieFromRequest(req, &config.HostCookie{}) pc.TrySync("rubicon", rubidata.buyerUID) fakewriter := httptest.NewRecorder() - pc.SetCookieOnResponse(fakewriter, "", 90*24*time.Hour) + pc.SetCookieOnResponse(fakewriter, req, "", 90*24*time.Hour) req.Header.Add("Cookie", fakewriter.Header().Get("Set-Cookie")) cacheClient, _ := dummycache.New() diff --git a/adapters/sovrn/sovrn_test.go b/adapters/sovrn/sovrn_test.go index 53d740ba310..c22f4069586 100644 --- a/adapters/sovrn/sovrn_test.go +++ b/adapters/sovrn/sovrn_test.go @@ -8,9 +8,9 @@ import ( "net/http/httptest" "testing" - "github.com/mxmCherry/openrtb" "github.com/PubMatic-OpenWrap/prebid-server/pbs" "github.com/PubMatic-OpenWrap/prebid-server/usersync" + "github.com/mxmCherry/openrtb" "context" "net/http" @@ -188,7 +188,7 @@ func SampleSovrnRequest(numberOfImpressions int, t *testing.T) *pbs.PBSRequest { pc := usersync.ParsePBSCookieFromRequest(httpReq, &config.HostCookie{}) pc.TrySync("sovrn", testSovrnUserId) fakewriter := httptest.NewRecorder() - pc.SetCookieOnResponse(fakewriter, "", 90*24*time.Hour) + pc.SetCookieOnResponse(fakewriter, httpReq, "", 90*24*time.Hour) httpReq.Header.Add("Cookie", fakewriter.Header().Get("Set-Cookie")) // parse the http request cacheClient, _ := dummycache.New() diff --git a/endpoints/setuid.go b/endpoints/setuid.go index 6f80faa17c6..6d183804978 100644 --- a/endpoints/setuid.go +++ b/endpoints/setuid.go @@ -5,13 +5,13 @@ import ( "net/http" "time" - "github.com/julienschmidt/httprouter" "github.com/PubMatic-OpenWrap/prebid-server/analytics" "github.com/PubMatic-OpenWrap/prebid-server/config" "github.com/PubMatic-OpenWrap/prebid-server/gdpr" "github.com/PubMatic-OpenWrap/prebid-server/openrtb_ext" "github.com/PubMatic-OpenWrap/prebid-server/pbsmetrics" "github.com/PubMatic-OpenWrap/prebid-server/usersync" + "github.com/julienschmidt/httprouter" ) func NewSetUIDEndpoint(cfg config.HostCookie, perms gdpr.Permissions, pbsanalytics analytics.PBSAnalyticsModule, metrics pbsmetrics.MetricsEngine) httprouter.Handle { @@ -76,7 +76,7 @@ func NewSetUIDEndpoint(cfg config.HostCookie, perms gdpr.Permissions, pbsanalyti so.Success = true } - pc.SetCookieOnResponse(w, cfg.Domain, cookieTTL) + pc.SetCookieOnResponse(w, r, cfg.Domain, cookieTTL) }) } diff --git a/pbs/usersync.go b/pbs/usersync.go index 8d1bc928949..c807ddec610 100644 --- a/pbs/usersync.go +++ b/pbs/usersync.go @@ -9,13 +9,13 @@ import ( "strings" "time" - "github.com/golang/glog" - "github.com/julienschmidt/httprouter" "github.com/PubMatic-OpenWrap/prebid-server/analytics" "github.com/PubMatic-OpenWrap/prebid-server/config" "github.com/PubMatic-OpenWrap/prebid-server/pbsmetrics" "github.com/PubMatic-OpenWrap/prebid-server/ssl" "github.com/PubMatic-OpenWrap/prebid-server/usersync" + "github.com/golang/glog" + "github.com/julienschmidt/httprouter" ) // Recaptcha code from https://github.com/haisum/recaptcha/blob/master/recaptcha.go @@ -95,7 +95,7 @@ func (deps *UserSyncDeps) OptOut(w http.ResponseWriter, r *http.Request, _ httpr pc := usersync.ParsePBSCookieFromRequest(r, deps.HostCookieConfig) pc.SetPreference(optout == "") - pc.SetCookieOnResponse(w, deps.HostCookieConfig.Domain, deps.HostCookieConfig.TTLDuration()) + pc.SetCookieOnResponse(w, r, deps.HostCookieConfig.Domain, deps.HostCookieConfig.TTLDuration()) if optout == "" { http.Redirect(w, r, deps.HostCookieConfig.OptInURL, 301) } else { diff --git a/usersync/cookie.go b/usersync/cookie.go index c660622d42d..c2a985aef80 100644 --- a/usersync/cookie.go +++ b/usersync/cookie.go @@ -5,6 +5,8 @@ import ( "encoding/json" "errors" "net/http" + "strconv" + "strings" "time" "github.com/PubMatic-OpenWrap/prebid-server/config" @@ -14,6 +16,9 @@ import ( // DEFAULT_TTL is the default amount of time which a cookie is considered valid. const DEFAULT_TTL = 14 * 24 * time.Hour const UID_COOKIE_NAME = "uids" +const chromeStr = "Chrome/" +const chromeMinVer = 67 +const chromeStrLen = len(chromeStr) // customBidderTTLs stores rules about how long a particular UID sync is valid for each bidder. // If a bidder does a cookie sync *without* listing a rule here, then the DEFAULT_TTL will be used. @@ -160,12 +165,38 @@ func (cookie *PBSCookie) GetId(bidderName openrtb_ext.BidderName) (id string, ex } // SetCookieOnResponse is a shortcut for "ToHTTPCookie(); cookie.setDomain(domain); setCookie(w, cookie)" -func (cookie *PBSCookie) SetCookieOnResponse(w http.ResponseWriter, domain string, ttl time.Duration) { +func (cookie *PBSCookie) SetCookieOnResponse(w http.ResponseWriter, r *http.Request, domain string, ttl time.Duration) { httpCookie := cookie.ToHTTPCookie(ttl) if domain != "" { httpCookie.Domain = domain } - http.SetCookie(w, httpCookie) + cookieStr := httpCookie.String() + if isChromeBrowser(r) { + cookieStr += "; SameSite=none" + } + //http.SetCookie(w, httpCookie) + if cookieStr != "" { + w.Header().Add("Set-Cookie", cookieStr) + } +} + +func isChromeBrowser(req *http.Request) bool { + result := false + ua := req.UserAgent() + + index := strings.Index(ua, chromeStr) + if index != -1 { + vIndex := index + chromeStrLen + dotIndex := strings.Index(ua[vIndex:], ".") + if dotIndex == -1 { + dotIndex = len(ua[vIndex:]) + } + version, _ := strconv.Atoi(ua[vIndex : vIndex+dotIndex]) + if version >= chromeMinVer { + result = true + } + } + return result } // Unsync removes the user's ID for the given family from this cookie. diff --git a/usersync/cookie_test.go b/usersync/cookie_test.go index 03228de3e3e..1bce2694157 100644 --- a/usersync/cookie_test.go +++ b/usersync/cookie_test.go @@ -353,7 +353,8 @@ func newSampleCookie() *PBSCookie { func writeThenRead(cookie *PBSCookie) *PBSCookie { w := httptest.NewRecorder() - cookie.SetCookieOnResponse(w, "mock-domain", 90*24*time.Hour) + req := httptest.NewRequest("GET", "http://www.prebid.com", nil) + cookie.SetCookieOnResponse(w, req, "mock-domain", 90*24*time.Hour) writtenCookie := w.HeaderMap.Get("Set-Cookie") header := http.Header{} From 6bf04b62abaf546b3557eadba1e7896479bd388e Mon Sep 17 00:00:00 2001 From: shalmali-patil Date: Fri, 26 Jul 2019 12:18:11 +0200 Subject: [PATCH 2/5] adding test cases for changes in SetCookieOnResponse --- usersync/cookie_test.go | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/usersync/cookie_test.go b/usersync/cookie_test.go index 1bce2694157..dc052d8214a 100644 --- a/usersync/cookie_test.go +++ b/usersync/cookie_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "strings" "testing" "time" @@ -362,3 +363,29 @@ func writeThenRead(cookie *PBSCookie) *PBSCookie { request := http.Request{Header: header} return ParsePBSCookieFromRequest(&request, &config.HostCookie{}) } + +func TestSetCookieOnResponseForSameSiteNone(t *testing.T) { + cookie := newSampleCookie() + w := httptest.NewRecorder() + req := httptest.NewRequest("GET", "http://www.prebid.com", nil) + req.Header.Set("User-Agent", "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/75.0.3770.142 Safari/537.36") + cookie.SetCookieOnResponse(w, req, "mock-domain", 90*24*time.Hour) + writtenCookie := w.HeaderMap.Get("Set-Cookie") + t.Log("Set-Cookie is: ", writtenCookie) + if !strings.Contains(writtenCookie, "SameSite=none") { + t.Error("Set-Cookie should contain SameSite=none") + } +} + +func TestSetCookieOnResponseForOlderChromeVersion(t *testing.T) { + cookie := newSampleCookie() + w := httptest.NewRecorder() + req := httptest.NewRequest("GET", "http://www.prebid.com", nil) + req.Header.Set("User-Agent", "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3770.142 Safari/537.36") + cookie.SetCookieOnResponse(w, req, "mock-domain", 90*24*time.Hour) + writtenCookie := w.HeaderMap.Get("Set-Cookie") + t.Log("Set-Cookie is: ", writtenCookie) + if strings.Contains(writtenCookie, "SameSite=none") { + t.Error("Set-Cookie should not contain SameSite=none") + } +} From 1fd23f32ac797bf432f0c5bc72a7b27b59df2208 Mon Sep 17 00:00:00 2001 From: shalmali-patil Date: Wed, 31 Jul 2019 15:38:16 +0200 Subject: [PATCH 3/5] committing changes for 1)setting a new 'Set-Cookie' header in /setuid flow to identify if sync-up is required and 2) checking this new cookie in /cookie_sync flow --- endpoints/cookie_sync.go | 15 ++++++++-- usersync/cookie.go | 60 +++++++++++++++++++++++++++------------- usersync/cookie_test.go | 4 +-- 3 files changed, 55 insertions(+), 24 deletions(-) diff --git a/endpoints/cookie_sync.go b/endpoints/cookie_sync.go index db2711a0c8a..484195cde39 100644 --- a/endpoints/cookie_sync.go +++ b/endpoints/cookie_sync.go @@ -9,6 +9,7 @@ import ( "math/rand" "net/http" "strconv" + "strings" "github.com/PubMatic-OpenWrap/prebid-server/analytics" "github.com/PubMatic-OpenWrap/prebid-server/config" @@ -106,8 +107,16 @@ func (deps *cookieSyncDeps) Endpoint(w http.ResponseWriter, r *http.Request, _ h parsedReq.Bidders = append(parsedReq.Bidders, string(bidder)) } } + isBrowserApplicable := usersync.IsBrowserApplicableForSameSite(r) + needSyncupForSameSite := false + if isBrowserApplicable { + sameSiteCookie, _ := r.Cookie(usersync.SameSiteCookieName) + if sameSiteCookie == nil || !strings.Contains(sameSiteCookie.String(), usersync.SameSiteCookieValue) { + needSyncupForSameSite = true + } + } - parsedReq.filterExistingSyncs(deps.syncers, userSyncCookie) + parsedReq.filterExistingSyncs(deps.syncers, userSyncCookie, needSyncupForSameSite) adapterSyncs := make(map[openrtb_ext.BidderName]bool) for _, b := range parsedReq.Bidders { // assume all bidders will be GDPR blocked @@ -193,7 +202,7 @@ type cookieSyncRequest struct { Limit int `json:"limit"` } -func (req *cookieSyncRequest) filterExistingSyncs(valid map[openrtb_ext.BidderName]usersync.Usersyncer, cookie *usersync.PBSCookie) { +func (req *cookieSyncRequest) filterExistingSyncs(valid map[openrtb_ext.BidderName]usersync.Usersyncer, cookie *usersync.PBSCookie, needSyncupForSameSite bool) { for i := 0; i < len(req.Bidders); i++ { thisBidder := req.Bidders[i] //added hack to support to old wrapper versions having indexExchange as partner @@ -201,7 +210,7 @@ func (req *cookieSyncRequest) filterExistingSyncs(valid map[openrtb_ext.BidderNa if thisBidder == "indexExchange" { thisBidder = "ix" } - if syncer, isValid := valid[openrtb_ext.BidderName(thisBidder)]; !isValid || cookie.HasLiveSync(syncer.FamilyName()) { + if syncer, isValid := valid[openrtb_ext.BidderName(thisBidder)]; !isValid || (cookie.HasLiveSync(syncer.FamilyName()) && !needSyncupForSameSite) { req.Bidders = append(req.Bidders[:i], req.Bidders[i+1:]...) i-- } diff --git a/usersync/cookie.go b/usersync/cookie.go index c2a985aef80..c1498055eed 100644 --- a/usersync/cookie.go +++ b/usersync/cookie.go @@ -13,12 +13,18 @@ import ( "github.com/PubMatic-OpenWrap/prebid-server/openrtb_ext" ) -// DEFAULT_TTL is the default amount of time which a cookie is considered valid. -const DEFAULT_TTL = 14 * 24 * time.Hour -const UID_COOKIE_NAME = "uids" -const chromeStr = "Chrome/" -const chromeMinVer = 67 -const chromeStrLen = len(chromeStr) +const ( + // DEFAULT_TTL is the default amount of time which a cookie is considered valid. + DEFAULT_TTL = 14 * 24 * time.Hour + UID_COOKIE_NAME = "uids" + chromeStr = "Chrome/" + chromeiOSStr = "CriOS/" + chromeMinVer = 67 + chromeStrLen = len(chromeStr) + chromeiOSStrLen = len(chromeiOSStr) + SameSiteCookieName = "SSCookie" + SameSiteCookieValue = "1" +) // customBidderTTLs stores rules about how long a particular UID sync is valid for each bidder. // If a bidder does a cookie sync *without* listing a rule here, then the DEFAULT_TTL will be used. @@ -171,30 +177,46 @@ func (cookie *PBSCookie) SetCookieOnResponse(w http.ResponseWriter, r *http.Requ httpCookie.Domain = domain } cookieStr := httpCookie.String() - if isChromeBrowser(r) { - cookieStr += "; SameSite=none" + var sameSiteCookie *http.Cookie + if IsBrowserApplicableForSameSite(r) { + cookieStr += "; SameSite=None" + sameSiteCookie = &http.Cookie{ + Name: SameSiteCookieName, + Value: SameSiteCookieValue, + Expires: time.Now().Add(ttl), + Path: "/", + } + w.Header().Add("Set-Cookie", sameSiteCookie.String()) } - //http.SetCookie(w, httpCookie) if cookieStr != "" { w.Header().Add("Set-Cookie", cookieStr) } } -func isChromeBrowser(req *http.Request) bool { +func IsBrowserApplicableForSameSite(req *http.Request) bool { result := false ua := req.UserAgent() index := strings.Index(ua, chromeStr) + criOSIndex := strings.Index(ua, chromeiOSStr) if index != -1 { - vIndex := index + chromeStrLen - dotIndex := strings.Index(ua[vIndex:], ".") - if dotIndex == -1 { - dotIndex = len(ua[vIndex:]) - } - version, _ := strconv.Atoi(ua[vIndex : vIndex+dotIndex]) - if version >= chromeMinVer { - result = true - } + result = checkChromeBrowserVersion(ua, index, chromeStrLen) + } else if criOSIndex != -1 { + result = checkChromeBrowserVersion(ua, criOSIndex, chromeiOSStrLen) + } + return result +} + +func checkChromeBrowserVersion(ua string, index int, chromeStrLength int) bool { + result := false + vIndex := index + chromeStrLength + dotIndex := strings.Index(ua[vIndex:], ".") + if dotIndex == -1 { + dotIndex = len(ua[vIndex:]) + } + version, _ := strconv.Atoi(ua[vIndex : vIndex+dotIndex]) + if version >= chromeMinVer { + result = true } return result } diff --git a/usersync/cookie_test.go b/usersync/cookie_test.go index dc052d8214a..dcc22002f72 100644 --- a/usersync/cookie_test.go +++ b/usersync/cookie_test.go @@ -372,8 +372,8 @@ func TestSetCookieOnResponseForSameSiteNone(t *testing.T) { cookie.SetCookieOnResponse(w, req, "mock-domain", 90*24*time.Hour) writtenCookie := w.HeaderMap.Get("Set-Cookie") t.Log("Set-Cookie is: ", writtenCookie) - if !strings.Contains(writtenCookie, "SameSite=none") { - t.Error("Set-Cookie should contain SameSite=none") + if !strings.Contains(writtenCookie, "SSCookie=1") { + t.Error("Set-Cookie should contain SSCookie=1") } } From 2ed3becb28c47cfe4b606d03439d444eff761b1b Mon Sep 17 00:00:00 2001 From: shalmali-patil Date: Tue, 6 Aug 2019 10:18:06 +0200 Subject: [PATCH 4/5] committing changes for setting SameSite attribute to SSCookie --- usersync/cookie.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/usersync/cookie.go b/usersync/cookie.go index c1498055eed..aed14efe7f5 100644 --- a/usersync/cookie.go +++ b/usersync/cookie.go @@ -24,6 +24,7 @@ const ( chromeiOSStrLen = len(chromeiOSStr) SameSiteCookieName = "SSCookie" SameSiteCookieValue = "1" + SameSiteAttribute = "; SameSite=None" ) // customBidderTTLs stores rules about how long a particular UID sync is valid for each bidder. @@ -179,20 +180,23 @@ func (cookie *PBSCookie) SetCookieOnResponse(w http.ResponseWriter, r *http.Requ cookieStr := httpCookie.String() var sameSiteCookie *http.Cookie if IsBrowserApplicableForSameSite(r) { - cookieStr += "; SameSite=None" + cookieStr += SameSiteAttribute sameSiteCookie = &http.Cookie{ Name: SameSiteCookieName, Value: SameSiteCookieValue, Expires: time.Now().Add(ttl), Path: "/", } - w.Header().Add("Set-Cookie", sameSiteCookie.String()) + sameSiteCookieStr := sameSiteCookie.String() + sameSiteCookieStr += SameSiteAttribute + w.Header().Add("Set-Cookie", sameSiteCookieStr) } if cookieStr != "" { w.Header().Add("Set-Cookie", cookieStr) } } +// IsBrowserApplicableForSameSite function checks if browser is Chrome and browser version is greater than the minimum version for adding the SameSite attribute func IsBrowserApplicableForSameSite(req *http.Request) bool { result := false ua := req.UserAgent() From 5c65a82f1755c62855c508802153130d891f1b21 Mon Sep 17 00:00:00 2001 From: shalmali-patil Date: Thu, 8 Aug 2019 14:19:18 +0200 Subject: [PATCH 5/5] updating logic for cookie_sync to check for existence of SSCookie and not for its actual value --- endpoints/cookie_sync.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/endpoints/cookie_sync.go b/endpoints/cookie_sync.go index 484195cde39..10858986b2e 100644 --- a/endpoints/cookie_sync.go +++ b/endpoints/cookie_sync.go @@ -9,7 +9,6 @@ import ( "math/rand" "net/http" "strconv" - "strings" "github.com/PubMatic-OpenWrap/prebid-server/analytics" "github.com/PubMatic-OpenWrap/prebid-server/config" @@ -110,8 +109,8 @@ func (deps *cookieSyncDeps) Endpoint(w http.ResponseWriter, r *http.Request, _ h isBrowserApplicable := usersync.IsBrowserApplicableForSameSite(r) needSyncupForSameSite := false if isBrowserApplicable { - sameSiteCookie, _ := r.Cookie(usersync.SameSiteCookieName) - if sameSiteCookie == nil || !strings.Contains(sameSiteCookie.String(), usersync.SameSiteCookieValue) { + _, err1 := r.Cookie(usersync.SameSiteCookieName) + if err1 == http.ErrNoCookie { needSyncupForSameSite = true } }