Prebid incorrectly builds syncOptions, when the Publisher sets different permissions for different bidders

[wojciech-bialy-wpm]

Type of issue

Bug

Description

When Prebid.js constructs syncOptions (via bidderFactory -> registerSyncInner), it does not check per bidder permissions - only checking if image / iframe / all properties exist.

Because of that, if Publisher enables image syncs for all bidders, but restricts iframe sync to one bidder only
filterSettings: { image: { bidders: '*', filter: 'include' }, iframe: { bidders: ['someBidder'], filter: 'include' } }

Then all bidders will receive
syncOptions = {iframeEnabled: true, pixelEnabled: true}
in syncOptions, all will prefer to respond with iframe syncs, and as a result all syncs except for someBidder will be blocked.

Test page and test results

I have prepared tests based on Prebid 9.26.0, with two bidders (Appnexus & sspBC) and different permission combinations.

Test 1: Both bidders only have image sync permissions
Url: https://bdr.wpcdn.pl/tests/usersyncs/sspbc-6.10-image_sync.html
Result: Both return image sync urls, both syncs are fired

Prebid MESSAGE: Invoking image pixel user sync for bidder: appnexus
Prebid MESSAGE: Invoking image pixel user sync for bidder: sspBC

Test 2: Both bidders have image and iframe sync permissions
Url: https://bdr.wpcdn.pl/tests/usersyncs/sspbc-6.10-iframe_sync.html
Result: Both return iframe sync urls, both syncs are fired

Prebid MESSAGE: Invoking iframe user sync for bidder: sspBC
Prebid MESSAGE: Invoking iframe user sync for bidder: appnexus

Test 3: Both bidders have image sync permission, but only sspBC has iframe sync permission
Url: https://bdr.wpcdn.pl/tests/usersyncs/sspbc-6.10-mixed_sync_1.html
Result: Both return iframe sync urls, sspBC sync is fired, Appnexus sync is blocked

Prebid appnexus WARNING: Activity control: userSync config denied 'syncUser' for 'bidder.appnexus': iframe syncs are not enabled for appnexus
Prebid MESSAGE: Invoking iframe user sync for bidder: sspBC

Test 4: Both bidders have image sync permission, but only Appnexus has iframe sync permission
Url: https://bdr.wpcdn.pl/tests/usersyncs/sspbc-6.10-mixed_sync_2.html
Result: Both return iframe sync urls, Appnexus sync is fired, sspBC sync is blocked

Prebid sspBC WARNING: Activity control: userSync config denied 'syncUser' for 'bidder.sspBC': iframe syncs are not enabled for sspBC
Prebid MESSAGE: Invoking iframe user sync for bidder: appnexus

The behavior in Test3 and Test4 is, of course, undesirable. Both bidders have image sync URLs, and could respond with them. They, however, respond with iframe sync since in both cases, regardless of actual permissions, bidders always receive
{iframeEnabled: true, pixelEnabled: true} in syncOptions

And that is because syncOptions do not check for per-bidder rules - only if iframe / image / all fields are present
(see https://github.com/prebid/Prebid.js/blob/master/src/adapters/bidderFactory.js)

let syncs = spec.getUserSyncs({ iframeEnabled: !!(filterConfig && (filterConfig.iframe || filterConfig.all)), pixelEnabled: !!(filterConfig && (filterConfig.image || filterConfig.all)), }, responses, gdprConsent, uspConsent, gppConsent);
(and, for that matter, there is no recognition of include/exclude rules)

Proposed change

Sync options should be constructed with respect to the bidder (spec.code).

For a very simple example, see https://github.com/grupawp/Prebid.js/blob/fix-registerSyncInner/src/adapters/bidderFactory.js
Againm, this at the moment does not discern between include/exclude rules, but I've added method bidderHasPermission that will check if there is a permission for a specific bidder (or for '*' wildcard).

With such modification, we can look at the last test example:

Test 5: Both bidders have image sync permission, but only Appnexus has iframe sync permission. Modified registerSyncInner method
Url: https://bdr.wpcdn.pl/tests/usersyncs/sspbc-6.10-mixed_sync_3.html
Result: Appnexus returns iframe sync, sspBC returns image sync. Both syncs are fired

Prebid MESSAGE: Invoking iframe user sync for bidder: appnexus
Prebid MESSAGE: Invoking image pixel user sync for bidder: sspBC

And this is a correct behavior, that will enable both bidders to sync users, and respect rules set by the publisher.

[wojciech-bialy-wpm]

Update

The best way to do this is, IMO, by using isActivityAllowed to build correct syncOptions for each bidder.

In such case, we would call getUserSyncs in the following manner
let syncs = spec.getUserSyncs({ iframeEnabled: isActivityAllowed(ACTIVITY_SYNC_USER, activityParams(MODULE_TYPE_BIDDER, spec.code, { syncType: 'iframe' })), pixelEnabled: isActivityAllowed(ACTIVITY_SYNC_USER, activityParams(MODULE_TYPE_BIDDER, spec.code, { syncType: 'image' })) }, responses, gdprConsent, uspConsent, gppConsent);

I have updated https://github.com/grupawp/Prebid.js/blob/fix-registerSyncInner/src/adapters/bidderFactory.js and Test5 accordingly.

[patmmccann]

Hi @wojciech-bialy-wpm , thanks so much for the bug report! Would you mind submitting your branch as a pr? Please click allow maintainers to make change on submission to expedite merge

[wojciech-bialy-wpm]

Hi @patmmccann, certainly!

I have created a PR here: #12615

(using a new branch, as the old one that I've used in the last test had some leftover clutter from previous attempt to update sync options)

[wojciech-bialy-wpm]

Hmm... test/spec/unit/core/bidderFactory_spec.js now has 3 failing tests, all pertaining to bidder aliases. I'll look into that.

By itself, user syncs for aliases work. See https://bdr.wpcdn.pl/tests/usersyncs/sspbc-6.10-mixed_sync_4.html, where we have sspBC (as before) and matomy (alias for appnexus). This test will (correctly) execute image sync for sspBC and iframe sync for matomy.

EDIT: Failing tests only check whether filterSettings are being read
expect(getConfigSpy.withArgs('userSync.filterSettings').calledOnce).to.equal(true);
Which we are not doing, since modification uses previously registered Activity data.