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

Jump to bottom

Remove legacy GDPR AMP config flag used to prevent buyer ID scrub on AMP requests #1565

Merged
merged 7 commits into from
Jan 5, 2021
Merged

Remove legacy GDPR AMP config flag used to prevent buyer ID scrub on AMP requests #1565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f9e7b2a
Remove legacy GDPR AMP config flag used to prevent buyer ID scrub on …
bsardo Nov 5, 2020
722dcb2
Add warning if amp_exception config flag is set
bsardo Dec 2, 2020
93d2823
Merge removal of GDPR AMP Exception with master
bsardo Dec 17, 2020
c0f7829
Change amp_exception config flag set warning to fatal error
bsardo Dec 17, 2020
79a202c
Merge with master
bsardo Dec 18, 2020
50700f0
Update gdpr.amp_exception error message
bsardo Dec 18, 2020
dc57a78
Remove mock AMPException methods and unnecessary test struct field
bsardo Dec 22, 2020
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
2 changes: 1 addition & 1 deletion config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ func (cfg *GDPR) validate(errs []error) []error {
glog.Warning("gdpr.host_vendor_id was not specified. Host company GDPR checks will be skipped.")
}
if cfg.AMPException == true {
glog.Warning("gdpr.amp_exception is deprecated and will be removed in a future version. 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).")
errs = append(errs, fmt.Errorf("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)"))
}
return errs
}
Expand Down
4 changes: 0 additions & 4 deletions endpoints/auction_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,10 +424,6 @@ func (m *auctionMockPermissions) PersonalInfoAllowed(ctx context.Context, bidder
return m.allowPI, m.allowGeo, m.allowID, nil
}

func (m *auctionMockPermissions) AMPException() bool {
return false
}

func TestBidSizeValidate(t *testing.T) {
bids := make(pbs.PBSBidSlice, 0)
// bid1 will be rejected due to undefined size when adunit has multiple sizes
Expand Down
4 changes: 0 additions & 4 deletions endpoints/cookie_sync_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,3 @@ func (g *gdprPerms) BidderSyncAllowed(ctx context.Context, bidder openrtb_ext.Bi
func (g *gdprPerms) PersonalInfoAllowed(ctx context.Context, bidder openrtb_ext.BidderName, PublisherID string, consent string) (bool, bool, bool, error) {
return true, true, true, nil
}

func (g *gdprPerms) AMPException() bool {
return false
}
4 changes: 0 additions & 4 deletions endpoints/setuid_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,10 +441,6 @@ func (g *mockPermsSetUID) PersonalInfoAllowed(ctx context.Context, bidder openrt
return g.allowPI, g.allowPI, g.allowPI, nil
}

func (g *mockPermsSetUID) AMPException() bool {
return false
}

func newFakeSyncer(familyName string) usersync.Usersyncer {
return fakeSyncer{
familyName: familyName,
Expand Down
3 changes: 1 addition & 2 deletions exchange/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ func cleanOpenRTBRequests(ctx context.Context,

gdpr := extractGDPR(req.BidRequest, usersyncIfAmbiguous)
consent := extractConsent(req.BidRequest)
ampGDPRException := (req.LegacyLabels.RType == metrics.ReqTypeAMP) && gDPR.AMPException()

ccpaEnforcer, err := extractCCPA(req.BidRequest, privacyConfig, &req.Account, aliases, integrationTypeMap[req.LegacyLabels.RType])
if err != nil {
Expand Down Expand Up @@ -125,7 +124,7 @@ func cleanOpenRTBRequests(ctx context.Context,
privacyEnforcement.GDPRID = false
}

privacyEnforcement.Apply(bidderRequest.BidRequest, ampGDPRException)
privacyEnforcement.Apply(bidderRequest.BidRequest)
}

return
Expand Down
4 changes: 0 additions & 4 deletions exchange/utils_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,6 @@ func (p *permissionsMock) PersonalInfoAllowed(ctx context.Context, bidder openrt
return p.personalInfoAllowed, p.personalInfoAllowed, p.personalInfoAllowed, nil
}

func (p *permissionsMock) AMPException() bool {
return false
}

func assertReq(t *testing.T, bidderRequests []BidderRequest,
applyCOPPA bool, consentedVendors map[string]bool) {
// assert individual bidder requests
Expand Down
3 changes: 0 additions & 3 deletions gdpr/gdpr.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ type Permissions interface {
//
// If the consent string was nonsensical, the returned error will be an ErrorMalformedConsent.
PersonalInfoAllowed(ctx context.Context, bidder openrtb_ext.BidderName, PublisherID string, consent string) (bool, bool, bool, error)

// Exposes the AMP execption flag
AMPException() bool
}

// Versions of the GDPR TCF technical specification.
Expand Down
12 changes: 0 additions & 12 deletions gdpr/impl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,6 @@ func (p *permissionsImpl) PersonalInfoAllowed(ctx context.Context, bidder openrt
return false, false, false, nil
}

func (p *permissionsImpl) AMPException() bool {
return p.cfg.AMPException
}

func (p *permissionsImpl) allowSync(ctx context.Context, vendorID uint16, consent string) (bool, error) {
// If we're not given a consent string, respect the preferences in the app config.
if consent == "" {
Expand Down Expand Up @@ -225,10 +221,6 @@ func (a AlwaysAllow) PersonalInfoAllowed(ctx context.Context, bidder openrtb_ext
return true, true, true, nil
}

func (a AlwaysAllow) AMPException() bool {
return false
}

// Exporting to allow for easy test setups
type AlwaysFail struct{}

Expand All @@ -243,7 +235,3 @@ func (a AlwaysFail) BidderSyncAllowed(ctx context.Context, bidder openrtb_ext.Bi
func (a AlwaysFail) PersonalInfoAllowed(ctx context.Context, bidder openrtb_ext.BidderName, PublisherID string, consent string) (bool, bool, bool, error) {
return false, false, false, nil
}

func (a AlwaysFail) AMPException() bool {
return false
}
12 changes: 6 additions & 6 deletions privacy/enforcement.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ func (e Enforcement) Any() bool {
}

// Apply cleans personally identifiable information from an OpenRTB bid request.
func (e Enforcement) Apply(bidRequest *openrtb.BidRequest, ampGDPRException bool) {
e.apply(bidRequest, ampGDPRException, NewScrubber())
func (e Enforcement) Apply(bidRequest *openrtb.BidRequest) {
e.apply(bidRequest, NewScrubber())
}

func (e Enforcement) apply(bidRequest *openrtb.BidRequest, ampGDPRException bool, scrubber Scrubber) {
func (e Enforcement) apply(bidRequest *openrtb.BidRequest, scrubber Scrubber) {
if bidRequest != nil && e.Any() {
bidRequest.Device = scrubber.ScrubDevice(bidRequest.Device, e.getDeviceIDScrubStrategy(), e.getIPv4ScrubStrategy(), e.getIPv6ScrubStrategy(), e.getGeoScrubStrategy())
bidRequest.User = scrubber.ScrubUser(bidRequest.User, e.getUserScrubStrategy(ampGDPRException), e.getGeoScrubStrategy())
bidRequest.User = scrubber.ScrubUser(bidRequest.User, e.getUserScrubStrategy(), e.getGeoScrubStrategy())
}
}

Expand Down Expand Up @@ -70,7 +70,7 @@ func (e Enforcement) getGeoScrubStrategy() ScrubStrategyGeo {
return ScrubStrategyGeoNone
}

func (e Enforcement) getUserScrubStrategy(ampGDPRException bool) ScrubStrategyUser {
func (e Enforcement) getUserScrubStrategy() ScrubStrategyUser {
if e.COPPA {
return ScrubStrategyUserIDAndDemographic
}
Expand All @@ -79,7 +79,7 @@ func (e Enforcement) getUserScrubStrategy(ampGDPRException bool) ScrubStrategyUs
return ScrubStrategyUserID
}

if e.GDPRID && !ampGDPRException {
if e.GDPRID {
return ScrubStrategyUserID
}

Expand Down
64 changes: 4 additions & 60 deletions privacy/enforcement_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ func TestApply(t *testing.T) {
testCases := []struct {
description string
enforcement Enforcement
ampGDPRException bool
expectedDeviceID ScrubStrategyDeviceID
expectedDeviceIPv4 ScrubStrategyIPV4
expectedDeviceIPv6 ScrubStrategyIPV6
Expand Down Expand Up @@ -124,31 +123,13 @@ func TestApply(t *testing.T) {
GDPRID: true,
LMT: false,
},
ampGDPRException: false,
expectedDeviceID: ScrubStrategyDeviceIDAll,
expectedDeviceIPv4: ScrubStrategyIPV4Lowest8,
expectedDeviceIPv6: ScrubStrategyIPV6Lowest16,
expectedDeviceGeo: ScrubStrategyGeoReducedPrecision,
expectedUser: ScrubStrategyUserID,
expectedUserGeo: ScrubStrategyGeoReducedPrecision,
},
{
description: "GDPR Only - Full - AMP Exception",
enforcement: Enforcement{
CCPA: false,
COPPA: false,
GDPRGeo: true,
GDPRID: true,
LMT: false,
},
ampGDPRException: true,
expectedDeviceID: ScrubStrategyDeviceIDAll,
expectedDeviceIPv4: ScrubStrategyIPV4Lowest8,
expectedDeviceIPv6: ScrubStrategyIPV6Lowest16,
expectedDeviceGeo: ScrubStrategyGeoReducedPrecision,
expectedUser: ScrubStrategyUserNone,
expectedUserGeo: ScrubStrategyGeoReducedPrecision,
},
{
description: "GDPR Only - ID Only",
enforcement: Enforcement{
Expand All @@ -158,31 +139,13 @@ func TestApply(t *testing.T) {
GDPRID: true,
LMT: false,
},
ampGDPRException: false,
expectedDeviceID: ScrubStrategyDeviceIDAll,
expectedDeviceIPv4: ScrubStrategyIPV4None,
expectedDeviceIPv6: ScrubStrategyIPV6None,
expectedDeviceGeo: ScrubStrategyGeoNone,
expectedUser: ScrubStrategyUserID,
expectedUserGeo: ScrubStrategyGeoNone,
},
{
description: "GDPR Only - ID Only - AMP Exception",
enforcement: Enforcement{
CCPA: false,
COPPA: false,
GDPRGeo: false,
GDPRID: true,
LMT: false,
},
ampGDPRException: true,
expectedDeviceID: ScrubStrategyDeviceIDAll,
expectedDeviceIPv4: ScrubStrategyIPV4None,
expectedDeviceIPv6: ScrubStrategyIPV6None,
expectedDeviceGeo: ScrubStrategyGeoNone,
expectedUser: ScrubStrategyUserNone,
expectedUserGeo: ScrubStrategyGeoNone,
},
{
description: "GDPR Only - Geo Only",
enforcement: Enforcement{
Expand All @@ -192,7 +155,6 @@ func TestApply(t *testing.T) {
GDPRID: false,
LMT: false,
},
ampGDPRException: false,
expectedDeviceID: ScrubStrategyDeviceIDNone,
expectedDeviceIPv4: ScrubStrategyIPV4Lowest8,
expectedDeviceIPv6: ScrubStrategyIPV6Lowest16,
Expand All @@ -217,32 +179,14 @@ func TestApply(t *testing.T) {
expectedUserGeo: ScrubStrategyGeoReducedPrecision,
},
{
description: "Interactions: COPPA Only + AMP Exception",
enforcement: Enforcement{
CCPA: false,
COPPA: true,
GDPRGeo: false,
GDPRID: false,
LMT: false,
},
ampGDPRException: true,
expectedDeviceID: ScrubStrategyDeviceIDAll,
expectedDeviceIPv4: ScrubStrategyIPV4Lowest8,
expectedDeviceIPv6: ScrubStrategyIPV6Lowest32,
expectedDeviceGeo: ScrubStrategyGeoFull,
expectedUser: ScrubStrategyUserIDAndDemographic,
expectedUserGeo: ScrubStrategyGeoFull,
},
{
description: "Interactions: COPPA + GDPR Full + AMP Exception",
description: "Interactions: COPPA + GDPR Full",
enforcement: Enforcement{
CCPA: false,
COPPA: true,
GDPRGeo: true,
GDPRID: true,
LMT: false,
},
ampGDPRException: true,
expectedDeviceID: ScrubStrategyDeviceIDAll,
expectedDeviceIPv4: ScrubStrategyIPV4Lowest8,
expectedDeviceIPv6: ScrubStrategyIPV6Lowest32,
Expand All @@ -264,7 +208,7 @@ func TestApply(t *testing.T) {
m.On("ScrubDevice", req.Device, test.expectedDeviceID, test.expectedDeviceIPv4, test.expectedDeviceIPv6, test.expectedDeviceGeo).Return(replacedDevice).Once()
m.On("ScrubUser", req.User, test.expectedUser, test.expectedUserGeo).Return(replacedUser).Once()

test.enforcement.apply(req, test.ampGDPRException, m)
test.enforcement.apply(req, m)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're not using ampGDPRException any longer, any reason to still keep it as part of the testCase struct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nope, I'll remove. Good catch.


m.AssertExpectations(t)
assert.Same(t, replacedDevice, req.Device, "Device")
Expand All @@ -284,7 +228,7 @@ func TestApplyNoneApplicable(t *testing.T) {
GDPRID: false,
LMT: false,
}
enforcement.apply(req, false, m)
enforcement.apply(req, m)

m.AssertNotCalled(t, "ScrubDevice")
m.AssertNotCalled(t, "ScrubUser")
Expand All @@ -294,7 +238,7 @@ func TestApplyNil(t *testing.T) {
m := &mockScrubber{}

enforcement := Enforcement{}
enforcement.apply(nil, false, m)
enforcement.apply(nil, m)

m.AssertNotCalled(t, "ScrubDevice")
m.AssertNotCalled(t, "ScrubUser")
Expand Down