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

Activities: Back To Spec #3081

Merged
merged 4 commits into from
Sep 11, 2023
Merged

Activities: Back To Spec #3081

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a7b0945
Activities: Back To Spec
SyntaxNode Sep 7, 2023
34b5c28
A Few Things
SyntaxNode Sep 7, 2023
170202d
Tpyo
SyntaxNode Sep 8, 2023
e57b20a
Expand Test Coverage
SyntaxNode Sep 8, 2023
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
7 changes: 1 addition & 6 deletions endpoints/cookie_sync.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,12 +149,7 @@ func (c *cookieSyncEndpoint) parseRequest(r *http.Request) (usersync.Request, pr
}
}

activityControl, activitiesErr := privacy.NewActivityControl(&account.Privacy)
if activitiesErr != nil {
if errortypes.ContainsFatalError([]error{activitiesErr}) {
activityControl = privacy.ActivityControl{}
}
}
activityControl := privacy.NewActivityControl(&account.Privacy)

syncTypeFilter, err := parseTypeFilter(request.FilterSettings)
if err != nil {
Expand Down
35 changes: 1 addition & 34 deletions endpoints/cookie_sync_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -974,38 +974,6 @@ func TestCookieSyncParseRequest(t *testing.T) {
expectedError: errCookieSyncAccountBlocked.Error(),
givenAccountRequired: true,
},

{
description: "Account Defaults - Invalid Activities",
givenBody: strings.NewReader(`{` +
`"bidders":["a", "b"],` +
`"account":"ValidAccountInvalidActivities"` +
`}`),
givenGDPRConfig: config.GDPR{Enabled: true, DefaultValue: "0"},
givenCCPAEnabled: true,
givenConfig: config.UserSync{
Cooperative: config.UserSyncCooperative{
EnabledByDefault: false,
PriorityGroups: [][]string{{"a", "b", "c"}},
},
},
expectedPrivacy: privacy.Policies{},
expectedRequest: usersync.Request{
Bidders: []string{"a", "b"},
Cooperative: usersync.Cooperative{
Enabled: false,
PriorityGroups: [][]string{{"a", "b", "c"}},
},
Limit: 0,
Privacy: usersyncPrivacy{
gdprPermissions: &fakePermissions{},
},
SyncTypeFilter: usersync.SyncTypeFilter{
IFrame: usersync.NewUniformBidderFilter(usersync.BidderFilterModeInclude),
Redirect: usersync.NewUniformBidderFilter(usersync.BidderFilterModeInclude),
},
},
},
}

for _, test := range testCases {
Expand Down Expand Up @@ -1934,8 +1902,7 @@ func TestCookieSyncActivityControlIntegration(t *testing.T) {

for _, test := range testCases {
t.Run(test.name, func(t *testing.T) {
activities, err := privacy.NewActivityControl(test.accountPrivacy)
assert.NoError(t, err)
activities := privacy.NewActivityControl(test.accountPrivacy)
up := usersyncPrivacy{
activityControl: activities,
}
Expand Down
12 changes: 4 additions & 8 deletions endpoints/openrtb2/amp_auction.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@ import (
"encoding/json"
"errors"
"fmt"
"github.com/prebid/prebid-server/privacy"
"net/http"
"net/url"
"strings"
"time"

"github.com/prebid/prebid-server/privacy"

"github.com/buger/jsonparser"
"github.com/golang/glog"
"github.com/julienschmidt/httprouter"
Expand Down Expand Up @@ -229,12 +230,7 @@ func (deps *endpointDeps) AmpAuction(w http.ResponseWriter, r *http.Request, _ h

tcf2Config := gdpr.NewTCF2Config(deps.cfg.GDPR.TCF2, account.GDPR)

activities, activitiesErr := privacy.NewActivityControl(&account.Privacy)
if activitiesErr != nil {
errL = append(errL, activitiesErr)
writeError(errL, w, &labels)
return
}
activityControl := privacy.NewActivityControl(&account.Privacy)

secGPC := r.Header.Get("Sec-GPC")

Expand All @@ -253,7 +249,7 @@ func (deps *endpointDeps) AmpAuction(w http.ResponseWriter, r *http.Request, _ h
HookExecutor: hookExecutor,
QueryParams: r.URL.Query(),
TCF2Config: tcf2Config,
Activities: activities,
Activities: activityControl,
TmaxAdjustments: deps.tmaxAdjustments,
}

Expand Down
11 changes: 2 additions & 9 deletions endpoints/openrtb2/auction.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,14 +194,7 @@ func (deps *endpointDeps) Auction(w http.ResponseWriter, r *http.Request, _ http

tcf2Config := gdpr.NewTCF2Config(deps.cfg.GDPR.TCF2, account.GDPR)

activities, activitiesErr := privacy.NewActivityControl(&account.Privacy)
if activitiesErr != nil {
errL = append(errL, activitiesErr)
if errortypes.ContainsFatalError(errL) {
writeError(errL, w, &labels)
return
}
}
activityControl := privacy.NewActivityControl(&account.Privacy)

ctx := context.Background()

Expand Down Expand Up @@ -252,7 +245,7 @@ func (deps *endpointDeps) Auction(w http.ResponseWriter, r *http.Request, _ http
PubID: labels.PubID,
HookExecutor: hookExecutor,
TCF2Config: tcf2Config,
Activities: activities,
Activities: activityControl,
TmaxAdjustments: deps.tmaxAdjustments,
}
auctionResponse, err := deps.ex.HoldAuction(ctx, auctionRequest, nil)
Expand Down
11 changes: 2 additions & 9 deletions endpoints/openrtb2/video_auction.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,14 +303,7 @@ func (deps *endpointDeps) VideoAuctionEndpoint(w http.ResponseWriter, r *http.Re
return
}

activities, activitiesErr := privacy.NewActivityControl(&account.Privacy)
if activitiesErr != nil {
errL = append(errL, activitiesErr)
if errortypes.ContainsFatalError(errL) {
handleError(&labels, w, errL, &vo, &debugLog)
return
}
}
activityControl := privacy.NewActivityControl(&account.Privacy)

secGPC := r.Header.Get("Sec-GPC")
auctionRequest := &exchange.AuctionRequest{
Expand All @@ -324,7 +317,7 @@ func (deps *endpointDeps) VideoAuctionEndpoint(w http.ResponseWriter, r *http.Re
PubID: labels.PubID,
HookExecutor: hookexecution.EmptyHookExecutor{},
TmaxAdjustments: deps.tmaxAdjustments,
Activities: activities,
Activities: activityControl,
}

auctionResponse, err := deps.ex.HoldAuction(ctx, auctionRequest, &debugLog)
Expand Down
14 changes: 0 additions & 14 deletions endpoints/openrtb2/video_auction_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,20 +65,6 @@ func TestVideoEndpointImpressionsNumber(t *testing.T) {
assert.Equal(t, resp.AdPods[0].Targeting[0].HbDeal, "ABC_123", "If DealID exists in bid response, hb_deal targeting needs to be added to resp")
}

func TestVideoEndpointInvalidPrivacyConfig(t *testing.T) {
ex := &mockExchangeVideo{}
reqBody := readVideoTestFile(t, "sample-requests/video/video_valid_sample.json")
req := httptest.NewRequest("POST", "/openrtb2/video", strings.NewReader(reqBody))
recorder := httptest.NewRecorder()

deps := mockDepsInvalidPrivacy(t, ex)
deps.VideoAuctionEndpoint(recorder, req, nil)

respBytes := recorder.Body.Bytes()
expectedErrorMessage := "Critical error while running the video endpoint: unable to parse component: bidderA.BidderB.bidderC"
assert.Equal(t, expectedErrorMessage, string(respBytes), "error message is incorrect")
}

func TestVideoEndpointImpressionsDuration(t *testing.T) {
ex := &mockExchangeVideo{}
reqBody := readVideoTestFile(t, "sample-requests/video/video_valid_sample_different_durations.json")
Expand Down
9 changes: 2 additions & 7 deletions endpoints/setuid.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,9 @@ func NewSetUIDEndpoint(cfg *config.Configuration, syncersByBidder map[string]use
return
}

activities, activitiesErr := privacy.NewActivityControl(&account.Privacy)
if activitiesErr != nil {
if errortypes.ContainsFatalError([]error{activitiesErr}) {
activities = privacy.ActivityControl{}
}
}
activityControl := privacy.NewActivityControl(&account.Privacy)

userSyncActivityAllowed := activities.Allow(privacy.ActivitySyncUser,
userSyncActivityAllowed := activityControl.Allow(privacy.ActivitySyncUser,
privacy.Component{Type: privacy.ComponentTypeBidder, Name: bidderName})
if !userSyncActivityAllowed {
w.WriteHeader(http.StatusUnavailableForLegalReasons)
Expand Down
5 changes: 1 addition & 4 deletions exchange/exchange_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2352,10 +2352,7 @@ func runSpec(t *testing.T, filename string, spec *exchangeSpec) {
impExtInfoMap[impID] = ImpExtInfo{}
}

activityControl, err := privacy.NewActivityControl(spec.AccountPrivacy)
if err != nil {
t.Errorf("%s: Exchange returned an unexpected error. Got %s", filename, err.Error())
}
activityControl := privacy.NewActivityControl(spec.AccountPrivacy)

auctionRequest := &AuctionRequest{
BidRequestWrapper: &openrtb_ext.RequestWrapper{BidRequest: &spec.IncomingRequest.OrtbRequest},
Expand Down
3 changes: 1 addition & 2 deletions exchange/utils_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4364,8 +4364,7 @@ func TestCleanOpenRTBRequestsActivities(t *testing.T) {
}

for _, test := range testCases {
activities, err := privacy.NewActivityControl(&test.privacyConfig)
assert.NoError(t, err, "")
activities := privacy.NewActivityControl(&test.privacyConfig)
auctionReq := AuctionRequest{
BidRequestWrapper: &openrtb_ext.RequestWrapper{BidRequest: test.req},
UserSyncs: &emptyUsersync{},
Expand Down
92 changes: 22 additions & 70 deletions privacy/activitycontrol.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,66 +18,35 @@ type ActivityControl struct {
plans map[Activity]ActivityPlan
}

func NewActivityControl(privacyConf *config.AccountPrivacy) (ActivityControl, error) {
func NewActivityControl(cfg *config.AccountPrivacy) ActivityControl {
ac := ActivityControl{}
var err error

if privacyConf == nil || privacyConf.AllowActivities == nil {
return ac, nil
}

plans := make(map[Activity]ActivityPlan)

plans[ActivitySyncUser], err = buildEnforcementPlan(privacyConf.AllowActivities.SyncUser)
if err != nil {
return ac, err
}
plans[ActivityFetchBids], err = buildEnforcementPlan(privacyConf.AllowActivities.FetchBids)
if err != nil {
return ac, err
}
plans[ActivityEnrichUserFPD], err = buildEnforcementPlan(privacyConf.AllowActivities.EnrichUserFPD)
if err != nil {
return ac, err
}
plans[ActivityReportAnalytics], err = buildEnforcementPlan(privacyConf.AllowActivities.ReportAnalytics)
if err != nil {
return ac, err
}
plans[ActivityTransmitUserFPD], err = buildEnforcementPlan(privacyConf.AllowActivities.TransmitUserFPD)
if err != nil {
return ac, err
}
plans[ActivityTransmitPreciseGeo], err = buildEnforcementPlan(privacyConf.AllowActivities.TransmitPreciseGeo)
if err != nil {
return ac, err
}
plans[ActivityTransmitUniqueRequestIds], err = buildEnforcementPlan(privacyConf.AllowActivities.TransmitUniqueRequestIds)
if err != nil {
return ac, err
}
plans[ActivityTransmitTids], err = buildEnforcementPlan(privacyConf.AllowActivities.TransmitTids)
if err != nil {
return ac, err
if cfg == nil || cfg.AllowActivities == nil {
return ac
}

plans := make(map[Activity]ActivityPlan, 8)
plans[ActivitySyncUser] = buildPlan(cfg.AllowActivities.SyncUser)
plans[ActivityFetchBids] = buildPlan(cfg.AllowActivities.FetchBids)
plans[ActivityEnrichUserFPD] = buildPlan(cfg.AllowActivities.EnrichUserFPD)
plans[ActivityReportAnalytics] = buildPlan(cfg.AllowActivities.ReportAnalytics)
plans[ActivityTransmitUserFPD] = buildPlan(cfg.AllowActivities.TransmitUserFPD)
plans[ActivityTransmitPreciseGeo] = buildPlan(cfg.AllowActivities.TransmitPreciseGeo)
plans[ActivityTransmitUniqueRequestIds] = buildPlan(cfg.AllowActivities.TransmitUniqueRequestIds)
plans[ActivityTransmitTids] = buildPlan(cfg.AllowActivities.TransmitTids)
ac.plans = plans

return ac, nil
return ac
}

func buildEnforcementPlan(activity config.Activity) (ActivityPlan, error) {
ef := ActivityPlan{}
rules, err := activityRulesToEnforcementRules(activity.Rules)
if err != nil {
return ef, err
func buildPlan(activity config.Activity) ActivityPlan {
return ActivityPlan{
rules: cfgToRules(activity.Rules),
defaultResult: cfgToDefaultResult(activity.Default),
}
ef.defaultResult = activityDefaultToDefaultResult(activity.Default)
ef.rules = rules
return ef, nil
}

func activityRulesToEnforcementRules(rules []config.ActivityRule) ([]Rule, error) {
func cfgToRules(rules []config.ActivityRule) []Rule {
var enfRules []Rule

for _, r := range rules {
Expand All @@ -88,34 +57,17 @@ func activityRulesToEnforcementRules(rules []config.ActivityRule) ([]Rule, error
result = ActivityDeny
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed that this line doesn't have any code coverage. It may be out of the scope of this PR's goals, so I'm fine leaving it as is, but wanted to point it out.

Copy link
Contributor

@VeronikaSolovei9 VeronikaSolovei9 Sep 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, we only test it with allow=true.

Here is a quick fix.
These functions should take a parameter that will control the result:

func getTestActivityConfig(allow bool) config.Activity {
	return config.Activity{
		Default: ptrutil.ToPtr(true),
		Rules: []config.ActivityRule{
			{
				Allow: allow,
				Condition: config.ActivityCondition{
					ComponentName: []string{"bidderA"},
					ComponentType: []string{"bidder"},
				},
			},
		},
	}
}

func getTestActivityPlan(activityRes ActivityResult) ActivityPlan {
	return ActivityPlan{
		defaultResult: true,
		rules: []Rule{
			ConditionRule{
				result:        activityRes,
				componentName: []string{"bidderA"},
				componentType: []string{"bidder"},
			},
		},
	}
}

Put "true" and "ActivityAllow" to everywhere where these parameters are needed.
And then modify existing test to receive false at least for one activity:

{
			name: "specified_and_correct",
			privacyConf: config.AccountPrivacy{
				AllowActivities: &config.AllowActivities{
					SyncUser:                 getTestActivityConfig(true),
					FetchBids:                getTestActivityConfig(true),
					EnrichUserFPD:            getTestActivityConfig(true),
					ReportAnalytics:          getTestActivityConfig(true),
					TransmitUserFPD:          getTestActivityConfig(true),
					TransmitPreciseGeo:       getTestActivityConfig(false),
					TransmitUniqueRequestIds: getTestActivityConfig(true),
					TransmitTids:             getTestActivityConfig(true),
				},
			},
			activityControl: ActivityControl{plans: map[Activity]ActivityPlan{
				ActivitySyncUser:                 getTestActivityPlan(ActivityAllow),
				ActivityFetchBids:                getTestActivityPlan(ActivityAllow),
				ActivityEnrichUserFPD:            getTestActivityPlan(ActivityAllow),
				ActivityReportAnalytics:          getTestActivityPlan(ActivityAllow),
				ActivityTransmitUserFPD:          getTestActivityPlan(ActivityAllow),
				ActivityTransmitPreciseGeo:       getTestActivityPlan(ActivityDeny),
				ActivityTransmitUniqueRequestIds: getTestActivityPlan(ActivityAllow),
				ActivityTransmitTids:             getTestActivityPlan(ActivityAllow),
			}},
		},

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch @AlexBVolcy. Thanks for the implementation plan @VeronikaSolovei9. I'll add this to the PR tomorrow.

}

componentName, err := conditionToRuleComponentNames(r.Condition.ComponentName)
if err != nil {
return nil, err
}

er := ComponentEnforcementRule{
er := ConditionRule{
result: result,
componentName: componentName,
componentName: r.Condition.ComponentName,
componentType: r.Condition.ComponentType,
}
enfRules = append(enfRules, er)
}
return enfRules, nil
}

func conditionToRuleComponentNames(conditions []string) ([]Component, error) {
sn := make([]Component, 0)
for _, condition := range conditions {
scope, err := ParseComponent(condition)
if err != nil {
return nil, err
}
sn = append(sn, scope)
}
return sn, nil
return enfRules
}

func activityDefaultToDefaultResult(activityDefault *bool) bool {
func cfgToDefaultResult(activityDefault *bool) bool {
if activityDefault == nil {
return defaultActivityResult
}
Expand Down
Loading
Loading