From e5102de932becf953b5373493d347de04ffd5817 Mon Sep 17 00:00:00 2001 From: Mansi Nahar Date: Wed, 21 Aug 2019 15:23:20 -0400 Subject: [PATCH] Fix leaking host cookie to all bidders (#1008) --- endpoints/openrtb2/auction.go | 24 ------------------- endpoints/openrtb2/auction_test.go | 37 ------------------------------ 2 files changed, 61 deletions(-) diff --git a/endpoints/openrtb2/auction.go b/endpoints/openrtb2/auction.go index d479d70877e..355bab584e2 100644 --- a/endpoints/openrtb2/auction.go +++ b/endpoints/openrtb2/auction.go @@ -894,7 +894,6 @@ func (deps *endpointDeps) setFieldsImplicitly(httpReq *http.Request, bidReq *ope } setImpsImplicitly(httpReq, bidReq.Imp) - deps.setUserImplicitly(httpReq, bidReq) setAuctionTypeImplicitly(bidReq) } @@ -1098,20 +1097,6 @@ func getStoredRequestId(data []byte) (string, bool, error) { return string(value), true, nil } -// setUserImplicitly uses implicit info from httpReq to populate bidReq.User -func (deps *endpointDeps) setUserImplicitly(httpReq *http.Request, bidReq *openrtb.BidRequest) { - if bidReq.User == nil || bidReq.User.ID == "" { - if id, ok := parseUserID(deps.cfg, httpReq); ok { - if bidReq.User == nil { - bidReq.User = &openrtb.User{} - } - if bidReq.User.ID == "" { - bidReq.User.ID = id - } - } - } -} - // setIPImplicitly sets the IP address on bidReq, if it's not explicitly defined and we can figure it out. func setIPImplicitly(httpReq *http.Request, bidReq *openrtb.BidRequest) { if bidReq.Device == nil || bidReq.Device.IP == "" { @@ -1136,15 +1121,6 @@ func setUAImplicitly(httpReq *http.Request, bidReq *openrtb.BidRequest) { } } -// parseUserId gets this user's ID for the host machine, if it exists. -func parseUserID(cfg *config.Configuration, httpReq *http.Request) (string, bool) { - if hostCookie, err := httpReq.Cookie(cfg.HostCookie.CookieName); hostCookie != nil && err == nil { - return hostCookie.Value, true - } else { - return "", false - } -} - // Check if a request comes from a Safari browser func checkSafari(r *http.Request) (isSafari bool) { isSafari = false diff --git a/endpoints/openrtb2/auction_test.go b/endpoints/openrtb2/auction_test.go index 27cda3b989d..65b13bbb47d 100644 --- a/endpoints/openrtb2/auction_test.go +++ b/endpoints/openrtb2/auction_test.go @@ -112,43 +112,6 @@ func TestExplicitUserId(t *testing.T) { } } -// TestImplicitUserId makes sure that that bidrequest.user.id gets populated from the host cookie, if it wasn't sent explicitly. -func TestImplicitUserId(t *testing.T) { - cookieName := "userid" - mockId := "12345" - cfg := &config.Configuration{ - MaxRequestSize: maxSize, - HostCookie: config.HostCookie{ - CookieName: cookieName, - }, - } - ex := &mockExchange{} - - request := httptest.NewRequest("POST", "/openrtb2/auction", strings.NewReader(validRequest(t, "site.json"))) - request.AddCookie(&http.Cookie{ - Name: cookieName, - Value: mockId, - }) - // NewMetrics() will create a new go_metrics MetricsEngine, bypassing the need for a crafted configuration set to support it. - // As a side effect this gives us some coverage of the go_metrics piece of the metrics engine. - theMetrics := pbsmetrics.NewMetrics(metrics.NewRegistry(), openrtb_ext.BidderList()) - - endpoint, _ := NewEndpoint(ex, newParamsValidator(t), empty_fetcher.EmptyFetcher{}, empty_fetcher.EmptyFetcher{}, cfg, theMetrics, analyticsConf.NewPBSAnalytics(&config.Analytics{}), map[string]string{}, []byte{}, openrtb_ext.BidderMap) - endpoint(httptest.NewRecorder(), request, nil) - - if ex.lastRequest == nil { - t.Fatalf("The request never made it into the Exchange.") - } - - if ex.lastRequest.User == nil { - t.Fatalf("The exchange should have received a request with a non-nil user.") - } - - if ex.lastRequest.User.ID != mockId { - t.Errorf("Bad User ID. Expected %s, got %s", mockId, ex.lastRequest.User.ID) - } -} - // TestGoodRequests makes sure we return 200s on good requests. func TestGoodRequests(t *testing.T) { exemplary := &getResponseFromDirectory{