feat: align MFA recovery with Auth API approvals

packages/mfa-recovery-controller/ARCHITECTURE.md

@@ -34,19 +34,18 @@ flowchart LR
 | --------------------------- | --------------------------------------------------------------------------- |
 | `pendingOperation`          | Only persisted field: encrypted `authorizing` / `writing` blob, or `null`   |
 | `escrows[]`                 | Build-time replica set; non-empty, unique `id`s; mutations require **all**  |
-| `authProvider`              | Profile id + request-bound AuthController token (Hydra JWT)                 |
+| `authProvider`              | Profile id + request-bound Auth API approval                                |
 | `identifierAuthProvider`    | Key-bound IdP assertion (passkey / OIDC / SIWE)                             |
 | `pendingOperationEncryptor` | Encrypts / decrypts the pending blob with wallet secure storage             |
 | `now`                       | Clock (Unix seconds) for AuthController token expiry; defaults to `unixNow` |
 | `#withLock`                 | Serializes every public method except `authenticateIdentifier`              |
 
-Each escrow has two identities: `id` is the replica id used in mutation
-`audiences` and receipts; `authAudience` is the audience the AuthController
-token must carry for that escrow. They are independent (Cubist uses `"cubist"`
-for both).
+`id` identifies the replica in mutations and receipts; `authAudience` identifies
+the approval audience. Cubist uses `id: "cubist"` and an `authAudience` matching
+`AUTH_CONTROLLER_AUDIENCE`.
 
 Identifier types pick the auth mode from a trusted table, not a client flag:
-`passkey` / `oidc` / `siwe` → key-bound. Any other type is rejected with
+`passkey` / `social` / `siwe` → key-bound. Any other type is rejected with
 `unknown_identifier_type`. `emailOtp` / `smsOtp` escrow-challenge is not wired
 yet (MFA-605 / MFA-606 / MFA-568).
 
@@ -113,10 +112,13 @@ Anything that fails is rejected rather than acted on:
   must match the mutation fields.
 - `register` has `expectedVersion` `0` and `identifier: null`; updates carry
   an identifier.
-- `writing` only: the AuthController token's `sub` is the mutation
-  `profileId`, its `ext.request_hash` is the mutation `requestHash`, updates
-  have `aal >= 2`, `register` / `updateIdentifiers` carry `identifiers_hash`,
-  and receipts are well-formed with at most one per escrow.
+- `writing` only: the approval binds `sub` to `profileId` and `ext.request_hash`
+  to `requestHash`, with `aal >= 2`. `register` / `updateIdentifiers` also need
+  `identifiers_hash` to match `approvedIdentifiers`, and every payload
+  identifier to equal one of them exactly. This runs before `writing` is
+  persisted: an escrow rejection is not a `Deny`, so a mismatch found there
+  would leave an unabortable write. Receipts must be well-formed with at most
+  one per escrow.
 
 ## Mutation sequence (`register` / updates)
 
@@ -164,15 +166,20 @@ sequenceDiagram
   end
 ```
 
-AuthController token request:
-
-- `requestHash` is the mutation `requestHash`; `audiences` is every escrow's
-  `authAudience`.
-- Updates set `requireTwoFactor`. `register` and `updateIdentifiers` pass the
-  payload `identifiers` so the token is bound to them.
-- The token is a compact Hydra JWT. The controller decodes its claims without
-  verifying the signature (escrows verify it) and only uses them for
-  pending-state checks and the `exp` refresh.
+Auth approvals:
+
+- The provider receives the operation's `requestHash` and each target escrow's
+  `authAudience`. All writes and reads set `requireTwoFactor` for AAL2.
+- The issuer controls approval lifetime. AAL2 requires recent MFA.
+- The approval's `identifiers` are persisted with its token as
+  `approvedIdentifiers` and refreshed with it. `register` and
+  `updateIdentifiers` send them to every escrow, which checks them against
+  `identifiers_hash` and rejects payload identifiers outside them. Hashing
+  sorts object keys, not arrays.
+- Missing or stale MFA rejects with `AuthenticationController`'s
+  `StepUpRequiredError`.
+- Escrows verify signatures. The controller decodes claims only for pending-state
+  validation and expiry-based refresh.
 
 Receipts:
 
@@ -204,11 +211,11 @@ retries only escrows that do not already have a stored receipt. Updates
 re-run identifier auth (a new key-bound token and PoP) for those escrows on
 every attempt, so the identifier provider must be able to authenticate again.
 
-## Cubist store sequence
+## Cubist integration
 
 The controller does not call CubeSigner, the registration API, or
-`KeyringController`. A Cubist deploy injects one `CubistEscrowProvider`
-(`id` and `authAudience`: `"cubist"`). The caller proves OIDC, calls
+`KeyringController`. A Cubist deploy injects one `CubistEscrowProvider`.
+The caller proves OIDC, calls
 `ensure-user`, opens the session, and constructs the provider with the
 authenticated `CubeSignerClient` and the pinned wrap and receipt public keys
 before `register`.
@@ -223,53 +230,7 @@ the mutation id, request hash, escrow id, `version === newVersion`,
 `receiptKeyId` (SHA-256 of the pinned receipt key), and the Ed25519 signature
 locally.
 
-`register(recoverySecret, identifiers)` takes an identifier **array** of at
-least two distinct key-bound identifiers. One `siwe` identifier is rejected.
-`applyMutation` is the store: the controller wraps the secret to the escrow
-wrap key, and the Cubist C2F persists that payload.
-
-`authenticateIdentifier` asks the injected identifier provider for a key-bound
-token. A SIWE provider signs with `KeyringController.signPersonalMessage`.
-The controller does not. `getRecoverySecret` then challenges the escrow, signs
-the proof of possession locally, calls `getSecret`, and unwraps. `epoch` is
-the replica `version`.
-
-```mermaid
-sequenceDiagram
-  participant Caller as Caller
-  participant Auth as RecoveryAuthProvider
-  participant IdP as SIWE identifier provider
-  participant Ctrl as MfaRecoveryController
-  participant Escrow as Cubist escrow
-
-  Note over Caller,Escrow: proveOidcIdentity, ensure-user, createOidcSession, then new CubistEscrowProvider
-
-  Caller->>Ctrl: register(secret, identifiers)
-  Ctrl->>Escrow: isAvailable (ping)
-  Ctrl->>Auth: getAuthenticatedProfileId
-  Ctrl->>Ctrl: persist authorizing
-  Ctrl->>Auth: authorizeRecoveryRequest
-  Auth-->>Ctrl: AuthController token (Hydra JWT)
-  Ctrl->>Ctrl: wrap secret, persist writing
-  Ctrl->>Escrow: applyMutation
-  Escrow-->>Ctrl: receipt
-  Ctrl->>Ctrl: verify receipt, clear pending
-
-  Caller->>Ctrl: authenticateIdentifier(siwe)
-  Ctrl->>IdP: getKeyBoundIdentifierToken
-  Note over IdP: signPersonalMessage
-  IdP-->>Ctrl: token
-  Ctrl-->>Caller: identifierSession
-
-  Caller->>Ctrl: getRecoverySecret(identifierSession)
-  Ctrl->>Escrow: isAvailable (ping)
-  Ctrl->>Escrow: generateChallenge
-  Ctrl->>Ctrl: sign proof of possession
-  Ctrl->>Escrow: getSecret
-  Escrow-->>Ctrl: wrapped secret + version + wrapKeyId
-  Ctrl->>Ctrl: check wrapKeyId, unwrap
-  Ctrl-->>Caller: secret bytes + epoch
-```
+For a single-escrow walkthrough, see [Cubist test sequence](CUBIST_TEST_SEQUENCE.md).
 
 ## Read sequence (`getRecoverySecret`)
 
@@ -283,6 +244,7 @@ sequenceDiagram
   participant C as Client
   participant M as MfaRecoveryController
   participant I as IdentifierAuthProvider
+  participant A as RecoveryAuthProvider
   participant E as Available escrows
 
   C->>M: authenticateIdentifier(identifier)
@@ -296,9 +258,11 @@ sequenceDiagram
   alt none available
     M-->>C: no_available_escrow
   end
+  M->>A: authorizeRecoveryRequest (read hash, AAL2)
+  A-->>M: request-bound approval
   M->>E: generateChallenge + PoP (partial OK)
   par read
-    M->>E: getSecret
+    M->>E: getSecret(approval, authorization, requestId, pkE)
     M->>M: check wrapKeyId, unwrap with ephemeral key
   end
   M->>M: selectHighestConsistentVersion
@@ -330,7 +294,7 @@ the highest one. The next mutation still requires `resume()` first.
 ```mermaid
 flowchart TD
   Start[identifier + requestHash] --> Mode{trusted type?}
-  Mode -->|passkey / oidc / siwe| KB[fresh proof key, IdP key-bound token]
+  Mode -->|passkey / social / siwe| KB[fresh proof key, IdP key-bound token]
   KB --> Chal[each escrow generateChallenge]
   Chal --> PoP[sign PoP over hash of token + challengeId + requestHash]
   Mode -->|emailOtp / smsOtp| Todo[TODO MFA-605 / MFA-606 / MFA-568]
@@ -341,22 +305,3 @@ On **mutation**, every remaining target escrow must authorize or the write
 does not start (`identifier_auth_failed`). On **read**, escrows whose
 challenge fails are skipped. `register` sends no identifier authorization
 (`null`).
-
-## Crash / repair
-
-```mermaid
-flowchart TD
-  Crash[crash or partial apply] --> Phase{pending phase?}
-  Phase -->|none| Idle[idle]
-  Phase -->|authorizing| AbortOrResume{client choice}
-  AbortOrResume -->|abort| Idle
-  AbortOrResume -->|resume| Auth[re-issue AuthController token]
-  Auth --> Write[replicate]
-  Phase -->|writing| ResumeOnly[resume only]
-  ResumeOnly --> Skip[skip escrows that already have verified receipts]
-  Skip --> Refresh[re-issue token only if expired]
-  Refresh --> Write
-  Write --> All{receipts for every configured escrow?}
-  All -->|yes| Clear[clear pending]
-  All -->|no| PendingErr[IncompleteMutationError]
-```

packages/mfa-recovery-controller/CHANGELOG.md

@@ -10,7 +10,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 
 - Initial release of `MfaRecoveryController`, which manages MFA recovery flows and persists in-flight operations in its state ([#10022](https://github.com/MetaMask/core/pull/10022))
+  - Requires recent, request-bound AAL2 approvals for writes and reads.
+  - Sends the approval's identifiers to escrows as `approvedIdentifiers` with `register` and `updateIdentifiers` mutations, and rejects payload identifiers that are not exactly among them before writing ([#10883](https://github.com/MetaMask/core/pull/10883))
   - Exports `MfaRecoveryController`, `getDefaultMfaRecoveryControllerState`, and the controller's action, event, messenger, state, and options types.
+  - Exports `AuthApiRecoveryAuthProvider`, `AuthApiRecoveryAuthProviderOptions` and `AuthApiRecoveryAuthProviderMessenger` for requesting approvals through `AuthenticationController:requestMfaApproval`, using the profile id from `AuthenticationController:refreshCanonicalProfileId` ([#10883](https://github.com/MetaMask/core/pull/10883))
   - Exports `CubistEscrowProvider` and `CubistEscrowProviderOptions` for storing recovery secrets with CubeSigner.
   - Exports `MfaRecoveryError`, `IncompleteMutationError`, and `MutationRejectedError`.
   - Exports `MIN_IDENTIFIERS`, `IDENTIFIER_AUTH_MODES`, and `getIdentifierAuthMode` for identifier authentication.

packages/mfa-recovery-controller/CUBIST_TEST_SEQUENCE.md

@@ -7,47 +7,14 @@ each one. Error branches, pending-state repair, and multi-escrow behavior are
 in [ARCHITECTURE.md](./ARCHITECTURE.md); treat the TypeScript in `src/` as the
 source of truth if a diagram drifts.
 
-| Step | Client call                                                  | Version |
-| ---- | ------------------------------------------------------------ | ------- |
-| 1    | `register(secret, [siwe, passkey])`                          | `0 → 1` |
-| 2    | `updateRecoverySecret(passkey, newSecret, epoch)`            | `1 → 2` |
-| 3    | `updateIdentifiers(siwe, [siwe, replacementPasskey], epoch)` | `2 → 3` |
+| Step | Client call                                                  | Read with            | Version |
+| ---- | ------------------------------------------------------------ | -------------------- | ------- |
+| 1    | `register(secret, [siwe, passkey])`                          | `siwe`               | `0 → 1` |
+| 2    | `updateRecoverySecret(passkey, newSecret, epoch)`            | `passkey`            | `1 → 2` |
+| 3    | `updateIdentifiers(siwe, [siwe, replacementPasskey], epoch)` | `replacementPasskey` | `2 → 3` |
 
 Each update passes the `epoch` returned by the previous read.
 
-## Overview
-
-```mermaid
-sequenceDiagram
-  autonumber
-  participant Mobile as Mobile client
-  participant C as MfaRecoveryController
-  participant Cubist as Cubist escrow
-
-  Note over Mobile,Cubist: Setup: sign in, open Cubist session, build controller
-
-  Mobile->>C: register(secret, [siwe, passkey])
-  C->>Cubist: applyMutation (version 0 → 1)
-  Cubist-->>C: signed receipt
-  Mobile->>C: read with siwe
-  C->>Cubist: getSecret
-  C-->>Mobile: secret, epoch 1
-
-  Mobile->>C: updateRecoverySecret(passkey, newSecret, 1)
-  C->>Cubist: applyMutation (version 1 → 2)
-  Cubist-->>C: signed receipt
-  Mobile->>C: read with passkey
-  C->>Cubist: getSecret
-  C-->>Mobile: newSecret, epoch 2
-
-  Mobile->>C: updateIdentifiers(siwe, [siwe, replacementPasskey], 2)
-  C->>Cubist: applyMutation (version 2 → 3)
-  Cubist-->>C: signed receipt
-  Mobile->>C: read with replacementPasskey
-  C->>Cubist: getSecret
-  C-->>Mobile: same secret, epoch 3
-```
-
 ## Setup
 
 The mobile client does this before it creates the controller. The controller
@@ -83,7 +50,9 @@ sequenceDiagram
 
 ## Write: `register`, `updateRecoverySecret`, `updateIdentifiers`
 
-All three mutations share one path.
+All mutations require a request-bound AAL2 approval. `register` and
+`updateIdentifiers` also send the approval's identifiers as
+`approvedIdentifiers`, bound by `identifiers_hash`.
 
 ```mermaid
 sequenceDiagram
@@ -98,7 +67,7 @@ sequenceDiagram
   C->>Auth: getAuthenticatedProfileId()
   Auth-->>C: profileId
   Note over C: Build the mutation (version epoch → epoch + 1)<br/>and its requestHash
-  C->>Auth: authorizeRecoveryRequest(requestHash, ...)
+  C->>Auth: authorizeRecoveryRequest(requestHash, requireTwoFactor: true, ...)
   Auth-->>C: JWT bound to requestHash
   C->>Cubist: isAvailable
 
@@ -107,17 +76,17 @@ sequenceDiagram
   end
 
   Note over C: Encrypt the secret to the Cubist wrap key
-  C->>Cubist: applyMutation(mutation, JWT, { token, proof } or null, payload)
+  C->>Cubist: applyMutation(mutation, JWT, { token, proof } or null, payload, approvedIdentifiers)
   Cubist-->>C: signed receipt
   Note over C: Verify the receipt with the pinned receipt key
   C-->>Mobile: done
 ```
 
-| Operation              | Identifier proof        | JWT requires     | Payload              |
-| ---------------------- | ----------------------- | ---------------- | -------------------- |
-| `register`             | none                    | identifiers      | identifiers + secret |
-| `updateRecoverySecret` | the identifier argument | 2FA              | secret               |
-| `updateIdentifiers`    | the identifier argument | 2FA, identifiers | identifiers          |
+| Operation              | Identifier proof        | Payload              |
+| ---------------------- | ----------------------- | -------------------- |
+| `register`             | none                    | identifiers + secret |
+| `updateRecoverySecret` | the identifier argument | secret               |
+| `updateIdentifiers`    | the identifier argument | identifiers          |
 
 If an escrow refuses the mutation (`MutationRejectedError`) and every replica
 does the same, the controller clears the pending mutation. A transport failure
@@ -126,47 +95,19 @@ to finish that write.
 
 ## Proof of possession
 
-Updates and reads prove control of a registered identifier in two parts. The
-identifier provider signs a token that names a fresh proof key and the
-`requestHash`. Then the controller signs a Cubist challenge with that proof
-key. Code: `requestKeyBoundIdentifierToken` and `authorizeEscrowsWithToken` in
-`src/identifier-auth.ts`.
-
-```mermaid
-sequenceDiagram
-  autonumber
-  participant C as MfaRecoveryController
-  participant Id as identifierAuthProvider
-  participant Cubist as Cubist escrow
-
-  rect rgba(128, 128, 128, 0.1)
-  Note over C,Id: Part 1: proof key + identifier token
-  Note over C: generateSigningKey() → Ed25519 proof key pair
-  C->>Id: getKeyBoundIdentifierToken(identifier, proofPublicKey, requestHash)
-  Note over Id: binding = hash([proofPublicKey, requestHash])<br/>SIWE: personal_sign a SIWE message, Nonce = binding<br/>Passkey: WebAuthn assertion, challenge = binding
-  Id-->>C: token { identifier, proofPublicKey, requestHash, providerAssertion }
-  end
+Updates and reads prove control of a registered identifier:
 
-  rect rgba(128, 128, 128, 0.1)
-  Note over C,Cubist: Part 2: challenge + signature
-  C->>Cubist: generateChallenge()
-  Cubist-->>C: challenge { id, escrowId, expiresAt }
-  Note over C: signature = sign(proofPrivateKey,<br/>hash([token, challenge.id, requestHash]))
-  Note over C: authorization = { kind: key-bound, token,<br/>proof: { challengeId, requestHash, signature } }
-  C->>Cubist: applyMutation or getSecret(authorization, ...)
-  Note over Cubist: providerAssertion is valid for the identifier<br/>and binds proofPublicKey + requestHash<br/>identifier is registered, challenge id is its own<br/>signature verifies under proofPublicKey
-  end
-```
+1. The controller generates an Ed25519 proof key and requests an identifier token
+   binding `hash([proofPublicKey, requestHash])`: the SIWE nonce or WebAuthn challenge.
+2. The proof key signs `hash([token, challenge.id, requestHash])` for each escrow.
+   The controller sends the approval and `{ kind: 'key-bound', token, proof }`
+   with `applyMutation` or `getSecret`.
+3. The escrow C2F verifies the provider assertion, registered identifier, request
+   binding, challenge, and proof signature.
 
-- The identifier provider signs once, over the proof key and `requestHash`.
-  The proof key then signs each escrow challenge, so the token works only for
-  this request.
-- The proof private key is never sent to the identifier provider or Cubist.
-  For reads it lives only in the session returned by `authenticateIdentifier`.
-- The Cubist-side checks happen in the escrow C2F (`cubist_secret_escrow`),
-  not in this package.
-- `register` sends no identifier proof (`null`); the JWT from `authProvider`
-  is the only authorization.
+The proof private key never leaves the controller/caller; read sessions keep it
+in memory. `register` sends `null` instead of an identifier proof. Implementation:
+`requestKeyBoundIdentifierToken` and `authorizeEscrowsWithToken` in `src/identifier-auth.ts`.
 
 ## Read: `authenticateIdentifier`, `getRecoverySecret`
 
@@ -175,6 +116,7 @@ sequenceDiagram
   autonumber
   participant Mobile as Mobile client
   participant C as MfaRecoveryController
+  participant Auth as authProvider
   participant Id as identifierAuthProvider
   participant Cubist as Cubist escrow
 
@@ -186,8 +128,10 @@ sequenceDiagram
   Mobile->>C: getRecoverySecret(session)
   Note over C: Recompute requestHash, must equal token.requestHash
   C->>Cubist: isAvailable
+  C->>Auth: authorizeRecoveryRequest(requestHash, requireTwoFactor: true, ...)
+  Auth-->>C: request-bound AAL2 approval
   Note over C,Cubist: Proof of possession, part 2: challenge + signature
-  C->>Cubist: getSecret({ token, proof }, requestId, pkE)
+  C->>Cubist: getSecret(approval, { token, proof }, requestId, pkE)
   Cubist-->>C: secret encrypted to the read key, version
   Note over C: Decrypt the secret
   C-->>Mobile: { recoverySecret, epoch: version }