From 564be26a16bea551d7d328578089be48a031a4ee Mon Sep 17 00:00:00 2001 From: lwin Date: Mon, 25 Aug 2025 08:34:54 +0800 Subject: [PATCH 1/6] feat: export seedless-onboarding-controller allowed events/actions --- .../src/SeedlessOnboardingController.test.ts | 6 +++--- .../seedless-onboarding-controller/src/index.ts | 2 ++ .../seedless-onboarding-controller/src/types.ts | 14 ++++++++------ .../tests/__fixtures__/mockMessenger.ts | 8 ++++---- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts index 9f350689f87..6febfa618d4 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts @@ -44,8 +44,8 @@ import { SeedlessOnboardingController, } from './SeedlessOnboardingController'; import type { - AllowedActions, - AllowedEvents, + SeedlessOnboardingControllerAllowedActions, + SeedlessOnboardingControllerAllowedEvents, SeedlessOnboardingControllerMessenger, SeedlessOnboardingControllerOptions, SeedlessOnboardingControllerState, @@ -115,7 +115,7 @@ type WithControllerCallback = ({ encryptor: VaultEncryptor; initialState: SeedlessOnboardingControllerState; messenger: SeedlessOnboardingControllerMessenger; - baseMessenger: Messenger; + baseMessenger: Messenger; toprfClient: ToprfSecureBackup; mockRefreshJWTToken: jest.Mock; mockRevokeRefreshToken: jest.Mock; diff --git a/packages/seedless-onboarding-controller/src/index.ts b/packages/seedless-onboarding-controller/src/index.ts index 4d445795530..721892ab225 100644 --- a/packages/seedless-onboarding-controller/src/index.ts +++ b/packages/seedless-onboarding-controller/src/index.ts @@ -11,7 +11,9 @@ export type { SeedlessOnboardingControllerGetStateAction, SeedlessOnboardingControllerStateChangeEvent, SeedlessOnboardingControllerActions, + SeedlessOnboardingControllerAllowedActions, SeedlessOnboardingControllerEvents, + SeedlessOnboardingControllerAllowedEvents, ToprfKeyDeriver, RecoveryErrorData, } from './types'; diff --git a/packages/seedless-onboarding-controller/src/types.ts b/packages/seedless-onboarding-controller/src/types.ts index d871eeb2581..f3d0b81a4cb 100644 --- a/packages/seedless-onboarding-controller/src/types.ts +++ b/packages/seedless-onboarding-controller/src/types.ts @@ -186,7 +186,7 @@ export type SeedlessOnboardingControllerGetStateAction = export type SeedlessOnboardingControllerActions = SeedlessOnboardingControllerGetStateAction; -export type AllowedActions = never; +export type SeedlessOnboardingControllerAllowedActions = never; // Events export type SeedlessOnboardingControllerStateChangeEvent = @@ -197,17 +197,19 @@ export type SeedlessOnboardingControllerStateChangeEvent = export type SeedlessOnboardingControllerEvents = SeedlessOnboardingControllerStateChangeEvent; -export type AllowedEvents = +export type SeedlessOnboardingControllerAllowedEvents = | KeyringControllerLockEvent | KeyringControllerUnlockEvent; // Messenger export type SeedlessOnboardingControllerMessenger = RestrictedMessenger< typeof controllerName, - SeedlessOnboardingControllerActions | AllowedActions, - SeedlessOnboardingControllerEvents | AllowedEvents, - AllowedActions['type'], - AllowedEvents['type'] + | SeedlessOnboardingControllerActions + | SeedlessOnboardingControllerAllowedActions, + | SeedlessOnboardingControllerEvents + | SeedlessOnboardingControllerAllowedEvents, + SeedlessOnboardingControllerAllowedActions['type'], + SeedlessOnboardingControllerAllowedEvents['type'] >; /** diff --git a/packages/seedless-onboarding-controller/tests/__fixtures__/mockMessenger.ts b/packages/seedless-onboarding-controller/tests/__fixtures__/mockMessenger.ts index 01a7124e147..a11ab2fcce5 100644 --- a/packages/seedless-onboarding-controller/tests/__fixtures__/mockMessenger.ts +++ b/packages/seedless-onboarding-controller/tests/__fixtures__/mockMessenger.ts @@ -1,8 +1,8 @@ import { Messenger } from '@metamask/base-controller'; import type { - AllowedActions, - AllowedEvents, + SeedlessOnboardingControllerAllowedActions, + SeedlessOnboardingControllerAllowedEvents, SeedlessOnboardingControllerMessenger, } from '../../src/types'; @@ -12,7 +12,7 @@ import type { * @returns base messenger, and messenger. You can pass this into the mocks below to mock messenger calls */ export function createCustomSeedlessOnboardingMessenger() { - const baseMessenger = new Messenger(); + const baseMessenger = new Messenger(); const messenger = baseMessenger.getRestricted({ name: 'SeedlessOnboardingController', allowedActions: [], @@ -26,7 +26,7 @@ export function createCustomSeedlessOnboardingMessenger() { } type OverrideMessengers = { - baseMessenger: Messenger; + baseMessenger: Messenger; messenger: SeedlessOnboardingControllerMessenger; }; From 17a2e5adddd4925c93684a20e0ea7aa79dd9bf64 Mon Sep 17 00:00:00 2001 From: lwin Date: Mon, 25 Aug 2025 09:27:43 +0800 Subject: [PATCH 2/6] chore: refactor vault lock/unlock operations --- .../src/SeedlessOnboardingController.test.ts | 39 ++- .../src/SeedlessOnboardingController.ts | 226 ++++++------------ .../src/constants.ts | 1 - .../src/types.ts | 15 +- .../src/utils.ts | 83 ++++++- .../tests/__fixtures__/mockMessenger.ts | 10 +- 6 files changed, 211 insertions(+), 163 deletions(-) diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts index 6febfa618d4..1af581e6ef5 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts @@ -115,7 +115,10 @@ type WithControllerCallback = ({ encryptor: VaultEncryptor; initialState: SeedlessOnboardingControllerState; messenger: SeedlessOnboardingControllerMessenger; - baseMessenger: Messenger; + baseMessenger: Messenger< + SeedlessOnboardingControllerAllowedActions, + SeedlessOnboardingControllerAllowedEvents + >; toprfClient: ToprfSecureBackup; mockRefreshJWTToken: jest.Mock; mockRevokeRefreshToken: jest.Mock; @@ -2321,6 +2324,38 @@ describe('SeedlessOnboardingController', () => { describe('submitPassword', () => { const MOCK_PASSWORD = 'mock-password'; + it('should be able to unlock the vault with password', async () => { + const mockToprfEncryptor = createMockToprfEncryptor(); + + const MOCK_ENCRYPTION_KEY = + mockToprfEncryptor.deriveEncKey(MOCK_PASSWORD); + const MOCK_PASSWORD_ENCRYPTION_KEY = + mockToprfEncryptor.derivePwEncKey(MOCK_PASSWORD); + const MOCK_AUTH_KEY_PAIR = + mockToprfEncryptor.deriveAuthKeyPair(MOCK_PASSWORD); + + const mockResult = await createMockVault( + MOCK_ENCRYPTION_KEY, + MOCK_PASSWORD_ENCRYPTION_KEY, + MOCK_AUTH_KEY_PAIR, + MOCK_PASSWORD, + ); + + const mockVault = mockResult.encryptedMockVault; + await withController( + { + state: { + vault: mockVault, + }, + }, + async ({ controller }) => { + await controller.submitPassword(MOCK_PASSWORD); + + expect(controller.state.vault).toBe(mockVault); + }, + ); + }); + it('should throw error if the vault is missing', async () => { await withController(async ({ controller }) => { await expect(controller.submitPassword(MOCK_PASSWORD)).rejects.toThrow( @@ -3409,7 +3444,7 @@ describe('SeedlessOnboardingController', () => { await expect( controller.storeKeyringEncryptionKey(''), ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.VaultEncryptionKeyUndefined, + SeedlessOnboardingControllerErrorMessage.WrongPasswordType, ); // Setup and store keyring encryption key. diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts index 2b9406fca91..6aa576a50d8 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts @@ -11,7 +11,7 @@ import { TOPRFErrorCode, TOPRFError, } from '@metamask/toprf-secure-backup'; -import { base64ToBytes, bytesToBase64, bigIntToHex } from '@metamask/utils'; +import { base64ToBytes, bytesToBase64 } from '@metamask/utils'; import { gcm } from '@noble/ciphers/aes'; import { bytesToUtf8, utf8ToBytes } from '@noble/ciphers/utils'; import { managedNonce } from '@noble/ciphers/webcrypto'; @@ -45,8 +45,15 @@ import type { VaultEncryptor, RefreshJWTToken, RevokeRefreshToken, + VaultData, + DeserializedVaultData, } from './types'; -import { decodeJWTToken, decodeNodeAuthToken } from './utils'; +import { + decodeJWTToken, + decodeNodeAuthToken, + deserializeVaultData, + serializeVaultData, +} from './utils'; const log = createModuleLogger(projectLogger, controllerName); @@ -182,6 +189,11 @@ export class SeedlessOnboardingController extends BaseController< readonly #revokeRefreshToken: RevokeRefreshToken; + /** + * The TTL of the password outdated cache in milliseconds. + */ + readonly #passwordOutdatedCacheTTL: number; + /** * Controller lock state. * @@ -189,11 +201,6 @@ export class SeedlessOnboardingController extends BaseController< */ #isUnlocked = false; - /** - * The TTL of the password outdated cache in milliseconds. - */ - readonly #passwordOutdatedCacheTTL: number; - /** * Creates a new SeedlessOnboardingController instance. * @@ -664,7 +671,7 @@ export class SeedlessOnboardingController extends BaseController< */ async submitPassword(password: string): Promise { return await this.#withControllerLock(async () => { - await this.#unlockVaultAndGetVaultData(password); + await this.#unlockVaultAndGetVaultData({ password }); this.#setUnlocked(); }); } @@ -785,7 +792,9 @@ export class SeedlessOnboardingController extends BaseController< const vaultKey = await this.#loadSeedlessEncryptionKey(pwEncKey); // Unlock the controller - await this.#unlockVaultAndGetVaultData(undefined, vaultKey); + await this.#unlockVaultAndGetVaultData({ + encryptionKey: vaultKey, + }); this.#setUnlocked(); } catch (error) { if (this.#isTokenExpiredError(error)) { @@ -903,8 +912,10 @@ export class SeedlessOnboardingController extends BaseController< ); } - const { parsedVaultData } = await this.#decryptAndParseVaultData(password); - return parsedVaultData.accessToken; + const { vaultData } = await this.#decryptAndParseVaultData({ + password, + }); + return vaultData.accessToken; } #setUnlocked(): void { @@ -1161,7 +1172,7 @@ export class SeedlessOnboardingController extends BaseController< toprfEncryptionKey: encKey, toprfPwEncryptionKey: pwEncKey, toprfAuthKeyPair: authKeyPair, - } = await this.#unlockVaultAndGetVaultData(oldPassword)); + } = await this.#unlockVaultAndGetVaultData({ password: oldPassword })); } const result = await this.toprfClient.changeEncKey({ nodeAuthTokens: this.state.nodeAuthTokens, @@ -1247,8 +1258,9 @@ export class SeedlessOnboardingController extends BaseController< * Unlocks the encrypted vault using the provided password and returns the decrypted vault data. * This method ensures thread-safety by using a mutex lock when accessing the vault. * - * @param password - The optional password to unlock the vault. - * @param encryptionKey - The optional encryption key to unlock the vault. + * @param params - The parameters for unlocking the vault. + * @param params.password - The optional password to unlock the vault. + * @param params.encryptionKey - The optional encryption key to unlock the vault. * @returns A promise that resolves to an object containing: * - toprfEncryptionKey: The decrypted TOPRF encryption key * - toprfAuthKeyPair: The decrypted TOPRF authentication key pair @@ -1260,53 +1272,41 @@ export class SeedlessOnboardingController extends BaseController< * - The password is incorrect (from encryptor.decrypt) * - The decrypted vault data is malformed */ - async #unlockVaultAndGetVaultData( - password?: string, - encryptionKey?: string, - ): Promise<{ - toprfEncryptionKey: Uint8Array; - toprfPwEncryptionKey: Uint8Array; - toprfAuthKeyPair: KeyPair; - revokeToken?: string; - accessToken: string; - }> { + async #unlockVaultAndGetVaultData(params?: { + password?: string; + encryptionKey?: string; + }): Promise { return this.#withVaultLock(async () => { - const { parsedVaultData, vaultEncryptionKey, vaultEncryptionSalt } = - await this.#decryptAndParseVaultData(password, encryptionKey); - - const { - toprfEncryptionKey, - toprfPwEncryptionKey, - toprfAuthKeyPair, - revokeToken, - accessToken, - } = parsedVaultData; + const { vaultData, vaultEncryptionKey, vaultEncryptionSalt } = + await this.#decryptAndParseVaultData(params); this.update((state) => { state.vaultEncryptionKey = vaultEncryptionKey; state.vaultEncryptionSalt = vaultEncryptionSalt; - state.revokeToken = revokeToken; - state.accessToken = accessToken; + state.revokeToken = vaultData.revokeToken; + state.accessToken = vaultData.accessToken; }); - return { - toprfEncryptionKey, - toprfPwEncryptionKey, - toprfAuthKeyPair, - revokeToken, - accessToken, - }; + return deserializeVaultData(vaultData); }); } /** * Decrypts the vault data and parses it into a usable format. * - * @param password - The optional password to decrypt the vault. - * @param encryptionKey - The optional encryption key to decrypt the vault. + * @param params - The parameters for decrypting the vault. + * @param params.password - The optional password to decrypt the vault. + * @param params.encryptionKey - The optional encryption key to decrypt the vault. * @returns A promise that resolves to an object containing: */ - async #decryptAndParseVaultData(password?: string, encryptionKey?: string) { + async #decryptAndParseVaultData(params?: { + password?: string; + encryptionKey?: string; + }): Promise<{ + vaultData: VaultData; + vaultEncryptionKey: string; + vaultEncryptionSalt?: string; + }> { let { vaultEncryptionKey, vaultEncryptionSalt } = this.state; const { vault: encryptedVault } = this.state; @@ -1314,27 +1314,14 @@ export class SeedlessOnboardingController extends BaseController< throw new Error(SeedlessOnboardingControllerErrorMessage.VaultError); } - if (encryptionKey) { - vaultEncryptionKey = encryptionKey; + if (params?.encryptionKey) { + vaultEncryptionKey = params.encryptionKey; } let decryptedVaultData: unknown; - if (password) { - assertIsValidPassword(password); - // Note that vault decryption using the password is a very costly operation as it involves deriving the encryption key - // from the password using an intentionally slow key derivation function. - // We should make sure that we only call it very intentionally. - const result = await this.#vaultEncryptor.decryptWithDetail( - password, - encryptedVault, - ); - decryptedVaultData = result.vault; - vaultEncryptionKey = result.exportedKeyString; - vaultEncryptionSalt = result.salt; - } else { - assertIsVaultEncryptionKeyDefined(vaultEncryptionKey); - + // if the encryption key is available, we will use it to decrypt the vault + if (vaultEncryptionKey) { const parsedEncryptedVault = JSON.parse(encryptedVault); if ( @@ -1351,12 +1338,25 @@ export class SeedlessOnboardingController extends BaseController< key, parsedEncryptedVault, ); + } else { + // if the encryption key is not available, we will use the password to decrypt the vault + assertIsValidPassword(params?.password); + // Note that vault decryption using the password is a very costly operation as it involves deriving the encryption key + // from the password using an intentionally slow key derivation function. + // We should make sure that we only call it very intentionally. + const result = await this.#vaultEncryptor.decryptWithDetail( + params.password, + encryptedVault, + ); + decryptedVaultData = result.vault; + vaultEncryptionKey = result.exportedKeyString; + vaultEncryptionSalt = result.salt; } - const parsedVaultData = this.#parseVaultData(decryptedVaultData); + const vaultData = this.#parseVaultData(decryptedVaultData); return { - parsedVaultData, + vaultData, vaultEncryptionKey, vaultEncryptionSalt, }; @@ -1480,18 +1480,10 @@ export class SeedlessOnboardingController extends BaseController< const { revokeToken } = this.state; const accessToken = await this.#getAccessToken(password); - - const { toprfEncryptionKey, toprfPwEncryptionKey, toprfAuthKeyPair } = - this.#serializeKeyData( - rawToprfEncryptionKey, - rawToprfPwEncryptionKey, - rawToprfAuthKeyPair, - ); - - const serializedVaultData = JSON.stringify({ - toprfEncryptionKey, - toprfPwEncryptionKey, - toprfAuthKeyPair, + const serializedVaultData = serializeVaultData({ + toprfAuthKeyPair: rawToprfAuthKeyPair, + toprfEncryptionKey: rawToprfEncryptionKey, + toprfPwEncryptionKey: rawToprfPwEncryptionKey, revokeToken, accessToken, }); @@ -1588,37 +1580,6 @@ export class SeedlessOnboardingController extends BaseController< return await withLock(this.#vaultOperationMutex, callback); } - /** - * Serialize the encryption key and authentication key pair. - * - * @param encKey - The encryption key to serialize. - * @param pwEncKey - The password encryption key to serialize. - * @param authKeyPair - The authentication key pair to serialize. - * @returns The serialized encryption key and authentication key pair. - */ - #serializeKeyData( - encKey: Uint8Array, - pwEncKey: Uint8Array, - authKeyPair: KeyPair, - ): { - toprfEncryptionKey: string; - toprfPwEncryptionKey: string; - toprfAuthKeyPair: string; - } { - const b64EncodedEncKey = bytesToBase64(encKey); - const b64EncodedPwEncKey = bytesToBase64(pwEncKey); - const b64EncodedAuthKeyPair = JSON.stringify({ - sk: bigIntToHex(authKeyPair.sk), // Convert BigInt to hex string - pk: bytesToBase64(authKeyPair.pk), - }); - - return { - toprfEncryptionKey: b64EncodedEncKey, - toprfPwEncryptionKey: b64EncodedPwEncKey, - toprfAuthKeyPair: b64EncodedAuthKeyPair, - }; - } - /** * Parse and deserialize the authentication data from the vault. * @@ -1626,13 +1587,7 @@ export class SeedlessOnboardingController extends BaseController< * @returns The parsed authentication data. * @throws If the vault data is not valid. */ - #parseVaultData(data: unknown): { - toprfEncryptionKey: Uint8Array; - toprfPwEncryptionKey: Uint8Array; - toprfAuthKeyPair: KeyPair; - revokeToken?: string; - accessToken: string; - } { + #parseVaultData(data: unknown): VaultData { if (typeof data !== 'string') { throw new Error( SeedlessOnboardingControllerErrorMessage.InvalidVaultData, @@ -1650,25 +1605,7 @@ export class SeedlessOnboardingController extends BaseController< assertIsValidVaultData(parsedVaultData); - const rawToprfEncryptionKey = base64ToBytes( - parsedVaultData.toprfEncryptionKey, - ); - const rawToprfPwEncryptionKey = base64ToBytes( - parsedVaultData.toprfPwEncryptionKey, - ); - const parsedToprfAuthKeyPair = JSON.parse(parsedVaultData.toprfAuthKeyPair); - const rawToprfAuthKeyPair = { - sk: BigInt(parsedToprfAuthKeyPair.sk), - pk: base64ToBytes(parsedToprfAuthKeyPair.pk), - }; - - return { - toprfEncryptionKey: rawToprfEncryptionKey, - toprfPwEncryptionKey: rawToprfPwEncryptionKey, - toprfAuthKeyPair: rawToprfAuthKeyPair, - revokeToken: parsedVaultData.revokeToken, - accessToken: parsedVaultData.accessToken, - }; + return parsedVaultData; } #assertIsUnlocked(): void { @@ -1827,7 +1764,10 @@ export class SeedlessOnboardingController extends BaseController< toprfPwEncryptionKey: rawToprfPwEncryptionKey, toprfAuthKeyPair: rawToprfAuthKeyPair, revokeToken, - } = await this.#unlockVaultAndGetVaultData(password, vaultEncryptionKey); + } = await this.#unlockVaultAndGetVaultData({ + password, + encryptionKey: vaultEncryptionKey, + }); if (!revokeToken) { throw new Error( SeedlessOnboardingControllerErrorMessage.InvalidRevokeToken, @@ -2083,19 +2023,3 @@ function assertIsEncryptedSeedlessEncryptionKeySet( ); } } - -/** - * Assert that the provided vault encryption key is a valid non-empty string. - * - * @param vaultEncryptionKey - The vault encryption key to check. - * @throws If the vault encryption key is not a valid string. - */ -function assertIsVaultEncryptionKeyDefined( - vaultEncryptionKey: string | undefined, -): asserts vaultEncryptionKey is string { - if (!vaultEncryptionKey) { - throw new Error( - SeedlessOnboardingControllerErrorMessage.VaultEncryptionKeyUndefined, - ); - } -} diff --git a/packages/seedless-onboarding-controller/src/constants.ts b/packages/seedless-onboarding-controller/src/constants.ts index 2a6f4ed9552..580f53deefe 100644 --- a/packages/seedless-onboarding-controller/src/constants.ts +++ b/packages/seedless-onboarding-controller/src/constants.ts @@ -56,7 +56,6 @@ export enum SeedlessOnboardingControllerErrorMessage { SRPNotBackedUpError = `${controllerName} - SRP not backed up`, EncryptedKeyringEncryptionKeyNotSet = `${controllerName} - Encrypted keyring encryption key is not set`, EncryptedSeedlessEncryptionKeyNotSet = `${controllerName} - Encrypted seedless encryption key is not set`, - VaultEncryptionKeyUndefined = `${controllerName} - Vault encryption key is not available`, MaxKeyChainLengthExceeded = `${controllerName} - Max key chain length exceeded`, FailedToFetchAuthPubKey = `${controllerName} - Failed to fetch latest auth pub key`, InvalidPasswordOutdatedCache = `${controllerName} - Invalid password outdated cache provided.`, diff --git a/packages/seedless-onboarding-controller/src/types.ts b/packages/seedless-onboarding-controller/src/types.ts index f3d0b81a4cb..b7c6d1acb3d 100644 --- a/packages/seedless-onboarding-controller/src/types.ts +++ b/packages/seedless-onboarding-controller/src/types.ts @@ -6,7 +6,7 @@ import type { KeyringControllerLockEvent, KeyringControllerUnlockEvent, } from '@metamask/keyring-controller'; -import type { NodeAuthTokens } from '@metamask/toprf-secure-backup'; +import type { KeyPair, NodeAuthTokens } from '@metamask/toprf-secure-backup'; import type { MutexInterface } from 'async-mutex'; import type { @@ -330,10 +330,6 @@ export type MutuallyExclusiveCallback = ({ * The structure of the data which is serialized and stored in the vault. */ export type VaultData = { - /** - * The node auth tokens from OAuth User authentication after the Social login. - */ - authTokens: NodeAuthTokens; /** * The encryption key to encrypt the seed phrase. */ @@ -357,6 +353,15 @@ export type VaultData = { accessToken: string; }; +export type DeserializedVaultData = Pick< + VaultData, + 'accessToken' | 'revokeToken' +> & { + toprfEncryptionKey: Uint8Array; + toprfPwEncryptionKey: Uint8Array; + toprfAuthKeyPair: KeyPair; +}; + export type SecretDataType = Uint8Array | string | number; /** diff --git a/packages/seedless-onboarding-controller/src/utils.ts b/packages/seedless-onboarding-controller/src/utils.ts index e15129730e1..b769c9f9f76 100644 --- a/packages/seedless-onboarding-controller/src/utils.ts +++ b/packages/seedless-onboarding-controller/src/utils.ts @@ -1,7 +1,18 @@ -import { base64ToBytes } from '@metamask/utils'; +import type { KeyPair } from '@metamask/toprf-secure-backup'; +import { + base64ToBytes, + bigIntToHex, + bytesToBase64, + hexToBigInt, +} from '@metamask/utils'; import { bytesToUtf8 } from '@noble/ciphers/utils'; -import type { DecodedBaseJWTToken, DecodedNodeAuthToken } from './types'; +import type { + DecodedBaseJWTToken, + DecodedNodeAuthToken, + DeserializedVaultData, + VaultData, +} from './types'; /** * Decode the node auth token from base64 to json object. @@ -33,3 +44,71 @@ export function decodeJWTToken(token: string): DecodedBaseJWTToken { const decoded = JSON.parse(bytesToUtf8(base64ToBytes(paddedPayload))); return decoded as DecodedBaseJWTToken; } + +/** + * Serialize the vault data. + * + * @param data - The vault data to serialize. + * @returns The serialized vault data. + */ +export function serializeVaultData(data: DeserializedVaultData): string { + const toprfEncryptionKey = bytesToBase64(data.toprfEncryptionKey); + const toprfPwEncryptionKey = bytesToBase64(data.toprfPwEncryptionKey); + const toprfAuthKeyPair = serializeToprfAuthKeyPair(data.toprfAuthKeyPair); + + return JSON.stringify({ + toprfEncryptionKey, + toprfPwEncryptionKey, + toprfAuthKeyPair, + revokeToken: data.revokeToken, + accessToken: data.accessToken, + }); +} + +/** + * Deserialize the vault data. + * + * @param value - The stringified vault data. + * @returns The deserialized vault data. + */ +export function deserializeVaultData(value: VaultData): DeserializedVaultData { + const toprfEncryptionKey = base64ToBytes(value.toprfEncryptionKey); + const toprfPwEncryptionKey = base64ToBytes(value.toprfPwEncryptionKey); + const toprfAuthKeyPair = deserializeAuthKeyPair(value.toprfAuthKeyPair); + + return { + ...value, + toprfEncryptionKey, + toprfPwEncryptionKey, + toprfAuthKeyPair, + }; +} + +/** + * Serialize TOPRF authentication key pair. + * + * @param keyPair - The authentication key pair to serialize. + * @returns The serialized authentication key pair. + */ +export function serializeToprfAuthKeyPair(keyPair: KeyPair): string { + const b64EncodedAuthKeyPair = JSON.stringify({ + sk: bigIntToHex(keyPair.sk), // Convert BigInt to hex string + pk: bytesToBase64(keyPair.pk), + }); + + return b64EncodedAuthKeyPair; +} + +/** + * Deserialize the authentication key pair. + * + * @param value - The stringified authentication key pair. + * @returns The deserialized authentication key pair. + */ +export function deserializeAuthKeyPair(value: string): KeyPair { + const parsedKeyPair = JSON.parse(value); + return { + sk: hexToBigInt(parsedKeyPair.sk), + pk: base64ToBytes(parsedKeyPair.pk), + }; +} diff --git a/packages/seedless-onboarding-controller/tests/__fixtures__/mockMessenger.ts b/packages/seedless-onboarding-controller/tests/__fixtures__/mockMessenger.ts index a11ab2fcce5..c5ec606047c 100644 --- a/packages/seedless-onboarding-controller/tests/__fixtures__/mockMessenger.ts +++ b/packages/seedless-onboarding-controller/tests/__fixtures__/mockMessenger.ts @@ -12,7 +12,10 @@ import type { * @returns base messenger, and messenger. You can pass this into the mocks below to mock messenger calls */ export function createCustomSeedlessOnboardingMessenger() { - const baseMessenger = new Messenger(); + const baseMessenger = new Messenger< + SeedlessOnboardingControllerAllowedActions, + SeedlessOnboardingControllerAllowedEvents + >(); const messenger = baseMessenger.getRestricted({ name: 'SeedlessOnboardingController', allowedActions: [], @@ -26,7 +29,10 @@ export function createCustomSeedlessOnboardingMessenger() { } type OverrideMessengers = { - baseMessenger: Messenger; + baseMessenger: Messenger< + SeedlessOnboardingControllerAllowedActions, + SeedlessOnboardingControllerAllowedEvents + >; messenger: SeedlessOnboardingControllerMessenger; }; From 6e6828a0a7ac7d79f6d09df62748fa8634e624d4 Mon Sep 17 00:00:00 2001 From: lwin Date: Wed, 27 Aug 2025 10:09:39 +0800 Subject: [PATCH 3/6] feat: cached decrypted vault --- .../src/SeedlessOnboardingController.test.ts | 237 ++++++------------ .../src/SeedlessOnboardingController.ts | 34 ++- 2 files changed, 101 insertions(+), 170 deletions(-) diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts index 1af581e6ef5..51a37930e65 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts @@ -1605,64 +1605,16 @@ describe('SeedlessOnboardingController', () => { ); }); - it('should throw an error if failed to parse vault data', async () => { + it('should throw error if encryptionSalt is different from the one in the vault', async () => { await withController( { state: getMockInitialControllerState({ withMockAuthenticatedUser: true, - withMockAuthPubKey: true, - vault: MOCK_VAULT, vaultEncryptionKey: MOCK_VAULT_ENCRYPTION_KEY, vaultEncryptionSalt: MOCK_VAULT_ENCRYPTION_SALT, }), }, - async ({ controller, encryptor, toprfClient }) => { - await controller.submitPassword(MOCK_PASSWORD); - - mockFetchAuthPubKey( - toprfClient, - base64ToBytes(controller.state.authPubKey as string), - ); - - jest - .spyOn(encryptor, 'decryptWithKey') - .mockResolvedValueOnce('{ "foo": "bar"'); - await expect( - controller.addNewSecretData( - NEW_KEY_RING_1.seedPhrase, - SecretType.Mnemonic, - { - keyringId: NEW_KEY_RING_1.id, - }, - ), - ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.InvalidVaultData, - ); - }, - ); - }); - - it('should throw error if encryptionSalt is different from the one in the vault', async () => { - await withController( - { - state: getMockInitialControllerState({ - withMockAuthenticatedUser: true, - }), - }, - async ({ controller, toprfClient }) => { - await mockCreateToprfKeyAndBackupSeedPhrase( - toprfClient, - controller, - MOCK_PASSWORD, - MOCK_SEED_PHRASE, - MOCK_KEYRING_ID, - ); - - mockFetchAuthPubKey( - toprfClient, - base64ToBytes(controller.state.authPubKey as string), - ); - + async ({ controller }) => { // intentionally mock the JSON.parse to return an object with a different salt jest.spyOn(global.JSON, 'parse').mockReturnValueOnce({ salt: 'different-salt', @@ -1683,119 +1635,6 @@ describe('SeedlessOnboardingController', () => { ); }); - it('should throw an error if vault unlocked has an unexpected shape', async () => { - await withController( - { - state: getMockInitialControllerState({ - withMockAuthenticatedUser: true, - vault: MOCK_VAULT, - }), - }, - async ({ controller, toprfClient, encryptor }) => { - mockcreateLocalKey(toprfClient, MOCK_PASSWORD); - - // persist the local enc key - jest.spyOn(toprfClient, 'persistLocalKey').mockResolvedValueOnce(); - // encrypt and store the secret data - handleMockSecretDataAdd(); - - jest.spyOn(encryptor, 'encryptWithDetail').mockResolvedValueOnce({ - vault: MOCK_VAULT, - exportedKeyString: MOCK_VAULT_ENCRYPTION_KEY, - }); - - await controller.createToprfKeyAndBackupSeedPhrase( - MOCK_PASSWORD, - NEW_KEY_RING_1.seedPhrase, - NEW_KEY_RING_1.id, - ); - - mockFetchAuthPubKey( - toprfClient, - base64ToBytes(controller.state.authPubKey as string), - ); - - jest - .spyOn(encryptor, 'decryptWithKey') - .mockResolvedValueOnce({ foo: 'bar' }); - await expect( - controller.addNewSecretData( - NEW_KEY_RING_2.seedPhrase, - SecretType.Mnemonic, - { - keyringId: NEW_KEY_RING_2.id, - }, - ), - ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.InvalidVaultData, - ); - - jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce('null'); - await expect( - controller.addNewSecretData( - NEW_KEY_RING_2.seedPhrase, - SecretType.Mnemonic, - { - keyringId: NEW_KEY_RING_2.id, - }, - ), - ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.VaultDataError, - ); - }, - ); - }); - - it('should throw an error if vault unlocked has invalid authentication data', async () => { - await withController( - { - state: getMockInitialControllerState({ - withMockAuthenticatedUser: true, - vault: MOCK_VAULT, - }), - }, - async ({ controller, toprfClient, encryptor }) => { - mockcreateLocalKey(toprfClient, MOCK_PASSWORD); - - // persist the local enc key - jest.spyOn(toprfClient, 'persistLocalKey').mockResolvedValueOnce(); - // encrypt and store the secret data - handleMockSecretDataAdd(); - - jest.spyOn(encryptor, 'encryptWithDetail').mockResolvedValueOnce({ - vault: MOCK_VAULT, - exportedKeyString: MOCK_VAULT_ENCRYPTION_KEY, - }); - - await controller.createToprfKeyAndBackupSeedPhrase( - MOCK_PASSWORD, - NEW_KEY_RING_1.seedPhrase, - NEW_KEY_RING_1.id, - ); - - mockFetchAuthPubKey( - toprfClient, - base64ToBytes(controller.state.authPubKey as string), - ); - - jest - .spyOn(encryptor, 'decryptWithKey') - .mockResolvedValueOnce(MOCK_VAULT); - await expect( - controller.addNewSecretData( - NEW_KEY_RING_2.seedPhrase, - SecretType.Mnemonic, - { - keyringId: NEW_KEY_RING_2.id, - }, - ), - ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.VaultDataError, - ); - }, - ); - }); - it('should throw an error if password is outdated', async () => { await withController( { @@ -2379,6 +2218,78 @@ describe('SeedlessOnboardingController', () => { }, ); }); + + it('should throw an error if vault unlocked has invalid authentication data', async () => { + const mockToprfEncryptor = createMockToprfEncryptor(); + + const MOCK_ENCRYPTION_KEY = + mockToprfEncryptor.deriveEncKey(MOCK_PASSWORD); + const MOCK_PASSWORD_ENCRYPTION_KEY = + mockToprfEncryptor.derivePwEncKey(MOCK_PASSWORD); + const MOCK_AUTH_KEY_PAIR = + mockToprfEncryptor.deriveAuthKeyPair(MOCK_PASSWORD); + + const mockResult = await createMockVault( + MOCK_ENCRYPTION_KEY, + MOCK_PASSWORD_ENCRYPTION_KEY, + MOCK_AUTH_KEY_PAIR, + MOCK_PASSWORD, + ); + + const mockVault = mockResult.encryptedMockVault; + + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + vault: mockVault, + }), + }, + async ({ controller, encryptor }) => { + jest.spyOn(encryptor, 'encryptWithDetail').mockResolvedValueOnce({ + vault: mockVault, + exportedKeyString: mockResult.vaultEncryptionKey, + }); + + jest + .spyOn(encryptor, 'decryptWithKey') + .mockResolvedValueOnce(mockVault); + await expect( + controller.submitPassword(MOCK_PASSWORD), + ).rejects.toThrow( + SeedlessOnboardingControllerErrorMessage.VaultDataError, + ); + }, + ); + }); + + it('should throw an error if vault unlocked has an unexpected shape', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + vault: JSON.stringify({ foo: 'bar' }), + }), + }, + async ({ controller, encryptor }) => { + jest + .spyOn(encryptor, 'decryptWithKey') + .mockResolvedValueOnce({ foo: 'bar' }); + await expect( + controller.submitPassword(MOCK_PASSWORD), + ).rejects.toThrow( + SeedlessOnboardingControllerErrorMessage.InvalidVaultData, + ); + + jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce('null'); + await expect( + controller.submitPassword(MOCK_PASSWORD), + ).rejects.toThrow( + SeedlessOnboardingControllerErrorMessage.VaultDataError, + ); + }, + ); + }); }); describe('verifyPassword', () => { diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts index 6aa576a50d8..ee675a8b260 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts @@ -201,6 +201,13 @@ export class SeedlessOnboardingController extends BaseController< */ #isUnlocked = false; + /** + * Cached decrypted vault data. + * + * This is used to cache the decrypted vault data to avoid decrypting the vault data multiple times. + */ + #cachedDecryptedVaultData: DeserializedVaultData | undefined; + /** * Creates a new SeedlessOnboardingController instance. * @@ -689,6 +696,7 @@ export class SeedlessOnboardingController extends BaseController< delete state.accessToken; }); + this.#cachedDecryptedVaultData = undefined; this.#isUnlocked = false; } @@ -1277,6 +1285,10 @@ export class SeedlessOnboardingController extends BaseController< encryptionKey?: string; }): Promise { return this.#withVaultLock(async () => { + if (this.#cachedDecryptedVaultData) { + return this.#cachedDecryptedVaultData; + } + const { vaultData, vaultEncryptionKey, vaultEncryptionSalt } = await this.#decryptAndParseVaultData(params); @@ -1287,7 +1299,9 @@ export class SeedlessOnboardingController extends BaseController< state.accessToken = vaultData.accessToken; }); - return deserializeVaultData(vaultData); + const deserializedVaultData = deserializeVaultData(vaultData); + this.#cachedDecryptedVaultData = deserializedVaultData; + return deserializedVaultData; }); } @@ -1480,17 +1494,18 @@ export class SeedlessOnboardingController extends BaseController< const { revokeToken } = this.state; const accessToken = await this.#getAccessToken(password); - const serializedVaultData = serializeVaultData({ + + const vaultData: DeserializedVaultData = { toprfAuthKeyPair: rawToprfAuthKeyPair, toprfEncryptionKey: rawToprfEncryptionKey, toprfPwEncryptionKey: rawToprfPwEncryptionKey, revokeToken, accessToken, - }); + }; await this.#updateVault({ password, - serializedVaultData, + vaultData, pwEncKey: rawToprfPwEncryptionKey, }); @@ -1507,22 +1522,27 @@ export class SeedlessOnboardingController extends BaseController< * * @param params - The parameters for updating the vault. * @param params.password - The password to encrypt the vault. - * @param params.serializedVaultData - The serialized authentication data to update the vault with. + * @param params.vaultData - The raw vault data to update the vault with. * @param params.pwEncKey - The global password encryption key. * @returns A promise that resolves to the updated vault. */ async #updateVault({ password, - serializedVaultData, + vaultData, pwEncKey, }: { password: string; - serializedVaultData: string; + vaultData: DeserializedVaultData; pwEncKey: Uint8Array; }): Promise { await this.#withVaultLock(async () => { assertIsValidPassword(password); + // cache the vault data to avoid decrypting the vault data multiple times + this.#cachedDecryptedVaultData = vaultData; + + const serializedVaultData = serializeVaultData(vaultData); + // Note that vault encryption using the password is a very costly operation as it involves deriving the encryption key // from the password using an intentionally slow key derivation function. // We should make sure that we only call it very intentionally. From 3ed941066c32e96d74818a4b4a173769e8cf7135 Mon Sep 17 00:00:00 2001 From: lwin Date: Wed, 27 Aug 2025 15:27:45 +0800 Subject: [PATCH 4/6] fix: updated tests and assertions --- .../jest.config.js | 6 +- .../src/SeedlessOnboardingController.test.ts | 75 ++++--------------- .../src/SeedlessOnboardingController.ts | 8 +- .../src/assertions.test.ts | 26 +++---- .../src/assertions.ts | 2 +- 5 files changed, 35 insertions(+), 82 deletions(-) diff --git a/packages/seedless-onboarding-controller/jest.config.js b/packages/seedless-onboarding-controller/jest.config.js index 0e525e1f766..245917af2de 100644 --- a/packages/seedless-onboarding-controller/jest.config.js +++ b/packages/seedless-onboarding-controller/jest.config.js @@ -17,10 +17,10 @@ module.exports = merge(baseConfig, { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 100, + branches: 98.82, functions: 100, - lines: 100, - statements: 100, + lines: 99.78, + statements: 99.78, }, }, diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts index 51a37930e65..9128aac8326 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts @@ -1605,36 +1605,6 @@ describe('SeedlessOnboardingController', () => { ); }); - it('should throw error if encryptionSalt is different from the one in the vault', async () => { - await withController( - { - state: getMockInitialControllerState({ - withMockAuthenticatedUser: true, - vaultEncryptionKey: MOCK_VAULT_ENCRYPTION_KEY, - vaultEncryptionSalt: MOCK_VAULT_ENCRYPTION_SALT, - }), - }, - async ({ controller }) => { - // intentionally mock the JSON.parse to return an object with a different salt - jest.spyOn(global.JSON, 'parse').mockReturnValueOnce({ - salt: 'different-salt', - }); - - await expect( - controller.addNewSecretData( - NEW_KEY_RING_1.seedPhrase, - SecretType.Mnemonic, - { - keyringId: NEW_KEY_RING_1.id, - }, - ), - ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.ExpiredCredentials, - ); - }, - ); - }); - it('should throw an error if password is outdated', async () => { await withController( { @@ -2220,23 +2190,7 @@ describe('SeedlessOnboardingController', () => { }); it('should throw an error if vault unlocked has invalid authentication data', async () => { - const mockToprfEncryptor = createMockToprfEncryptor(); - - const MOCK_ENCRYPTION_KEY = - mockToprfEncryptor.deriveEncKey(MOCK_PASSWORD); - const MOCK_PASSWORD_ENCRYPTION_KEY = - mockToprfEncryptor.derivePwEncKey(MOCK_PASSWORD); - const MOCK_AUTH_KEY_PAIR = - mockToprfEncryptor.deriveAuthKeyPair(MOCK_PASSWORD); - - const mockResult = await createMockVault( - MOCK_ENCRYPTION_KEY, - MOCK_PASSWORD_ENCRYPTION_KEY, - MOCK_AUTH_KEY_PAIR, - MOCK_PASSWORD, - ); - - const mockVault = mockResult.encryptedMockVault; + const mockVault = JSON.stringify({ foo: 'bar' }); await withController( { @@ -2246,42 +2200,45 @@ describe('SeedlessOnboardingController', () => { }), }, async ({ controller, encryptor }) => { - jest.spyOn(encryptor, 'encryptWithDetail').mockResolvedValueOnce({ - vault: mockVault, - exportedKeyString: mockResult.vaultEncryptionKey, - }); - jest .spyOn(encryptor, 'decryptWithKey') .mockResolvedValueOnce(mockVault); await expect( controller.submitPassword(MOCK_PASSWORD), ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.VaultDataError, + SeedlessOnboardingControllerErrorMessage.InvalidVaultData, ); }, ); }); it('should throw an error if vault unlocked has an unexpected shape', async () => { + const mockVault = 'corrupted-vault-json'; + await withController( { state: getMockInitialControllerState({ withMockAuthenticatedUser: true, - vault: JSON.stringify({ foo: 'bar' }), + vault: mockVault, }), }, async ({ controller, encryptor }) => { - jest - .spyOn(encryptor, 'decryptWithKey') - .mockResolvedValueOnce({ foo: 'bar' }); + jest.spyOn(encryptor, 'decryptWithDetail').mockResolvedValueOnce({ + vault: mockVault, + exportedKeyString: 'mock-encryption-key', + salt: 'mock-salt', + }); await expect( controller.submitPassword(MOCK_PASSWORD), ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.InvalidVaultData, + SeedlessOnboardingControllerErrorMessage.VaultDataError, ); - jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce('null'); + jest.spyOn(encryptor, 'decryptWithDetail').mockResolvedValueOnce({ + vault: null, + exportedKeyString: 'mock-encryption-key', + salt: 'mock-salt', + }); await expect( controller.submitPassword(MOCK_PASSWORD), ).rejects.toThrow( diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts index ee675a8b260..5f43425f06f 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts @@ -1609,18 +1609,14 @@ export class SeedlessOnboardingController extends BaseController< */ #parseVaultData(data: unknown): VaultData { if (typeof data !== 'string') { - throw new Error( - SeedlessOnboardingControllerErrorMessage.InvalidVaultData, - ); + throw new Error(SeedlessOnboardingControllerErrorMessage.VaultDataError); } let parsedVaultData: unknown; try { parsedVaultData = JSON.parse(data); } catch { - throw new Error( - SeedlessOnboardingControllerErrorMessage.InvalidVaultData, - ); + throw new Error(SeedlessOnboardingControllerErrorMessage.VaultDataError); } assertIsValidVaultData(parsedVaultData); diff --git a/packages/seedless-onboarding-controller/src/assertions.test.ts b/packages/seedless-onboarding-controller/src/assertions.test.ts index c21635f6fca..3b50f70870a 100644 --- a/packages/seedless-onboarding-controller/src/assertions.test.ts +++ b/packages/seedless-onboarding-controller/src/assertions.test.ts @@ -22,10 +22,10 @@ describe('assertIsValidVaultData', () => { it('should throw when value is null or undefined', () => { expect(() => { assertIsValidVaultData(null); - }).toThrow(SeedlessOnboardingControllerErrorMessage.VaultDataError); + }).toThrow(SeedlessOnboardingControllerErrorMessage.InvalidVaultData); expect(() => { assertIsValidVaultData(undefined); - }).toThrow(SeedlessOnboardingControllerErrorMessage.VaultDataError); + }).toThrow(SeedlessOnboardingControllerErrorMessage.InvalidVaultData); }); it('should throw when toprfEncryptionKey is missing or not a string', () => { @@ -34,7 +34,7 @@ describe('assertIsValidVaultData', () => { expect(() => { assertIsValidVaultData(invalidData); - }).toThrow(SeedlessOnboardingControllerErrorMessage.VaultDataError); + }).toThrow(SeedlessOnboardingControllerErrorMessage.InvalidVaultData); const invalidData2 = { ...createValidVaultData(), toprfEncryptionKey: 123, @@ -42,7 +42,7 @@ describe('assertIsValidVaultData', () => { expect(() => { assertIsValidVaultData(invalidData2); - }).toThrow(SeedlessOnboardingControllerErrorMessage.VaultDataError); + }).toThrow(SeedlessOnboardingControllerErrorMessage.InvalidVaultData); }); it('should throw when toprfPwEncryptionKey is missing or not a string', () => { @@ -51,7 +51,7 @@ describe('assertIsValidVaultData', () => { expect(() => { assertIsValidVaultData(invalidData); - }).toThrow(SeedlessOnboardingControllerErrorMessage.VaultDataError); + }).toThrow(SeedlessOnboardingControllerErrorMessage.InvalidVaultData); const invalidData2 = { ...createValidVaultData(), @@ -60,7 +60,7 @@ describe('assertIsValidVaultData', () => { expect(() => { assertIsValidVaultData(invalidData2); - }).toThrow(SeedlessOnboardingControllerErrorMessage.VaultDataError); + }).toThrow(SeedlessOnboardingControllerErrorMessage.InvalidVaultData); }); it('should throw when toprfAuthKeyPair is missing or not a string', () => { @@ -69,7 +69,7 @@ describe('assertIsValidVaultData', () => { expect(() => { assertIsValidVaultData(invalidData); - }).toThrow(SeedlessOnboardingControllerErrorMessage.VaultDataError); + }).toThrow(SeedlessOnboardingControllerErrorMessage.InvalidVaultData); const invalidData2 = { ...createValidVaultData(), @@ -78,7 +78,7 @@ describe('assertIsValidVaultData', () => { expect(() => { assertIsValidVaultData(invalidData2); - }).toThrow(SeedlessOnboardingControllerErrorMessage.VaultDataError); + }).toThrow(SeedlessOnboardingControllerErrorMessage.InvalidVaultData); }); it('should throw when revokeToken exists but is not a string or undefined', () => { @@ -89,7 +89,7 @@ describe('assertIsValidVaultData', () => { expect(() => { assertIsValidVaultData(invalidData); - }).toThrow(SeedlessOnboardingControllerErrorMessage.VaultDataError); + }).toThrow(SeedlessOnboardingControllerErrorMessage.InvalidVaultData); const invalidData2 = { ...createValidVaultData(), @@ -98,7 +98,7 @@ describe('assertIsValidVaultData', () => { expect(() => { assertIsValidVaultData(invalidData2); - }).toThrow(SeedlessOnboardingControllerErrorMessage.VaultDataError); + }).toThrow(SeedlessOnboardingControllerErrorMessage.InvalidVaultData); const invalidData3 = { ...createValidVaultData(), @@ -107,7 +107,7 @@ describe('assertIsValidVaultData', () => { expect(() => { assertIsValidVaultData(invalidData3); - }).toThrow(SeedlessOnboardingControllerErrorMessage.VaultDataError); + }).toThrow(SeedlessOnboardingControllerErrorMessage.InvalidVaultData); }); it('should throw when accessToken is missing or not a string', () => { @@ -116,7 +116,7 @@ describe('assertIsValidVaultData', () => { expect(() => { assertIsValidVaultData(invalidData); - }).toThrow(SeedlessOnboardingControllerErrorMessage.VaultDataError); + }).toThrow(SeedlessOnboardingControllerErrorMessage.InvalidVaultData); const invalidData2 = { ...createValidVaultData(), @@ -125,7 +125,7 @@ describe('assertIsValidVaultData', () => { expect(() => { assertIsValidVaultData(invalidData2); - }).toThrow(SeedlessOnboardingControllerErrorMessage.VaultDataError); + }).toThrow(SeedlessOnboardingControllerErrorMessage.InvalidVaultData); }); }); diff --git a/packages/seedless-onboarding-controller/src/assertions.ts b/packages/seedless-onboarding-controller/src/assertions.ts index c949ae9bccf..dcbd13216b0 100644 --- a/packages/seedless-onboarding-controller/src/assertions.ts +++ b/packages/seedless-onboarding-controller/src/assertions.ts @@ -97,6 +97,6 @@ export function assertIsValidVaultData( !('accessToken' in value) || // accessToken is not defined typeof value.accessToken !== 'string' // accessToken is not a string ) { - throw new Error(SeedlessOnboardingControllerErrorMessage.VaultDataError); + throw new Error(SeedlessOnboardingControllerErrorMessage.InvalidVaultData); } } From 7a02ab836200e520f8586794843c89ef41113dfe Mon Sep 17 00:00:00 2001 From: lwin Date: Wed, 27 Aug 2025 21:18:35 +0800 Subject: [PATCH 5/6] fix: updated messenger types --- .../src/SeedlessOnboardingController.test.ts | 12 ++++++++---- .../src/index.ts | 2 -- .../src/types.ts | 16 ++++++---------- .../tests/__fixtures__/mockMessenger.ts | 19 ++++++++++++------- 4 files changed, 26 insertions(+), 23 deletions(-) diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts index 9128aac8326..494a47a10d2 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts @@ -44,13 +44,16 @@ import { SeedlessOnboardingController, } from './SeedlessOnboardingController'; import type { - SeedlessOnboardingControllerAllowedActions, - SeedlessOnboardingControllerAllowedEvents, + SeedlessOnboardingControllerEvents, SeedlessOnboardingControllerMessenger, SeedlessOnboardingControllerOptions, SeedlessOnboardingControllerState, VaultEncryptor, } from './types'; +import type { + ExtractAvailableAction, + ExtractAvailableEvent, +} from '../../base-controller/tests/helpers'; import { mockSeedlessOnboardingMessenger } from '../tests/__fixtures__/mockMessenger'; import { handleMockSecretDataGet, @@ -116,8 +119,9 @@ type WithControllerCallback = ({ initialState: SeedlessOnboardingControllerState; messenger: SeedlessOnboardingControllerMessenger; baseMessenger: Messenger< - SeedlessOnboardingControllerAllowedActions, - SeedlessOnboardingControllerAllowedEvents + ExtractAvailableAction, + | SeedlessOnboardingControllerEvents + | ExtractAvailableEvent >; toprfClient: ToprfSecureBackup; mockRefreshJWTToken: jest.Mock; diff --git a/packages/seedless-onboarding-controller/src/index.ts b/packages/seedless-onboarding-controller/src/index.ts index 721892ab225..4d445795530 100644 --- a/packages/seedless-onboarding-controller/src/index.ts +++ b/packages/seedless-onboarding-controller/src/index.ts @@ -11,9 +11,7 @@ export type { SeedlessOnboardingControllerGetStateAction, SeedlessOnboardingControllerStateChangeEvent, SeedlessOnboardingControllerActions, - SeedlessOnboardingControllerAllowedActions, SeedlessOnboardingControllerEvents, - SeedlessOnboardingControllerAllowedEvents, ToprfKeyDeriver, RecoveryErrorData, } from './types'; diff --git a/packages/seedless-onboarding-controller/src/types.ts b/packages/seedless-onboarding-controller/src/types.ts index b7c6d1acb3d..6cdbf46d4d6 100644 --- a/packages/seedless-onboarding-controller/src/types.ts +++ b/packages/seedless-onboarding-controller/src/types.ts @@ -186,7 +186,7 @@ export type SeedlessOnboardingControllerGetStateAction = export type SeedlessOnboardingControllerActions = SeedlessOnboardingControllerGetStateAction; -export type SeedlessOnboardingControllerAllowedActions = never; +type AllowedActions = never; // Events export type SeedlessOnboardingControllerStateChangeEvent = @@ -197,19 +197,15 @@ export type SeedlessOnboardingControllerStateChangeEvent = export type SeedlessOnboardingControllerEvents = SeedlessOnboardingControllerStateChangeEvent; -export type SeedlessOnboardingControllerAllowedEvents = - | KeyringControllerLockEvent - | KeyringControllerUnlockEvent; +type AllowedEvents = KeyringControllerLockEvent | KeyringControllerUnlockEvent; // Messenger export type SeedlessOnboardingControllerMessenger = RestrictedMessenger< typeof controllerName, - | SeedlessOnboardingControllerActions - | SeedlessOnboardingControllerAllowedActions, - | SeedlessOnboardingControllerEvents - | SeedlessOnboardingControllerAllowedEvents, - SeedlessOnboardingControllerAllowedActions['type'], - SeedlessOnboardingControllerAllowedEvents['type'] + SeedlessOnboardingControllerActions | AllowedActions, + SeedlessOnboardingControllerEvents | AllowedEvents, + AllowedActions['type'], + AllowedEvents['type'] >; /** diff --git a/packages/seedless-onboarding-controller/tests/__fixtures__/mockMessenger.ts b/packages/seedless-onboarding-controller/tests/__fixtures__/mockMessenger.ts index c5ec606047c..de3624f4952 100644 --- a/packages/seedless-onboarding-controller/tests/__fixtures__/mockMessenger.ts +++ b/packages/seedless-onboarding-controller/tests/__fixtures__/mockMessenger.ts @@ -1,10 +1,14 @@ import { Messenger } from '@metamask/base-controller'; import type { - SeedlessOnboardingControllerAllowedActions, - SeedlessOnboardingControllerAllowedEvents, - SeedlessOnboardingControllerMessenger, + ExtractAvailableAction, + ExtractAvailableEvent, +} from '../../../base-controller/tests/helpers'; +import type { + SeedlessOnboardingControllerActions, + SeedlessOnboardingControllerEvents, } from '../../src/types'; +import { type SeedlessOnboardingControllerMessenger } from '../../src/types'; /** * creates a custom seedless onboarding messenger, in case tests need different permissions @@ -13,8 +17,9 @@ import type { */ export function createCustomSeedlessOnboardingMessenger() { const baseMessenger = new Messenger< - SeedlessOnboardingControllerAllowedActions, - SeedlessOnboardingControllerAllowedEvents + SeedlessOnboardingControllerActions | never, + | SeedlessOnboardingControllerEvents + | ExtractAvailableEvent >(); const messenger = baseMessenger.getRestricted({ name: 'SeedlessOnboardingController', @@ -30,8 +35,8 @@ export function createCustomSeedlessOnboardingMessenger() { type OverrideMessengers = { baseMessenger: Messenger< - SeedlessOnboardingControllerAllowedActions, - SeedlessOnboardingControllerAllowedEvents + ExtractAvailableAction, + ExtractAvailableEvent >; messenger: SeedlessOnboardingControllerMessenger; }; From 2fccc635abeb9c422eb22dc708033da2873f93b7 Mon Sep 17 00:00:00 2001 From: lwin Date: Thu, 28 Aug 2025 11:24:59 +0800 Subject: [PATCH 6/6] test: add salt-expired test --- .../jest.config.js | 6 +- .../src/SeedlessOnboardingController.test.ts | 67 ++++++++++++++++++- .../tests/__fixtures__/mockMessenger.ts | 9 +-- 3 files changed, 69 insertions(+), 13 deletions(-) diff --git a/packages/seedless-onboarding-controller/jest.config.js b/packages/seedless-onboarding-controller/jest.config.js index 245917af2de..0e525e1f766 100644 --- a/packages/seedless-onboarding-controller/jest.config.js +++ b/packages/seedless-onboarding-controller/jest.config.js @@ -17,10 +17,10 @@ module.exports = merge(baseConfig, { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 98.82, + branches: 100, functions: 100, - lines: 99.78, - statements: 99.78, + lines: 100, + statements: 100, }, }, diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts index 494a47a10d2..bad54bf9d67 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts @@ -44,7 +44,6 @@ import { SeedlessOnboardingController, } from './SeedlessOnboardingController'; import type { - SeedlessOnboardingControllerEvents, SeedlessOnboardingControllerMessenger, SeedlessOnboardingControllerOptions, SeedlessOnboardingControllerState, @@ -120,8 +119,7 @@ type WithControllerCallback = ({ messenger: SeedlessOnboardingControllerMessenger; baseMessenger: Messenger< ExtractAvailableAction, - | SeedlessOnboardingControllerEvents - | ExtractAvailableEvent + ExtractAvailableEvent >; toprfClient: ToprfSecureBackup; mockRefreshJWTToken: jest.Mock; @@ -3923,6 +3921,69 @@ describe('SeedlessOnboardingController', () => { }, ); }); + + /** + * This test is to verify that the controller throws an error if the encryption salt is expired. + * The test creates a mock vault with a different salt value in the state to simulate an expired salt. + * It then creates mock keys associated with the new global password and uses these values as mock return values for the recoverEncKey and recoverPwEncKey calls. + * The test expects the controller to throw an error indicating that the password could not be recovered since the encryption salt from state is different from the salt in the mock vault. + */ + it('should throw an error if the encryption salt is expired', async () => { + const encryptedSeedlessEncryptionKey = bytesToBase64( + initialEncryptedSeedlessEncryptionKey, + ); + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + authPubKey: INITIAL_AUTH_PUB_KEY, // Use the base64 encoded key + vault: MOCK_VAULT, + vaultEncryptionKey: MOCK_VAULT_ENCRYPTION_KEY, + // Mock a different salt value in state to simulate an expired salt + vaultEncryptionSalt: 'DIFFERENT-SALT', + withMockAuthPubKey: true, + encryptedSeedlessEncryptionKey, + }), + }, + async ({ controller, toprfClient }) => { + // Here we are creating mock keys associated with the new global password + // and these values are used as mock return values for the recoverEncKey and recoverPwEncKey calls + const mockToprfEncryptor = createMockToprfEncryptor(); + const newEncKey = mockToprfEncryptor.deriveEncKey(GLOBAL_PASSWORD); + const newPwEncKey = + mockToprfEncryptor.derivePwEncKey(GLOBAL_PASSWORD); + const newAuthKeyPair = + mockToprfEncryptor.deriveAuthKeyPair(GLOBAL_PASSWORD); + + const recoverEncKeySpy = jest + .spyOn(toprfClient, 'recoverEncKey') + .mockResolvedValueOnce({ + encKey: newEncKey, + pwEncKey: newPwEncKey, + authKeyPair: newAuthKeyPair, + rateLimitResetResult: Promise.resolve(), + keyShareIndex: 1, + }); + + const recoverPwEncKeySpy = jest + .spyOn(toprfClient, 'recoverPwEncKey') + .mockResolvedValueOnce({ + pwEncKey: initialPwEncKey, + }); + + await expect( + controller.submitGlobalPassword({ + globalPassword: GLOBAL_PASSWORD, + }), + ).rejects.toThrow( + SeedlessOnboardingControllerErrorMessage.CouldNotRecoverPassword, + ); + + expect(recoverEncKeySpy).toHaveBeenCalled(); + expect(recoverPwEncKeySpy).toHaveBeenCalled(); + }, + ); + }); }); describe('token refresh functionality', () => { diff --git a/packages/seedless-onboarding-controller/tests/__fixtures__/mockMessenger.ts b/packages/seedless-onboarding-controller/tests/__fixtures__/mockMessenger.ts index de3624f4952..b6473a5e972 100644 --- a/packages/seedless-onboarding-controller/tests/__fixtures__/mockMessenger.ts +++ b/packages/seedless-onboarding-controller/tests/__fixtures__/mockMessenger.ts @@ -4,10 +4,6 @@ import type { ExtractAvailableAction, ExtractAvailableEvent, } from '../../../base-controller/tests/helpers'; -import type { - SeedlessOnboardingControllerActions, - SeedlessOnboardingControllerEvents, -} from '../../src/types'; import { type SeedlessOnboardingControllerMessenger } from '../../src/types'; /** @@ -17,9 +13,8 @@ import { type SeedlessOnboardingControllerMessenger } from '../../src/types'; */ export function createCustomSeedlessOnboardingMessenger() { const baseMessenger = new Messenger< - SeedlessOnboardingControllerActions | never, - | SeedlessOnboardingControllerEvents - | ExtractAvailableEvent + ExtractAvailableAction, + ExtractAvailableEvent >(); const messenger = baseMessenger.getRestricted({ name: 'SeedlessOnboardingController',