diff --git a/app/scripts/controllers/authentication/authentication-controller.test.ts b/app/scripts/controllers/authentication/authentication-controller.test.ts index db20a5a85043..4069d7aa316c 100644 --- a/app/scripts/controllers/authentication/authentication-controller.test.ts +++ b/app/scripts/controllers/authentication/authentication-controller.test.ts @@ -28,6 +28,7 @@ describe('authentication/authentication-controller - constructor() tests', () => test('should initialize with default state', () => { const controller = new AuthenticationController({ messenger: createAuthenticationMessenger(), + metametrics: createMockAuthMetaMetrics(), }); expect(controller.state.isSignedIn).toBe(false); @@ -38,6 +39,7 @@ describe('authentication/authentication-controller - constructor() tests', () => const controller = new AuthenticationController({ messenger: createAuthenticationMessenger(), state: mockSignedInState(), + metametrics: createMockAuthMetaMetrics(), }); expect(controller.state.isSignedIn).toBe(true); @@ -48,10 +50,11 @@ describe('authentication/authentication-controller - constructor() tests', () => describe('authentication/authentication-controller - performSignIn() tests', () => { test('Should create access token and update state', async () => { const mockEndpoints = mockAuthenticationFlowEndpoints(); + const metametrics = createMockAuthMetaMetrics(); const { messenger, mockSnapGetPublicKey, mockSnapSignMessage } = createMockAuthenticationMessenger(); - const controller = new AuthenticationController({ messenger }); + const controller = new AuthenticationController({ messenger, metametrics }); const result = await controller.performSignIn(); expect(mockSnapGetPublicKey).toBeCalled(); @@ -84,8 +87,9 @@ describe('authentication/authentication-controller - performSignIn() tests', () const mockEndpoints = mockAuthenticationFlowEndpoints({ endpointFail, }); + const metametrics = createMockAuthMetaMetrics(); const { messenger } = createMockAuthenticationMessenger(); - const controller = new AuthenticationController({ messenger }); + const controller = new AuthenticationController({ messenger, metametrics }); await expect(controller.performSignIn()).rejects.toThrow(); expect(controller.state.isSignedIn).toBe(false); @@ -111,10 +115,12 @@ describe('authentication/authentication-controller - performSignIn() tests', () describe('authentication/authentication-controller - performSignOut() tests', () => { test('Should remove signed in user and any access tokens', () => { + const metametrics = createMockAuthMetaMetrics(); const { messenger } = createMockAuthenticationMessenger(); const controller = new AuthenticationController({ messenger, state: mockSignedInState(), + metametrics, }); controller.performSignOut(); @@ -123,10 +129,12 @@ describe('authentication/authentication-controller - performSignOut() tests', () }); test('Should throw error if attempting to sign out when user is not logged in', () => { + const metametrics = createMockAuthMetaMetrics(); const { messenger } = createMockAuthenticationMessenger(); const controller = new AuthenticationController({ messenger, state: { isSignedIn: false }, + metametrics, }); expect(() => controller.performSignOut()).toThrow(); @@ -135,21 +143,25 @@ describe('authentication/authentication-controller - performSignOut() tests', () describe('authentication/authentication-controller - getBearerToken() tests', () => { test('Should throw error if not logged in', async () => { + const metametrics = createMockAuthMetaMetrics(); const { messenger } = createMockAuthenticationMessenger(); const controller = new AuthenticationController({ messenger, state: { isSignedIn: false }, + metametrics, }); await expect(controller.getBearerToken()).rejects.toThrow(); }); test('Should return original access token in state', async () => { + const metametrics = createMockAuthMetaMetrics(); const { messenger } = createMockAuthenticationMessenger(); const originalState = mockSignedInState(); const controller = new AuthenticationController({ messenger, state: originalState, + metametrics, }); const result = await controller.getBearerToken(); @@ -158,6 +170,7 @@ describe('authentication/authentication-controller - getBearerToken() tests', () }); test('Should return new access token if state is invalid', async () => { + const metametrics = createMockAuthMetaMetrics(); const { messenger } = createMockAuthenticationMessenger(); mockAuthenticationFlowEndpoints(); const originalState = mockSignedInState(); @@ -172,6 +185,7 @@ describe('authentication/authentication-controller - getBearerToken() tests', () const controller = new AuthenticationController({ messenger, state: originalState, + metametrics, }); const result = await controller.getBearerToken(); @@ -182,21 +196,25 @@ describe('authentication/authentication-controller - getBearerToken() tests', () describe('authentication/authentication-controller - getSessionProfile() tests', () => { test('Should throw error if not logged in', async () => { + const metametrics = createMockAuthMetaMetrics(); const { messenger } = createMockAuthenticationMessenger(); const controller = new AuthenticationController({ messenger, state: { isSignedIn: false }, + metametrics, }); await expect(controller.getSessionProfile()).rejects.toThrow(); }); test('Should return original access token in state', async () => { + const metametrics = createMockAuthMetaMetrics(); const { messenger } = createMockAuthenticationMessenger(); const originalState = mockSignedInState(); const controller = new AuthenticationController({ messenger, state: originalState, + metametrics, }); const result = await controller.getSessionProfile(); @@ -205,6 +223,7 @@ describe('authentication/authentication-controller - getSessionProfile() tests', }); test('Should return new access token if state is invalid', async () => { + const metametrics = createMockAuthMetaMetrics(); const { messenger } = createMockAuthenticationMessenger(); mockAuthenticationFlowEndpoints(); const originalState = mockSignedInState(); @@ -219,6 +238,7 @@ describe('authentication/authentication-controller - getSessionProfile() tests', const controller = new AuthenticationController({ messenger, state: originalState, + metametrics, }); const result = await controller.getSessionProfile(); @@ -292,3 +312,13 @@ function mockAuthenticationFlowEndpoints(params?: { mockAccessTokenEndpoint, }; } + +function createMockAuthMetaMetrics() { + const getMetaMetricsId = jest.fn().mockReturnValue('MOCK_METAMETRICS_ID'); + const setMetaMetricsId = jest.fn(); + + return { + getMetaMetricsId, + setMetaMetricsId, + }; +} diff --git a/app/scripts/controllers/authentication/authentication-controller.ts b/app/scripts/controllers/authentication/authentication-controller.ts index b8a0b964827b..813210e4124b 100644 --- a/app/scripts/controllers/authentication/authentication-controller.ts +++ b/app/scripts/controllers/authentication/authentication-controller.ts @@ -35,6 +35,11 @@ type SessionData = { expiresIn: string; }; +type MetaMetricsAuth = { + getMetaMetricsId: () => string; + setMetaMetricsId: (serverMetaMetricsId: string | null) => string; +}; + export type AuthenticationControllerState = { /** * Global isSignedIn state. @@ -100,12 +105,21 @@ export default class AuthenticationController extends BaseController< AuthenticationControllerState, AuthenticationControllerMessenger > { + #metametrics: MetaMetricsAuth; + constructor({ messenger, state, + metametrics, }: { messenger: AuthenticationControllerMessenger; state?: AuthenticationControllerState; + + /** + * Not using the Messaging System as we + * do not want to tie this strictly to extension + */ + metametrics: MetaMetricsAuth; }) { super({ messenger, @@ -114,6 +128,8 @@ export default class AuthenticationController extends BaseController< state: { ...defaultState, ...state }, }); + this.#metametrics = metametrics; + this.#registerMessageHandlers(); } @@ -156,6 +172,8 @@ export default class AuthenticationController extends BaseController< public performSignOut(): void { this.#assertLoggedIn(); + this.#metametrics.setMetaMetricsId(null); + this.update((state) => { state.isSignedIn = false; state.sessionData = undefined; @@ -217,7 +235,11 @@ export default class AuthenticationController extends BaseController< // 2. Login const rawMessage = createLoginRawMessage(nonce, publicKey); const signature = await this.#snapSignMessage(rawMessage); - const loginResponse = await login(rawMessage, signature); + const loginResponse = await login( + rawMessage, + signature, + this.#metametrics.getMetaMetricsId(), + ); if (!loginResponse?.token) { throw new Error(`Unable to login`); } @@ -228,6 +250,8 @@ export default class AuthenticationController extends BaseController< metametricsId: loginResponse.profile.metametrics_id, }; + this.#metametrics.setMetaMetricsId(profile.metametricsId); + // 3. Trade for Access Token const accessToken = await getAccessToken(loginResponse.token); if (!accessToken) { diff --git a/app/scripts/controllers/authentication/services.ts b/app/scripts/controllers/authentication/services.ts index 345302e905cd..486cb449c500 100644 --- a/app/scripts/controllers/authentication/services.ts +++ b/app/scripts/controllers/authentication/services.ts @@ -47,6 +47,7 @@ export type LoginResponse = { export async function login( rawMessage: string, signature: string, + clientMetaMetricsId: string, ): Promise { try { const response = await fetch(AUTH_LOGIN_ENDPOINT, { @@ -57,6 +58,10 @@ export async function login( body: JSON.stringify({ signature, raw_message: rawMessage, + metametrics: { + metametrics_id: clientMetaMetricsId, + agent: 'extension', + }, }), }); diff --git a/app/scripts/controllers/metametrics.js b/app/scripts/controllers/metametrics.js index 73e9fd9609df..cc86983178b2 100644 --- a/app/scripts/controllers/metametrics.js +++ b/app/scripts/controllers/metametrics.js @@ -89,6 +89,8 @@ const exceptionsToFilter = { * @typedef {object} MetaMetricsControllerState * @property {string} [metaMetricsId] - The user's metaMetricsId that will be * attached to all non-anonymized event payloads + * @property {string} [serverMetaMetricsId] - A server generated metametrics id that will be + * attached to logged in user events * @property {boolean} [participateInMetaMetrics] - The user's preference for * participating in the MetaMetrics analytics program. This setting controls * whether or not events are tracked @@ -154,6 +156,7 @@ export default class MetaMetricsController { this.store = new ObservableStore({ participateInMetaMetrics: null, metaMetricsId: null, + serverMetaMetricsId: null, eventsBeforeMetricsOptIn: [], traits: {}, previousUserTraits: {}, @@ -497,9 +500,10 @@ export default class MetaMetricsController { ) { return; } - const { metaMetricsId } = this.state; - const idTrait = metaMetricsId ? 'userId' : 'anonymousId'; - const idValue = metaMetricsId ?? METAMETRICS_ANONYMOUS_ID; + const { metaMetricsId, serverMetaMetricsId } = this.state; + const unifiedMetaMetrics = serverMetaMetricsId ?? metaMetricsId; + const idTrait = unifiedMetaMetrics ? 'userId' : 'anonymousId'; + const idValue = unifiedMetaMetrics ?? METAMETRICS_ANONYMOUS_ID; this._submitSegmentAPICall('page', { messageId: buildUniqueMessageId({ actionId }), [idTrait]: idValue, @@ -652,6 +656,21 @@ export default class MetaMetricsController { }); } + // Retrieve (or generate if doesn't exist) the client metametrics id + getClientMetaMetricsId() { + let { metaMetricsId } = this.state; + if (!metaMetricsId) { + metaMetricsId = this.generateMetaMetricsId(); + this.store.updateState({ metaMetricsId }); + } + return metaMetricsId; + } + + // New authentication can return a server metametrics id that we can utilize in our events + setServerMetaMetricsId(metaMetricsId) { + this.store.updateState({ serverMetaMetricsId: metaMetricsId }); + } + /** PRIVATE METHODS */ /** @@ -904,7 +923,8 @@ export default class MetaMetricsController { * @param {object} userTraits */ _identify(userTraits) { - const { metaMetricsId } = this.state; + const { metaMetricsId, serverMetaMetricsId } = this.state; + const unifiedMetaMetrics = serverMetaMetricsId ?? metaMetricsId; if (!userTraits || Object.keys(userTraits).length === 0) { console.warn('MetaMetricsController#_identify: No userTraits found'); @@ -913,7 +933,7 @@ export default class MetaMetricsController { try { this._submitSegmentAPICall('identify', { - userId: metaMetricsId, + userId: unifiedMetaMetrics, traits: userTraits, }); } catch (err) { @@ -990,7 +1010,9 @@ export default class MetaMetricsController { flushImmediately, } = options || {}; let idType = 'userId'; - let idValue = this.state.metaMetricsId; + const unifiedMetaMetrics = + this.state.serverMetaMetricsId ?? this.state.metaMetricsId; + let idValue = unifiedMetaMetrics; let excludeMetaMetricsId = options?.excludeMetaMetricsId ?? false; // This is carried over from the old implementation, and will likely need // to be updated to work with the new tracking plan. I think we should use diff --git a/app/scripts/controllers/metametrics.test.js b/app/scripts/controllers/metametrics.test.js index 9a7540c078f4..daf97fbdc904 100644 --- a/app/scripts/controllers/metametrics.test.js +++ b/app/scripts/controllers/metametrics.test.js @@ -236,6 +236,41 @@ describe('MetaMetricsController', function () { }); }); + describe('getClientMetaMetricsId', function () { + it('should generate or return the metametrics id', function () { + const metaMetricsController = getMetaMetricsController({ + participateInMetaMetrics: true, + metaMetricsId: null, + }); + + // Starts off being empty. + assert.equal(metaMetricsController.state.metaMetricsId, null); + + // Create a new metametrics id. + const clientMetaMetricsId = + metaMetricsController.getClientMetaMetricsId(); + assert.equal(clientMetaMetricsId.startsWith('0x'), true); + + // Return same metametrics id. + const sameMetaMetricsId = metaMetricsController.getClientMetaMetricsId(); + assert.equal(clientMetaMetricsId, sameMetaMetricsId); + }); + }); + + describe('setServerMetaMetricsId', function () { + it('should set a server metametrics id', function () { + const metaMetricsController = getMetaMetricsController(); + assert.equal(metaMetricsController.state.serverMetaMetricsId, null); + + const SERVER_METAMETRICS_ID = '0xServerMetaMetricsId'; + metaMetricsController.setServerMetaMetricsId(SERVER_METAMETRICS_ID); + assert.equal( + metaMetricsController.state.serverMetaMetricsId, + SERVER_METAMETRICS_ID, + ); + }); + }); + describe('identify', function () { it('should call segment.identify for valid traits if user is participating in metametrics', async function () { const metaMetricsController = getMetaMetricsController({ @@ -311,6 +346,26 @@ describe('MetaMetricsController', function () { metaMetricsController.identify(MOCK_INVALID_TRAITS); mock.verify(); }); + + it('should prioritize using the server metametrics id once set', function () { + const metaMetricsController = getMetaMetricsController({ + participateInMetaMetrics: true, + }); + const SERVER_METAMETRICS_ID = '0xServerMetaMetricsId'; + metaMetricsController.setServerMetaMetricsId(SERVER_METAMETRICS_ID); + const mock = sinon.mock(segment); + + // Test Identify + mock.expects('identify').once().withArgs({ + userId: SERVER_METAMETRICS_ID, + traits: MOCK_TRAITS, + messageId: Utils.generateRandomId(), + timestamp: new Date(), + }); + + metaMetricsController.identify(MOCK_TRAITS); + mock.verify(); + }); }); describe('setParticipateInMetaMetrics', function () { @@ -563,6 +618,31 @@ describe('MetaMetricsController', function () { }), ); }); + + it('should prioritize using the server metametrics id once set', function () { + const metaMetricsController = getMetaMetricsController({ + participateInMetaMetrics: true, + }); + const SERVER_METAMETRICS_ID = '0xServerMetaMetricsId'; + metaMetricsController.setServerMetaMetricsId(SERVER_METAMETRICS_ID); + const spy = sinon.spy(segment, 'track'); + + metaMetricsController.submitEvent({ + event: 'Fake Event', + category: 'Unit Test', + }); + + assert.ok( + spy.calledWith({ + event: 'Fake Event', + userId: SERVER_METAMETRICS_ID, + context: DEFAULT_TEST_CONTEXT, + properties: DEFAULT_EVENT_PROPERTIES, + messageId: Utils.generateRandomId(), + timestamp: new Date(), + }), + ); + }); }); describe('Change Transaction XXX anonymous event namnes', function () { @@ -757,6 +837,39 @@ describe('MetaMetricsController', function () { ); mock.verify(); }); + + it('should prioritize using the server metametrics id once set', function () { + const metaMetricsController = getMetaMetricsController({ + participateInMetaMetrics: true, + }); + const SERVER_METAMETRICS_ID = '0xServerMetaMetricsId'; + metaMetricsController.setServerMetaMetricsId(SERVER_METAMETRICS_ID); + const mock = sinon.mock(segment); + + // Test Identify + mock + .expects('page') + .once() + .withArgs({ + name: 'home', + userId: SERVER_METAMETRICS_ID, + context: DEFAULT_TEST_CONTEXT, + properties: { + params: null, + ...DEFAULT_PAGE_PROPERTIES, + }, + messageId: Utils.generateRandomId(), + timestamp: new Date(), + }); + + metaMetricsController.trackPage({ + name: 'home', + params: null, + environmentType: ENVIRONMENT_TYPE_BACKGROUND, + page: METAMETRICS_BACKGROUND_PAGE_OBJECT, + }); + mock.verify(); + }); }); describe('deterministic messageId', function () { diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index a8286d740cac..ab32bdff159a 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -1395,6 +1395,12 @@ export default class MetamaskController extends EventEmitter { name: 'AuthenticationController', allowedActions: ['SnapController:handleRequest'], }), + metametrics: { + getMetaMetricsId: () => + this.metaMetricsController.getClientMetaMetricsId(), + setMetaMetricsId: (newId) => + this.metaMetricsController.setServerMetaMetricsId(newId), + }, }); this.userStorageController = new UserStorageController({ state: initState.UserStorageController,