Skip to content

Commit 9d017d7

Browse files
himanshuchawla009weitingsun
authored andcommitted
fix(seedless-onboarding): handle both vault formats in encryptorAdapter
The encryptorAdapter introduced in PR #26258 overrides encryptWithKey to return browser-passworder format ({ data }) instead of the mobile Encryptor format ({ cipher }). However, decrypt and decryptWithDetail were not overridden — they were spread from the mobile Encryptor, which reads the cipher field. This caused a crash on the next unlock after any background JWT token refresh: TypeError: The first argument must be one of type string, Buffer... Received type undefined (quick-crypto.ts:101) Fix: add normalizeVaultFormat which injects cipher = data when a vault has data but no cipher, and override decrypt and decryptWithDetail in the adapter to normalize before delegating to the underlying Encryptor. Also harden decryptWithKey to accept both data and cipher fields (for pre-adapter vaults that only carry cipher), and throw explicitly when both fields are absent. Adds end-to-end tests that reproduce the bug scenario: background token refresh writes a data-format vault via encryptWithKey, then decrypt / decryptWithDetail must recover it on the next unlock.
1 parent d066615 commit 9d017d7

File tree

2 files changed

+303
-6
lines changed

2 files changed

+303
-6
lines changed

app/core/Engine/controllers/seedless-onboarding-controller/index.test.ts

Lines changed: 261 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,18 +68,24 @@ const getAuthTokenHandlerMocks = () => {
6868
jest.mock('../../../Encryptor', () => {
6969
const mockEncryptWithKeyFn = jest.fn();
7070
const mockDecryptWithKeyFn = jest.fn();
71+
const mockDecryptFn = jest.fn();
72+
const mockDecryptWithDetailFn = jest.fn();
7173

7274
return {
7375
Encryptor: jest.fn().mockImplementation(() => ({
7476
encryptWithKey: mockEncryptWithKeyFn,
7577
decryptWithKey: mockDecryptWithKeyFn,
78+
decrypt: mockDecryptFn,
79+
decryptWithDetail: mockDecryptWithDetailFn,
7680
})),
7781
LEGACY_DERIVATION_OPTIONS: {
7882
algorithm: 'PBKDF2',
7983
params: { iterations: 600000 },
8084
},
8185
__mockEncryptWithKey: mockEncryptWithKeyFn,
8286
__mockDecryptWithKey: mockDecryptWithKeyFn,
87+
__mockDecrypt: mockDecryptFn,
88+
__mockDecryptWithDetail: mockDecryptWithDetailFn,
8389
};
8490
});
8591

@@ -88,10 +94,14 @@ const getEncryptorMocks = () => {
8894
const mocks = jest.requireMock('../../../Encryptor') as {
8995
__mockEncryptWithKey: jest.Mock;
9096
__mockDecryptWithKey: jest.Mock;
97+
__mockDecrypt: jest.Mock;
98+
__mockDecryptWithDetail: jest.Mock;
9199
};
92100
return {
93101
mockEncryptWithKey: mocks.__mockEncryptWithKey,
94102
mockDecryptWithKey: mocks.__mockDecryptWithKey,
103+
mockDecrypt: mocks.__mockDecrypt,
104+
mockDecryptWithDetail: mocks.__mockDecryptWithDetail,
95105
};
96106
};
97107

@@ -116,9 +126,20 @@ describe('seedless onboarding controller init', () => {
116126
// Reset web3AuthNetwork to valid value before each test
117127
mockWeb3AuthNetwork.value = 'sapphire_devnet';
118128

119-
const { mockEncryptWithKey, mockDecryptWithKey } = getEncryptorMocks();
129+
const {
130+
mockEncryptWithKey,
131+
mockDecryptWithKey,
132+
mockDecrypt,
133+
mockDecryptWithDetail,
134+
} = getEncryptorMocks();
120135
mockEncryptWithKey.mockResolvedValue(mockEncryptResult);
121136
mockDecryptWithKey.mockResolvedValue({ test: 'decrypted-data' });
137+
mockDecrypt.mockResolvedValue({ test: 'decrypted-data' });
138+
mockDecryptWithDetail.mockResolvedValue({
139+
vault: { test: 'decrypted-data' },
140+
exportedKeyString: 'mock-key-string',
141+
salt: 'mock-salt',
142+
});
122143

123144
const baseControllerMessenger = new ExtendedMessenger<MockAnyNamespace>({
124145
namespace: MOCK_ANY_NAMESPACE,
@@ -230,6 +251,245 @@ describe('seedless onboarding controller init', () => {
230251
});
231252
expect(decrypted).toEqual({ test: 'decrypted-data' });
232253
});
254+
255+
it('decryptWithKey falls back to cipher field for pre-adapter vaults', async () => {
256+
const { mockDecryptWithKey } = getEncryptorMocks();
257+
seedlessOnboardingControllerInit(initRequestMock);
258+
259+
const encryptorAdapter =
260+
seedlessOnboardingControllerClassMock.mock.calls[0][0].encryptor;
261+
262+
const mockKey = { key: 'test-key', lib: 'test-lib', exportable: true };
263+
// Legacy vault format: uses 'cipher' instead of 'data'
264+
const legacyEncryptedObject = {
265+
cipher: 'legacy-cipher-data',
266+
iv: 'test-iv',
267+
salt: 'test-salt',
268+
};
269+
270+
// Cast as never: the package type expects `data` (DefaultEncryptionResult), but we
271+
// intentionally pass a legacy vault with `cipher` only to test the fallback path.
272+
await encryptorAdapter.decryptWithKey(
273+
mockKey,
274+
legacyEncryptedObject as never,
275+
);
276+
277+
expect(mockDecryptWithKey).toHaveBeenCalledWith(mockKey, {
278+
cipher: 'legacy-cipher-data',
279+
iv: 'test-iv',
280+
salt: 'test-salt',
281+
lib: undefined,
282+
keyMetadata: undefined,
283+
});
284+
});
285+
286+
it('decryptWithKey throws when both data and cipher are absent', async () => {
287+
seedlessOnboardingControllerInit(initRequestMock);
288+
289+
const encryptorAdapter =
290+
seedlessOnboardingControllerClassMock.mock.calls[0][0].encryptor;
291+
292+
const mockKey = { key: 'test-key', lib: 'test-lib', exportable: true };
293+
const malformedObject = { iv: 'test-iv', salt: 'test-salt' };
294+
295+
// Cast as never: the package type expects `data` (DefaultEncryptionResult), but we
296+
// intentionally pass a malformed vault (missing both fields) to test the error path.
297+
await expect(
298+
encryptorAdapter.decryptWithKey(mockKey, malformedObject as never),
299+
).rejects.toThrow(
300+
'SeedlessOnboardingController encryptorAdapter: vault is missing both "data" and "cipher" fields',
301+
);
302+
});
303+
304+
it('decrypt normalizes data-format vault before decryption', async () => {
305+
const { mockDecrypt } = getEncryptorMocks();
306+
seedlessOnboardingControllerInit(initRequestMock);
307+
308+
const encryptorAdapter =
309+
seedlessOnboardingControllerClassMock.mock.calls[0][0].encryptor;
310+
311+
const dataFormatVault = JSON.stringify({
312+
data: 'encrypted-data',
313+
iv: 'test-iv',
314+
salt: 'test-salt',
315+
});
316+
317+
await encryptorAdapter.decrypt('password', dataFormatVault);
318+
319+
// Should normalize 'data' → 'cipher' before passing to underlying encryptor
320+
const expectedNormalized = JSON.stringify({
321+
data: 'encrypted-data',
322+
iv: 'test-iv',
323+
salt: 'test-salt',
324+
cipher: 'encrypted-data',
325+
});
326+
expect(mockDecrypt).toHaveBeenCalledWith('password', expectedNormalized);
327+
});
328+
329+
it('decrypt passes cipher-format vault through unchanged', async () => {
330+
const { mockDecrypt } = getEncryptorMocks();
331+
seedlessOnboardingControllerInit(initRequestMock);
332+
333+
const encryptorAdapter =
334+
seedlessOnboardingControllerClassMock.mock.calls[0][0].encryptor;
335+
336+
const cipherFormatVault = JSON.stringify({
337+
cipher: 'encrypted-cipher',
338+
iv: 'test-iv',
339+
salt: 'test-salt',
340+
});
341+
342+
await encryptorAdapter.decrypt('password', cipherFormatVault);
343+
344+
expect(mockDecrypt).toHaveBeenCalledWith('password', cipherFormatVault);
345+
});
346+
347+
it('decryptWithDetail normalizes data-format vault before decryption', async () => {
348+
const { mockDecryptWithDetail } = getEncryptorMocks();
349+
seedlessOnboardingControllerInit(initRequestMock);
350+
351+
const encryptorAdapter =
352+
seedlessOnboardingControllerClassMock.mock.calls[0][0].encryptor;
353+
354+
const dataFormatVault = JSON.stringify({
355+
data: 'encrypted-data',
356+
iv: 'test-iv',
357+
salt: 'test-salt',
358+
});
359+
360+
await encryptorAdapter.decryptWithDetail('password', dataFormatVault);
361+
362+
const expectedNormalized = JSON.stringify({
363+
data: 'encrypted-data',
364+
iv: 'test-iv',
365+
salt: 'test-salt',
366+
cipher: 'encrypted-data',
367+
});
368+
expect(mockDecryptWithDetail).toHaveBeenCalledWith(
369+
'password',
370+
expectedNormalized,
371+
);
372+
});
373+
374+
it('decryptWithDetail passes cipher-format vault through unchanged', async () => {
375+
const { mockDecryptWithDetail } = getEncryptorMocks();
376+
seedlessOnboardingControllerInit(initRequestMock);
377+
378+
const encryptorAdapter =
379+
seedlessOnboardingControllerClassMock.mock.calls[0][0].encryptor;
380+
381+
const cipherFormatVault = JSON.stringify({
382+
cipher: 'encrypted-cipher',
383+
iv: 'test-iv',
384+
salt: 'test-salt',
385+
});
386+
387+
await encryptorAdapter.decryptWithDetail('password', cipherFormatVault);
388+
389+
expect(mockDecryptWithDetail).toHaveBeenCalledWith(
390+
'password',
391+
cipherFormatVault,
392+
);
393+
});
394+
395+
/**
396+
* End-to-end bug reproduction scenario:
397+
*
398+
* Bug (pre-fix): While the wallet is unlocked, a background JWT token
399+
* refresh triggers SeedlessOnboardingController#updateVault, which calls
400+
* encryptWithKey via the adapter. The adapter returns { data, iv, salt }
401+
* (browser-passworder format). This vault string (with `data`, no `cipher`)
402+
* is persisted to state.
403+
*
404+
* On next unlock, the controller reads the persisted vault and calls
405+
* decrypt / decryptWithDetail. The underlying mobile Encryptor reads
406+
* `cipher` from the parsed vault — finds undefined — and crashes with
407+
* "TypeError: The first argument must be one of type string, Buffer..."
408+
*
409+
* Fix: normalizeVaultFormat detects a vault that has `data` but no
410+
* `cipher` and injects cipher = data before handing it to the Encryptor.
411+
*/
412+
describe('end-to-end: background token refresh followed by unlock', () => {
413+
it('decrypt can read a vault written by encryptWithKey (data-format vault)', async () => {
414+
const { mockDecrypt } = getEncryptorMocks();
415+
seedlessOnboardingControllerInit(initRequestMock);
416+
417+
const encryptorAdapter =
418+
seedlessOnboardingControllerClassMock.mock.calls[0][0].encryptor;
419+
420+
const mockKey = {
421+
key: 'test-key',
422+
lib: 'test-lib',
423+
exportable: true,
424+
keyMetadata: { algorithm: 'PBKDF2', params: { iterations: 600000 } },
425+
};
426+
427+
// Step 1: background token refresh writes vault via encryptWithKey
428+
// → adapter converts cipher → data, returning browser-passworder format
429+
const encryptedVault = await encryptorAdapter.encryptWithKey(mockKey, {
430+
secret: 'seed-phrase',
431+
});
432+
expect(encryptedVault).toHaveProperty('data'); // data, not cipher
433+
expect(encryptedVault).not.toHaveProperty('cipher');
434+
435+
// Step 2: controller serializes and persists the vault
436+
const persistedVaultString = JSON.stringify(encryptedVault);
437+
438+
// Step 3: wallet locks, vaultEncryptionKey is cleared (persist: false).
439+
// On next unlock the controller calls decrypt with the persisted string.
440+
// Without the fix this would crash because the Encryptor reads `cipher`
441+
// (undefined here) and passes it to QuickCryptoLib.decrypt.
442+
await encryptorAdapter.decrypt('user-password', persistedVaultString);
443+
444+
// The underlying Encryptor must receive the vault with `cipher` injected
445+
const expectedNormalized = JSON.stringify({
446+
...encryptedVault,
447+
cipher: encryptedVault.data,
448+
});
449+
expect(mockDecrypt).toHaveBeenCalledWith(
450+
'user-password',
451+
expectedNormalized,
452+
);
453+
});
454+
455+
it('decryptWithDetail can read a vault written by encryptWithKey (data-format vault)', async () => {
456+
const { mockDecryptWithDetail } = getEncryptorMocks();
457+
seedlessOnboardingControllerInit(initRequestMock);
458+
459+
const encryptorAdapter =
460+
seedlessOnboardingControllerClassMock.mock.calls[0][0].encryptor;
461+
462+
const mockKey = {
463+
key: 'test-key',
464+
lib: 'test-lib',
465+
exportable: true,
466+
keyMetadata: { algorithm: 'PBKDF2', params: { iterations: 600000 } },
467+
};
468+
469+
// Step 1: background token refresh writes vault via encryptWithKey
470+
const encryptedVault = await encryptorAdapter.encryptWithKey(mockKey, {
471+
secret: 'seed-phrase',
472+
});
473+
474+
// Step 2: persist
475+
const persistedVaultString = JSON.stringify(encryptedVault);
476+
477+
// Step 3: unlock via decryptWithDetail (used when vaultEncryptionKey is absent)
478+
await encryptorAdapter.decryptWithDetail(
479+
'user-password',
480+
persistedVaultString,
481+
);
482+
483+
const expectedNormalized = JSON.stringify({
484+
...encryptedVault,
485+
cipher: encryptedVault.data,
486+
});
487+
expect(mockDecryptWithDetail).toHaveBeenCalledWith(
488+
'user-password',
489+
expectedNormalized,
490+
);
491+
});
492+
});
233493
});
234494

235495
describe('controller initialization parameters', () => {

app/core/Engine/controllers/seedless-onboarding-controller/index.ts

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,25 @@ interface ControllerEncryptionResult {
3131
keyMetadata?: KeyDerivationOptions;
3232
}
3333

34+
/**
35+
* Normalizes a serialized vault so the mobile Encryptor can decrypt it
36+
* regardless of whether the vault was written by the adapter (using 'data')
37+
* or by the original mobile Encryptor (using 'cipher').
38+
*
39+
* Background: the SeedlessOnboardingController stores vaults via
40+
* encryptWithKey (adapter, 'data' field) and via encryptWithDetail (mobile
41+
* Encryptor, 'cipher' field). The mobile Encryptor's decrypt / decryptWithDetail
42+
* always reads 'cipher', so vaults produced by the adapter must be normalized
43+
* before being handed to those methods.
44+
*/
45+
function normalizeVaultFormat(text: string): string {
46+
const payload: Record<string, unknown> = JSON.parse(text);
47+
if (payload.data !== undefined && payload.cipher === undefined) {
48+
return JSON.stringify({ ...payload, cipher: payload.data });
49+
}
50+
return text;
51+
}
52+
3453
/**
3554
* Adapter that wraps the mobile Encryptor to be compatible with
3655
* SeedlessOnboardingController's VaultEncryptor interface.
@@ -53,15 +72,33 @@ const encryptorAdapter = {
5372
},
5473
decryptWithKey: async (
5574
key: EncryptionKey,
56-
encryptedObject: ControllerEncryptionResult,
57-
): Promise<unknown> =>
58-
encryptor.decryptWithKey(key, {
59-
cipher: encryptedObject.data,
75+
// Accept both adapter format ('data') and legacy mobile format ('cipher').
76+
// 'data' is intentionally optional here: pre-adapter vaults only carry
77+
// 'cipher', so making 'data' required would be a lie about the runtime
78+
// shape and would allow the fallback to be silently removed.
79+
encryptedObject: Omit<ControllerEncryptionResult, 'data'> & {
80+
data?: string;
81+
cipher?: string;
82+
},
83+
): Promise<unknown> => {
84+
const cipher = encryptedObject.data ?? encryptedObject.cipher;
85+
if (!cipher) {
86+
throw new Error(
87+
'SeedlessOnboardingController encryptorAdapter: vault is missing both "data" and "cipher" fields',
88+
);
89+
}
90+
return encryptor.decryptWithKey(key, {
91+
cipher,
6092
iv: encryptedObject.iv,
6193
salt: encryptedObject.salt,
6294
lib: encryptedObject.lib,
6395
keyMetadata: encryptedObject.keyMetadata,
64-
}),
96+
});
97+
},
98+
decrypt: async (password: string, text: string): Promise<unknown> =>
99+
encryptor.decrypt(password, normalizeVaultFormat(text)),
100+
decryptWithDetail: async (password: string, text: string) =>
101+
encryptor.decryptWithDetail(password, normalizeVaultFormat(text)),
65102
};
66103

67104
/**

0 commit comments

Comments
 (0)