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

Lotame id system domain update #6063

Closed
wants to merge 8 commits into from
Closed

Lotame id system domain update #6063

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
625d4df
Fix the userId example so the page will load
markaconrad Nov 25, 2020
7d3e79b
LotamePanoramaIdSystem: Fix parseInt typo; store cookies on the root …
markaconrad Nov 25, 2020
815bd0a
Remove extra space
markaconrad Nov 25, 2020
3447bcd
LotamePanoramaIdSystem: Resolve the domain later; Add tests
markaconrad Nov 25, 2020
9e04594
Merge branch 'master' into lotameIdSystem_domainUpdate
markaconrad Dec 1, 2020
dc5ef22
Per LGTM check, remove useless assignment
markaconrad Dec 1, 2020
bd2fe9c
For a particular error case, don't update the profile id in storage
markaconrad Dec 11, 2020
14f6991
Add tests for the error 111 ignore case
markaconrad Dec 11, 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
4 changes: 3 additions & 1 deletion integrationExamples/gpt/userId_example.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@
}
})(window, document);
// window.__cmp('showConsentTool');

function getHashedEmail() {}
</script>

<script>
Expand Down Expand Up @@ -323,7 +325,7 @@
</head>

<body>
<h2>Rubicon Project Prebid</h2>
<h2>Prebid User ID Example</h2>

<div id='test-div'>
<script>
Expand Down
54 changes: 47 additions & 7 deletions modules/lotamePanoramaIdSystem.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,9 @@ const KEY_EXPIRY = `${KEY_ID}_expiry`;
const KEY_PROFILE = '_cc_id';
const MODULE_NAME = 'lotamePanoramaId';
const NINE_MONTHS_MS = 23328000 * 1000;
const DAYS_TO_CACHE = 7;
const DAY_MS = 60 * 60 * 24 * 1000;

export const storage = getStorageManager(null, MODULE_NAME);
let cookieDomain;

/**
* Set the Lotame First Party Profile ID in the first party namespace
Expand All @@ -26,7 +25,7 @@ export const storage = getStorageManager(null, MODULE_NAME);
function setProfileId(profileId) {
if (storage.cookiesAreEnabled()) {
let expirationDate = new Date(utils.timestamp() + NINE_MONTHS_MS).toUTCString();
storage.setCookie(KEY_PROFILE, profileId, expirationDate, 'Lax', undefined, undefined);
storage.setCookie(KEY_PROFILE, profileId, expirationDate, 'Lax', cookieDomain, undefined);
}
if (storage.hasLocalStorage()) {
storage.setDataInLocalStorage(KEY_PROFILE, profileId, undefined);
Expand Down Expand Up @@ -78,7 +77,7 @@ function getFromStorage(key) {
function saveLotameCache(
key,
value,
expirationTimestamp = utils.timestamp() + DAYS_TO_CACHE * DAY_MS
expirationTimestamp
) {
if (key && value) {
let expirationDate = new Date(expirationTimestamp).toUTCString();
Expand All @@ -88,7 +87,7 @@ function saveLotameCache(
value,
expirationDate,
'Lax',
undefined,
cookieDomain,
undefined
);
}
Expand All @@ -115,7 +114,7 @@ function getLotameLocalCache() {
try {
const rawExpiry = getFromStorage(KEY_EXPIRY);
if (utils.isStr(rawExpiry)) {
cache.expiryTimestampMs = parseInt(rawExpiry, 0);
cache.expiryTimestampMs = parseInt(rawExpiry, 10);
}
} catch (error) {
utils.logError(error);
Expand Down Expand Up @@ -148,6 +147,42 @@ export const lotamePanoramaIdSubmodule = {
*/
name: MODULE_NAME,

/**
* Find the root domain
* @param {string} fullDomain
* @return {?string}
*/
findCookieDomain: function (fullDomain) {
if (!storage.cookiesAreEnabled()) {
return fullDomain;
}

const domainParts = fullDomain.split('.');
if (domainParts.length == 2) {
return fullDomain;
}
let rootDomain;
let continueSearching;
let startIndex = -2;
const TEST_COOKIE_NAME = 'lotame_domain_check';
const TEST_COOKIE_VALUE = 'writeable';
do {
rootDomain = domainParts.slice(startIndex).join('.');
let expirationDate = new Date(
utils.timestamp() + 10 * 1000
).toUTCString();
storage.setCookie(TEST_COOKIE_NAME, TEST_COOKIE_VALUE, expirationDate, 'Lax', `${rootDomain}`, undefined);
const value = storage.getCookie(TEST_COOKIE_NAME, undefined);
if (value === TEST_COOKIE_VALUE) {
continueSearching = false;
} else {
startIndex += -1;
continueSearching = Math.abs(startIndex) <= domainParts.length;
}
} while (continueSearching);
return rootDomain;
},
Comment on lines +155 to +184
Copy link
Collaborator

Choose a reason for hiding this comment

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

there's at least one or two other id modules that are doing essentially the same thing -- finding the root domain at which to write the 1p cookie. what are people's thoughts on moving this to a method in utils or userId module that can be re-used rather than repeated in multiple modules?

cc @patmmccann @Fawke @jdwieland8282 @robertrmartinez @pycnvr

Copy link
Collaborator

Choose a reason for hiding this comment

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

@smenzer Yes, I like your idea.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@idettman do you have thoughts here? is this something we can/should break out to be a common userid (or even common prebid global) method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you all thinking this should be done in context of this PR or separately?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it probably makes the most sense to break this out into a separate PR, then you can use this PR to apply it to the Lotame ID system

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, I will give that shot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That part is attempted in #6124

Copy link
Collaborator

Choose a reason for hiding this comment

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

@markaconrad thanks for putting in #6124! I assume once that goes through, you would remove this block and on line 206 below you'd use the user id function instead of this right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are welcome! Yes, that is the plan


/**
* Decode the stored id value for passing to bid requests
* @function decode
Expand All @@ -168,6 +203,7 @@ export const lotamePanoramaIdSubmodule = {
* @returns {IdResponse|undefined}
*/
getId(config, consentData, cacheIdObj) {
cookieDomain = this.findCookieDomain(String(document.domain));
let localCache = getLotameLocalCache();

let refreshNeeded = Date.now() > localCache.expiryTimestampMs;
Expand Down Expand Up @@ -205,7 +241,11 @@ export const lotamePanoramaIdSubmodule = {
if (response) {
try {
let responseObj = JSON.parse(response);
saveLotameCache(KEY_EXPIRY, responseObj.expiry_ts);
saveLotameCache(
KEY_EXPIRY,
responseObj.expiry_ts,
responseObj.expiry_ts
);

if (utils.isStr(responseObj.profile_id)) {
setProfileId(responseObj.profile_id);
Expand Down
20 changes: 20 additions & 0 deletions test/spec/modules/lotamePanoramaIdSystem_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,4 +446,24 @@ describe('LotameId', function() {
'lotamePanoramaId': '1234'
});
});

describe('findCookieDomain', function() {
beforeEach(function() {
getCookieStub
.onFirstCall()
.returns(null) // .co.uk
.onSecondCall()
.returns('writeable') // realdomain.co.uk;
});

it('should just find the root domain', function() {
var domain = lotamePanoramaIdSubmodule.findCookieDomain('sub.realdomain.co.uk')
expect(domain).to.be.eq('realdomain.co.uk')
});

it('should find the full domain when no subdomain is present', function() {
var domain = lotamePanoramaIdSubmodule.findCookieDomain('realdomain.co.uk')
expect(domain).to.be.eq('realdomain.co.uk')
});
});
});