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

GDPR: require host specify default value #1859

Merged
merged 9 commits into from
Jun 16, 2021
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