diff --git a/packages/seedless-onboarding-controller/CHANGELOG.md b/packages/seedless-onboarding-controller/CHANGELOG.md index a991b2ac4a..9dcb4b2a63 100644 --- a/packages/seedless-onboarding-controller/CHANGELOG.md +++ b/packages/seedless-onboarding-controller/CHANGELOG.md @@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/utils` from `^11.9.0` to `^11.11.0` ([#9074](https://github.com/MetaMask/core/pull/9074)) - Bump `@metamask/keyring-controller` from `^27.0.0` to `^27.1.0` ([#9129](https://github.com/MetaMask/core/pull/9129)) +### Fixed + +- Fix `InvalidPrimarySecretDataType` thrown for legacy accounts where client clock skew sorts a non-mnemonic item ahead of the primary SRP ([#9247](https://github.com/MetaMask/core/pull/9247)) + ## [10.0.2] ### Changed diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts index bc1406e3e6..542cf5ae91 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts @@ -3472,6 +3472,58 @@ describe('SeedlessOnboardingController', () => { }, ); }); + + it('should promote the primary SRP to first when a legacy non-mnemonic sorts ahead of it (clock skew)', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + }), + }, + async ({ controller, toprfClient }) => { + mockRecoverEncKey(toprfClient, MOCK_PASSWORD); + + // Legacy data (no dataType, no createdAt). An imported private key + // has the earliest client timestamp (e.g. due to clock skew on the + // device that imported it), so it sorts ahead of the primary SRP. + // The fetch must still recover by promoting the primary SRP, not + // throw InvalidPrimarySecretDataType. + const mockSecretDataGet = handleMockSecretDataGet({ + status: 200, + body: createMockSecretDataGetResponse( + [ + { + data: new Uint8Array(Buffer.from('importedPk', 'utf-8')), + timestamp: 100, + type: SecretType.PrivateKey, + itemId: 'imported-pk-id', + // No dataType or createdAt (legacy) + }, + { + data: new Uint8Array(Buffer.from('primarySrp', 'utf-8')), + timestamp: 200, + type: SecretType.Mnemonic, + itemId: 'primary-srp-id', + // No dataType or createdAt (legacy) + }, + ], + MOCK_PASSWORD, + ), + }); + + const secretData = await controller.fetchAllSecretData(MOCK_PASSWORD); + + expect(mockSecretDataGet.isDone()).toBe(true); + expect(secretData).toHaveLength(2); + // Primary SRP (mnemonic) promoted to first despite later timestamp. + expect(secretData[0].type).toBe(SecretType.Mnemonic); + expect(secretData[0].data).toStrictEqual(stringToBytes('primarySrp')); + // The private key is preserved, not dropped. + expect(secretData[1].type).toBe(SecretType.PrivateKey); + expect(secretData[1].data).toStrictEqual(stringToBytes('importedPk')); + }, + ); + }); }); describe('submitPassword', () => { diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts index 91020b33f2..6ec7f2e203 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts @@ -1646,22 +1646,38 @@ export class SeedlessOnboardingController< // Sort: PrimarySrp first, then by createdAt/timestamp (oldest first) results.sort((a, b) => SecretMetadata.compare(a, b, 'asc')); - // Validate the first item is the primary SRP - const firstItem = results[0]; - const isDataTypePrimary = - firstItem.dataType === undefined || - firstItem.dataType === null || - firstItem.dataType === EncAccountDataType.PrimarySrp; - const isMnemonic = SecretMetadata.matchesType( - firstItem, - SecretType.Mnemonic, - ); + // Find the primary SRP instead of assuming it is at index 0: legacy + // items have no `createdAt`, so ordering falls back to the client + // `timestamp`, which clock skew can sort a private key ahead of. + // A candidate is a mnemonic whose `dataType` is `PrimarySrp` or unset. + // `dataType` (a plaintext server field) is the only thing separating + // primary from imported SRP; legacy items lack it and the encrypted + // `SecretType` groups both, so among legacy mnemonics the primary is + // indistinguishable — we take the oldest (best-effort). + const primaryIndex = results.findIndex((item) => { + const isDataTypePrimary = + item.dataType === undefined || + item.dataType === null || + item.dataType === EncAccountDataType.PrimarySrp; + return ( + isDataTypePrimary && + SecretMetadata.matchesType(item, SecretType.Mnemonic) + ); + }); - if (!isDataTypePrimary || !isMnemonic) { + // No recoverable primary SRP exists in the metadata store. + if (primaryIndex === -1) { throw new Error( SeedlessOnboardingControllerErrorMessage.InvalidPrimarySecretDataType, ); } + + // Ensure the primary SRP is first; callers rely on `results[0]` being it. + if (primaryIndex > 0) { + const [primary] = results.splice(primaryIndex, 1); + results.unshift(primary); + } + return results; }