Add enforcement for deviceAccess by idettman · Pull Request #4913 · prebid/Prebid.js

idettman · 2020-02-28T18:30:13Z

Type of change

Does this change affect user-facing APIs or examples documented on http://prebid.org?

Description of change

We discussed in a Prebid.js meeting and came up with a radically simpler (and quicker) approach, relying on the publisher to determine when to turn off device access and then configure:

turn off usersync (already supported with the syncEnabled flag)
turn off userId modules (just don't configure them)
turn off utils setCookie,getCookie,hasLocalStorage (new config option)
review to ensure that all modules are either using the core utils functions or paying attention to a new config option.

The proposed new config option is:
pbjs.setConfig({ deviceAccess: false });

Acceptance Criteria

When the deviceAccess flag config option is set to false, no cookies are read or set
When the deviceAccess flag config option is set to true, cookies may be read and set
When the deviceAccess flag config option is not set, cookies may be read and set

…ebid.js into master-rubicon-clean

# Conflicts: # modules/advangelistsBidAdapter.js # test/spec/modules/advangelistsBidAdapter_spec.js

…ter-remote-updated

…ter-origin

…ter-origin-rp

…gin-master

src/utils.js

pycnvr · 2020-03-03T21:59:08Z

Hi @idettman

It might be useful to add a bit more protection for pubCommonIdSystem. With deviceAccess == false, it still keeps generating new uuid as userId.pubcid even though they are not persisted.

In pubCommonIdSystem.js line 67, add a check to prevent id creation.

    const newId = (create && utils.hasDeviceAccess()) ? utils.generateUUID() : undefined;

harpere · 2020-03-07T16:59:26Z

modules/digiTrustIdSystem.js

  date.setTime(date.getTime() + 604800000);
-  var exp = 'expires=' + date.toUTCString();
-  document.cookie = key + '=' + encId(id) + '; ' + exp + '; path=/;SameSite=none;';
+  utils.setCookie(key, encId(id), date.toUTCString(), 'none');


this may require more refactoring because encId(), calls encodeURIComponent() and so does setCookie(). encoding twice could be a problem if it's not decoded twice on the other end. So maybe we refactor encId() or maybe add an option to setCookie() to NOT encode.

Let me look into this. The id is typically passed through and decoded via a server-side process. I'll see if there is a reasonable place to account for possible double encoding in that pipeline.

@goosemanjack - would like to get this ticket merged. Should we remove the call to encId()?

I made a change here. Removed the call to encodeURI from encId. @goosemanjack - please review when you get a chance, but we need to merge this PR.

osazos · 2020-03-09T14:39:42Z

modules/adagioBidAdapter.js

  try {
    const w = utils.getWindowTop();
-    const ls = w.localStorage.getItem(ADAGIO_LOCALSTORAGE_KEY);
+    const ls = (utils.hasLocalStorage()) ? w.localStorage.getItem(ADAGIO_LOCALSTORAGE_KEY) : null;


Hi,

As utils.getDataFromLocalStorage() checks if localStorage is available and returns null if nothing is found, we can just use:

const ls = utils.getDataFromLocalStorage(ADAGIO_LOCALSTORAGE_KEY);

Also, we call localStorage twice later in our bidder : lines 37 and 51. For this lines, we should use:

utils.removeDataFromLocalStorage(ADAGIO_LOCALSTORAGE_KEY);

Thanks!

@osazos - you can go ahead and make the change you've suggested.

@bretg, I created #4978.
Honestly, I don't know if there is a better way to submit my change. Please fell free to tell me.

@osazos - I merged your changes from #4978 -- will close that PR.

Gaudeamus · 2020-03-10T11:55:12Z

modules/mgidBidAdapter.js

@@ -342,15 +342,15 @@ function getLanguage() {

 function getLocalStorageSafely(key) {
  try {


Hello @bretg!
I'm ok with the changes
However if utils.getDataFromLocalStorage(key) makes all safety checks, try/catch here could be avoided. Same in setLocalStorageSafely

merging changes from prebid#4978

We need to get moving on this merge, so I made an update -- the encId() function appears to be necessary to stringify and btoa() the ID. Just removed the layer of URL encoding in that function as duplicate of what happens in the PBJS setCookie fn.

bretg · 2020-03-17T17:31:49Z

Thanks for the reviews. Merging to get into today's release.

* Add microadBidAdapter * Remove unnecessary encodeURIComponent from microadBidAdapter * Submit Advangelists Prebid Adapter * Submit Advangelists Prebid Adapter 1.1 * Correct procudtion endpoint for prebid * analytics update with wrapper name * reverted error merge * update changed default value of netRevenue to true * Re-add rubicon analytics without deprecated getTopWindowUrl util * Cache referrer on auction_init instead of bid_requested * add config for deviceAccess enforcement in utils getCookie setCookie and hasLocalStorage * add tests, and updates to names/comments * add deviceAccess enforcement to util method cookiesAreEnabled * update to adspendBidAdapter for device access enforcement * update to pubCommonId for device access enforcement * update widespaceBidAdapter for device access enforcement. updated utils storage methods with jsdoc typing, hasLocalStorage changed to only return boolean type * update to digiTrustIdSystem for device access enforcement * updated utils.setCookie signature, adding a 'domain' argument. update to invibesBidAdapter for device access enforcement * updated utils to group similar storage methods * updated kargoBidAdapter for add-enforcement-device-access * update to minimize changes in adapters using browser storage * updated modules with localStorage or cookie methods to use utils * updated invibesBidAdapter use of storage methods to use utils * fix for wrong obj reference for date string * revert accidental change to package.json * fix linting error, trailing comma. removed unnecessary cookie get set functions and replaced with util methods * reverted change to removed extra methods since it breaks unit test * fix for pubcommonidsystem id generation * update jsdoc param 'domain' * Update adagioBidAdapter.js merging changes from prebid#4978 * Removing encoding in DT encId We need to get moving on this merge, so I made an update -- the encId() function appears to be necessary to stringify and btoa() the ID. Just removed the layer of URL encoding in that function as duplicate of what happens in the PBJS setCookie fn. Co-authored-by: nakamoto <nakamoto_tatsuya@microad.co.jp> Co-authored-by: Chandra Prakash <chandra.prakash@advangelists.com> Co-authored-by: Eric Harper <eharper@rubiconproject.com> Co-authored-by: TJ Eastmond <teastmond@la-wlandaverde-mac.local> Co-authored-by: Mark Monday <mmonday@rubiconproject.com> Co-authored-by: msm0504 <51493331+msm0504@users.noreply.github.com> Co-authored-by: bretg <bgorsline@gmail.com>

nakamoto and others added 30 commits February 16, 2019 21:30

Add microadBidAdapter

8d635ff

Remove unnecessary encodeURIComponent from microadBidAdapter

3055b90

Submit Advangelists Prebid Adapter

67fb91b

Submit Advangelists Prebid Adapter 1.1

3ebb916

Correct procudtion endpoint for prebid

4f5c451

Merge branch 'microad-bid-adapter' of git://github.com/strong-zero/Pr…

2dc6d1d

…ebid.js into master-rubicon-clean

Merge remote-tracking branch 'origin/master' into master-rubicon-clean

fa3e081

# Conflicts: # modules/advangelistsBidAdapter.js # test/spec/modules/advangelistsBidAdapter_spec.js

Merge branch 'master' of https://github.com/prebid/Prebid.js into mas…

600a46e

…ter-remote-updated

Merge branch 'master' of https://github.com/prebid/Prebid.js

7f578b3

Merge branch 'master' of https://github.com/prebid/Prebid.js

176a312

Merge branch 'master' of https://github.com/prebid/Prebid.js

9abf89c

Merge remote-tracking branch 'upstream/master'

6ce04ab

Merge branch 'master' of https://github.com/prebid/Prebid.js into mas…

415e2f6

…ter-origin

Merge branch 'master' of https://github.com/prebid/Prebid.js into mas…

61fb82c

…ter-origin-rp

analytics update with wrapper name

3cc4c67

Merge branch 'master' of https://github.com/prebid/Prebid.js into mas…

cd81e02

…ter-origin-rp

Merge branch 'master' of https://github.com/prebid/Prebid.js into mas…

e2b4e04

…ter-origin-rp

Merge branch 'master' of https://github.com/prebid/Prebid.js into mas…

53b5970

…ter-origin-rp

Merge branch 'master' of https://github.com/prebid/Prebid.js into mas…

5c00ed5

…ter-origin-rp

reverted error merge

28848ad

Merge branch 'master' of https://github.com/prebid/Prebid.js into mas…

ab635ee

…ter-origin-rp

Merge branch 'master' of https://github.com/prebid/Prebid.js into mas…

93308f5

…ter-origin-rp

Merge branch 'master' of https://github.com/prebid/Prebid.js

1cfd52d

Merge branch 'master' of https://github.com/prebid/Prebid.js

d619807

Merge branch 'master' of https://github.com/prebid/Prebid.js

9893f0f

Merge remote-tracking branch 'upstream/master'

986a251

Merge branch 'master' of https://github.com/prebid/Prebid.js into ori…

2cae7c0

…gin-master

Merge branch 'master' of https://github.com/prebid/Prebid.js

d7fd252

Merge branch 'master' of https://github.com/prebid/Prebid.js

c7c01ba

Merge branch 'master' of https://github.com/prebid/Prebid.js

24a28aa

idettman force-pushed the add-enforcement-device-access branch 2 times, most recently from dbf8a53 to c3c5b09 Compare February 29, 2020 05:55

idettman added the needs docs label Feb 29, 2020

jsnellbaker reviewed Mar 3, 2020

View reviewed changes

src/utils.js Show resolved Hide resolved

pycnvr mentioned this pull request Mar 3, 2020

Proposal for Prebid.js Enforcement of Device Access #4747

Closed

Isaac A. Dettman added 2 commits March 6, 2020 09:25

fix for pubcommonidsystem id generation

891ecbd

update jsdoc param 'domain'

f281ee2

harpere reviewed Mar 7, 2020

View reviewed changes

rade-popovic approved these changes Mar 9, 2020

View reviewed changes

osazos requested changes Mar 9, 2020

View reviewed changes

ghost approved these changes Mar 11, 2020

View reviewed changes

bretg requested a review from jsnellbaker March 12, 2020 14:48

Gaudeamus approved these changes Mar 12, 2020

View reviewed changes

bretg mentioned this pull request Mar 13, 2020

adding deviceAccess flag and TCF2 prebid/prebid.github.io#1857

Merged

bretg removed the needs docs label Mar 13, 2020

bretg approved these changes Mar 13, 2020

View reviewed changes

osazos mentioned this pull request Mar 13, 2020

Add enforcement device access (Adagio fix) #4978

Closed

jsnellbaker requested a review from sumit116 March 16, 2020 13:19

bretg added 2 commits March 17, 2020 10:21

Update adagioBidAdapter.js

b0827c3

merging changes from prebid#4978

Removing encoding in DT encId

025886c

We need to get moving on this merge, so I made an update -- the encId() function appears to be necessary to stringify and btoa() the ID. Just removed the layer of URL encoding in that function as duplicate of what happens in the PBJS setCookie fn.

jsnellbaker approved these changes Mar 17, 2020

View reviewed changes

bretg merged commit a95f911 into prebid:master Mar 17, 2020

bretg removed needs 2nd review Core module updates require two approvals from the core team needs review labels Mar 17, 2020

robertrmartinez deleted the add-enforcement-device-access branch July 5, 2023 19:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add enforcement for deviceAccess#4913

Add enforcement for deviceAccess#4913
bretg merged 82 commits intoprebid:masterfrom
rubicon-project:add-enforcement-device-access

idettman commented Feb 28, 2020 •

edited

Loading

Uh oh!

Uh oh!

pycnvr commented Mar 3, 2020

Uh oh!

harpere Mar 7, 2020

Uh oh!

goosemanjack Mar 9, 2020

Uh oh!

bretg Mar 12, 2020

Uh oh!

bretg Mar 17, 2020

Uh oh!

osazos Mar 9, 2020 •

edited

Loading

Uh oh!

bretg Mar 12, 2020

Uh oh!

osazos Mar 13, 2020

Uh oh!

bretg Mar 17, 2020

Uh oh!

Gaudeamus Mar 10, 2020

Uh oh!

bretg commented Mar 17, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

Comments

		@@ -342,15 +342,15 @@ function getLanguage() {

		function getLocalStorageSafely(key) {
		try {

Conversation

idettman commented Feb 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Type of change

Description of change

Acceptance Criteria

Uh oh!

Uh oh!

pycnvr commented Mar 3, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

osazos Mar 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bretg commented Mar 17, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

Comments

idettman commented Feb 28, 2020 •

edited

Loading

osazos Mar 9, 2020 •

edited

Loading