Skip to content

Commit 56a3582

Browse files
authored
feat: handle primary SRP switch cases (#5478)
## Explanation We discovered that users leverage the "forgot password" flow to switch their primary SRPs in clients. This is breaking auth & user storage, and this PR fixes that so that our controller can handle use cases where the primary SRP changes to another one. Additional changes in clients should be made: - Signing out the user when submitting the forgot password form on extension Please note that no changes in this PR introduces breaking changes. ## References Related to: https://consensyssoftware.atlassian.net/browse/IDENTITY-60 ## Changelog ### `@metamask/profile-sync-controller` - **REMOVED**: `AuthenticationController` doesn't use the `_snapPublicKeyCache` anymore. - **CHANGED**: `UserStorageController` `storageKeyCache` can now hold multiple values. - **CHANGED**: `SDK` `getStorageKey` now takes a `message` argument for cache navigation purposes - **CHANGED**: `SDK` `setStorageKey` now takes an additional `message` argument to leverage cache usages ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes
1 parent 9752bb8 commit 56a3582

File tree

5 files changed

+50
-31
lines changed

5 files changed

+50
-31
lines changed

packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,15 +88,14 @@ describe('authentication/authentication-controller - performSignIn() tests', ()
8888
it('leverages the _snapSignMessageCache', async () => {
8989
const metametrics = createMockAuthMetaMetrics();
9090
const mockEndpoints = arrangeAuthAPIs();
91-
const { messenger, mockSnapGetPublicKey, mockSnapSignMessage } =
91+
const { messenger, mockSnapSignMessage } =
9292
createMockAuthenticationMessenger();
9393

9494
const controller = new AuthenticationController({ messenger, metametrics });
9595

9696
await controller.performSignIn();
9797
controller.performSignOut();
9898
await controller.performSignIn();
99-
expect(mockSnapGetPublicKey).toHaveBeenCalledTimes(1);
10099
expect(mockSnapSignMessage).toHaveBeenCalledTimes(1);
101100
mockEndpoints.mockNonceUrl.done();
102101
mockEndpoints.mockSrpLoginUrl.done();

packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -219,19 +219,20 @@ export default class AuthenticationController extends BaseController<
219219
return null;
220220
}
221221

222-
return {
223-
...this.state.sessionData,
224-
profile: {
225-
...this.state.sessionData.profile,
226-
metaMetricsId: await this.#metametrics.getMetaMetricsId(),
227-
},
228-
};
222+
return this.state.sessionData;
229223
}
230224

231225
async #setLoginResponseToState(loginResponse: LoginResponse) {
226+
const metaMetricsId = await this.#metametrics.getMetaMetricsId();
232227
this.update((state) => {
233228
state.isSignedIn = true;
234-
state.sessionData = loginResponse;
229+
state.sessionData = {
230+
...loginResponse,
231+
profile: {
232+
...loginResponse.profile,
233+
metaMetricsId,
234+
},
235+
};
235236
});
236237
}
237238

@@ -280,27 +281,19 @@ export default class AuthenticationController extends BaseController<
280281
return this.state.isSignedIn;
281282
}
282283

283-
#_snapPublicKeyCache: string | undefined;
284-
285284
/**
286285
* Returns the auth snap public key.
287286
*
288287
* @returns The snap public key.
289288
*/
290289
async #snapGetPublicKey(): Promise<string> {
291-
if (this.#_snapPublicKeyCache) {
292-
return this.#_snapPublicKeyCache;
293-
}
294-
295290
this.#assertIsUnlocked('#snapGetPublicKey');
296291

297292
const result = (await this.messagingSystem.call(
298293
'SnapController:handleRequest',
299294
createSnapPublicKeyRequest(),
300295
)) as string;
301296

302-
this.#_snapPublicKeyCache = result;
303-
304297
return result;
305298
}
306299

packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ export default class UserStorageController extends BaseController<
310310

311311
#isUnlocked = false;
312312

313-
#storageKeyCache: string | null = null;
313+
#storageKeyCache: Record<`metamask:${string}`, string> = {};
314314

315315
readonly #keyringController = {
316316
setupLockedStateSubscriptions: () => {
@@ -377,9 +377,10 @@ export default class UserStorageController extends BaseController<
377377
},
378378
{
379379
storage: {
380-
getStorageKey: async () => this.#storageKeyCache,
381-
setStorageKey: async (key) => {
382-
this.#storageKeyCache = key;
380+
getStorageKey: async (message) =>
381+
this.#storageKeyCache[message] ?? null,
382+
setStorageKey: async (message, key) => {
383+
this.#storageKeyCache[message] = key;
383384
},
384385
},
385386
},
@@ -596,8 +597,13 @@ export default class UserStorageController extends BaseController<
596597
return await this.#userStorage.getStorageKey();
597598
}
598599

600+
/**
601+
* Flushes the storage key cache.
602+
* CAUTION: This is only public for testing purposes.
603+
* It should not be used in production code.
604+
*/
599605
public flushStorageKeyCache(): void {
600-
this.#storageKeyCache = null;
606+
this.#storageKeyCache = {};
601607
}
602608

603609
#_snapSignMessageCache: Record<`metamask:${string}`, string> = {};

packages/profile-sync-controller/src/sdk/user-storage.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,24 @@ describe('User Storage', () => {
534534
);
535535
expect(mockAuthSignMessage).toHaveBeenCalled(); // SignMessage called since generating new key
536536
});
537+
538+
it('uses existing storage key (in storage)', async () => {
539+
const { auth } = arrangeAuth('SRP', MOCK_SRP);
540+
const { userStorage, mockGetStorageKey } = arrangeUserStorage(auth);
541+
mockGetStorageKey.mockResolvedValue(MOCK_STORAGE_KEY);
542+
543+
const mockAuthSignMessage = jest
544+
.spyOn(auth, 'signMessage')
545+
.mockResolvedValue(MOCK_STORAGE_KEY);
546+
547+
handleMockUserStoragePut();
548+
549+
await userStorage.setItem(
550+
`${USER_STORAGE_FEATURE_NAMES.notifications}.notification_settings`,
551+
'some fake data',
552+
);
553+
expect(mockAuthSignMessage).not.toHaveBeenCalled(); // SignMessage not called since key already exists
554+
});
537555
});
538556

539557
/**

packages/profile-sync-controller/src/sdk/user-storage.ts

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ export type UserStorageConfig = {
2222
};
2323

2424
export type StorageOptions = {
25-
getStorageKey: () => Promise<string | null>;
26-
setStorageKey: (val: string) => Promise<void>;
25+
getStorageKey: (message: `metamask:${string}`) => Promise<string | null>;
26+
setStorageKey: (message: `metamask:${string}`, val: string) => Promise<void>;
2727
};
2828

2929
export type UserStorageOptions = {
@@ -110,17 +110,20 @@ export class UserStorage {
110110
}
111111

112112
async getStorageKey(): Promise<string> {
113-
const storageKey = await this.options.storage?.getStorageKey();
113+
const userProfile = await this.config.auth.getUserProfile();
114+
const message = `metamask:${userProfile.profileId}` as const;
115+
116+
const storageKey = await this.options.storage?.getStorageKey(message);
114117
if (storageKey) {
115118
return storageKey;
116119
}
117120

118-
const userProfile = await this.config.auth.getUserProfile();
119-
const storageKeySignature = await this.config.auth.signMessage(
120-
`metamask:${userProfile.profileId}`,
121-
);
121+
const storageKeySignature = await this.config.auth.signMessage(message);
122122
const hashedStorageKeySignature = createSHA256Hash(storageKeySignature);
123-
await this.options.storage?.setStorageKey(hashedStorageKeySignature);
123+
await this.options.storage?.setStorageKey(
124+
message,
125+
hashedStorageKeySignature,
126+
);
124127
return hashedStorageKeySignature;
125128
}
126129

0 commit comments

Comments
 (0)