Skip to content

Commit

Permalink
fix(cherry-pick): Adding "cookie id" to metrics event (#26697) (#27227)
Browse files Browse the repository at this point in the history
Cherry picking #26697 to V12.3.0

#26697 sets up a stream communication with the label
'metamask-cookie-handler'. The data sent from the page will be
intercepted at the content-script and forwarded to the extension. The
'ga_client_id' sent from the page is saved as
`marketingCampaignCookieId` in the metaMetricsController state and
fetched when building the context for each event.
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27227?quickstart=1)

## **Related issues**

Fixes:

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
  • Loading branch information
NiranjanaBinoy authored Sep 18, 2024
1 parent 63cf08b commit 3373b9a
Show file tree
Hide file tree
Showing 14 changed files with 703 additions and 16 deletions.
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

0 comments on commit 3373b9a

Please sign in to comment.