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

feat: add server metametrics id #24479

Closed
wants to merge 3 commits into from
Closed

feat: add server metametrics id #24479

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
66670fd
feat: add server metametrics id
Prithpal-Sooriya May 10, 2024
351bdb2
test: add metametrics tests
Prithpal-Sooriya May 14, 2024
fb074a5
Merge branch 'develop' of github.com:MetaMask/metamask-extension into…
Prithpal-Sooriya May 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 32 additions & 2 deletions app/scripts/controllers/authentication/authentication-controller.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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();
Expand Down Expand Up @@ -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);
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -172,6 +185,7 @@ describe('authentication/authentication-controller - getBearerToken() tests', ()
const controller = new AuthenticationController({
messenger,
state: originalState,
metametrics,
});

const result = await controller.getBearerToken();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -219,6 +238,7 @@ describe('authentication/authentication-controller - getSessionProfile() tests',
const controller = new AuthenticationController({
messenger,
state: originalState,
metametrics,
});

const result = await controller.getSessionProfile();
Expand Down Expand Up @@ -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,
};
}
26 changes: 25 additions & 1 deletion app/scripts/controllers/authentication/authentication-controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ type SessionData = {
expiresIn: string;
};

type MetaMetricsAuth = {
getMetaMetricsId: () => string;
setMetaMetricsId: (serverMetaMetricsId: string | null) => string;
};

export type AuthenticationControllerState = {
/**
* Global isSignedIn state.
Expand Down Expand Up @@ -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;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As comment mentioned, we need this to be used on Mobile. Seeing that the metametrics controller is still inside the extension, my guess is that mobile uses a different approach.

}) {
super({
messenger,
Expand All @@ -114,6 +128,8 @@ export default class AuthenticationController extends BaseController<
state: { ...defaultState, ...state },
});

this.#metametrics = metametrics;

this.#registerMessageHandlers();
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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`);
}
Expand All @@ -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) {
Expand Down
5 changes: 5 additions & 0 deletions app/scripts/controllers/authentication/services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export type LoginResponse = {
export async function login(
rawMessage: string,
signature: string,
clientMetaMetricsId: string,
): Promise<LoginResponse | null> {
try {
const response = await fetch(AUTH_LOGIN_ENDPOINT, {
Expand All @@ -57,6 +58,10 @@ export async function login(
body: JSON.stringify({
signature,
raw_message: rawMessage,
metametrics: {
metametrics_id: clientMetaMetricsId,
agent: 'extension',
},
}),
});

Expand Down
34 changes: 28 additions & 6 deletions app/scripts/controllers/metametrics.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -154,6 +156,7 @@ export default class MetaMetricsController {
this.store = new ObservableStore({
participateInMetaMetrics: null,
metaMetricsId: null,
serverMetaMetricsId: null,
eventsBeforeMetricsOptIn: [],
traits: {},
previousUserTraits: {},
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 */

/**
Expand Down Expand Up @@ -904,7 +923,8 @@ export default class MetaMetricsController {
* @param {object} userTraits
*/
_identify(userTraits) {
const { metaMetricsId } = this.state;
const { metaMetricsId, serverMetaMetricsId } = this.state;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Effected Segment Events:

  • Identify
  • TrackPage
  • TrackEvent

const unifiedMetaMetrics = serverMetaMetricsId ?? metaMetricsId;

if (!userTraits || Object.keys(userTraits).length === 0) {
console.warn('MetaMetricsController#_identify: No userTraits found');
Expand All @@ -913,7 +933,7 @@ export default class MetaMetricsController {

try {
this._submitSegmentAPICall('identify', {
userId: metaMetricsId,
userId: unifiedMetaMetrics,
traits: userTraits,
});
} catch (err) {
Expand Down Expand Up @@ -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
Expand Down