Repository navigation
feat: align MFA recovery with Auth API approvals - #10883
Open
huggingbot wants to merge 65 commits into
Open
huggingbot wants to merge 65 commits into
huggingbot wants to merge 65 commits into
Conversation
- 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.
- 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.
…ization 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.
…MFA recovery controller - Added detailed architecture documentation for the `@metamask/mfa-recovery-controller`, outlining its role as an orchestration layer for MFA recovery, including state machine and mutation sequences. - Updated the handling of mutations to bind version allocation to the payload `epoch` for `register`, `updateRecoverySecret`, and `updateIdentifiers` actions. - Enhanced error handling to ensure that mutations are persisted correctly and can be resumed or aborted as necessary. - Updated tests to reflect changes in the mutation handling and added checks for the new `epoch` parameter in relevant actions. This commit introduces breaking changes that require consumers to adapt to the new mutation versioning scheme.
- Added support for P-256 proof signature generation and verification in the `crypto` module. - Introduced new utility functions for base64url encoding and decoding to facilitate JWK coordinate handling. - Updated tests to validate the new signing and verification processes, ensuring robust cryptographic operations. - Included `@noble/curves` as a dependency to leverage optimized elliptic curve operations. These changes improve the security and reliability of the MFA recovery process.
- Modified `getRecoverySecret` to return an object containing both the recovery secret and the selected `epoch`, enabling clients to use the correct version for subsequent mutations. - Updated related types and documentation to reflect the new return structure. - Adjusted tests to validate the new behavior, ensuring that the epoch is correctly returned and utilized in updates. These changes introduce breaking changes that require consumers to adapt to the new return format of `getRecoverySecret`.
- Added Jest types to the TypeScript configuration, enhancing type support for testing. - This change improves the development experience by ensuring proper type checking for Jest-related code. No breaking changes introduced.
…bist escrow Wrap each escrow's secret at apply, drop OTP challenge APIs, include receiptKeyId in unsigned receipt hashes, and treat token expiry as unix seconds.
… epoch off the mutation body
…oller Co-authored-by: Cursor <cursoragent@cursor.com>
… only before receipts New register/update calls require idle pending state. abort() is allowed in writing when no escrow has acknowledged the mutation, and identifiers must be distinct.
…pdate validation logic
…turn highest version without being blocked by a pending write
…dd new scripts for invoking and registering with the controller - Added `CubistEscrowProvider` to handle Cubist escrow commands. - Introduced scripts for invoking escrow and registering with the controller. - Updated `package.json` to include new dependencies and scripts. - Enhanced TypeScript configuration to include scripts in the build process. - Updated changelog to reflect the new features and changes.
…d session management - Added `authenticateIdentifier` method to handle identifier authentication and return an ephemeral session. - Introduced `IdentifierSession` type to encapsulate session data required for recovery secret retrieval. - Updated `getRecoverySecret` to accept an `IdentifierSession` instead of an identifier, ensuring session validation. - Enhanced tests to cover identifier authentication scenarios, including tampering checks and session reuse. - Refactored authorization logic to utilize key-bound tokens for improved security and session management.
…for identifier authentication - Renamed `getRecoverySecret` to accept an `IdentifierSession` instead of an identifier. - Added `authenticateIdentifier` to the public methods list in the architecture documentation. - Updated sequence diagrams to reflect the new authentication flow and session management. - Enhanced changelog to document the changes in public methods and breaking changes related to session handling.
…d session management - Added `authenticateIdentifier` method to handle identifier authentication and return an ephemeral session. - Introduced `IdentifierSession` type to encapsulate session data required for recovery secret retrieval. - Updated `getRecoverySecret` to accept an `IdentifierSession` instead of an identifier, ensuring session validation. - Enhanced tests to cover identifier authentication scenarios, including tampering checks and session reuse. - Refactored authorization logic to utilize key-bound tokens for improved security and session management.
…ng and authorization flow
… integration test documentation - Introduced a new section in ARCHITECTURE.md outlining the Cubist store sequence, detailing the interaction between the MfaRecoveryController and Cubist escrow. - Added CUBIST_TEST_SEQUENCE.md to document the integration test flow for the Cubist MFA recovery process, including sequence diagrams for clarity.
…er' into cw/mfa-recovery-controller Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # packages/mfa-recovery-controller/src/MfaRecoveryController.ts # packages/mfa-recovery-controller/src/identifier-auth.ts # packages/mfa-recovery-controller/src/types.ts
…ra JWTs Decode compact access-token claims (exp, sub, aal, request_hash, identifiers_hash) instead of a local token object.
… cw/mfa-recovery-controller Co-authored-by: Cursor <cursoragent@cursor.com>
…tailed integration test flow - Updated the documentation for the Cubist MFA recovery integration test sequence in CUBIST_TEST_SEQUENCE.md. - Enhanced the overview section with a sequence diagram illustrating the interaction between Mobile, MfaRecoveryController, and Cubist escrow. - Added detailed descriptions of the setup process, write path mutations, and read path operations. - Included notes on the use of development-only token routes and stub providers for testing.
…rity and consistency - Updated the documentation to clarify the role of the client in the MFA recovery sequence, replacing references to "Mobile" with "Client." - Streamlined the sequence diagrams and descriptions to enhance readability and understanding of the integration test flow. - Adjusted notes regarding the setup process to reflect the client's responsibilities more accurately.
…w sequence diagrams and clarifications - Revised the documentation to clarify the client's role in the MFA recovery process, specifically updating references to the MPC token service. - Added new sequence diagrams illustrating the happy path and full path with errors for better understanding of the integration flow. - Enhanced notes regarding the use of development-only token routes for testing purposes.
- Introduced a new private method `#assertEscrowsAvailable` to check the availability of configured escrows before performing mutations. - Updated existing methods to call this new assertion method, ensuring that all configured escrows are available before proceeding with operations. - Enhanced tests to verify that recovery requests are not authorized when escrows are unavailable, and that appropriate errors are thrown. - Improved test coverage for scenarios involving escrow availability during recovery secret updates and resumptions.
Co-authored-by: Cursor <cursoragent@cursor.com>
…ontroller Co-authored-by: Cursor <cursoragent@cursor.com>
…enhanced cryptographic operations - Added dependencies for `@metamask/cryptography` and `@metamask/superstruct` to improve cryptographic functionalities and data validation. - Updated various functions in the `crypto` module to support asynchronous operations, ensuring compatibility with the new cryptography methods. - Enhanced the `verifyReceipt` and `sign` functions to return promises, allowing for better handling of asynchronous operations. - Refactored tests to accommodate the changes in the cryptographic functions, ensuring all tests are now asynchronous. - Updated the changelog to reflect these significant changes in functionality and dependencies.
…-recovery-controller - Integrated `@metamask/superstruct` to enhance data validation for mutation receipts and other structures. - Refactored `isMutationReceipt` function to utilize `superstruct` for improved validation logic. - Added unit tests for `isMutationReceipt` to ensure correct handling of valid and invalid receipt shapes. - Updated `cubist-escrow-provider` to leverage structured validation for response parsing, enhancing error handling. - Included new test cases to validate receipt rejection scenarios based on various invalid fields.
- Added `json-stable-stringify` to the `mfa-recovery-controller` package for improved JSON serialization. - Updated the `canonicalize` function to utilize `json-stable-stringify`, ensuring consistent output for complex data structures. - Introduced new unit tests to validate the handling of `Uint8Array` values and the removal of undefined values in objects and arrays. - Removed the previous `sortKeys` function, streamlining the canonicalization process.
- Documented the initial release of the `MfaRecoveryController` in the changelog. - Removed detailed entries from the previous changelog section to streamline the release notes.
- Merged the entries for `zod` from version `4.4.3` into `4.3.6`, ensuring a single version (`4.6.6`) is used across the project. - Removed the redundant `zod@npm:^4.4.3` entry to streamline the dependency management.
- Updated the architecture documentation to clarify roles and responsibilities of components, including `pendingOperationEncryptor` and `now`. - Improved descriptions of identifier types and their authentication modes. - Added details on pending state validation and mutation sequence for better understanding of the recovery process. - Streamlined the public surface section to reflect all exposed messenger actions. - Enhanced clarity on the state machine transitions and rules for mutation handling.
- Enhanced the initial release notes to detail the functionalities of `MfaRecoveryController`, including exports for various types and error handling. - Added descriptions for `CubistEscrowProvider` and related types for recovery operations.
…and remove Jest configurations - Deleted Jest configuration files and updated package.json to use Vitest for testing. - Updated test files to replace Jest APIs with Vitest equivalents, ensuring compatibility with the new testing framework. - Removed references to the `@metamask/mfa-recovery-controller` package from the Jest runner map in `test-runner-package-map.cjs`. - Adjusted oxlint suppressions to reflect the removal of Jest configurations.
huggingbot
force-pushed
the
feat/mfa-recovery-auth-approvals
branch
from
October 20, 2026 05:21
a6e0c97 to
88338db
Compare
Contributor
|
we need to use the authentication controller for token exchange |
…h-approvals Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # oxlint-suppressions.json # packages/mfa-recovery-controller/ARCHITECTURE.md # packages/mfa-recovery-controller/CHANGELOG.md # packages/mfa-recovery-controller/CUBIST_TEST_SEQUENCE.md # packages/mfa-recovery-controller/README.md # packages/mfa-recovery-controller/package.json # packages/mfa-recovery-controller/scripts/register-with-controller.ts # packages/mfa-recovery-controller/src/MfaRecoveryController.test.ts # packages/mfa-recovery-controller/src/MfaRecoveryController.ts # packages/mfa-recovery-controller/src/auth-controller-token.test.ts # packages/mfa-recovery-controller/src/auth-controller-token.ts # packages/mfa-recovery-controller/src/crypto.test.ts # packages/mfa-recovery-controller/src/crypto.ts # packages/mfa-recovery-controller/src/escrow-providers/cubist-escrow-provider.test.ts # packages/mfa-recovery-controller/src/escrow-providers/cubist-escrow-provider.ts # packages/mfa-recovery-controller/src/escrow-utils.ts # packages/mfa-recovery-controller/src/identifier-auth.ts # packages/mfa-recovery-controller/src/index.ts # packages/mfa-recovery-controller/src/pending-operation-validation.ts # packages/mfa-recovery-controller/src/types.ts # packages/mfa-recovery-controller/tests/stubs.ts # yarn.lock
Pass Auth's identifier list to the escrow as approvedIdentifiers instead of rebuilding the mutation, and map Auth API 403s to authentication_required.
3 of 4 tasks
…onical profile id Escrows report a subset mismatch as an error, not a Deny, which left the mutation in an unabortable writing phase. The cached canonical profile id can be stale after pairing elsewhere, so every approval's sub mismatched. Co-authored-by: Cursor <cursoragent@cursor.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
Aligns
@metamask/mfa-recovery-controllerwith Auth's MFA approvals and the escrow'sapprovedIdentifierscheck.AuthApiRecoveryAuthProviderrequests approvals viaAuthenticationController:requestMfaApproval(feat(profile-sync-controller): addAuthenticationController.requestMfaApproval#11071) and resolves the profile ID viarefreshCanonicalProfileId. Missing MFA throwsStepUpRequiredError: verify, thenresume().register/updateIdentifierssend Auth'sidentifiersasapprovedIdentifiers. Non-matching payload identifiers (verifier included) are rejected beforewriting, so the mutation stays abortable.References
Checklist