From bfb6cf2c9a6d55e4730c1c408290ab4b2548d65c Mon Sep 17 00:00:00 2001 From: Demetrio Girardi Date: Thu, 1 Jun 2023 10:16:56 -0700 Subject: [PATCH 1/2] Core: introduce new `transmitTid` activity control and turn off TIDs by default --- libraries/objectGuard/objectGuard.js | 13 +--- src/activities/activities.js | 5 ++ src/activities/redactor.js | 44 +++++++++-- src/adapters/bidderFactory.js | 43 ++++++++++- test/spec/activities/redactor_spec.js | 12 ++- test/spec/unit/core/adapterManager_spec.js | 20 ++++- test/spec/unit/core/bidderFactory_spec.js | 85 ++++++++++++++++++++-- 7 files changed, 188 insertions(+), 34 deletions(-) diff --git a/libraries/objectGuard/objectGuard.js b/libraries/objectGuard/objectGuard.js index a404f8653f8..cf3d2f38256 100644 --- a/libraries/objectGuard/objectGuard.js +++ b/libraries/objectGuard/objectGuard.js @@ -1,4 +1,4 @@ -import {isData, objectTransformer} from '../../src/activities/redactor.js'; +import {isData, objectTransformer, sessionedApplies} from '../../src/activities/redactor.js'; import {deepAccess, deepClone, deepEqual, deepSetValue} from '../../src/utils.js'; /** @@ -41,15 +41,6 @@ export function objectGuard(rules) { const wpTransformer = objectTransformer(writeRules); - function mkApplies(session, args) { - return function applies(rule) { - if (!session.hasOwnProperty(rule.name)) { - session[rule.name] = rule.applies(...args); - } - return session[rule.name]; - } - } - function mkGuard(obj, tree, applies) { return new Proxy(obj, { get(target, prop, receiver) { @@ -76,7 +67,7 @@ export function objectGuard(rules) { return function guard(obj, ...args) { const session = {}; return { - obj: mkGuard(obj, root.children || {}, mkApplies(session, args)), + obj: mkGuard(obj, root.children || {}, sessionedApplies(session, ...args)), verify: mkVerify(wpTransformer(session, obj, ...args)) } }; diff --git a/src/activities/activities.js b/src/activities/activities.js index fdb64587bfa..0a17750b0b0 100644 --- a/src/activities/activities.js +++ b/src/activities/activities.js @@ -45,3 +45,8 @@ export const ACTIVITY_TRANSMIT_UFPD = 'transmitUfpd'; * transmitPreciseGeo: some component wants access to (and send along) geolocation info */ export const ACTIVITY_TRANSMIT_PRECISE_GEO = 'transmitPreciseGeo'; + +/** + * transmit TID: some component wants access ot (and send along) transaction IDs + */ +export const ACTIVITY_TRANSMIT_TID = 'transmitTid'; diff --git a/src/activities/redactor.js b/src/activities/redactor.js index 3c80019c750..d50df72648c 100644 --- a/src/activities/redactor.js +++ b/src/activities/redactor.js @@ -1,6 +1,12 @@ import {deepAccess} from '../utils.js'; -import {isActivityAllowed} from './rules.js'; -import {ACTIVITY_TRANSMIT_EIDS, ACTIVITY_TRANSMIT_PRECISE_GEO, ACTIVITY_TRANSMIT_UFPD} from './activities.js'; +import {config} from '../config.js'; +import {isActivityAllowed, registerActivityControl} from './rules.js'; +import { + ACTIVITY_TRANSMIT_EIDS, + ACTIVITY_TRANSMIT_PRECISE_GEO, + ACTIVITY_TRANSMIT_TID, + ACTIVITY_TRANSMIT_UFPD +} from './activities.js'; export const ORTB_UFPD_PATHS = ['user.data', 'user.ext.data']; export const ORTB_EIDS_PATHS = ['user.eids', 'user.ext.eids']; @@ -74,16 +80,12 @@ export function objectTransformer(rules) { }) return function applyTransform(session, obj, ...args) { const result = []; + const applies = sessionedApplies(session, ...args); rules.forEach(rule => { if (session[rule.name] === false) return; for (const [head, tail] of rule.paths) { const parent = head == null ? obj : deepAccess(obj, head); - result.push(rule.run(obj, head, parent, tail, () => { - if (!session.hasOwnProperty(rule.name)) { - session[rule.name] = !!rule.applies(...args); - } - return session[rule.name] - })) + result.push(rule.run(obj, head, parent, tail, applies.bind(null, rule))); if (session[rule.name] === false) return; } }) @@ -91,6 +93,15 @@ export function objectTransformer(rules) { } } +export function sessionedApplies(session, ...args) { + return function applies(rule) { + if (!session.hasOwnProperty(rule.name)) { + session[rule.name] = !!rule.applies(...args); + } + return session[rule.name]; + } +} + export function isData(val) { return val != null && (typeof val !== 'object' || Object.keys(val).length > 0) } @@ -107,6 +118,11 @@ function bidRequestTransmitRules(isAllowed = isActivityAllowed) { name: ACTIVITY_TRANSMIT_EIDS, paths: ['userId', 'userIdAsEids'], applies: appliesWhenActivityDenied(ACTIVITY_TRANSMIT_EIDS, isAllowed), + }, + { + name: ACTIVITY_TRANSMIT_TID, + paths: ['ortb2Imp.ext.tid'], + applies: appliesWhenActivityDenied(ACTIVITY_TRANSMIT_TID, isAllowed) } ].map(redactRule) } @@ -130,6 +146,11 @@ export function ortb2TransmitRules(isAllowed = isActivityAllowed) { get(val) { return Math.round((val + Number.EPSILON) * 100) / 100; } + }, + { + name: ACTIVITY_TRANSMIT_TID, + paths: ['source.tid'], + applies: appliesWhenActivityDenied(ACTIVITY_TRANSMIT_TID, isAllowed), } ].map(redactRule); } @@ -155,3 +176,10 @@ export function redactorFactory(isAllowed = isActivityAllowed) { * that can redact disallowed data from ORTB2 and/or bid request objects. */ export const redactor = redactorFactory(); + +// by default, TIDs are off since version 8 +registerActivityControl(ACTIVITY_TRANSMIT_TID, 'enableTIDs config', () => { + if (!config.getConfig('enableTIDs')) { + return {allow: false, reason: 'TIDs are disabled'} + } +}); diff --git a/src/adapters/bidderFactory.js b/src/adapters/bidderFactory.js index e74c4edccb1..c8f03bcfc94 100644 --- a/src/adapters/bidderFactory.js +++ b/src/adapters/bidderFactory.js @@ -13,7 +13,7 @@ import { isArray, isPlainObject, logError, - logWarn, + logWarn, memoize, parseQueryStringParameters, parseSizesInput, uniques @@ -22,6 +22,10 @@ import {hook} from '../hook.js'; import {auctionManager} from '../auctionManager.js'; import {bidderSettings} from '../bidderSettings.js'; import {useMetrics} from '../utils/perfMetrics.js'; +import {isActivityAllowed} from '../activities/rules.js'; +import {activityParams} from '../activities/activityParams.js'; +import {MODULE_TYPE_BIDDER} from '../activities/modules.js'; +import {ACTIVITY_TRANSMIT_TID} from '../activities/activities.js'; /** * This file aims to support Adapters during the Prebid 0.x -> 1.x transition. @@ -180,6 +184,38 @@ export function registerBidder(spec) { } } +function guardTids(bidderCode) { + if (isActivityAllowed(ACTIVITY_TRANSMIT_TID, activityParams(MODULE_TYPE_BIDDER, bidderCode))) { + return { + bidRequest: (br) => br, + bidderRequest: (br) => br + }; + } + function get(target, prop, receiver) { + if (['transactionId', 'auctionId'].includes(prop)) { + return null; + } + return Reflect.get(target, prop, receiver); + } + const bidRequest = memoize((br) => new Proxy(br, {get}), (arg) => arg.bidId) + /** + * Return a view on bidd(er) requests where auctionId/transactionId are nulled if the bidder is not allowed `transmitTid`. + * + * Because both auctionId and transactionId are used for Prebid's own internal bookkeeping, we cannot simply erase them + * from request objects; and because request objects are quite complex and not easily cloneable, we hide the IDs + * with a proxy instead. This should be used only around the adapter logic. + */ + return { + bidRequest, + bidderRequest: (br) => new Proxy(br, { + get(target, prop, receiver) { + if (prop === 'bids') return br.bids.map(bidRequest); + return get(target, prop, receiver); + } + }) + } +} + /** * Make a new bidder from the given spec. This is exported mainly for testing. * Adapters will probably find it more convenient to use registerBidder instead. @@ -196,6 +232,7 @@ export function newBidder(spec) { if (!Array.isArray(bidderRequest.bids)) { return; } + const tidGuard = guardTids(bidderRequest.bidderCode); const adUnitCodesHandled = {}; function addBidWithCode(adUnitCode, bid) { @@ -221,7 +258,7 @@ export function newBidder(spec) { } const validBidRequests = adapterMetrics(bidderRequest) - .measureTime('validate', () => bidderRequest.bids.filter(filterAndWarn)); + .measureTime('validate', () => bidderRequest.bids.filter((br) => filterAndWarn(tidGuard.bidRequest(br)))); if (validBidRequests.length === 0) { afterAllResponses(); @@ -236,7 +273,7 @@ export function newBidder(spec) { } }); - processBidderRequests(spec, validBidRequests, bidderRequest, ajax, configEnabledCallback, { + processBidderRequests(spec, validBidRequests.map(tidGuard.bidRequest), tidGuard.bidderRequest(bidderRequest), ajax, configEnabledCallback, { onRequest: requestObject => events.emit(CONSTANTS.EVENTS.BEFORE_BIDDER_HTTP, bidderRequest, requestObject), onResponse: (resp) => { onTimelyResponse(spec.code); diff --git a/test/spec/activities/redactor_spec.js b/test/spec/activities/redactor_spec.js index 5cfa1cfc643..f54b2dcfb95 100644 --- a/test/spec/activities/redactor_spec.js +++ b/test/spec/activities/redactor_spec.js @@ -7,7 +7,7 @@ import { import {ACTIVITY_PARAM_COMPONENT_NAME, ACTIVITY_PARAM_COMPONENT_TYPE} from '../../../src/activities/params.js'; import { ACTIVITY_TRANSMIT_EIDS, - ACTIVITY_TRANSMIT_PRECISE_GEO, + ACTIVITY_TRANSMIT_PRECISE_GEO, ACTIVITY_TRANSMIT_TID, ACTIVITY_TRANSMIT_UFPD } from '../../../src/activities/activities.js'; import {deepAccess, deepSetValue} from '../../../src/utils.js'; @@ -271,6 +271,10 @@ describe('redactor', () => { testAllowDeny(ACTIVITY_TRANSMIT_EIDS, (allowed) => { testPropertiesAreRemoved(() => redactor.bidRequest, ['userId', 'userIdAsEids'], allowed); }); + + testAllowDeny(ACTIVITY_TRANSMIT_TID, (allowed) => { + testPropertiesAreRemoved(() => redactor.bidRequest, ['ortb2Imp.ext.tid'], allowed); + }) }); describe('.ortb2', () => { @@ -282,6 +286,10 @@ describe('redactor', () => { testPropertiesAreRemoved(() => redactor.ortb2, ORTB_UFPD_PATHS, allowed) }); + testAllowDeny(ACTIVITY_TRANSMIT_TID, (allowed) => { + testPropertiesAreRemoved(() => redactor.ortb2, ['source.tid'], allowed); + }); + testAllowDeny(ACTIVITY_TRANSMIT_PRECISE_GEO, (allowed) => { ORTB_GEO_PATHS.forEach(path => { it(`should ${allowed ? 'NOT ' : ''} round down ${path}`, () => { @@ -291,6 +299,6 @@ describe('redactor', () => { expect(deepAccess(ortb2, path)).to.eql(allowed ? 1.2345 : 1.23); }) }) - }) + }); }); }) diff --git a/test/spec/unit/core/adapterManager_spec.js b/test/spec/unit/core/adapterManager_spec.js index af2f7b65f96..cff26df2e4d 100644 --- a/test/spec/unit/core/adapterManager_spec.js +++ b/test/spec/unit/core/adapterManager_spec.js @@ -1862,10 +1862,22 @@ describe('adapterManager tests', function () { requests.appnexus.bids.forEach((bid) => expect(bid.ortb2).to.eql(requests.appnexus.ortb2)); }); - it('should populate ortb2.source.tid with auctionId', () => { - const reqs = adapterManager.makeBidRequests(adUnits, 0, 'mockAuctionId', 1000, [], {global: {}}); - expect(reqs[0].ortb2.source.tid).to.equal('mockAuctionId'); - }) + describe('source.tid', () => { + beforeEach(() => { + sinon.stub(dep, 'redact').returns({ + ortb2: (o) => o, + bidRequest: (b) => b, + }); + }); + afterEach(() => { + dep.redact.restore(); + }); + + it('should be populated with auctionId', () => { + const reqs = adapterManager.makeBidRequests(adUnits, 0, 'mockAuctionId', 1000, [], {global: {}}); + expect(reqs[0].ortb2.source.tid).to.equal('mockAuctionId'); + }) + }); it('should merge in bid-level ortb2Imp with adUnit-level ortb2Imp', () => { const adUnit = { diff --git a/test/spec/unit/core/bidderFactory_spec.js b/test/spec/unit/core/bidderFactory_spec.js index 3ee3e908356..751c90af50f 100644 --- a/test/spec/unit/core/bidderFactory_spec.js +++ b/test/spec/unit/core/bidderFactory_spec.js @@ -12,6 +12,10 @@ import {auctionManager} from '../../../../src/auctionManager.js'; import {stubAuctionIndex} from '../../../helpers/indexStub.js'; import {bidderSettings} from '../../../../src/bidderSettings.js'; import {decorateAdUnitsWithNativeParams} from '../../../../src/native.js'; +import * as activityRules from 'src/activities/rules.js'; +import {sandbox} from 'sinon'; +import {MODULE_TYPE_BIDDER} from '../../../../src/activities/modules.js'; +import {ACTIVITY_TRANSMIT_TID} from '../../../../src/activities/activities.js'; const CODE = 'sampleBidder'; const MOCK_BIDS_REQUEST = { @@ -66,24 +70,25 @@ describe('bidders created by newBidder', function () { }); describe('when the ajax response is irrelevant', function () { + let sandbox; let ajaxStub; let getConfigSpy; let aliasRegistryStub, aliasRegistry; beforeEach(function () { - ajaxStub = sinon.stub(ajax, 'ajax'); + sandbox = sinon.sandbox.create(); + sandbox.stub(activityRules, 'isActivityAllowed').callsFake(() => true); + ajaxStub = sandbox.stub(ajax, 'ajax'); addBidResponseStub.reset(); - getConfigSpy = sinon.spy(config, 'getConfig'); + getConfigSpy = sandbox.spy(config, 'getConfig'); doneStub.reset(); aliasRegistry = {}; - aliasRegistryStub = sinon.stub(adapterManager, 'aliasRegistry'); + aliasRegistryStub = sandbox.stub(adapterManager, 'aliasRegistry'); aliasRegistryStub.get(() => aliasRegistry); }); afterEach(function () { - ajaxStub.restore(); - getConfigSpy.restore(); - aliasRegistryStub.restore(); + sandbox.restore(); }); it('should let registerSyncs run with invalid alias and aliasSync enabled', function () { @@ -135,6 +140,74 @@ describe('bidders created by newBidder', function () { expect(getConfigSpy.withArgs('userSync.filterSettings').calledOnce).to.equal(false); }); + describe('transaction IDs', () => { + Object.entries({ + 'be hidden': false, + 'not be hidden': true, + }).forEach(([t, allowed]) => { + const expectation = allowed ? (val) => expect(val).to.exist : (val) => expect(val).to.not.exist; + + function checkBidRequest(br) { + ['auctionId', 'transactionId'].forEach((prop) => expectation(br[prop])); + } + + function checkBidderRequest(br) { + expectation(br.auctionId); + br.bids.forEach(checkBidRequest); + } + + it(`should ${t} from the spec logic when the transmitTid activity is${allowed ? '' : ' not'} allowed`, () => { + spec.isBidRequestValid.callsFake(br => { + checkBidRequest(br); + return true; + }); + spec.buildRequests.callsFake((bidReqs, bidderReq) => { + checkBidderRequest(bidderReq); + bidReqs.forEach(checkBidRequest); + return {method: 'POST'}; + }); + spec.interpretResponse.callsFake(() => [ + { + requestId: 'bid', + cpm: 123, + ttl: 300, + creativeId: 'crid', + netRevenue: true, + currency: 'USD' + } + ]) + activityRules.isActivityAllowed.reset(); + activityRules.isActivityAllowed.callsFake(() => allowed); + + ajaxStub.callsFake((_, callback) => callback.success(null, {getResponseHeader: sinon.stub()})); + + const bidder = newBidder(spec); + + bidder.callBids({ + bidderCode: 'mockBidder', + auctionId: 'aid', + bids: [ + { + adUnitCode: 'mockAU', + bidId: 'bid', + transactionId: 'tid', + auctionId: 'aid' + } + ] + }, addBidResponseStub, doneStub, ajaxStub, onTimelyResponseStub, wrappedCallback); + + sinon.assert.calledWithMatch(activityRules.isActivityAllowed, ACTIVITY_TRANSMIT_TID, { + componentType: MODULE_TYPE_BIDDER, + componentName: 'mockBidder' + }); + sinon.assert.calledWithMatch(addBidResponseStub, sinon.match.any, { + transactionId: 'tid', + auctionId: 'aid' + }) + }); + }); + }); + it('should handle bad bid requests gracefully', function () { const bidder = newBidder(spec); From 738cbb627d2211a740af8bea872259de7d3bbaa7 Mon Sep 17 00:00:00 2001 From: Demetrio Girardi Date: Thu, 1 Jun 2023 12:22:16 -0700 Subject: [PATCH 2/2] enable TIDs for e2e tests --- test/pages/banner.html | 1 + test/pages/bidderSettings.html | 1 + test/pages/consent_mgt_gdpr.html | 1 + test/pages/currency.html | 1 + test/pages/instream.html | 1 + test/pages/multiple_bidders.html | 2 +- test/pages/native.html | 1 + test/pages/outstream.html | 5 ++++- 8 files changed, 11 insertions(+), 2 deletions(-) diff --git a/test/pages/banner.html b/test/pages/banner.html index 2e88d356647..19593ff4909 100644 --- a/test/pages/banner.html +++ b/test/pages/banner.html @@ -57,6 +57,7 @@ }); pbjs.que.push(function () { + pbjs.setConfig({enableTIDs: true}); pbjs.addAdUnits(adUnits); pbjs.requestBids({ bidsBackHandler: sendAdServerRequest }); }); diff --git a/test/pages/bidderSettings.html b/test/pages/bidderSettings.html index 205fc250be1..1b4d60c166f 100644 --- a/test/pages/bidderSettings.html +++ b/test/pages/bidderSettings.html @@ -67,6 +67,7 @@ }); pbjs.que.push(function () { + pbjs.setConfig({enableTIDs: true}); pbjs.addAdUnits(adUnits); pbjs.bidderSettings = { appnexus: { diff --git a/test/pages/consent_mgt_gdpr.html b/test/pages/consent_mgt_gdpr.html index 2fd9e963d60..b22d1e958e0 100644 --- a/test/pages/consent_mgt_gdpr.html +++ b/test/pages/consent_mgt_gdpr.html @@ -146,6 +146,7 @@ pbjs.que.push(function () { pbjs.addAdUnits(adUnits); pbjs.setConfig({ + enableTIDs: true, consentManagement: { gdpr: { cmpApi: 'static', diff --git a/test/pages/currency.html b/test/pages/currency.html index 84abe75147c..27e0d801aec 100644 --- a/test/pages/currency.html +++ b/test/pages/currency.html @@ -77,6 +77,7 @@ pbjs.que.push(function() { pbjs.setConfig({ + enableTIDs: true, "currency": { "adServerCurrency": "GBP", "granularityMultiplier": 1, diff --git a/test/pages/instream.html b/test/pages/instream.html index 3b5bfb3d102..ca42815c1f8 100644 --- a/test/pages/instream.html +++ b/test/pages/instream.html @@ -60,6 +60,7 @@ pbjs.addAdUnits(videoAdUnit); pbjs.setConfig({ + enableTIDs: true, debug: true, cache: { url: 'https://prebid.adnxs.com/pbc/v1/cache' diff --git a/test/pages/multiple_bidders.html b/test/pages/multiple_bidders.html index b743306d1ba..8fb93f2b379 100644 --- a/test/pages/multiple_bidders.html +++ b/test/pages/multiple_bidders.html @@ -72,7 +72,7 @@ } }] }]; - + pbjs.setConfig({enableTIDs: true}); pbjs.addAdUnits(adUnits); pbjs.requestBids({ timeout: PREBID_TIMEOUT, diff --git a/test/pages/native.html b/test/pages/native.html index d5216dfb6e7..ab477d20c95 100644 --- a/test/pages/native.html +++ b/test/pages/native.html @@ -83,6 +83,7 @@ }); pbjs.que.push(function () { + pbjs.setConfig({enableTIDs: true}); pbjs.addAdUnits(adUnits); pbjs.requestBids({ bidsBackHandler: sendAdServerRequest }); }); diff --git a/test/pages/outstream.html b/test/pages/outstream.html index 96a219f02af..69195419923 100644 --- a/test/pages/outstream.html +++ b/test/pages/outstream.html @@ -61,7 +61,10 @@ googletag.cmd = googletag.cmd || []; pbjs.que.push(function () { - pbjs.setConfig({ debug: true }); + pbjs.setConfig({ + enableTIDs: true, + debug: true + }); pbjs.addAdUnits(outstreamVideoAdUnit); pbjs.bidderSettings = { appnexus: {