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.

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

fix(cherry-pick): Adding "cookie id" to metrics event (#26697) #27227

Merged
merged 1 commit into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 16 additions & 2 deletions app/scripts/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ import { createOffscreen } from './offscreen';
/* eslint-enable import/first */

import { TRIGGER_TYPES } from './controllers/metamask-notifications/constants/notification-schema';
import { COOKIE_ID_MARKETING_WHITELIST_ORIGINS } from './constants/marketing-site-whitelist';

// eslint-disable-next-line @metamask/design-tokens/color-no-hex
const BADGE_COLOR_APPROVAL = '#0376C9';
Expand Down Expand Up @@ -123,6 +124,7 @@ if (inTest || process.env.METAMASK_DEBUG) {
}

const phishingPageUrl = new URL(process.env.PHISHING_WARNING_PAGE_URL);

// normalized (adds a trailing slash to the end of the domain if it's missing)
// the URL once and reuse it:
const phishingPageHref = phishingPageUrl.toString();
Expand Down Expand Up @@ -865,10 +867,10 @@ export function setupController(
senderUrl.origin === phishingPageUrl.origin &&
senderUrl.pathname === phishingPageUrl.pathname
) {
const portStream =
const portStreamForPhishingPage =
overrides?.getPortStream?.(remotePort) || new PortStream(remotePort);
controller.setupPhishingCommunication({
connectionStream: portStream,
connectionStream: portStreamForPhishingPage,
});
} else {
// this is triggered when a new tab is opened, or origin(url) is changed
Expand All @@ -888,6 +890,18 @@ export function setupController(
}
});
}
if (
senderUrl &&
COOKIE_ID_MARKETING_WHITELIST_ORIGINS.some(
(origin) => origin === senderUrl.origin,
)
) {
const portStreamForCookieHandlerPage =
overrides?.getPortStream?.(remotePort) || new PortStream(remotePort);
controller.setUpCookieHandlerCommunication({
connectionStream: portStreamForCookieHandlerPage,
});
}
connectExternalExtension(remotePort);
}
};
Expand Down
15 changes: 15 additions & 0 deletions app/scripts/constants/marketing-site-whitelist.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
export const COOKIE_ID_MARKETING_WHITELIST = [
'https://metamask.io',
'https://learn.metamask.io',
'https://mmi-support.zendesk.com',
'https://community.metamask.io',
'https://support.metamask.io',
];

if (process.env.IN_TEST) {
COOKIE_ID_MARKETING_WHITELIST.push('http://127.0.0.1:8080');
}

// Extract the origin of each URL in the whitelist
export const COOKIE_ID_MARKETING_WHITELIST_ORIGINS =
COOKIE_ID_MARKETING_WHITELIST.map((url) => new URL(url).origin);
1 change: 1 addition & 0 deletions app/scripts/constants/sentry-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ export const SENTRY_BACKGROUND_STATE = {
segmentApiCalls: false,
traits: false,
dataCollectionForMarketing: false,
marketingCampaignCookieId: true,
},
NameController: {
names: false,
Expand Down
17 changes: 17 additions & 0 deletions app/scripts/constants/stream.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// contexts
export const CONTENT_SCRIPT = 'metamask-contentscript';
export const METAMASK_INPAGE = 'metamask-inpage';

// stream channels
export const METAMASK_PROVIDER = 'metamask-provider';
export const METAMASK_COOKIE_HANDLER = 'metamask-cookie-handler';
export const PHISHING_SAFELIST = 'metamask-phishing-safelist';
export const PHISHING_STREAM = 'phishing';

// For more information about these legacy streams, see here:
// https://github.com/MetaMask/metamask-extension/issues/15491
// TODO:LegacyProvider: Delete
export const LEGACY_CONTENT_SCRIPT = 'contentscript';
export const LEGACY_INPAGE = 'inpage';
export const LEGACY_PROVIDER = 'provider';
export const LEGACY_PUBLIC_CONFIG = 'publicConfig';
55 changes: 42 additions & 13 deletions app/scripts/contentscript.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,16 @@ import {
getIsBrowserPrerenderBroken,
} from '../../shared/modules/browser-runtime.utils';
import shouldInjectProvider from '../../shared/modules/provider-injection';
import {
initializeCookieHandlerSteam,
isDetectedCookieMarketingSite,
} from './streams/cookie-handler-stream';
import { logStreamDisconnectWarning } from './streams/shared';
import {
METAMASK_COOKIE_HANDLER,
PHISHING_STREAM,
METAMASK_PROVIDER,
} from './constants/stream';

// contexts
const CONTENT_SCRIPT = 'metamask-contentscript';
Expand Down Expand Up @@ -73,6 +83,11 @@ function setupPhishingPageStreams() {
);

phishingPageChannel = phishingPageMux.createStream(PHISHING_SAFELIST);
phishingPageMux.ignoreStream(METAMASK_COOKIE_HANDLER);
phishingPageMux.ignoreStream(LEGACY_PUBLIC_CONFIG);
phishingPageMux.ignoreStream(LEGACY_PROVIDER);
phishingPageMux.ignoreStream(METAMASK_PROVIDER);
phishingPageMux.ignoreStream(PHISHING_STREAM);
}

const setupPhishingExtStreams = () => {
Expand Down Expand Up @@ -116,6 +131,11 @@ const setupPhishingExtStreams = () => {
error,
),
);
phishingExtMux.ignoreStream(METAMASK_COOKIE_HANDLER);
phishingExtMux.ignoreStream(LEGACY_PUBLIC_CONFIG);
phishingExtMux.ignoreStream(LEGACY_PROVIDER);
phishingExtMux.ignoreStream(METAMASK_PROVIDER);
phishingExtMux.ignoreStream(PHISHING_STREAM);

// eslint-disable-next-line no-use-before-define
phishingExtPort.onDisconnect.addListener(onDisconnectDestroyPhishingStreams);
Expand Down Expand Up @@ -213,6 +233,11 @@ const setupPageStreams = () => {
);

pageChannel = pageMux.createStream(PROVIDER);
pageMux.ignoreStream(METAMASK_COOKIE_HANDLER);
pageMux.ignoreStream(LEGACY_PROVIDER);
pageMux.ignoreStream(LEGACY_PUBLIC_CONFIG);
pageMux.ignoreStream(PHISHING_SAFELIST);
pageMux.ignoreStream(PHISHING_STREAM);
};

// The field below is used to ensure that replay is done only once for each restart.
Expand Down Expand Up @@ -248,6 +273,10 @@ const setupExtensionStreams = () => {
extensionPhishingStream = extensionMux.createStream('phishing');
extensionPhishingStream.once('data', redirectToPhishingWarning);

extensionMux.ignoreStream(METAMASK_COOKIE_HANDLER);
extensionMux.ignoreStream(LEGACY_PROVIDER);
extensionMux.ignoreStream(PHISHING_SAFELIST);

// eslint-disable-next-line no-use-before-define
extensionPort.onDisconnect.addListener(onDisconnectDestroyStreams);
};
Expand Down Expand Up @@ -288,6 +317,11 @@ const setupLegacyPageStreams = () => {
legacyPageMux.createStream(LEGACY_PROVIDER);
legacyPagePublicConfigChannel =
legacyPageMux.createStream(LEGACY_PUBLIC_CONFIG);

legacyPageMux.ignoreStream(METAMASK_COOKIE_HANDLER);
legacyPageMux.ignoreStream(METAMASK_PROVIDER);
legacyPageMux.ignoreStream(PHISHING_SAFELIST);
legacyPageMux.ignoreStream(PHISHING_STREAM);
};

// TODO:LegacyProvider: Delete
Expand Down Expand Up @@ -331,6 +365,10 @@ const setupLegacyExtensionStreams = () => {
error,
),
);
legacyExtMux.ignoreStream(METAMASK_COOKIE_HANDLER);
legacyExtMux.ignoreStream(LEGACY_PROVIDER);
legacyExtMux.ignoreStream(PHISHING_SAFELIST);
legacyExtMux.ignoreStream(PHISHING_STREAM);
};

/**
Expand Down Expand Up @@ -431,19 +469,6 @@ function getNotificationTransformStream() {
return stream;
}

/**
* Error handler for page to extension stream disconnections
*
* @param {string} remoteLabel - Remote stream name
* @param {Error} error - Stream connection error
*/
function logStreamDisconnectWarning(remoteLabel, error) {
console.debug(
`MetaMask: Content script lost connection to "${remoteLabel}".`,
error,
);
}

/**
* The function notifies inpage when the extension stream connection is ready. When the
* 'metamask_chainChanged' method is received from the extension, it implies that the
Expand Down Expand Up @@ -525,6 +550,10 @@ const start = () => {
return;
}

if (isDetectedCookieMarketingSite) {
initializeCookieHandlerSteam();
}

if (shouldInjectProvider()) {
initStreams();

Expand Down
14 changes: 14 additions & 0 deletions app/scripts/controllers/metametrics.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ export default class MetaMetricsController {
participateInMetaMetrics: null,
metaMetricsId: null,
dataCollectionForMarketing: null,
marketingCampaignCookieId: null,
eventsBeforeMetricsOptIn: [],
traits: {},
previousUserTraits: {},
Expand Down Expand Up @@ -466,6 +467,8 @@ export default class MetaMetricsController {
if (participateInMetaMetrics) {
this.trackEventsAfterMetricsOptIn();
this.clearEventsAfterMetricsOptIn();
} else if (this.state.marketingCampaignCookieId) {
this.setMarketingCampaignCookieId(null);
}

///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask)
Expand All @@ -477,10 +480,20 @@ export default class MetaMetricsController {

setDataCollectionForMarketing(dataCollectionForMarketing) {
const { metaMetricsId } = this.state;

this.store.updateState({ dataCollectionForMarketing });

if (!dataCollectionForMarketing && this.state.marketingCampaignCookieId) {
this.setMarketingCampaignCookieId(null);
}

return metaMetricsId;
}

setMarketingCampaignCookieId(marketingCampaignCookieId) {
this.store.updateState({ marketingCampaignCookieId });
}

get state() {
return this.store.getState();
}
Expand Down Expand Up @@ -704,6 +717,7 @@ export default class MetaMetricsController {
userAgent: window.navigator.userAgent,
page,
referrer,
marketingCampaignCookieId: this.state.marketingCampaignCookieId,
};
}

Expand Down
82 changes: 81 additions & 1 deletion app/scripts/controllers/metametrics.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const VERSION = '0.0.1-test';
const FAKE_CHAIN_ID = '0x1338';
const LOCALE = 'en_US';
const TEST_META_METRICS_ID = '0xabc';
const TEST_GA_COOKIE_ID = '123456.123455';
const DUMMY_ACTION_ID = 'DUMMY_ACTION_ID';
const MOCK_EXTENSION_ID = 'testid';

Expand Down Expand Up @@ -50,6 +51,7 @@ const DEFAULT_TEST_CONTEXT = {
page: METAMETRICS_BACKGROUND_PAGE_OBJECT,
referrer: undefined,
userAgent: window.navigator.userAgent,
marketingCampaignCookieId: null,
};

const DEFAULT_SHARED_PROPERTIES = {
Expand Down Expand Up @@ -113,6 +115,7 @@ const SAMPLE_NON_PERSISTED_EVENT = {
function getMetaMetricsController({
participateInMetaMetrics = true,
metaMetricsId = TEST_META_METRICS_ID,
marketingCampaignCookieId = null,
preferencesStore = getMockPreferencesStore(),
getCurrentChainId = () => FAKE_CHAIN_ID,
onNetworkDidChange = () => {
Expand All @@ -130,6 +133,7 @@ function getMetaMetricsController({
initState: {
participateInMetaMetrics,
metaMetricsId,
marketingCampaignCookieId,
fragments: {
testid: SAMPLE_PERSISTED_EVENT,
testid2: SAMPLE_NON_PERSISTED_EVENT,
Expand Down Expand Up @@ -160,6 +164,9 @@ describe('MetaMetricsController', function () {
expect(metaMetricsController.state.metaMetricsId).toStrictEqual(
TEST_META_METRICS_ID,
);
expect(
metaMetricsController.state.marketingCampaignCookieId,
).toStrictEqual(null);
expect(metaMetricsController.locale).toStrictEqual(
LOCALE.replace('_', '-'),
);
Expand Down Expand Up @@ -339,6 +346,21 @@ describe('MetaMetricsController', function () {
TEST_META_METRICS_ID,
);
});
it('should nullify the marketingCampaignCookieId when participateInMetaMetrics is toggled off', async function () {
const metaMetricsController = getMetaMetricsController({
participateInMetaMetrics: true,
metaMetricsId: TEST_META_METRICS_ID,
dataCollectionForMarketing: true,
marketingCampaignCookieId: TEST_GA_COOKIE_ID,
});
expect(
metaMetricsController.state.marketingCampaignCookieId,
).toStrictEqual(TEST_GA_COOKIE_ID);
await metaMetricsController.setParticipateInMetaMetrics(false);
expect(
metaMetricsController.state.marketingCampaignCookieId,
).toStrictEqual(null);
});
});

describe('submitEvent', function () {
Expand Down Expand Up @@ -1252,7 +1274,65 @@ describe('MetaMetricsController', function () {
expect(Object.keys(segmentApiCalls).length === 0).toStrictEqual(true);
});
});

describe('setMarketingCampaignCookieId', function () {
it('should update marketingCampaignCookieId in the context when cookieId is available', async function () {
const metaMetricsController = getMetaMetricsController({
participateInMetaMetrics: true,
metaMetricsId: TEST_META_METRICS_ID,
dataCollectionForMarketing: true,
});
metaMetricsController.setMarketingCampaignCookieId(TEST_GA_COOKIE_ID);
expect(
metaMetricsController.state.marketingCampaignCookieId,
).toStrictEqual(TEST_GA_COOKIE_ID);
const spy = jest.spyOn(segment, 'track');
metaMetricsController.submitEvent(
{
event: 'Fake Event',
category: 'Unit Test',
properties: {
test: 1,
},
},
{ isOptIn: true },
);
expect(spy).toHaveBeenCalledTimes(1);
expect(spy).toHaveBeenCalledWith(
{
event: 'Fake Event',
anonymousId: METAMETRICS_ANONYMOUS_ID,
context: {
...DEFAULT_TEST_CONTEXT,
marketingCampaignCookieId: TEST_GA_COOKIE_ID,
},
properties: {
test: 1,
...DEFAULT_EVENT_PROPERTIES,
},
messageId: Utils.generateRandomId(),
timestamp: new Date(),
},
spy.mock.calls[0][1],
);
});
});
describe('setDataCollectionForMarketing', function () {
it('should nullify the marketingCampaignCookieId when Data collection for marketing is toggled off', async function () {
const metaMetricsController = getMetaMetricsController({
participateInMetaMetrics: true,
metaMetricsId: TEST_META_METRICS_ID,
dataCollectionForMarketing: true,
marketingCampaignCookieId: TEST_GA_COOKIE_ID,
});
expect(
metaMetricsController.state.marketingCampaignCookieId,
).toStrictEqual(TEST_GA_COOKIE_ID);
await metaMetricsController.setDataCollectionForMarketing(false);
expect(
metaMetricsController.state.marketingCampaignCookieId,
).toStrictEqual(null);
});
});
afterEach(function () {
// flush the queues manually after each test
segment.flush();
Expand Down
Loading
Loading