Skip to content

Commit

Permalink
GDPR: require host specify default value (prebid#1859)
Browse files Browse the repository at this point in the history
  • Loading branch information
bsardo authored and sachin-pubmatic committed Aug 2, 2021
1 parent 81b9420 commit 1c83e92
Show file tree
Hide file tree
Showing 9 changed files with 95 additions and 49 deletions.
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
7 changes: 6 additions & 1 deletion exchange/exchange_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1803,14 +1803,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
21 changes: 13 additions & 8 deletions exchange/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ func TestCleanOpenRTBRequests(t *testing.T) {
for _, test := range testCases {
metricsMock := metrics.MetricsEngineMock{}
permissions := permissionsMock{allowAllBidders: true, passGeo: true, passID: true}
bidderRequests, _, err := cleanOpenRTBRequests(context.Background(), test.req, nil, &permissions, &metricsMock, "0", privacyConfig, nil)
bidderRequests, _, err := cleanOpenRTBRequests(context.Background(), test.req, nil, &permissions, &metricsMock, gdpr.SignalNo, privacyConfig, nil)
if test.hasError {
assert.NotNil(t, err, "Error shouldn't be nil")
} else {
Expand Down Expand Up @@ -636,7 +636,7 @@ func TestCleanOpenRTBRequestsCCPA(t *testing.T) {
nil,
&permissionsMock{allowAllBidders: true, passGeo: true, passID: true},
&metrics.MetricsEngineMock{},
"0",
gdpr.SignalNo,
privacyConfig,
nil)
result := bidderRequests[0]
Expand Down Expand Up @@ -698,7 +698,7 @@ func TestCleanOpenRTBRequestsCCPAErrors(t *testing.T) {
}
permissions := permissionsMock{allowAllBidders: true, passGeo: true, passID: true}
metrics := metrics.MetricsEngineMock{}
_, _, errs := cleanOpenRTBRequests(context.Background(), auctionReq, &reqExtStruct, &permissions, &metrics, "0", privacyConfig, nil)
_, _, errs := cleanOpenRTBRequests(context.Background(), auctionReq, &reqExtStruct, &permissions, &metrics, gdpr.SignalNo, privacyConfig, nil)

assert.ElementsMatch(t, []error{test.expectError}, errs, test.description)
}
Expand Down Expand Up @@ -740,7 +740,7 @@ func TestCleanOpenRTBRequestsCOPPA(t *testing.T) {

permissions := permissionsMock{allowAllBidders: true, passGeo: true, passID: true}
metrics := metrics.MetricsEngineMock{}
bidderRequests, privacyLabels, errs := cleanOpenRTBRequests(context.Background(), auctionReq, nil, &permissions, &metrics, "0", config.Privacy{}, nil)
bidderRequests, privacyLabels, errs := cleanOpenRTBRequests(context.Background(), auctionReq, nil, &permissions, &metrics, gdpr.SignalNo, config.Privacy{}, nil)
result := bidderRequests[0]

assert.Nil(t, errs)
Expand Down Expand Up @@ -849,7 +849,7 @@ func TestCleanOpenRTBRequestsSChain(t *testing.T) {

permissions := permissionsMock{allowAllBidders: true, passGeo: true, passID: true}
metrics := metrics.MetricsEngineMock{}
bidderRequests, _, errs := cleanOpenRTBRequests(context.Background(), auctionReq, extRequest, &permissions, &metrics, "0", config.Privacy{}, nil)
bidderRequests, _, errs := cleanOpenRTBRequests(context.Background(), auctionReq, extRequest, &permissions, &metrics, gdpr.SignalNo, config.Privacy{}, nil)
if test.hasError == true {
assert.NotNil(t, errs)
assert.Len(t, bidderRequests, 0)
Expand Down Expand Up @@ -1432,7 +1432,7 @@ func TestCleanOpenRTBRequestsLMT(t *testing.T) {

permissions := permissionsMock{allowAllBidders: true, passGeo: true, passID: true}
metrics := metrics.MetricsEngineMock{}
results, privacyLabels, errs := cleanOpenRTBRequests(context.Background(), auctionReq, nil, &permissions, &metrics, "0", privacyConfig, nil)
results, privacyLabels, errs := cleanOpenRTBRequests(context.Background(), auctionReq, nil, &permissions, &metrics, gdpr.SignalNo, privacyConfig, nil)
result := results[0]

assert.Nil(t, errs)
Expand Down Expand Up @@ -1639,13 +1639,18 @@ func TestCleanOpenRTBRequestsGDPR(t *testing.T) {
Account: accountConfig,
}

gdprDefaultValue := gdpr.SignalYes
if test.gdprDefaultValue == "0" {
gdprDefaultValue = gdpr.SignalNo
}

results, privacyLabels, errs := cleanOpenRTBRequests(
context.Background(),
auctionReq,
nil,
&permissionsMock{allowAllBidders: true, passGeo: !test.gdprScrub, passID: !test.gdprScrub, activitiesError: test.permissionsError},
&metrics.MetricsEngineMock{},
test.gdprDefaultValue,
gdprDefaultValue,
privacyConfig,
nil)
result := results[0]
Expand Down Expand Up @@ -1736,7 +1741,7 @@ func TestCleanOpenRTBRequestsGDPRBlockBidRequest(t *testing.T) {
nil,
&permissionsMock{allowedBidders: test.gdprAllowedBidders, passGeo: true, passID: true, activitiesError: nil},
&metricsMock,
"0",
gdpr.SignalNo,
privacyConfig,
nil)

Expand Down
10 changes: 8 additions & 2 deletions gdpr/gdpr.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,15 @@ func NewPermissions(ctx context.Context, cfg config.GDPR, vendorIDs map[openrtb_
return &AlwaysAllow{}
}

gdprDefaultValue := SignalYes
if cfg.DefaultValue == "0" {
gdprDefaultValue = SignalNo
}

permissionsImpl := &permissionsImpl{
cfg: cfg,
vendorIDs: vendorIDs,
cfg: cfg,
gdprDefaultValue: gdprDefaultValue,
vendorIDs: vendorIDs,
fetchVendorList: map[uint8]func(ctx context.Context, id uint16) (vendorlist.VendorList, error){
tcf2SpecVersion: newVendorListFetcher(ctx, cfg, client, vendorListURLMaker)},
}
Expand Down
Loading

0 comments on commit 1c83e92

Please sign in to comment.