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.

Sign up for GitHub

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

Add a PI exemption environment variable to PBS #916

Merged
merged 2 commits into from
May 29, 2019
Merged

Add a PI exemption environment variable to PBS #916

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8b58a82
Refactored to official name of config item
May 21, 2019
7e51b3d
Changes suggested by Mansi Nahar
May 28, 2019
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
16 changes: 13 additions & 3 deletions config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,11 @@ func (cfg *AuctionTimeouts) LimitAuctionTimeout(requested time.Duration) time.Du
}

type GDPR struct {
HostVendorID int `mapstructure:"host_vendor_id"`
UsersyncIfAmbiguous bool `mapstructure:"usersync_if_ambiguous"`
Timeouts GDPRTimeouts `mapstructure:"timeouts_ms"`
HostVendorID int `mapstructure:"host_vendor_id"`
UsersyncIfAmbiguous bool `mapstructure:"usersync_if_ambiguous"`
Timeouts GDPRTimeouts `mapstructure:"timeouts_ms"`
NonStandardPublishers []string `mapstructure:"non_standard_publishers,flow"`
NonStandardPublisherMap map[string]int
}

func (cfg *GDPR) validate(errs configErrors) configErrors {
Expand Down Expand Up @@ -384,6 +386,13 @@ func New(v *viper.Viper) (*Configuration, error) {
if errs := c.validate(); len(errs) > 0 {
return &c, errs
}

// To look for a request's publisher_id into the NonStandardPublishers in
// O(1) time, we fill this hash table located in the NonStandardPublisherMap field of GDPR
c.GDPR.NonStandardPublisherMap = make(map[string]int)
for i := 0; i < len(c.GDPR.NonStandardPublishers); i++ {
c.GDPR.NonStandardPublisherMap[c.GDPR.NonStandardPublishers[i]] = 1
}
return &c, nil
}

Expand Down Expand Up @@ -599,6 +608,7 @@ func SetupViper(v *viper.Viper, filename string) {
v.SetDefault("gdpr.usersync_if_ambiguous", false)
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{""})
v.SetDefault("currency_converter.fetch_url", "https://cdn.jsdelivr.net/gh/prebid/currency-file@1/latest.json")
v.SetDefault("currency_converter.fetch_interval_seconds", 0) // #280 Not activated for the time being
v.SetDefault("default_request.type", "")
Expand Down
17 changes: 17 additions & 0 deletions config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ var fullConfig = []byte(`
gdpr:
host_vendor_id: 15
usersync_if_ambiguous: true
non_standard_publishers: ["siteID","fake-site-id","appID","agltb3B1Yi1pbmNyDAsSA0FwcBiJkfIUDA"]
host_cookie:
cookie_name: userid
family: prebid
Expand Down Expand Up @@ -157,6 +158,22 @@ func TestFullConfig(t *testing.T) {
cmpInts(t, "http_client.idle_connection_timeout_seconds", cfg.Client.IdleConnTimeout, 30)
cmpInts(t, "gdpr.host_vendor_id", cfg.GDPR.HostVendorID, 15)
cmpBools(t, "gdpr.usersync_if_ambiguous", cfg.GDPR.UsersyncIfAmbiguous, true)

//Assert the NonStandardPublishers was correctly unmarshalled
cmpStrings(t, "gdpr.non_standard_publishers", cfg.GDPR.NonStandardPublishers[0], "siteID")
cmpStrings(t, "gdpr.non_standard_publishers", cfg.GDPR.NonStandardPublishers[1], "fake-site-id")
cmpStrings(t, "gdpr.non_standard_publishers", cfg.GDPR.NonStandardPublishers[2], "appID")
cmpStrings(t, "gdpr.non_standard_publishers", cfg.GDPR.NonStandardPublishers[3], "agltb3B1Yi1pbmNyDAsSA0FwcBiJkfIUDA")

//Assert the NonStandardPublisherMap hash table was built correctly
var found bool
for i := 0; i < len(cfg.GDPR.NonStandardPublishers); i++ {
_, found = cfg.GDPR.NonStandardPublisherMap[cfg.GDPR.NonStandardPublishers[i]]
cmpBools(t, "cfg.GDPR.NonStandardPublisherMap", found, true)
}
_, found = cfg.GDPR.NonStandardPublisherMap["appnexus"]
cmpBools(t, "cfg.GDPR.NonStandardPublisherMap", found, false)

cmpStrings(t, "currency_converter.fetch_url", cfg.CurrencyConverter.FetchURL, "https://currency.prebid.org")
cmpInts(t, "currency_converter.fetch_interval_seconds", cfg.CurrencyConverter.FetchIntervalSeconds, 1800)
cmpStrings(t, "recaptcha_secret", cfg.RecaptchaSecret, "asdfasdfasdfasdf")
Expand Down
2 changes: 1 addition & 1 deletion endpoints/auction_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ func (m *auctionMockPermissions) BidderSyncAllowed(ctx context.Context, bidder o
return m.allowBidderSync, nil
}

func (m *auctionMockPermissions) PersonalInfoAllowed(ctx context.Context, bidder openrtb_ext.BidderName, consent string) (bool, error) {
func (m *auctionMockPermissions) PersonalInfoAllowed(ctx context.Context, bidder openrtb_ext.BidderName, PublisherID string, consent string) (bool, error) {
return m.allowPI, nil
}

Expand Down
2 changes: 1 addition & 1 deletion endpoints/cookie_sync_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,6 @@ func (g *gdprPerms) BidderSyncAllowed(ctx context.Context, bidder openrtb_ext.Bi
return ok, nil
}

func (g *gdprPerms) PersonalInfoAllowed(ctx context.Context, bidder openrtb_ext.BidderName, consent string) (bool, error) {
func (g *gdprPerms) PersonalInfoAllowed(ctx context.Context, bidder openrtb_ext.BidderName, PublisherID string, consent string) (bool, error) {
return true, nil
}
2 changes: 1 addition & 1 deletion endpoints/setuid_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,6 @@ func (g *mockPermsSetUID) BidderSyncAllowed(ctx context.Context, bidder openrtb_
return false, nil
}

func (g *mockPermsSetUID) PersonalInfoAllowed(ctx context.Context, bidder openrtb_ext.BidderName, consent string) (bool, error) {
func (g *mockPermsSetUID) PersonalInfoAllowed(ctx context.Context, bidder openrtb_ext.BidderName, PublisherID string, consent string) (bool, error) {
return g.allowPI, nil
}
11 changes: 10 additions & 1 deletion exchange/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,16 @@ func cleanOpenRTBRequests(ctx context.Context,
for bidder, bidReq := range requestsByBidder {
// Fixes #820
coreBidder := resolveBidder(bidder.String(), aliases)
if ok, err := gDPR.PersonalInfoAllowed(ctx, coreBidder, consent); !ok && err == nil {

var publisher_id string
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this var name please be changed to publisherID based on Go's naming convention?

if bidReq.Site != nil && bidReq.Site.Publisher != nil && bidReq.Site.Publisher.ID != "" {
publisher_id = bidReq.Site.Publisher.ID
} else if bidReq.App != nil && bidReq.App.Publisher != nil {
publisher_id = bidReq.App.Publisher.ID
} else {
publisher_id = ""
}
if ok, err := gDPR.PersonalInfoAllowed(ctx, coreBidder, publisher_id, consent); !ok && err == nil {
cleanPI(bidReq, labels.RType == pbsmetrics.ReqTypeAMP)
}
}
Expand Down
2 changes: 1 addition & 1 deletion exchange/utils_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func (p *permissionsMock) BidderSyncAllowed(ctx context.Context, bidder openrtb_
return true, nil
}

func (p *permissionsMock) PersonalInfoAllowed(ctx context.Context, bidder openrtb_ext.BidderName, consent string) (bool, error) {
func (p *permissionsMock) PersonalInfoAllowed(ctx context.Context, bidder openrtb_ext.BidderName, PublisherID string, consent string) (bool, error) {
if bidder == "appnexus" {
return true, nil
}
Expand Down
2 changes: 1 addition & 1 deletion gdpr/gdpr.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ type Permissions interface {
// Determines whether or not to send PI information to a bidder, or mask it out.
//
// If the consent string was nonsenical, the returned error will be an ErrorMalformedConsent.
PersonalInfoAllowed(ctx context.Context, bidder openrtb_ext.BidderName, consent string) (bool, error)
PersonalInfoAllowed(ctx context.Context, bidder openrtb_ext.BidderName, PublisherID string, consent string) (bool, error)
}

// NewPermissions gets an instance of the Permissions for use elsewhere in the project.
Expand Down
9 changes: 7 additions & 2 deletions gdpr/impl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,12 @@ func (p *permissionsImpl) BidderSyncAllowed(ctx context.Context, bidder openrtb_
return false, nil
}

func (p *permissionsImpl) PersonalInfoAllowed(ctx context.Context, bidder openrtb_ext.BidderName, consent string) (bool, error) {
func (p *permissionsImpl) PersonalInfoAllowed(ctx context.Context, bidder openrtb_ext.BidderName, PublisherID string, consent string) (bool, error) {
_, ok := p.cfg.NonStandardPublisherMap[PublisherID]
if ok {
return true, nil
}

id, ok := p.vendorIDs[bidder]
if ok {
return p.allowPI(ctx, id, consent)
Expand Down Expand Up @@ -125,6 +130,6 @@ func (a AlwaysAllow) BidderSyncAllowed(ctx context.Context, bidder openrtb_ext.B
return true, nil
}

func (a AlwaysAllow) PersonalInfoAllowed(ctx context.Context, bidder openrtb_ext.BidderName, consent string) (bool, error) {
func (a AlwaysAllow) PersonalInfoAllowed(ctx context.Context, bidder openrtb_ext.BidderName, PublisherID string, consent string) (bool, error) {
return true, nil
}
14 changes: 12 additions & 2 deletions gdpr/impl_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,11 +175,21 @@ func TestAllowPersonalInfo(t *testing.T) {
}

// PI needs both purposes to succeed
allowPI, err := perms.PersonalInfoAllowed(context.Background(), openrtb_ext.BidderAppnexus, "BOS2bx5OS2bx5ABABBAAABoAAAABBwAA")
allowPI, err := perms.PersonalInfoAllowed(context.Background(), openrtb_ext.BidderAppnexus, "", "BOS2bx5OS2bx5ABABBAAABoAAAABBwAA")
assertNilErr(t, err)
assertBoolsEqual(t, false, allowPI)

allowPI, err = perms.PersonalInfoAllowed(context.Background(), openrtb_ext.BidderPubmatic, "BOS2bx5OS2bx5ABABBAAABoAAAABBwAA")
allowPI, err = perms.PersonalInfoAllowed(context.Background(), openrtb_ext.BidderPubmatic, "", "BOS2bx5OS2bx5ABABBAAABoAAAABBwAA")
assertNilErr(t, err)
assertBoolsEqual(t, true, allowPI)

// Assert that an item that otherwise would not be allowed PI access, gets approved because it is found in the GDPR.NonStandardPublishers array
perms.cfg.NonStandardPublishers = []string{"siteID", "fake-site-id", "appNexusAppID"}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd recommend we add only appNexusAppID to the NonStandardPublisherMap directly since only that value is tested in this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean the perms.cfg.NonStandardPublishers array?

perms.cfg.NonStandardPublisherMap = make(map[string]int)
for _, publisher_id := range perms.cfg.NonStandardPublishers {
perms.cfg.NonStandardPublisherMap[publisher_id] = 1
}
allowPI, err = perms.PersonalInfoAllowed(context.Background(), openrtb_ext.BidderAppnexus, "appNexusAppID", "BOS2bx5OS2bx5ABABBAAABoAAAABBwAA")
assertNilErr(t, err)
assertBoolsEqual(t, true, allowPI)
}
Expand Down