Skip to content

Commit

Permalink
GDPR: require host specify default value (#1859)
Browse files Browse the repository at this point in the history
  • Loading branch information
bsardo authored Jun 16, 2021
1 parent 037bd7f commit f894d18
Show file tree
Hide file tree
Showing 11 changed files with 113 additions and 60 deletions.
14 changes: 8 additions & 6 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ type HTTPClient struct {
IdleConnTimeout int `mapstructure:"idle_connection_timeout_seconds"`
}

func (cfg *Configuration) validate() []error {
func (cfg *Configuration) validate(v *viper.Viper) []error {
var errs []error
errs = cfg.AuctionTimeouts.validate(errs)
errs = cfg.StoredRequests.validate(errs)
Expand All @@ -105,7 +105,7 @@ func (cfg *Configuration) validate() []error {
if cfg.MaxRequestSize < 0 {
errs = append(errs, fmt.Errorf("cfg.max_request_size must be >= 0. Got %d", cfg.MaxRequestSize))
}
errs = cfg.GDPR.validate(errs)
errs = cfg.GDPR.validate(v, errs)
errs = cfg.CurrencyConverter.validate(errs)
errs = validateAdapters(cfg.Adapters, errs)
errs = cfg.Debug.validate(errs)
Expand Down Expand Up @@ -207,8 +207,10 @@ type GDPR struct {
EEACountriesMap map[string]struct{}
}

func (cfg *GDPR) validate(errs []error) []error {
if cfg.DefaultValue != "0" && cfg.DefaultValue != "1" {
func (cfg *GDPR) validate(v *viper.Viper, errs []error) []error {
if !v.IsSet("gdpr.default_value") {
errs = append(errs, fmt.Errorf("gdpr.default_value is required and must be specified"))
} else if cfg.DefaultValue != "0" && cfg.DefaultValue != "1" {
errs = append(errs, fmt.Errorf("gdpr.default_value must be 0 or 1"))
}
if cfg.HostVendorID < 0 || cfg.HostVendorID > 0xffff {
Expand Down Expand Up @@ -520,7 +522,7 @@ func New(v *viper.Viper) (*Configuration, error) {

glog.Info("Logging the resolved configuration:")
logGeneral(reflect.ValueOf(c), " \t")
if errs := c.validate(); len(errs) > 0 {
if errs := c.validate(v); len(errs) > 0 {
return &c, errortypes.NewAggregateError("validation errors", errs)
}

Expand Down Expand Up @@ -938,9 +940,9 @@ func SetupViper(v *viper.Viper, filename string) {
v.SetDefault("analytics.pubstack.buffers.count", 100)
v.SetDefault("analytics.pubstack.buffers.timeout", "900s")
v.SetDefault("amp_timeout_adjustment_ms", 0)
v.BindEnv("gdpr.default_value")
v.SetDefault("gdpr.enabled", true)
v.SetDefault("gdpr.host_vendor_id", 0)
v.SetDefault("gdpr.default_value", "1")
v.SetDefault("gdpr.timeouts_ms.init_vendorlist_fetches", 0)
v.SetDefault("gdpr.timeouts_ms.active_vendorlist_fetch", 0)
v.SetDefault("gdpr.non_standard_publishers", []string{""})
Expand Down
76 changes: 46 additions & 30 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,7 @@ func TestExternalCacheURLValidate(t *testing.T) {
}

func TestDefaults(t *testing.T) {
v := viper.New()
SetupViper(v, "")
cfg, err := New(v)
assert.NoError(t, err, "Setting up config should work but it doesn't")
cfg, _ := newDefaultConfig(t)

cmpInts(t, "port", cfg.Port, 8000)
cmpInts(t, "admin_port", cfg.AdminPort, 6060)
Expand Down Expand Up @@ -148,7 +145,7 @@ func TestDefaults(t *testing.T) {
var fullConfig = []byte(`
gdpr:
host_vendor_id: 15
default_value: "0"
default_value: "1"
non_standard_publishers: ["siteID","fake-site-id","appID","agltb3B1Yi1pbmNyDAsSA0FwcBiJkfIUDA"]
ccpa:
enforce: true
Expand Down Expand Up @@ -352,7 +349,7 @@ func TestFullConfig(t *testing.T) {
cmpInts(t, "http_client_cache.max_idle_connections_per_host", cfg.CacheClient.MaxIdleConnsPerHost, 2)
cmpInts(t, "http_client_cache.idle_connection_timeout_seconds", cfg.CacheClient.IdleConnTimeout, 3)
cmpInts(t, "gdpr.host_vendor_id", cfg.GDPR.HostVendorID, 15)
cmpStrings(t, "gdpr.default_value", cfg.GDPR.DefaultValue, "0")
cmpStrings(t, "gdpr.default_value", cfg.GDPR.DefaultValue, "1")

//Assert the NonStandardPublishers was correctly unmarshalled
cmpStrings(t, "gdpr.non_standard_publishers", cfg.GDPR.NonStandardPublishers[0], "siteID")
Expand Down Expand Up @@ -429,6 +426,7 @@ func TestFullConfig(t *testing.T) {
func TestUnmarshalAdapterExtraInfo(t *testing.T) {
v := viper.New()
SetupViper(v, "")
v.Set("gdpr.default_value", "0")
v.SetConfigType("yaml")
v.ReadConfig(bytes.NewBuffer(adapterExtraInfoConfig))
cfg, err := New(v)
Expand Down Expand Up @@ -484,14 +482,18 @@ func TestValidConfig(t *testing.T) {
},
}

v := viper.New()
v.Set("gdpr.default_value", "0")

resolvedStoredRequestsConfig(&cfg)
err := cfg.validate()
err := cfg.validate(v)
assert.Nil(t, err, "OpenRTB filesystem config should work. %v", err)
}

func TestMigrateConfig(t *testing.T) {
v := viper.New()
SetupViper(v, "")
v.Set("gdpr.default_value", "0")
v.SetConfigType("yaml")
v.ReadConfig(bytes.NewBuffer(oldStoredRequestsConfig))
migrateConfig(v)
Expand All @@ -508,10 +510,7 @@ func TestMigrateConfigFromEnv(t *testing.T) {
defer os.Unsetenv("PBS_STORED_REQUESTS_FILESYSTEM")
}
os.Setenv("PBS_STORED_REQUESTS_FILESYSTEM", "true")
v := viper.New()
SetupViper(v, "")
cfg, err := New(v)
assert.NoError(t, err, "Setting up config should work but it doesn't")
cfg, _ := newDefaultConfig(t)
cmpBools(t, "stored_requests.filesystem.enabled", true, cfg.StoredRequests.Files.Enabled)
}

Expand Down Expand Up @@ -591,6 +590,7 @@ func TestMigrateConfigPurposeOneTreatment(t *testing.T) {
func TestInvalidAdapterEndpointConfig(t *testing.T) {
v := viper.New()
SetupViper(v, "")
v.Set("gdpr.default_value", "0")
v.SetConfigType("yaml")
v.ReadConfig(bytes.NewBuffer(invalidAdapterEndpointConfig))
_, err := New(v)
Expand All @@ -600,23 +600,24 @@ func TestInvalidAdapterEndpointConfig(t *testing.T) {
func TestInvalidAdapterUserSyncURLConfig(t *testing.T) {
v := viper.New()
SetupViper(v, "")
v.Set("gdpr.default_value", "0")
v.SetConfigType("yaml")
v.ReadConfig(bytes.NewBuffer(invalidUserSyncURLConfig))
_, err := New(v)
assert.Error(t, err, "invalid user_sync URL in config should return an error")
}

func TestNegativeRequestSize(t *testing.T) {
cfg := newDefaultConfig(t)
cfg, v := newDefaultConfig(t)
cfg.MaxRequestSize = -1
assertOneError(t, cfg.validate(), "cfg.max_request_size must be >= 0. Got -1")
assertOneError(t, cfg.validate(v), "cfg.max_request_size must be >= 0. Got -1")
}

func TestNegativePrometheusTimeout(t *testing.T) {
cfg := newDefaultConfig(t)
cfg, v := newDefaultConfig(t)
cfg.Metrics.Prometheus.Port = 8001
cfg.Metrics.Prometheus.TimeoutMillisRaw = 0
assertOneError(t, cfg.validate(), "metrics.prometheus.timeout_ms must be positive if metrics.prometheus.port is defined. Got timeout=0 and port=8001")
assertOneError(t, cfg.validate(v), "metrics.prometheus.timeout_ms must be positive if metrics.prometheus.port is defined. Got timeout=0 and port=8001")
}

func TestInvalidHostVendorID(t *testing.T) {
Expand All @@ -638,44 +639,57 @@ func TestInvalidHostVendorID(t *testing.T) {
}

for _, tt := range tests {
cfg := newDefaultConfig(t)
cfg, v := newDefaultConfig(t)
cfg.GDPR.HostVendorID = tt.vendorID
errs := cfg.validate()
errs := cfg.validate(v)

assert.Equal(t, 1, len(errs), tt.description)
assert.EqualError(t, errs[0], tt.wantErrorMsg, tt.description)
}
}

func TestInvalidAMPException(t *testing.T) {
cfg := newDefaultConfig(t)
cfg, v := newDefaultConfig(t)
cfg.GDPR.AMPException = true
assertOneError(t, cfg.validate(), "gdpr.amp_exception has been discontinued and must be removed from your config. If you need to disable GDPR for AMP, you may do so per-account (gdpr.integration_enabled.amp) or at the host level for the default account (account_defaults.gdpr.integration_enabled.amp)")
assertOneError(t, cfg.validate(v), "gdpr.amp_exception has been discontinued and must be removed from your config. If you need to disable GDPR for AMP, you may do so per-account (gdpr.integration_enabled.amp) or at the host level for the default account (account_defaults.gdpr.integration_enabled.amp)")
}

func TestInvalidGDPRDefaultValue(t *testing.T) {
cfg := newDefaultConfig(t)
cfg, v := newDefaultConfig(t)
cfg.GDPR.DefaultValue = "2"
assertOneError(t, cfg.validate(), "gdpr.default_value must be 0 or 1")
assertOneError(t, cfg.validate(v), "gdpr.default_value must be 0 or 1")
}

func TestMissingGDPRDefaultValue(t *testing.T) {
v := viper.New()

cfg, _ := newDefaultConfig(t)
assertOneError(t, cfg.validate(v), "gdpr.default_value is required and must be specified")
}

func TestNegativeCurrencyConverterFetchInterval(t *testing.T) {
v := viper.New()
v.Set("gdpr.default_value", "0")

cfg := Configuration{
CurrencyConverter: CurrencyConverter{
FetchIntervalSeconds: -1,
},
}
err := cfg.validate()
err := cfg.validate(v)
assert.NotNil(t, err, "cfg.currency_converter.fetch_interval_seconds should prevent negative values, but it doesn't")
}

func TestOverflowedCurrencyConverterFetchInterval(t *testing.T) {
v := viper.New()
v.Set("gdpr.default_value", "0")

cfg := Configuration{
CurrencyConverter: CurrencyConverter{
FetchIntervalSeconds: (0xffff) + 1,
},
}
err := cfg.validate()
err := cfg.validate(v)
assert.NotNil(t, err, "cfg.currency_converter.fetch_interval_seconds prevent values over %d, but it doesn't", 0xffff)
}

Expand Down Expand Up @@ -733,6 +747,7 @@ func TestNewCallsRequestValidation(t *testing.T) {
for _, test := range testCases {
v := viper.New()
SetupViper(v, "")
v.Set("gdpr.default_value", "0")
v.SetConfigType("yaml")
v.ReadConfig(bytes.NewBuffer([]byte(
`request_validation:
Expand All @@ -750,31 +765,32 @@ func TestNewCallsRequestValidation(t *testing.T) {
}

func TestValidateDebug(t *testing.T) {
cfg := newDefaultConfig(t)
cfg, v := newDefaultConfig(t)
cfg.Debug.TimeoutNotification.SamplingRate = 1.1

err := cfg.validate()
err := cfg.validate(v)
assert.NotNil(t, err, "cfg.debug.timeout_notification.sampling_rate should not be allowed to be greater than 1.0, but it was allowed")
}

func TestValidateAccountsConfigRestrictions(t *testing.T) {
cfg := newDefaultConfig(t)
cfg, v := newDefaultConfig(t)
cfg.Accounts.Files.Enabled = true
cfg.Accounts.HTTP.Endpoint = "http://localhost"
cfg.Accounts.Postgres.ConnectionInfo.Database = "accounts"

errs := cfg.validate()
errs := cfg.validate(v)
assert.Len(t, errs, 1)
assert.Contains(t, errs, errors.New("accounts.postgres: retrieving accounts via postgres not available, use accounts.files"))
}

func newDefaultConfig(t *testing.T) *Configuration {
func newDefaultConfig(t *testing.T) (*Configuration, *viper.Viper) {
v := viper.New()
SetupViper(v, "")
v.Set("gdpr.default_value", "0")
v.SetConfigType("yaml")
cfg, err := New(v)
assert.NoError(t, err)
return cfg
assert.NoError(t, err, "Setting up config should work but it doesn't")
return cfg, v
}

func assertOneError(t *testing.T, errs []error, message string) {
Expand Down
1 change: 1 addition & 0 deletions endpoints/auction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,7 @@ func TestCacheVideoOnly(t *testing.T) {
ctx := context.TODO()
v := viper.New()
config.SetupViper(v, "")
v.Set("gdpr.default_value", "0")
cfg, err := config.New(v)
if err != nil {
t.Fatal(err.Error())
Expand Down
15 changes: 10 additions & 5 deletions exchange/exchange.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ type exchange struct {
gDPR gdpr.Permissions
currencyConverter *currency.RateConverter
externalURL string
gdprDefaultValue string
gdprDefaultValue gdpr.Signal
privacyConfig config.Privacy
categoriesFetcher stored_requests.CategoryFetcher
bidIDGenerator BidIDGenerator
Expand Down Expand Up @@ -110,6 +110,11 @@ func (randomDeduplicateBidBooleanGenerator) Generate() bool {
}

func NewExchange(adapters map[openrtb_ext.BidderName]adaptedBidder, cache prebid_cache_client.Client, cfg *config.Configuration, metricsEngine metrics.MetricsEngine, infos config.BidderInfos, gDPR gdpr.Permissions, currencyConverter *currency.RateConverter, categoriesFetcher stored_requests.CategoryFetcher) Exchange {
gdprDefaultValue := gdpr.SignalYes
if cfg.GDPR.DefaultValue == "0" {
gdprDefaultValue = gdpr.SignalNo
}

return &exchange{
adapterMap: adapters,
bidderInfo: infos,
Expand All @@ -120,7 +125,7 @@ func NewExchange(adapters map[openrtb_ext.BidderName]adaptedBidder, cache prebid
externalURL: cfg.ExternalURL,
gDPR: gDPR,
me: metricsEngine,
gdprDefaultValue: cfg.GDPR.DefaultValue,
gdprDefaultValue: gdprDefaultValue,
privacyConfig: config.Privacy{
CCPA: cfg.CCPA,
GDPR: cfg.GDPR,
Expand Down Expand Up @@ -301,7 +306,7 @@ func (e *exchange) HoldAuction(ctx context.Context, r AuctionRequest, debugLog *
return e.buildBidResponse(ctx, liveAdapters, adapterBids, r.BidRequest, adapterExtra, auc, bidResponseExt, cacheInstructions.returnCreative, errs)
}

func (e *exchange) parseGDPRDefaultValue(bidRequest *openrtb2.BidRequest) string {
func (e *exchange) parseGDPRDefaultValue(bidRequest *openrtb2.BidRequest) gdpr.Signal {
gdprDefaultValue := e.gdprDefaultValue
var geo *openrtb2.Geo = nil

Expand All @@ -314,10 +319,10 @@ func (e *exchange) parseGDPRDefaultValue(bidRequest *openrtb2.BidRequest) string
// 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.privacyConfig.GDPR.EEACountriesMap[strings.ToUpper(geo.Country)]; found {
gdprDefaultValue = "1"
gdprDefaultValue = gdpr.SignalYes
} else if len(geo.Country) == 3 {
// The country field is formatted properly as a three character country code
gdprDefaultValue = "0"
gdprDefaultValue = gdpr.SignalNo
}
}

Expand Down
7 changes: 6 additions & 1 deletion exchange/exchange_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2162,14 +2162,19 @@ func newExchangeForTests(t *testing.T, filename string, expectations map[string]
t.Fatalf("Failed to create a category Fetcher: %v", error)
}

gdprDefaultValue := gdpr.SignalYes
if privacyConfig.GDPR.DefaultValue == "0" {
gdprDefaultValue = gdpr.SignalNo
}

return &exchange{
adapterMap: bidderAdapters,
me: metricsConf.NewMetricsEngine(&config.Configuration{}, openrtb_ext.CoreBidderNames()),
cache: &wellBehavedCache{},
cacheTime: 0,
gDPR: &permissionsMock{allowAllBidders: true},
currencyConverter: currency.NewRateConverter(&http.Client{}, "", time.Duration(0)),
gdprDefaultValue: privacyConfig.GDPR.DefaultValue,
gdprDefaultValue: gdprDefaultValue,
privacyConfig: privacyConfig,
categoriesFetcher: categoriesFetcher,
bidderInfo: bidderInfos,
Expand Down
2 changes: 1 addition & 1 deletion exchange/targeting_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func runTargetingAuction(t *testing.T, mockBids map[openrtb_ext.BidderName][]*op
cacheTime: time.Duration(0),
gDPR: gdpr.AlwaysAllow{},
currencyConverter: currency.NewRateConverter(&http.Client{}, "", time.Duration(0)),
gdprDefaultValue: "1",
gdprDefaultValue: gdpr.SignalYes,
categoriesFetcher: categoriesFetcher,
bidIDGenerator: &mockBidIDGenerator{false, false},
}
Expand Down
4 changes: 2 additions & 2 deletions exchange/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func cleanOpenRTBRequests(ctx context.Context,
requestExt *openrtb_ext.ExtRequest,
gDPR gdpr.Permissions,
metricsEngine metrics.MetricsEngine,
gdprDefaultValue string,
gdprDefaultValue gdpr.Signal,
privacyConfig config.Privacy,
account *config.Account) (allowedBidderRequests []BidderRequest, privacyLabels metrics.PrivacyLabels, errs []error) {

Expand Down Expand Up @@ -87,7 +87,7 @@ func cleanOpenRTBRequests(ctx context.Context,
if err != nil {
errs = append(errs, err)
}
gdprEnforced := gdprSignal == gdpr.SignalYes || (gdprSignal == gdpr.SignalAmbiguous && gdprDefaultValue == "1")
gdprEnforced := gdprSignal == gdpr.SignalYes || (gdprSignal == gdpr.SignalAmbiguous && gdprDefaultValue == gdpr.SignalYes)

ccpaEnforcer, err := extractCCPA(req.BidRequest, privacyConfig, &req.Account, aliases, integrationTypeMap[req.LegacyLabels.RType])
if err != nil {
Expand Down
Loading

0 comments on commit f894d18

Please sign in to comment.