Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable geo activation of GDPR flag #1427

Merged
merged 9 commits into from
Aug 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,11 @@ type GDPR struct {
TCF1 TCF1 `mapstructure:"tcf1"`
TCF2 TCF2 `mapstructure:"tcf2"`
AMPException bool `mapstructure:"amp_exception"`
// EEACountries (EEA = European Economic Area) are a list of countries where we should assume GDPR applies.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect! Thank you.

// If the gdpr flag is unset in a request, but geo.country is set, we will assume GDPR applies if and only
// if the country matches one on this list. If both the GDPR flag and country are not set, we default
// to UsersyncIfAmbiguous
EEACountries []string `mapstructure:"eea_countries"`
}

func (cfg *GDPR) validate(errs configErrors) configErrors {
Expand Down Expand Up @@ -903,6 +908,10 @@ func SetupViper(v *viper.Viper, filename string) {
v.SetDefault("gdpr.tcf2.purpose_one_treatement.enabled", true)
v.SetDefault("gdpr.tcf2.purpose_one_treatement.access_allowed", true)
v.SetDefault("gdpr.amp_exception", false)
v.SetDefault("gdpr.eea_countries", []string{"ALA", "AUT", "BEL", "BGR", "HRV", "CYP", "CZE", "DNK", "EST",
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
"FIN", "FRA", "GUF", "DEU", "GIB", "GRC", "GLP", "GGY", "HUN", "ISL", "IRL", "IMN", "ITA", "JEY", "LVA",
"LIE", "LTU", "LUX", "MLT", "MTQ", "MYT", "NLD", "NOR", "POL", "PRT", "REU", "ROU", "BLM", "MAF", "SPM",
"SVK", "SVN", "ESP", "SWE", "GBR"})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a future PR we might want to have a test in config/config_test.go for to assert the country list parsing from the config file

v.SetDefault("ccpa.enforce", false)
v.SetDefault("lmt.enforce", true)
v.SetDefault("currency_converter.fetch_url", "https://cdn.jsdelivr.net/gh/prebid/currency-file@1/latest.json")
Expand Down
25 changes: 24 additions & 1 deletion exchange/exchange.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ type exchange struct {
UsersyncIfAmbiguous bool
defaultTTLs config.DefaultTTLs
privacyConfig config.Privacy
eeaCountries map[string]struct{}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a future PR, maybe we can refactor to have all GDPR related fields inside some privacy wrapper in order to simplify the exchange struct

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's why I added privacyConfig. I didn't think to comment about adding this in there though.

}

// Container to pass out response ext data from the GetAllBids goroutines back into the main thread
Expand All @@ -75,6 +76,10 @@ type bidResponseWrapper struct {
func NewExchange(client *http.Client, cache prebid_cache_client.Client, cfg *config.Configuration, metricsEngine pbsmetrics.MetricsEngine, infos adapters.BidderInfos, gDPR gdpr.Permissions, currencyConverter *currencies.RateConverter) Exchange {
e := new(exchange)

var s struct{}
for _, c := range cfg.GDPR.EEACountries {
e.eeaCountries[c] = s
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inside config/config.go we build the NonStandardPublisherMap and BlacklistedAppMap in a similar fashion, maybe we can refactor in the future to set up the map in the config file instead.

153   type GDPR struct {
154       HostVendorID            int          `mapstructure:"host_vendor_id"`
155       UsersyncIfAmbiguous     bool         `mapstructure:"usersync_if_ambiguous"`
156       Timeouts                GDPRTimeouts `mapstructure:"timeouts_ms"`
157       NonStandardPublishers   []string     `mapstructure:"non_standard_publishers,flow"`
158       NonStandardPublisherMap map[string]int
159       TCF1                    TCF1 `mapstructure:"tcf1"`
160       TCF2                    TCF2 `mapstructure:"tcf2"`
161       AMPException            bool `mapstructure:"amp_exception"`
162       // EEACountries (EEA = European Economic Area) are a list of countries where we should assume GDPR applies.
163       // If the gdpr flag is unset in a request, but geo.country is set, we will assume GDPR applies if and only
164       // if the country matches one on this list. If both the GDPR flag and country are not set, we default
165       // to UsersyncIfAmbiguous
166       EEACountries []string `mapstructure:"eea_countries"`
    +     EEACountryMap map[string]struct{}
167   }
168   *--360 lines: func (cfg *GDPR) validate(errs configErrors) configErrors {------------------------------------
528   // New uses viper to get our server configurations.
529   func New(v *viper.Viper) (*Configuration, error) {
530       var c Configuration
531       if err := v.Unmarshal(&c); err != nil {
532           return nil, fmt.Errorf("viper failed to unmarshal app config: %v", err)
533       }
534       c.setDerivedDefaults()
535  
536   *--  8 lines: if err := c.RequestValidation.Parse(); err != nil {--------------------------------------------
544  
545       // To look for a request's publisher_id in the NonStandardPublishers list in
546       // O(1) time, we fill this hash table located in the NonStandardPublisherMap field of GDPR
547       c.GDPR.NonStandardPublisherMap = make(map[string]int)
548       for i := 0; i < len(c.GDPR.NonStandardPublishers); i++ {
549           c.GDPR.NonStandardPublisherMap[c.GDPR.NonStandardPublishers[i]] = 1
550       }
551  
    +     var s struct{}
    +     for _, c := range c.GDPR.EEACountries {
    +     	c.GDPR.EEACountryMap[c] = s
    +     }
552       // To look for a request's app_id in O(1) time, we fill this hash table located in the
553       // the BlacklistedApps field of the Configuration struct defined in this file
554       c.BlacklistedAppMap = make(map[string]bool)
555       for i := 0; i < len(c.BlacklistedApps); i++ {
556           c.BlacklistedAppMap[c.BlacklistedApps[i]] = true
557       }
config/config.go

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's only used in the exchange endpoint though. I'm personally happy with its current location.

e.adapterMap = newAdapterMap(client, cfg, infos, metricsEngine)
e.cache = cache
e.cacheTime = time.Duration(cfg.CacheURL.ExpectedTimeMillis) * time.Millisecond
Expand Down Expand Up @@ -121,9 +126,27 @@ func (e *exchange) HoldAuction(ctx context.Context, bidRequest *openrtb.BidReque
e.me.RecordImps(impLabels)
}

// Make our best guess if GDPR applies
usersyncIfAmbiguous := e.UsersyncIfAmbiguous
var geo *openrtb.Geo = nil
if bidRequest.User != nil && bidRequest.User.Geo != nil {
geo = bidRequest.User.Geo
} else if bidRequest.Device != nil && bidRequest.Device.Geo != nil {
geo = bidRequest.Device.Geo
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
}
if geo != nil {
// If we have a country set, and it is on the list, we assume GDPR applies if not set on the request.
// Otherwise we assume it does not apply as long as it appears "valid" (is 3 characters long).
if _, found := e.eeaCountries[strings.ToUpper(geo.Country)]; found {
usersyncIfAmbiguous = false
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
} else if len(geo.Country) == 3 {
// The country field is formatted properly as a three character country code
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
usersyncIfAmbiguous = true
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make it extra readable maybe we could refactor this entire block to live its own function

129     // Make our best guess if GDPR applies
    +   usersyncIfAmbiguous := processUserSyncEnforcement(bidRequest, e)
130 -   usersyncIfAmbiguous := e.UsersyncIfAmbiguous
131 -   var geo *openrtb.Geo = nil
132 -   if bidRequest.User != nil && bidRequest.User.Geo != nil {
133 -       geo = bidRequest.User.Geo
134 -   } else if bidRequest.Device != nil && bidRequest.Device.Geo != nil {
135 -       geo = bidRequest.Device.Geo
136 -   }
137 -   if geo != nil {
138 -       // If we have a country set, and it is on the list, we assume GDPR applies if not set on the request.
139 -       // Otherwise we assume it does not apply as long as it appears "valid" (is 3 characters long).
140 -       if _, found := e.eeaCountries[strings.ToUpper(geo.Country)]; found {
141 -           usersyncIfAmbiguous = false
142 -       } else if len(geo.Country) == 3 {
143 -           // The country field is formatted properly as a three character country code
144 -           usersyncIfAmbiguous = true
145 -       }
146 -   }
exchange/exchange.go

// Slice of BidRequests, each a copy of the original cleaned to only contain bidder data for the named bidder
blabels := make(map[openrtb_ext.BidderName]*pbsmetrics.AdapterLabels)
cleanRequests, aliases, privacyLabels, errs := cleanOpenRTBRequests(ctx, bidRequest, requestExt, usersyncs, blabels, labels, e.gDPR, e.UsersyncIfAmbiguous, e.privacyConfig)
cleanRequests, aliases, privacyLabels, errs := cleanOpenRTBRequests(ctx, bidRequest, requestExt, usersyncs, blabels, labels, e.gDPR, usersyncIfAmbiguous, e.privacyConfig)

e.me.RecordRequestPrivacy(privacyLabels)

Expand Down
27 changes: 19 additions & 8 deletions exchange/exchange_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -909,6 +909,9 @@ func runSpec(t *testing.T, filename string, spec *exchangeSpec) {
LMT: config.LMT{
Enforce: spec.EnforceLMT,
},
GDPR: config.GDPR{
UsersyncIfAmbiguous: !spec.AssumeGDPRApplies,
},
}

ex := newExchangeForTests(t, filename, spec.OutgoingRequests, aliases, privacyConfig)
Expand Down Expand Up @@ -1026,15 +1029,22 @@ func newExchangeForTests(t *testing.T, filename string, expectations map[string]
}
}

var s struct{}
eeac := make(map[string]struct{})
for _, c := range []string{"FIN", "FRA", "GUF"} {
eeac[c] = s
}

return &exchange{
adapterMap: adapters,
me: metricsConf.NewMetricsEngine(&config.Configuration{}, openrtb_ext.BidderList()),
cache: &wellBehavedCache{},
cacheTime: 0,
gDPR: gdpr.AlwaysAllow{},
gDPR: gdpr.AlwaysFail{},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hans I'm getting this output. Is it just me?

╭─ ~/go/src/github.com/prebid/prebid-server/exchange  ‹PR1427*›
╰─➤  go test
# github.com/prebid/prebid-server/exchange [github.com/prebid/prebid-server/exchange.test]
./exchange_test.go:1046:3: cannot use "github.com/prebid/prebid-server/gdpr".AlwaysFail literal (type "github.com/prebid/prebid-server/gdpr".AlwaysFail) as type "github.com/prebid/prebid-server/gdpr".Permissions in field value:
        "github.com/prebid/prebid-server/gdpr".AlwaysFail does not implement "github.com/prebid/prebid-server/gdpr".Permissions (wrong type for PersonalInfoAllowed method)
                have PersonalInfoAllowed(context.Context, openrtb_ext.BidderName, string, string) (bool, bool, error)
                want PersonalInfoAllowed(context.Context, openrtb_ext.BidderName, string, string) (bool, bool, bool, error)
FAIL    github.com/prebid/prebid-server/exchange [build failed]

currencyConverter: currencies.NewRateConverter(&http.Client{}, "", time.Duration(0)),
UsersyncIfAmbiguous: false,
UsersyncIfAmbiguous: privacyConfig.GDPR.UsersyncIfAmbiguous,
privacyConfig: privacyConfig,
eeaCountries: eeac,
}
}

Expand Down Expand Up @@ -1882,12 +1892,13 @@ func TestUpdateHbPbCatDur(t *testing.T) {
}

type exchangeSpec struct {
IncomingRequest exchangeRequest `json:"incomingRequest"`
OutgoingRequests map[string]*bidderSpec `json:"outgoingRequests"`
Response exchangeResponse `json:"response,omitempty"`
EnforceCCPA bool `json:"enforceCcpa"`
EnforceLMT bool `json:"enforceLmt"`
DebugLog *DebugLog `json:"debuglog,omitempty"`
IncomingRequest exchangeRequest `json:"incomingRequest"`
OutgoingRequests map[string]*bidderSpec `json:"outgoingRequests"`
Response exchangeResponse `json:"response,omitempty"`
EnforceCCPA bool `json:"enforceCcpa"`
EnforceLMT bool `json:"enforceLmt"`
AssumeGDPRApplies bool `json:"assume_gdpr_applies"`
DebugLog *DebugLog `json:"debuglog,omitempty"`
}

type exchangeRequest struct {
Expand Down
64 changes: 64 additions & 0 deletions exchange/exchangetest/gdpr-geo-eu-off-device.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
{
"assume_gdpr_applies": false,
"incomingRequest": {
"ortbRequest": {
"id": "some-request-id",
"site": {
"page": "test.somepage.com"
},
"imp": [{
"id": "my-imp-id",
"video": {
"mimes": ["video/mp4"]
},
"ext": {
"appnexus": {
"placementId": 1
}
}
}],
"user": {
"buyeruid": "some-buyer-id"
},
"device": {
"geo": {
"country": "FRA"
}
}
}
},
"outgoingRequests": {
"appnexus": {
"expectRequest": {
"ortbRequest": {
"id": "some-request-id",
"site": {
"page": "test.somepage.com"
},
"imp": [{
"id": "my-imp-id",
"video": {
"mimes": ["video/mp4"]
},
"ext": {
"bidder": {
"placementId": 1
}
}
}],
"user": {
},
"device": {
"geo": {
"country": "FRA"
}
}
},
"bidAdjustment": 1.0
},
"mockResponse": {
"errors": ["appnexus-error"]
}
}
}
}
60 changes: 60 additions & 0 deletions exchange/exchangetest/gdpr-geo-eu-off.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
{
"assume_gdpr_applies": false,
"incomingRequest": {
"ortbRequest": {
"id": "some-request-id",
"site": {
"page": "test.somepage.com"
},
"imp": [{
"id": "my-imp-id",
"video": {
"mimes": ["video/mp4"]
},
"ext": {
"appnexus": {
"placementId": 1
}
}
}],
"user": {
"buyeruid": "some-buyer-id",
"geo": {
"country": "FRA"
}
}
}
},
"outgoingRequests": {
"appnexus": {
"expectRequest": {
"ortbRequest": {
"id": "some-request-id",
"site": {
"page": "test.somepage.com"
},
"imp": [{
"id": "my-imp-id",
"video": {
"mimes": ["video/mp4"]
},
"ext": {
"bidder": {
"placementId": 1
}
}
}],
"user": {
"geo": {
"country": "FRA"
}
}
},
"bidAdjustment": 1.0
},
"mockResponse": {
"errors": ["appnexus-error"]
}
}
}
}
60 changes: 60 additions & 0 deletions exchange/exchangetest/gdpr-geo-eu-on.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
{
"assume_gdpr_applies": true,
"incomingRequest": {
"ortbRequest": {
"id": "some-request-id",
"site": {
"page": "test.somepage.com"
},
"imp": [{
"id": "my-imp-id",
"video": {
"mimes": ["video/mp4"]
},
"ext": {
"appnexus": {
"placementId": 1
}
}
}],
"user": {
"buyeruid": "some-buyer-id",
"geo": {
"country": "FRA"
}
}
}
},
"outgoingRequests": {
"appnexus": {
"expectRequest": {
"ortbRequest": {
"id": "some-request-id",
"site": {
"page": "test.somepage.com"
},
"imp": [{
"id": "my-imp-id",
"video": {
"mimes": ["video/mp4"]
},
"ext": {
"bidder": {
"placementId": 1
}
}
}],
"user": {
"geo": {
"country": "FRA"
}
}
},
"bidAdjustment": 1.0
},
"mockResponse": {
"errors": ["appnexus-error"]
}
}
}
}
61 changes: 61 additions & 0 deletions exchange/exchangetest/gdpr-geo-usa-off.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
{
"assume_gdpr_applies": false,
"incomingRequest": {
"ortbRequest": {
"id": "some-request-id",
"site": {
"page": "test.somepage.com"
},
"imp": [{
"id": "my-imp-id",
"video": {
"mimes": ["video/mp4"]
},
"ext": {
"appnexus": {
"placementId": 1
}
}
}],
"user": {
"buyeruid": "some-buyer-id",
"geo": {
"country": "USA"
}
}
}
},
"outgoingRequests": {
"appnexus": {
"expectRequest": {
"ortbRequest": {
"id": "some-request-id",
"site": {
"page": "test.somepage.com"
},
"imp": [{
"id": "my-imp-id",
"video": {
"mimes": ["video/mp4"]
},
"ext": {
"bidder": {
"placementId": 1
}
}
}],
"user": {
"buyeruid": "some-buyer-id",
"geo": {
"country": "USA"
}
}
},
"bidAdjustment": 1.0
},
"mockResponse": {
"errors": ["appnexus-error"]
}
}
}
}
Loading