From 7b3f28c081879026cdaab96b7ea93fc5373ddece Mon Sep 17 00:00:00 2001 From: ieow <4881057+ieow@users.noreply.github.com> Date: Mon, 31 Aug 2026 14:31:38 +0800 Subject: [PATCH 1/3] Add `mfa-recovery-controller` package with initial implementation - Introduced the `@metamask/mfa-recovery-controller` package to manage MFA recovery flows across MetaMask clients. - Added core functionalities including methods for registering recovery secrets, updating identifiers, and handling recovery states. - Updated relevant documentation and code ownership files to include the new package. - Ensured integration with existing MetaMask infrastructure by updating configuration files and dependencies. This commit lays the groundwork for enhanced MFA recovery capabilities in MetaMask, improving user experience and security. --- .github/CODEOWNERS | 4 + README.md | 2 + codeowners.ts | 5 + packages/mfa-recovery-controller/CHANGELOG.md | 14 + packages/mfa-recovery-controller/LICENSE | 6 + .../mfa-recovery-controller/LICENSE.APACHE2 | 201 +++++ packages/mfa-recovery-controller/LICENSE.MIT | 21 + packages/mfa-recovery-controller/README.md | 21 + .../mfa-recovery-controller/jest.config.js | 26 + packages/mfa-recovery-controller/package.json | 77 ++ ...aRecoveryController-method-action-types.ts | 88 ++ .../src/MfaRecoveryController.test.ts | 744 +++++++++++++++++ .../src/MfaRecoveryController.ts | 783 ++++++++++++++++++ .../src/crypto.test.ts | 46 + .../mfa-recovery-controller/src/crypto.ts | 201 +++++ .../mfa-recovery-controller/src/errors.ts | 29 + .../src/identifier-auth.ts | 32 + packages/mfa-recovery-controller/src/index.ts | 42 + .../src/state-machine.ts | 51 ++ packages/mfa-recovery-controller/src/types.ts | 208 +++++ .../mfa-recovery-controller/tests/stubs.ts | 603 ++++++++++++++ .../tsconfig.build.json | 17 + .../mfa-recovery-controller/tsconfig.json | 15 + packages/mfa-recovery-controller/typedoc.json | 7 + teams.json | 1 + tsconfig.build.json | 7 +- tsconfig.json | 31 +- yarn.lock | 20 + 28 files changed, 3286 insertions(+), 16 deletions(-) create mode 100644 packages/mfa-recovery-controller/CHANGELOG.md create mode 100644 packages/mfa-recovery-controller/LICENSE create mode 100644 packages/mfa-recovery-controller/LICENSE.APACHE2 create mode 100644 packages/mfa-recovery-controller/LICENSE.MIT create mode 100644 packages/mfa-recovery-controller/README.md create mode 100644 packages/mfa-recovery-controller/jest.config.js create mode 100644 packages/mfa-recovery-controller/package.json create mode 100644 packages/mfa-recovery-controller/src/MfaRecoveryController-method-action-types.ts create mode 100644 packages/mfa-recovery-controller/src/MfaRecoveryController.test.ts create mode 100644 packages/mfa-recovery-controller/src/MfaRecoveryController.ts create mode 100644 packages/mfa-recovery-controller/src/crypto.test.ts create mode 100644 packages/mfa-recovery-controller/src/crypto.ts create mode 100644 packages/mfa-recovery-controller/src/errors.ts create mode 100644 packages/mfa-recovery-controller/src/identifier-auth.ts create mode 100644 packages/mfa-recovery-controller/src/index.ts create mode 100644 packages/mfa-recovery-controller/src/state-machine.ts create mode 100644 packages/mfa-recovery-controller/src/types.ts create mode 100644 packages/mfa-recovery-controller/tests/stubs.ts create mode 100644 packages/mfa-recovery-controller/tsconfig.build.json create mode 100644 packages/mfa-recovery-controller/tsconfig.json create mode 100644 packages/mfa-recovery-controller/typedoc.json diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index a2047d1fa8..b3e93d7cfb 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -123,6 +123,7 @@ ## Web3Auth Team /packages/seedless-onboarding-controller @MetaMask/web3auth /packages/passkey-controller @MetaMask/web3auth +/packages/mfa-recovery-controller @MetaMask/web3auth /packages/shield-controller @MetaMask/web3auth /packages/subscription-controller @MetaMask/web3auth /packages/claims-controller @MetaMask/web3auth @@ -231,6 +232,9 @@ /packages/passkey-controller/package.json @MetaMask/web3auth @MetaMask/core-platform /packages/passkey-controller/CHANGELOG.md @MetaMask/web3auth @MetaMask/core-platform /packages/passkey-controller/tsconfig.* @MetaMask/web3auth @MetaMask/core-platform +/packages/mfa-recovery-controller/package.json @MetaMask/web3auth @MetaMask/core-platform +/packages/mfa-recovery-controller/CHANGELOG.md @MetaMask/web3auth @MetaMask/core-platform +/packages/mfa-recovery-controller/tsconfig.* @MetaMask/web3auth @MetaMask/core-platform /packages/logging-controller/package.json @MetaMask/confirmations @MetaMask/core-platform /packages/logging-controller/CHANGELOG.md @MetaMask/confirmations @MetaMask/core-platform /packages/logging-controller/tsconfig.* @MetaMask/confirmations @MetaMask/core-platform diff --git a/README.md b/README.md index 7ea1324a13..29db701941 100644 --- a/README.md +++ b/README.md @@ -91,6 +91,7 @@ yarn skills --reset # clear saved local selection - [`@metamask/message-manager`](packages/message-manager) - [`@metamask/messenger`](packages/messenger) - [`@metamask/messenger-cli`](packages/messenger-cli) +- [`@metamask/mfa-recovery-controller`](packages/mfa-recovery-controller) - [`@metamask/money-account-api-data-service`](packages/money-account-api-data-service) - [`@metamask/money-account-balance-service`](packages/money-account-balance-service) - [`@metamask/money-account-controller`](packages/money-account-controller) @@ -197,6 +198,7 @@ linkStyle default opacity:0.5 message_manager(["@metamask/message-manager"]); messenger(["@metamask/messenger"]); messenger_cli(["@metamask/messenger-cli"]); + mfa_recovery_controller(["@metamask/mfa-recovery-controller"]); money_account_api_data_service(["@metamask/money-account-api-data-service"]); money_account_balance_service(["@metamask/money-account-balance-service"]); money_account_controller(["@metamask/money-account-controller"]); diff --git a/codeowners.ts b/codeowners.ts index b497380a2d..62a3ae477c 100644 --- a/codeowners.ts +++ b/codeowners.ts @@ -203,6 +203,9 @@ const PACKAGES: Record = { 'message-manager': { teams: ['@MetaMask/confirmations'], }, + 'mfa-recovery-controller': { + teams: ['@MetaMask/web3auth'], + }, messenger: { teams: ['@MetaMask/core-platform'], }, @@ -570,6 +573,7 @@ function buildTeamSections(): CodeownersSection[] { rules: [ buildRuleForPackage('seedless-onboarding-controller'), buildRuleForPackage('passkey-controller'), + buildRuleForPackage('mfa-recovery-controller'), buildRuleForPackage('shield-controller'), buildRuleForPackage('subscription-controller'), buildRuleForPackage('claims-controller'), @@ -672,6 +676,7 @@ function buildPackageReleaseSection(): CodeownersSection { 'geolocation-controller', 'keyring-controller', 'passkey-controller', + 'mfa-recovery-controller', 'logging-controller', 'message-manager', 'multichain-account-service', diff --git a/packages/mfa-recovery-controller/CHANGELOG.md b/packages/mfa-recovery-controller/CHANGELOG.md new file mode 100644 index 0000000000..b1bd921bd9 --- /dev/null +++ b/packages/mfa-recovery-controller/CHANGELOG.md @@ -0,0 +1,14 @@ +# Changelog + +All notable changes to this project will be documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), +and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +## [Unreleased] + +### Added + +- Add `MfaRecoveryController` with injectable `RecoveryAuthProvider` and `RecoveryEscrowProvider` interfaces, a persisted `idle` / `authorizing` / `writing` mutation state machine, and `register`, `updateRecoverySecret`, `updateIdentifiers`, `getRecoverySecret`, `resume`, and `abort` methods + +[Unreleased]: https://github.com/MetaMask/core/ diff --git a/packages/mfa-recovery-controller/LICENSE b/packages/mfa-recovery-controller/LICENSE new file mode 100644 index 0000000000..9ec4f4514e --- /dev/null +++ b/packages/mfa-recovery-controller/LICENSE @@ -0,0 +1,6 @@ +This project is licensed under either of + + * MIT license ([LICENSE.MIT](LICENSE.MIT)) + * Apache License, Version 2.0 ([LICENSE.APACHE2](LICENSE.APACHE2)) + +at your option. diff --git a/packages/mfa-recovery-controller/LICENSE.APACHE2 b/packages/mfa-recovery-controller/LICENSE.APACHE2 new file mode 100644 index 0000000000..e6e77b0890 --- /dev/null +++ b/packages/mfa-recovery-controller/LICENSE.APACHE2 @@ -0,0 +1,201 @@ + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright [yyyy] [name of copyright owner] + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. diff --git a/packages/mfa-recovery-controller/LICENSE.MIT b/packages/mfa-recovery-controller/LICENSE.MIT new file mode 100644 index 0000000000..fe29e78e0f --- /dev/null +++ b/packages/mfa-recovery-controller/LICENSE.MIT @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2026 MetaMask + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/packages/mfa-recovery-controller/README.md b/packages/mfa-recovery-controller/README.md new file mode 100644 index 0000000000..ba45a0667f --- /dev/null +++ b/packages/mfa-recovery-controller/README.md @@ -0,0 +1,21 @@ +# `@metamask/mfa-recovery-controller` + +Manages MFA recovery flows across MetaMask clients. Auth and escrow storage are +injected so the same controller can run against different identity providers and +escrow backends. + +Pending mutations are persisted as encrypted `authorizing` / `writing` state so +a crash can `resume()` the same mutation. `abort()` is allowed only before the +first escrow write. + +## Installation + +`yarn add @metamask/mfa-recovery-controller` + +or + +`npm install @metamask/mfa-recovery-controller` + +## Contributing + +This package is part of a monorepo. Instructions for contributing can be found in the [monorepo README](https://github.com/MetaMask/core#readme). diff --git a/packages/mfa-recovery-controller/jest.config.js b/packages/mfa-recovery-controller/jest.config.js new file mode 100644 index 0000000000..ca08413339 --- /dev/null +++ b/packages/mfa-recovery-controller/jest.config.js @@ -0,0 +1,26 @@ +/* + * For a detailed explanation regarding each configuration property and type check, visit: + * https://jestjs.io/docs/configuration + */ + +const merge = require('deepmerge'); +const path = require('path'); + +const baseConfig = require('../../jest.config.packages'); + +const displayName = path.basename(__dirname); + +module.exports = merge(baseConfig, { + // The display name when running multiple projects + displayName, + + // An object that configures minimum threshold enforcement for coverage results + coverageThreshold: { + global: { + branches: 100, + functions: 100, + lines: 100, + statements: 100, + }, + }, +}); diff --git a/packages/mfa-recovery-controller/package.json b/packages/mfa-recovery-controller/package.json new file mode 100644 index 0000000000..6afdc4e99c --- /dev/null +++ b/packages/mfa-recovery-controller/package.json @@ -0,0 +1,77 @@ +{ + "name": "@metamask/mfa-recovery-controller", + "version": "0.0.0", + "description": "Manages MFA recovery flows across MetaMask clients", + "keywords": [ + "Ethereum", + "MetaMask" + ], + "homepage": "https://github.com/MetaMask/core/tree/main/packages/mfa-recovery-controller#readme", + "bugs": { + "url": "https://github.com/MetaMask/core/issues" + }, + "license": "(MIT OR Apache-2.0)", + "repository": { + "type": "git", + "url": "https://github.com/MetaMask/core.git" + }, + "files": [ + "dist/" + ], + "sideEffects": false, + "main": "./dist/index.cjs", + "types": "./dist/index.d.cts", + "exports": { + ".": { + "import": { + "types": "./dist/index.d.mts", + "default": "./dist/index.mjs" + }, + "require": { + "types": "./dist/index.d.cts", + "default": "./dist/index.cjs" + } + }, + "./package.json": "./package.json" + }, + "publishConfig": { + "access": "public", + "registry": "https://registry.npmjs.org/" + }, + "scripts": { + "build": "ts-bridge --project tsconfig.build.json --verbose --clean --no-references", + "build:all": "ts-bridge --project tsconfig.build.json --verbose --clean", + "build:docs": "typedoc", + "changelog:update": "../../scripts/update-changelog.sh @metamask/mfa-recovery-controller", + "changelog:validate": "../../scripts/validate-changelog.sh @metamask/mfa-recovery-controller", + "lint:tsconfigs": "tsx ../../scripts/lint-tsconfigs/lint-tsconfigs.mts", + "lint:tsconfigs:fix": "tsx ../../scripts/lint-tsconfigs/lint-tsconfigs.mts --fix", + "messenger-action-types:check": "tsx ../../packages/messenger-cli/src/cli.ts --formatter oxfmt --esm --check", + "messenger-action-types:generate": "tsx ../../packages/messenger-cli/src/cli.ts --formatter oxfmt --esm --generate", + "since-latest-release": "../../scripts/since-latest-release.sh", + "test": "NODE_OPTIONS=--experimental-vm-modules jest --reporters=jest-silent-reporter", + "test:clean": "NODE_OPTIONS=--experimental-vm-modules jest --clearCache", + "test:verbose": "NODE_OPTIONS=--experimental-vm-modules jest --verbose", + "test:watch": "NODE_OPTIONS=--experimental-vm-modules jest --watch" + }, + "dependencies": { + "@metamask/base-controller": "^9.1.0", + "@metamask/messenger": "^2.0.0", + "@metamask/utils": "^11.11.0" + }, + "devDependencies": { + "@metamask/auto-changelog": "^6.1.0", + "@ts-bridge/cli": "^0.6.4", + "@types/jest": "^30.0.0", + "deepmerge": "^4.2.2", + "jest": "^30.4.2", + "ts-jest": "^29.4.11", + "tsx": "^4.20.5", + "typedoc": "^0.25.13", + "typedoc-plugin-missing-exports": "^2.0.0", + "typescript": "~5.3.3" + }, + "engines": { + "node": "^18.18 || >=20" + } +} diff --git a/packages/mfa-recovery-controller/src/MfaRecoveryController-method-action-types.ts b/packages/mfa-recovery-controller/src/MfaRecoveryController-method-action-types.ts new file mode 100644 index 0000000000..cd20e9023c --- /dev/null +++ b/packages/mfa-recovery-controller/src/MfaRecoveryController-method-action-types.ts @@ -0,0 +1,88 @@ +/** + * This file is auto generated. + * Do not edit manually. + */ + +import type { MfaRecoveryController } from './MfaRecoveryController.js'; + +/** + * Creates version 1 of a recovery record. + * + * @param recoverySecret - Secret replicated in full to every escrow. + * @param identifiers - Ownership-approved identifier set. Must be non-empty. + */ +export type MfaRecoveryControllerRegisterAction = { + type: `MfaRecoveryController:register`; + handler: MfaRecoveryController['register']; +}; + +/** + * Replaces the recovery secret. + * + * @param identifier - Currently registered identifier used to authorize. + * @param recoverySecret - New secret. + */ +export type MfaRecoveryControllerUpdateRecoverySecretAction = { + type: `MfaRecoveryController:updateRecoverySecret`; + handler: MfaRecoveryController['updateRecoverySecret']; +}; + +/** + * Replaces the complete identifier set. + * + * @param identifier - Currently registered identifier used to authorize. + * @param identifiers - New non-empty identifier set. + */ +export type MfaRecoveryControllerUpdateIdentifiersAction = { + type: `MfaRecoveryController:updateIdentifiers`; + handler: MfaRecoveryController['updateIdentifiers']; +}; + +/** + * Reads the recovery secret from available escrows and returns the highest + * consistent version. + * + * @param identifier - Identifier used to authorize the read. + * @returns Recovered secret bytes. + */ +export type MfaRecoveryControllerGetRecoverySecretAction = { + type: `MfaRecoveryController:getRecoverySecret`; + handler: MfaRecoveryController['getRecoverySecret']; +}; + +/** + * Completes a persisted pending mutation, if any. + */ +export type MfaRecoveryControllerResumeAction = { + type: `MfaRecoveryController:resume`; + handler: MfaRecoveryController['resume']; +}; + +/** + * Drops a mutation that has not yet begun writing. Writing mutations must be + * resumed instead. + */ +export type MfaRecoveryControllerAbortAction = { + type: `MfaRecoveryController:abort`; + handler: MfaRecoveryController['abort']; +}; + +/** + * @returns Current recovery phase. + */ +export type MfaRecoveryControllerGetPhaseAction = { + type: `MfaRecoveryController:getPhase`; + handler: MfaRecoveryController['getPhase']; +}; + +/** + * Union of all MfaRecoveryController action types. + */ +export type MfaRecoveryControllerMethodActions = + | MfaRecoveryControllerRegisterAction + | MfaRecoveryControllerUpdateRecoverySecretAction + | MfaRecoveryControllerUpdateIdentifiersAction + | MfaRecoveryControllerGetRecoverySecretAction + | MfaRecoveryControllerResumeAction + | MfaRecoveryControllerAbortAction + | MfaRecoveryControllerGetPhaseAction; diff --git a/packages/mfa-recovery-controller/src/MfaRecoveryController.test.ts b/packages/mfa-recovery-controller/src/MfaRecoveryController.test.ts new file mode 100644 index 0000000000..c6e17f2c2a --- /dev/null +++ b/packages/mfa-recovery-controller/src/MfaRecoveryController.test.ts @@ -0,0 +1,744 @@ +import { deriveStateFromMetadata } from '@metamask/base-controller'; +import { Messenger, MOCK_ANY_NAMESPACE } from '@metamask/messenger'; +import type { + MockAnyNamespace, + MessengerActions, + MessengerEvents, +} from '@metamask/messenger'; +import { bytesToHex } from '@metamask/utils'; + +import { + StubAuthProvider, + StubEscrowProvider, + StubIdentifierAuthProvider, + passthroughEncryptor, +} from '../tests/stubs.js'; +import { hash } from './crypto.js'; +import { MutationRepairPendingError } from './errors.js'; +import type { + MfaRecoveryControllerMessenger, + MfaRecoveryControllerOptions, +} from './MfaRecoveryController.js'; +import { MfaRecoveryController } from './MfaRecoveryController.js'; +import type { Identifier } from './types.js'; + +const PASSKEY: Identifier = { + type: 'passkey', + namespace: 'example.com', + value: 'cred-1', + verifier: { publicKey: 'pk' }, +}; + +const EMAIL: Identifier = { + type: 'emailOtp', + namespace: 'email', + value: 'user@example.com', + verifier: null, +}; + +const SECRET = new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8]); +const SECRET_2 = new Uint8Array([8, 7, 6, 5, 4, 3, 2, 1]); + +describe('MfaRecoveryController', () => { + describe('constructor', () => { + it('fills in missing initial state with defaults', async () => { + await withController(({ controller }) => { + expect(controller.state).toMatchInlineSnapshot(` + { + "pendingOperation": null, + } + `); + }); + }); + + it('throws if no escrows are configured', () => { + const rootMessenger = getRootMessenger(); + expect( + () => + new MfaRecoveryController({ + messenger: getMessenger(rootMessenger), + authProvider: new StubAuthProvider(), + identifierAuthProvider: new StubIdentifierAuthProvider(), + escrows: [], + pendingOperationEncryptor: passthroughEncryptor, + collectChallengeResponse: async (): Promise => 'otp', + }), + ).toThrow('At least one escrow is required'); + }); + + it('throws if escrow ids are duplicated', () => { + const rootMessenger = getRootMessenger(); + expect( + () => + new MfaRecoveryController({ + messenger: getMessenger(rootMessenger), + authProvider: new StubAuthProvider(), + identifierAuthProvider: new StubIdentifierAuthProvider(), + escrows: [ + new StubEscrowProvider('escrow-a'), + new StubEscrowProvider('escrow-a'), + ], + pendingOperationEncryptor: passthroughEncryptor, + collectChallengeResponse: async (): Promise => 'otp', + }), + ).toThrow('Duplicate escrow id'); + }); + }); + + describe('metadata', () => { + it('includes expected state in debug snapshots', async () => { + await withController(({ controller }) => { + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'includeInDebugSnapshot', + ), + ).toMatchInlineSnapshot(`{}`); + }); + }); + + it('includes expected state in state logs', async () => { + await withController(({ controller }) => { + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'includeInStateLogs', + ), + ).toMatchInlineSnapshot(`{}`); + }); + }); + + it('persists expected state', async () => { + await withController(({ controller }) => { + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'persist', + ), + ).toMatchInlineSnapshot(` + { + "pendingOperation": null, + } + `); + }); + }); + + it('exposes expected state to UI', async () => { + await withController(({ controller }) => { + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'usedInUi', + ), + ).toMatchInlineSnapshot(` + { + "pendingOperation": null, + } + `); + }); + }); + }); + + describe('register', () => { + it('replicates the recovery secret to every escrow', async () => { + await withController(async ({ controller }) => { + await controller.register(SECRET, [PASSKEY]); + expect(controller.state.pendingOperation).toBeNull(); + expect(await controller.getPhase()).toBe('idle'); + + const recovered = await controller.getRecoverySecret(PASSKEY); + expect(bytesToHex(recovered)).toBe(bytesToHex(SECRET)); + }); + }); + + it('rejects an empty identifier list', async () => { + await withController(async ({ controller }) => { + await expect(controller.register(SECRET, [])).rejects.toThrow( + 'Registration requires at least one identifier', + ); + }); + }); + + it('rejects a second registration for the same profile', async () => { + await withController(async ({ controller }) => { + await controller.register(SECRET, [PASSKEY]); + await expect(controller.register(SECRET, [PASSKEY])).rejects.toThrow( + 'Profile is already registered', + ); + }); + }); + }); + + describe('getRecoverySecret', () => { + it('returns the highest consistent version when one escrow is down', async () => { + await withController(async ({ controller, escrowB }) => { + await controller.register(SECRET, [PASSKEY]); + escrowB.available = false; + const recovered = await controller.getRecoverySecret(PASSKEY); + expect(bytesToHex(recovered)).toBe(bytesToHex(SECRET)); + }); + }); + + it('throws when matching versions disagree', async () => { + await withController(async ({ controller, escrowB }) => { + await controller.register(SECRET, [PASSKEY]); + escrowB.patchRecord('profile-1', { + lastMutationId: '0xdifferent', + }); + await expect(controller.getRecoverySecret(PASSKEY)).rejects.toThrow( + 'Replica corruption', + ); + }); + }); + + it('throws when no escrow is available', async () => { + await withController(async ({ controller, escrowA, escrowB }) => { + escrowA.available = false; + escrowB.available = false; + await expect(controller.getRecoverySecret(PASSKEY)).rejects.toThrow( + 'No escrow is available', + ); + }); + }); + + it('throws when every available escrow fails the read', async () => { + await withController(async ({ controller, escrowA, escrowB }) => { + await controller.register(SECRET, [PASSKEY]); + escrowA.failGetSecret = true; + escrowB.failGetSecret = true; + await expect(controller.getRecoverySecret(PASSKEY)).rejects.toThrow( + 'No escrow returned a recovery secret', + ); + }); + }); + }); + + describe('updateRecoverySecret', () => { + it('replaces the secret at every escrow', async () => { + await withController(async ({ controller }) => { + await controller.register(SECRET, [PASSKEY]); + await controller.updateRecoverySecret(PASSKEY, SECRET_2); + const recovered = await controller.getRecoverySecret(PASSKEY); + expect(bytesToHex(recovered)).toBe(bytesToHex(SECRET_2)); + }); + }); + + it('throws when a configured escrow is unavailable', async () => { + await withController(async ({ controller, escrowB }) => { + await controller.register(SECRET, [PASSKEY]); + escrowB.available = false; + await expect( + controller.updateRecoverySecret(PASSKEY, SECRET_2), + ).rejects.toThrow('All configured escrows are required for mutation'); + }); + }); + + it('throws when replicas disagree on version', async () => { + await withController(async ({ controller, escrowB }) => { + await controller.register(SECRET, [PASSKEY]); + escrowB.patchRecord('profile-1', { version: 2 }); + await expect( + controller.updateRecoverySecret(PASSKEY, SECRET_2), + ).rejects.toThrow('Escrows disagree on recovery version'); + }); + }); + + it('throws when a replica is missing the recovery record', async () => { + await withController(async ({ controller, escrowB }) => { + await controller.register(SECRET, [PASSKEY]); + escrowB.clearRecord('profile-1'); + await expect( + controller.updateRecoverySecret(PASSKEY, SECRET_2), + ).rejects.toThrow('Recovery record missing at a configured escrow'); + }); + }); + }); + + describe('updateIdentifiers', () => { + it('replaces the identifier set using escrow-challenge auth', async () => { + await withController(async ({ controller }) => { + await controller.register(SECRET, [PASSKEY, EMAIL]); + await controller.updateIdentifiers(EMAIL, [EMAIL]); + const recovered = await controller.getRecoverySecret(EMAIL); + expect(bytesToHex(recovered)).toBe(bytesToHex(SECRET)); + }); + }); + + it('rejects an empty identifier list', async () => { + await withController(async ({ controller }) => { + await controller.register(SECRET, [PASSKEY]); + await expect(controller.updateIdentifiers(PASSKEY, [])).rejects.toThrow( + 'Identifier list must be non-empty', + ); + }); + }); + }); + + describe('resume and abort', () => { + it('resumes a persisted writing mutation after a replica failure', async () => { + await withController(async ({ controller, escrowA }) => { + await controller.register(SECRET, [PASSKEY]); + escrowA.failNextApplyCount = 1; + await expect( + controller.updateRecoverySecret(PASSKEY, SECRET_2), + ).rejects.toBeInstanceOf(MutationRepairPendingError); + expect(await controller.getPhase()).toBe('writing'); + + await controller.resume(); + expect(await controller.getPhase()).toBe('idle'); + const recovered = await controller.getRecoverySecret(PASSKEY); + expect(bytesToHex(recovered)).toBe(bytesToHex(SECRET_2)); + }); + }); + + it('aborts an authorizing mutation and refuses to abort writing', async () => { + await withController(async ({ options }) => { + const authorizing = await passthroughEncryptor.encrypt({ + phase: 'authorizing', + mutation: { + id: '0xmut', + profileId: 'profile-1', + operation: 'register', + expectedVersion: 0, + newVersion: 1, + payloadHash: '0x', + audiences: ['escrow-a', 'escrow-b'], + requestHash: '0x', + }, + payload: { + recoverySecret: bytesToHex(SECRET), + identifiers: [PASSKEY], + }, + identifier: null, + }); + const authorizingMessenger = getMessenger(getRootMessenger()); + const idleController = new MfaRecoveryController({ + ...options, + messenger: authorizingMessenger, + state: { pendingOperation: authorizing }, + }); + expect(await idleController.getPhase()).toBe('authorizing'); + await idleController.abort(); + expect(await idleController.getPhase()).toBe('idle'); + + const writing = await passthroughEncryptor.encrypt({ + phase: 'writing', + mutation: { + id: '0xmut', + profileId: 'profile-1', + operation: 'register', + expectedVersion: 0, + newVersion: 1, + payloadHash: '0x', + audiences: ['escrow-a', 'escrow-b'], + requestHash: '0x', + }, + authControllerToken: { + profileId: 'profile-1', + requestHash: '0x', + issuer: 'stub-auth', + expiresAt: Date.now() + 60_000, + signature: 'stub-auth-signature', + }, + payload: { + recoverySecret: bytesToHex(SECRET), + identifiers: [PASSKEY], + }, + identifier: null, + receipts: [], + }); + const writingMessenger = getMessenger(getRootMessenger()); + const writingController = new MfaRecoveryController({ + ...options, + messenger: writingMessenger, + state: { pendingOperation: writing }, + }); + await expect(writingController.abort()).rejects.toThrow( + 'Cannot abort a mutation once writing has begun', + ); + expect(await writingController.getPhase()).toBe('writing'); + }); + }); + + it('resumes an authorizing mutation', async () => { + await withController(async ({ options }) => { + const payload = { + recoverySecret: bytesToHex(SECRET), + identifiers: [PASSKEY], + }; + const payloadHash = await hash(payload); + const fields = { + id: '0xmutauth', + profileId: 'profile-1', + operation: 'register' as const, + expectedVersion: 0, + newVersion: 1, + payloadHash, + audiences: ['escrow-a', 'escrow-b'], + }; + const mutation = { + ...fields, + requestHash: await hash(fields), + }; + const authorizing = await passthroughEncryptor.encrypt({ + phase: 'authorizing', + mutation, + payload, + identifier: null, + }); + const controller = new MfaRecoveryController({ + ...options, + messenger: getMessenger(getRootMessenger()), + state: { pendingOperation: authorizing }, + }); + await controller.resume(); + expect(await controller.getPhase()).toBe('idle'); + expect(bytesToHex(await controller.getRecoverySecret(PASSKEY))).toBe( + bytesToHex(SECRET), + ); + }); + }); + + it('is a no-op to resume or abort when idle', async () => { + await withController(async ({ controller }) => { + await controller.resume(); + await controller.abort(); + expect(await controller.getPhase()).toBe('idle'); + }); + }); + + it('clears a writing mutation that already has every receipt', async () => { + await withController(async ({ controller, options }) => { + await controller.register(SECRET, [PASSKEY]); + const mutation = { + id: '0xmut', + profileId: 'profile-1', + operation: 'updateRecoverySecret' as const, + expectedVersion: 1, + newVersion: 2, + payloadHash: await hash({ + recoverySecret: bytesToHex(SECRET_2), + }), + audiences: ['escrow-a', 'escrow-b'], + requestHash: '0x', + }; + mutation.requestHash = await hash({ + id: mutation.id, + profileId: mutation.profileId, + operation: mutation.operation, + expectedVersion: mutation.expectedVersion, + newVersion: mutation.newVersion, + payloadHash: mutation.payloadHash, + audiences: mutation.audiences, + }); + const receipts = [ + { + mutationId: mutation.id, + requestHash: mutation.requestHash, + escrowId: 'escrow-a', + version: 2, + signature: '0x', + }, + { + mutationId: mutation.id, + requestHash: mutation.requestHash, + escrowId: 'escrow-b', + version: 2, + signature: '0x', + }, + ]; + const writing = await passthroughEncryptor.encrypt({ + phase: 'writing', + mutation, + authControllerToken: { + profileId: 'profile-1', + requestHash: mutation.requestHash, + twoFactor: true, + issuer: 'stub-auth', + expiresAt: Date.now() + 60_000, + signature: 'stub-auth-signature', + }, + payload: { recoverySecret: bytesToHex(SECRET_2) }, + identifier: PASSKEY, + receipts, + }); + const resumed = new MfaRecoveryController({ + ...options, + messenger: getMessenger(getRootMessenger()), + state: { pendingOperation: writing }, + }); + await resumed.resume(); + expect(await resumed.getPhase()).toBe('idle'); + }); + }); + + it('rejects a persisted mutation with the wrong escrow audience', async () => { + await withController(async ({ options }) => { + const writing = await passthroughEncryptor.encrypt({ + phase: 'authorizing', + mutation: { + id: '0xmut', + profileId: 'profile-1', + operation: 'register', + expectedVersion: 0, + newVersion: 1, + payloadHash: '0x', + audiences: ['escrow-other'], + requestHash: '0x', + }, + payload: { + recoverySecret: bytesToHex(SECRET), + identifiers: [PASSKEY], + }, + identifier: null, + }); + const controller = new MfaRecoveryController({ + ...options, + messenger: getMessenger(getRootMessenger()), + state: { pendingOperation: writing }, + }); + await expect(controller.resume()).rejects.toThrow( + 'Persisted mutation audiences do not match configured escrows', + ); + }); + }); + + it('rejects a pending payload that does not match the mutation hash', async () => { + await withController(async ({ options }) => { + const writing = await passthroughEncryptor.encrypt({ + phase: 'writing', + mutation: { + id: '0xmut', + profileId: 'profile-1', + operation: 'register', + expectedVersion: 0, + newVersion: 1, + payloadHash: '0xdead', + audiences: ['escrow-a', 'escrow-b'], + requestHash: '0x', + }, + authControllerToken: { + profileId: 'profile-1', + requestHash: '0x', + issuer: 'stub-auth', + expiresAt: Date.now() + 60_000, + signature: 'stub-auth-signature', + }, + payload: { + recoverySecret: bytesToHex(SECRET), + identifiers: [PASSKEY], + }, + identifier: null, + receipts: [], + }); + const controller = new MfaRecoveryController({ + ...options, + messenger: getMessenger(getRootMessenger()), + state: { pendingOperation: writing }, + }); + await expect(controller.resume()).rejects.toThrow( + 'Pending payload does not match mutation', + ); + }); + }); + + it('rejects a receipt from an unknown escrow', async () => { + await withController(async ({ options }) => { + const payload = { + recoverySecret: bytesToHex(SECRET), + identifiers: [PASSKEY], + }; + const payloadHash = await hash(payload); + const writing = await passthroughEncryptor.encrypt({ + phase: 'writing', + mutation: { + id: '0xmut', + profileId: 'profile-1', + operation: 'register', + expectedVersion: 0, + newVersion: 1, + payloadHash, + audiences: ['escrow-a', 'escrow-b'], + requestHash: '0x', + }, + authControllerToken: { + profileId: 'profile-1', + requestHash: '0x', + issuer: 'stub-auth', + expiresAt: Date.now() + 60_000, + signature: 'stub-auth-signature', + }, + payload, + identifier: null, + receipts: [ + { + mutationId: '0xmut', + requestHash: '0x', + escrowId: 'escrow-z', + version: 1, + signature: '0x', + }, + ], + }); + const controller = new MfaRecoveryController({ + ...options, + messenger: getMessenger(getRootMessenger()), + state: { pendingOperation: writing }, + }); + await expect(controller.resume()).rejects.toThrow( + 'Receipt escrow is not configured', + ); + }); + }); + + it('rejects an invalid persisted receipt', async () => { + await withController(async ({ options, escrowA }) => { + escrowA.invalidReceipts = true; + const payload = { + recoverySecret: bytesToHex(SECRET), + identifiers: [PASSKEY], + }; + const payloadHash = await hash(payload); + const writing = await passthroughEncryptor.encrypt({ + phase: 'writing', + mutation: { + id: '0xmut', + profileId: 'profile-1', + operation: 'register', + expectedVersion: 0, + newVersion: 1, + payloadHash, + audiences: ['escrow-a', 'escrow-b'], + requestHash: '0x', + }, + authControllerToken: { + profileId: 'profile-1', + requestHash: '0x', + issuer: 'stub-auth', + expiresAt: Date.now() + 60_000, + signature: 'stub-auth-signature', + }, + payload, + identifier: null, + receipts: [ + { + mutationId: '0xmut', + requestHash: '0x', + escrowId: 'escrow-a', + version: 1, + signature: '0x', + }, + ], + }); + const controller = new MfaRecoveryController({ + ...options, + messenger: getMessenger(getRootMessenger()), + state: { pendingOperation: writing }, + }); + await expect(controller.resume()).rejects.toThrow( + 'Invalid mutation receipt', + ); + }); + }); + + it('rejects a newly issued invalid receipt', async () => { + await withController(async ({ controller, escrowA }) => { + escrowA.invalidReceipts = true; + await expect(controller.register(SECRET, [PASSKEY])).rejects.toThrow( + 'Invalid mutation receipt', + ); + }); + }); + + it('refreshes an expired AuthController token while resuming', async () => { + await withController(async ({ controller, escrowA, options }) => { + await controller.register(SECRET, [PASSKEY]); + escrowA.failNextApplyCount = 1; + await expect( + controller.updateRecoverySecret(PASSKEY, SECRET_2), + ).rejects.toBeInstanceOf(MutationRepairPendingError); + const pending = JSON.parse( + controller.state.pendingOperation as string, + ) as { authControllerToken: { expiresAt: number } }; + pending.authControllerToken.expiresAt = 0; + const resumed = new MfaRecoveryController({ + ...options, + messenger: getMessenger(getRootMessenger()), + state: { pendingOperation: JSON.stringify(pending) }, + }); + await resumed.resume(); + expect(await resumed.getPhase()).toBe('idle'); + }); + }); + }); + + describe('identifier registry', () => { + it('rejects unknown identifier types', async () => { + await withController(async ({ controller }) => { + await expect( + controller.register(SECRET, [ + { type: 'unknown', namespace: 'x', value: 'y', verifier: null }, + ]), + ).rejects.toThrow('Unknown identifier type'); + }); + }); + }); +}); + +type RootMessenger = Messenger< + MockAnyNamespace, + MessengerActions, + MessengerEvents +>; + +type WithControllerCallback = (payload: { + controller: MfaRecoveryController; + rootMessenger: RootMessenger; + controllerMessenger: MfaRecoveryControllerMessenger; + escrowA: StubEscrowProvider; + escrowB: StubEscrowProvider; + options: MfaRecoveryControllerOptions; +}) => Promise | ReturnValue; + +function getRootMessenger(): RootMessenger { + return new Messenger({ namespace: MOCK_ANY_NAMESPACE }); +} + +function getMessenger( + rootMessenger: RootMessenger, +): MfaRecoveryControllerMessenger { + return new Messenger({ + namespace: 'MfaRecoveryController', + parent: rootMessenger, + }); +} + +async function withController( + testFunction: WithControllerCallback, +): Promise { + const rootMessenger = getRootMessenger(); + const controllerMessenger = getMessenger(rootMessenger); + const escrowA = new StubEscrowProvider('escrow-a'); + const escrowB = new StubEscrowProvider('escrow-b'); + const options: MfaRecoveryControllerOptions = { + messenger: controllerMessenger, + authProvider: new StubAuthProvider(), + identifierAuthProvider: new StubIdentifierAuthProvider(), + escrows: [escrowA, escrowB], + pendingOperationEncryptor: passthroughEncryptor, + collectChallengeResponse: async (): Promise => 'otp', + }; + const controller = new MfaRecoveryController(options); + return await testFunction({ + controller, + rootMessenger, + controllerMessenger, + escrowA, + escrowB, + options, + }); +} diff --git a/packages/mfa-recovery-controller/src/MfaRecoveryController.ts b/packages/mfa-recovery-controller/src/MfaRecoveryController.ts new file mode 100644 index 0000000000..caecc45763 --- /dev/null +++ b/packages/mfa-recovery-controller/src/MfaRecoveryController.ts @@ -0,0 +1,783 @@ +import type { + ControllerGetStateAction, + ControllerStateChangeEvent, + StateMetadata, +} from '@metamask/base-controller'; +import { BaseController } from '@metamask/base-controller'; +import type { Messenger } from '@metamask/messenger'; +import { areUint8ArraysEqual } from '@metamask/utils'; + +import { + bytesToSecretHex, + generateSigningKey, + hash, + randomId, + sign, +} from './crypto.js'; +import { MfaRecoveryError, MutationRepairPendingError } from './errors.js'; +import { getIdentifierAuthMode } from './identifier-auth.js'; +import type { MfaRecoveryControllerMethodActions } from './MfaRecoveryController-method-action-types.js'; +import { + assertAbortAllowed, + assertSameAudienceIds, + getRecoveryPhase, +} from './state-machine.js'; +import type { + AuthControllerToken, + EscrowAuthChallenge, + Identifier, + IdentifierAuthorization, + Mutation, + MutationPayload, + MutationReceipt, + PendingOperation, + PendingOperationEncryptor, + RecoveryAuthProvider, + RecoveryEscrowProvider, + RecoveryIdentifierAuthProvider, + RecoveryPhase, + RegisterPayload, + UpdateIdentifiersPayload, + WritingPendingOperation, +} from './types.js'; + +export type { + AuthControllerToken, + EncryptedPendingOperation, + Identifier, + IdentifierAuthorization, + Mutation, + MutationPayload, + MutationReceipt, + PendingOperation, + PendingOperationEncryptor, + RecoveryAuthProvider, + RecoveryEscrowProvider, + RecoveryIdentifierAuthProvider, + RecoveryPhase, +} from './types.js'; + +const CONTROLLER_NAME = 'MfaRecoveryController'; + +const MESSENGER_EXPOSED_METHODS = [ + 'register', + 'updateRecoverySecret', + 'updateIdentifiers', + 'getRecoverySecret', + 'resume', + 'abort', + 'getPhase', +] as const; + +export type MfaRecoveryControllerState = { + /** + * Encrypted pending mutation, or `null` when idle. + */ + pendingOperation: string | null; +}; + +const mfaRecoveryControllerMetadata = { + pendingOperation: { + persist: true, + includeInDebugSnapshot: false, + includeInStateLogs: false, + usedInUi: true, + }, +} satisfies StateMetadata; + +/** + * @returns The default {@link MfaRecoveryController} state. + */ +export function getDefaultMfaRecoveryControllerState(): MfaRecoveryControllerState { + return { pendingOperation: null }; +} + +export type MfaRecoveryControllerGetStateAction = ControllerGetStateAction< + typeof CONTROLLER_NAME, + MfaRecoveryControllerState +>; + +export type MfaRecoveryControllerActions = + | MfaRecoveryControllerGetStateAction + | MfaRecoveryControllerMethodActions; + +type AllowedActions = never; + +export type MfaRecoveryControllerStateChangeEvent = ControllerStateChangeEvent< + typeof CONTROLLER_NAME, + MfaRecoveryControllerState +>; + +export type MfaRecoveryControllerEvents = MfaRecoveryControllerStateChangeEvent; + +type AllowedEvents = never; + +export type MfaRecoveryControllerMessenger = Messenger< + typeof CONTROLLER_NAME, + MfaRecoveryControllerActions | AllowedActions, + MfaRecoveryControllerEvents | AllowedEvents +>; + +export type MfaRecoveryControllerOptions = { + /** + * The messenger suited for this controller. + */ + messenger: MfaRecoveryControllerMessenger; + + /** + * The desired state with which to initialize this controller. Missing + * properties will be filled in with defaults. + */ + state?: Partial; + + /** + * Authenticates the MetaMask profile and issues request-bound attestations. + */ + authProvider: RecoveryAuthProvider; + + /** + * Obtains key-bound identifier tokens from an IdP. + */ + identifierAuthProvider: RecoveryIdentifierAuthProvider; + + /** + * Ordered, non-empty set of recovery escrow replicas. This is build-time + * configuration; public methods cannot override it. + */ + escrows: RecoveryEscrowProvider[]; + + /** + * Encrypts pending mutation state using wallet secure storage. + */ + pendingOperationEncryptor: PendingOperationEncryptor; + + /** + * Collects a provider-specific response for an escrow-challenge identifier + * (for example an Email/SMS OTP). + */ + collectChallengeResponse: ( + challenge: EscrowAuthChallenge, + ) => Promise; + + /** + * Clock used for AuthController token expiry checks. Defaults to `Date.now`. + */ + now?: () => number; +}; + +/** + * Coordinates MFA recovery secret replication across independent escrows. + */ +export class MfaRecoveryController extends BaseController< + typeof CONTROLLER_NAME, + MfaRecoveryControllerState, + MfaRecoveryControllerMessenger +> { + readonly #authProvider: RecoveryAuthProvider; + + readonly #identifierAuthProvider: RecoveryIdentifierAuthProvider; + + readonly #escrows: RecoveryEscrowProvider[]; + + readonly #encryptor: PendingOperationEncryptor; + + readonly #collectChallengeResponse: ( + challenge: EscrowAuthChallenge, + ) => Promise; + + readonly #now: () => number; + + #lock: Promise = Promise.resolve(); + + constructor({ + messenger, + state, + authProvider, + identifierAuthProvider, + escrows, + pendingOperationEncryptor, + collectChallengeResponse, + now = (): number => Date.now(), + }: MfaRecoveryControllerOptions) { + if (escrows.length === 0) { + throw new MfaRecoveryError( + 'At least one escrow is required', + 'empty_escrow_set', + ); + } + const ids = escrows.map((escrow) => escrow.id); + if (new Set(ids).size !== ids.length) { + throw new MfaRecoveryError('Duplicate escrow id', 'duplicate_escrow_id'); + } + + super({ + messenger, + metadata: mfaRecoveryControllerMetadata, + name: CONTROLLER_NAME, + state: { + ...getDefaultMfaRecoveryControllerState(), + ...state, + }, + }); + + this.#authProvider = authProvider; + this.#identifierAuthProvider = identifierAuthProvider; + this.#escrows = escrows; + this.#encryptor = pendingOperationEncryptor; + this.#collectChallengeResponse = collectChallengeResponse; + this.#now = now; + + this.messenger.registerMethodActionHandlers( + this, + MESSENGER_EXPOSED_METHODS, + ); + } + + /** + * Creates version 1 of a recovery record. + * + * @param recoverySecret - Secret replicated in full to every escrow. + * @param identifiers - Ownership-approved identifier set. Must be non-empty. + */ + async register( + recoverySecret: Uint8Array, + identifiers: Identifier[], + ): Promise { + this.#assertKnownIdentifierTypes(identifiers); + if (identifiers.length === 0) { + throw new MfaRecoveryError( + 'Registration requires at least one identifier', + 'empty_identifiers', + ); + } + await this.#runRecoveryMutation({ + operation: 'register', + payload: { + recoverySecret: bytesToSecretHex(recoverySecret), + identifiers, + }, + identifier: null, + }); + } + + /** + * Replaces the recovery secret. + * + * @param identifier - Currently registered identifier used to authorize. + * @param recoverySecret - New secret. + */ + async updateRecoverySecret( + identifier: Identifier, + recoverySecret: Uint8Array, + ): Promise { + this.#assertKnownIdentifierTypes([identifier]); + await this.#runRecoveryMutation({ + operation: 'updateRecoverySecret', + payload: { recoverySecret: bytesToSecretHex(recoverySecret) }, + identifier, + }); + } + + /** + * Replaces the complete identifier set. + * + * @param identifier - Currently registered identifier used to authorize. + * @param identifiers - New non-empty identifier set. + */ + async updateIdentifiers( + identifier: Identifier, + identifiers: Identifier[], + ): Promise { + this.#assertKnownIdentifierTypes([identifier, ...identifiers]); + if (identifiers.length === 0) { + throw new MfaRecoveryError( + 'Identifier list must be non-empty', + 'empty_identifiers', + ); + } + await this.#runRecoveryMutation({ + operation: 'updateIdentifiers', + payload: { identifiers }, + identifier, + }); + } + + /** + * Reads the recovery secret from available escrows and returns the highest + * consistent version. + * + * @param identifier - Identifier used to authorize the read. + * @returns Recovered secret bytes. + */ + async getRecoverySecret(identifier: Identifier): Promise { + return await this.#withLock(async () => { + this.#assertKnownIdentifierTypes([identifier]); + const requestId = randomId(); + const requestHash = await hash({ + operation: 'getRecoverySecret', + requestId, + }); + const available = await this.#getAvailableEscrows(); + if (available.length === 0) { + throw new MfaRecoveryError( + 'No escrow is available', + 'no_available_escrow', + ); + } + const authorizations = await this.#authorizeIdentifier({ + escrows: available, + identifier, + requestHash, + }); + const results = await Promise.allSettled( + available.map(async (escrow, index) => ({ + escrowId: escrow.id, + ...(await escrow.getSecret(authorizations[index], requestId)), + })), + ); + return this.#selectHighestConsistentVersion(results).recoverySecret; + }); + } + + /** + * Completes a persisted pending mutation, if any. + */ + async resume(): Promise { + await this.#withLock(async () => { + await this.#repairPendingMutation(this.#escrows); + }); + } + + /** + * Drops a mutation that has not yet begun writing. Writing mutations must be + * resumed instead. + */ + async abort(): Promise { + await this.#withLock(async () => { + const pending = await this.#loadPending(); + assertAbortAllowed(getRecoveryPhase(pending)); + if (pending) { + await this.#clearPending(); + } + }); + } + + /** + * @returns Current recovery phase. + */ + async getPhase(): Promise { + return getRecoveryPhase(await this.#loadPending()); + } + + async #runRecoveryMutation({ + operation, + payload, + identifier, + }: { + operation: Mutation['operation']; + payload: MutationPayload; + identifier: Identifier | null; + }): Promise { + await this.#withLock(async () => { + const configured = this.#escrows; + await this.#repairPendingMutation(configured); + const profileId = await this.#authProvider.getAuthenticatedProfileId(); + const targets = await this.#requireAllEscrows(configured); + const audiences = configured.map((escrow) => escrow.id); + const currentVersion = await this.#resolveCurrentRecoveryVersion({ + operation, + profileId, + targets, + }); + const payloadHash = await hash(payload); + const mutation = await this.#createMutation({ + id: randomId(), + profileId, + operation, + expectedVersion: currentVersion, + newVersion: currentVersion + 1, + payloadHash, + audiences, + }); + + await this.#persistPending({ + phase: 'authorizing', + mutation, + payload, + identifier, + }); + + const authControllerToken = await this.#authorizeMutation( + mutation, + payload, + ); + const pending: WritingPendingOperation = { + phase: 'writing', + mutation, + authControllerToken, + payload, + identifier, + receipts: [], + }; + await this.#persistPending(pending); + await this.#replicateMutation({ escrows: targets, pending }); + }); + } + + async #createMutation( + fields: Omit, + ): Promise { + return { + ...fields, + requestHash: await hash({ + id: fields.id, + profileId: fields.profileId, + operation: fields.operation, + expectedVersion: fields.expectedVersion, + newVersion: fields.newVersion, + payloadHash: fields.payloadHash, + audiences: fields.audiences, + }), + }; + } + + async #authorizeMutation( + mutation: Mutation, + payload: MutationPayload, + ): Promise { + const identifiers = + mutation.operation === 'register' || + mutation.operation === 'updateIdentifiers' + ? (payload as RegisterPayload | UpdateIdentifiersPayload).identifiers + : undefined; + return await this.#authProvider.authorizeRecoveryRequest({ + requestHash: mutation.requestHash, + ...(mutation.operation === 'register' ? {} : { requireTwoFactor: true }), + ...(identifiers === undefined ? {} : { identifiers }), + }); + } + + async #repairPendingMutation( + escrows: RecoveryEscrowProvider[], + ): Promise { + const saved = await this.#loadPending(); + if (!saved) { + return; + } + assertSameAudienceIds( + saved.mutation.audiences, + escrows.map((escrow) => escrow.id), + ); + const authControllerToken = + saved.phase === 'authorizing' || + this.#tokenNeedsRefresh(saved.authControllerToken) + ? await this.#authorizeMutation(saved.mutation, saved.payload) + : saved.authControllerToken; + const pending: WritingPendingOperation = { + phase: 'writing', + mutation: saved.mutation, + authControllerToken, + payload: saved.payload, + identifier: saved.identifier, + receipts: saved.phase === 'writing' ? saved.receipts : [], + }; + await this.#persistPending(pending); + await this.#replicateMutation({ + escrows: await this.#requireAllEscrows(escrows), + pending, + }); + } + + async #replicateMutation({ + escrows, + pending, + }: { + escrows: RecoveryEscrowProvider[]; + pending: WritingPendingOperation; + }): Promise { + const { mutation, payload, identifier } = pending; + const expectedPayloadHash = await hash(payload); + if (expectedPayloadHash !== mutation.payloadHash) { + throw new MfaRecoveryError( + 'Pending payload does not match mutation', + 'payload_mismatch', + ); + } + const configuredIds = new Set(escrows.map((escrow) => escrow.id)); + pending.receipts.forEach((receipt) => { + const escrow = escrows.find((item) => item.id === receipt.escrowId); + if (!configuredIds.has(receipt.escrowId) || escrow === undefined) { + throw new MfaRecoveryError( + 'Receipt escrow is not configured', + 'unknown_receipt_escrow', + ); + } + if (!escrow.verifyReceipt(receipt, mutation)) { + throw new MfaRecoveryError( + 'Invalid mutation receipt', + 'invalid_receipt', + ); + } + }); + const acknowledged = new Set( + pending.receipts.map((receipt) => receipt.escrowId), + ); + const targets = escrows.filter((escrow) => !acknowledged.has(escrow.id)); + if (targets.length === 0) { + await this.#clearPending(); + return; + } + const authorizations = + mutation.operation === 'register' + ? targets.map(() => null) + : await this.#authorizeIdentifier({ + escrows: targets, + identifier: identifier as Identifier, + requestHash: mutation.requestHash, + }); + const results = await Promise.allSettled( + targets.map((escrow, index) => + escrow.applyMutation( + mutation, + pending.authControllerToken, + authorizations[index], + payload, + ), + ), + ); + + const receipts: MutationReceipt[] = [...pending.receipts]; + results.forEach((result, index) => { + if (result.status === 'fulfilled') { + const escrow = targets[index]; + if (!escrow.verifyReceipt(result.value, mutation)) { + throw new MfaRecoveryError( + 'Invalid mutation receipt', + 'invalid_receipt', + ); + } + receipts.push(result.value); + } + }); + await this.#persistPending({ ...pending, receipts }); + + if ( + new Set(receipts.map((receipt) => receipt.escrowId)).size !== + escrows.length + ) { + throw new MutationRepairPendingError(mutation.id); + } + await this.#clearPending(); + } + + async #authorizeIdentifier({ + escrows, + identifier, + requestHash, + }: { + escrows: RecoveryEscrowProvider[]; + identifier: Identifier; + requestHash: string; + }): Promise { + const mode = getIdentifierAuthMode(identifier.type); + if (mode === 'key-bound') { + const proofKey = await generateSigningKey(); + const token = + await this.#identifierAuthProvider.getKeyBoundIdentifierToken({ + identifier, + proofPublicKey: proofKey.publicKey, + requestHash, + }); + const challenges = await Promise.all( + escrows.map((escrow) => escrow.generateChallenge()), + ); + return await Promise.all( + challenges.map(async (challenge) => { + const message = await hash([token, challenge.id, requestHash]); + return { + kind: 'key-bound' as const, + token, + proof: { + challengeId: challenge.id, + requestHash, + signature: await sign(proofKey.privateKey, message), + }, + }; + }), + ); + } + + const challenges = await Promise.all( + escrows.map((escrow) => + escrow.beginIdentifierAuthentication({ identifier, requestHash }), + ), + ); + const responses = await Promise.all( + challenges.map((challenge) => this.#collectChallengeResponse(challenge)), + ); + const grants = await Promise.all( + escrows.map((escrow, index) => + escrow.completeIdentifierAuthentication( + challenges[index].id, + responses[index], + ), + ), + ); + return grants.map((grant) => ({ + kind: 'escrow-challenge' as const, + grant, + })); + } + + async #resolveCurrentRecoveryVersion({ + operation, + profileId, + targets, + }: { + operation: Mutation['operation']; + profileId: string; + targets: RecoveryEscrowProvider[]; + }): Promise { + const metadatas = await Promise.all( + targets.map((escrow) => escrow.getRecoveryMetadata(profileId)), + ); + if (operation === 'register') { + if (metadatas.some((metadata) => metadata !== null)) { + throw new MfaRecoveryError( + 'Profile is already registered', + 'already_registered', + ); + } + return 0; + } + const present = metadatas.filter( + (metadata): metadata is NonNullable => metadata !== null, + ); + if (present.length !== metadatas.length) { + throw new MfaRecoveryError( + 'Recovery record missing at a configured escrow', + 'missing_record', + ); + } + const selected = present[0]; + if ( + !present.every( + (metadata) => + metadata.version === selected.version && + metadata.lastMutationId === selected.lastMutationId, + ) + ) { + throw new MfaRecoveryError( + 'Escrows disagree on recovery version', + 'version_divergence', + ); + } + return selected.version; + } + + #selectHighestConsistentVersion( + results: PromiseSettledResult<{ + escrowId: string; + recoverySecret: Uint8Array; + version: number; + lastMutationId: string; + }>[], + ): { recoverySecret: Uint8Array; version: number; lastMutationId: string } { + const responses = results + .filter((result) => result.status === 'fulfilled') + .map((result) => result.value); + if (responses.length === 0) { + throw new MfaRecoveryError( + 'No escrow returned a recovery secret', + 'read_failed', + ); + } + const highestVersion = Math.max( + ...responses.map((response) => response.version), + ); + const highest = responses.filter( + (response) => response.version === highestVersion, + ); + const selected = highest[0]; + if ( + !highest.every( + (response) => + response.lastMutationId === selected.lastMutationId && + areUint8ArraysEqual(response.recoverySecret, selected.recoverySecret), + ) + ) { + throw new MfaRecoveryError('Replica corruption', 'replica_corruption'); + } + return selected; + } + + async #getAvailableEscrows(): Promise { + const flags = await Promise.all( + this.#escrows.map(async (escrow) => ({ + escrow, + available: await escrow.isAvailable(), + })), + ); + return flags.filter((item) => item.available).map((item) => item.escrow); + } + + async #requireAllEscrows( + escrows: RecoveryEscrowProvider[], + ): Promise { + const available = await Promise.all( + escrows.map(async (escrow) => ({ + escrow, + available: await escrow.isAvailable(), + })), + ); + if (available.some((item) => !item.available)) { + throw new MfaRecoveryError( + 'All configured escrows are required for mutation', + 'escrow_unavailable', + ); + } + return escrows; + } + + #tokenNeedsRefresh(token: AuthControllerToken): boolean { + return token.expiresAt <= this.#now(); + } + + #assertKnownIdentifierTypes(identifiers: Identifier[]): void { + for (const identifier of identifiers) { + getIdentifierAuthMode(identifier.type); + } + } + + async #loadPending(): Promise { + const { pendingOperation } = this.state; + if (pendingOperation === null) { + return null; + } + return await this.#encryptor.decrypt(pendingOperation); + } + + async #persistPending(operation: PendingOperation): Promise { + const encrypted = await this.#encryptor.encrypt(operation); + this.update((state) => { + state.pendingOperation = encrypted; + }); + } + + async #clearPending(): Promise { + this.update((state) => { + state.pendingOperation = null; + }); + } + + async #withLock( + fn: () => Promise, + ): Promise { + const run = this.#lock.then(fn, fn); + this.#lock = run.then( + () => undefined, + () => undefined, + ); + return await run; + } +} diff --git a/packages/mfa-recovery-controller/src/crypto.test.ts b/packages/mfa-recovery-controller/src/crypto.test.ts new file mode 100644 index 0000000000..8143c857f9 --- /dev/null +++ b/packages/mfa-recovery-controller/src/crypto.test.ts @@ -0,0 +1,46 @@ +import { bytesToHex } from '@metamask/utils'; + +import { + bytesToSecretHex, + canonicalize, + canonicalizeIdentifiers, + hash, + secretHexToBytes, +} from './crypto.js'; +import type { Identifier } from './types.js'; + +describe('crypto', () => { + it('canonicalizes objects with sorted keys and Uint8Array values', () => { + expect(canonicalize({ b: 1, a: new Uint8Array([1, 2]) })).toBe( + `{"a":"${bytesToHex(new Uint8Array([1, 2]))}","b":1}`, + ); + }); + + it('hashes independently of key order', async () => { + expect(await hash({ b: 1, a: 2 })).toBe(await hash({ a: 2, b: 1 })); + }); + + it('round-trips secret hex without a 0x prefix', () => { + const bytes = new Uint8Array([255, 0, 16]); + const encoded = bytesToSecretHex(bytes); + expect(secretHexToBytes(encoded.slice(2))).toStrictEqual(bytes); + }); + + it('sorts identifiers for ownership hashes', () => { + const first: Identifier = { + type: 'passkey', + namespace: 'b.com', + value: '2', + verifier: null, + }; + const second: Identifier = { + type: 'passkey', + namespace: 'a.com', + value: '1', + verifier: null, + }; + expect(canonicalize(canonicalizeIdentifiers([first, second]))).toBe( + canonicalize(canonicalizeIdentifiers([second, first])), + ); + }); +}); diff --git a/packages/mfa-recovery-controller/src/crypto.ts b/packages/mfa-recovery-controller/src/crypto.ts new file mode 100644 index 0000000000..83aa9272a8 --- /dev/null +++ b/packages/mfa-recovery-controller/src/crypto.ts @@ -0,0 +1,201 @@ +import { bytesToHex, hexToBytes, stringToBytes } from '@metamask/utils'; +import type { Hex } from '@metamask/utils'; + +import type { Identifier } from './types.js'; + +const ECDSA = { name: 'ECDSA', namedCurve: 'P-256' } as const; +const ECDSA_SIGN = { name: 'ECDSA', hash: 'SHA-256' } as const; + +/** + * Recursively sorts object keys so hashes are independent of property order. + * + * @param value - JSON-compatible value. + * @returns A canonical JSON string. + */ +export function canonicalize(value: unknown): string { + return JSON.stringify(sortKeys(value)); +} + +/** + * SHA-256 of the canonical JSON form of `value`, as a 0x-prefixed hex string. + * + * @param value - Value to hash. + * @returns Hex digest. + */ +export async function hash(value: unknown): Promise { + const digest = await globalThis.crypto.subtle.digest( + 'SHA-256', + stringToBytes(canonicalize(value)), + ); + return bytesToHex(new Uint8Array(digest)); +} + +/** + * Canonical identifier list bound into AuthController `identifiersHash`. + * + * @param identifiers - Identifier set. + * @returns Sorted identifier records including verifier material. + */ +export function canonicalizeIdentifiers(identifiers: Identifier[]): unknown { + return [...identifiers] + .map((identifier) => ({ + namespace: identifier.namespace, + type: identifier.type, + value: identifier.value, + verifier: identifier.verifier, + })) + .sort((left, right) => + canonicalize(left).localeCompare(canonicalize(right)), + ); +} + +/** + * @returns A random 0x-prefixed id. + */ +export function randomId(): Hex { + return bytesToHex(globalThis.crypto.getRandomValues(new Uint8Array(16))); +} + +/** + * Generates an ephemeral ECDSA P-256 proof key pair for key-bound identifier PoP. + * + * @returns JWK-encoded public and private keys. + */ +export async function generateSigningKey(): Promise<{ + publicKey: string; + privateKey: string; +}> { + const pair = await globalThis.crypto.subtle.generateKey(ECDSA, true, [ + 'sign', + 'verify', + ]); + const [publicJwk, privateJwk] = await Promise.all([ + globalThis.crypto.subtle.exportKey('jwk', pair.publicKey), + globalThis.crypto.subtle.exportKey('jwk', pair.privateKey), + ]); + return { + publicKey: JSON.stringify(publicJwk), + privateKey: JSON.stringify(privateJwk), + }; +} + +/** + * Signs a message with the ephemeral proof private key. + * + * @param privateKey - JWK JSON private key. + * @param message - Prehashed message hex or string. + * @returns Hex signature. + */ +export async function sign(privateKey: string, message: string): Promise { + const key = await globalThis.crypto.subtle.importKey( + 'jwk', + JSON.parse(privateKey) as JsonWebKey, + ECDSA, + false, + ['sign'], + ); + const signature = await globalThis.crypto.subtle.sign( + ECDSA_SIGN, + key, + stringToBytes(message), + ); + return bytesToHex(new Uint8Array(signature)); +} + +/** + * Verifies a proof signature against a public key. + * + * @param publicKey - JWK JSON public key. + * @param signature - Hex signature. + * @param message - Prehashed message. + * @returns Whether the signature is valid. + */ +export async function verifySignature( + publicKey: string, + signature: string, + message: string, +): Promise { + const key = await globalThis.crypto.subtle.importKey( + 'jwk', + JSON.parse(publicKey) as JsonWebKey, + ECDSA, + false, + ['verify'], + ); + return await globalThis.crypto.subtle.verify( + ECDSA_SIGN, + key, + hexToBytes(signature as Hex), + stringToBytes(message), + ); +} + +/** + * Receipt domain-separated digest from the recovery ADR. + * + * @param receiptFields - Receipt fields covered by the signature. + * @param receiptFields.escrowId - Escrow that issued the receipt. + * @param receiptFields.mutationId - Mutation id. + * @param receiptFields.requestHash - Mutation request hash. + * @param receiptFields.version - Applied version. + * @returns Hex digest. + */ +export async function hashMutationReceipt(receiptFields: { + escrowId: string; + mutationId: string; + requestHash: string; + version: number; +}): Promise { + return await hash([ + 'mfa-recovery-mutation-receipt-v1', + receiptFields.escrowId, + receiptFields.mutationId, + receiptFields.requestHash, + receiptFields.version, + ]); +} + +/** + * Converts bytes to 0x-prefixed hex. + * + * @param bytes - Secret bytes. + * @returns Hex string. + */ +export function bytesToSecretHex(bytes: Uint8Array): Hex { + return bytesToHex(bytes); +} + +/** + * Converts a 0x-prefixed hex secret to bytes. + * + * @param secretHex - Hex string. + * @returns Secret bytes. + */ +export function secretHexToBytes(secretHex: string): Uint8Array { + return hexToBytes( + (secretHex.startsWith('0x') ? secretHex : `0x${secretHex}`) as Hex, + ); +} + +/** + * Recursively sorts keys and encodes Uint8Array as hex. + * + * @param value - Value to normalize. + * @returns JSON-safe canonical form. + */ +function sortKeys(value: unknown): unknown { + if (value instanceof Uint8Array) { + return bytesToHex(value); + } + if (Array.isArray(value)) { + return value.map(sortKeys); + } + if (value !== null && typeof value === 'object') { + return Object.fromEntries( + Object.keys(value as Record) + .sort() + .map((key) => [key, sortKeys((value as Record)[key])]), + ); + } + return value; +} diff --git a/packages/mfa-recovery-controller/src/errors.ts b/packages/mfa-recovery-controller/src/errors.ts new file mode 100644 index 0000000000..b6887291fc --- /dev/null +++ b/packages/mfa-recovery-controller/src/errors.ts @@ -0,0 +1,29 @@ +/** + * Errors thrown by {@link MfaRecoveryController}. + */ +export class MfaRecoveryError extends Error { + readonly code: string; + + constructor(message: string, code: string) { + super(message); + this.name = 'MfaRecoveryError'; + this.code = code; + Object.setPrototypeOf(this, new.target.prototype); + } +} + +/** + * Thrown when a mutation was applied at some but not all configured escrows. + * Call {@link MfaRecoveryController.resume} to retry the same mutation. + */ +export class MutationRepairPendingError extends MfaRecoveryError { + readonly mutationId: string; + + constructor(mutationId: string) { + super( + `Mutation ${mutationId} is not acknowledged by every escrow`, + 'mutation_repair_pending', + ); + this.mutationId = mutationId; + } +} diff --git a/packages/mfa-recovery-controller/src/identifier-auth.ts b/packages/mfa-recovery-controller/src/identifier-auth.ts new file mode 100644 index 0000000000..e6b781634c --- /dev/null +++ b/packages/mfa-recovery-controller/src/identifier-auth.ts @@ -0,0 +1,32 @@ +import { MfaRecoveryError } from './errors.js'; +import type { IdentifierAuthMode } from './types.js'; + +/** + * Trusted identifier-type registry. Escrows and the controller derive the + * authentication mode from this table, never from a client-selected flag. + */ +export const IDENTIFIER_AUTH_MODES: Record = { + oidc: 'key-bound', + passkey: 'key-bound', + siwe: 'key-bound', + emailOtp: 'escrow-challenge', + smsOtp: 'escrow-challenge', +}; + +/** + * Returns the auth mode for an identifier type. + * + * @param type - Identifier type. + * @returns Auth mode. + * @throws If the type is not in the trusted registry. + */ +export function getIdentifierAuthMode(type: string): IdentifierAuthMode { + const mode = IDENTIFIER_AUTH_MODES[type]; + if (mode === undefined) { + throw new MfaRecoveryError( + `Unknown identifier type: ${type}`, + 'unknown_identifier_type', + ); + } + return mode; +} diff --git a/packages/mfa-recovery-controller/src/index.ts b/packages/mfa-recovery-controller/src/index.ts new file mode 100644 index 0000000000..257996841c --- /dev/null +++ b/packages/mfa-recovery-controller/src/index.ts @@ -0,0 +1,42 @@ +export { + MfaRecoveryController, + getDefaultMfaRecoveryControllerState, +} from './MfaRecoveryController.js'; +export type { + MfaRecoveryControllerActions, + MfaRecoveryControllerEvents, + MfaRecoveryControllerGetStateAction, + MfaRecoveryControllerMessenger, + MfaRecoveryControllerOptions, + MfaRecoveryControllerState, + MfaRecoveryControllerStateChangeEvent, +} from './MfaRecoveryController.js'; +export type { + MfaRecoveryControllerAbortAction, + MfaRecoveryControllerGetPhaseAction, + MfaRecoveryControllerGetRecoverySecretAction, + MfaRecoveryControllerRegisterAction, + MfaRecoveryControllerResumeAction, + MfaRecoveryControllerUpdateIdentifiersAction, + MfaRecoveryControllerUpdateRecoverySecretAction, +} from './MfaRecoveryController-method-action-types.js'; +export { MfaRecoveryError, MutationRepairPendingError } from './errors.js'; +export { + IDENTIFIER_AUTH_MODES, + getIdentifierAuthMode, +} from './identifier-auth.js'; +export type { + AuthControllerToken, + EncryptedPendingOperation, + Identifier, + IdentifierAuthorization, + Mutation, + MutationPayload, + MutationReceipt, + PendingOperation, + PendingOperationEncryptor, + RecoveryAuthProvider, + RecoveryEscrowProvider, + RecoveryIdentifierAuthProvider, + RecoveryPhase, +} from './types.js'; diff --git a/packages/mfa-recovery-controller/src/state-machine.ts b/packages/mfa-recovery-controller/src/state-machine.ts new file mode 100644 index 0000000000..63fa5da736 --- /dev/null +++ b/packages/mfa-recovery-controller/src/state-machine.ts @@ -0,0 +1,51 @@ +import { MfaRecoveryError } from './errors.js'; +import type { PendingOperation, RecoveryPhase } from './types.js'; + +/** + * Returns the persisted recovery phase. + * + * @param pending - Loaded pending operation, if any. + * @returns Current phase. + */ +export function getRecoveryPhase( + pending: PendingOperation | null, +): RecoveryPhase { + return pending?.phase ?? 'idle'; +} + +/** + * Abort is allowed only before the first escrow write. Once `writing` begins + * the same mutation must be retried to completion. + * + * @param phase - Current phase. + * @throws If the mutation is already being written. + */ +export function assertAbortAllowed(phase: RecoveryPhase): void { + if (phase === 'writing') { + throw new MfaRecoveryError( + 'Cannot abort a mutation once writing has begun', + 'abort_not_allowed', + ); + } +} + +/** + * Ensures a persisted mutation targets the build-configured escrow set. + * + * @param audiences - Mutation audience ids. + * @param escrowIds - Configured escrow ids. + */ +export function assertSameAudienceIds( + audiences: string[], + escrowIds: string[], +): void { + if ( + audiences.length !== escrowIds.length || + audiences.some((id, index) => id !== escrowIds[index]) + ) { + throw new MfaRecoveryError( + 'Persisted mutation audiences do not match configured escrows', + 'audience_mismatch', + ); + } +} diff --git a/packages/mfa-recovery-controller/src/types.ts b/packages/mfa-recovery-controller/src/types.ts new file mode 100644 index 0000000000..63d0c1d853 --- /dev/null +++ b/packages/mfa-recovery-controller/src/types.ts @@ -0,0 +1,208 @@ +/** + * MFA recovery protocol types from the recovery ADR. + */ + +export type Identifier = { + type: string; + namespace: string; + value: string; + verifier: unknown; +}; + +export type PoPChallenge = { + id: string; + escrowId: string; + expiresAt: number; +}; + +export type IdentifierAuthMode = 'key-bound' | 'escrow-challenge'; + +export type KeyBoundIdentifierToken = { + identifier: Identifier; + proofPublicKey: string; + requestHash: string; + providerAssertion: unknown; +}; + +export type ProofOfPossession = { + challengeId: string; + requestHash: string; + signature: string; +}; + +export type EscrowAuthChallenge = { + id: string; + escrowId: string; + identifier: Identifier; + requestHash: string; + expiresAt: number; +}; + +export type EscrowIdentifierGrant = { + id: string; +}; + +export type IdentifierAuthorization = + | { + kind: 'key-bound'; + token: KeyBoundIdentifierToken; + proof: ProofOfPossession; + } + | { + kind: 'escrow-challenge'; + grant: EscrowIdentifierGrant; + }; + +export type MutationOperation = + | 'register' + | 'updateRecoverySecret' + | 'updateIdentifiers'; + +export type Mutation = { + id: string; + profileId: string; + operation: MutationOperation; + expectedVersion: number; + newVersion: number; + payloadHash: string; + audiences: string[]; + requestHash: string; +}; + +export type AuthControllerToken = { + profileId: string; + requestHash: string; + twoFactor?: true; + identifiersHash?: string; + identifierOwnershipApproved?: true; + issuer: string; + expiresAt: number; + signature: string; +}; + +export type MutationReceipt = { + mutationId: string; + requestHash: string; + escrowId: string; + version: number; + signature: string; +}; + +export type RegisterPayload = { + recoverySecret: string; + identifiers: Identifier[]; +}; + +export type UpdateRecoverySecretPayload = { + recoverySecret: string; +}; + +export type UpdateIdentifiersPayload = { + identifiers: Identifier[]; +}; + +export type MutationPayload = + | RegisterPayload + | UpdateRecoverySecretPayload + | UpdateIdentifiersPayload; + +export type AuthorizingPendingOperation = { + phase: 'authorizing'; + mutation: Mutation; + payload: MutationPayload; + identifier: Identifier | null; +}; + +export type WritingPendingOperation = { + phase: 'writing'; + mutation: Mutation; + authControllerToken: AuthControllerToken; + payload: MutationPayload; + identifier: Identifier | null; + receipts: MutationReceipt[]; +}; + +export type PendingOperation = + | AuthorizingPendingOperation + | WritingPendingOperation; + +export type RecoveryPhase = 'idle' | PendingOperation['phase']; + +/** + * Ciphertext of {@link PendingOperation}. Wallet secure storage encrypts this + * value; a raw recovery secret is never persisted in the clear. + */ +export type EncryptedPendingOperation = string; + +export type RecoveryRecordMetadata = { + version: number; + lastMutationId: string; +}; + +export type RecoverySecretResponse = { + recoverySecret: Uint8Array; + version: number; + lastMutationId: string; +}; + +/** + * Authenticates the MetaMask profile and issues request-bound attestations. + */ +export type RecoveryAuthProvider = { + getAuthenticatedProfileId: () => Promise; + authorizeRecoveryRequest: (params: { + requestHash: string; + requireTwoFactor?: boolean; + identifiers?: Identifier[]; + }) => Promise; +}; + +/** + * Obtains a key-bound identifier assertion from an IdP (OIDC, Passkey, SIWE). + */ +export type RecoveryIdentifierAuthProvider = { + getKeyBoundIdentifierToken: (params: { + identifier: Identifier; + proofPublicKey: string; + requestHash: string; + }) => Promise; +}; + +/** + * One recovery escrow replica. Implementations may talk to different backends. + */ +export type RecoveryEscrowProvider = { + readonly id: string; + isAvailable: () => Promise; + generateChallenge: () => Promise; + beginIdentifierAuthentication: (params: { + identifier: Identifier; + requestHash: string; + }) => Promise; + completeIdentifierAuthentication: ( + challengeId: string, + response: unknown, + ) => Promise; + getRecoveryMetadata: ( + profileId: string, + ) => Promise; + getSecret: ( + authorization: IdentifierAuthorization, + requestId: string, + ) => Promise; + applyMutation: ( + mutation: Mutation, + authControllerToken: AuthControllerToken, + identifierAuthorization: IdentifierAuthorization | null, + payload: MutationPayload, + ) => Promise; + verifyReceipt: (receipt: MutationReceipt, mutation: Mutation) => boolean; +}; + +/** + * Encrypts pending mutation state using wallet secure storage. + */ +export type PendingOperationEncryptor = { + encrypt: (operation: PendingOperation) => Promise; + decrypt: (ciphertext: EncryptedPendingOperation) => Promise; +}; diff --git a/packages/mfa-recovery-controller/tests/stubs.ts b/packages/mfa-recovery-controller/tests/stubs.ts new file mode 100644 index 0000000000..579dcc7955 --- /dev/null +++ b/packages/mfa-recovery-controller/tests/stubs.ts @@ -0,0 +1,603 @@ +import { + canonicalizeIdentifiers, + hash, + hashMutationReceipt, + secretHexToBytes, + verifySignature, +} from '../src/crypto.js'; +import { MfaRecoveryError } from '../src/errors.js'; +import { getIdentifierAuthMode } from '../src/identifier-auth.js'; +import type { + AuthControllerToken, + EscrowAuthChallenge, + EscrowIdentifierGrant, + Identifier, + IdentifierAuthorization, + KeyBoundIdentifierToken, + Mutation, + MutationPayload, + MutationReceipt, + PendingOperation, + PendingOperationEncryptor, + PoPChallenge, + RecoveryAuthProvider, + RecoveryEscrowProvider, + RecoveryIdentifierAuthProvider, + RecoveryRecordMetadata, + RecoverySecretResponse, + RegisterPayload, + UpdateIdentifiersPayload, + UpdateRecoverySecretPayload, +} from '../src/types.js'; + +type RecoveryRecord = { + profileId: string; + identifiers: Identifier[]; + recoverySecret: string; + version: number; + lastMutationId: string; + appliedMutations: Record; +}; + +type StoredPoPChallenge = PoPChallenge & { consumed: boolean }; + +type StoredEscrowAuth = EscrowAuthChallenge & { + completed: boolean; + consumed: boolean; + grant?: EscrowIdentifierGrant; +}; + +const CHALLENGE_TTL_MS = 5 * 60 * 1000; + +/** + * In-memory AuthController used in tests. + */ +export class StubAuthProvider implements RecoveryAuthProvider { + profileId = 'profile-1'; + + now: () => number = () => Date.now(); + + async getAuthenticatedProfileId(): Promise { + return this.profileId; + } + + async authorizeRecoveryRequest(params: { + requestHash: string; + requireTwoFactor?: boolean; + identifiers?: Identifier[]; + }): Promise { + const identifiersHash = + params.identifiers === undefined + ? undefined + : await hash(canonicalizeIdentifiers(params.identifiers)); + return { + profileId: this.profileId, + requestHash: params.requestHash, + ...(params.requireTwoFactor ? { twoFactor: true as const } : {}), + ...(identifiersHash === undefined + ? {} + : { + identifiersHash, + identifierOwnershipApproved: true as const, + }), + issuer: 'stub-auth', + expiresAt: this.now() + 60 * 60 * 1000, + signature: 'stub-auth-signature', + }; + } +} + +/** + * In-memory identifier IdP used in tests. + */ +export class StubIdentifierAuthProvider implements RecoveryIdentifierAuthProvider { + async getKeyBoundIdentifierToken(params: { + identifier: Identifier; + proofPublicKey: string; + requestHash: string; + }): Promise { + return { + identifier: params.identifier, + proofPublicKey: params.proofPublicKey, + requestHash: params.requestHash, + providerAssertion: { + bound: await hash({ + proofPublicKey: params.proofPublicKey, + requestHash: params.requestHash, + }), + }, + }; + } +} + +/** + * In-memory escrow replica used in tests. + */ +export class StubEscrowProvider implements RecoveryEscrowProvider { + available = true; + + failNextApplyCount = 0; + + failGetSecret = false; + + invalidReceipts = false; + + now: () => number = () => Date.now(); + + readonly #records = new Map(); + + readonly #identifierIndex = new Map(); + + readonly #popChallenges = new Map(); + + readonly #escrowAuth = new Map(); + + readonly #grants = new Map< + string, + { + identifier: Identifier; + requestHash: string; + expiresAt: number; + consumed: boolean; + } + >(); + + readonly id: string; + + constructor(id: string) { + this.id = id; + } + + async isAvailable(): Promise { + return this.available; + } + + async generateChallenge(): Promise { + const challenge: StoredPoPChallenge = { + id: `${this.id}-pop-${this.#popChallenges.size + 1}`, + escrowId: this.id, + expiresAt: this.now() + CHALLENGE_TTL_MS, + consumed: false, + }; + this.#popChallenges.set(challenge.id, challenge); + return { + id: challenge.id, + escrowId: challenge.escrowId, + expiresAt: challenge.expiresAt, + }; + } + + async beginIdentifierAuthentication(params: { + identifier: Identifier; + requestHash: string; + }): Promise { + if (getIdentifierAuthMode(params.identifier.type) !== 'escrow-challenge') { + throw new MfaRecoveryError( + 'Identifier type is not escrow-challenge', + 'invalid_auth_mode', + ); + } + const challenge: StoredEscrowAuth = { + id: `${this.id}-otp-${this.#escrowAuth.size + 1}`, + escrowId: this.id, + identifier: params.identifier, + requestHash: params.requestHash, + expiresAt: this.now() + CHALLENGE_TTL_MS, + completed: false, + consumed: false, + }; + this.#escrowAuth.set(challenge.id, challenge); + return { + id: challenge.id, + escrowId: challenge.escrowId, + identifier: challenge.identifier, + requestHash: challenge.requestHash, + expiresAt: challenge.expiresAt, + }; + } + + async completeIdentifierAuthentication( + challengeId: string, + _response: unknown, + ): Promise { + const challenge = this.#escrowAuth.get(challengeId); + if ( + challenge === undefined || + challenge.expiresAt <= this.now() || + challenge.completed + ) { + throw new MfaRecoveryError( + 'Invalid escrow auth challenge', + 'invalid_challenge', + ); + } + challenge.completed = true; + const grant = { id: `${challenge.id}-grant` }; + challenge.grant = grant; + this.#grants.set(grant.id, { + identifier: challenge.identifier, + requestHash: challenge.requestHash, + expiresAt: challenge.expiresAt, + consumed: false, + }); + return grant; + } + + async getRecoveryMetadata( + profileId: string, + ): Promise { + const record = this.#records.get(profileId); + if (record === undefined) { + return null; + } + return { version: record.version, lastMutationId: record.lastMutationId }; + } + + async getSecret( + authorization: IdentifierAuthorization, + requestId: string, + ): Promise { + if (this.failGetSecret) { + throw new MfaRecoveryError( + 'Injected getSecret failure', + 'get_secret_failed', + ); + } + const requestHash = await hash({ + operation: 'getRecoverySecret', + requestId, + }); + const { profileId } = await this.#verifyIdentifierAuthorization( + authorization, + requestHash, + ); + const record = this.#records.get(profileId); + if (record === undefined) { + throw new MfaRecoveryError('No recovery record', 'not_registered'); + } + return { + recoverySecret: secretHexToBytes(record.recoverySecret), + version: record.version, + lastMutationId: record.lastMutationId, + }; + } + + async applyMutation( + mutation: Mutation, + authControllerToken: AuthControllerToken, + identifierAuthorization: IdentifierAuthorization | null, + payload: MutationPayload, + ): Promise { + if (this.failNextApplyCount > 0) { + this.failNextApplyCount -= 1; + throw new MfaRecoveryError('Injected apply failure', 'apply_failed'); + } + + await this.#verifyMutationAuthorization(mutation, authControllerToken); + + const existing = this.#records.get(mutation.profileId)?.appliedMutations[ + mutation.id + ]; + if (existing) { + if ( + existing.requestHash !== mutation.requestHash || + existing.version !== mutation.newVersion + ) { + throw new MfaRecoveryError( + 'Conflicting mutation replay', + 'mutation_conflict', + ); + } + return await this.#signReceipt(mutation); + } + + const record = this.#records.get(mutation.profileId); + const currentVersion = record?.version ?? 0; + if ( + mutation.expectedVersion !== currentVersion || + mutation.newVersion !== currentVersion + 1 + ) { + throw new MfaRecoveryError('Version mismatch', 'version_mismatch'); + } + + if (mutation.operation === 'register') { + if (identifierAuthorization !== null || record !== undefined) { + throw new MfaRecoveryError('Invalid registration', 'invalid_register'); + } + const registerPayload = payload as RegisterPayload; + await this.#assertIdentifierOwnership( + authControllerToken, + registerPayload.identifiers, + ); + this.#assertIdentifiersAvailable( + mutation.profileId, + registerPayload.identifiers, + ); + const next: RecoveryRecord = { + profileId: mutation.profileId, + identifiers: registerPayload.identifiers, + recoverySecret: registerPayload.recoverySecret, + version: mutation.newVersion, + lastMutationId: mutation.id, + appliedMutations: { + [mutation.id]: { + requestHash: mutation.requestHash, + version: mutation.newVersion, + }, + }, + }; + this.#records.set(mutation.profileId, next); + this.#replaceIdentifierIndex( + mutation.profileId, + [], + registerPayload.identifiers, + ); + return await this.#signReceipt(mutation); + } + + if (record === undefined || authControllerToken.twoFactor !== true) { + throw new MfaRecoveryError( + 'Update not authorized', + 'update_unauthorized', + ); + } + if (identifierAuthorization === null) { + throw new MfaRecoveryError( + 'Identifier authorization required', + 'missing_identifier_auth', + ); + } + const authorization = await this.#verifyIdentifierAuthorization( + identifierAuthorization, + mutation.requestHash, + ); + if (authorization.profileId !== mutation.profileId) { + throw new MfaRecoveryError('Profile mismatch', 'profile_mismatch'); + } + + if (mutation.operation === 'updateRecoverySecret') { + record.recoverySecret = ( + payload as UpdateRecoverySecretPayload + ).recoverySecret; + } else { + const { identifiers } = payload as UpdateIdentifiersPayload; + await this.#assertIdentifierOwnership(authControllerToken, identifiers); + this.#assertIdentifiersAvailable(mutation.profileId, identifiers); + this.#replaceIdentifierIndex( + mutation.profileId, + record.identifiers, + identifiers, + ); + record.identifiers = identifiers; + } + record.version = mutation.newVersion; + record.lastMutationId = mutation.id; + record.appliedMutations[mutation.id] = { + requestHash: mutation.requestHash, + version: mutation.newVersion, + }; + return await this.#signReceipt(mutation); + } + + verifyReceipt(receipt: MutationReceipt, mutation: Mutation): boolean { + if (this.invalidReceipts) { + return false; + } + return ( + receipt.mutationId === mutation.id && + receipt.requestHash === mutation.requestHash && + receipt.escrowId === this.id && + receipt.version === mutation.newVersion + ); + } + + /** + * Test helper: remove a replica record. + * + * @param profileId - Profile id. + */ + clearRecord(profileId: string): void { + this.#records.delete(profileId); + } + + /** + * Test helper: force a replica to a specific record version. + * + * @param profileId - Profile id. + * @param patch - Fields to overwrite. + */ + patchRecord( + profileId: string, + patch: Partial< + Pick + >, + ): void { + const record = this.#records.get(profileId); + if (record === undefined) { + throw new Error(`No record for ${profileId}`); + } + Object.assign(record, patch); + } + + async #signReceipt(mutation: Mutation): Promise { + const signature = await hashMutationReceipt({ + escrowId: this.id, + mutationId: mutation.id, + requestHash: mutation.requestHash, + version: mutation.newVersion, + }); + return { + mutationId: mutation.id, + requestHash: mutation.requestHash, + escrowId: this.id, + version: mutation.newVersion, + signature, + }; + } + + async #verifyMutationAuthorization( + mutation: Mutation, + token: AuthControllerToken, + ): Promise { + const requestHash = await hash({ + id: mutation.id, + profileId: mutation.profileId, + operation: mutation.operation, + expectedVersion: mutation.expectedVersion, + newVersion: mutation.newVersion, + payloadHash: mutation.payloadHash, + audiences: mutation.audiences, + }); + if ( + requestHash !== mutation.requestHash || + token.requestHash !== mutation.requestHash || + token.profileId !== mutation.profileId || + token.signature !== 'stub-auth-signature' || + token.expiresAt <= this.now() || + !mutation.audiences.includes(this.id) + ) { + throw new MfaRecoveryError( + 'Invalid mutation authorization', + 'invalid_mutation_auth', + ); + } + } + + async #assertIdentifierOwnership( + token: AuthControllerToken, + identifiers: Identifier[], + ): Promise { + if ( + token.identifierOwnershipApproved !== true || + token.identifiersHash !== + (await hash(canonicalizeIdentifiers(identifiers))) + ) { + throw new MfaRecoveryError( + 'Identifier ownership not approved', + 'ownership_not_approved', + ); + } + if (identifiers.length === 0) { + throw new MfaRecoveryError('Empty identifier list', 'empty_identifiers'); + } + } + + async #verifyIdentifierAuthorization( + authorization: IdentifierAuthorization, + requestHash: string, + ): Promise<{ profileId: string }> { + let identifier: Identifier; + if (authorization.kind === 'key-bound') { + const { token, proof } = authorization; + identifier = token.identifier; + if (getIdentifierAuthMode(identifier.type) !== 'key-bound') { + throw new MfaRecoveryError( + 'Not a key-bound identifier', + 'invalid_auth_mode', + ); + } + if ( + token.requestHash !== requestHash || + proof.requestHash !== requestHash + ) { + throw new MfaRecoveryError( + 'Request hash mismatch', + 'request_hash_mismatch', + ); + } + const challenge = this.#popChallenges.get(proof.challengeId); + if ( + challenge === undefined || + challenge.consumed || + challenge.escrowId !== this.id || + challenge.expiresAt <= this.now() + ) { + throw new MfaRecoveryError( + 'Invalid PoP challenge', + 'invalid_challenge', + ); + } + const message = await hash([token, proof.challengeId, requestHash]); + if ( + !(await verifySignature(token.proofPublicKey, proof.signature, message)) + ) { + throw new MfaRecoveryError('Invalid PoP signature', 'invalid_pop'); + } + challenge.consumed = true; + } else { + const grant = this.#grants.get(authorization.grant.id); + if ( + grant === undefined || + grant.consumed || + grant.requestHash !== requestHash || + grant.expiresAt <= this.now() + ) { + throw new MfaRecoveryError('Invalid escrow grant', 'invalid_grant'); + } + grant.consumed = true; + identifier = grant.identifier; + if (getIdentifierAuthMode(identifier.type) !== 'escrow-challenge') { + throw new MfaRecoveryError( + 'Not an escrow-challenge identifier', + 'invalid_auth_mode', + ); + } + } + + const profileId = this.#identifierIndex.get( + canonicalIdentifier(identifier), + ); + if (profileId === undefined) { + throw new MfaRecoveryError('Unknown identifier', 'unknown_identifier'); + } + return { profileId }; + } + + #assertIdentifiersAvailable( + profileId: string, + identifiers: Identifier[], + ): void { + for (const identifier of identifiers) { + const owner = this.#identifierIndex.get(canonicalIdentifier(identifier)); + if (owner !== undefined && owner !== profileId) { + throw new MfaRecoveryError( + 'Identifier owned by another profile', + 'identifier_taken', + ); + } + } + } + + #replaceIdentifierIndex( + profileId: string, + previous: Identifier[], + next: Identifier[], + ): void { + for (const identifier of previous) { + this.#identifierIndex.delete(canonicalIdentifier(identifier)); + } + for (const identifier of next) { + this.#identifierIndex.set(canonicalIdentifier(identifier), profileId); + } + } +} + +/** + * JSON round-trip encryptor for tests. Production uses wallet secure storage. + */ +export const passthroughEncryptor: PendingOperationEncryptor = { + async encrypt(operation: PendingOperation): Promise { + return JSON.stringify(operation); + }, + async decrypt(ciphertext: string): Promise { + return JSON.parse(ciphertext) as PendingOperation; + }, +}; + +/** + * @param identifier - Identifier to canonicalize. + * @returns Index key. + */ +function canonicalIdentifier(identifier: Identifier): string { + return `${identifier.type}:${identifier.namespace}:${identifier.value}`; +} diff --git a/packages/mfa-recovery-controller/tsconfig.build.json b/packages/mfa-recovery-controller/tsconfig.build.json new file mode 100644 index 0000000000..249f327913 --- /dev/null +++ b/packages/mfa-recovery-controller/tsconfig.build.json @@ -0,0 +1,17 @@ +{ + "extends": "../../tsconfig.packages.build.json", + "compilerOptions": { + "baseUrl": "./", + "outDir": "./dist", + "rootDir": "./src" + }, + "references": [ + { + "path": "../base-controller/tsconfig.build.json" + }, + { + "path": "../messenger/tsconfig.build.json" + } + ], + "include": ["../../types", "./src"] +} diff --git a/packages/mfa-recovery-controller/tsconfig.json b/packages/mfa-recovery-controller/tsconfig.json new file mode 100644 index 0000000000..890b592b55 --- /dev/null +++ b/packages/mfa-recovery-controller/tsconfig.json @@ -0,0 +1,15 @@ +{ + "extends": "../../tsconfig.packages.json", + "compilerOptions": { + "baseUrl": "./" + }, + "references": [ + { + "path": "../base-controller" + }, + { + "path": "../messenger" + } + ], + "include": ["../../types", "./src", "./tests"] +} diff --git a/packages/mfa-recovery-controller/typedoc.json b/packages/mfa-recovery-controller/typedoc.json new file mode 100644 index 0000000000..c9da015dbf --- /dev/null +++ b/packages/mfa-recovery-controller/typedoc.json @@ -0,0 +1,7 @@ +{ + "entryPoints": ["./src/index.ts"], + "excludePrivate": true, + "hideGenerator": true, + "out": "docs", + "tsconfig": "./tsconfig.build.json" +} diff --git a/teams.json b/teams.json index 3078fda3c4..0e0641c09e 100644 --- a/teams.json +++ b/teams.json @@ -62,6 +62,7 @@ "metamask/wallet": "team-core-platform", "metamask/wallet-cli": "team-core-platform", "metamask/passkey-controller": "team-onboarding", + "metamask/mfa-recovery-controller": "team-onboarding", "metamask/seedless-onboarding-controller": "team-onboarding", "metamask/shield-controller": "team-shield", "metamask/subscription-controller": "team-shield", diff --git a/tsconfig.build.json b/tsconfig.build.json index 01a1a01b60..949b70ca04 100644 --- a/tsconfig.build.json +++ b/tsconfig.build.json @@ -161,14 +161,17 @@ "path": "./packages/messenger/tsconfig.build.json" }, { - "path": "./packages/money-account-api-data-service/tsconfig.build.json" + "path": "./packages/mfa-recovery-controller/tsconfig.build.json" }, { - "path": "./packages/money-account-balance-service/tsconfig.build.json" + "path": "./packages/money-account-api-data-service/tsconfig.build.json" }, { "path": "./packages/money-account-api-data-service/tsconfig.build.json" }, + { + "path": "./packages/money-account-balance-service/tsconfig.build.json" + }, { "path": "./packages/money-account-controller/tsconfig.build.json" }, diff --git a/tsconfig.json b/tsconfig.json index accc0479c8..473759e3c0 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -101,6 +101,12 @@ { "path": "./packages/earn-controller" }, + { + "path": "./packages/eip-5792-middleware" + }, + { + "path": "./packages/eip-7702-internal-rpc-middleware" + }, { "path": "./packages/eip1193-permission-middleware" }, @@ -143,6 +149,9 @@ { "path": "./packages/local-node-utils" }, + { + "path": "./packages/logging-controller" + }, { "path": "./packages/message-manager" }, @@ -153,14 +162,17 @@ "path": "./packages/messenger-cli" }, { - "path": "./packages/money-account-api-data-service" + "path": "./packages/mfa-recovery-controller" }, { - "path": "./packages/money-account-balance-service" + "path": "./packages/money-account-api-data-service" }, { "path": "./packages/money-account-api-data-service" }, + { + "path": "./packages/money-account-balance-service" + }, { "path": "./packages/money-account-controller" }, @@ -272,6 +284,9 @@ { "path": "./packages/stellar-quickstart-up" }, + { + "path": "./packages/storage-service" + }, { "path": "./packages/subscription-controller" }, @@ -284,18 +299,6 @@ { "path": "./packages/user-operation-controller" }, - { - "path": "./packages/eip-5792-middleware" - }, - { - "path": "./packages/eip-7702-internal-rpc-middleware" - }, - { - "path": "./packages/logging-controller" - }, - { - "path": "./packages/storage-service" - }, { "path": "./packages/wallet" }, diff --git a/yarn.lock b/yarn.lock index 9acaac50d8..eedaea184f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7846,6 +7846,26 @@ __metadata: languageName: node linkType: hard +"@metamask/mfa-recovery-controller@workspace:packages/mfa-recovery-controller": + version: 0.0.0-use.local + resolution: "@metamask/mfa-recovery-controller@workspace:packages/mfa-recovery-controller" + dependencies: + "@metamask/auto-changelog": "npm:^6.1.0" + "@metamask/base-controller": "npm:^9.1.0" + "@metamask/messenger": "npm:^2.0.0" + "@metamask/utils": "npm:^11.11.0" + "@ts-bridge/cli": "npm:^0.6.4" + "@types/jest": "npm:^30.0.0" + deepmerge: "npm:^4.2.2" + jest: "npm:^30.4.2" + ts-jest: "npm:^29.4.11" + tsx: "npm:^4.20.5" + typedoc: "npm:^0.25.13" + typedoc-plugin-missing-exports: "npm:^2.0.0" + typescript: "npm:~5.3.3" + languageName: unknown + linkType: soft + "@metamask/money-account-api-data-service@npm:^0.4.1, @metamask/money-account-api-data-service@workspace:packages/money-account-api-data-service": version: 0.0.0-use.local resolution: "@metamask/money-account-api-data-service@workspace:packages/money-account-api-data-service" From 0b4047c7aaf5b8285f1398c4a7cdc54453f7fdf9 Mon Sep 17 00:00:00 2001 From: ieow <4881057+ieow@users.noreply.github.com> Date: Mon, 31 Aug 2026 18:10:23 +0800 Subject: [PATCH 2/3] Update `mfa-recovery-controller` with breaking changes and new features - Introduced a new `escrow-utils.ts` module for handling escrow-related operations, including receipt verification and selecting the highest consistent recovery secret version. - Updated `MfaRecoveryController` to require `RecoveryEscrowProvider.verifyReceipt` to accept an expected escrow ID, enhancing receipt verification. - Exposed state updates through the `MfaRecoveryController:stateChanged` event, allowing better tracking of state changes. - Improved documentation in `CHANGELOG.md` and `README.md` to reflect changes in state management and mutation handling. - Fixed issues related to clearing acknowledged mutations and validating persisted mutation states. This commit enhances the robustness of the MFA recovery process and improves the overall user experience in MetaMask. --- packages/mfa-recovery-controller/CHANGELOG.md | 12 + packages/mfa-recovery-controller/README.md | 7 +- .../src/MfaRecoveryController.test.ts | 572 +++++++++++++++--- .../src/MfaRecoveryController.ts | 211 +++---- .../src/escrow-utils.ts | 116 ++++ packages/mfa-recovery-controller/src/index.ts | 2 +- .../src/pending-operation-validation.ts | 214 +++++++ packages/mfa-recovery-controller/src/types.ts | 24 +- .../mfa-recovery-controller/tests/stubs.ts | 9 +- 9 files changed, 979 insertions(+), 188 deletions(-) create mode 100644 packages/mfa-recovery-controller/src/escrow-utils.ts create mode 100644 packages/mfa-recovery-controller/src/pending-operation-validation.ts diff --git a/packages/mfa-recovery-controller/CHANGELOG.md b/packages/mfa-recovery-controller/CHANGELOG.md index b1bd921bd9..a451639d3e 100644 --- a/packages/mfa-recovery-controller/CHANGELOG.md +++ b/packages/mfa-recovery-controller/CHANGELOG.md @@ -11,4 +11,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `MfaRecoveryController` with injectable `RecoveryAuthProvider` and `RecoveryEscrowProvider` interfaces, a persisted `idle` / `authorizing` / `writing` mutation state machine, and `register`, `updateRecoverySecret`, `updateIdentifiers`, `getRecoverySecret`, `resume`, and `abort` methods +### Changed + +- **BREAKING:** Require `RecoveryEscrowProvider.verifyReceipt` to receive the expected escrow id so receipt verification is explicitly bound to the configured escrow target +- Expose controller state updates through the non-deprecated `MfaRecoveryController:stateChanged` messenger event +- Retry only escrows without persisted mutation receipts and validate persisted mutation state before resuming it + +### Fixed + +- Clear fully acknowledged pending mutations without checking unavailable acknowledged escrows +- Persist valid receipts from concurrent escrow writes before reporting an invalid receipt response +- Reject malformed persisted mutation state before authorization or escrow writes + [Unreleased]: https://github.com/MetaMask/core/ diff --git a/packages/mfa-recovery-controller/README.md b/packages/mfa-recovery-controller/README.md index ba45a0667f..59b1d2d5fe 100644 --- a/packages/mfa-recovery-controller/README.md +++ b/packages/mfa-recovery-controller/README.md @@ -4,9 +4,10 @@ Manages MFA recovery flows across MetaMask clients. Auth and escrow storage are injected so the same controller can run against different identity providers and escrow backends. -Pending mutations are persisted as encrypted `authorizing` / `writing` state so -a crash can `resume()` the same mutation. `abort()` is allowed only before the -first escrow write. +Pending mutations are persisted as encoded encrypted `authorizing` / `writing` +state so a crash can `resume()` the same mutation. `abort()` is allowed only +before the first escrow write. The encoded ciphertext is stored as a string so +BaseController state serialization preserves the encrypted value. ## Installation diff --git a/packages/mfa-recovery-controller/src/MfaRecoveryController.test.ts b/packages/mfa-recovery-controller/src/MfaRecoveryController.test.ts index c6e17f2c2a..b2153b8b77 100644 --- a/packages/mfa-recovery-controller/src/MfaRecoveryController.test.ts +++ b/packages/mfa-recovery-controller/src/MfaRecoveryController.test.ts @@ -13,14 +13,18 @@ import { StubIdentifierAuthProvider, passthroughEncryptor, } from '../tests/stubs.js'; -import { hash } from './crypto.js'; +import { canonicalizeIdentifiers, hash } from './crypto.js'; import { MutationRepairPendingError } from './errors.js'; import type { MfaRecoveryControllerMessenger, MfaRecoveryControllerOptions, } from './MfaRecoveryController.js'; import { MfaRecoveryController } from './MfaRecoveryController.js'; -import type { Identifier } from './types.js'; +import type { + Identifier, + MutationReceipt, + WritingPendingOperation, +} from './types.js'; const PASSKEY: Identifier = { type: 'passkey', @@ -145,16 +149,33 @@ describe('MfaRecoveryController', () => { describe('register', () => { it('replicates the recovery secret to every escrow', async () => { - await withController(async ({ controller }) => { + await withController(async ({ controller, escrowA }) => { + const verifyReceipt = jest.spyOn(escrowA, 'verifyReceipt'); await controller.register(SECRET, [PASSKEY]); expect(controller.state.pendingOperation).toBeNull(); expect(await controller.getPhase()).toBe('idle'); + expect(verifyReceipt.mock.calls[0][2]).toBe('escrow-a'); const recovered = await controller.getRecoverySecret(PASSKEY); expect(bytesToHex(recovered)).toBe(bytesToHex(SECRET)); }); }); + it('publishes state changes through the stateChanged event', async () => { + await withController(async ({ controller, rootMessenger }) => { + const listener = jest.fn(); + rootMessenger.subscribe('MfaRecoveryController:stateChanged', listener); + + await controller.register(SECRET, [PASSKEY]); + + expect(listener).toHaveBeenCalled(); + expect(listener).toHaveBeenLastCalledWith( + { pendingOperation: null }, + expect.any(Array), + ); + }); + }); + it('rejects an empty identifier list', async () => { await withController(async ({ controller }) => { await expect(controller.register(SECRET, [])).rejects.toThrow( @@ -217,6 +238,27 @@ describe('MfaRecoveryController', () => { }); }); + describe('configured escrows', () => { + it('uses a snapshot of the configured escrow array', async () => { + await withController( + async ({ controller, escrowA, escrowB, options }) => { + options.escrows.splice(0, options.escrows.length); + + await controller.register(SECRET, [PASSKEY]); + + expect(await escrowA.getRecoveryMetadata('profile-1')).toStrictEqual({ + version: 1, + lastMutationId: expect.any(String), + }); + expect(await escrowB.getRecoveryMetadata('profile-1')).toStrictEqual({ + version: 1, + lastMutationId: expect.any(String), + }); + }, + ); + }); + }); + describe('updateRecoverySecret', () => { it('replaces the secret at every escrow', async () => { await withController(async ({ controller }) => { @@ -295,25 +337,68 @@ describe('MfaRecoveryController', () => { }); }); + it('repairs only the missing replica when an acknowledged replica is offline', async () => { + await withController(async ({ controller, escrowA, escrowB }) => { + await controller.register(SECRET, [PASSKEY]); + escrowA.failNextApplyCount = 1; + await expect( + controller.updateRecoverySecret(PASSKEY, SECRET_2), + ).rejects.toBeInstanceOf(MutationRepairPendingError); + + escrowB.available = false; + + await controller.resume(); + + expect(await controller.getPhase()).toBe('idle'); + expect(bytesToHex(await controller.getRecoverySecret(PASSKEY))).toBe( + bytesToHex(SECRET_2), + ); + }); + }); + + it('clears a fully acknowledged mutation without checking escrow availability', async () => { + await withController(async ({ controller, escrowA, escrowB }) => { + await controller.register(SECRET, [PASSKEY]); + const pending = await getValidWritingPending(); + pending.receipts = ['escrow-a', 'escrow-b'].map( + (escrowId): MutationReceipt => ({ + mutationId: pending.mutation.id, + requestHash: pending.mutation.requestHash, + escrowId, + version: pending.mutation.newVersion, + signature: '0xsignature', + }), + ); + const encrypted = await passthroughEncryptor.encrypt(pending); + const availabilityA = jest.spyOn(escrowA, 'isAvailable'); + const availabilityB = jest.spyOn(escrowB, 'isAvailable'); + availabilityA.mockClear(); + availabilityB.mockClear(); + escrowA.available = false; + escrowB.available = false; + + const resumed = new MfaRecoveryController({ + ...getOptions(escrowA, escrowB), + messenger: getMessenger(getRootMessenger()), + state: { pendingOperation: encrypted }, + }); + + await resumed.resume(); + + expect(await resumed.getPhase()).toBe('idle'); + expect(availabilityA).not.toHaveBeenCalled(); + expect(availabilityB).not.toHaveBeenCalled(); + }); + }); + it('aborts an authorizing mutation and refuses to abort writing', async () => { await withController(async ({ options }) => { + const validPending = await getValidWritingPending(); const authorizing = await passthroughEncryptor.encrypt({ phase: 'authorizing', - mutation: { - id: '0xmut', - profileId: 'profile-1', - operation: 'register', - expectedVersion: 0, - newVersion: 1, - payloadHash: '0x', - audiences: ['escrow-a', 'escrow-b'], - requestHash: '0x', - }, - payload: { - recoverySecret: bytesToHex(SECRET), - identifiers: [PASSKEY], - }, - identifier: null, + mutation: validPending.mutation, + payload: validPending.payload, + identifier: validPending.identifier, }); const authorizingMessenger = getMessenger(getRootMessenger()); const idleController = new MfaRecoveryController({ @@ -325,32 +410,7 @@ describe('MfaRecoveryController', () => { await idleController.abort(); expect(await idleController.getPhase()).toBe('idle'); - const writing = await passthroughEncryptor.encrypt({ - phase: 'writing', - mutation: { - id: '0xmut', - profileId: 'profile-1', - operation: 'register', - expectedVersion: 0, - newVersion: 1, - payloadHash: '0x', - audiences: ['escrow-a', 'escrow-b'], - requestHash: '0x', - }, - authControllerToken: { - profileId: 'profile-1', - requestHash: '0x', - issuer: 'stub-auth', - expiresAt: Date.now() + 60_000, - signature: 'stub-auth-signature', - }, - payload: { - recoverySecret: bytesToHex(SECRET), - identifiers: [PASSKEY], - }, - identifier: null, - receipts: [], - }); + const writing = await passthroughEncryptor.encrypt(validPending); const writingMessenger = getMessenger(getRootMessenger()); const writingController = new MfaRecoveryController({ ...options, @@ -403,6 +463,47 @@ describe('MfaRecoveryController', () => { }); }); + it('resumes a persisted authorizing updateIdentifiers mutation', async () => { + await withController(async ({ controller, options }) => { + await controller.register(SECRET, [PASSKEY]); + const payload = { identifiers: [EMAIL] }; + const payloadHash = await hash(payload); + const mutationFields = { + id: '0xmutidentifiers', + profileId: 'profile-1', + operation: 'updateIdentifiers' as const, + expectedVersion: 1, + newVersion: 2, + payloadHash, + audiences: ['escrow-a', 'escrow-b'], + }; + const mutation = { + ...mutationFields, + requestHash: await hash(mutationFields), + }; + const authorizing = { + phase: 'authorizing' as const, + mutation, + payload, + identifier: PASSKEY, + }; + const resumed = new MfaRecoveryController({ + ...options, + messenger: getMessenger(getRootMessenger()), + state: { + pendingOperation: await passthroughEncryptor.encrypt(authorizing), + }, + }); + + await resumed.resume(); + + expect(await resumed.getPhase()).toBe('idle'); + expect(bytesToHex(await resumed.getRecoverySecret(EMAIL))).toBe( + bytesToHex(SECRET), + ); + }); + }); + it('is a no-op to resume or abort when idle', async () => { await withController(async ({ controller }) => { await controller.resume(); @@ -553,21 +654,35 @@ describe('MfaRecoveryController', () => { identifiers: [PASSKEY], }; const payloadHash = await hash(payload); + const mutation = { + id: '0xmut', + profileId: 'profile-1', + operation: 'register' as const, + expectedVersion: 0, + newVersion: 1, + payloadHash, + audiences: ['escrow-a', 'escrow-b'], + requestHash: '', + }; + mutation.requestHash = await hash({ + id: mutation.id, + profileId: mutation.profileId, + operation: mutation.operation, + expectedVersion: mutation.expectedVersion, + newVersion: mutation.newVersion, + payloadHash: mutation.payloadHash, + audiences: mutation.audiences, + }); const writing = await passthroughEncryptor.encrypt({ phase: 'writing', - mutation: { - id: '0xmut', - profileId: 'profile-1', - operation: 'register', - expectedVersion: 0, - newVersion: 1, - payloadHash, - audiences: ['escrow-a', 'escrow-b'], - requestHash: '0x', - }, + mutation, authControllerToken: { profileId: 'profile-1', - requestHash: '0x', + requestHash: mutation.requestHash, + identifiersHash: await hash( + canonicalizeIdentifiers(payload.identifiers), + ), + identifierOwnershipApproved: true, issuer: 'stub-auth', expiresAt: Date.now() + 60_000, signature: 'stub-auth-signature', @@ -577,7 +692,7 @@ describe('MfaRecoveryController', () => { receipts: [ { mutationId: '0xmut', - requestHash: '0x', + requestHash: mutation.requestHash, escrowId: 'escrow-z', version: 1, signature: '0x', @@ -603,21 +718,35 @@ describe('MfaRecoveryController', () => { identifiers: [PASSKEY], }; const payloadHash = await hash(payload); + const mutation = { + id: '0xmut', + profileId: 'profile-1', + operation: 'register' as const, + expectedVersion: 0, + newVersion: 1, + payloadHash, + audiences: ['escrow-a', 'escrow-b'], + requestHash: '', + }; + mutation.requestHash = await hash({ + id: mutation.id, + profileId: mutation.profileId, + operation: mutation.operation, + expectedVersion: mutation.expectedVersion, + newVersion: mutation.newVersion, + payloadHash: mutation.payloadHash, + audiences: mutation.audiences, + }); const writing = await passthroughEncryptor.encrypt({ phase: 'writing', - mutation: { - id: '0xmut', - profileId: 'profile-1', - operation: 'register', - expectedVersion: 0, - newVersion: 1, - payloadHash, - audiences: ['escrow-a', 'escrow-b'], - requestHash: '0x', - }, + mutation, authControllerToken: { profileId: 'profile-1', - requestHash: '0x', + requestHash: mutation.requestHash, + identifiersHash: await hash( + canonicalizeIdentifiers(payload.identifiers), + ), + identifierOwnershipApproved: true, issuer: 'stub-auth', expiresAt: Date.now() + 60_000, signature: 'stub-auth-signature', @@ -627,7 +756,7 @@ describe('MfaRecoveryController', () => { receipts: [ { mutationId: '0xmut', - requestHash: '0x', + requestHash: mutation.requestHash, escrowId: 'escrow-a', version: 1, signature: '0x', @@ -654,6 +783,58 @@ describe('MfaRecoveryController', () => { }); }); + it('persists valid receipts before reporting an invalid receipt', async () => { + await withController(async ({ controller, escrowA, escrowB }) => { + escrowA.invalidReceipts = true; + + await expect(controller.register(SECRET, [PASSKEY])).rejects.toThrow( + 'Invalid mutation receipt', + ); + + const pending = JSON.parse( + controller.state.pendingOperation as string, + ) as { + receipts: { escrowId: string }[]; + }; + expect(pending.receipts).toStrictEqual([ + expect.objectContaining({ escrowId: 'escrow-b' }), + ]); + expect(await escrowB.getRecoveryMetadata('profile-1')).toStrictEqual({ + version: 1, + lastMutationId: expect.any(String), + }); + }); + }); + + it('rejects a receipt whose escrow id does not match its target', async () => { + await withController(async ({ controller, escrowA }) => { + jest.spyOn(escrowA, 'applyMutation').mockResolvedValue({ + mutationId: '0xmutation', + requestHash: '0xrequest', + escrowId: 'escrow-z', + version: 1, + signature: '0xsignature', + }); + jest.spyOn(escrowA, 'verifyReceipt').mockReturnValue(true); + + await expect(controller.register(SECRET, [PASSKEY])).rejects.toThrow( + 'Invalid mutation receipt', + ); + }); + }); + + it('treats receipt verification errors as invalid receipts', async () => { + await withController(async ({ controller, escrowA }) => { + jest.spyOn(escrowA, 'verifyReceipt').mockImplementation(() => { + throw new Error('verification failed'); + }); + + await expect(controller.register(SECRET, [PASSKEY])).rejects.toThrow( + 'Invalid mutation receipt', + ); + }); + }); + it('refreshes an expired AuthController token while resuming', async () => { await withController(async ({ controller, escrowA, options }) => { await controller.register(SECRET, [PASSKEY]); @@ -674,6 +855,207 @@ describe('MfaRecoveryController', () => { expect(await resumed.getPhase()).toBe('idle'); }); }); + + it('rejects a null decrypted pending operation before external calls', async () => { + await withController(async ({ escrowA, escrowB, options }) => { + const auth = jest.spyOn( + options.authProvider, + 'authorizeRecoveryRequest', + ); + const availabilityA = jest.spyOn(escrowA, 'isAvailable'); + const availabilityB = jest.spyOn(escrowB, 'isAvailable'); + const controller = new MfaRecoveryController({ + ...options, + messenger: getMessenger(getRootMessenger()), + state: { pendingOperation: 'null' }, + }); + + await expect(controller.resume()).rejects.toThrow( + 'Invalid pending operation', + ); + + expect(auth).not.toHaveBeenCalled(); + expect(availabilityA).not.toHaveBeenCalled(); + expect(availabilityB).not.toHaveBeenCalled(); + }); + }); + + it.each([ + { + name: 'an invalid phase', + mutate: (pending: Record): void => { + pending.phase = 'invalid'; + }, + }, + { + name: 'a malformed mutation', + mutate: (pending: Record): void => { + pending.mutation = null; + }, + }, + { + name: 'an operation and payload mismatch', + mutate: (pending: Record): void => { + const mutation = pending.mutation as Record; + mutation.operation = 'updateIdentifiers'; + pending.payload = { recoverySecret: bytesToHex(SECRET) }; + }, + }, + { + name: 'a malformed payload', + mutate: (pending: Record): void => { + pending.payload = null; + }, + }, + { + name: 'a payload without a recovery secret', + mutate: (pending: Record): void => { + pending.payload = {}; + }, + }, + { + name: 'an identifier payload with invalid entries', + mutate: (pending: Record): void => { + const mutation = pending.mutation as Record; + mutation.operation = 'updateIdentifiers'; + pending.payload = { identifiers: [{}] }; + }, + }, + { + name: 'a non-null register identifier', + mutate: (pending: Record): void => { + const mutation = pending.mutation as Record; + mutation.operation = 'register'; + pending.payload = { + recoverySecret: bytesToHex(SECRET), + identifiers: [PASSKEY], + }; + pending.identifier = PASSKEY; + }, + }, + { + name: 'a null update identifier', + mutate: (pending: Record): void => { + pending.identifier = null; + }, + }, + { + name: 'an unknown identifier type', + mutate: (pending: Record): void => { + pending.identifier = { + type: 'unknown', + namespace: 'example.com', + value: 'unknown', + verifier: null, + }; + }, + }, + { + name: 'a non-consecutive version', + mutate: (pending: Record): void => { + const mutation = pending.mutation as Record; + mutation.expectedVersion = 4; + mutation.newVersion = 6; + }, + }, + { + name: 'a mismatched payload hash', + mutate: (pending: Record): void => { + const mutation = pending.mutation as Record; + mutation.payloadHash = '0xwrong-payload-hash'; + }, + }, + { + name: 'a mismatched mutation request hash', + mutate: (pending: Record): void => { + const mutation = pending.mutation as Record; + mutation.requestHash = '0xwrong-request-hash'; + }, + }, + { + name: 'an out-of-order audience list', + mutate: (pending: Record): void => { + const mutation = pending.mutation as Record; + mutation.audiences = ['escrow-b', 'escrow-a']; + }, + }, + { + name: 'an invalid AuthController token', + mutate: (pending: Record): void => { + const token = pending.authControllerToken as Record; + token.requestHash = '0xwrong-token-request-hash'; + }, + }, + { + name: 'a malformed AuthController token', + mutate: (pending: Record): void => { + pending.authControllerToken = null; + }, + }, + { + name: 'an AuthController token with invalid fields', + mutate: (pending: Record): void => { + pending.authControllerToken = {}; + }, + }, + { + name: 'an invalid receipt array', + mutate: (pending: Record): void => { + pending.receipts = null; + }, + }, + { + name: 'a receipt with invalid fields', + mutate: (pending: Record): void => { + pending.receipts = [{}]; + }, + }, + { + name: 'duplicate receipts', + mutate: (pending: Record): void => { + const mutation = pending.mutation as Record; + const receipt = { + mutationId: mutation.id, + requestHash: mutation.requestHash, + escrowId: 'escrow-a', + version: mutation.newVersion, + signature: '0xsignature', + }; + pending.receipts = [receipt, receipt]; + }, + }, + ])( + 'rejects persisted state with $name before external calls', + async ({ mutate }) => { + await withController(async ({ escrowA, escrowB, options }) => { + const pending = JSON.parse( + JSON.stringify(await getValidWritingPending()), + ) as Record; + mutate(pending); + const auth = jest.spyOn( + options.authProvider, + 'authorizeRecoveryRequest', + ); + const availabilityA = jest.spyOn(escrowA, 'isAvailable'); + const availabilityB = jest.spyOn(escrowB, 'isAvailable'); + const applyA = jest.spyOn(escrowA, 'applyMutation'); + const applyB = jest.spyOn(escrowB, 'applyMutation'); + const controller = new MfaRecoveryController({ + ...options, + messenger: getMessenger(getRootMessenger()), + state: { pendingOperation: JSON.stringify(pending) }, + }); + + await expect(controller.resume()).rejects.toThrow(/./u); + + expect(auth).not.toHaveBeenCalled(); + expect(availabilityA).not.toHaveBeenCalled(); + expect(availabilityB).not.toHaveBeenCalled(); + expect(applyA).not.toHaveBeenCalled(); + expect(applyB).not.toHaveBeenCalled(); + }); + }, + ); }); describe('identifier registry', () => { @@ -717,6 +1099,52 @@ function getMessenger( }); } +function getOptions( + escrowA: StubEscrowProvider, + escrowB: StubEscrowProvider, +): MfaRecoveryControllerOptions { + return { + messenger: getMessenger(getRootMessenger()), + authProvider: new StubAuthProvider(), + identifierAuthProvider: new StubIdentifierAuthProvider(), + escrows: [escrowA, escrowB], + pendingOperationEncryptor: passthroughEncryptor, + collectChallengeResponse: async (): Promise => 'otp', + }; +} + +async function getValidWritingPending(): Promise { + const payload = { recoverySecret: bytesToHex(SECRET_2) }; + const payloadHash = await hash(payload); + const fields = { + id: '0xvalid-pending', + profileId: 'profile-1', + operation: 'updateRecoverySecret' as const, + expectedVersion: 1, + newVersion: 2, + payloadHash, + audiences: ['escrow-a', 'escrow-b'], + }; + return { + phase: 'writing', + mutation: { + ...fields, + requestHash: await hash(fields), + }, + authControllerToken: { + profileId: 'profile-1', + requestHash: await hash(fields), + twoFactor: true, + issuer: 'stub-auth', + expiresAt: Date.now() + 60_000, + signature: 'stub-auth-signature', + }, + payload, + identifier: PASSKEY, + receipts: [], + }; +} + async function withController( testFunction: WithControllerCallback, ): Promise { @@ -724,13 +1152,9 @@ async function withController( const controllerMessenger = getMessenger(rootMessenger); const escrowA = new StubEscrowProvider('escrow-a'); const escrowB = new StubEscrowProvider('escrow-b'); - const options: MfaRecoveryControllerOptions = { + const options = { + ...getOptions(escrowA, escrowB), messenger: controllerMessenger, - authProvider: new StubAuthProvider(), - identifierAuthProvider: new StubIdentifierAuthProvider(), - escrows: [escrowA, escrowB], - pendingOperationEncryptor: passthroughEncryptor, - collectChallengeResponse: async (): Promise => 'otp', }; const controller = new MfaRecoveryController(options); return await testFunction({ diff --git a/packages/mfa-recovery-controller/src/MfaRecoveryController.ts b/packages/mfa-recovery-controller/src/MfaRecoveryController.ts index caecc45763..dbfd098535 100644 --- a/packages/mfa-recovery-controller/src/MfaRecoveryController.ts +++ b/packages/mfa-recovery-controller/src/MfaRecoveryController.ts @@ -1,11 +1,10 @@ import type { ControllerGetStateAction, - ControllerStateChangeEvent, + ControllerStateChangedEvent, StateMetadata, } from '@metamask/base-controller'; import { BaseController } from '@metamask/base-controller'; import type { Messenger } from '@metamask/messenger'; -import { areUint8ArraysEqual } from '@metamask/utils'; import { bytesToSecretHex, @@ -15,13 +14,16 @@ import { sign, } from './crypto.js'; import { MfaRecoveryError, MutationRepairPendingError } from './errors.js'; +import { + isFulfilledResult, + isMutationReceipt, + selectHighestConsistentVersion, + verifyMutationReceipt, +} from './escrow-utils.js'; import { getIdentifierAuthMode } from './identifier-auth.js'; import type { MfaRecoveryControllerMethodActions } from './MfaRecoveryController-method-action-types.js'; -import { - assertAbortAllowed, - assertSameAudienceIds, - getRecoveryPhase, -} from './state-machine.js'; +import { assertValidPendingOperation } from './pending-operation-validation.js'; +import { assertAbortAllowed, getRecoveryPhase } from './state-machine.js'; import type { AuthControllerToken, EscrowAuthChallenge, @@ -103,12 +105,14 @@ export type MfaRecoveryControllerActions = type AllowedActions = never; -export type MfaRecoveryControllerStateChangeEvent = ControllerStateChangeEvent< - typeof CONTROLLER_NAME, - MfaRecoveryControllerState ->; +export type MfaRecoveryControllerStateChangedEvent = + ControllerStateChangedEvent< + typeof CONTROLLER_NAME, + MfaRecoveryControllerState + >; -export type MfaRecoveryControllerEvents = MfaRecoveryControllerStateChangeEvent; +export type MfaRecoveryControllerEvents = + MfaRecoveryControllerStateChangedEvent; type AllowedEvents = never; @@ -179,6 +183,8 @@ export class MfaRecoveryController extends BaseController< readonly #escrows: RecoveryEscrowProvider[]; + readonly #escrowIds: string[]; + readonly #encryptor: PendingOperationEncryptor; readonly #collectChallengeResponse: ( @@ -205,8 +211,8 @@ export class MfaRecoveryController extends BaseController< 'empty_escrow_set', ); } - const ids = escrows.map((escrow) => escrow.id); - if (new Set(ids).size !== ids.length) { + const escrowIds = escrows.map((escrow) => escrow.id); + if (new Set(escrowIds).size !== escrowIds.length) { throw new MfaRecoveryError('Duplicate escrow id', 'duplicate_escrow_id'); } @@ -222,7 +228,8 @@ export class MfaRecoveryController extends BaseController< this.#authProvider = authProvider; this.#identifierAuthProvider = identifierAuthProvider; - this.#escrows = escrows; + this.#escrows = [...escrows]; + this.#escrowIds = [...escrowIds]; this.#encryptor = pendingOperationEncryptor; this.#collectChallengeResponse = collectChallengeResponse; this.#now = now; @@ -335,7 +342,7 @@ export class MfaRecoveryController extends BaseController< ...(await escrow.getSecret(authorizations[index], requestId)), })), ); - return this.#selectHighestConsistentVersion(results).recoverySecret; + return selectHighestConsistentVersion(results).recoverySecret; }); } @@ -383,7 +390,7 @@ export class MfaRecoveryController extends BaseController< await this.#repairPendingMutation(configured); const profileId = await this.#authProvider.getAuthenticatedProfileId(); const targets = await this.#requireAllEscrows(configured); - const audiences = configured.map((escrow) => escrow.id); + const audiences = [...this.#escrowIds]; const currentVersion = await this.#resolveCurrentRecoveryVersion({ operation, profileId, @@ -464,55 +471,54 @@ export class MfaRecoveryController extends BaseController< if (!saved) { return; } - assertSameAudienceIds( - saved.mutation.audiences, - escrows.map((escrow) => escrow.id), - ); - const authControllerToken = - saved.phase === 'authorizing' || - this.#tokenNeedsRefresh(saved.authControllerToken) - ? await this.#authorizeMutation(saved.mutation, saved.payload) - : saved.authControllerToken; - const pending: WritingPendingOperation = { - phase: 'writing', - mutation: saved.mutation, - authControllerToken, - payload: saved.payload, - identifier: saved.identifier, - receipts: saved.phase === 'writing' ? saved.receipts : [], - }; - await this.#persistPending(pending); - await this.#replicateMutation({ - escrows: await this.#requireAllEscrows(escrows), - pending, - }); + if (saved.phase === 'authorizing') { + const availableEscrows = await this.#requireAllEscrows(escrows); + const pending: WritingPendingOperation = { + phase: 'writing', + mutation: saved.mutation, + authControllerToken: await this.#authorizeMutation( + saved.mutation, + saved.payload, + ), + payload: saved.payload, + identifier: saved.identifier, + receipts: [], + }; + await this.#persistPending(pending); + await this.#replicateMutation({ + escrows: availableEscrows, + pending, + availabilityChecked: true, + }); + return; + } + + await this.#replicateMutation({ escrows, pending: saved }); } async #replicateMutation({ escrows, pending, + availabilityChecked = false, }: { escrows: RecoveryEscrowProvider[]; pending: WritingPendingOperation; + availabilityChecked?: boolean; }): Promise { const { mutation, payload, identifier } = pending; - const expectedPayloadHash = await hash(payload); - if (expectedPayloadHash !== mutation.payloadHash) { - throw new MfaRecoveryError( - 'Pending payload does not match mutation', - 'payload_mismatch', - ); - } - const configuredIds = new Set(escrows.map((escrow) => escrow.id)); + const entries = escrows.map((escrow) => ({ + escrow, + id: this.#getConfiguredEscrowId(escrow), + })); pending.receipts.forEach((receipt) => { - const escrow = escrows.find((item) => item.id === receipt.escrowId); - if (!configuredIds.has(receipt.escrowId) || escrow === undefined) { + const entry = entries.find((item) => item.id === receipt.escrowId); + if (entry === undefined) { throw new MfaRecoveryError( 'Receipt escrow is not configured', 'unknown_receipt_escrow', ); } - if (!escrow.verifyReceipt(receipt, mutation)) { + if (!verifyMutationReceipt(receipt, mutation, entry.escrow, entry.id)) { throw new MfaRecoveryError( 'Invalid mutation receipt', 'invalid_receipt', @@ -522,44 +528,69 @@ export class MfaRecoveryController extends BaseController< const acknowledged = new Set( pending.receipts.map((receipt) => receipt.escrowId), ); - const targets = escrows.filter((escrow) => !acknowledged.has(escrow.id)); + const targets = entries + .filter((entry) => !acknowledged.has(entry.id)) + .map((entry) => entry.escrow); if (targets.length === 0) { await this.#clearPending(); return; } + const availableTargets = availabilityChecked + ? targets + : await this.#requireAllEscrows(targets); + const refreshedToken = this.#tokenNeedsRefresh(pending.authControllerToken) + ? await this.#authorizeMutation(mutation, payload) + : pending.authControllerToken; + const effectivePending = + refreshedToken === pending.authControllerToken + ? pending + : { ...pending, authControllerToken: refreshedToken }; + if (effectivePending !== pending) { + await this.#persistPending(effectivePending); + } const authorizations = mutation.operation === 'register' - ? targets.map(() => null) + ? availableTargets.map(() => null) : await this.#authorizeIdentifier({ - escrows: targets, + escrows: availableTargets, identifier: identifier as Identifier, requestHash: mutation.requestHash, }); const results = await Promise.allSettled( - targets.map((escrow, index) => + availableTargets.map((escrow, index) => escrow.applyMutation( mutation, - pending.authControllerToken, + effectivePending.authControllerToken, authorizations[index], payload, ), ), ); - const receipts: MutationReceipt[] = [...pending.receipts]; + const receipts: MutationReceipt[] = [...effectivePending.receipts]; + let hasInvalidReceipt = false; results.forEach((result, index) => { - if (result.status === 'fulfilled') { - const escrow = targets[index]; - if (!escrow.verifyReceipt(result.value, mutation)) { - throw new MfaRecoveryError( - 'Invalid mutation receipt', - 'invalid_receipt', - ); - } - receipts.push(result.value); + if (!isFulfilledResult(result)) { + return; + } + const entry = { + escrow: availableTargets[index], + id: this.#getConfiguredEscrowId(availableTargets[index]), + }; + if ( + !isMutationReceipt(result.value) || + !verifyMutationReceipt(result.value, mutation, entry.escrow, entry.id) + ) { + hasInvalidReceipt = true; + return; } + receipts.push(result.value); }); - await this.#persistPending({ ...pending, receipts }); + await this.#persistPending({ ...effectivePending, receipts }); + + if (hasInvalidReceipt) { + throw new MfaRecoveryError('Invalid mutation receipt', 'invalid_receipt'); + } if ( new Set(receipts.map((receipt) => receipt.escrowId)).size !== @@ -589,7 +620,7 @@ export class MfaRecoveryController extends BaseController< requestHash, }); const challenges = await Promise.all( - escrows.map((escrow) => escrow.generateChallenge()), + escrows.map((escrow) => escrow.generateChallenge(proofKey.publicKey)), ); return await Promise.all( challenges.map(async (challenge) => { @@ -675,42 +706,6 @@ export class MfaRecoveryController extends BaseController< return selected.version; } - #selectHighestConsistentVersion( - results: PromiseSettledResult<{ - escrowId: string; - recoverySecret: Uint8Array; - version: number; - lastMutationId: string; - }>[], - ): { recoverySecret: Uint8Array; version: number; lastMutationId: string } { - const responses = results - .filter((result) => result.status === 'fulfilled') - .map((result) => result.value); - if (responses.length === 0) { - throw new MfaRecoveryError( - 'No escrow returned a recovery secret', - 'read_failed', - ); - } - const highestVersion = Math.max( - ...responses.map((response) => response.version), - ); - const highest = responses.filter( - (response) => response.version === highestVersion, - ); - const selected = highest[0]; - if ( - !highest.every( - (response) => - response.lastMutationId === selected.lastMutationId && - areUint8ArraysEqual(response.recoverySecret, selected.recoverySecret), - ) - ) { - throw new MfaRecoveryError('Replica corruption', 'replica_corruption'); - } - return selected; - } - async #getAvailableEscrows(): Promise { const flags = await Promise.all( this.#escrows.map(async (escrow) => ({ @@ -754,7 +749,15 @@ export class MfaRecoveryController extends BaseController< if (pendingOperation === null) { return null; } - return await this.#encryptor.decrypt(pendingOperation); + const pending = (await this.#encryptor.decrypt( + pendingOperation, + )) as unknown; + await assertValidPendingOperation(pending, this.#escrowIds); + return pending as PendingOperation; + } + + #getConfiguredEscrowId(escrow: RecoveryEscrowProvider): string { + return this.#escrowIds[this.#escrows.indexOf(escrow)]; } async #persistPending(operation: PendingOperation): Promise { diff --git a/packages/mfa-recovery-controller/src/escrow-utils.ts b/packages/mfa-recovery-controller/src/escrow-utils.ts new file mode 100644 index 0000000000..7566fd6803 --- /dev/null +++ b/packages/mfa-recovery-controller/src/escrow-utils.ts @@ -0,0 +1,116 @@ +import { areUint8ArraysEqual } from '@metamask/utils'; + +import { MfaRecoveryError } from './errors.js'; +import type { + Mutation, + MutationReceipt, + RecoveryEscrowProvider, +} from './types.js'; + +export type RecoverySecretResult = { + escrowId: string; + recoverySecret: Uint8Array; + version: number; + lastMutationId: string; +}; + +/** + * Identifies a fulfilled promise result for TypeScript narrowing. + * + * @param result - Settled promise result. + * @returns Whether the result is fulfilled. + */ +export function isFulfilledResult( + result: PromiseSettledResult, +): result is PromiseFulfilledResult { + return result.status === 'fulfilled'; +} + +/** + * Checks the serialized shape of a mutation receipt. + * + * @param value - Decrypted or provider-returned value. + * @returns Whether the value has a mutation receipt shape. + */ +export function isMutationReceipt(value: unknown): value is MutationReceipt { + return ( + isRecord(value) && + typeof value.mutationId === 'string' && + typeof value.requestHash === 'string' && + typeof value.escrowId === 'string' && + typeof value.version === 'number' && + Number.isInteger(value.version) && + value.version >= 0 && + typeof value.signature === 'string' + ); +} + +/** + * Verifies a receipt against its expected target escrow. + * + * The provider performs cryptographic verification using its build-pinned + * configuration; the target identity check is performed here as well. + * + * @param receipt - Receipt returned by an escrow. + * @param mutation - Mutation the receipt must acknowledge. + * @param escrow - Expected target escrow provider. + * @param expectedEscrowId - Build-configured identity of the target escrow. + * @returns Whether the receipt is valid for the target. + */ +export function verifyMutationReceipt( + receipt: MutationReceipt, + mutation: Mutation, + escrow: RecoveryEscrowProvider, + expectedEscrowId: string, +): boolean { + if (receipt.escrowId !== expectedEscrowId) { + return false; + } + try { + return escrow.verifyReceipt(receipt, mutation, expectedEscrowId); + } catch { + return false; + } +} + +/** + * Selects the highest consistent successful recovery-secret response. + * + * @param results - Settled escrow read results. + * @returns The selected recovery-secret response. + * @throws If no response succeeds or matching highest versions disagree. + */ +export function selectHighestConsistentVersion( + results: PromiseSettledResult[], +): Omit { + const responses = results + .filter(isFulfilledResult) + .map((result) => result.value); + if (responses.length === 0) { + throw new MfaRecoveryError( + 'No escrow returned a recovery secret', + 'read_failed', + ); + } + const highestVersion = Math.max( + ...responses.map((response) => response.version), + ); + const highest = responses.filter( + (response) => response.version === highestVersion, + ); + const selected = highest[0]; + if ( + !highest.every( + (response) => + response.lastMutationId === selected.lastMutationId && + areUint8ArraysEqual(response.recoverySecret, selected.recoverySecret), + ) + ) { + throw new MfaRecoveryError('Replica corruption', 'replica_corruption'); + } + return selected; +} + +function isRecord(value: unknown): value is Record { + return value !== null && typeof value === 'object' && !Array.isArray(value); +} diff --git a/packages/mfa-recovery-controller/src/index.ts b/packages/mfa-recovery-controller/src/index.ts index 257996841c..cf8f807fea 100644 --- a/packages/mfa-recovery-controller/src/index.ts +++ b/packages/mfa-recovery-controller/src/index.ts @@ -9,7 +9,7 @@ export type { MfaRecoveryControllerMessenger, MfaRecoveryControllerOptions, MfaRecoveryControllerState, - MfaRecoveryControllerStateChangeEvent, + MfaRecoveryControllerStateChangedEvent, } from './MfaRecoveryController.js'; export type { MfaRecoveryControllerAbortAction, diff --git a/packages/mfa-recovery-controller/src/pending-operation-validation.ts b/packages/mfa-recovery-controller/src/pending-operation-validation.ts new file mode 100644 index 0000000000..6b4f5789f2 --- /dev/null +++ b/packages/mfa-recovery-controller/src/pending-operation-validation.ts @@ -0,0 +1,214 @@ +import { hash } from './crypto.js'; +import { MfaRecoveryError } from './errors.js'; +import { isMutationReceipt } from './escrow-utils.js'; +import { getIdentifierAuthMode } from './identifier-auth.js'; +import { assertSameAudienceIds } from './state-machine.js'; +import type { + AuthControllerToken, + Identifier, + Mutation, + MutationPayload, + MutationReceipt, + RegisterPayload, + UpdateIdentifiersPayload, + UpdateRecoverySecretPayload, +} from './types.js'; + +/** + * Validates decrypted pending state before it can trigger authorization or + * escrow operations. + * + * @param pending - Decrypted pending state. + * @param escrowIds - Build-configured escrow ids. + * @throws If the pending state is malformed or inconsistent. + */ +export async function assertValidPendingOperation( + pending: unknown, + escrowIds: string[], +): Promise { + if (!isRecord(pending)) { + throwInvalidPendingOperation(); + } + if (pending.phase !== 'authorizing' && pending.phase !== 'writing') { + throwInvalidPendingOperation(); + } + if (!isRecord(pending.mutation)) { + throwInvalidPendingOperation(); + } + + const mutationValue = pending.mutation; + if ( + typeof mutationValue.id !== 'string' || + typeof mutationValue.profileId !== 'string' || + !isMutationOperation(mutationValue.operation) || + typeof mutationValue.expectedVersion !== 'number' || + !Number.isInteger(mutationValue.expectedVersion) || + mutationValue.expectedVersion < 0 || + typeof mutationValue.newVersion !== 'number' || + !Number.isInteger(mutationValue.newVersion) || + mutationValue.newVersion < 0 || + mutationValue.newVersion !== mutationValue.expectedVersion + 1 || + typeof mutationValue.payloadHash !== 'string' || + typeof mutationValue.requestHash !== 'string' || + !Array.isArray(mutationValue.audiences) || + !mutationValue.audiences.every((audience): audience is string => { + return typeof audience === 'string'; + }) + ) { + throwInvalidPendingOperation(); + } + const mutation = mutationValue as unknown as Mutation; + assertSameAudienceIds(mutation.audiences, escrowIds); + + const { payload } = pending; + if (!isMutationPayload(mutation.operation, payload)) { + throwInvalidPendingOperation(); + } + const payloadIdentifiers = + mutation.operation === 'updateRecoverySecret' + ? [] + : (payload as RegisterPayload | UpdateIdentifiersPayload).identifiers; + for (const identifier of payloadIdentifiers) { + getIdentifierAuthMode(identifier.type); + } + + if ( + mutation.operation === 'register' + ? pending.identifier !== null + : !isIdentifier(pending.identifier) + ) { + throwInvalidPendingOperation(); + } + if (mutation.operation !== 'register' && isIdentifier(pending.identifier)) { + getIdentifierAuthMode(pending.identifier.type); + } + + if ((await hash(payload)) !== mutation.payloadHash) { + throw new MfaRecoveryError( + 'Pending payload does not match mutation', + 'payload_mismatch', + ); + } + const requestHash = await hash({ + id: mutation.id, + profileId: mutation.profileId, + operation: mutation.operation, + expectedVersion: mutation.expectedVersion, + newVersion: mutation.newVersion, + payloadHash: mutation.payloadHash, + audiences: mutation.audiences, + }); + if (requestHash !== mutation.requestHash) { + throwInvalidPendingOperation(); + } + + if (pending.phase !== 'writing') { + return; + } + if ( + !isAuthControllerToken(pending.authControllerToken) || + pending.authControllerToken.profileId !== mutation.profileId || + pending.authControllerToken.requestHash !== mutation.requestHash || + (mutation.operation !== 'register' && + pending.authControllerToken.twoFactor !== true) || + ((mutation.operation === 'register' || + mutation.operation === 'updateIdentifiers') && + (pending.authControllerToken.identifierOwnershipApproved !== true || + typeof pending.authControllerToken.identifiersHash !== 'string')) || + !isMutationReceiptArray(pending.receipts) + ) { + throwInvalidPendingOperation(); + } +} + +function isRecord(value: unknown): value is Record { + return value !== null && typeof value === 'object' && !Array.isArray(value); +} + +function isMutationOperation(value: unknown): value is Mutation['operation'] { + return ( + value === 'register' || + value === 'updateRecoverySecret' || + value === 'updateIdentifiers' + ); +} + +function isMutationPayload( + operation: Mutation['operation'], + payload: unknown, +): payload is MutationPayload { + if (!isRecord(payload)) { + return false; + } + if (operation === 'updateRecoverySecret') { + return ( + Object.keys(payload).every((key) => key === 'recoverySecret') && + typeof (payload as UpdateRecoverySecretPayload).recoverySecret === + 'string' + ); + } + const { identifiers } = payload as RegisterPayload | UpdateIdentifiersPayload; + return ( + Object.keys(payload).every((key) => + operation === 'register' + ? key === 'recoverySecret' || key === 'identifiers' + : key === 'identifiers', + ) && + (operation === 'register' + ? typeof (payload as RegisterPayload).recoverySecret === 'string' + : true) && + Array.isArray(identifiers) && + identifiers.length > 0 && + identifiers.every(isIdentifier) + ); +} + +function isIdentifier(value: unknown): value is Identifier { + return ( + isRecord(value) && + typeof value.type === 'string' && + typeof value.namespace === 'string' && + typeof value.value === 'string' && + Object.prototype.hasOwnProperty.call(value, 'verifier') + ); +} + +function isAuthControllerToken(value: unknown): value is AuthControllerToken { + if (!isRecord(value)) { + return false; + } + return ( + typeof value.profileId === 'string' && + typeof value.requestHash === 'string' && + (value.twoFactor === undefined || value.twoFactor === true) && + (value.identifiersHash === undefined || + typeof value.identifiersHash === 'string') && + (value.identifierOwnershipApproved === undefined || + value.identifierOwnershipApproved === true) && + typeof value.issuer === 'string' && + typeof value.expiresAt === 'number' && + Number.isFinite(value.expiresAt) && + typeof value.signature === 'string' + ); +} + +function isMutationReceiptArray(value: unknown): value is MutationReceipt[] { + if (!Array.isArray(value)) { + return false; + } + const escrowIds = new Set(); + for (const receipt of value) { + if (!isMutationReceipt(receipt) || escrowIds.has(receipt.escrowId)) { + return false; + } + escrowIds.add(receipt.escrowId); + } + return true; +} + +function throwInvalidPendingOperation(): never { + throw new MfaRecoveryError( + 'Invalid pending operation', + 'invalid_pending_operation', + ); +} diff --git a/packages/mfa-recovery-controller/src/types.ts b/packages/mfa-recovery-controller/src/types.ts index 63d0c1d853..ffb74f5ea6 100644 --- a/packages/mfa-recovery-controller/src/types.ts +++ b/packages/mfa-recovery-controller/src/types.ts @@ -129,8 +129,12 @@ export type PendingOperation = export type RecoveryPhase = 'idle' | PendingOperation['phase']; /** - * Ciphertext of {@link PendingOperation}. Wallet secure storage encrypts this - * value; a raw recovery secret is never persisted in the clear. + * Encoded ciphertext of {@link PendingOperation}. Wallet secure storage + * encrypts this value; a raw recovery secret is never persisted in the clear. + * + * The recovery spec's pseudocode represents this as `Uint8Array`, but the + * controller stores the encoded ciphertext as a string so BaseController state + * persistence and serialization preserve it without binary coercion. */ export type EncryptedPendingOperation = string; @@ -174,7 +178,7 @@ export type RecoveryIdentifierAuthProvider = { export type RecoveryEscrowProvider = { readonly id: string; isAvailable: () => Promise; - generateChallenge: () => Promise; + generateChallenge: (publicKey: string) => Promise; beginIdentifierAuthentication: (params: { identifier: Identifier; requestHash: string; @@ -196,7 +200,19 @@ export type RecoveryEscrowProvider = { identifierAuthorization: IdentifierAuthorization | null, payload: MutationPayload, ) => Promise; - verifyReceipt: (receipt: MutationReceipt, mutation: Mutation) => boolean; + /** + * Verifies a receipt cryptographically using the build-pinned key for the + * expected escrow and confirms it targets that escrow. + * + * @param receipt - Receipt returned by an escrow. + * @param mutation - Mutation acknowledged by the receipt. + * @param expectedEscrowId - Escrow identity expected by the caller. + */ + verifyReceipt: ( + receipt: MutationReceipt, + mutation: Mutation, + expectedEscrowId: string, + ) => boolean; }; /** diff --git a/packages/mfa-recovery-controller/tests/stubs.ts b/packages/mfa-recovery-controller/tests/stubs.ts index 579dcc7955..a8d0443410 100644 --- a/packages/mfa-recovery-controller/tests/stubs.ts +++ b/packages/mfa-recovery-controller/tests/stubs.ts @@ -379,14 +379,19 @@ export class StubEscrowProvider implements RecoveryEscrowProvider { return await this.#signReceipt(mutation); } - verifyReceipt(receipt: MutationReceipt, mutation: Mutation): boolean { + verifyReceipt( + receipt: MutationReceipt, + mutation: Mutation, + expectedEscrowId: string, + ): boolean { if (this.invalidReceipts) { return false; } return ( receipt.mutationId === mutation.id && receipt.requestHash === mutation.requestHash && - receipt.escrowId === this.id && + receipt.escrowId === expectedEscrowId && + expectedEscrowId === this.id && receipt.version === mutation.newVersion ); } From 91964b3c124aa121a6d3959cb81ddc4ebaa31930 Mon Sep 17 00:00:00 2001 From: ieow <4881057+ieow@users.noreply.github.com> Date: Tue, 1 Sep 2026 15:23:51 +0800 Subject: [PATCH 3/3] Enhance `MfaRecoveryController` with additional test cases and authorization handling - Added tests to ensure recovery secret retrieval continues despite failures in escrow availability checks and authorization processes. - Updated the controller to handle cases where authorization for one escrow fails, preventing unauthorized writes and ensuring consistent recovery secret retrieval. - Refactored authorization handling to return structured results for better error management and state validation. These changes improve the robustness of the MFA recovery process, ensuring a smoother user experience in MetaMask. --- .../src/MfaRecoveryController.test.ts | 71 ++++++++++++ .../src/MfaRecoveryController.ts | 102 +++++++++++++----- 2 files changed, 144 insertions(+), 29 deletions(-) diff --git a/packages/mfa-recovery-controller/src/MfaRecoveryController.test.ts b/packages/mfa-recovery-controller/src/MfaRecoveryController.test.ts index b2153b8b77..66dc43a063 100644 --- a/packages/mfa-recovery-controller/src/MfaRecoveryController.test.ts +++ b/packages/mfa-recovery-controller/src/MfaRecoveryController.test.ts @@ -204,6 +204,58 @@ describe('MfaRecoveryController', () => { }); }); + it('continues reading when an escrow availability check fails', async () => { + await withController(async ({ controller, escrowA }) => { + await controller.register(SECRET, [PASSKEY]); + jest + .spyOn(escrowA, 'isAvailable') + .mockRejectedValue(new Error('health check failed')); + + const recovered = await controller.getRecoverySecret(PASSKEY); + + expect(bytesToHex(recovered)).toBe(bytesToHex(SECRET)); + }); + }); + + it('continues reading when key-bound authorization fails for one escrow', async () => { + await withController(async ({ controller, escrowA }) => { + await controller.register(SECRET, [PASSKEY]); + jest + .spyOn(escrowA, 'generateChallenge') + .mockRejectedValue(new Error('challenge failed')); + + const recovered = await controller.getRecoverySecret(PASSKEY); + + expect(bytesToHex(recovered)).toBe(bytesToHex(SECRET)); + }); + }); + + it('continues reading when escrow-challenge authorization fails for one escrow', async () => { + await withController(async ({ controller, escrowA }) => { + await controller.register(SECRET, [EMAIL]); + jest + .spyOn(escrowA, 'beginIdentifierAuthentication') + .mockRejectedValue(new Error('challenge failed')); + + const recovered = await controller.getRecoverySecret(EMAIL); + + expect(bytesToHex(recovered)).toBe(bytesToHex(SECRET)); + }); + }); + + it('continues reading when completing one escrow challenge fails', async () => { + await withController(async ({ controller, escrowA }) => { + await controller.register(SECRET, [EMAIL]); + jest + .spyOn(escrowA, 'completeIdentifierAuthentication') + .mockRejectedValue(new Error('grant failed')); + + const recovered = await controller.getRecoverySecret(EMAIL); + + expect(bytesToHex(recovered)).toBe(bytesToHex(SECRET)); + }); + }); + it('throws when matching versions disagree', async () => { await withController(async ({ controller, escrowB }) => { await controller.register(SECRET, [PASSKEY]); @@ -279,6 +331,25 @@ describe('MfaRecoveryController', () => { }); }); + it('does not write when one escrow cannot be authorized', async () => { + await withController(async ({ controller, escrowA, escrowB }) => { + await controller.register(SECRET, [PASSKEY]); + jest + .spyOn(escrowA, 'generateChallenge') + .mockRejectedValue(new Error('challenge failed')); + const applyA = jest.spyOn(escrowA, 'applyMutation'); + const applyB = jest.spyOn(escrowB, 'applyMutation'); + + await expect( + controller.updateRecoverySecret(PASSKEY, SECRET_2), + ).rejects.toThrow('Unable to authorize every escrow'); + + expect(applyA).not.toHaveBeenCalled(); + expect(applyB).not.toHaveBeenCalled(); + expect(await controller.getPhase()).toBe('writing'); + }); + }); + it('throws when replicas disagree on version', async () => { await withController(async ({ controller, escrowB }) => { await controller.register(SECRET, [PASSKEY]); diff --git a/packages/mfa-recovery-controller/src/MfaRecoveryController.ts b/packages/mfa-recovery-controller/src/MfaRecoveryController.ts index dbfd098535..36f70cf33e 100644 --- a/packages/mfa-recovery-controller/src/MfaRecoveryController.ts +++ b/packages/mfa-recovery-controller/src/MfaRecoveryController.ts @@ -71,6 +71,11 @@ const MESSENGER_EXPOSED_METHODS = [ 'getPhase', ] as const; +type AuthorizedEscrow = { + escrow: RecoveryEscrowProvider; + authorization: IdentifierAuthorization; +}; + export type MfaRecoveryControllerState = { /** * Encrypted pending mutation, or `null` when idle. @@ -331,15 +336,15 @@ export class MfaRecoveryController extends BaseController< 'no_available_escrow', ); } - const authorizations = await this.#authorizeIdentifier({ + const authorizedEscrows = await this.#authorizeIdentifier({ escrows: available, identifier, requestHash, }); const results = await Promise.allSettled( - available.map(async (escrow, index) => ({ + authorizedEscrows.map(async ({ escrow, authorization }) => ({ escrowId: escrow.id, - ...(await escrow.getSecret(authorizations[index], requestId)), + ...(await escrow.getSecret(authorization, requestId)), })), ); return selectHighestConsistentVersion(results).recoverySecret; @@ -550,18 +555,27 @@ export class MfaRecoveryController extends BaseController< } const authorizations = mutation.operation === 'register' - ? availableTargets.map(() => null) + ? undefined : await this.#authorizeIdentifier({ escrows: availableTargets, identifier: identifier as Identifier, requestHash: mutation.requestHash, }); + if ( + authorizations !== undefined && + authorizations.length !== availableTargets.length + ) { + throw new MfaRecoveryError( + 'Unable to authorize every escrow', + 'identifier_auth_failed', + ); + } const results = await Promise.allSettled( availableTargets.map((escrow, index) => escrow.applyMutation( mutation, effectivePending.authControllerToken, - authorizations[index], + authorizations?.[index]?.authorization ?? null, payload, ), ), @@ -609,7 +623,7 @@ export class MfaRecoveryController extends BaseController< escrows: RecoveryEscrowProvider[]; identifier: Identifier; requestHash: string; - }): Promise { + }): Promise { const mode = getIdentifierAuthMode(identifier.type); if (mode === 'key-bound') { const proofKey = await generateSigningKey(); @@ -619,44 +633,70 @@ export class MfaRecoveryController extends BaseController< proofPublicKey: proofKey.publicKey, requestHash, }); - const challenges = await Promise.all( + const challengeResults = await Promise.allSettled( escrows.map((escrow) => escrow.generateChallenge(proofKey.publicKey)), ); - return await Promise.all( - challenges.map(async (challenge) => { + const challenges = challengeResults.flatMap((result, index) => + isFulfilledResult(result) + ? [{ escrow: escrows[index], challenge: result.value }] + : [], + ); + const authorizationResults = await Promise.allSettled( + challenges.map(async ({ escrow, challenge }) => { const message = await hash([token, challenge.id, requestHash]); return { - kind: 'key-bound' as const, - token, - proof: { - challengeId: challenge.id, - requestHash, - signature: await sign(proofKey.privateKey, message), + escrow, + authorization: { + kind: 'key-bound' as const, + token, + proof: { + challengeId: challenge.id, + requestHash, + signature: await sign(proofKey.privateKey, message), + }, }, }; }), ); + return authorizationResults + .filter(isFulfilledResult) + .map((result) => result.value); } - const challenges = await Promise.all( + const challengeResults = await Promise.allSettled( escrows.map((escrow) => escrow.beginIdentifierAuthentication({ identifier, requestHash }), ), ); - const responses = await Promise.all( - challenges.map((challenge) => this.#collectChallengeResponse(challenge)), + const challenges = challengeResults.flatMap((result, index) => + isFulfilledResult(result) + ? [{ escrow: escrows[index], challenge: result.value }] + : [], ); - const grants = await Promise.all( - escrows.map((escrow, index) => - escrow.completeIdentifierAuthentication( - challenges[index].id, - responses[index], + const responseResults = await Promise.allSettled( + challenges.map(async (entry) => ({ + ...entry, + response: await this.#collectChallengeResponse(entry.challenge), + })), + ); + const responses = responseResults + .filter(isFulfilledResult) + .map((result) => result.value); + const grantResults = await Promise.allSettled( + responses.map(async ({ escrow, challenge, response }) => ({ + escrow, + grant: await escrow.completeIdentifierAuthentication( + challenge.id, + response, ), - ), + })), ); - return grants.map((grant) => ({ - kind: 'escrow-challenge' as const, - grant, + return grantResults.filter(isFulfilledResult).map(({ value }) => ({ + escrow: value.escrow, + authorization: { + kind: 'escrow-challenge' as const, + grant: value.grant, + }, })); } @@ -707,13 +747,17 @@ export class MfaRecoveryController extends BaseController< } async #getAvailableEscrows(): Promise { - const flags = await Promise.all( + const flags = await Promise.allSettled( this.#escrows.map(async (escrow) => ({ escrow, available: await escrow.isAvailable(), })), ); - return flags.filter((item) => item.available).map((item) => item.escrow); + return flags.flatMap((result) => + isFulfilledResult(result) && result.value.available + ? [result.value.escrow] + : [], + ); } async #requireAllEscrows(