diff --git a/packages/seedless-onboarding-controller/CHANGELOG.md b/packages/seedless-onboarding-controller/CHANGELOG.md index 08b26bbab90..3ace674891c 100644 --- a/packages/seedless-onboarding-controller/CHANGELOG.md +++ b/packages/seedless-onboarding-controller/CHANGELOG.md @@ -7,6 +7,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `SeedlessPasswordChangePhase` enum and an optional `passwordChangePhase` state field that persists a non-sensitive password-change lifecycle phase used as a recovery signal. An unset/`undefined` phase means no password change is in progress ([#10148](https://github.com/MetaMask/core/pull/10148)) +- Add `PasswordSyncStatus` enum, returned by `resolvePasswordSyncState` and `reconcilePassword` to tell clients which step to run next after either an interrupted local password change or an another-device password change ([#10148](https://github.com/MetaMask/core/pull/10148)) +- Add `resolvePasswordSyncState({ skipCache })` and `SeedlessOnboardingControllerResolvePasswordSyncStateAction` to resolve remote password state at unlock without consuming a password, replacing the removed `checkIsPasswordOutdated` read ([#10148](https://github.com/MetaMask/core/pull/10148)) +- Add `reconcilePassword({ globalPassword })` and `SeedlessOnboardingControllerReconcilePasswordAction` to bring local Seedless state up to date with the remote password after either an interrupted local password change or a password change made on another device ([#10148](https://github.com/MetaMask/core/pull/10148)) + - It performs the password-chain unlock and local vault rewrite internally, re-encrypts `encryptedKeyringEncryptionKey` under the new wrapping key so `loadKeyringEncryptionKey` keeps working, and advances the lifecycle to `LOCAL_KEYRING_PENDING` so the client reconciles the local Keyring before unlocking normally. +- Add `clearPasswordChangePhase` and `markPasswordChangeKeySyncPending` lifecycle-advance methods, along with `SeedlessOnboardingControllerClearPasswordChangePhaseAction` and `SeedlessOnboardingControllerMarkPasswordChangeKeySyncPendingAction`. `clearPasswordChangePhase` is the only way back to "no change in progress" and must be called once key synchronization and local persistence are verified ([#10148](https://github.com/MetaMask/core/pull/10148)) +- Add `SeedlessOnboardingControllerErrorMessage.PasswordChangeInProgress`, thrown when a password change is started while another one is unresolved ([#10148](https://github.com/MetaMask/core/pull/10148)) + +### Changed + +- **BREAKING:** `changePassword` is now lifecycle-aware: it writes the `SEEDLESS_CHANGE_PENDING`, `SEEDLESS_COMMITTED`, and `LOCAL_KEYRING_PENDING` phases and rejects a concurrent change with `PasswordChangeInProgress` ([#10148](https://github.com/MetaMask/core/pull/10148)) + - Clients must not start a second password change while a lifecycle is unfinished, and must drive the lifecycle to completion by calling `clearPasswordChangePhase`. See [0002](./docs/0002-password-change-recovery-flow.md) for the client integration guide. + +### Removed + +- **BREAKING:** Remove the `checkIsPasswordOutdated` method and `SeedlessOnboardingControllerCheckIsPasswordOutdatedAction` ([#10148](https://github.com/MetaMask/core/pull/10148)) + - Call `resolvePasswordSyncState({ skipCache })` instead. It performs the same remote check and additionally returns the required recovery step, so a boolean `true` result now corresponds to `PasswordSyncStatus.PasswordOutdated`. +- **BREAKING:** Remove the `submitGlobalPassword` and `syncLatestGlobalPassword` methods, along with `SeedlessOnboardingControllerSubmitGlobalPasswordAction` and `SeedlessOnboardingControllerSyncLatestGlobalPasswordAction` ([#10148](https://github.com/MetaMask/core/pull/10148)) + - Call `reconcilePassword({ globalPassword })` instead. It runs both steps internally in the correct order, re-wraps the stored Keyring encryption key, and records the lifecycle phase, none of which happened when the two methods were called directly. + ## [11.0.0] ### Changed diff --git a/packages/seedless-onboarding-controller/docs/0001-seedless-password-change-recovery.md b/packages/seedless-onboarding-controller/docs/0001-seedless-password-change-recovery.md new file mode 100644 index 00000000000..acfb376d3fd --- /dev/null +++ b/packages/seedless-onboarding-controller/docs/0001-seedless-password-change-recovery.md @@ -0,0 +1,378 @@ +# ADR 0001: Recovering server-first Seedless password changes + +- Status: Proposed +- Date: 2026-09-07 +- Scope: MetaMask extension password changes backed by Seedless + +## Context + +Changing a Seedless password updates multiple independently persisted states: + +1. The remote Seedless/TOPRF password, backup data, and key shares. +2. The local Seedless controller state. +3. The local KeyringController vault password. +4. The Keyring encryption key stored in Seedless. +5. The password-change lifecycle state. + +These changes cannot be committed atomically. A process, browser, app, device, network request, or persistence operation can fail between any two steps. + +The proposed ordering commits the remote Seedless change first. After remote commitment, recovery must support both possible local states: + +- The local Keyring is still protected by the old password. Recovery uses the new Seedless password to recover the stored Keyring encryption key, calls `submitEncryptionKey`, and re-encrypts the local Keyring with the new password. +- The local Keyring is already protected by the new password, but key synchronization failed. Recovery unlocks with the new password, exports the current Keyring encryption key, and synchronizes it to Seedless. + +The lifecycle state helps select the recovery checks, but it cannot establish the actual state. The process may terminate before the lifecycle update is persisted. Cryptographic verification and authoritative server-state checks must therefore determine the recovery branch. + +## Decision + +Use a durable, idempotent lifecycle state machine around the server-first operation. + +- Lock the wallet whenever the operation cannot establish a consistent state. +- On the next unlock, inspect the lifecycle state before normal unlock error handling. +- Require an authoritative Seedless server-state check for every unfinished state. +- Use cryptographic verification to determine whether the local Keyring is old or new. +- Re-run already-completed operations safely instead of attempting an in-process rollback. +- Lock the wallet from the client whenever any password-change or recovery step fails, before exposing an error or intermediary screen. +- Clear the lifecycle only after the current Keyring encryption key is synchronized to Seedless and all required local state is durably persisted. (There is no separate `COMPLETE` state: "no change in progress" and "done" are both represented by an unset/`undefined` phase.) +- Keep any state that cannot be distinguished safely as `unknown`. + +The lifecycle names below are descriptive. They can be mapped to the final implementation enum without changing the recovery semantics. The implementation uses `undefined` for the "no change in progress / done" state rather than a dedicated `IDLE`/`COMPLETE` enum member. + +## Implementation scope + +The implementation is split between shared controller capabilities and client-specific orchestration/UI. The KeyringController and SeedlessOnboardingController should provide safe primitives; neither controller alone can own the complete transaction because the operation spans both controllers and multiple persistence systems. + +### Controller and shared-contract scope + +#### SeedlessOnboardingController + +The controller already provides most of the required recovery primitives: + +- `changePassword` performs the Seedless password/vault change and handles the controller-level token-refresh path. +- `loadKeyringEncryptionKey` can recover the stored Keyring encryption key after the new Seedless password is submitted. +- `storeKeyringEncryptionKey` encrypts and stores the current Keyring encryption key in controller state. +- `reconcilePassword` provides the password-sync operation needed to rehydrate and update local Seedless state after either an interrupted local password change or an another-device password change. +- `checkIsPasswordOutdated({ skipCache: true })` provides a cache-bypassed password-state check. +- Controller locking already serializes controller-level operations. + +The new controller work is: + +- Add a persisted password-change lifecycle state/phase to `SeedlessOnboardingControllerState`, with persistence metadata. The lifecycle must not store passwords, SRPs, raw Keyring encryption keys, or decrypted backup material. +- Modify `changePassword` to update the lifecycle after each relevant operation: before the remote change, after remote commitment, after the local Seedless vault/state update, and when the operation fails or becomes ambiguous. +- Keep `storeKeyringEncryptionKey` lifecycle-neutral. Password-change-specific commits persist the encrypted Keyring encryption key and lifecycle phase together, while later client-driven synchronization uses the explicit lifecycle-advance methods. +- Do not let `storeKeyringEncryptionKey` clear the lifecycle by itself. Completion also requires client confirmation of the local Keyring state, remote synchronization, and durable persistence. +- Ensure a thrown error after a partial mutation does not reset the lifecycle to the pre-operation state. The last known phase must remain available for recovery. +- Facilitate the existing password-sync operations for both post-remote-commit recovery branches: + - Old local Keyring: submit the new Seedless password, load the stored Keyring encryption key, and allow the client to call `submitEncryptionKey` before re-encrypting locally. + - New local Keyring: submit the new password, verify/unlock the local Keyring, export its current Keyring encryption key, and store/synchronize it. +- Preserve the existing token-refresh and controller-lock behavior while making lifecycle transitions observable to clients. + +The controller must not infer completion from a successful in-memory update or from a rejected Promise. The client remains responsible for coordinating the KeyringController and for the final durable clear of the lifecycle. + +#### KeyringController + +No new KeyringController recovery API is required by this ADR. The client reuses the existing primitives: + +- `verifyPassword` cryptographically checks whether the local Keyring uses the old or new password. +- `submitEncryptionKey` unlocks an old local Keyring after its encryption key is recovered from Seedless. +- `changePassword` re-encrypts the local Keyring with the new password. +- `exportEncryptionKey` provides the current key for synchronization when the local Keyring is already new. + +The KeyringController remains responsible only for local vault operations. Remote Seedless status, cross-controller orchestration, and final lifecycle completion remain outside it. + +Wallet locking for password-change errors is also a client responsibility. The controllers should return the operation result/error and expose the locking primitives, while the client decides when to lock and which recovery/intermediary screen to present. + +#### Password-change coordinator and lifecycle persistence + +- The lifecycle state is persisted by `SeedlessOnboardingController`, while the client orchestration layer coordinates KeyringController operations against those phases. +- Persist only non-sensitive transaction data, such as lifecycle phase, transaction identifier, timestamps, retry metadata, and non-sensitive error classification. +- Write `SEEDLESS_CHANGE_PENDING` before the first remote mutation. +- Write `SEEDLESS_COMMITTED` only after remote commitment is confirmed by the server or an authoritative status check. +- Advance the lifecycle at the explicit password-change and recovery boundaries so a later unlock can identify the last known boundary. `storeKeyringEncryptionKey` only persists the encrypted key and does not advance or clear the lifecycle. +- Use an awaitable durable persistence operation for lifecycle transitions and the final clear. The generic debounced state-change path must not be the only durability boundary. +- Serialize password-change and recovery operations. A second request must be rejected or queued until the first transaction is cleared (no change in progress) or reaches an explicitly recoverable terminal state. +- Make recovery verify the actual cryptographic state before mutating either controller. +- Keep the recovery transaction active until Keyring encryption-key synchronization and local persistence are confirmed. Do not clear the lifecycle marker early. + +#### External server/API dependency + +The controller changes reuse the existing Seedless/TOPRF operations. The following server/API behavior must be confirmed or added outside the controller lifecycle-state changes: + +- Idempotent password-change and Keyring-key synchronization requests. +- A transaction identifier that can be queried after an ambiguous response. +- An authoritative distinction between old, new, and partial remote state. +- Verification that the current Keyring encryption key is the one stored by the completed backup. + +If these capabilities are unavailable, the client must preserve `unknown` rather than infer that an error means “nothing changed.” + +### Client scope + +The extension and mobile client must implement the same recovery contract. The platform-specific location of each responsibility may differ, but the ordering, state transitions, cryptographic checks, and completion criteria must not. + +#### Client orchestration + +Each client must provide an orchestration layer that: + +- Replaces the local-first/rollback flow with the server-first lifecycle flow. +- Persists the lifecycle phase before and after each irreversible boundary. +- Uses one transaction identifier for the password change and all retries. +- Locks the wallet if any password-change or recovery step returns an error, including remote Seedless operations, local controller operations, key synchronization, verification, and persistence. +- Performs the lock before surfacing the error, navigating to an intermediary screen, or returning control to a normal wallet screen. +- Keeps the wallet locked when recovery cannot establish a consistent state. If the lock operation itself fails, the client must keep the wallet in a recovery-blocked UI and must not expose wallet access. +- Prevents a second password-change transaction from running concurrently. + +After remote commitment, the orchestrator must cryptographically classify the local Keyring: + +- **Old Keyring:** recover the stored Keyring encryption key with the new Seedless password, call `submitEncryptionKey`, re-encrypt locally, export the current key, and synchronize it. +- **New Keyring:** verify/unlock with the new password, export the current key, and synchronize it. + +The orchestrator must not infer the local state from the lifecycle marker, a rejected Promise, or a cached password-outdated result. + +#### Client unlock and recovery + +Each client must route an unfinished lifecycle through recovery before normal unlock failure handling: + +- Read the last durably persisted lifecycle state before classifying a password as invalid. +- Perform an authoritative Seedless server-state check for every unfinished state. +- Bypass or invalidate cached password-outdated results during recovery. +- Verify cryptographically whether the supplied new password unlocks the local Keyring. +- Select the old-Keyring or new-Keyring recovery branch based on verification. +- Retry already-completed operations safely after process termination or a lost response. +- Leave the state as `unknown` when the server result or local cryptographic state cannot be established. + +#### Client persistence + +Each client must provide a durable persistence boundary for lifecycle state: + +- Lifecycle transitions must have an explicit, awaitable durable-write path. +- The client must be able to read the last lifecycle state before normal unlock routing begins. +- The lifecycle must be cleared only after the synchronized Keyring encryption key and all required local controller state are durably persisted. +- A generic debounce may remain acceptable for unrelated state, but it cannot prove that password-change state is durable. +- Lifecycle state must contain only non-sensitive metadata and must never contain passwords, SRPs, raw Keyring encryption keys, or decrypted backup material. + +#### Client UI and user behavior + +Each client must provide UI behavior for `SEEDLESS_CHANGE_PENDING`, `SEEDLESS_COMMITTED`, `LOCAL_KEYRING_PENDING`, `KEY_SYNC_PENDING`, and `UNKNOWN` (no dedicated UI state is needed for "no change in progress" — that is the normal wallet UI): + +- Show a recovery-blocked state for every unfinished lifecycle state. +- Treat any password-change error as a locked-wallet state before showing an error modal, retry screen, or other intermediary UI. +- Ask for the new password after remote commitment is established. +- Do not ask for the old Keyring password when Seedless can provide the Keyring encryption key through `submitEncryptionKey`. +- Explain whether the remote change is unresolved, local Keyring recovery is in progress, or Keyring-key synchronization is pending. +- Preserve retryable recovery actions across app/browser restarts and backgrounding. +- Prevent a second password change while recovery is pending. +- Do not display raw server/controller errors or sensitive recovery data. +- Do not expose the wallet as fully recovered until key synchronization is verified and the lifecycle is cleared. +- Keep reset wallet as an explicit last resort. It must not be triggered automatically for a recoverable partial state or used to hide an unresolved remote result. + +The client owns this lock/error boundary because it controls navigation and intermediary screens. This allows UX changes without changing the controller’s cryptographic responsibilities, while ensuring that no client-specific screen accidentally leaves a partially changed wallet unlocked. + +#### Client tests + +Each client must add unit, integration, and end-to-end coverage for: + +- Every lifecycle transition and durable-write boundary. +- Termination before and after remote commitment. +- Remote commitment with an old local Keyring. +- Remote commitment with an already-new local Keyring and failed key synchronization. +- Lost responses and idempotent retries. +- Stale/missing lifecycle state and stale password-outdated cache. +- Recovery UI, retry behavior, wallet locking, and explicit reset-wallet fallback. + +Each platform may map these tests to its own orchestration, persistence, authentication, navigation, and UI modules. The fault-injection scenarios and expected final states must remain equivalent. + +### Cross-client contract + +All clients must agree on: + +- Lifecycle state names and meanings. +- Which states require a server check before unlock. +- The two cryptographic recovery branches. +- Idempotency and transaction-identifier semantics. +- The definition of durable synchronization and the completion (clear) boundary. +- The meaning of `unknown` and the conditions under which reset wallet may be offered. +- The requirement that any password-change or recovery error locks the wallet before an error or intermediary screen is shown. + +Platform-specific UI can differ, but it must not change the recovery decision or silently treat an ambiguous state as a failed or completed password change. + +## Lifecycle state matrix + +This table defines what the user and UI should experience when the lifecycle state is encountered during unlock. The current server and local states are defined separately below so that recovery behavior is not confused with state observation. + +| Lifecycle state | Sync Server state check required before unlock? | User behaviors | UI requirements | +| ------------------------- | --------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| _No phase (`undefined`)_ | No. A server check may run as part of normal Seedless behavior, but it is not a recovery prerequisite. | Enter the current wallet password and continue normally. The user may start a new password change. | Show the normal locked or unlocked wallet UI. | +| `SEEDLESS_CHANGE_PENDING` | Yes. The remote request may not have started, may have failed before mutation, or may have committed with a lost response. | Do not assume which password is valid. If the server proves that the change did not commit, enter the old password. If it proves commitment, enter the new password. If the result remains ambiguous, `unknown`. Do not start another password change. | Show a password-change recovery screen. Do not report an entered password as an ordinary unlock failure while recovery is pending. Explain that the previous password change must be resolved first. | +| `SEEDLESS_COMMITTED` | Yes. Confirm the remote Seedless password and required backup/key-share changes. | Enter the new Seedless password. The user should not need the old Keyring password when the stored Keyring encryption key is recoverable from Seedless. | Keep wallet access behind a recovery screen. Explain that the remote change succeeded but local recovery still needs to finish. | +| `LOCAL_KEYRING_PENDING` | Yes. Confirm the remote new-password state before recovering the Keyring encryption key or synchronizing a local key. | Enter the new password. Allow recovery to determine cryptographically whether the local Keyring is old or new; do not ask the user to guess which state occurred. | Keep wallet access blocked until the local Keyring is reconciled and its current encryption key is synchronized. Show progress and retryable errors without clearing the recovery state. | +| `KEY_SYNC_PENDING` | Yes. The remote password is expected to be new, but the remote copy of the Keyring encryption key may be old, new, missing, or unknown. | Enter the new password, unlock the local Keyring, and allow the current Keyring encryption key to be exported and synchronized. Do not start another password change. | Show that the wallet password has changed but backup synchronization is incomplete. Do not expose the wallet as fully recovered until synchronization is verified. | +| `UNKNOWN` | Yes, whenever a server status check or cryptographic verification may resolve the state. If it cannot, remain `unknown`. | unknown | Keep the wallet locked and show a recovery-blocked state. Do not silently retry a non-idempotent operation or claim that either password is authoritative. | + +## Server and local state matrix + +| Lifecycle state | Server State | Local State | Recovered server state | Recovered local state | +| ------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| _No phase (`undefined`)_ | Stable and synchronized. The password is the current password; no change is pending. | Local Keyring, local Seedless state, and the persisted Keyring encryption key are stable and synchronized. | No change. The server remains in its current stable state. | No change. The local state remains in its current stable state. | +| `SEEDLESS_CHANGE_PENDING` | `unknown` until authoritative server status resolves whether the remote password change and backup updates are old, new, or partial. | Normally old/old, but local state may already have changed if lifecycle persistence was delayed or lost. Verify the local Keyring and local Seedless state independently. | Definitively old: clear the phase after durable cleanup. Definitively new: transition to `SEEDLESS_COMMITTED` or `LOCAL_KEYRING_PENDING`. Ambiguous: `unknown`. | Do not mutate until the server result is resolved. After remote commitment, cryptographically classify the local Keyring as old or new and follow the matching branch. | +| `SEEDLESS_COMMITTED` | New Seedless password and new remote backup/key-share state, confirmed through server verification. If this cannot be established, `unknown`. | Local Keyring may be old or new. Local Seedless state and the stored Keyring encryption key may be old, new, or not durably persisted. | Remains new and committed. | Transition to `LOCAL_KEYRING_PENDING`; cryptographically determine whether to recover the old local Keyring or synchronize the already-new local Keyring. | +| `LOCAL_KEYRING_PENDING` | Remote Seedless is new and committed. | Local Keyring state is unresolved: old with a recoverable stored encryption key, new with a locally exportable current key, or `unknown`. | Remains new and committed. | **Old Keyring:** recover the stored key with the new Seedless password, call `submitEncryptionKey`, re-encrypt locally, export the current key, and synchronize it. **New Keyring:** unlock with the new password, export the current key, and synchronize it. In both cases, verify and durably persist before clearing the lifecycle. | +| `KEY_SYNC_PENDING` | New Seedless password and remote backup/key-share state. The synchronized Keyring encryption key is not confirmed. | Local Keyring uses the new password. The Seedless copy of its Keyring encryption key is stale, missing, or not durably confirmed. | New password with the current Keyring encryption key synchronized and verified. | Local Keyring, local Seedless state, synchronized key, and lifecycle marker are durably persisted. Only then clear the lifecycle. | +| `UNKNOWN` | `unknown`. The server may have accepted some, all, or none of the remote password-change or key-synchronization operations. | `unknown`. Local Keyring, local Seedless state, or durable lifecycle state may reflect different points in the operation. | unknown | unknown | + +## Failure and recovery rules + +### Definitive remote failure before mutation + +**Trigger** + +Seedless returns a definitive error and server status confirms that the remote password change did not commit. + +**Wallet state** + +Remote Seedless, local Seedless, and local Keyring remain old and synchronized. + +**Recovery plan** + +Lock the wallet if required, ask the user to unlock with the old password, and durably clear the pending lifecycle state. A later retry starts from no phase set. + +### Remote error after a possible mutation + +**Trigger** + +Timeout, connection reset, lost response, client crash, or server error while remote Seedless operations were in flight. + +**Wallet state** + +unknown + +**Recovery plan** + +unknown + +The implementation must not classify this as “server failed” solely from the rejected Promise. + +### Local Keyring failure after remote commitment + +**Trigger** + +`KeyringController:changePassword` throws, or the process terminates while local Keyring persistence is pending. + +**Wallet state** + +Remote Seedless is new. The local Keyring may be old, new in memory, new and durably persisted, or unknown. + +**Recovery plan** + +Cryptographically verify the local Keyring: + +- Old local Keyring: recover the stored Keyring encryption key with the new Seedless password, call `submitEncryptionKey`, re-encrypt locally, then synchronize the current key. +- New local Keyring: export the current Keyring encryption key and synchronize it to Seedless. +- Indeterminate result: unknown. + +### Key synchronization failure or lost response + +**Trigger** + +The local Keyring is new, but the synchronization request fails or returns an ambiguous result. + +**Wallet state** + +Remote Seedless is new. The remote synchronized Keyring encryption key is old, new, missing, or unknown. + +**Recovery plan** + +Keep `KEY_SYNC_PENDING`. Unlock with the new password, export the current Keyring encryption key, retry using the same transaction identity, and verify the remote result. Do not clear the lifecycle until synchronization and local persistence are durable. If the remote result cannot be verified, unknown. + +### Lifecycle persistence failure + +**Trigger** + +The logical state transition succeeds, but the state-change notification is queued, debounced, lost, or fails before reaching durable storage. + +**Wallet state** + +The durable lifecycle marker may lag behind the actual cryptographic state. A marker such as `SEEDLESS_CHANGE_PENDING` does not prove that remote or local later steps did not happen. + +**Recovery plan** + +Use the marker only to trigger broader verification. Check remote state and cryptographically verify the local Keyring. Repeat the appropriate recovery branch idempotently. Do not treat an in-memory update or queued persistence operation as durable. + +### Process, browser, app, or device termination + +**Trigger** + +Termination occurs before or after any remote request, local controller update, synchronization request, or lifecycle write. + +**Wallet state** + +The state is determined by the last durable evidence, but may differ from the last in-memory state. + +**Recovery plan** + +At next startup/unlock, inspect the lifecycle marker and then verify actual state. Use the old-Keyring recovery branch if the new password does not unlock the local Keyring; use the new-Keyring synchronization branch if it does. If remote commitment cannot be established, unknown. + +## Idempotency requirements + +The recovery operations must be safe across retries and lost responses. + +- Assign one transaction identifier to the entire password change and reuse it during recovery. +- Make the remote password change and key synchronization idempotent or queryable by transaction identifier. +- Verify the local Keyring before changing its password again. +- Treat storing the already-current Keyring encryption key as a deterministic no-op or safe overwrite. +- Do not perform a compensating rollback based solely on an error response. +- Serialize password-change and recovery requests so two transactions cannot update the same vault concurrently. +- Persist lifecycle transitions with an awaitable durability boundary. + +## Consequences + +### Positive + +- A committed remote Seedless change has a deterministic recovery direction: bring the local Keyring forward to the new password. +- The old-Keyring and new-Keyring post-commit states are both recoverable through the new password when the stored Keyring encryption key is available. +- In-process rollback is not required for normal partial-failure handling. +- Recovery remains possible even if the lifecycle update was lost, because cryptographic verification is authoritative for the local Keyring state. + +### Negative and residual risks + +- This is still a distributed transaction. Server-first ordering does not make the operation atomic. +- Ambiguous remote outcomes remain `unknown` unless the server supports authoritative status lookup and idempotency. +- Recovery adds a special unlock path and additional user-facing states. +- The remote server must support safe retries; otherwise a lost response can still create an unrecoverable ambiguity. +- If the stored Keyring encryption key cannot be recovered, the old-Keyring branch cannot proceed and the state remains `unknown`. +- Locking the wallet protects the vault but does not itself restore consistency; the recovery path must be available before normal unlock failure handling. +- Mobile and extension must implement compatible lifecycle and recovery semantics to avoid platform-dependent outcomes. + +## Acceptance criteria + +Fault-injection tests must terminate or fail the operation at every boundary: + +- Before the remote request. +- During every remote Seedless round trip. +- After remote commitment but before the response. +- Before and after local Seedless persistence. +- Before, during, and after local Keyring password change. +- Before, during, and after Keyring encryption-key synchronization. +- Before and after each lifecycle persistence write. +- Immediately before clearing the lifecycle. + +After restart, each test must verify that: + +- The correct password is requested for the recovered state. +- The local Keyring can be unlocked with the new password after recovery. +- Seedless recovers the current Keyring encryption key. +- Retrying recovery produces the same final state. +- A lost response does not cause a second non-idempotent password change. +- The lifecycle is never cleared before key synchronization and local persistence. +- An unresolved server result remains `unknown`. + +## Open questions + +The following are intentionally left as `unknown` until addressed separately: + +- How TOPRF/Seedless exposes authoritative status after an ambiguous request. +- Whether password changes and key synchronization support idempotency keys. +- Recovery when the remote result is `unknown`. +- Recovery when the stored Keyring encryption key is unavailable or corrupt. +- The exact durable-persistence guarantee available on each platform. +- The final lifecycle enum names and migration strategy. +- Retry limits, rate-limit behavior, and user-facing copy for recovery-blocked states. diff --git a/packages/seedless-onboarding-controller/docs/0002-password-change-recovery-flow.md b/packages/seedless-onboarding-controller/docs/0002-password-change-recovery-flow.md new file mode 100644 index 00000000000..8d922017cb6 --- /dev/null +++ b/packages/seedless-onboarding-controller/docs/0002-password-change-recovery-flow.md @@ -0,0 +1,282 @@ +# Password-change recovery flow + +- Related ADR: [0001](./0001-seedless-password-change-recovery.md) +- Related Option B plan: [0003](./0003-controller-owned-password-change-recovery-plan.md) + +This is the operational and technical guide for the Seedless password-change recovery flow: what the controller owns, what the client owns, the public API, the recovery flow, the client integration guide, and the technical invariants. + +## Principles + +- **Server-first.** The remote Seedless password changes first. Recovery then brings local state forward to the new password. There is no rollback. +- **No retries, no concurrency.** A password change is never re-run as a fresh `changePassword` / `changeEncKey` call while the previous outcome is unresolved. The controller mutex serializes controller operations; the client adds a coordinator lock that also covers the `KeyringController` step. +- **Lifecycle is a signal, not proof.** `passwordChangePhase` only tells the client that recovery _may_ be needed. Recovery always re-verifies actual remote and local state before acting. +- **Lock before error.** Any password-change or recovery failure locks the wallet _before_ an error modal or intermediary screen is shown. +- **`UNKNOWN` is honest.** If remote or local state cannot be established, the wallet stays locked and the phase is preserved. Never infer a result from a rejected Promise. + +## The lifecycle phase + +Persisted on `SeedlessOnboardingControllerState.passwordChangePhase` (`persist: true`). Missing / `undefined` means no change is in progress. The field holds no secrets. + +| Phase | Meaning | +| ------------------------- | ----------------------------------------------------------------------------- | +| `SEEDLESS_CHANGE_PENDING` | A change started; the remote outcome is not yet confirmed. | +| `SEEDLESS_COMMITTED` | The remote Seedless password change is confirmed committed. | +| `LOCAL_KEYRING_PENDING` | The local Seedless vault has been rewritten with the new password. | +| `KEY_SYNC_PENDING` | The Keyring encryption key has been stored; awaiting final verification/sync. | +| `UNKNOWN` | The result of one or more steps could not be established. | + +## The sync status + +Returned by `resolvePasswordSyncState` and `reconcilePassword` as `PasswordSyncStatus`. The client routes UI from this status. + +| Status | Meaning | Client action | +| -------------------- | --------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------- | +| `in-sync` | Local and remote passwords are synchronized; no recovery action is needed. | Unlock normally. | +| `password-outdated` | No lifecycle is in flight but the remote password changed (another device). | Prompt for the new password, then `reconcilePassword`. | +| `enter-new-password` | Remote committed (or the local Seedless side still needs the new password). | Prompt for the new password, then `reconcilePassword`. | +| `reconcile-keyring` | Seedless side reconciled (phase is `LOCAL_KEYRING_PENDING`). | Cryptographically classify the local Keyring, then run the old/new branch. | +| `sync-key` | Phase is `KEY_SYNC_PENDING`. | Export, store, and sync the current Keyring encryption key, then `clearPasswordChangePhase`. | +| `unknown` | Remote or local state could not be established. | Keep the wallet locked. Preserve the phase. Offer reset wallet only as an explicit last resort. | + +## Controller public API + +The controller owns the Seedless-side sequencing. The client owns the Keyring-side steps and UI routing. + +For password-change recovery and another-device password sync, clients call `resolvePasswordSyncState` followed by `reconcilePassword`, then use the lifecycle advances and Keyring-key methods described below. The old public `submitGlobalPassword` and `syncLatestGlobalPassword` methods have been removed: their sequencing is now internal to `reconcilePassword`, which also re-wraps `encryptedKeyringEncryptionKey` and advances to `LOCAL_KEYRING_PENDING`. + +### Read / resolve (no password) + +```ts +SeedlessOnboardingController:resolvePasswordSyncState({ + skipCache?: boolean, +}): Promise +``` + +Single unlock-time call (call on page render _and_ on password submit). Merges the legacy `checkIsPasswordOutdated` read with password-change recovery routing. + +- No phase (`undefined`): authoritative outdated check; `skipCache` honored (cache on render, force-remote on submit). Returns `in-sync` or `password-outdated`. +- `SEEDLESS_CHANGE_PENDING`: forces a remote check (ignores `skipCache`). Clears the phase (`in-sync`) or advances to `SEEDLESS_COMMITTED` (`enter-new-password`). +- Other phases: returns the matching status without a remote call. +- On any failure: returns `unknown` and preserves the phase. + +### Apply (with password) + +```ts +SeedlessOnboardingController:reconcilePassword({ + globalPassword: string, +}): Promise +``` + +Reconciles the Seedless side with the supplied password. + +- `SEEDLESS_COMMITTED` / `LOCAL_KEYRING_PENDING`: re-runs the internal chain-unlock and local-vault rewrite (idempotent), advances to `LOCAL_KEYRING_PENDING`, returns `reconcile-keyring`. +- No phase (`undefined`): re-checks the remote password and, if outdated, runs the same password-sync flow, advances to `LOCAL_KEYRING_PENDING`, and returns `reconcile-keyring` so the client reconciles the local Keyring after a password change on another device. If not outdated, a no-op that returns `in-sync`. +- `SEEDLESS_CHANGE_PENDING`: returns `unknown` (resolve remote state via `resolvePasswordSyncState` first). +- On any failure: returns `unknown` and preserves the phase. + +### Lifecycle advance (client-driven) + +```ts +SeedlessOnboardingController:markPasswordChangeKeySyncPending(): Promise +SeedlessOnboardingController:clearPasswordChangePhase(): Promise +``` + +All idempotent, serialized under the controller lock, with no-op guards. + +- `markPasswordChangeKeySyncPending` — advance to `KEY_SYNC_PENDING` after the Keyring encryption key is stored. +- `clearPasswordChangePhase` — clear the phase once Keyring encryption-key synchronization is verified and all required local state is durably persisted (or after a definitive remote non-commit). This is the only way back to no change in progress. + +## Recovery flow + +``` +unlock render / submit + │ + ▼ + resolvePasswordSyncState({ skipCache }) + │ + ▼ + ┌────────────────────┬───────────────────┬──────────────────┬─────────────────┬───────────┬─────────┐ + │ in-sync │ password-outdated │ enter-new-password │ reconcile-keyring │ sync-key │ unknown │ + │ unlock normally │ prompt new pwd │ prompt new pwd │ classify Keyring │ finish sync│ locked │ + └───────────────────┴───────────────────┴───────────────────┴─────────────────┴───────────┴─────────┘ + │ │ │ │ │ + │ ▼ ▼ ▼ ▼ + │ reconcilePassword reconcilePassword old/new branch clearPasswordChangePhase + │ ({ globalPassword }) ({ globalPassword }) (see below) (after sync verified) + ▼ + normal unlock │ │ + ▼ ▼ + Seedless reconciled Seedless reconciled + → reconcile-keyring → reconcile-keyring + │ │ + ▼ ▼ + classify local Keyring via KeyringController:verifyPassword(newPassword) +``` + +### Old-Keyring branch (local Keyring still on the old password) + +1. `loadKeyringEncryptionKey()` — recover the stored Keyring encryption key with the new Seedless password. +2. `KeyringController:submitEncryptionKey` — unlock the old local Keyring. +3. `KeyringController:changePassword(newPassword)` — re-encrypt the local Keyring. +4. `KeyringController:exportEncryptionKey` — export the current Keyring encryption key. +5. `storeKeyringEncryptionKey()` — store the current key locally (encrypted with the new Seedless password). +6. `markPasswordChangeKeySyncPending()` — advance to `KEY_SYNC_PENDING`. +7. Sync the Keyring encryption key to the remote Seedless backup. +8. `clearPasswordChangePhase()` — finish (only after sync is verified and local state is durably persisted). + +### New-Keyring branch (local Keyring already on the new password) + +1. `KeyringController:verifyPassword(newPassword)` — unlock/verify the local Keyring with the new password. +2. `KeyringController:exportEncryptionKey` — export the current Keyring encryption key. +3. `storeKeyringEncryptionKey()` — store the current key locally. +4. `markPasswordChangeKeySyncPending()` — advance to `KEY_SYNC_PENDING`. +5. Sync the Keyring encryption key to the remote Seedless backup. +6. `clearPasswordChangePhase()` — finish. + +### `KEY_SYNC_PENDING` (resuming after a restart) + +1. Unlock with the new password. +2. `KeyringController:exportEncryptionKey` — export the current Keyring encryption key. +3. `storeKeyringEncryptionKey()` — re-store/sync the current key. +4. Sync to the remote Seedless backup and verify. +5. `clearPasswordChangePhase()`. + +## Client integration guide + +1. **Coordinator lock.** Add a single lock covering the whole Seedless + Keyring transaction. The controller mutex already serializes controller operations; this lock extends serialization across the `KeyringController` step. Reject a second password change while the lifecycle is unfinished. + +2. **Persist `SEEDLESS_CHANGE_PENDING` before the first remote mutation.** The controller writes this itself inside `changePassword`; the client must ensure the controller state slice is persisted (debounced, same as other persisted controller state) before any irreversible step. + +3. **Unlock routing.** On unlock (page render _and_ password submit), read `passwordChangePhase` from controller state, then call `resolvePasswordSyncState({ skipCache })`. Route UI from the returned status using the table above. Do not classify a password as invalid until recovery has run. + +4. **Two-step UX.** + - Step 1 (password-less): `resolvePasswordSyncState` decides whether the old or new password is needed. + - Step 2 (password-consuming): only after step 1 returns `enter-new-password` / `password-outdated`, prompt for the new password and call `reconcilePassword({ globalPassword })`. + +5. **Keyring classification.** On `reconcile-keyring`, call `KeyringController:verifyPassword(newPassword)` to choose the old-Keyring or new-Keyring branch. Do **not** infer the local Keyring state from the lifecycle phase. + +6. **Lock before error.** Any failure from `changePassword`, `resolvePasswordSyncState`, `reconcilePassword`, or any Keyring step must lock the wallet _before_ surfacing an error modal, retry screen, or intermediary UI. If the lock itself fails, keep the wallet in a recovery-blocked UI and never expose wallet access. + +7. **Completion boundary.** Call `clearPasswordChangePhase()` only after the synchronized Keyring encryption key and all required local state are durably persisted. This clears the lifecycle so the next unlock is normal. + +8. **`UNKNOWN` is terminal for this attempt.** If `resolvePasswordSyncState` or `reconcilePassword` returns `unknown`, keep the wallet locked, preserve the phase, and stop. Do not retry `changePassword` / `changeEncKey`. Offer reset wallet only as an explicit last resort for a confirmed unrecoverable state. + +9. **Cache.** `resolvePasswordSyncState` honors `skipCache` for the no-phase outdated check only. Use `skipCache: false` (default) on render and `skipCache: true` on submit. `SEEDLESS_CHANGE_PENDING` always forces a remote check. + +### Password out of sync (another device changed the remote password) + +No local `changePassword` is in flight (`passwordChangePhase` is unset). The remote Seedless password is newer than this device. + +1. Lock the wallet before any error or “wrong password” UI. +2. On unlock render / submit, call `resolvePasswordSyncState({ skipCache })`. Expect `password-outdated` (or `in-sync` if this device is current). +3. Prompt for the **new** global password. Do not ask for the old Keyring password. +4. Call `reconcilePassword({ globalPassword })`. That rewrites the local Seedless vault, re-wraps `encryptedKeyringEncryptionKey`, sets `LOCAL_KEYRING_PENDING`, and returns `reconcile-keyring`. +5. Classify the local Keyring with `KeyringController:verifyPassword(newPassword)` and run the [old-Keyring](#old-keyring-branch-local-keyring-still-on-the-old-password) or [new-Keyring](#new-keyring-branch-local-keyring-already-on-the-new-password) branch. +6. Finish with `clearPasswordChangePhase()` only after key sync and local persistence. + +The chain-unlock and vault-rewrite steps are internal; clients do not call separate password-sync primitives. + +### Password-change error (this device’s `changePassword` failed) + +A local change started and left a phase. Do **not** retry `changePassword` / `changeEncKey`. + +1. Lock the wallet **before** showing the error, retry screen, or any intermediary UI. +2. On the next unlock, call `resolvePasswordSyncState({ skipCache })` and route on the status: + + | Status | What happened | Client action | + | -------------------- | ----------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------- | + | `in-sync` | Remote did not commit (`SEEDLESS_CHANGE_PENDING` resolved to old). Phase cleared. | Unlock with the **old** password. | + | `enter-new-password` | Remote committed (`SEEDLESS_CHANGE_PENDING` → `SEEDLESS_COMMITTED`, or already `SEEDLESS_COMMITTED`). | Prompt for the **new** password, then `reconcilePassword`. | + | `reconcile-keyring` | Local Seedless vault already rewritten (`LOCAL_KEYRING_PENDING`). | Classify Keyring; run the old/new branch. If Seedless is locked, call `reconcilePassword` first (idempotent). | + | `sync-key` | Keyring is on the new password; backup key sync is unfinished. | Unlock with the new password; export / store / remote-sync; `clearPasswordChangePhase`. | + | `unknown` | Remote or local state could not be established. | Keep locked. Preserve the phase. Do not retry `changePassword`. | + +3. After `reconcilePassword` returns `reconcile-keyring`, run the same Keyring branches as another-device sync. `loadKeyringEncryptionKey` is valid because reconciliation re-wraps the stored key under the new Seedless wrapping key. + +## Technical details + +### Lifecycle model and persistence + +The lifecycle is one optional persisted field: + +```ts +passwordChangePhase?: SeedlessPasswordChangePhase; +``` + +It is persisted as ordinary controller state (`persist: true`) through the normal debounced `stateChange` flow — the same path as every other persisted field. There is **no** separate awaitable durability hook on the controller. The lifecycle is a recovery signal only: it is not proof that a remote or local operation completed, and it is not proof that the lifecycle itself reached durable storage before the next step ran. A crash can leave the durable marker behind the actual cryptographic state, so recovery always re-verifies actual remote and local state before acting on the phase. A missing or stale marker is recoverable: `#checkIsPasswordOutdated({ skipCache: true })` detects a remote change with no marker at all, and cryptographic Keyring verification classifies the local state. + +### Lifecycle write points + +Controller `this.update(...)` calls happen: + +- before the first remote mutation (`SEEDLESS_CHANGE_PENDING`); +- after authoritative remote commitment (`SEEDLESS_COMMITTED`); +- after the local Seedless vault rewrite (`LOCAL_KEYRING_PENDING`); +- the password-change commit writes the vault, `authPubKey`, encrypted + Keyring key, and lifecycle phase together; +- after an explicit clear (no change in progress). + +These publish `SeedlessOnboardingController:stateChange`; they are not awaited durability boundaries. + +### Phase preservation on failure + +On a failed step the controller **preserves the last known phase**; it does not overwrite it with `UNKNOWN` and does not clear the phase on every error. The last written phase is the recovery signal: e.g. if `#changeEncryptionKey` rejects, the phase stays `SEEDLESS_CHANGE_PENDING` and the client performs an authoritative password-outdated check to choose the recovery branch. If the failure happened before the first lifecycle write, the phase stays unset (nothing to recover). `UNKNOWN` is a recovery-time determination made by the client when an authoritative server check or local cryptographic verification cannot establish the state — not a phase written by `changePassword`'s catch block. + +### No retries, no concurrency + +A password-change operation must never be retried as a fresh `changePassword` / `changeEncKey` call while the previous outcome is unresolved. Race conditions here are dangerous and could block users from their wallets. The existing controller mutex (`#withControllerLock` / `#controllerOperationMutex`) serializes all mutable controller operations; the client coordinator adds a single lock that also covers the `KeyringController` operation. The lifecycle exists to signal that recovery is needed — not to enable retries. `#assertPasswordInSync` also guards against TOPRF operations while a change is pending (with a `skipPhaseCheck` bypass for `changePassword`'s own token-refresh retry). + +### `changePassword` behavior + +`changePassword` is now lifecycle-aware: it writes `SEEDLESS_CHANGE_PENDING` before the first remote mutation, `SEEDLESS_COMMITTED` after authoritative remote commitment, and `LOCAL_KEYRING_PENDING` after the local Seedless vault rewrite. It rejects a second concurrent change with `PasswordChangeInProgress`. It reuses the existing `verifyVaultPassword`, `#assertPasswordInSync({ skipCache: true })`, `#changeEncryptionKey` (via `#executeWithTokenRefresh`), and `#commitPasswordChangeState`. The final commit writes the rewritten vault, `authPubKey`, encrypted Keyring key, and lifecycle phase in one state update. A rejected `#changeEncryptionKey` Promise is not proof that the server did not mutate; only a definitive server result may clear the lifecycle. + +### `storeKeyringEncryptionKey` behavior + +`storeKeyringEncryptionKey` encrypts the current Keyring encryption key under the current Seedless password encryption key and writes `encryptedKeyringEncryptionKey` on controller state. It never clears the lifecycle by itself — completion also requires client confirmation of the local Keyring state, remote synchronization, and durable persistence. `loadKeyringEncryptionKey` is read-only with respect to lifecycle state; loading a key does not complete recovery. + +### Recovery mechanism + +`reconcilePassword` runs the password-sync flow internally: + +- `#submitGlobalPassword` — `toprfClient.recoverPwEncKey` walks the server-side password-key history chain (`maxPwChainLength`) to find the `pwEncKey` matching this device's `authPubKey`, then unlocks the vault. +- `#syncLatestGlobalPasswordInner` — `toprfClient.recoverEncKey` derives encryption material from the candidate password, rewrites the local Seedless vault with the new password's keys, and re-encrypts `encryptedKeyringEncryptionKey` under the new `toprfPwEncryptionKey` so `loadKeyringEncryptionKey` still works. + +Both run through `#executeWithTokenRefresh`. `storeKeyringEncryptionKey` of the already-current key is a local overwrite and is safe to re-run. + +### Remote-state classification + +After a lost, timed-out, or incomplete `changeEncKey` response, recovery classifies remote state via `toprfClient.fetchAuthPubKey` (with `skipCache: true`), comparing the remote `authPubKey` with the last durable local `authPubKey`: + +| Observation | Classification | Lifecycle effect | +| ------------------------------------------------------------------------------------------------------------------------- | -------------- | ------------------------------------------------------------------------ | +| Fetch succeeds and remote `authPubKey` equals the pre-change local `authPubKey`. | **Old** | Safe to treat as uncommitted. Clear the phase only after this check. | +| Fetch succeeds and remote `authPubKey` equals the expected post-change key, or the new password recovers remote material. | **New** | Treat as committed. Advance to `SEEDLESS_COMMITTED` or later recovery. | +| Fetch fails, comparison is impossible, or local `authPubKey` is missing/stale. | **Unknown** | Preserve the phase. Keep the wallet locked. Do not retry `changeEncKey`. | + +There is no API that reports partial backup or key-share state; such cases are classified as **Unknown**. The TOPRF server does not accept a transaction ID / idempotency key today; that remains a "good to have" for a future release. Until then, ambiguous remote results stay `UNKNOWN`. + +## What the controller does not do + +- It does not call `KeyringController` (`AllowedActions = never`). Keyring classification, re-encryption, and key export are client responsibilities. +- It does not provide an awaitable durability boundary for lifecycle writes. The lifecycle is persisted as ordinary debounced controller state. Recovery re-verifies actual state, so a stale/missing marker is recoverable. +- It does not retry `changePassword` / `changeEncKey`. Password reconciliation performs the chain unlock and local vault rewrite internally. +- It does not lock the wallet. Locking is a client responsibility (the client owns navigation/intermediary screens). + +## Controller-side status + +All controller-package work is complete: + +- Lifecycle model, helpers, metadata, exports. +- Lifecycle-aware `changePassword` with concurrency guard and phase preservation on error. +- Lifecycle-neutral `storeKeyringEncryptionKey`; password-change lifecycle + phases are committed by the password-change flow and explicit lifecycle + methods. +- `resolvePasswordSyncState` + `reconcilePassword` (Option A: controller owns the Seedless side). +- `markPasswordChangeKeySyncPending` / `clearPasswordChangePhase`. +- Messenger action types, package exports, and unit tests (290 tests, 100% statement / 99.22% branch coverage). + +Remaining work is **not** in this package: + +- **Client integration** — coordinator, unlock routing, locking, UI, and E2E coverage (see the [Client integration guide](#client-integration-guide) above). +- **Open decisions** — rate-limit behavior for recovery; whether clients need an explicit migration version bump for persisted state created before `passwordChangePhase` existed. +- **Option B** — future migration where the controller also owns the `KeyringController` side (see [0003](./0003-controller-owned-password-change-recovery-plan.md)). diff --git a/packages/seedless-onboarding-controller/docs/0003-controller-owned-password-change-recovery-plan.md b/packages/seedless-onboarding-controller/docs/0003-controller-owned-password-change-recovery-plan.md new file mode 100644 index 00000000000..ccd65da1780 --- /dev/null +++ b/packages/seedless-onboarding-controller/docs/0003-controller-owned-password-change-recovery-plan.md @@ -0,0 +1,135 @@ +# Plan 0003: Migrate password-change recovery into the controller (Option B) + +- Status: Planned (post-testing migration) +- Related: [ADR 0001](./0001-seedless-password-change-recovery.md), [Recovery flow 0002](./0002-password-change-recovery-flow.md) +- Scope: `SeedlessOnboardingController` only + +## Context + +The first implementation (see [0002](./0002-password-change-recovery-flow.md)) ships **Option A**: the controller owns all _Seedless-side_ recovery sequencing, but the _Keyring-side_ steps (`verifyPassword`, `submitEncryptionKey`, `changePassword`, `exportEncryptionKey`) stay in the client because `SeedlessOnboardingController` has no `KeyringController` dependency (`AllowedActions = never`). + +This document plans the migration to **Option B**: the controller owns the entire recovery, including the Keyring side. Motivation: the recovery transaction spans two controllers, and we cannot rely on every client sequencing the Keyring-side steps correctly. Centralizing the full transaction removes a class of client-integration bugs. + +This migration is deferred until Option A is shipped and tested, so the recovery contract is exercised end-to-end before the coupling is introduced. + +The `encryptedKeyringEncryptionKey` state field remains optional during the +compatibility period. Users created before that field was introduced may have +valid Seedless state without it, and the controller cannot reconstruct the +missing value from persisted data. The migration therefore includes an +unlock-time backfill before the field becomes a precondition for password +changes and password reconciliation. + +## Goal + +A single controller method performs the entire recovery for any set phase and returns only a final status. The client no longer sequences Seedless or Keyring operations; it only supplies the password and reacts to the status. + +## Current state (Option A) + +- `reconcilePassword({ globalPassword })` does the Seedless-side steps (`#checkIsPasswordOutdated({ skipCache: true })`, chain unlock, local vault rewrite, lifecycle advances) and returns a result describing the remaining Keyring-side step. Remote-state resolution for `SeedlessChangePending` is owned by `resolvePasswordSyncState()` (password-less), which the client calls first. +- The client classifies the local Keyring via `KeyringController:verifyPassword`, then runs the old-Keyring or new-Keyring branch itself, calling `KeyringController:submitEncryptionKey` / `changePassword` / `exportEncryptionKey` and the controller's `loadKeyringEncryptionKey` / `storeKeyringEncryptionKey` / `markPasswordChangeKeySyncPending` / `clearPasswordChangePhase`. +- `AllowedActions = never`; the controller does not call `KeyringController`. +- `encryptedKeyringEncryptionKey` may be missing for legacy users or users whose initial Keyring-key synchronization did not complete. Missing state is a migration signal, not proof that the user has no Keyring encryption key. +- During unlock, the client must backfill a missing value by calling `KeyringController:exportEncryptionKey` and then `SeedlessOnboardingController:storeKeyringEncryptionKey` before exposing the normal unlocked wallet flow. The backfill is lifecycle-neutral. + +## Target state (Option B) + +- `AllowedActions` includes `KeyringController:verifyPassword`, `KeyringController:submitEncryptionKey`, `KeyringController:changePassword`, `KeyringController:exportEncryptionKey` (and `KeyringController:setLocked` if locking is folded in). +- `reconcilePassword({ globalPassword })` performs the full transaction: + 1. During unlock, backfill a missing `encryptedKeyringEncryptionKey` from `KeyringController:exportEncryptionKey` before allowing password-change recovery. + 2. Resolve remote state for `SeedlessChangePending` via `resolvePasswordSyncState()` (which runs `#checkIsPasswordOutdated({ skipCache: true })`). + 3. Reconcile the Seedless side (internal chain unlock + local vault rewrite) for `SeedlessCommitted` / `LocalKeyringPending`. + 4. Classify the local Keyring via `KeyringController:verifyPassword(newPassword)`. + 5. Old-Keyring branch: `loadKeyringEncryptionKey` → `KeyringController:submitEncryptionKey` → `KeyringController:changePassword` → `KeyringController:exportEncryptionKey` → `storeKeyringEncryptionKey` → `markPasswordChangeKeySyncPending`. + 6. New-Keyring branch: `KeyringController:exportEncryptionKey` → `storeKeyringEncryptionKey` → `markPasswordChangeKeySyncPending`. + 7. `KeySyncPending`: `KeyringController:exportEncryptionKey` → `storeKeyringEncryptionKey` → remote key sync → `clearPasswordChangePhase`. + 8. Return a final status only (`PasswordSyncStatus.InSync | Unknown`). +- The client supplies the password, calls one method, and routes UI from the status. It performs no cross-controller sequencing. + +## Changes + +### 0. Keyring encryption-key backfill and precondition + +- Keep `encryptedKeyringEncryptionKey` optional for backward-compatible state + loading and migration. +- Treat a missing value as an incomplete local synchronization state. It must + not be interpreted as evidence that the local Keyring encryption key does + not exist. +- During unlock, after the Keyring is available, call + `KeyringController:exportEncryptionKey` when the Seedless controller has no + `encryptedKeyringEncryptionKey`, then persist the result through + `SeedlessOnboardingController:storeKeyringEncryptionKey`. +- Perform this backfill for both legacy users and any new account or + rehydration flow that reaches unlock without a stored value. +- Keep the backfill idempotent and lifecycle-neutral. It must not clear or + advance `passwordChangePhase`. +- If export or persistence fails, keep the wallet locked and do not begin + `changePassword` or `reconcilePassword`. +- After the migration is complete, require the value as a precondition for + `changePassword` and `reconcilePassword`. A missing value must fail closed + into the recovery/unknown path rather than allowing a password mutation to + proceed without a recoverable Keyring encryption key. + +### 1. Messenger dependency + +- Add the `KeyringController` action types to `AllowedActions` in `SeedlessOnboardingController.ts`. +- Confirm there is no cycle: `KeyringController` must not depend on `SeedlessOnboardingController`. (Expected to hold; `KeyringController` is lower-level.) +- Update `SeedlessOnboardingControllerMessenger` and any package-level messenger assembly/permission wiring so the controller is granted the `KeyringController:*` actions it calls. +- Update the mock messenger (`tests/__fixtures__/mockMessenger.ts`) so `KeyringController` actions are callable in tests. + +### 2. Controller method + +- Fold the Keyring-side steps into `reconcilePassword`. Keep the existing private helpers; replace the "return a plan" shape with a final-status shape. +- Preserve all existing invariants: + - No retries of `changePassword` / `changeEncKey`; reconcile only via the password-sync flow. + - Preserve the last known phase on failure; do not write `UNKNOWN` from the happy path. + - Require `encryptedKeyringEncryptionKey` before starting a password-change or reconciliation transaction after the backfill migration. + - Keep `storeKeyringEncryptionKey` lifecycle-neutral as a public method. Its private persistence helper only updates the encrypted key; lifecycle phases are committed separately with the password-change state. + - Serialize under `#withControllerLock`. The cross-controller Keyring operations happen while the controller lock is held; document that the client coordinator lock (Phase 7) must not deadlock with it. + +### 3. Contracts and exports + +- Update [0002](./0002-password-change-recovery-flow.md): the client contract shrinks to "call `reconcilePassword`, route on status". The Keyring-side client steps move to the controller. +- Update the controller-side status and remaining-work notes in [0002](./0002-password-change-recovery-flow.md). +- Re-export the new result/status types from `src/index.ts`. +- Regenerate `SeedlessOnboardingController-method-action-types.ts` (the method signature change is picked up automatically). + +### 4. Clients + +- Remove client-side Keyring-side recovery sequencing (the old-Keyring / new-Keyring branches). +- Keep: the single coordinator lock, wallet locking on error, unlock routing to call `reconcilePassword`, and UI per status. +- Update client tests to assert against the new status-only result. + +## Trade-offs and risks + +- **Coupling:** `SeedlessOnboardingController` gains a hard dependency on `KeyringController`. This is precedented in the monorepo but breaks this controller's current self-contained design. Reuse in contexts without `KeyringController` becomes harder. +- **Lock ordering:** the controller lock now spans `KeyringController` calls. The client coordinator lock must order consistently with it to avoid deadlock. Document the ordering; prefer the controller acquiring its lock first and the coordinator lock wrapping the whole call. +- **Test surface:** controller tests must mock `KeyringController` actions; fault-injection moves from client E2E into controller unit tests. +- **Rollback:** if Option B proves problematic, Option A remains the fallback. Keep the Option A return shape recoverable by reverting the messenger dependency and method body. + +## Test plan + +- Legacy unlock with no `encryptedKeyringEncryptionKey` calls + `KeyringController:exportEncryptionKey`, stores the result, and does not + advance the password-change lifecycle. +- New-account creation and rehydration persist the exported Keyring + encryption key before normal unlock completes. +- Export or persistence failure keeps the wallet locked and prevents + `changePassword` and `reconcilePassword` from starting. +- After backfill, `changePassword` and `reconcilePassword` reject or route to + `Unknown` when the required encrypted Keyring key is missing. +- Controller unit tests for every phase, each branch (old/new Keyring), and each failure injection point (remote check error, chain-unlock error, `verifyPassword` error, `submitEncryptionKey` error, `changePassword` error, `exportEncryptionKey` error, `storeKeyringEncryptionKey` error, remote key-sync error). +- Assert the final status and the resulting `passwordChangePhase` for each. +- Assert no `changePassword`/`changeEncKey` retry occurs on any recovery path. +- Assert the controller lock is released on every failure path. +- Client integration tests reduced to: one call per phase, status routing, locking, and UI. + +## Migration order + +1. Land Option A and ship it; gather client integration feedback. +2. Add unlock-time backfill for missing `encryptedKeyringEncryptionKey` using `KeyringController:exportEncryptionKey` and `storeKeyringEncryptionKey`. +3. Ensure new-account creation and rehydration persist the exported Keyring encryption key before normal unlock. +4. Add the `KeyringController` messenger dependency and mock wiring (behind no behavior change yet). +5. Enforce the stored encrypted Keyring key as a precondition for `changePassword` and `reconcilePassword`. +6. Fold the Keyring-side steps into `reconcilePassword`; change the return shape to final status. +7. Update the recovery flow guide (0002), exports, and clients. +8. Run the full controller + client test suites; remove the now-dead client sequencing code. diff --git a/packages/seedless-onboarding-controller/docs/0004-controller-state-lock-audit.md b/packages/seedless-onboarding-controller/docs/0004-controller-state-lock-audit.md new file mode 100644 index 00000000000..d65f42f0b65 --- /dev/null +++ b/packages/seedless-onboarding-controller/docs/0004-controller-state-lock-audit.md @@ -0,0 +1,358 @@ +# Audit 0004: Controller state-lock and password-sync races + +- Status: Finding F-001 fixed; follow-up findings remain open +- Date: 2026-09-10 +- Scope: `SeedlessOnboardingController` +- Related: [ADR 0001](./0001-seedless-password-change-recovery.md), [Recovery flow 0002](./0002-password-change-recovery-flow.md), [Option B plan 0003](./0003-controller-owned-password-change-recovery-plan.md) + +## Executive summary + +The controller has an in-memory controller mutex, but the mutex only protects +the callbacks that acquire it. It does not protect state snapshots taken before +lock acquisition, public methods that do not use the mutex, or operations that +mutate controller state through another lock. + +One high-impact race was verified between `resolvePasswordSyncState` and +`changePassword`. `resolvePasswordSyncState` selected its recovery branch from +a phase read taken before it acquired the controller lock. A concurrent +`changePassword` could finish the Seedless-side rewrite and advance the phase to +`LOCAL_KEYRING_PENDING`; the resolver could then use its stale +`SEEDLESS_CHANGE_PENDING` branch, clear the newer phase, and return `in-sync` +while the local Keyring still required reconciliation. + +That finding is fixed in the current implementation. A deterministic regression +test reproduces the interleaving and verifies that the resolver returns +`reconcile-keyring` and preserves `LOCAL_KEYRING_PENDING`. + +The broader audit remains open. The recommended model is operation-level +serialization around lifecycle-sensitive operations, not a mutex around each +individual `BaseController.update` call. + +## Audit question + +Can password synchronization, password changes, and other public controller +operations concurrently observe and mutate related state such that the +controller returns a status that no longer describes the state it acted on? + +## Relevant implementation model + +### Controller and vault locks + +`SeedlessOnboardingController` defines separate controller and vault mutexes: + +- [`#controllerOperationMutex` and `#vaultOperationMutex`](../src/SeedlessOnboardingController.ts#L425-L427) +- [`#withControllerLock`](../src/SeedlessOnboardingController.ts#L2345-L2349) +- [`#withVaultLock`](../src/SeedlessOnboardingController.ts#L2362-L2366) + +The controller mutex serializes callbacks that use `#withControllerLock`. It is +not re-entrant, so internal helpers use explicit `skipLock` options when their +caller already owns the lock. + +The vault mutex protects encryption, decryption, and vault writes. It does not +by itself serialize lifecycle state, authentication state, or all controller +state updates. + +### State update behavior + +`BaseController.update` calculates and installs the next state synchronously, +then publishes state-change events: + +- [`BaseController.update`](../../base-controller/src/BaseController.ts#L323-L355) + +This prevents a single update from being torn apart. It does not prevent an +asynchronous operation from reading state, awaiting I/O, and later applying a +decision based on that stale read. Therefore, locking individual updates would +not fix the verified race. + +## Finding F-001: stale lifecycle branch selection + +- Severity: High +- Status: Fixed +- Affected operation: `resolvePasswordSyncState` +- Concurrent operation: `changePassword` +- Failure class: time-of-check/time-of-use race + +### Pre-fix behavior + +The resolver read `passwordChangePhase`, selected a `switch` branch, and only +then acquired the controller lock for the mutating pending-phase branch. The +branch selection was therefore outside the critical section. + +`changePassword` holds the controller lock while it performs the remote change +and local Seedless rewrite: + +- [`changePassword`](../src/SeedlessOnboardingController.ts#L977-L1076) +- [`#commitPasswordChangeState`](../src/SeedlessOnboardingController.ts#L2203-L2258) + +### Reproduction sequence + +1. The controller is unlocked and `changePassword` starts. +2. `changePassword` records `SEEDLESS_CHANGE_PENDING` and waits for the remote + password-change operation. +3. `resolvePasswordSyncState` reads `SEEDLESS_CHANGE_PENDING` before waiting for + the controller lock. +4. `changePassword` completes the remote operation and commits the local + Seedless vault with `LOCAL_KEYRING_PENDING`. +5. `changePassword` releases the controller lock. +6. The resolver acquires the lock but continues through its previously selected + `SEEDLESS_CHANGE_PENDING` branch. +7. The remote check now observes the new Seedless state. The resolver can clear + the phase and return `in-sync`. +8. The client skips the required local Keyring reconciliation because the + lifecycle marker and returned status no longer describe the actual state. + +The same stale-decision pattern can occur when the resolver initially observes +an unset phase while a concurrent password change later starts and completes. + +### Remediation applied + +`resolvePasswordSyncState` now: + +1. Acquires the controller lock before reading `passwordChangePhase`. +2. Holds that lock through the remote check and any lifecycle transition. +3. Calls the password-outdated helper with `skipLock: true` because the + resolver already owns the controller lock. +4. Routes a phase that changed while the resolver was waiting using the latest + state. + +- [Fixed `resolvePasswordSyncState`](../src/SeedlessOnboardingController.ts#L2458-L2517) +- [Regression test](../src/SeedlessOnboardingController.test.ts#L4918-L5004) + +### Required invariant + +The phase snapshot, remote-state check, returned status, and lifecycle +transition must be one controller-serialized operation. A resolver must never +clear or advance a phase based on a snapshot taken before it acquired the +controller lock. + +## Locking decision + +### Use operation-level locking + +Lifecycle-sensitive operations should acquire the controller lock before their +first state read and hold it across all awaits that affect the decision. This +includes: + +- The password-change lifecycle phase. +- The local Seedless vault and authentication key transition. +- The encrypted Keyring encryption key. +- Password-outdated cache updates. +- Authentication and refresh-token state used by the operation. + +Internal helpers should not independently acquire the same non-reentrant mutex. +They should be private unlocked helpers, called only from a locked public +operation or from another helper with an explicit ownership contract. + +### Do not mutex individual state updates + +Adding a mutex around `BaseController.update` would not solve stale snapshots +and would make synchronous state updates difficult to compose with existing +async APIs. It could also create deadlocks when an operation already holds the +controller lock and calls a helper that tries to update state. + +The lock should protect the logical transaction, not only the final assignment. + +### Preserve lock ordering + +Where both locks are needed, use this ordering: + +1. Controller lock. +2. Vault lock. +3. State update while both relevant operation invariants are held. + +No path should acquire the vault lock and then wait for the controller lock. +Public wrappers and private helpers should make this ordering explicit. + +The controller lock also cannot serialize operations performed directly by the +`KeyringController`. A client coordinator, or a future controller-owned +implementation, must cover the cross-controller transaction. + +## Follow-up findings + +These findings were identified during the audit and are not fixed by F-001. + +### F-002: Keyring encryption-key methods bypass the controller lock + +- Severity: High +- Status: Open +- Affected methods: + - [`storeKeyringEncryptionKey`](../src/SeedlessOnboardingController.ts#L1514-L1524) + - [`loadKeyringEncryptionKey`](../src/SeedlessOnboardingController.ts#L1532-L1535) + +`storeKeyringEncryptionKey` awaits vault access and then updates controller +state without the controller mutex. It can therefore write an encrypted key +after a password-change commit, lifecycle clear, or another recovery step has +changed the wrapping key or phase. + +`loadKeyringEncryptionKey` is a read, but it is used to make recovery decisions. +It can observe a different combination of vault and encrypted-key state from +the one that existed when its operation started. + +Recommended remediation: + +- Add locked public wrappers and private unlocked helpers. +- Keep the public calls under the controller-to-vault lock order. +- Ensure key reads and writes used by a lifecycle transition share the same + operation boundary as the phase transition. + +### F-003: Token refresh and refresh-token rotation are not controller-serialized + +- Severity: High +- Status: Open +- Affected methods: + - [`fetchMetadataAccessCreds`](../src/SeedlessOnboardingController.ts#L519-L545) + - [`refreshAuthTokens`](../src/SeedlessOnboardingController.ts#L2808-L2824) + - [`rotateRefreshToken`](../src/SeedlessOnboardingController.ts#L2981-L3024) + +`refreshAuthTokens` coalesces concurrent refresh requests, but that only +deduplicates refresh calls. It does not serialize refresh state and vault writes +against password changes or recovery. The refresh path re-authenticates, updates +tokens, rewrites the vault, and may rotate refresh tokens. + +Recommended remediation: + +- Split public locked wrappers from private refresh implementations. +- Ensure internal callers that already hold the controller lock use the private + implementation rather than recursively acquiring the mutex. +- Serialize token/vault rewrites with password-change commits. +- Preserve one lock ordering for refresh, password change, and recovery. + +### F-004: `clearState` can replace state during an in-flight operation + +- Severity: High +- Status: Open +- Affected method: [`clearState`](../src/SeedlessOnboardingController.ts#L1447-L1453) + +`clearState` replaces the complete controller state without acquiring the +controller mutex. If called while a remote operation or vault rewrite is +awaiting, it can remove lifecycle, authentication, vault, and recovery data. +A later continuation can then write a partial or newly inconsistent state. + +Recommended remediation: + +- Serialize state clearing with the controller operation mutex. +- Reject or cancel it while a password-change/recovery transaction is active. +- Make the caller explicitly confirm that destructive state reset is intended. + +### F-005: Public lock bypasses weaken the lock contract + +- Severity: Medium +- Status: Open +- Affected methods: + - [`authenticate({ skipLock })`](../src/SeedlessOnboardingController.ts#L577-L647) + - [`verifyVaultPassword({ skipLock })`](../src/SeedlessOnboardingController.ts#L1104-L1119) + +These options are useful for internal callers that already own the mutex, but +they are present in public method signatures and messenger action handlers. +External callers can bypass serialization and mutate or inspect state while a +password operation is in flight. + +Recommended remediation: + +- Move unlocked variants to private helpers. +- Remove lock bypass options from public action signatures where possible. +- If an escape hatch must remain, document and enforce that it is internal-only. + +### F-006: Lifecycle advance methods do not validate the transition owner + +- Severity: Medium +- Status: Open +- Affected methods: + - [`clearPasswordChangePhase`](../src/SeedlessOnboardingController.ts#L2399-L2406) + - [`markPasswordChangeKeySyncPending`](../src/SeedlessOnboardingController.ts#L2417-L2429) + +Both methods use the controller lock, but they accept no expected phase, +transaction identifier, or generation. A delayed client call can therefore +clear or overwrite a newer lifecycle transaction after it acquires the lock. + +Recommended remediation: + +- Add a non-sensitive transaction identifier or monotonically increasing + lifecycle generation. +- Require the caller to provide the expected current phase/generation. +- Reject transitions that are not valid for the current lifecycle state. + +### F-007: Other direct state mutators are outside the operation lock + +- Severity: Medium +- Status: Open +- Affected methods: + - [`setMigrationVersion`](../src/SeedlessOnboardingController.ts#L794-L795) + - [`updateBackupMetadataState`](../src/SeedlessOnboardingController.ts#L1085-L1093) + +These methods are synchronous and do not necessarily lose fields because +`update` derives each patch from the current state. They can nevertheless +change state between awaited steps of a lifecycle operation and are not covered +by a single operation-level invariant. + +Recommended remediation: + +- Classify each method as lifecycle-sensitive, safe concurrent metadata, or + initialization-only. +- Lock lifecycle-sensitive methods. +- Keep unrelated metadata operations out of password-change critical sections + only if their invariants are explicitly independent. + +### F-008: In-memory phase updates are not proof of durable persistence + +- Severity: High for crash recovery +- Status: Open + +`#writePasswordChangePhase` calls `update`, which synchronously changes +in-memory state and publishes state-change events: + +- [`#writePasswordChangePhase`](../src/SeedlessOnboardingController.ts#L2381-L2387) + +The public lifecycle methods await the in-memory operation, but do not expose an +explicit acknowledgement that the downstream persisted state has been durably +written. A crash after an in-memory transition can leave durable state behind +the actual remote or local cryptographic state. + +Recommended remediation: + +- Provide an awaitable durable persistence boundary for lifecycle transitions. +- Do not clear the phase until key synchronization and required local writes + have been durably confirmed. +- Treat the persisted phase as a recovery trigger, never as proof of completion. + +## Test plan + +The controller should add deterministic concurrency tests for each operation +that crosses an `await` and later updates state. + +Required scenarios: + +- `resolvePasswordSyncState` waits behind `changePassword` and observes + `LOCAL_KEYRING_PENDING`. +- `resolvePasswordSyncState` waits behind a phase transition and never clears a + newer phase. +- `storeKeyringEncryptionKey` overlaps with a password-change state commit. +- Token refresh overlaps with password-change vault rewriting. +- `clearState` is rejected or serialized while recovery is active. +- Delayed `clearPasswordChangePhase` and + `markPasswordChangeKeySyncPending` cannot modify a newer transaction. +- Every failure path releases the controller and vault locks. +- Lifecycle state remains recoverable after an interrupted durable write. + +The F-001 regression test currently passes with: + +```sh +yarn workspace @metamask/seedless-onboarding-controller run test --no-coverage --runInBand +yarn workspace @metamask/seedless-onboarding-controller run build +``` + +## Acceptance criteria + +The state-lock design is complete when: + +- Every lifecycle-sensitive public operation reads state only after acquiring + the shared operation lock. +- Every lifecycle transition is validated against the current transaction or + generation. +- Controller and vault lock ordering is documented and enforced. +- Public lock bypasses are removed or made internal-only. +- Token refresh and vault rewrites cannot interleave with password changes. +- Durable lifecycle writes have an awaitable completion signal. +- Cross-controller Keyring operations are covered by a coordinator lock. +- Concurrency and process-interruption tests verify the final state, returned + status, and recoverability rather than only individual method results. diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController-method-action-types.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController-method-action-types.ts index 1281dd0c075..7f2da4d5d1f 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController-method-action-types.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController-method-action-types.ts @@ -183,48 +183,6 @@ export type SeedlessOnboardingControllerSetLockedAction = { handler: SeedlessOnboardingController['setLocked']; }; -/** - * Sync the latest global password to the controller. - * reset vault with latest globalPassword, - * persist the latest global password authPubKey - * - * @param params - The parameters for syncing the latest global password. - * @param params.globalPassword - The latest global password. - * @returns A promise that resolves to the success of the operation. - */ -export type SeedlessOnboardingControllerSyncLatestGlobalPasswordAction = { - type: `SeedlessOnboardingController:syncLatestGlobalPassword`; - handler: SeedlessOnboardingController['syncLatestGlobalPassword']; -}; - -/** - * @description Unlock the controller with the latest global password. - * - * @param params - The parameters for unlocking the controller. - * @param params.maxKeyChainLength - The maximum chain length of the pwd encryption keys. - * @param params.globalPassword - The latest global password. - * @returns A promise that resolves to the success of the operation. - */ -export type SeedlessOnboardingControllerSubmitGlobalPasswordAction = { - type: `SeedlessOnboardingController:submitGlobalPassword`; - handler: SeedlessOnboardingController['submitGlobalPassword']; -}; - -/** - * @description Check if the current password is outdated compare to the global password. - * - * @param options - Optional options object. - * @param options.globalAuthPubKey - The global auth public key to compare with the current auth public key. - * If not provided, the global auth public key will be fetched from the backend. - * @param options.skipCache - If true, bypass the cache and force a fresh check. - * @param options.skipLock - Whether to skip the lock acquisition. (to prevent deadlock in case the caller already acquired the lock) - * @returns A promise that resolves to true if the password is outdated, false otherwise. - */ -export type SeedlessOnboardingControllerCheckIsPasswordOutdatedAction = { - type: `SeedlessOnboardingController:checkIsPasswordOutdated`; - handler: SeedlessOnboardingController['checkIsPasswordOutdated']; -}; - /** * Check if the user is authenticated with the seedless onboarding flow by checking the token values in the state. * @@ -253,6 +211,9 @@ export type SeedlessOnboardingControllerClearStateAction = { * Store the keyring encryption key in state, encrypted under the current * encryption key. * + * Remains lifecycle-neutral: the client calls this during recovery without + * advancing the lifecycle phase. + * * @param keyringEncryptionKey - The keyring encryption key. */ export type SeedlessOnboardingControllerStoreKeyringEncryptionKeyAction = { @@ -271,6 +232,100 @@ export type SeedlessOnboardingControllerLoadKeyringEncryptionKeyAction = { handler: SeedlessOnboardingController['loadKeyringEncryptionKey']; }; +/** + * Clear the password-change lifecycle. + * + * Used after a definitive remote failure (server did not commit) or once + * Keyring encryption-key synchronization is verified and all required local + * writes have succeeded. The controller clears the phase so the next unlock + * is normal. + * + * @returns A promise that resolves once the lifecycle has been cleared. + */ +export type SeedlessOnboardingControllerClearPasswordChangePhaseAction = { + type: `SeedlessOnboardingController:clearPasswordChangePhase`; + handler: SeedlessOnboardingController['clearPasswordChangePhase']; +}; + +/** + * Mark the password-change lifecycle as `KEY_SYNC_PENDING`. + * + * Called by the client coordinator before it synchronizes the current + * Keyring encryption key to Seedless. The controller only records the + * boundary; it does not perform or verify synchronization. + * + * @returns A promise that resolves once the phase has been persisted. + */ +export type SeedlessOnboardingControllerMarkPasswordChangeKeySyncPendingAction = + { + type: `SeedlessOnboardingController:markPasswordChangeKeySyncPending`; + handler: SeedlessOnboardingController['markPasswordChangeKeySyncPending']; + }; + +/** + * Resolve the current password-sync state without consuming a password. + * + * Merges the legacy `checkIsPasswordOutdated` read with password-change + * recovery routing, so the client makes a single call at unlock (both on + * page render and on password submit) and routes UI from the returned status. + * + * Phase handling: + * - No phase (`undefined`): run the authoritative outdated check. `skipCache` + * is honored, so the client can read from cache on render and force a remote + * call on submit. Returns `InSync` or `PasswordOutdated` (another device + * changed the remote password). + * - `SEEDLESS_CHANGE_PENDING`: the remote outcome is ambiguous, so `skipCache` + * is ignored and a remote check is forced. Clears the phase (remote did not + * commit) or advances to `SEEDLESS_COMMITTED` (remote committed). Returns + * `InSync` or `EnterNewPassword`. + * - Other phases: return the next recovery step without mutating state. + * + * This method does not consume a password; the client prompts for the + * correct password and then calls `reconcilePassword`. + * + * @param options - The options. + * @param options.skipCache - Whether to bypass the outdated cache. Ignored + * for `SEEDLESS_CHANGE_PENDING`, which always forces a remote check. + * @returns The sync/recovery resolution. On any failure the last known phase + * is preserved and `PasswordSyncStatus.Unknown` is returned. + */ +export type SeedlessOnboardingControllerResolvePasswordSyncStateAction = { + type: `SeedlessOnboardingController:resolvePasswordSyncState`; + handler: SeedlessOnboardingController['resolvePasswordSyncState']; +}; + +/** + * Reconcile the local Seedless password with the remote password. + * + * For `SEEDLESS_COMMITTED` or `LOCAL_KEYRING_PENDING` it re-runs the existing + * password-sync flow (chain unlock + local vault rewrite) + * with the new password and advances the phase to `LOCAL_KEYRING_PENDING`. + * These operations are idempotent, so re-running them is safe whether or not + * the local Seedless vault was already rewritten. The controller is left + * unlocked. + * + * For no phase (`undefined`) it re-checks whether the remote password is + * outdated. If it is, it runs the same password-sync flow, advances to + * `LOCAL_KEYRING_PENDING`, and returns `ReconcileKeyring` so the client can + * reconcile the local Keyring (e.g. after another device changed the remote + * password). If the remote password is not outdated it is a no-op. + * + * The client remains responsible for the Keyring side (classifying the local + * Keyring via `KeyringController:verifyPassword` and running the old-Keyring + * or new-Keyring branch), because this controller does not depend on + * `KeyringController`. See + * [0003](./docs/0003-controller-owned-password-change-recovery-plan.md). + * + * @param params - The reconciliation parameters. + * @param params.globalPassword - The current global password. + * @returns The reconciliation result. On any failure the last known phase is + * preserved and `PasswordSyncStatus.Unknown` is returned. + */ +export type SeedlessOnboardingControllerReconcilePasswordAction = { + type: `SeedlessOnboardingController:reconcilePassword`; + handler: SeedlessOnboardingController['reconcilePassword']; +}; + /** * Refresh expired nodeAuthTokens, accessToken, and metadataAccessToken using * the stored refresh token. @@ -376,13 +431,14 @@ export type SeedlessOnboardingControllerMethodActions = | SeedlessOnboardingControllerGetSecretDataBackupStateAction | SeedlessOnboardingControllerSubmitPasswordAction | SeedlessOnboardingControllerSetLockedAction - | SeedlessOnboardingControllerSyncLatestGlobalPasswordAction - | SeedlessOnboardingControllerSubmitGlobalPasswordAction - | SeedlessOnboardingControllerCheckIsPasswordOutdatedAction | SeedlessOnboardingControllerGetIsUserAuthenticatedAction | SeedlessOnboardingControllerClearStateAction | SeedlessOnboardingControllerStoreKeyringEncryptionKeyAction | SeedlessOnboardingControllerLoadKeyringEncryptionKeyAction + | SeedlessOnboardingControllerClearPasswordChangePhaseAction + | SeedlessOnboardingControllerMarkPasswordChangeKeySyncPendingAction + | SeedlessOnboardingControllerResolvePasswordSyncStateAction + | SeedlessOnboardingControllerReconcilePasswordAction | SeedlessOnboardingControllerRefreshAuthTokensAction | SeedlessOnboardingControllerRotateRefreshTokenAction | SeedlessOnboardingControllerRevokePendingRefreshTokensAction diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts index 3ff1b8651a8..507f4c45d15 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts @@ -38,7 +38,6 @@ import { bytesToBase64, bytesToString, stringToBytes, - bigIntToHex, } from '@metamask/utils'; import { gcm } from '@noble/ciphers/aes'; import { utf8ToBytes } from '@noble/ciphers/utils'; @@ -70,8 +69,10 @@ import { SeedlessOnboardingMigrationVersion, AuthConnection, SecretType, + SeedlessPasswordChangePhase, + PasswordSyncStatus, } from './constants.js'; -import { PasswordSyncError, RecoveryError } from './errors.js'; +import { RecoveryError } from './errors.js'; import { SecretMetadata } from './SecretMetadata.js'; import { SeedlessOnboardingController, @@ -528,6 +529,34 @@ async function mockCreateToprfKeyAndBackupSeedPhrase< ); } +/** + * Creates a mock user with a vault and stores the Keyring encryption key. + * + * @param toprfClient - The ToprfSecureBackup instance. + * @param controller - The SeedlessOnboardingController instance. + * @param baseMessenger - The root messenger to call the methods through. + * @param password - The mock password. + */ +async function newUserSetup( + toprfClient: ToprfSecureBackup, + controller: SeedlessOnboardingController, + baseMessenger: RootMessenger, + password: string, +): Promise { + await mockCreateToprfKeyAndBackupSeedPhrase( + toprfClient, + controller, + baseMessenger, + password, + MOCK_SEED_PHRASE, + MOCK_KEYRING_ID, + ); + await baseMessenger.call( + 'SeedlessOnboardingController:storeKeyringEncryptionKey', + MOCK_KEYRING_ENCRYPTION_KEY, + ); +} + /** * Creates a mock vault. * @@ -643,6 +672,7 @@ async function decryptVault( * @param options.encryptedSeedlessEncryptionKey - The mock encrypted seedless encryption key. * @param options.pendingToBeRevokedTokens - The mock pending to be revoked tokens. * @param options.migrationVersion - The mock migration version. + * @param options.passwordChangePhase - The mock password-change phase. * @returns The initial controller state with the mock authenticated user. */ function getMockInitialControllerState(options?: { @@ -665,6 +695,7 @@ function getMockInitialControllerState(options?: { }[] | undefined; migrationVersion?: number; + passwordChangePhase?: SeedlessPasswordChangePhase; }): Partial { const state = getInitialSeedlessOnboardingControllerStateWithDefaults(); @@ -718,6 +749,10 @@ function getMockInitialControllerState(options?: { state.migrationVersion = options.migrationVersion; } + if (options?.passwordChangePhase !== undefined) { + state.passwordChangePhase = options.passwordChangePhase; + } + return state; } @@ -1110,10 +1145,47 @@ describe('SeedlessOnboardingController', () => { }, ); }); + + it('should preserve the existing revoke token when it is omitted', async () => { + const existingRevokeToken = 'existing-revoke-token'; + + await withController( + { + state: { + ...getMockInitialControllerState({ + withMockAuthenticatedUser: true, + }), + revokeToken: existingRevokeToken, + }, + }, + async ({ controller, toprfClient, baseMessenger }) => { + jest.spyOn(toprfClient, 'authenticate').mockResolvedValue({ + nodeAuthTokens: MOCK_NODE_AUTH_TOKENS, + isNewUser: false, + }); + + await baseMessenger.call( + 'SeedlessOnboardingController:authenticate', + { + idTokens, + authConnectionId, + userId, + authConnection, + socialLoginEmail, + refreshToken, + accessToken, + metadataAccessToken, + }, + ); + + expect(controller.state.revokeToken).toBe(existingRevokeToken); + }, + ); + }); }); - describe('checkPasswordOutdated', () => { - it('should return false if password is not outdated (authPubKey matches)', async () => { + describe('resolvePasswordSyncState (no lifecycle: outdated check)', () => { + it('should return InSync if password is not outdated (authPubKey matches)', async () => { await withController( { state: getMockInitialControllerState({ @@ -1125,21 +1197,21 @@ describe('SeedlessOnboardingController', () => { const spy = jest.spyOn(toprfClient, 'fetchAuthPubKey'); mockFetchAuthPubKey(toprfClient, base64ToBytes(MOCK_AUTH_PUB_KEY)); const result = await baseMessenger.call( - 'SeedlessOnboardingController:checkIsPasswordOutdated', + 'SeedlessOnboardingController:resolvePasswordSyncState', ); - expect(result).toBe(false); + expect(result).toBe(PasswordSyncStatus.InSync); // Call again to test cache const result2 = await baseMessenger.call( - 'SeedlessOnboardingController:checkIsPasswordOutdated', + 'SeedlessOnboardingController:resolvePasswordSyncState', ); - expect(result2).toBe(false); + expect(result2).toBe(PasswordSyncStatus.InSync); // Should only call fetchAuthPubKey once due to cache expect(spy).toHaveBeenCalledTimes(1); }, ); }); - it('should return true if password is outdated (authPubKey does not match)', async () => { + it('should return PasswordOutdated if password is outdated (authPubKey does not match)', async () => { await withController( { state: getMockInitialControllerState({ @@ -1151,14 +1223,14 @@ describe('SeedlessOnboardingController', () => { const spy = jest.spyOn(toprfClient, 'fetchAuthPubKey'); mockFetchAuthPubKey(toprfClient, base64ToBytes(MOCK_AUTH_PUB_KEY)); const result = await baseMessenger.call( - 'SeedlessOnboardingController:checkIsPasswordOutdated', + 'SeedlessOnboardingController:resolvePasswordSyncState', ); - expect(result).toBe(true); + expect(result).toBe(PasswordSyncStatus.PasswordOutdated); // Call again to test cache const result2 = await baseMessenger.call( - 'SeedlessOnboardingController:checkIsPasswordOutdated', + 'SeedlessOnboardingController:resolvePasswordSyncState', ); - expect(result2).toBe(true); + expect(result2).toBe(PasswordSyncStatus.PasswordOutdated); // Should only call fetchAuthPubKey once due to cache expect(spy).toHaveBeenCalledTimes(1); }, @@ -1177,26 +1249,26 @@ describe('SeedlessOnboardingController', () => { const spy = jest.spyOn(toprfClient, 'fetchAuthPubKey'); mockFetchAuthPubKey(toprfClient, base64ToBytes(MOCK_AUTH_PUB_KEY)); const result = await baseMessenger.call( - 'SeedlessOnboardingController:checkIsPasswordOutdated', + 'SeedlessOnboardingController:resolvePasswordSyncState', { skipCache: true, }, ); - expect(result).toBe(false); + expect(result).toBe(PasswordSyncStatus.InSync); // Call again with skipCache: true, should call fetchAuthPubKey again const result2 = await baseMessenger.call( - 'SeedlessOnboardingController:checkIsPasswordOutdated', + 'SeedlessOnboardingController:resolvePasswordSyncState', { skipCache: true, }, ); - expect(result2).toBe(false); + expect(result2).toBe(PasswordSyncStatus.InSync); expect(spy).toHaveBeenCalledTimes(2); }, ); }); - it('should throw SRPNotBackedUpError if no authPubKey in state', async () => { + it('should return Unknown if no authPubKey in state', async () => { await withController( { state: getMockInitialControllerState({ @@ -1204,18 +1276,15 @@ describe('SeedlessOnboardingController', () => { }), }, async ({ baseMessenger }) => { - await expect( - baseMessenger.call( - 'SeedlessOnboardingController:checkIsPasswordOutdated', - ), - ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.SRPNotBackedUpError, + const result = await baseMessenger.call( + 'SeedlessOnboardingController:resolvePasswordSyncState', ); + expect(result).toBe(PasswordSyncStatus.Unknown); }, ); }); - it('should throw InsufficientAuthToken if no nodeAuthTokens in state', async () => { + it('should return Unknown if no nodeAuthTokens in state', async () => { await withController( { state: { @@ -1227,18 +1296,15 @@ describe('SeedlessOnboardingController', () => { }, }, async ({ baseMessenger }) => { - await expect( - baseMessenger.call( - 'SeedlessOnboardingController:checkIsPasswordOutdated', - ), - ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.InsufficientAuthToken, + const result = await baseMessenger.call( + 'SeedlessOnboardingController:resolvePasswordSyncState', ); + expect(result).toBe(PasswordSyncStatus.Unknown); }, ); }); - it('should throw FailedToFetchAuthPubKey error when fetchAuthPubKey fails', async () => { + it('should return Unknown when fetchAuthPubKey fails', async () => { await withController( { state: getMockInitialControllerState({ @@ -1252,13 +1318,10 @@ describe('SeedlessOnboardingController', () => { .spyOn(toprfClient, 'fetchAuthPubKey') .mockRejectedValueOnce(new Error('Network error')); - await expect( - baseMessenger.call( - 'SeedlessOnboardingController:checkIsPasswordOutdated', - ), - ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.FailedToFetchAuthPubKey, + const result = await baseMessenger.call( + 'SeedlessOnboardingController:resolvePasswordSyncState', ); + expect(result).toBe(PasswordSyncStatus.Unknown); }, ); }); @@ -1894,6 +1957,42 @@ describe('SeedlessOnboardingController', () => { }); }); + it('should throw PasswordChangeInProgress if a password change is pending', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + withMockAuthPubKey: true, + vault: MOCK_VAULT, + vaultEncryptionKey: MOCK_VAULT_ENCRYPTION_KEY, + vaultEncryptionSalt: MOCK_VAULT_ENCRYPTION_SALT, + passwordChangePhase: SeedlessPasswordChangePhase.SeedlessCommitted, + }), + }, + async ({ baseMessenger }) => { + // Unlock first so #assertIsUnlocked() passes; the phase stays + // set because submitPassword does not touch it. + await baseMessenger.call( + 'SeedlessOnboardingController:submitPassword', + MOCK_PASSWORD, + ); + + await expect( + baseMessenger.call( + 'SeedlessOnboardingController:addNewSecretData', + NEW_KEY_RING_1.seedPhrase, + EncAccountDataType.ImportedSrp, + { + keyringId: NEW_KEY_RING_1.id, + }, + ), + ).rejects.toThrow( + SeedlessOnboardingControllerErrorMessage.PasswordChangeInProgress, + ); + }, + ); + }); + it('should be able to add a new seed phrase backup', async () => { await withController( { @@ -3983,6 +4082,25 @@ describe('SeedlessOnboardingController', () => { const { encKey: newEncKey, authKeyPair: newAuthKeyPair } = mockChangeEncKey(toprfClient, NEW_MOCK_PASSWORD); + // Observe lifecycle transitions as the password change progresses: + // SEEDLESS_CHANGE_PENDING is written before the remote mutation, + // SEEDLESS_COMMITTED after it succeeds, then + // LOCAL_KEYRING_PENDING once the local vault is rewritten. + const observedPhaseTransitions: ( + | SeedlessPasswordChangePhase + | undefined + )[] = []; + let previousPhase = controller.state.passwordChangePhase; + baseMessenger.subscribe( + 'SeedlessOnboardingController:stateChange', + (state) => { + if (state.passwordChangePhase !== previousPhase) { + observedPhaseTransitions.push(state.passwordChangePhase); + } + previousPhase = state.passwordChangePhase; + }, + ); + await baseMessenger.call( 'SeedlessOnboardingController:changePassword', NEW_MOCK_PASSWORD, @@ -4012,6 +4130,82 @@ describe('SeedlessOnboardingController', () => { expect(newEncKeyFromVault).toStrictEqual(newEncKey); expect(newAuthKeyPairFromVault.sk).toStrictEqual(newAuthKeyPair.sk); expect(newAuthKeyPairFromVault.pk).toStrictEqual(newAuthKeyPair.pk); + + // The lifecycle advances through every phase in order and ends on + // LOCAL_KEYRING_PENDING, signalling the local rewrite completed. + expect(observedPhaseTransitions).toStrictEqual([ + SeedlessPasswordChangePhase.SeedlessChangePending, + SeedlessPasswordChangePhase.SeedlessCommitted, + SeedlessPasswordChangePhase.LocalKeyringPending, + ]); + expect(controller.state.passwordChangePhase).toBe( + SeedlessPasswordChangePhase.LocalKeyringPending, + ); + }, + ); + }); + + it('should set LOCAL_KEYRING_PENDING and the re-encrypted keyring key in the same state update', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + withMockAuthPubKey: true, + }), + }, + async ({ controller, toprfClient, baseMessenger }) => { + await newUserSetup( + toprfClient, + controller, + baseMessenger, + MOCK_PASSWORD, + ); + const oldEncryptedKeyringEncryptionKey = + controller.state.encryptedKeyringEncryptionKey; + + mockFetchAuthPubKey( + toprfClient, + base64ToBytes(controller.state.authPubKey as string), + ); + mockRecoverEncKey(toprfClient, MOCK_PASSWORD); + mockChangeEncKey(toprfClient, NEW_MOCK_PASSWORD); + + // Capture the encryptedKeyringEncryptionKey value at the moment + // LOCAL_KEYRING_PENDING first appears. If the phase and the + // re-encrypted key are written in the same update, this equals + // the final value; if they are separate updates, it is the stale + // (pre-re-encryption) value. + let keyAtLocalKeyringPending: string | undefined; + baseMessenger.subscribe( + 'SeedlessOnboardingController:stateChange', + (state) => { + if ( + state.passwordChangePhase === + SeedlessPasswordChangePhase.LocalKeyringPending && + keyAtLocalKeyringPending === undefined + ) { + keyAtLocalKeyringPending = state.encryptedKeyringEncryptionKey; + } + }, + ); + + await baseMessenger.call( + 'SeedlessOnboardingController:changePassword', + NEW_MOCK_PASSWORD, + MOCK_PASSWORD, + ); + + expect(controller.state.passwordChangePhase).toBe( + SeedlessPasswordChangePhase.LocalKeyringPending, + ); + // The phase and the re-encrypted key are set in one update, so the + // key at the moment the phase advances is already the new one. + expect(keyAtLocalKeyringPending).toStrictEqual( + controller.state.encryptedKeyringEncryptionKey, + ); + expect(keyAtLocalKeyringPending).not.toStrictEqual( + oldEncryptedKeyringEncryptionKey, + ); }, ); }); @@ -4094,12 +4288,16 @@ describe('SeedlessOnboardingController', () => { expect(newEncKeyFromVault).toStrictEqual(newEncKey); expect(newAuthKeyPairFromVault.sk).toStrictEqual(newAuthKeyPair.sk); expect(newAuthKeyPairFromVault.pk).toStrictEqual(newAuthKeyPair.pk); + + expect(controller.state.passwordChangePhase).toBe( + SeedlessPasswordChangePhase.LocalKeyringPending, + ); }, ); }); it('should throw an error if the controller is locked', async () => { - await withController(async ({ baseMessenger }) => { + await withController(async ({ controller, baseMessenger }) => { await expect( baseMessenger.call( 'SeedlessOnboardingController:changePassword', @@ -4109,9 +4307,57 @@ describe('SeedlessOnboardingController', () => { ).rejects.toThrow( SeedlessOnboardingControllerErrorMessage.ControllerLocked, ); + + // No lifecycle is written when the controller rejects up front. + expect(controller.state.passwordChangePhase).toBeUndefined(); }); }); + it('should reject a second password change while one is already in progress', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + withMockAuthPubKey: true, + passwordChangePhase: + SeedlessPasswordChangePhase.SeedlessChangePending, + }), + }, + async ({ controller, toprfClient, baseMessenger }) => { + await mockCreateToprfKeyAndBackupSeedPhrase( + toprfClient, + controller, + baseMessenger, + MOCK_PASSWORD, + MOCK_SEED_PHRASE, + MOCK_KEYRING_ID, + ); + + // A previous change left the lifecycle set (e.g. a + // crash after the remote commit). Recovery has not finished, so a + // fresh change must not start. + expect(controller.state.passwordChangePhase).toBe( + SeedlessPasswordChangePhase.SeedlessChangePending, + ); + + await expect( + baseMessenger.call( + 'SeedlessOnboardingController:changePassword', + NEW_MOCK_PASSWORD, + MOCK_PASSWORD, + ), + ).rejects.toThrow( + SeedlessOnboardingControllerErrorMessage.PasswordChangeInProgress, + ); + + // The guard must not mutate the persisted phase. + expect(controller.state.passwordChangePhase).toBe( + SeedlessPasswordChangePhase.SeedlessChangePending, + ); + }, + ); + }); + it('should throw error if password is outdated', async () => { await withController( { @@ -4144,6 +4390,11 @@ describe('SeedlessOnboardingController', () => { ).rejects.toThrow( SeedlessOnboardingControllerErrorMessage.FailedToChangePassword, ); + + // The outdated-password check rejects before the first lifecycle + // write (SEEDLESS_CHANGE_PENDING), so there is nothing to recover + // and the lifecycle stays unset. + expect(controller.state.passwordChangePhase).toBeUndefined(); }, ); }); @@ -4189,6 +4440,15 @@ describe('SeedlessOnboardingController', () => { ).rejects.toThrow( SeedlessOnboardingControllerErrorMessage.FailedToChangePassword, ); + + // The lifecycle was advanced to SEEDLESS_CHANGE_PENDING before the + // remote mutation, so on remote failure that phase is preserved as + // the recovery signal. The client performs an authoritative + // password-outdated check to decide the branch; the controller does + // not overwrite it to UNKNOWN. + expect(controller.state.passwordChangePhase).toBe( + SeedlessPasswordChangePhase.SeedlessChangePending, + ); }, ); }); @@ -4243,6 +4503,10 @@ describe('SeedlessOnboardingController', () => { newPassword: NEW_MOCK_PASSWORD, }), ); + + expect(controller.state.passwordChangePhase).toBe( + SeedlessPasswordChangePhase.LocalKeyringPending, + ); }, ); }); @@ -4355,6 +4619,10 @@ describe('SeedlessOnboardingController', () => { const [legacyTransformed] = transformDataItems?.([legacyItem]) ?? []; expect(legacyTransformed?.version).toBe('v1'); + + expect(controller.state.passwordChangePhase).toBe( + SeedlessPasswordChangePhase.LocalKeyringPending, + ); }, ); }); @@ -4417,6 +4685,10 @@ describe('SeedlessOnboardingController', () => { newPassword: NEW_MOCK_PASSWORD, }), ); + + expect(controller.state.passwordChangePhase).toBe( + SeedlessPasswordChangePhase.LocalKeyringPending, + ); }, ); }); @@ -4453,1445 +4725,1686 @@ describe('SeedlessOnboardingController', () => { ).rejects.toThrow( SeedlessOnboardingControllerErrorMessage.FailedToChangePassword, ); + + // The fetch failure rejects before the first lifecycle write + // (SEEDLESS_CHANGE_PENDING), so there is nothing to recover and the + // lifecycle stays unset. + expect(controller.state.passwordChangePhase).toBeUndefined(); }, ); }); + + describe('clearPasswordChangePhase', () => { + it('clears an in-progress lifecycle', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + passwordChangePhase: SeedlessPasswordChangePhase.Unknown, + }), + }, + async ({ controller }) => { + await controller.clearPasswordChangePhase(); + + expect(controller.state.passwordChangePhase).toBeUndefined(); + }, + ); + }); + + it('is a no-op when no change is in progress', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + }), + }, + async ({ controller }) => { + await controller.clearPasswordChangePhase(); + + expect(controller.state.passwordChangePhase).toBeUndefined(); + }, + ); + }); + }); + + describe('markPasswordChangeKeySyncPending', () => { + it('advances the lifecycle to KEY_SYNC_PENDING', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + passwordChangePhase: + SeedlessPasswordChangePhase.LocalKeyringPending, + }), + }, + async ({ controller }) => { + await controller.markPasswordChangeKeySyncPending(); + + expect(controller.state.passwordChangePhase).toBe( + SeedlessPasswordChangePhase.KeySyncPending, + ); + }, + ); + }); + + it('is a no-op when already KEY_SYNC_PENDING', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + passwordChangePhase: SeedlessPasswordChangePhase.KeySyncPending, + }), + }, + async ({ controller }) => { + await controller.markPasswordChangeKeySyncPending(); + + expect(controller.state.passwordChangePhase).toBe( + SeedlessPasswordChangePhase.KeySyncPending, + ); + }, + ); + }); + }); }); - describe('clearState', () => { - it('should clear the state', async () => { + describe('resolvePasswordSyncState (recovery phases)', () => { + it('returns in-sync when no phase is set and the password is in sync', async () => { await withController( { state: getMockInitialControllerState({ withMockAuthenticatedUser: true, + withMockAuthPubKey: true, }), }, - async ({ controller, baseMessenger }) => { - const { state } = controller; + async ({ toprfClient, controller }) => { + mockFetchAuthPubKey(toprfClient, base64ToBytes(MOCK_AUTH_PUB_KEY)); + const result = await controller.resolvePasswordSyncState(); + expect(result).toBe(PasswordSyncStatus.InSync); + expect(controller.state.passwordChangePhase).toBeUndefined(); + }, + ); + }); - expect(state.nodeAuthTokens).toBeDefined(); - expect(state.userId).toBeDefined(); - expect(state.authConnectionId).toBeDefined(); - - baseMessenger.call('SeedlessOnboardingController:clearState'); - expect(controller.state).toStrictEqual( - getInitialSeedlessOnboardingControllerStateWithDefaults(), + it('returns enter-new-password when the phase is SEEDLESS_COMMITTED', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + withMockAuthPubKey: true, + passwordChangePhase: SeedlessPasswordChangePhase.SeedlessCommitted, + }), + }, + async ({ controller }) => { + const result = await controller.resolvePasswordSyncState(); + expect(result).toBe(PasswordSyncStatus.EnterNewPassword); + expect(controller.state.passwordChangePhase).toBe( + SeedlessPasswordChangePhase.SeedlessCommitted, ); }, ); }); - }); - - describe('vault', () => { - const MOCK_PASSWORD = 'mock-password'; - it('should throw an error if the password is an empty string', async () => { + it('returns reconcile-keyring when the phase is LOCAL_KEYRING_PENDING', async () => { await withController( { state: getMockInitialControllerState({ withMockAuthenticatedUser: true, + withMockAuthPubKey: true, + passwordChangePhase: + SeedlessPasswordChangePhase.LocalKeyringPending, }), }, - async ({ toprfClient, baseMessenger }) => { - // create the local enc key - mockcreateLocalKey(toprfClient, MOCK_PASSWORD); - // persist the local enc key - jest.spyOn(toprfClient, 'persistLocalKey').mockResolvedValueOnce(); - // mock the secret data add - const mockSecretDataAdd = handleMockSecretDataAdd(); - await expect( - baseMessenger.call( - 'SeedlessOnboardingController:createToprfKeyAndBackupSeedPhrase', - '', - MOCK_SEED_PHRASE, - MOCK_KEYRING_ID, - ), - ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.InvalidEmptyPassword, - ); + async ({ controller }) => { + const result = await controller.resolvePasswordSyncState(); + expect(result).toBe(PasswordSyncStatus.ReconcileKeyring); + }, + ); + }); - expect(mockSecretDataAdd.isDone()).toBe(true); + it('returns sync-key when the phase is KEY_SYNC_PENDING', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + withMockAuthPubKey: true, + passwordChangePhase: SeedlessPasswordChangePhase.KeySyncPending, + }), + }, + async ({ controller }) => { + const result = await controller.resolvePasswordSyncState(); + expect(result).toBe(PasswordSyncStatus.SyncKey); }, ); }); - it('should throw an error if the passowrd is of wrong type', async () => { + it('returns unknown when the phase is UNKNOWN', async () => { await withController( { state: getMockInitialControllerState({ withMockAuthenticatedUser: true, + withMockAuthPubKey: true, + passwordChangePhase: SeedlessPasswordChangePhase.Unknown, }), }, - async ({ toprfClient, baseMessenger }) => { - // create the local enc key - mockcreateLocalKey(toprfClient, MOCK_PASSWORD); - // persist the local enc key - jest.spyOn(toprfClient, 'persistLocalKey').mockResolvedValueOnce(); - // mock the secret data add - const mockSecretDataAdd = handleMockSecretDataAdd(); - await expect( - baseMessenger.call( - 'SeedlessOnboardingController:createToprfKeyAndBackupSeedPhrase', - // @ts-expect-error Intentionally passing wrong password type - 123, - MOCK_SEED_PHRASE, - 'MOCK_KEYRING_ID', - ), - ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.WrongPasswordType, - ); + async ({ controller }) => { + const result = await controller.resolvePasswordSyncState(); + expect(result).toBe(PasswordSyncStatus.Unknown); + }, + ); + }); - expect(mockSecretDataAdd.isDone()).toBe(true); + it('treats an unrecognized persisted phase as in-sync', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + withMockAuthPubKey: true, + passwordChangePhase: + 'unrecognized' as unknown as SeedlessPasswordChangePhase, + }), + }, + async ({ controller }) => { + const result = await controller.resolvePasswordSyncState(); + expect(result).toBe(PasswordSyncStatus.InSync); }, ); }); - }); - describe('lock', () => { - const MOCK_PASSWORD = 'mock-password'; + it('clears the phase when SEEDLESS_CHANGE_PENDING and remote did not commit', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + withMockAuthPubKey: true, + passwordChangePhase: + SeedlessPasswordChangePhase.SeedlessChangePending, + }), + }, + async ({ toprfClient, controller }) => { + // Remote auth pub key matches the local one -> not outdated. + mockFetchAuthPubKey(toprfClient, base64ToBytes(MOCK_AUTH_PUB_KEY)); + const result = await controller.resolvePasswordSyncState(); + expect(result).toBe(PasswordSyncStatus.InSync); + expect(controller.state.passwordChangePhase).toBeUndefined(); + }, + ); + }); - it('should lock the controller', async () => { - const mutexAcquireSpy = jest - .spyOn(Mutex.prototype, 'acquire') - .mockResolvedValueOnce(jest.fn()); + it('advances to SEEDLESS_COMMITTED when SEEDLESS_CHANGE_PENDING and remote committed', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + authPubKey: MOCK_AUTH_PUB_KEY_OUTDATED, + passwordChangePhase: + SeedlessPasswordChangePhase.SeedlessChangePending, + }), + }, + async ({ toprfClient, controller }) => { + // Remote auth pub key differs from the stale local one -> outdated. + mockFetchAuthPubKey(toprfClient, base64ToBytes(MOCK_AUTH_PUB_KEY)); + const result = await controller.resolvePasswordSyncState(); + expect(result).toBe(PasswordSyncStatus.EnterNewPassword); + expect(controller.state.passwordChangePhase).toBe( + SeedlessPasswordChangePhase.SeedlessCommitted, + ); + }, + ); + }); + it('re-reads the lifecycle phase after waiting for an in-flight password change', async () => { await withController( { state: getMockInitialControllerState({ withMockAuthenticatedUser: true, + withMockAuthPubKey: true, }), }, async ({ controller, toprfClient, baseMessenger }) => { - await mockCreateToprfKeyAndBackupSeedPhrase( + const oldPassword = 'old-mock-password'; + const newPassword = 'new-mock-password'; + + await newUserSetup( toprfClient, controller, baseMessenger, - MOCK_PASSWORD, - MOCK_SEED_PHRASE, - MOCK_KEYRING_ID, + oldPassword, ); - await baseMessenger.call('SeedlessOnboardingController:setLocked'); + const currentAuthPubKey = base64ToBytes( + controller.state.authPubKey as string, + ); + const newToprfEncryptor = createMockToprfEncryptor(); + const changeEncryptionKeyResult: ChangeEncryptionKeyResult = { + encKey: newToprfEncryptor.deriveEncKey(newPassword), + pwEncKey: newToprfEncryptor.derivePwEncKey(newPassword), + authKeyPair: newToprfEncryptor.deriveAuthKeyPair(newPassword), + }; - // verify that the mutex acquire was called - expect(mutexAcquireSpy).toHaveBeenCalled(); + let resolveChangeEncKeyStarted!: () => void; + const changeEncKeyStarted = new Promise((resolve) => { + resolveChangeEncKeyStarted = resolve; + }); + let resolveChangeEncKey!: (result: ChangeEncryptionKeyResult) => void; + const changeEncKeyResult = new Promise( + (resolve) => { + resolveChangeEncKey = resolve; + }, + ); - await expect( - baseMessenger.call( - 'SeedlessOnboardingController:addNewSecretData', - MOCK_SEED_PHRASE, - EncAccountDataType.ImportedSrp, - { - keyringId: MOCK_KEYRING_ID, - }, - ), - ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.ControllerLocked, + jest + .spyOn(toprfClient, 'fetchAuthPubKey') + .mockResolvedValueOnce({ + authPubKey: currentAuthPubKey, + keyIndex: 1, + }) + .mockResolvedValue({ + authPubKey: changeEncryptionKeyResult.authKeyPair.pk, + keyIndex: 1, + }); + jest.spyOn(toprfClient, 'changeEncKey').mockImplementation(() => { + resolveChangeEncKeyStarted(); + return changeEncKeyResult; + }); + + const changePasswordPromise = baseMessenger.call( + 'SeedlessOnboardingController:changePassword', + newPassword, + oldPassword, + ); + await changeEncKeyStarted; + expect(controller.state.passwordChangePhase).toBe( + SeedlessPasswordChangePhase.SeedlessChangePending, + ); + + // resolvePasswordSyncState snapshots the pending phase before it + // waits for the controller lock. The password change completes first, + // so the resolver must use LOCAL_KEYRING_PENDING instead of clearing + // the lifecycle and returning in-sync. + const resolvePasswordSyncStatePromise = + controller.resolvePasswordSyncState(); + resolveChangeEncKey(changeEncryptionKeyResult); + await changePasswordPromise; + + expect(await resolvePasswordSyncStatePromise).toBe( + PasswordSyncStatus.ReconcileKeyring, + ); + expect(controller.state.passwordChangePhase).toBe( + SeedlessPasswordChangePhase.LocalKeyringPending, + ); + }, + ); + }); + + it('returns unknown and preserves the phase when the remote check fails', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + withMockAuthPubKey: true, + passwordChangePhase: + SeedlessPasswordChangePhase.SeedlessChangePending, + }), + }, + async ({ toprfClient, controller }) => { + jest + .spyOn(toprfClient, 'fetchAuthPubKey') + .mockRejectedValueOnce(new Error('network failure')); + const result = await controller.resolvePasswordSyncState(); + expect(result).toBe(PasswordSyncStatus.Unknown); + // The phase is preserved as the recovery signal. + expect(controller.state.passwordChangePhase).toBe( + SeedlessPasswordChangePhase.SeedlessChangePending, ); }, ); }); }); - describe('SeedPhraseMetadata', () => { - it('should be able to create a seed phrase metadata with default options', () => { - // should be able to create a SecretMetadata instance via constructor - const seedPhraseMetadata = new SecretMetadata(MOCK_SEED_PHRASE); - expect(seedPhraseMetadata.data).toBeDefined(); - expect(seedPhraseMetadata.timestamp).toBeDefined(); - expect(seedPhraseMetadata.type).toBe(SecretType.Mnemonic); - // V2 fields should be undefined - expect(seedPhraseMetadata.dataType).toBeUndefined(); - expect(seedPhraseMetadata.itemId).toBeUndefined(); - expect(seedPhraseMetadata.createdAt).toBeUndefined(); - expect(seedPhraseMetadata.storageVersion).toBeUndefined(); + describe('reconcilePassword', () => { + const OLD_PASSWORD = 'old-mock-password'; + const NEW_PASSWORD = 'new-mock-password'; - // should be able to create a SecretMetadata instance with a timestamp via constructor - const timestamp = 18_000; - const seedPhraseMetadata2 = new SecretMetadata(MOCK_SEED_PHRASE, { - timestamp, - }); - expect(seedPhraseMetadata2.data).toBeDefined(); - expect(seedPhraseMetadata2.timestamp).toBe(timestamp); - expect(seedPhraseMetadata2.data).toStrictEqual(MOCK_SEED_PHRASE); - expect(seedPhraseMetadata2.type).toBe(SecretType.Mnemonic); - expect(seedPhraseMetadata2.dataType).toBeUndefined(); + it('returns in-sync when no phase is set and the password is in sync', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + withMockAuthPubKey: true, + }), + }, + async ({ toprfClient, controller }) => { + mockFetchAuthPubKey(toprfClient, base64ToBytes(MOCK_AUTH_PUB_KEY)); + const result = await controller.reconcilePassword({ + globalPassword: NEW_PASSWORD, + }); + expect(result).toBe(PasswordSyncStatus.InSync); + }, + ); }); - it('should be able to add metadata to a seed phrase', () => { - const timestamp = 18_000; - const seedPhraseMetadata = new SecretMetadata(MOCK_SEED_PHRASE, { - type: SecretType.PrivateKey, - timestamp, - }); - expect(seedPhraseMetadata.type).toBe(SecretType.PrivateKey); - expect(seedPhraseMetadata.timestamp).toBe(timestamp); + it('returns unknown when the phase is SEEDLESS_CHANGE_PENDING', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + withMockAuthPubKey: true, + passwordChangePhase: + SeedlessPasswordChangePhase.SeedlessChangePending, + }), + }, + async ({ controller }) => { + const result = await controller.reconcilePassword({ + globalPassword: NEW_PASSWORD, + }); + expect(result).toBe(PasswordSyncStatus.Unknown); + expect(controller.state.passwordChangePhase).toBe( + SeedlessPasswordChangePhase.SeedlessChangePending, + ); + }, + ); }); - it('should be able to serialized and parse a seed phrase metadata', () => { - const seedPhraseMetadata = new SecretMetadata(MOCK_SEED_PHRASE); - const serializedSeedPhraseBytes = seedPhraseMetadata.toBytes(); + it('returns sync-key when the phase is KEY_SYNC_PENDING', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + withMockAuthPubKey: true, + passwordChangePhase: SeedlessPasswordChangePhase.KeySyncPending, + }), + }, + async ({ controller }) => { + const result = await controller.reconcilePassword({ + globalPassword: NEW_PASSWORD, + }); + expect(result).toBe(PasswordSyncStatus.SyncKey); + }, + ); + }); - const parsedSeedPhraseMetadata = SecretMetadata.fromRawMetadata( - serializedSeedPhraseBytes, - {}, + it('returns unknown when the phase is UNKNOWN', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + withMockAuthPubKey: true, + passwordChangePhase: SeedlessPasswordChangePhase.Unknown, + }), + }, + async ({ controller }) => { + const result = await controller.reconcilePassword({ + globalPassword: NEW_PASSWORD, + }); + expect(result).toBe(PasswordSyncStatus.Unknown); + }, ); - expect(parsedSeedPhraseMetadata.data).toBeDefined(); - expect(parsedSeedPhraseMetadata.timestamp).toBeDefined(); - expect(parsedSeedPhraseMetadata.data).toStrictEqual(MOCK_SEED_PHRASE); }); - it('should be able to compare seed phrase metadata by timestamp', () => { - const mockSeedPhraseMetadata1 = new SecretMetadata(MOCK_SEED_PHRASE, { - timestamp: 1000, - }); - const mockSeedPhraseMetadata2 = new SecretMetadata(MOCK_SEED_PHRASE, { - timestamp: 2000, - }); - - // ascending order: earlier timestamp first - expect( - SecretMetadata.compareByTimestamp( - mockSeedPhraseMetadata1, - mockSeedPhraseMetadata2, - 'asc', - ), - ).toBeLessThan(0); - - // descending order: later timestamp first - expect( - SecretMetadata.compareByTimestamp( - mockSeedPhraseMetadata1, - mockSeedPhraseMetadata2, - 'desc', - ), - ).toBeGreaterThan(0); - - // default order (no parameter): should use ascending order - expect( - SecretMetadata.compareByTimestamp( - mockSeedPhraseMetadata1, - mockSeedPhraseMetadata2, - ), - ).toBeLessThan(0); - }); - - describe('compare', () => { - it('should sort PrimarySrp first regardless of createdAt or timestamp', () => { - const primarySrp = new SecretMetadata(MOCK_SEED_PHRASE, { - timestamp: 2000, - dataType: EncAccountDataType.PrimarySrp, - createdAt: '00000002-0000-1000-8000-000000000002', - }); - const importedSrp = new SecretMetadata(MOCK_SEED_PHRASE, { - timestamp: 1000, - dataType: EncAccountDataType.ImportedSrp, - createdAt: '00000001-0000-1000-8000-000000000001', - }); - - expect( - SecretMetadata.compare(primarySrp, importedSrp, 'asc'), - ).toBeLessThan(0); - expect( - SecretMetadata.compare(importedSrp, primarySrp, 'asc'), - ).toBeGreaterThan(0); - // Also in desc order - expect( - SecretMetadata.compare(primarySrp, importedSrp, 'desc'), - ).toBeLessThan(0); - }); - - it('should return 0 when both items are PrimarySrp (handles data corruption gracefully)', () => { - const primarySrp1 = new SecretMetadata(MOCK_SEED_PHRASE, { - timestamp: 1000, - dataType: EncAccountDataType.PrimarySrp, - createdAt: '00000001-0000-1000-8000-000000000001', - }); - const primarySrp2 = new SecretMetadata(MOCK_SEED_PHRASE, { - timestamp: 2000, - dataType: EncAccountDataType.PrimarySrp, - createdAt: '00000002-0000-1000-8000-000000000002', - }); - - expect(SecretMetadata.compare(primarySrp1, primarySrp2, 'asc')).toBe(0); - expect(SecretMetadata.compare(primarySrp2, primarySrp1, 'asc')).toBe(0); - expect(SecretMetadata.compare(primarySrp1, primarySrp2, 'desc')).toBe( - 0, - ); - }); - - it('should compare by createdAt (TIMEUUID) when both have createdAt', () => { - const earlier = new SecretMetadata(MOCK_SEED_PHRASE, { - timestamp: 1000, - dataType: EncAccountDataType.ImportedSrp, - createdAt: '00000001-0000-1000-8000-000000000001', - }); - const later = new SecretMetadata(MOCK_SEED_PHRASE, { - timestamp: 2000, - dataType: EncAccountDataType.ImportedSrp, - createdAt: '00000002-0000-1000-8000-000000000002', - }); - - expect(SecretMetadata.compare(earlier, later, 'asc')).toBeLessThan(0); - expect(SecretMetadata.compare(later, earlier, 'asc')).toBeGreaterThan( - 0, - ); - expect(SecretMetadata.compare(earlier, later, 'desc')).toBeGreaterThan( - 0, - ); - }); - - it('should fall back to timestamp when both have null createdAt', () => { - const earlier = new SecretMetadata(MOCK_SEED_PHRASE, { - timestamp: 1000, - dataType: EncAccountDataType.ImportedSrp, - }); - const later = new SecretMetadata(MOCK_SEED_PHRASE, { - timestamp: 2000, - dataType: EncAccountDataType.ImportedSrp, - }); - - expect(SecretMetadata.compare(earlier, later, 'asc')).toBeLessThan(0); - expect(SecretMetadata.compare(later, earlier, 'asc')).toBeGreaterThan( - 0, - ); - expect(SecretMetadata.compare(earlier, later, 'desc')).toBeGreaterThan( - 0, - ); - }); - - it('should use asc order by default', () => { - const earlier = new SecretMetadata(MOCK_SEED_PHRASE, { - timestamp: 1000, - dataType: EncAccountDataType.ImportedSrp, - }); - const later = new SecretMetadata(MOCK_SEED_PHRASE, { - timestamp: 2000, - dataType: EncAccountDataType.ImportedSrp, - }); - - expect(SecretMetadata.compare(earlier, later)).toBeLessThan(0); - }); + it('treats an unrecognized persisted phase as in-sync', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + withMockAuthPubKey: true, + passwordChangePhase: + 'unrecognized' as unknown as SeedlessPasswordChangePhase, + }), + }, + async ({ controller }) => { + const result = await controller.reconcilePassword({ + globalPassword: NEW_PASSWORD, + }); + expect(result).toBe(PasswordSyncStatus.InSync); + }, + ); }); - it('should default type to Mnemonic when parsing metadata without type field', () => { - // Create raw metadata JSON without type field - const rawMetadataWithoutType = JSON.stringify({ - data: bytesToBase64(MOCK_SEED_PHRASE), - timestamp: Date.now(), - }); - const rawMetadataBytes = stringToBytes(rawMetadataWithoutType); + it('syncs the Seedless side and advances to Keyring reconciliation when no phase is set and the remote password is outdated', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + withMockAuthPubKey: true, + }), + }, + async ({ controller, toprfClient, baseMessenger }) => { + await newUserSetup( + toprfClient, + controller, + baseMessenger, + OLD_PASSWORD, + ); - const parsed = SecretMetadata.fromRawMetadata(rawMetadataBytes, {}); - expect(parsed.type).toBe(SecretType.Mnemonic); - expect(parsed.data).toStrictEqual(MOCK_SEED_PHRASE); - }); + // Remote auth pub key differs from the local one -> outdated, so + // the no-phase branch re-checks and runs the password-sync flow. + mockFetchAuthPubKey( + toprfClient, + base64ToBytes(MOCK_AUTH_PUB_KEY_OUTDATED), + ); - it('should be able to overwrite the default Generic DataType', () => { - const secret1 = new SecretMetadata('private-key-1', { - type: SecretType.PrivateKey, - }); - expect(secret1.data).toBe('private-key-1'); - expect(secret1.type).toBe(SecretType.PrivateKey); + // Mock the password-sync flow for the new password. recoverEncKey + // is called by both chain unlock and local vault rewrite. + const mockToprfEncryptor = createMockToprfEncryptor(); + const encKey = mockToprfEncryptor.deriveEncKey(NEW_PASSWORD); + const pwEncKey = mockToprfEncryptor.derivePwEncKey(NEW_PASSWORD); + const authKeyPair = + mockToprfEncryptor.deriveAuthKeyPair(NEW_PASSWORD); + jest.spyOn(toprfClient, 'recoverEncKey').mockResolvedValue({ + encKey, + authKeyPair, + pwEncKey, + rateLimitResetResult: Promise.resolve(), + keyShareIndex: 1, + }); + // recoverPwEncKey recovers the vault key the existing vault was + // encrypted with, so it must return the OLD password's pwEncKey. + jest.spyOn(toprfClient, 'recoverPwEncKey').mockResolvedValueOnce({ + pwEncKey: mockToprfEncryptor.derivePwEncKey(OLD_PASSWORD), + }); - // should be able to convert to bytes - const secret1Bytes = secret1.toBytes(); - const parsedSecret1 = SecretMetadata.fromRawMetadata( - secret1Bytes, - {}, - ); - expect(parsedSecret1.data).toBe('private-key-1'); - expect(parsedSecret1.type).toBe(SecretType.PrivateKey); + const observedPhases: SeedlessPasswordChangePhase[] = []; + let localKeyringPendingStateUpdates = 0; + let previousPhase = controller.state.passwordChangePhase; + baseMessenger.subscribe( + 'SeedlessOnboardingController:stateChange', + (state) => { + if ( + state.passwordChangePhase === + SeedlessPasswordChangePhase.LocalKeyringPending + ) { + localKeyringPendingStateUpdates += 1; + } + if ( + state.passwordChangePhase !== undefined && + state.passwordChangePhase !== previousPhase + ) { + observedPhases.push(state.passwordChangePhase); + } + previousPhase = state.passwordChangePhase; + }, + ); - const secret2 = new SecretMetadata(MOCK_SEED_PHRASE, { - type: SecretType.Mnemonic, - }); - expect(secret2.data).toStrictEqual(MOCK_SEED_PHRASE); - expect(secret2.type).toBe(SecretType.Mnemonic); + const result = await controller.reconcilePassword({ + globalPassword: NEW_PASSWORD, + }); - const secret2Bytes = secret2.toBytes(); - const parsedSecret2 = SecretMetadata.fromRawMetadata( - secret2Bytes, - {}, + expect(result).toBe(PasswordSyncStatus.ReconcileKeyring); + expect(observedPhases).toStrictEqual([ + SeedlessPasswordChangePhase.SeedlessCommitted, + SeedlessPasswordChangePhase.LocalKeyringPending, + ]); + expect(localKeyringPendingStateUpdates).toBe(1); + // Another-device recovery must continue through the local Keyring + // reconciliation boundary after the Seedless side is synchronized. + expect(controller.state.passwordChangePhase).toBe( + SeedlessPasswordChangePhase.LocalKeyringPending, + ); + // Vault rewrite must re-wrap the stored Keyring encryption key so + // the old-Keyring branch can load it after sync. + expect(await controller.loadKeyringEncryptionKey()).toBe( + MOCK_KEYRING_ENCRYPTION_KEY, + ); + }, ); - expect(parsedSecret2.data).toStrictEqual(MOCK_SEED_PHRASE); - expect(parsedSecret2.type).toBe(SecretType.Mnemonic); - }); - - it('should be able to parse the array of Mixed SecretMetadata', () => { - const mockPrivKeyString = '0xdeadbeef'; - const secret1 = new SecretMetadata(mockPrivKeyString, { - type: SecretType.PrivateKey, - }); - const secret2 = new SecretMetadata(MOCK_SEED_PHRASE, { - type: SecretType.Mnemonic, - }); - - const secrets = [secret1.toBytes(), secret2.toBytes()]; - - const parsedSecrets = secrets - .map((secret) => SecretMetadata.fromRawMetadata(secret, {})) - .sort((a, b) => SecretMetadata.compareByTimestamp(a, b, 'asc')); - expect(parsedSecrets).toHaveLength(2); - expect(parsedSecrets[0].data).toBe(mockPrivKeyString); - expect(parsedSecrets[0].type).toBe(SecretType.PrivateKey); - expect(parsedSecrets[1].data).toStrictEqual(MOCK_SEED_PHRASE); - expect(parsedSecrets[1].type).toBe(SecretType.Mnemonic); }); - it('should be able to filter the array of SecretMetadata by type', () => { - const mockPrivKeyString = '0xdeadbeef'; - const secret1 = new SecretMetadata(mockPrivKeyString, { - type: SecretType.PrivateKey, - }); - const secret2 = new SecretMetadata(MOCK_SEED_PHRASE, { - type: SecretType.Mnemonic, - }); - const secret3 = new SecretMetadata(MOCK_SEED_PHRASE); - - const secrets = [secret1.toBytes(), secret2.toBytes(), secret3.toBytes()]; - - const allSecrets = secrets - .map((secret) => SecretMetadata.fromRawMetadata(secret, {})) - .sort((a, b) => SecretMetadata.compareByTimestamp(a, b, 'asc')); + it('returns unknown when the no-phase outdated check fails', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + withMockAuthPubKey: true, + }), + }, + async ({ toprfClient, controller }) => { + jest + .spyOn(toprfClient, 'fetchAuthPubKey') + .mockRejectedValueOnce(new Error('Network error')); - const mnemonicSecrets = allSecrets.filter((secret) => - SecretMetadata.matchesType(secret, SecretType.Mnemonic), - ); - expect(mnemonicSecrets).toHaveLength(2); - expect(mnemonicSecrets[0].data).toStrictEqual(MOCK_SEED_PHRASE); - expect(mnemonicSecrets[0].type).toBe(SecretType.Mnemonic); - expect(mnemonicSecrets[1].data).toStrictEqual(MOCK_SEED_PHRASE); - expect(mnemonicSecrets[1].type).toBe(SecretType.Mnemonic); + const result = await controller.reconcilePassword({ + globalPassword: NEW_PASSWORD, + }); - const privateKeySecrets = allSecrets.filter((secret) => - SecretMetadata.matchesType(secret, SecretType.PrivateKey), + expect(result).toBe(PasswordSyncStatus.Unknown); + }, ); - - expect(privateKeySecrets).toHaveLength(1); - expect(privateKeySecrets[0].data).toBe(mockPrivKeyString); - expect(privateKeySecrets[0].type).toBe(SecretType.PrivateKey); }); - it('should derive type from dataType (V2)', () => { - const srp1 = new SecretMetadata(MOCK_SEED_PHRASE, { - dataType: EncAccountDataType.PrimarySrp, - }); - expect(srp1.type).toBe(SecretType.Mnemonic); - expect(srp1.dataType).toBe(EncAccountDataType.PrimarySrp); + it('reconciles the Seedless side and advances to LOCAL_KEYRING_PENDING when SEEDLESS_COMMITTED', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + withMockAuthPubKey: true, + passwordChangePhase: SeedlessPasswordChangePhase.SeedlessCommitted, + }), + }, + async ({ controller, toprfClient, baseMessenger }) => { + await newUserSetup( + toprfClient, + controller, + baseMessenger, + OLD_PASSWORD, + ); - const srp2 = new SecretMetadata(MOCK_SEED_PHRASE, { - dataType: EncAccountDataType.ImportedSrp, - }); - expect(srp2.type).toBe(SecretType.Mnemonic); - expect(srp2.dataType).toBe(EncAccountDataType.ImportedSrp); + // Mock the password-sync flow for the new password. recoverEncKey + // is called by both chain unlock and local vault rewrite. + const mockToprfEncryptor = createMockToprfEncryptor(); + const encKey = mockToprfEncryptor.deriveEncKey(NEW_PASSWORD); + const pwEncKey = mockToprfEncryptor.derivePwEncKey(NEW_PASSWORD); + const authKeyPair = + mockToprfEncryptor.deriveAuthKeyPair(NEW_PASSWORD); + jest.spyOn(toprfClient, 'recoverEncKey').mockResolvedValue({ + encKey, + authKeyPair, + pwEncKey, + rateLimitResetResult: Promise.resolve(), + keyShareIndex: 1, + }); + // recoverPwEncKey recovers the vault key the existing vault was + // encrypted with, so it must return the OLD password's pwEncKey. + jest.spyOn(toprfClient, 'recoverPwEncKey').mockResolvedValueOnce({ + pwEncKey: mockToprfEncryptor.derivePwEncKey(OLD_PASSWORD), + }); - const pk = new SecretMetadata('0xdeadbeef', { - dataType: EncAccountDataType.ImportedPrivateKey, - }); - expect(pk.type).toBe(SecretType.PrivateKey); - expect(pk.dataType).toBe(EncAccountDataType.ImportedPrivateKey); - }); + let localKeyringPendingStateUpdates = 0; + baseMessenger.subscribe( + 'SeedlessOnboardingController:stateChange', + (state) => { + if ( + state.passwordChangePhase === + SeedlessPasswordChangePhase.LocalKeyringPending + ) { + localKeyringPendingStateUpdates += 1; + } + }, + ); - it('should be able to create SecretMetadata with storage metadata', () => { - const secretMetadata = new SecretMetadata(MOCK_SEED_PHRASE, { - dataType: EncAccountDataType.PrimarySrp, - itemId: 'test-item-id', - createdAt: '00000001-0000-1000-8000-000000000001', - }); + const result = await controller.reconcilePassword({ + globalPassword: NEW_PASSWORD, + }); - expect(secretMetadata.data).toStrictEqual(MOCK_SEED_PHRASE); - expect(secretMetadata.type).toBe(SecretType.Mnemonic); - expect(secretMetadata.itemId).toBe('test-item-id'); - expect(secretMetadata.dataType).toBe(EncAccountDataType.PrimarySrp); - expect(secretMetadata.createdAt).toBe( - '00000001-0000-1000-8000-000000000001', + expect(result).toBe(PasswordSyncStatus.ReconcileKeyring); + expect(controller.state.passwordChangePhase).toBe( + SeedlessPasswordChangePhase.LocalKeyringPending, + ); + expect(localKeyringPendingStateUpdates).toBe(1); + expect(await controller.loadKeyringEncryptionKey()).toBe( + MOCK_KEYRING_ENCRYPTION_KEY, + ); + }, ); }); - it('should have undefined storage metadata when not provided', () => { - const secretMetadata = new SecretMetadata(MOCK_SEED_PHRASE); - - expect(secretMetadata.data).toStrictEqual(MOCK_SEED_PHRASE); - expect(secretMetadata.itemId).toBeUndefined(); - expect(secretMetadata.dataType).toBeUndefined(); - expect(secretMetadata.createdAt).toBeUndefined(); - }); + it('persists recovery state atomically so an interrupted rewrite can be retried', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + withMockAuthPubKey: true, + passwordChangePhase: SeedlessPasswordChangePhase.SeedlessCommitted, + }), + }, + async ({ controller, toprfClient, baseMessenger }) => { + await newUserSetup( + toprfClient, + controller, + baseMessenger, + OLD_PASSWORD, + ); - it('should NOT serialize storage metadata in toBytes()', () => { - const secretMetadata = new SecretMetadata(MOCK_SEED_PHRASE, { - dataType: EncAccountDataType.PrimarySrp, - itemId: 'test-item-id', - createdAt: '00000001-0000-1000-8000-000000000001', - }); + const vaultBeforeSync = controller.state.vault; + const oldEncryptedKeyringEncryptionKey = + controller.state.encryptedKeyringEncryptionKey; - const serializedBytes = secretMetadata.toBytes(); - const serializedString = bytesToString(serializedBytes); - const parsed = JSON.parse(serializedString); + const mockToprfEncryptor = createMockToprfEncryptor(); + const newEncKey = mockToprfEncryptor.deriveEncKey(NEW_PASSWORD); + const newPwEncKey = mockToprfEncryptor.derivePwEncKey(NEW_PASSWORD); + const newAuthKeyPair = + mockToprfEncryptor.deriveAuthKeyPair(NEW_PASSWORD); + jest.spyOn(toprfClient, 'recoverEncKey').mockResolvedValue({ + encKey: newEncKey, + authKeyPair: newAuthKeyPair, + pwEncKey: newPwEncKey, + rateLimitResetResult: Promise.resolve(), + keyShareIndex: 1, + }); + jest + .spyOn(toprfClient, 'recoverPwEncKey') + .mockResolvedValueOnce({ + // The first attempt unlocks the old local vault. + pwEncKey: mockToprfEncryptor.derivePwEncKey(OLD_PASSWORD), + }) + .mockResolvedValue({ + // A retry after the rewrite unlocks the new local vault. + pwEncKey: newPwEncKey, + }); - // Storage metadata should NOT be in serialized data - expect(parsed.itemId).toBeUndefined(); - expect(parsed.dataType).toBeUndefined(); - expect(parsed.createdAt).toBeUndefined(); + let interruptPersistence = true; + const controllerWithUpdate = controller as unknown as { + update: (...args: unknown[]) => unknown; + }; + const originalUpdate = controllerWithUpdate.update.bind(controller); + jest + .spyOn(controllerWithUpdate, 'update') + .mockImplementation((...args: unknown[]) => { + const result = originalUpdate(...args) as { + nextState: SeedlessOnboardingControllerState; + }; + if ( + interruptPersistence && + result.nextState.vault !== vaultBeforeSync + ) { + interruptPersistence = false; + throw new Error('simulated persistence interruption'); + } + return result; + }); - // Only encrypted metadata should be present - expect(parsed.data).toBeDefined(); - expect(parsed.timestamp).toBeDefined(); - expect(parsed.type).toBe(SecretType.Mnemonic); - }); + expect( + await controller.reconcilePassword({ + globalPassword: NEW_PASSWORD, + }), + ).toBe(PasswordSyncStatus.Unknown); - it('should be able to parse raw metadata with storage metadata', () => { - const originalMetadata = new SecretMetadata(MOCK_SEED_PHRASE, { - type: SecretType.Mnemonic, - }); - const serializedBytes = originalMetadata.toBytes(); + // The rewritten vault must never be persisted without its matching + // auth key, Keyring ciphertext, and recovery phase. + expect(controller.state.authPubKey).toBe( + bytesToBase64(newAuthKeyPair.pk), + ); + expect(controller.state.encryptedKeyringEncryptionKey).not.toBe( + oldEncryptedKeyringEncryptionKey, + ); + expect(controller.state.passwordChangePhase).toBe( + SeedlessPasswordChangePhase.LocalKeyringPending, + ); + const recoveredKeyringEncryptionKey = managedNonce(gcm)( + newPwEncKey, + ).decrypt( + base64ToBytes( + controller.state.encryptedKeyringEncryptionKey as string, + ), + ); + expect(bytesToString(recoveredKeyringEncryptionKey)).toBe( + MOCK_KEYRING_ENCRYPTION_KEY, + ); - const parsedMetadata = SecretMetadata.fromRawMetadata(serializedBytes, { - itemId: 'server-assigned-id', - dataType: EncAccountDataType.ImportedSrp, - createdAt: '00000002-0000-1000-8000-000000000002', - }); + // Simulate a restart so the retry cannot use a stale decrypted-vault + // cache from before the interrupted rewrite. + await controller.setLocked(); - expect(parsedMetadata.data).toStrictEqual(MOCK_SEED_PHRASE); - expect(parsedMetadata.type).toBe(SecretType.Mnemonic); - expect(parsedMetadata.itemId).toBe('server-assigned-id'); - expect(parsedMetadata.dataType).toBe(EncAccountDataType.ImportedSrp); - expect(parsedMetadata.createdAt).toBe( - '00000002-0000-1000-8000-000000000002', + expect( + await controller.reconcilePassword({ + globalPassword: NEW_PASSWORD, + }), + ).toBe(PasswordSyncStatus.ReconcileKeyring); + expect(await controller.loadKeyringEncryptionKey()).toBe( + MOCK_KEYRING_ENCRYPTION_KEY, + ); + }, ); }); - }); - - describe('store and recover keyring encryption key', () => { - const GLOBAL_PASSWORD = 'global-password'; - const RECOVERED_PASSWORD = 'recovered-password'; - it('should store and recover keyring encryption key', async () => { + it('reconciles the Seedless side when no Keyring encryption key is stored', async () => { await withController( { state: getMockInitialControllerState({ withMockAuthenticatedUser: true, withMockAuthPubKey: true, + passwordChangePhase: SeedlessPasswordChangePhase.SeedlessCommitted, }), }, async ({ controller, toprfClient, baseMessenger }) => { - // Setup and store keyring encryption key. await mockCreateToprfKeyAndBackupSeedPhrase( toprfClient, controller, baseMessenger, - RECOVERED_PASSWORD, + OLD_PASSWORD, MOCK_SEED_PHRASE, MOCK_KEYRING_ID, ); - await baseMessenger.call( - 'SeedlessOnboardingController:storeKeyringEncryptionKey', - MOCK_KEYRING_ENCRYPTION_KEY, - ); - - // Mock recoverEncKey for the global password const mockToprfEncryptor = createMockToprfEncryptor(); - const encKey = mockToprfEncryptor.deriveEncKey(GLOBAL_PASSWORD); - const pwEncKey = mockToprfEncryptor.derivePwEncKey(GLOBAL_PASSWORD); - const authKeyPair = - mockToprfEncryptor.deriveAuthKeyPair(GLOBAL_PASSWORD); - jest.spyOn(toprfClient, 'recoverEncKey').mockResolvedValueOnce({ - encKey, - authKeyPair, - pwEncKey, + jest.spyOn(toprfClient, 'recoverEncKey').mockResolvedValue({ + encKey: mockToprfEncryptor.deriveEncKey(NEW_PASSWORD), + authKeyPair: mockToprfEncryptor.deriveAuthKeyPair(NEW_PASSWORD), + pwEncKey: mockToprfEncryptor.derivePwEncKey(NEW_PASSWORD), rateLimitResetResult: Promise.resolve(), keyShareIndex: 1, }); - - // Mock toprfClient.recoverPassword - const recoveredPwEncKey = - mockToprfEncryptor.derivePwEncKey(RECOVERED_PASSWORD); jest.spyOn(toprfClient, 'recoverPwEncKey').mockResolvedValueOnce({ - pwEncKey: recoveredPwEncKey, + pwEncKey: mockToprfEncryptor.derivePwEncKey(OLD_PASSWORD), }); - await baseMessenger.call('SeedlessOnboardingController:setLocked'); + expect( + await controller.reconcilePassword({ + globalPassword: NEW_PASSWORD, + }), + ).toBe(PasswordSyncStatus.ReconcileKeyring); + expect( + controller.state.encryptedKeyringEncryptionKey, + ).toBeUndefined(); + }, + ); + }); - await baseMessenger.call( - 'SeedlessOnboardingController:submitGlobalPassword', - { - globalPassword: GLOBAL_PASSWORD, - }, + it('returns unknown and preserves the phase when reconciliation fails', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + withMockAuthPubKey: true, + passwordChangePhase: SeedlessPasswordChangePhase.SeedlessCommitted, + }), + }, + async ({ controller, toprfClient, baseMessenger }) => { + await mockCreateToprfKeyAndBackupSeedPhrase( + toprfClient, + controller, + baseMessenger, + OLD_PASSWORD, + MOCK_SEED_PHRASE, + MOCK_KEYRING_ID, ); - const keyringEncryptionKey = await baseMessenger.call( - 'SeedlessOnboardingController:loadKeyringEncryptionKey', - ); + // Make the password-sync flow fail. + jest + .spyOn(toprfClient, 'recoverPwEncKey') + .mockRejectedValueOnce(new Error('recover failed')); - expect(keyringEncryptionKey).toStrictEqual( - MOCK_KEYRING_ENCRYPTION_KEY, + const result = await controller.reconcilePassword({ + globalPassword: NEW_PASSWORD, + }); + + expect(result).toBe(PasswordSyncStatus.Unknown); + // The phase is preserved as the recovery signal. + expect(controller.state.passwordChangePhase).toBe( + SeedlessPasswordChangePhase.SeedlessCommitted, ); - expect(toprfClient.recoverEncKey).toHaveBeenCalled(); - expect(toprfClient.recoverPwEncKey).toHaveBeenCalled(); }, ); }); - it('should throw if key not set', async () => { + it('returns unknown when the password-key chain limit is exceeded', async () => { await withController( { state: getMockInitialControllerState({ withMockAuthenticatedUser: true, withMockAuthPubKey: true, - vault: 'mock-vault', + passwordChangePhase: SeedlessPasswordChangePhase.SeedlessCommitted, }), }, async ({ controller, toprfClient, baseMessenger }) => { - await expect( - baseMessenger.call( - 'SeedlessOnboardingController:storeKeyringEncryptionKey', - '', - ), - ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.WrongPasswordType, - ); - - // Setup and store keyring encryption key. await mockCreateToprfKeyAndBackupSeedPhrase( toprfClient, controller, baseMessenger, - RECOVERED_PASSWORD, + OLD_PASSWORD, MOCK_SEED_PHRASE, MOCK_KEYRING_ID, ); + mockRecoverEncKey(toprfClient, NEW_PASSWORD); + jest + .spyOn(toprfClient, 'recoverPwEncKey') + .mockRejectedValueOnce( + new TOPRFError( + TOPRFErrorCode.MaxKeyChainLengthExceeded, + 'Max key chain length exceeded', + ), + ); - await expect( - baseMessenger.call( - 'SeedlessOnboardingController:loadKeyringEncryptionKey', - ), - ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.EncryptedKeyringEncryptionKeyNotSet, + expect( + await controller.reconcilePassword({ + globalPassword: NEW_PASSWORD, + }), + ).toBe(PasswordSyncStatus.Unknown); + expect(controller.state.passwordChangePhase).toBe( + SeedlessPasswordChangePhase.SeedlessCommitted, ); }, ); }); - it('should store and load keyring encryption key', async () => { + it('returns unknown when recovered vault credentials are stale', async () => { await withController( { state: getMockInitialControllerState({ withMockAuthenticatedUser: true, withMockAuthPubKey: true, + passwordChangePhase: SeedlessPasswordChangePhase.SeedlessCommitted, }), }, async ({ controller, toprfClient, baseMessenger }) => { - // Setup and store keyring encryption key. await mockCreateToprfKeyAndBackupSeedPhrase( toprfClient, controller, baseMessenger, - RECOVERED_PASSWORD, + OLD_PASSWORD, MOCK_SEED_PHRASE, MOCK_KEYRING_ID, ); + const staleState = { + ...controller.state, + vaultEncryptionSalt: 'stale-salt', + }; + await controller.setLocked(); + ( + controller as unknown as { + update: ( + callback: (state: SeedlessOnboardingControllerState) => void, + ) => void; + } + ).update((state) => { + state.vaultEncryptionSalt = staleState.vaultEncryptionSalt; + }); - await baseMessenger.call( - 'SeedlessOnboardingController:storeKeyringEncryptionKey', - MOCK_KEYRING_ENCRYPTION_KEY, - ); + mockRecoverEncKey(toprfClient, NEW_PASSWORD); + jest.spyOn(toprfClient, 'recoverPwEncKey').mockResolvedValueOnce({ + pwEncKey: createMockToprfEncryptor().derivePwEncKey(OLD_PASSWORD), + }); - const result = await baseMessenger.call( - 'SeedlessOnboardingController:loadKeyringEncryptionKey', - ); - expect(result).toStrictEqual(MOCK_KEYRING_ENCRYPTION_KEY); + expect( + await controller.reconcilePassword({ + globalPassword: NEW_PASSWORD, + }), + ).toBe(PasswordSyncStatus.Unknown); }, ); }); - it('should load keyring encryption key after change password', async () => { + it('returns unknown when the stored Seedless encryption key is missing', async () => { await withController( { state: getMockInitialControllerState({ withMockAuthenticatedUser: true, withMockAuthPubKey: true, + passwordChangePhase: SeedlessPasswordChangePhase.SeedlessCommitted, }), }, async ({ controller, toprfClient, baseMessenger }) => { - // Setup and store keyring encryption key. await mockCreateToprfKeyAndBackupSeedPhrase( toprfClient, controller, baseMessenger, - RECOVERED_PASSWORD, + OLD_PASSWORD, MOCK_SEED_PHRASE, MOCK_KEYRING_ID, ); + await controller.setLocked(); + ( + controller as unknown as { + update: ( + callback: (state: SeedlessOnboardingControllerState) => void, + ) => void; + } + ).update((state) => { + delete state.encryptedSeedlessEncryptionKey; + }); + mockRecoverEncKey(toprfClient, NEW_PASSWORD); + jest.spyOn(toprfClient, 'recoverPwEncKey').mockResolvedValueOnce({ + pwEncKey: createMockToprfEncryptor().derivePwEncKey(OLD_PASSWORD), + }); - await baseMessenger.call( - 'SeedlessOnboardingController:storeKeyringEncryptionKey', - MOCK_KEYRING_ENCRYPTION_KEY, - ); + expect( + await controller.reconcilePassword({ + globalPassword: NEW_PASSWORD, + }), + ).toBe(PasswordSyncStatus.Unknown); + }, + ); + }); - await mockChangePassword( - controller, + it('returns unknown when the chain unlock reports a TOPRF error', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + withMockAuthPubKey: true, + passwordChangePhase: SeedlessPasswordChangePhase.SeedlessCommitted, + }), + }, + async ({ controller, toprfClient, baseMessenger }) => { + await mockCreateToprfKeyAndBackupSeedPhrase( toprfClient, - RECOVERED_PASSWORD, - GLOBAL_PASSWORD, - ); - - await baseMessenger.call( - 'SeedlessOnboardingController:changePassword', - GLOBAL_PASSWORD, - RECOVERED_PASSWORD, - ); - - const result = await baseMessenger.call( - 'SeedlessOnboardingController:loadKeyringEncryptionKey', + controller, + baseMessenger, + OLD_PASSWORD, + MOCK_SEED_PHRASE, + MOCK_KEYRING_ID, ); + mockRecoverEncKey(toprfClient, NEW_PASSWORD); + jest + .spyOn(toprfClient, 'recoverPwEncKey') + .mockRejectedValueOnce( + new TOPRFError( + TOPRFErrorCode.CouldNotFetchPassword, + 'Could not fetch password', + ), + ); - expect(result).toStrictEqual(MOCK_KEYRING_ENCRYPTION_KEY); + expect( + await controller.reconcilePassword({ + globalPassword: NEW_PASSWORD, + }), + ).toBe(PasswordSyncStatus.Unknown); }, ); }); - it('should recover keyring encryption key after change password', async () => { + it('returns unknown when token refresh fails during chain unlock', async () => { await withController( { state: getMockInitialControllerState({ withMockAuthenticatedUser: true, withMockAuthPubKey: true, + passwordChangePhase: SeedlessPasswordChangePhase.SeedlessCommitted, }), }, - async ({ controller, toprfClient, baseMessenger }) => { - // Setup and store keyring encryption key. + async ({ + controller, + toprfClient, + baseMessenger, + mockRefreshJWTToken, + }) => { await mockCreateToprfKeyAndBackupSeedPhrase( toprfClient, controller, baseMessenger, - RECOVERED_PASSWORD, + OLD_PASSWORD, MOCK_SEED_PHRASE, MOCK_KEYRING_ID, ); - - await baseMessenger.call( - 'SeedlessOnboardingController:storeKeyringEncryptionKey', - MOCK_KEYRING_ENCRYPTION_KEY, + mockRecoverEncKey(toprfClient, NEW_PASSWORD); + jest + .spyOn(toprfClient, 'recoverPwEncKey') + .mockRejectedValueOnce( + new TOPRFError( + TOPRFErrorCode.AuthTokenExpired, + 'Auth token expired', + ), + ); + mockRefreshJWTToken.mockRejectedValueOnce( + new Error('Failed to refresh token'), ); - await mockChangePassword( - controller, + expect( + await controller.reconcilePassword({ + globalPassword: NEW_PASSWORD, + }), + ).toBe(PasswordSyncStatus.Unknown); + }, + ); + }); + }); + + describe('password-change lifecycle neutrality of key storage', () => { + const MOCK_PASSWORD = 'mock-password'; + + it('storeKeyringEncryptionKey does not change the lifecycle phase', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + withMockAuthPubKey: true, + passwordChangePhase: + SeedlessPasswordChangePhase.LocalKeyringPending, + }), + }, + async ({ controller, toprfClient, baseMessenger }) => { + await newUserSetup( toprfClient, - RECOVERED_PASSWORD, - GLOBAL_PASSWORD, + controller, + baseMessenger, + MOCK_PASSWORD, ); - await baseMessenger.call( - 'SeedlessOnboardingController:changePassword', - GLOBAL_PASSWORD, - RECOVERED_PASSWORD, + // The shared key-storage operation must remain lifecycle-neutral + // so it cannot advance or regress recovery when called by the client. + expect(controller.state.passwordChangePhase).toBe( + SeedlessPasswordChangePhase.LocalKeyringPending, ); + }, + ); + }); - // Mock recoverEncKey for the global password - const mockToprfEncryptor = createMockToprfEncryptor(); - const encKey = mockToprfEncryptor.deriveEncKey(GLOBAL_PASSWORD); - const pwEncKey = mockToprfEncryptor.derivePwEncKey(GLOBAL_PASSWORD); - const authKeyPair = - mockToprfEncryptor.deriveAuthKeyPair(GLOBAL_PASSWORD); - jest.spyOn(toprfClient, 'recoverEncKey').mockResolvedValueOnce({ - encKey, - pwEncKey, - authKeyPair, - rateLimitResetResult: Promise.resolve(), - keyShareIndex: 1, - }); - - // Mock toprfClient.recoverPwEncKey - const recoveredPwEncKey = - mockToprfEncryptor.derivePwEncKey(GLOBAL_PASSWORD); - jest.spyOn(toprfClient, 'recoverPwEncKey').mockResolvedValueOnce({ - pwEncKey: recoveredPwEncKey, - }); + it('loadKeyringEncryptionKey does not change the lifecycle phase', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + withMockAuthPubKey: true, + passwordChangePhase: + SeedlessPasswordChangePhase.LocalKeyringPending, + }), + }, + async ({ controller, toprfClient, baseMessenger }) => { + await newUserSetup( + toprfClient, + controller, + baseMessenger, + MOCK_PASSWORD, + ); - await baseMessenger.call('SeedlessOnboardingController:setLocked'); + const loaded = await baseMessenger.call( + 'SeedlessOnboardingController:loadKeyringEncryptionKey', + ); - await baseMessenger.call( - 'SeedlessOnboardingController:submitGlobalPassword', - { - globalPassword: GLOBAL_PASSWORD, - }, + expect(loaded).toStrictEqual(MOCK_KEYRING_ENCRYPTION_KEY); + // Loading a key is read-only with respect to lifecycle state. + expect(controller.state.passwordChangePhase).toBe( + SeedlessPasswordChangePhase.LocalKeyringPending, ); + }, + ); + }); + }); + + describe('clearState', () => { + it('should clear the state', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + }), + }, + async ({ controller, baseMessenger }) => { + const { state } = controller; - const keyringEncryptionKey = await baseMessenger.call( - 'SeedlessOnboardingController:loadKeyringEncryptionKey', - ); + expect(state.nodeAuthTokens).toBeDefined(); + expect(state.userId).toBeDefined(); + expect(state.authConnectionId).toBeDefined(); - expect(keyringEncryptionKey).toStrictEqual( - MOCK_KEYRING_ENCRYPTION_KEY, + baseMessenger.call('SeedlessOnboardingController:clearState'); + expect(controller.state).toStrictEqual( + getInitialSeedlessOnboardingControllerStateWithDefaults(), ); }, ); }); + }); - it('should throw if encryptedKeyringEncryptionKey not set', async () => { + describe('vault', () => { + const MOCK_PASSWORD = 'mock-password'; + + it('should throw an error if the password is an empty string', async () => { await withController( { state: getMockInitialControllerState({ withMockAuthenticatedUser: true, - withMockAuthPubKey: true, }), }, async ({ toprfClient, baseMessenger }) => { - // Mock recoverEncKey for the global password - const mockToprfEncryptor = createMockToprfEncryptor(); - const encKey = mockToprfEncryptor.deriveEncKey(GLOBAL_PASSWORD); - const pwEncKey = mockToprfEncryptor.derivePwEncKey(GLOBAL_PASSWORD); - const authKeyPair = - mockToprfEncryptor.deriveAuthKeyPair(GLOBAL_PASSWORD); - jest.spyOn(toprfClient, 'recoverEncKey').mockResolvedValueOnce({ - encKey, - pwEncKey, - authKeyPair, - rateLimitResetResult: Promise.resolve(), - keyShareIndex: 1, - }); - - // Mock toprfClient.recoverPwEncKey - const recoveredPwEncKey = - mockToprfEncryptor.derivePwEncKey(RECOVERED_PASSWORD); - jest.spyOn(toprfClient, 'recoverPwEncKey').mockResolvedValueOnce({ - pwEncKey: recoveredPwEncKey, - }); - + // create the local enc key + mockcreateLocalKey(toprfClient, MOCK_PASSWORD); + // persist the local enc key + jest.spyOn(toprfClient, 'persistLocalKey').mockResolvedValueOnce(); + // mock the secret data add + const mockSecretDataAdd = handleMockSecretDataAdd(); await expect( baseMessenger.call( - 'SeedlessOnboardingController:submitGlobalPassword', - { - globalPassword: GLOBAL_PASSWORD, - }, + 'SeedlessOnboardingController:createToprfKeyAndBackupSeedPhrase', + '', + MOCK_SEED_PHRASE, + MOCK_KEYRING_ID, ), ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.CouldNotRecoverPassword, + SeedlessOnboardingControllerErrorMessage.InvalidEmptyPassword, ); + + expect(mockSecretDataAdd.isDone()).toBe(true); }, ); }); - it('should throw SRPNotBackedUpError if no authPubKey in state', async () => { + it('should throw an error if the passowrd is of wrong type', async () => { await withController( { - state: getMockInitialControllerState({}), + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + }), }, - async ({ baseMessenger }) => { + async ({ toprfClient, baseMessenger }) => { + // create the local enc key + mockcreateLocalKey(toprfClient, MOCK_PASSWORD); + // persist the local enc key + jest.spyOn(toprfClient, 'persistLocalKey').mockResolvedValueOnce(); + // mock the secret data add + const mockSecretDataAdd = handleMockSecretDataAdd(); await expect( baseMessenger.call( - 'SeedlessOnboardingController:submitGlobalPassword', - { - globalPassword: GLOBAL_PASSWORD, - }, + 'SeedlessOnboardingController:createToprfKeyAndBackupSeedPhrase', + // @ts-expect-error Intentionally passing wrong password type + 123, + MOCK_SEED_PHRASE, + 'MOCK_KEYRING_ID', ), ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.SRPNotBackedUpError, + SeedlessOnboardingControllerErrorMessage.WrongPasswordType, ); + + expect(mockSecretDataAdd.isDone()).toBe(true); }, ); }); + }); + + describe('lock', () => { + const MOCK_PASSWORD = 'mock-password'; + + it('should lock the controller', async () => { + const mutexAcquireSpy = jest + .spyOn(Mutex.prototype, 'acquire') + .mockResolvedValueOnce(jest.fn()); - it('should propagate errors from recoverEncKey', async () => { await withController( { state: getMockInitialControllerState({ withMockAuthenticatedUser: true, - withMockAuthPubKey: true, }), }, - async ({ toprfClient, baseMessenger }) => { - jest - .spyOn(toprfClient, 'recoverEncKey') - .mockRejectedValueOnce( - new TOPRFError( - TOPRFErrorCode.CouldNotDeriveEncryptionKey, - 'Could not derive encryption key', - ), - ); + async ({ controller, toprfClient, baseMessenger }) => { + await mockCreateToprfKeyAndBackupSeedPhrase( + toprfClient, + controller, + baseMessenger, + MOCK_PASSWORD, + MOCK_SEED_PHRASE, + MOCK_KEYRING_ID, + ); + + await baseMessenger.call('SeedlessOnboardingController:setLocked'); + + // verify that the mutex acquire was called + expect(mutexAcquireSpy).toHaveBeenCalled(); await expect( baseMessenger.call( - 'SeedlessOnboardingController:submitGlobalPassword', + 'SeedlessOnboardingController:addNewSecretData', + MOCK_SEED_PHRASE, + EncAccountDataType.ImportedSrp, { - globalPassword: GLOBAL_PASSWORD, + keyringId: MOCK_KEYRING_ID, }, ), - ).rejects.toStrictEqual( - new RecoveryError( - SeedlessOnboardingControllerErrorMessage.IncorrectPassword, - ), + ).rejects.toThrow( + SeedlessOnboardingControllerErrorMessage.ControllerLocked, ); }, ); }); + }); - it('should propagate errors from toprfClient.recoverPassword', async () => { - await withController( - { - state: getMockInitialControllerState({ - withMockAuthenticatedUser: true, - withMockAuthPubKey: true, - }), - }, - async ({ toprfClient, baseMessenger }) => { - const mockToprfEncryptor = createMockToprfEncryptor(); - const encKey = mockToprfEncryptor.deriveEncKey(GLOBAL_PASSWORD); - const pwEncKey = mockToprfEncryptor.derivePwEncKey(GLOBAL_PASSWORD); - const authKeyPair = - mockToprfEncryptor.deriveAuthKeyPair(GLOBAL_PASSWORD); - jest.spyOn(toprfClient, 'recoverEncKey').mockResolvedValueOnce({ - encKey, - pwEncKey, - authKeyPair, - rateLimitResetResult: Promise.resolve(), - keyShareIndex: 1, - }); + describe('SeedPhraseMetadata', () => { + it('should be able to create a seed phrase metadata with default options', () => { + // should be able to create a SecretMetadata instance via constructor + const seedPhraseMetadata = new SecretMetadata(MOCK_SEED_PHRASE); + expect(seedPhraseMetadata.data).toBeDefined(); + expect(seedPhraseMetadata.timestamp).toBeDefined(); + expect(seedPhraseMetadata.type).toBe(SecretType.Mnemonic); + // V2 fields should be undefined + expect(seedPhraseMetadata.dataType).toBeUndefined(); + expect(seedPhraseMetadata.itemId).toBeUndefined(); + expect(seedPhraseMetadata.createdAt).toBeUndefined(); + expect(seedPhraseMetadata.storageVersion).toBeUndefined(); - jest - .spyOn(toprfClient, 'recoverPwEncKey') - .mockRejectedValueOnce( - new TOPRFError( - TOPRFErrorCode.CouldNotFetchPassword, - 'Could not fetch password', - ), - ); + // should be able to create a SecretMetadata instance with a timestamp via constructor + const timestamp = 18_000; + const seedPhraseMetadata2 = new SecretMetadata(MOCK_SEED_PHRASE, { + timestamp, + }); + expect(seedPhraseMetadata2.data).toBeDefined(); + expect(seedPhraseMetadata2.timestamp).toBe(timestamp); + expect(seedPhraseMetadata2.data).toStrictEqual(MOCK_SEED_PHRASE); + expect(seedPhraseMetadata2.type).toBe(SecretType.Mnemonic); + expect(seedPhraseMetadata2.dataType).toBeUndefined(); + }); + + it('should be able to add metadata to a seed phrase', () => { + const timestamp = 18_000; + const seedPhraseMetadata = new SecretMetadata(MOCK_SEED_PHRASE, { + type: SecretType.PrivateKey, + timestamp, + }); + expect(seedPhraseMetadata.type).toBe(SecretType.PrivateKey); + expect(seedPhraseMetadata.timestamp).toBe(timestamp); + }); + + it('should be able to serialized and parse a seed phrase metadata', () => { + const seedPhraseMetadata = new SecretMetadata(MOCK_SEED_PHRASE); + const serializedSeedPhraseBytes = seedPhraseMetadata.toBytes(); + + const parsedSeedPhraseMetadata = SecretMetadata.fromRawMetadata( + serializedSeedPhraseBytes, + {}, + ); + expect(parsedSeedPhraseMetadata.data).toBeDefined(); + expect(parsedSeedPhraseMetadata.timestamp).toBeDefined(); + expect(parsedSeedPhraseMetadata.data).toStrictEqual(MOCK_SEED_PHRASE); + }); + + it('should be able to compare seed phrase metadata by timestamp', () => { + const mockSeedPhraseMetadata1 = new SecretMetadata(MOCK_SEED_PHRASE, { + timestamp: 1000, + }); + const mockSeedPhraseMetadata2 = new SecretMetadata(MOCK_SEED_PHRASE, { + timestamp: 2000, + }); + + // ascending order: earlier timestamp first + expect( + SecretMetadata.compareByTimestamp( + mockSeedPhraseMetadata1, + mockSeedPhraseMetadata2, + 'asc', + ), + ).toBeLessThan(0); + + // descending order: later timestamp first + expect( + SecretMetadata.compareByTimestamp( + mockSeedPhraseMetadata1, + mockSeedPhraseMetadata2, + 'desc', + ), + ).toBeGreaterThan(0); + + // default order (no parameter): should use ascending order + expect( + SecretMetadata.compareByTimestamp( + mockSeedPhraseMetadata1, + mockSeedPhraseMetadata2, + ), + ).toBeLessThan(0); + }); + + describe('compare', () => { + it('should sort PrimarySrp first regardless of createdAt or timestamp', () => { + const primarySrp = new SecretMetadata(MOCK_SEED_PHRASE, { + timestamp: 2000, + dataType: EncAccountDataType.PrimarySrp, + createdAt: '00000002-0000-1000-8000-000000000002', + }); + const importedSrp = new SecretMetadata(MOCK_SEED_PHRASE, { + timestamp: 1000, + dataType: EncAccountDataType.ImportedSrp, + createdAt: '00000001-0000-1000-8000-000000000001', + }); + + expect( + SecretMetadata.compare(primarySrp, importedSrp, 'asc'), + ).toBeLessThan(0); + expect( + SecretMetadata.compare(importedSrp, primarySrp, 'asc'), + ).toBeGreaterThan(0); + // Also in desc order + expect( + SecretMetadata.compare(primarySrp, importedSrp, 'desc'), + ).toBeLessThan(0); + }); + + it('should return 0 when both items are PrimarySrp (handles data corruption gracefully)', () => { + const primarySrp1 = new SecretMetadata(MOCK_SEED_PHRASE, { + timestamp: 1000, + dataType: EncAccountDataType.PrimarySrp, + createdAt: '00000001-0000-1000-8000-000000000001', + }); + const primarySrp2 = new SecretMetadata(MOCK_SEED_PHRASE, { + timestamp: 2000, + dataType: EncAccountDataType.PrimarySrp, + createdAt: '00000002-0000-1000-8000-000000000002', + }); + + expect(SecretMetadata.compare(primarySrp1, primarySrp2, 'asc')).toBe(0); + expect(SecretMetadata.compare(primarySrp2, primarySrp1, 'asc')).toBe(0); + expect(SecretMetadata.compare(primarySrp1, primarySrp2, 'desc')).toBe( + 0, + ); + }); - await expect( - baseMessenger.call( - 'SeedlessOnboardingController:submitGlobalPassword', - { - globalPassword: GLOBAL_PASSWORD, - }, - ), - ).rejects.toStrictEqual( - new PasswordSyncError( - SeedlessOnboardingControllerErrorMessage.CouldNotRecoverPassword, - ), - ); - }, - ); - }); + it('should compare by createdAt (TIMEUUID) when both have createdAt', () => { + const earlier = new SecretMetadata(MOCK_SEED_PHRASE, { + timestamp: 1000, + dataType: EncAccountDataType.ImportedSrp, + createdAt: '00000001-0000-1000-8000-000000000001', + }); + const later = new SecretMetadata(MOCK_SEED_PHRASE, { + timestamp: 2000, + dataType: EncAccountDataType.ImportedSrp, + createdAt: '00000002-0000-1000-8000-000000000002', + }); - it('should not propagate unknown errors from #toprfClient.recoverPassword', async () => { - await withController( - { - state: getMockInitialControllerState({ - withMockAuthenticatedUser: true, - withMockAuthPubKey: true, - }), - }, - async ({ toprfClient, baseMessenger }) => { - const mockToprfEncryptor = createMockToprfEncryptor(); - const encKey = mockToprfEncryptor.deriveEncKey(GLOBAL_PASSWORD); - const pwEncKey = mockToprfEncryptor.derivePwEncKey(GLOBAL_PASSWORD); - const authKeyPair = - mockToprfEncryptor.deriveAuthKeyPair(GLOBAL_PASSWORD); - jest.spyOn(toprfClient, 'recoverEncKey').mockResolvedValueOnce({ - encKey, - pwEncKey, - authKeyPair, - rateLimitResetResult: Promise.resolve(), - keyShareIndex: 1, - }); + expect(SecretMetadata.compare(earlier, later, 'asc')).toBeLessThan(0); + expect(SecretMetadata.compare(later, earlier, 'asc')).toBeGreaterThan( + 0, + ); + expect(SecretMetadata.compare(earlier, later, 'desc')).toBeGreaterThan( + 0, + ); + }); - jest - .spyOn(toprfClient, 'recoverPwEncKey') - .mockRejectedValueOnce(new Error('Unknown error')); + it('should fall back to timestamp when both have null createdAt', () => { + const earlier = new SecretMetadata(MOCK_SEED_PHRASE, { + timestamp: 1000, + dataType: EncAccountDataType.ImportedSrp, + }); + const later = new SecretMetadata(MOCK_SEED_PHRASE, { + timestamp: 2000, + dataType: EncAccountDataType.ImportedSrp, + }); - await expect( - baseMessenger.call( - 'SeedlessOnboardingController:submitGlobalPassword', - { - globalPassword: GLOBAL_PASSWORD, - }, - ), - ).rejects.toStrictEqual( - new PasswordSyncError( - SeedlessOnboardingControllerErrorMessage.CouldNotRecoverPassword, - ), - ); - }, - ); - }); + expect(SecretMetadata.compare(earlier, later, 'asc')).toBeLessThan(0); + expect(SecretMetadata.compare(later, earlier, 'asc')).toBeGreaterThan( + 0, + ); + expect(SecretMetadata.compare(earlier, later, 'desc')).toBeGreaterThan( + 0, + ); + }); - it('should throw MaxKeyChainLengthExceeded error when max key chain length is exceeded', async () => { - await withController( - { - state: getMockInitialControllerState({ - withMockAuthenticatedUser: true, - withMockAuthPubKey: true, - }), - }, - async ({ toprfClient, baseMessenger }) => { - const mockToprfEncryptor = createMockToprfEncryptor(); - const encKey = mockToprfEncryptor.deriveEncKey(GLOBAL_PASSWORD); - const pwEncKey = mockToprfEncryptor.derivePwEncKey(GLOBAL_PASSWORD); - const authKeyPair = - mockToprfEncryptor.deriveAuthKeyPair(GLOBAL_PASSWORD); + it('should use asc order by default', () => { + const earlier = new SecretMetadata(MOCK_SEED_PHRASE, { + timestamp: 1000, + dataType: EncAccountDataType.ImportedSrp, + }); + const later = new SecretMetadata(MOCK_SEED_PHRASE, { + timestamp: 2000, + dataType: EncAccountDataType.ImportedSrp, + }); - // Mock recoverEncKey to succeed - jest.spyOn(toprfClient, 'recoverEncKey').mockResolvedValueOnce({ - encKey, - pwEncKey, - authKeyPair, - rateLimitResetResult: Promise.resolve(), - keyShareIndex: 1, - }); + expect(SecretMetadata.compare(earlier, later)).toBeLessThan(0); + }); + }); - // Mock recoverPwEncKey to throw max key chain length error - jest - .spyOn(toprfClient, 'recoverPwEncKey') - .mockRejectedValueOnce( - new TOPRFError( - TOPRFErrorCode.MaxKeyChainLengthExceeded, - 'Max key chain length exceeded', - ), - ); + it('should default type to Mnemonic when parsing metadata without type field', () => { + // Create raw metadata JSON without type field + const rawMetadataWithoutType = JSON.stringify({ + data: bytesToBase64(MOCK_SEED_PHRASE), + timestamp: Date.now(), + }); + const rawMetadataBytes = stringToBytes(rawMetadataWithoutType); - await expect( - baseMessenger.call( - 'SeedlessOnboardingController:submitGlobalPassword', - { - globalPassword: GLOBAL_PASSWORD, - }, - ), - ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.MaxKeyChainLengthExceeded, - ); - }, - ); + const parsed = SecretMetadata.fromRawMetadata(rawMetadataBytes, {}); + expect(parsed.type).toBe(SecretType.Mnemonic); + expect(parsed.data).toStrictEqual(MOCK_SEED_PHRASE); }); - }); - describe('syncLatestGlobalPassword', () => { - const OLD_PASSWORD = 'old-mock-password'; - const GLOBAL_PASSWORD = 'new-global-password'; - const mockToprfEncryptor = createMockToprfEncryptor(); - let MOCK_VAULT: string; - let MOCK_VAULT_ENCRYPTION_KEY: string; - let MOCK_VAULT_ENCRYPTION_SALT: string; - let INITIAL_AUTH_PUB_KEY: string; - let initialAuthKeyPair: KeyPair; // Store initial keypair for vault creation - let initialEncKey: Uint8Array; // Store initial encKey for vault creation - let initialPwEncKey: Uint8Array; // Store initial pwEncKey for vault creation - let initialEncryptedSeedlessEncryptionKey: Uint8Array; // Store initial encryptedSeedlessEncryptionKey for vault creation - let newEncKey: Uint8Array; - let newPwEncKey: Uint8Array; - let newAuthKeyPair: KeyPair; - - // Generate initial keys and vault state before tests run - beforeAll(async () => { - initialEncKey = mockToprfEncryptor.deriveEncKey(OLD_PASSWORD); - initialPwEncKey = mockToprfEncryptor.derivePwEncKey(OLD_PASSWORD); - initialAuthKeyPair = mockToprfEncryptor.deriveAuthKeyPair(OLD_PASSWORD); - INITIAL_AUTH_PUB_KEY = bytesToBase64(initialAuthKeyPair.pk); + it('should be able to overwrite the default Generic DataType', () => { + const secret1 = new SecretMetadata('private-key-1', { + type: SecretType.PrivateKey, + }); + expect(secret1.data).toBe('private-key-1'); + expect(secret1.type).toBe(SecretType.PrivateKey); - const mockResult = await createMockVault( - initialEncKey, - initialPwEncKey, - initialAuthKeyPair, - OLD_PASSWORD, - revokeToken, + // should be able to convert to bytes + const secret1Bytes = secret1.toBytes(); + const parsedSecret1 = SecretMetadata.fromRawMetadata( + secret1Bytes, + {}, ); + expect(parsedSecret1.data).toBe('private-key-1'); + expect(parsedSecret1.type).toBe(SecretType.PrivateKey); - MOCK_VAULT = mockResult.encryptedMockVault; - MOCK_VAULT_ENCRYPTION_KEY = mockResult.vaultEncryptionKey; - MOCK_VAULT_ENCRYPTION_SALT = mockResult.vaultEncryptionSalt; + const secret2 = new SecretMetadata(MOCK_SEED_PHRASE, { + type: SecretType.Mnemonic, + }); + expect(secret2.data).toStrictEqual(MOCK_SEED_PHRASE); + expect(secret2.type).toBe(SecretType.Mnemonic); - const aes = managedNonce(gcm)(initialPwEncKey); - initialEncryptedSeedlessEncryptionKey = aes.encrypt( - utf8ToBytes(MOCK_VAULT_ENCRYPTION_KEY), + const secret2Bytes = secret2.toBytes(); + const parsedSecret2 = SecretMetadata.fromRawMetadata( + secret2Bytes, + {}, ); + expect(parsedSecret2.data).toStrictEqual(MOCK_SEED_PHRASE); + expect(parsedSecret2.type).toBe(SecretType.Mnemonic); }); - // Remove beforeEach as setup is done in beforeAll now - beforeEach(() => { - // Mock recoverEncKey for the new global password - newEncKey = mockToprfEncryptor.deriveEncKey(GLOBAL_PASSWORD); - newPwEncKey = mockToprfEncryptor.derivePwEncKey(GLOBAL_PASSWORD); - newAuthKeyPair = mockToprfEncryptor.deriveAuthKeyPair(GLOBAL_PASSWORD); - }); - - it('should successfully sync the latest global password', async () => { - const b64EncKey = bytesToBase64(initialEncryptedSeedlessEncryptionKey); - await withController( - { - // Pass the pre-generated state values - state: getMockInitialControllerState({ - withMockAuthenticatedUser: true, - authPubKey: INITIAL_AUTH_PUB_KEY, // Use the base64 encoded key - vault: MOCK_VAULT, - vaultEncryptionKey: MOCK_VAULT_ENCRYPTION_KEY, - vaultEncryptionSalt: MOCK_VAULT_ENCRYPTION_SALT, - withMockAuthPubKey: true, - encryptedSeedlessEncryptionKey: b64EncKey, - }), - }, - async ({ controller, toprfClient, encryptor, baseMessenger }) => { - // Unlock controller first - requires vaultEncryptionKey/Salt or password - // Since we provide key/salt in state, submitPassword isn't strictly needed here - // but we keep it to match the method's requirement of being unlocked - // We'll use the key/salt implicitly by not providing password to unlockVaultAndGetBackupEncKey - await baseMessenger.call( - 'SeedlessOnboardingController:submitPassword', - OLD_PASSWORD, - ); // Unlock using the standard method + it('should be able to parse the array of Mixed SecretMetadata', () => { + const mockPrivKeyString = '0xdeadbeef'; + const secret1 = new SecretMetadata(mockPrivKeyString, { + type: SecretType.PrivateKey, + }); + const secret2 = new SecretMetadata(MOCK_SEED_PHRASE, { + type: SecretType.Mnemonic, + }); - const recoverEncKeySpy = jest.spyOn(toprfClient, 'recoverEncKey'); - const encryptorSpy = jest.spyOn(encryptor, 'encryptWithDetail'); + const secrets = [secret1.toBytes(), secret2.toBytes()]; - recoverEncKeySpy.mockResolvedValueOnce({ - encKey: newEncKey, - pwEncKey: newPwEncKey, - authKeyPair: newAuthKeyPair, - rateLimitResetResult: Promise.resolve(), - keyShareIndex: 1, - }); + const parsedSecrets = secrets + .map((secret) => SecretMetadata.fromRawMetadata(secret, {})) + .sort((a, b) => SecretMetadata.compareByTimestamp(a, b, 'asc')); + expect(parsedSecrets).toHaveLength(2); + expect(parsedSecrets[0].data).toBe(mockPrivKeyString); + expect(parsedSecrets[0].type).toBe(SecretType.PrivateKey); + expect(parsedSecrets[1].data).toStrictEqual(MOCK_SEED_PHRASE); + expect(parsedSecrets[1].type).toBe(SecretType.Mnemonic); + }); - // We still need verifyPassword to work conceptually, even if unlock is bypassed - // verifyPasswordSpy.mockResolvedValueOnce(); // Don't mock, let the real one run inside syncLatestGlobalPassword + it('should be able to filter the array of SecretMetadata by type', () => { + const mockPrivKeyString = '0xdeadbeef'; + const secret1 = new SecretMetadata(mockPrivKeyString, { + type: SecretType.PrivateKey, + }); + const secret2 = new SecretMetadata(MOCK_SEED_PHRASE, { + type: SecretType.Mnemonic, + }); + const secret3 = new SecretMetadata(MOCK_SEED_PHRASE); - await baseMessenger.call('SeedlessOnboardingController:setLocked'); + const secrets = [secret1.toBytes(), secret2.toBytes(), secret3.toBytes()]; - // Mock recoverEncKey for the global password - const encKey = mockToprfEncryptor.deriveEncKey(GLOBAL_PASSWORD); - const pwEncKey = mockToprfEncryptor.derivePwEncKey(GLOBAL_PASSWORD); - const authKeyPair = - mockToprfEncryptor.deriveAuthKeyPair(GLOBAL_PASSWORD); - jest.spyOn(toprfClient, 'recoverEncKey').mockResolvedValueOnce({ - encKey, - pwEncKey, - authKeyPair, - rateLimitResetResult: Promise.resolve(), - keyShareIndex: 1, - }); + const allSecrets = secrets + .map((secret) => SecretMetadata.fromRawMetadata(secret, {})) + .sort((a, b) => SecretMetadata.compareByTimestamp(a, b, 'asc')); - // Mock toprfClient.recoverPwEncKey - const recoveredPwEncKey = - mockToprfEncryptor.derivePwEncKey(OLD_PASSWORD); - jest.spyOn(toprfClient, 'recoverPwEncKey').mockResolvedValueOnce({ - pwEncKey: recoveredPwEncKey, - }); + const mnemonicSecrets = allSecrets.filter((secret) => + SecretMetadata.matchesType(secret, SecretType.Mnemonic), + ); + expect(mnemonicSecrets).toHaveLength(2); + expect(mnemonicSecrets[0].data).toStrictEqual(MOCK_SEED_PHRASE); + expect(mnemonicSecrets[0].type).toBe(SecretType.Mnemonic); + expect(mnemonicSecrets[1].data).toStrictEqual(MOCK_SEED_PHRASE); + expect(mnemonicSecrets[1].type).toBe(SecretType.Mnemonic); - await baseMessenger.call( - 'SeedlessOnboardingController:submitGlobalPassword', - { - globalPassword: GLOBAL_PASSWORD, - }, - ); + const privateKeySecrets = allSecrets.filter((secret) => + SecretMetadata.matchesType(secret, SecretType.PrivateKey), + ); - await baseMessenger.call( - 'SeedlessOnboardingController:syncLatestGlobalPassword', - { - globalPassword: GLOBAL_PASSWORD, - }, - ); + expect(privateKeySecrets).toHaveLength(1); + expect(privateKeySecrets[0].data).toBe(mockPrivKeyString); + expect(privateKeySecrets[0].type).toBe(SecretType.PrivateKey); + }); - // Assertions - expect(recoverEncKeySpy).toHaveBeenCalledWith( - expect.objectContaining({ password: GLOBAL_PASSWORD }), - ); + it('should derive type from dataType (V2)', () => { + const srp1 = new SecretMetadata(MOCK_SEED_PHRASE, { + dataType: EncAccountDataType.PrimarySrp, + }); + expect(srp1.type).toBe(SecretType.Mnemonic); + expect(srp1.dataType).toBe(EncAccountDataType.PrimarySrp); - // Check if vault was re-encrypted with the new password and keys - const expectedSerializedVaultData = JSON.stringify({ - toprfEncryptionKey: bytesToBase64(newEncKey), - toprfPwEncryptionKey: bytesToBase64(newPwEncKey), - toprfAuthKeyPair: JSON.stringify({ - sk: bigIntToHex(newAuthKeyPair.sk), - pk: bytesToBase64(newAuthKeyPair.pk), - }), - revokeToken: controller.state.revokeToken, - accessToken: controller.state.accessToken, - }); - expect(encryptorSpy).toHaveBeenCalledWith( - GLOBAL_PASSWORD, - expectedSerializedVaultData, - ); + const srp2 = new SecretMetadata(MOCK_SEED_PHRASE, { + dataType: EncAccountDataType.ImportedSrp, + }); + expect(srp2.type).toBe(SecretType.Mnemonic); + expect(srp2.dataType).toBe(EncAccountDataType.ImportedSrp); - // Check if authPubKey was updated in state - expect(controller.state.authPubKey).toBe( - bytesToBase64(newAuthKeyPair.pk), - ); - // Check if vault content actually changed - expect(controller.state.vault).not.toBe(MOCK_VAULT); - }, - ); + const pk = new SecretMetadata('0xdeadbeef', { + dataType: EncAccountDataType.ImportedPrivateKey, + }); + expect(pk.type).toBe(SecretType.PrivateKey); + expect(pk.dataType).toBe(EncAccountDataType.ImportedPrivateKey); }); - it('should persist the latest accessToken when state token is newer than vault token', async () => { - const futureExp = Math.floor(Date.now() / 1000) + 3600; // 1 hour from now - const newerAccessToken = createMockJWTToken({ exp: futureExp }); // refreshed accessToken - const b64EncKey = bytesToBase64(initialEncryptedSeedlessEncryptionKey); + it('should be able to create SecretMetadata with storage metadata', () => { + const secretMetadata = new SecretMetadata(MOCK_SEED_PHRASE, { + dataType: EncAccountDataType.PrimarySrp, + itemId: 'test-item-id', + createdAt: '00000001-0000-1000-8000-000000000001', + }); - await withController( - { - state: getMockInitialControllerState({ - withMockAuthenticatedUser: true, - authPubKey: INITIAL_AUTH_PUB_KEY, // Use the base64 encoded key - vault: MOCK_VAULT, - vaultEncryptionKey: MOCK_VAULT_ENCRYPTION_KEY, - vaultEncryptionSalt: MOCK_VAULT_ENCRYPTION_SALT, - withMockAuthPubKey: true, - encryptedSeedlessEncryptionKey: b64EncKey, - }), - }, - async ({ - controller, - toprfClient, - encryptor, - mockRefreshJWTToken, - baseMessenger, - }) => { - // Unlock controller first - requires vaultEncryptionKey/Salt or password - // Since we provide key/salt in state, submitPassword isn't strictly needed here - // but we keep it to match the method's requirement of being unlocked - // We'll use the key/salt implicitly by not providing password to unlockVaultAndGetBackupEncKey - await baseMessenger.call( - 'SeedlessOnboardingController:submitPassword', - OLD_PASSWORD, - ); // Unlock using the standard method + expect(secretMetadata.data).toStrictEqual(MOCK_SEED_PHRASE); + expect(secretMetadata.type).toBe(SecretType.Mnemonic); + expect(secretMetadata.itemId).toBe('test-item-id'); + expect(secretMetadata.dataType).toBe(EncAccountDataType.PrimarySrp); + expect(secretMetadata.createdAt).toBe( + '00000001-0000-1000-8000-000000000001', + ); + }); - const recoverEncKeySpy = jest.spyOn(toprfClient, 'recoverEncKey'); - const encryptorSpy = jest.spyOn(encryptor, 'encryptWithDetail'); + it('should have undefined storage metadata when not provided', () => { + const secretMetadata = new SecretMetadata(MOCK_SEED_PHRASE); - recoverEncKeySpy.mockResolvedValueOnce({ - encKey: newEncKey, - pwEncKey: newPwEncKey, - authKeyPair: newAuthKeyPair, - rateLimitResetResult: Promise.resolve(), - keyShareIndex: 1, - }); - // Lock the wallet - await baseMessenger.call('SeedlessOnboardingController:setLocked'); + expect(secretMetadata.data).toStrictEqual(MOCK_SEED_PHRASE); + expect(secretMetadata.itemId).toBeUndefined(); + expect(secretMetadata.dataType).toBeUndefined(); + expect(secretMetadata.createdAt).toBeUndefined(); + }); - // The following mocks are to simulate the token expiry and refresh. - // mock token expiry - jest - .spyOn(controller, 'checkNodeAuthTokenExpired') - .mockReturnValueOnce(true); - // mock token refresh - mockRefreshJWTToken.mockResolvedValueOnce({ - idTokens: ['newIdToken'], - accessToken: newerAccessToken, - metadataAccessToken: 'new-metadata-access-token', - }); - // mock toprfClient.authenticate which is called to generate new NodeAuthTokens - jest.spyOn(toprfClient, 'authenticate').mockResolvedValueOnce({ - nodeAuthTokens: MOCK_NODE_AUTH_TOKENS, - isNewUser: false, - }); + it('should NOT serialize storage metadata in toBytes()', () => { + const secretMetadata = new SecretMetadata(MOCK_SEED_PHRASE, { + dataType: EncAccountDataType.PrimarySrp, + itemId: 'test-item-id', + createdAt: '00000001-0000-1000-8000-000000000001', + }); - // Mock recoverEncKey for the global password - const encKey = mockToprfEncryptor.deriveEncKey(GLOBAL_PASSWORD); - const pwEncKey = mockToprfEncryptor.derivePwEncKey(GLOBAL_PASSWORD); - const authKeyPair = - mockToprfEncryptor.deriveAuthKeyPair(GLOBAL_PASSWORD); - jest.spyOn(toprfClient, 'recoverEncKey').mockResolvedValueOnce({ - encKey, - pwEncKey, - authKeyPair, - rateLimitResetResult: Promise.resolve(), - keyShareIndex: 1, - }); + const serializedBytes = secretMetadata.toBytes(); + const serializedString = bytesToString(serializedBytes); + const parsed = JSON.parse(serializedString); - // Mock toprfClient.recoverPwEncKey - const recoveredPwEncKey = - mockToprfEncryptor.derivePwEncKey(OLD_PASSWORD); - jest.spyOn(toprfClient, 'recoverPwEncKey').mockResolvedValueOnce({ - pwEncKey: recoveredPwEncKey, - }); + // Storage metadata should NOT be in serialized data + expect(parsed.itemId).toBeUndefined(); + expect(parsed.dataType).toBeUndefined(); + expect(parsed.createdAt).toBeUndefined(); - await baseMessenger.call( - 'SeedlessOnboardingController:submitGlobalPassword', - { - globalPassword: GLOBAL_PASSWORD, - }, - ); + // Only encrypted metadata should be present + expect(parsed.data).toBeDefined(); + expect(parsed.timestamp).toBeDefined(); + expect(parsed.type).toBe(SecretType.Mnemonic); + }); - // assert that the newer access token is set in the state - expect(controller.state.accessToken).toBe(newerAccessToken); + it('should be able to parse raw metadata with storage metadata', () => { + const originalMetadata = new SecretMetadata(MOCK_SEED_PHRASE, { + type: SecretType.Mnemonic, + }); + const serializedBytes = originalMetadata.toBytes(); - await baseMessenger.call( - 'SeedlessOnboardingController:syncLatestGlobalPassword', - { - globalPassword: GLOBAL_PASSWORD, - }, - ); + const parsedMetadata = SecretMetadata.fromRawMetadata(serializedBytes, { + itemId: 'server-assigned-id', + dataType: EncAccountDataType.ImportedSrp, + createdAt: '00000002-0000-1000-8000-000000000002', + }); - // Check if vault was re-encrypted with the new password and keys - const expectedSerializedVaultData = JSON.stringify({ - toprfEncryptionKey: bytesToBase64(newEncKey), - toprfPwEncryptionKey: bytesToBase64(newPwEncKey), - toprfAuthKeyPair: JSON.stringify({ - sk: bigIntToHex(newAuthKeyPair.sk), - pk: bytesToBase64(newAuthKeyPair.pk), - }), - revokeToken: controller.state.revokeToken, - accessToken: newerAccessToken, - }); - expect(encryptorSpy).toHaveBeenCalledWith( - GLOBAL_PASSWORD, - expectedSerializedVaultData, - ); - }, + expect(parsedMetadata.data).toStrictEqual(MOCK_SEED_PHRASE); + expect(parsedMetadata.type).toBe(SecretType.Mnemonic); + expect(parsedMetadata.itemId).toBe('server-assigned-id'); + expect(parsedMetadata.dataType).toBe(EncAccountDataType.ImportedSrp); + expect(parsedMetadata.createdAt).toBe( + '00000002-0000-1000-8000-000000000002', ); }); + }); + + describe('store and recover keyring encryption key', () => { + const GLOBAL_PASSWORD = 'global-password'; + const RECOVERED_PASSWORD = 'recovered-password'; - it('should throw an error if recovering the encryption key for the global password fails', async () => { + it('should throw if key not set', async () => { await withController( { state: getMockInitialControllerState({ withMockAuthenticatedUser: true, - authPubKey: INITIAL_AUTH_PUB_KEY, - vault: MOCK_VAULT, - vaultEncryptionKey: MOCK_VAULT_ENCRYPTION_KEY, - vaultEncryptionSalt: MOCK_VAULT_ENCRYPTION_SALT, + withMockAuthPubKey: true, + vault: 'mock-vault', }), }, - async ({ toprfClient, baseMessenger }) => { - // Unlock controller first - await baseMessenger.call( - 'SeedlessOnboardingController:submitPassword', - OLD_PASSWORD, + async ({ controller, toprfClient, baseMessenger }) => { + await expect( + baseMessenger.call( + 'SeedlessOnboardingController:storeKeyringEncryptionKey', + '', + ), + ).rejects.toThrow( + SeedlessOnboardingControllerErrorMessage.WrongPasswordType, ); - const recoverEncKeySpy = jest - .spyOn(toprfClient, 'recoverEncKey') - .mockRejectedValueOnce( - new RecoveryError( - SeedlessOnboardingControllerErrorMessage.LoginFailedError, - ), - ); + await mockCreateToprfKeyAndBackupSeedPhrase( + toprfClient, + controller, + baseMessenger, + RECOVERED_PASSWORD, + MOCK_SEED_PHRASE, + MOCK_KEYRING_ID, + ); await expect( baseMessenger.call( - 'SeedlessOnboardingController:syncLatestGlobalPassword', - { - globalPassword: GLOBAL_PASSWORD, - }, + 'SeedlessOnboardingController:loadKeyringEncryptionKey', ), ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.LoginFailedError, - ); - - expect(recoverEncKeySpy).toHaveBeenCalledWith( - expect.objectContaining({ password: GLOBAL_PASSWORD }), + SeedlessOnboardingControllerErrorMessage.EncryptedKeyringEncryptionKeyNotSet, ); }, ); }); - it('should throw an error if creating the new vault fails', async () => { - const state = getMockInitialControllerState({ - withMockAuthenticatedUser: true, - authPubKey: INITIAL_AUTH_PUB_KEY, - vault: MOCK_VAULT, - vaultEncryptionKey: MOCK_VAULT_ENCRYPTION_KEY, - vaultEncryptionSalt: MOCK_VAULT_ENCRYPTION_SALT, - }); - delete state.revokeToken; - delete state.accessToken; - + it('should store and load keyring encryption key', async () => { await withController( { - state, + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + withMockAuthPubKey: true, + }), }, - async ({ toprfClient, encryptor, baseMessenger }) => { - // Unlock controller first - await baseMessenger.call( - 'SeedlessOnboardingController:submitPassword', - OLD_PASSWORD, + async ({ controller, toprfClient, baseMessenger }) => { + await newUserSetup( + toprfClient, + controller, + baseMessenger, + RECOVERED_PASSWORD, ); - const recoverEncKeySpy = jest.spyOn(toprfClient, 'recoverEncKey'); - const encryptorSpy = jest.spyOn(encryptor, 'encryptWithDetail'); - - // Make recoverEncKey succeed - recoverEncKeySpy.mockResolvedValueOnce({ - encKey: newEncKey, - pwEncKey: newPwEncKey, - authKeyPair: newAuthKeyPair, - rateLimitResetResult: Promise.resolve(), - keyShareIndex: 1, - }); - - // Make encryptWithDetail always fail to ensure we catch any call to it - encryptorSpy.mockRejectedValue(new Error('Vault creation failed')); - - await expect( - baseMessenger.call( - 'SeedlessOnboardingController:syncLatestGlobalPassword', - { - globalPassword: GLOBAL_PASSWORD, - }, - ), - ).rejects.toThrow('Vault creation failed'); - - expect(recoverEncKeySpy).toHaveBeenCalledWith( - expect.objectContaining({ password: GLOBAL_PASSWORD }), + const result = await baseMessenger.call( + 'SeedlessOnboardingController:loadKeyringEncryptionKey', ); - expect(encryptorSpy).toHaveBeenCalled(); + expect(result).toStrictEqual(MOCK_KEYRING_ENCRYPTION_KEY); }, ); }); - /** - * This test is to verify that the controller throws an error if the encryption salt is expired. - * The test creates a mock vault with a different salt value in the state to simulate an expired salt. - * It then creates mock keys associated with the new global password and uses these values as mock return values for the recoverEncKey and recoverPwEncKey calls. - * The test expects the controller to throw an error indicating that the password could not be recovered since the encryption salt from state is different from the salt in the mock vault. - */ - it('should throw an error if the encryption salt is expired', async () => { - const encryptedSeedlessEncryptionKey = bytesToBase64( - initialEncryptedSeedlessEncryptionKey, - ); + it('should load keyring encryption key after change password', async () => { await withController( { state: getMockInitialControllerState({ withMockAuthenticatedUser: true, - authPubKey: INITIAL_AUTH_PUB_KEY, // Use the base64 encoded key - vault: MOCK_VAULT, - vaultEncryptionKey: MOCK_VAULT_ENCRYPTION_KEY, - // Mock a different salt value in state to simulate an expired salt - vaultEncryptionSalt: 'DIFFERENT-SALT', withMockAuthPubKey: true, - encryptedSeedlessEncryptionKey, }), }, - async ({ toprfClient, baseMessenger }) => { - // Here we are creating mock keys associated with the new global password - // and these values are used as mock return values for the recoverEncKey and recoverPwEncKey calls - const recoverEncKeySpy = jest - .spyOn(toprfClient, 'recoverEncKey') - .mockResolvedValueOnce({ - encKey: newEncKey, - pwEncKey: newPwEncKey, - authKeyPair: newAuthKeyPair, - rateLimitResetResult: Promise.resolve(), - keyShareIndex: 1, - }); + async ({ controller, toprfClient, baseMessenger }) => { + await newUserSetup( + toprfClient, + controller, + baseMessenger, + RECOVERED_PASSWORD, + ); - const recoverPwEncKeySpy = jest - .spyOn(toprfClient, 'recoverPwEncKey') - .mockResolvedValueOnce({ - pwEncKey: initialPwEncKey, - }); + await mockChangePassword( + controller, + toprfClient, + RECOVERED_PASSWORD, + GLOBAL_PASSWORD, + ); - await expect( - baseMessenger.call( - 'SeedlessOnboardingController:submitGlobalPassword', - { - globalPassword: GLOBAL_PASSWORD, - }, - ), - ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.CouldNotRecoverPassword, + await baseMessenger.call( + 'SeedlessOnboardingController:changePassword', + GLOBAL_PASSWORD, + RECOVERED_PASSWORD, + ); + + const result = await baseMessenger.call( + 'SeedlessOnboardingController:loadKeyringEncryptionKey', ); - expect(recoverEncKeySpy).toHaveBeenCalled(); - expect(recoverPwEncKeySpy).toHaveBeenCalled(); + expect(result).toStrictEqual(MOCK_KEYRING_ENCRYPTION_KEY); }, ); }); @@ -5932,7 +6445,7 @@ describe('SeedlessOnboardingController', () => { // This should not trigger token refresh since access token check is skipped when locked await baseMessenger.call( - 'SeedlessOnboardingController:checkIsPasswordOutdated', + 'SeedlessOnboardingController:resolvePasswordSyncState', ); // Verify that refreshAuthTokens was not called @@ -5955,14 +6468,11 @@ describe('SeedlessOnboardingController', () => { .spyOn(toprfClient, 'fetchAuthPubKey') .mockRejectedValue(new Error('Network error')); - // This should throw the wrapped error without retrying - await expect( - baseMessenger.call( - 'SeedlessOnboardingController:checkIsPasswordOutdated', - ), - ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.FailedToFetchAuthPubKey, + // This should return Unknown without retrying (non-token error) + const result = await baseMessenger.call( + 'SeedlessOnboardingController:resolvePasswordSyncState', ); + expect(result).toBe(PasswordSyncStatus.Unknown); // Verify that fetchAuthPubKey was only called once (no retry) expect(toprfClient.fetchAuthPubKey).toHaveBeenCalledTimes(1); @@ -6104,8 +6614,8 @@ describe('SeedlessOnboardingController', () => { }); }); - describe('checkIsPasswordOutdated with token refresh', () => { - it('should retry checkIsPasswordOutdated after refreshing expired tokens', async () => { + describe('resolvePasswordSyncState with token refresh', () => { + it('should retry resolvePasswordSyncState after refreshing expired tokens', async () => { await withController( { state: { @@ -6141,7 +6651,7 @@ describe('SeedlessOnboardingController', () => { }); await baseMessenger.call( - 'SeedlessOnboardingController:checkIsPasswordOutdated', + 'SeedlessOnboardingController:resolvePasswordSyncState', ); expect(mockRefreshJWTToken).toHaveBeenCalled(); @@ -6354,235 +6864,6 @@ describe('SeedlessOnboardingController', () => { }); }); - describe('syncLatestGlobalPassword with token refresh', () => { - const OLD_PASSWORD = 'old-mock-password'; - const GLOBAL_PASSWORD = 'new-global-password'; - let MOCK_VAULT: string; - let MOCK_VAULT_ENCRYPTION_KEY: string; - let MOCK_VAULT_ENCRYPTION_SALT: string; - let INITIAL_AUTH_PUB_KEY: string; - let initialAuthKeyPair: KeyPair; // Store initial keypair for vault creation - let initialEncKey: Uint8Array; // Store initial encKey for vault creation - let initialPwEncKey: Uint8Array; // Store initial pwEncKey for vault creation - - // Generate initial keys and vault state before tests run - beforeAll(async () => { - const mockToprfEncryptor = createMockToprfEncryptor(); - initialEncKey = mockToprfEncryptor.deriveEncKey(OLD_PASSWORD); - initialPwEncKey = mockToprfEncryptor.derivePwEncKey(OLD_PASSWORD); - initialAuthKeyPair = mockToprfEncryptor.deriveAuthKeyPair(OLD_PASSWORD); - INITIAL_AUTH_PUB_KEY = bytesToBase64(initialAuthKeyPair.pk); - - const mockResult = await createMockVault( - initialEncKey, - initialPwEncKey, - initialAuthKeyPair, - OLD_PASSWORD, - ); - - MOCK_VAULT = mockResult.encryptedMockVault; - MOCK_VAULT_ENCRYPTION_KEY = mockResult.vaultEncryptionKey; - MOCK_VAULT_ENCRYPTION_SALT = mockResult.vaultEncryptionSalt; - }); - - it('should retry syncLatestGlobalPassword after refreshing expired tokens', async () => { - await withController( - { - state: getMockInitialControllerState({ - withMockAuthenticatedUser: true, - authPubKey: INITIAL_AUTH_PUB_KEY, - vault: MOCK_VAULT, - vaultEncryptionKey: MOCK_VAULT_ENCRYPTION_KEY, - vaultEncryptionSalt: MOCK_VAULT_ENCRYPTION_SALT, - }), - }, - async ({ - controller, - toprfClient, - encryptor, - mockRefreshJWTToken, - baseMessenger, - }) => { - // Unlock controller first - await baseMessenger.call( - 'SeedlessOnboardingController:submitPassword', - OLD_PASSWORD, - ); - - // Capture before the call — proactive renewRefreshToken inside - // refreshAuthTokens will rotate state.refreshToken afterwards. - const originalRefreshToken = controller.state.refreshToken; - - const recoverEncKeySpy = jest.spyOn(toprfClient, 'recoverEncKey'); - const encryptorSpy = jest.spyOn(encryptor, 'encryptWithDetail'); - - // Mock recoverEncKey for the new global password - const mockToprfEncryptor = createMockToprfEncryptor(); - const newEncKey = mockToprfEncryptor.deriveEncKey(GLOBAL_PASSWORD); - const newPwEncKey = - mockToprfEncryptor.derivePwEncKey(GLOBAL_PASSWORD); - const newAuthKeyPair = - mockToprfEncryptor.deriveAuthKeyPair(GLOBAL_PASSWORD); - - // Mock recoverEncKey to fail first with token expired error, then succeed - recoverEncKeySpy - .mockImplementationOnce(() => { - throw new TOPRFError( - TOPRFErrorCode.AuthTokenExpired, - 'Auth token expired', - ); - }) - .mockResolvedValueOnce({ - encKey: newEncKey, - pwEncKey: newPwEncKey, - authKeyPair: newAuthKeyPair, - rateLimitResetResult: Promise.resolve(), - keyShareIndex: 1, - }); - - // Mock authenticate for token refresh - jest.spyOn(toprfClient, 'authenticate').mockResolvedValue({ - nodeAuthTokens: MOCK_NODE_AUTH_TOKENS, - isNewUser: false, - }); - - await baseMessenger.call( - 'SeedlessOnboardingController:syncLatestGlobalPassword', - { - globalPassword: GLOBAL_PASSWORD, - }, - ); - - // Verify that getNewRefreshToken was called - expect(mockRefreshJWTToken).toHaveBeenCalledWith({ - connection: controller.state.authConnection, - refreshToken: originalRefreshToken, - }); - - // Verify that recoverEncKey was called twice (once failed, once succeeded) - expect(recoverEncKeySpy).toHaveBeenCalledTimes(2); - - // Verify that authenticate was called during token refresh - expect(toprfClient.authenticate).toHaveBeenCalled(); - - // Check if vault was re-encrypted with the new password and keys - const expectedSerializedVaultData = JSON.stringify({ - toprfEncryptionKey: bytesToBase64(newEncKey), - toprfPwEncryptionKey: bytesToBase64(newPwEncKey), - toprfAuthKeyPair: JSON.stringify({ - sk: bigIntToHex(newAuthKeyPair.sk), - pk: bytesToBase64(newAuthKeyPair.pk), - }), - revokeToken: controller.state.revokeToken, - accessToken: controller.state.accessToken, - }); - expect(encryptorSpy).toHaveBeenCalledWith( - GLOBAL_PASSWORD, - expectedSerializedVaultData, - ); - - // Check if authPubKey was updated in state - expect(controller.state.authPubKey).toBe( - bytesToBase64(newAuthKeyPair.pk), - ); - }, - ); - }); - - it('should fail if token refresh fails during syncLatestGlobalPassword', async () => { - await withController( - { - state: getMockInitialControllerState({ - withMockAuthenticatedUser: true, - authPubKey: INITIAL_AUTH_PUB_KEY, - vault: MOCK_VAULT, - vaultEncryptionKey: MOCK_VAULT_ENCRYPTION_KEY, - vaultEncryptionSalt: MOCK_VAULT_ENCRYPTION_SALT, - }), - }, - async ({ toprfClient, mockRefreshJWTToken, baseMessenger }) => { - // Unlock controller first - await baseMessenger.call( - 'SeedlessOnboardingController:submitPassword', - OLD_PASSWORD, - ); - - // Mock recoverEncKey to fail with token expired error - jest - .spyOn(toprfClient, 'recoverEncKey') - .mockImplementationOnce(() => { - throw new TOPRFError( - TOPRFErrorCode.AuthTokenExpired, - 'Auth token expired', - ); - }); - - // Mock getNewRefreshToken to fail - mockRefreshJWTToken.mockRejectedValueOnce( - new Error('Failed to get new refresh token'), - ); - - await expect( - baseMessenger.call( - 'SeedlessOnboardingController:syncLatestGlobalPassword', - { - globalPassword: GLOBAL_PASSWORD, - }, - ), - ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.FailedToRefreshJWTTokens, - ); - - // Verify that getNewRefreshToken was called - expect(mockRefreshJWTToken).toHaveBeenCalled(); - }, - ); - }); - - it('should not retry on non-token-related errors during syncLatestGlobalPassword', async () => { - await withController( - { - state: getMockInitialControllerState({ - withMockAuthenticatedUser: true, - authPubKey: INITIAL_AUTH_PUB_KEY, - vault: MOCK_VAULT, - vaultEncryptionKey: MOCK_VAULT_ENCRYPTION_KEY, - vaultEncryptionSalt: MOCK_VAULT_ENCRYPTION_SALT, - }), - }, - async ({ toprfClient, mockRefreshJWTToken, baseMessenger }) => { - // Unlock controller first - await baseMessenger.call( - 'SeedlessOnboardingController:submitPassword', - OLD_PASSWORD, - ); - - // Mock recoverEncKey to fail with a non-token error - jest - .spyOn(toprfClient, 'recoverEncKey') - .mockRejectedValue(new Error('Some other error')); - - await expect( - baseMessenger.call( - 'SeedlessOnboardingController:syncLatestGlobalPassword', - { - globalPassword: GLOBAL_PASSWORD, - }, - ), - ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.LoginFailedError, - ); - - // Verify that getNewRefreshToken was NOT called - expect(mockRefreshJWTToken).not.toHaveBeenCalled(); - - // Verify that recoverEncKey was only called once (no retry) - expect(toprfClient.recoverEncKey).toHaveBeenCalledTimes(1); - }, - ); - }); - }); - describe('addNewSecretData with token refresh', () => { const NEW_KEY_RING = { id: 'new-keyring-1', @@ -6840,103 +7121,6 @@ describe('SeedlessOnboardingController', () => { }); }); - describe('recover keyring encryption key with token refresh', () => { - // const OLD_PASSWORD = 'old-mock-password'; - // const GLOBAL_PASSWORD = 'new-global-password'; - let MOCK_VAULT: string; - let MOCK_VAULT_ENCRYPTION_KEY: string; - let MOCK_VAULT_ENCRYPTION_SALT: string; - let INITIAL_AUTH_PUB_KEY: string; - let initialAuthKeyPair: KeyPair; // Store initial keypair for vault creation - let initialEncKey: Uint8Array; // Store initial encKey for vault creation - let initialPwEncKey: Uint8Array; // Store initial pwEncKey for vault creation - let initialEncryptedSeedlessEncryptionKey: Uint8Array; // Store initial encryptedSeedlessEncryptionKey for vault creation - // Generate initial keys and vault state before tests run - beforeAll(async () => { - const mockToprfEncryptor = createMockToprfEncryptor(); - initialEncKey = mockToprfEncryptor.deriveEncKey(MOCK_PASSWORD); - initialPwEncKey = mockToprfEncryptor.derivePwEncKey(MOCK_PASSWORD); - - initialAuthKeyPair = - mockToprfEncryptor.deriveAuthKeyPair(MOCK_PASSWORD); - INITIAL_AUTH_PUB_KEY = bytesToBase64(initialAuthKeyPair.pk); - - const mockResult = await createMockVault( - initialEncKey, - initialPwEncKey, - initialAuthKeyPair, - MOCK_PASSWORD, - ); - - MOCK_VAULT = mockResult.encryptedMockVault; - MOCK_VAULT_ENCRYPTION_KEY = mockResult.vaultEncryptionKey; - MOCK_VAULT_ENCRYPTION_SALT = mockResult.vaultEncryptionSalt; - const aes = managedNonce(gcm)(mockResult.pwEncKey); - initialEncryptedSeedlessEncryptionKey = aes.encrypt( - utf8ToBytes(MOCK_VAULT_ENCRYPTION_KEY), - ); - }); - - it('should retry after refreshing expired tokens', async () => { - await withController( - { - state: getMockInitialControllerState({ - withMockAuthPubKey: true, - withMockAuthenticatedUser: true, - authPubKey: INITIAL_AUTH_PUB_KEY, - vault: MOCK_VAULT, - vaultEncryptionKey: MOCK_VAULT_ENCRYPTION_KEY, - vaultEncryptionSalt: MOCK_VAULT_ENCRYPTION_SALT, - encryptedSeedlessEncryptionKey: bytesToBase64( - initialEncryptedSeedlessEncryptionKey, - ), - }), - }, - async ({ toprfClient, mockRefreshJWTToken, baseMessenger }) => { - await baseMessenger.call( - 'SeedlessOnboardingController:submitPassword', - MOCK_PASSWORD, - ); - - // Mock recoverEncKey - mockRecoverEncKey(toprfClient, MOCK_PASSWORD); - // second call after refresh token - mockRecoverEncKey(toprfClient, MOCK_PASSWORD); - - // Mock recoverPassword - jest - .spyOn(toprfClient, 'recoverPwEncKey') - .mockImplementationOnce(() => { - // First call fails with token expired error - throw new TOPRFError( - TOPRFErrorCode.AuthTokenExpired, - 'Auth token expired', - ); - }) - .mockResolvedValueOnce({ - pwEncKey: initialPwEncKey, - }); - - // Mock authenticate for token refresh - jest.spyOn(toprfClient, 'authenticate').mockResolvedValueOnce({ - nodeAuthTokens: MOCK_NODE_AUTH_TOKENS, - isNewUser: false, - }); - - await baseMessenger.call( - 'SeedlessOnboardingController:submitGlobalPassword', - { - globalPassword: MOCK_PASSWORD, - }, - ); - - expect(mockRefreshJWTToken).toHaveBeenCalled(); - expect(toprfClient.recoverPwEncKey).toHaveBeenCalledTimes(2); - }, - ); - }); - }); - describe('refreshAuthTokens', () => { const mockToprfEncryptor = createMockToprfEncryptor(); let MOCK_ENCRYPTION_KEY: Uint8Array; @@ -7339,6 +7523,40 @@ describe('SeedlessOnboardingController', () => { ); }); + it('should not clear a newer in-flight refresh promise', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + }), + }, + async ({ controller, toprfClient, mockRefreshJWTToken }) => { + jest.spyOn(toprfClient, 'authenticate').mockResolvedValue({ + nodeAuthTokens: MOCK_NODE_AUTH_TOKENS, + isNewUser: false, + }); + + let nestedRefreshPromise!: Promise; + mockRefreshJWTToken.mockImplementationOnce(() => { + // The nested call starts before the outer call assigns its + // in-flight promise. The outer assignment must not be cleared + // by the nested call's finally handler. + nestedRefreshPromise = controller.refreshAuthTokens(); + return Promise.resolve({ + idTokens: ['newIdToken'], + metadataAccessToken: 'mock-metadata-access-token', + accessToken, + }); + }); + + await controller.refreshAuthTokens(); + await nestedRefreshPromise; + + expect(mockRefreshJWTToken).toHaveBeenCalledTimes(2); + }, + ); + }); + it('should clear the in-flight promise after failure, allowing subsequent calls to succeed', async () => { await withController( { @@ -8794,6 +9012,39 @@ describe('SeedlessOnboardingController', () => { }, ); }); + + it('should retain all pending tokens when every revocation fails', async () => { + const pendingTokens = [ + { + refreshToken: 'old-refresh-token-1', + revokeToken: 'old-revoke-token-1', + }, + { + refreshToken: 'old-refresh-token-2', + revokeToken: 'old-revoke-token-2', + }, + ]; + + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + pendingToBeRevokedTokens: pendingTokens, + }), + }, + async ({ controller, mockRevokeRefreshToken, baseMessenger }) => { + mockRevokeRefreshToken.mockRejectedValue(new Error('Revoke failed')); + + await baseMessenger.call( + 'SeedlessOnboardingController:revokePendingRefreshTokens', + ); + + expect(controller.state.pendingToBeRevokedTokens).toStrictEqual( + pendingTokens, + ); + }, + ); + }); }); describe('metadata', () => { diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts index ba24723ff4e..9e2e4881905 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts @@ -50,6 +50,8 @@ import { SecretType, SeedlessOnboardingControllerErrorMessage, SeedlessOnboardingMigrationVersion, + SeedlessPasswordChangePhase, + PasswordSyncStatus, Web3AuthNetwork, } from './constants.js'; import { @@ -83,6 +85,14 @@ import { const log = createModuleLogger(projectLogger, controllerName); +type UpdatedVaultState = Pick< + SeedlessOnboardingControllerState, + | 'vault' + | 'vaultEncryptionKey' + | 'vaultEncryptionSalt' + | 'encryptedSeedlessEncryptionKey' +>; + const MESSENGER_EXPOSED_METHODS = [ 'fetchMetadataAccessCreds', 'preloadToprfNodeDetails', @@ -91,14 +101,15 @@ const MESSENGER_EXPOSED_METHODS = [ 'addNewSecretData', 'fetchAllSecretData', 'changePassword', + 'clearPasswordChangePhase', + 'markPasswordChangeKeySyncPending', + 'resolvePasswordSyncState', + 'reconcilePassword', 'updateBackupMetadataState', 'verifyVaultPassword', 'getSecretDataBackupState', 'submitPassword', 'setLocked', - 'syncLatestGlobalPassword', - 'submitGlobalPassword', - 'checkIsPasswordOutdated', 'getIsUserAuthenticated', 'clearState', 'storeKeyringEncryptionKey', @@ -385,6 +396,13 @@ const seedlessOnboardingMetadata: StateMetadata => { await this.#assertPasswordInSync({ skipCache: true, - skipLock: true, // skip lock since we already have the lock }); // verify the password and unlock the vault @@ -755,7 +772,6 @@ export class SeedlessOnboardingController< await this.#assertPasswordInSync({ skipCache: true, - skipLock: true, // skip lock since we already have the lock }); if (this.state.migrationVersion < SeedlessOnboardingMigrationVersion.V1) { @@ -962,6 +978,19 @@ export class SeedlessOnboardingController< ): Promise { return await this.#withControllerLock(async () => { this.#assertIsUnlocked(); + + // Reject a second password change while a previous one is unresolved. + // The controller mutex serializes calls, but a previous change may have + // released the lock with the lifecycle still set (recovery pending). + // Starting a fresh `changePassword`/`changeEncKey` then would race with + // recovery and could block the user from their wallet. Recovery must + // finish and clear the lifecycle first. + if (this.state.passwordChangePhase !== undefined) { + throw new SeedlessOnboardingError( + SeedlessOnboardingControllerErrorMessage.PasswordChangeInProgress, + ); + } + // verify the old password of the encrypted vault await this.verifyVaultPassword(oldPassword, { skipLock: true, // skip lock since we already have the lock @@ -970,7 +999,10 @@ export class SeedlessOnboardingController< const attemptChangePassword = async (): Promise => { const { latestKeyIndex } = await this.#assertPasswordInSync({ skipCache: true, - skipLock: true, // skip lock since we already have the lock + // `changePassword` writes the phase before its token-refresh retry + // and guards concurrency itself at entry, so its own assert must + // not be blocked by the phase it just wrote. + skipPhaseCheck: true, }); // load keyring encryption key if it exists let keyringEncryptionKey: string | undefined; @@ -978,6 +1010,14 @@ export class SeedlessOnboardingController< keyringEncryptionKey = await this.loadKeyringEncryptionKey(); } + // Persist the lifecycle before the first remote mutation so a later + // crash or lost response leaves a recovery signal. The password change + // is never retried; recovery reconciles local state via the existing + // password-sync flow. + this.#writePasswordChangePhase( + SeedlessPasswordChangePhase.SeedlessChangePending, + ); + // update the encryption key with new password and update the Metadata Store const { encKey: newEncKey, @@ -989,20 +1029,22 @@ export class SeedlessOnboardingController< latestKeyIndex, }); - // update and encrypt the vault with new password - await this.#createNewVaultWithAuthData({ - password: newPassword, - rawToprfEncryptionKey: newEncKey, - rawToprfPwEncryptionKey: newPwEncKey, - rawToprfAuthKeyPair: newAuthKeyPair, - }); + // The remote Seedless change is committed. Persist the boundary so + // recovery knows the remote password is new. + this.#writePasswordChangePhase( + SeedlessPasswordChangePhase.SeedlessCommitted, + ); this.#resetPasswordOutdatedCache(); - // store the keyring encryption key if it exists - if (keyringEncryptionKey) { - await this.storeKeyringEncryptionKey(keyringEncryptionKey); - } + await this.#commitPasswordChangeState({ + password: newPassword, + encKey: newEncKey, + pwEncKey: newPwEncKey, + authKeyPair: newAuthKeyPair, + keyringEncryptionKey, + phase: SeedlessPasswordChangePhase.LocalKeyringPending, + }); }; try { @@ -1012,6 +1054,14 @@ export class SeedlessOnboardingController< ); } catch (error) { log('Error changing password', error); + // Preserve the last known lifecycle phase. The phase written before + // the failed step is the recovery signal: e.g. if `changeEncKey` + // rejected, the phase is `SEEDLESS_CHANGE_PENDING` and the client + // performs an authoritative password-outdated check to decide the + // recovery branch. Overwriting it with `UNKNOWN` here would discard + // that signal and leave the client unable to choose a branch. If the + // failure happened before the first lifecycle write, the lifecycle + // stays `IDLE` (nothing to recover). throw new SeedlessOnboardingError( SeedlessOnboardingControllerErrorMessage.FailedToChangePassword, { @@ -1153,66 +1203,34 @@ export class SeedlessOnboardingController< } /** - * Sync the latest global password to the controller. - * reset vault with latest globalPassword, - * persist the latest global password authPubKey + * Rewrite the local Seedless vault under the latest global password. * - * @param params - The parameters for syncing the latest global password. - * @param params.globalPassword - The latest global password. - * @returns A promise that resolves to the success of the operation. - */ - async syncLatestGlobalPassword({ - globalPassword, - }: { - globalPassword: string; - }): Promise { - return await this.#withControllerLock(async () => { - this.#assertIsUnlocked(); - const doSyncPassword = async (): Promise => { - // update vault with latest globalPassword - const { encKey, pwEncKey, authKeyPair } = - await this.#recoverEncKey(globalPassword); - // update and encrypt the vault with new password - await this.#createNewVaultWithAuthData({ - password: globalPassword, - rawToprfEncryptionKey: encKey, - rawToprfPwEncryptionKey: pwEncKey, - rawToprfAuthKeyPair: authKeyPair, - }); - - this.#resetPasswordOutdatedCache(); - }; - return await this.#executeWithTokenRefresh( - doSyncPassword, - 'syncLatestGlobalPassword', - ); - }); - } - - /** - * @description Unlock the controller with the latest global password. + * Rewrites the local Seedless vault under the latest global password, + * re-encrypts `encryptedKeyringEncryptionKey` under the new wrapping key, + * and resets the password-outdated cache. Must be called while the + * controller lock is held (or from a context that does not hold the lock, + * in which case the caller manages locking). * - * @param params - The parameters for unlocking the controller. - * @param params.maxKeyChainLength - The maximum chain length of the pwd encryption keys. - * @param params.globalPassword - The latest global password. - * @returns A promise that resolves to the success of the operation. + * @param globalPassword - The latest global password. */ - async submitGlobalPassword({ - globalPassword, - maxKeyChainLength = 5, - }: { - globalPassword: string; - maxKeyChainLength?: number; - }): Promise { - return await this.#withControllerLock(async () => { - return await this.#executeWithTokenRefresh(async () => { - const currentDeviceAuthPubKey = this.#recoverAuthPubKey(); - await this.#submitGlobalPassword({ - targetAuthPubKey: currentDeviceAuthPubKey, - globalPassword, - maxKeyChainLength, - }); - }, 'submitGlobalPassword'); + async #syncLatestGlobalPasswordInner(globalPassword: string): Promise { + // Decrypt under the old wrapping key before the vault rewrite so the + // ciphertext can be persisted under the new `toprfPwEncryptionKey`. + let keyringEncryptionKey: string | undefined; + if (this.state.encryptedKeyringEncryptionKey) { + keyringEncryptionKey = await this.loadKeyringEncryptionKey(); + } + + const { encKey, pwEncKey, authKeyPair } = + await this.#recoverEncKey(globalPassword); + this.#resetPasswordOutdatedCache(); + await this.#commitPasswordChangeState({ + password: globalPassword, + encKey, + pwEncKey, + authKeyPair, + keyringEncryptionKey, + phase: SeedlessPasswordChangePhase.LocalKeyringPending, }); } @@ -1262,7 +1280,8 @@ export class SeedlessOnboardingController< this.#setUnlocked(); // Pick the latest access token - the token from state might be newer (from refreshAuthTokens) - // than the token stored in the vault. The vault will be updated later by syncLatestGlobalPassword. + // than the token stored in the vault. The vault will be updated later + // by the password-sync flow. this.#pickLatestAccessToken( accessTokenBeforeUnlock, decryptedVaultData.accessToken, @@ -1287,12 +1306,10 @@ export class SeedlessOnboardingController< * @param options.globalAuthPubKey - The global auth public key to compare with the current auth public key. * If not provided, the global auth public key will be fetched from the backend. * @param options.skipCache - If true, bypass the cache and force a fresh check. - * @param options.skipLock - Whether to skip the lock acquisition. (to prevent deadlock in case the caller already acquired the lock) * @returns A promise that resolves to true if the password is outdated, false otherwise. */ - async checkIsPasswordOutdated(options?: { + async #checkIsPasswordOutdated(options?: { skipCache?: boolean; - skipLock?: boolean; globalAuthPubKey?: SEC1EncodedPublicKey; }): Promise { const doCheckIsPasswordExpired = async (): Promise => { @@ -1354,10 +1371,7 @@ export class SeedlessOnboardingController< }; return await this.#executeWithTokenRefresh( - async () => - options?.skipLock - ? await doCheckIsPasswordExpired() - : await this.#withControllerLock(doCheckIsPasswordExpired), + async () => await doCheckIsPasswordExpired(), 'checkIsPasswordOutdated', ); } @@ -1484,12 +1498,21 @@ export class SeedlessOnboardingController< * Store the keyring encryption key in state, encrypted under the current * encryption key. * + * Remains lifecycle-neutral: the client calls this during recovery without + * advancing the lifecycle phase. + * * @param keyringEncryptionKey - The keyring encryption key. */ async storeKeyringEncryptionKey(keyringEncryptionKey: string): Promise { const { toprfPwEncryptionKey: encKey } = await this.#unlockVaultAndGetVaultData(); - await this.#storeKeyringEncryptionKey(encKey, keyringEncryptionKey); + const encryptedKeyringEncryptionKey = this.#encryptKeyringEncryptionKey( + keyringEncryptionKey, + encKey, + ); + this.update((state) => { + state.encryptedKeyringEncryptionKey = encryptedKeyringEncryptionKey; + }); } /** @@ -1504,27 +1527,6 @@ export class SeedlessOnboardingController< return await this.#loadKeyringEncryptionKey(encKey); } - /** - * Encrypt the keyring encryption key and store it in state. - * - * @param encKey - The encryption key. - * @param keyringEncryptionKey - The keyring encryption key. - */ - async #storeKeyringEncryptionKey( - encKey: Uint8Array, - keyringEncryptionKey: string, - ): Promise { - const aes = managedNonce(gcm)(encKey); - const encryptedKeyringEncryptionKey = aes.encrypt( - utf8ToBytes(keyringEncryptionKey), - ); - this.update((state) => { - state.encryptedKeyringEncryptionKey = bytesToBase64( - encryptedKeyringEncryptionKey, - ); - }); - } - /** * Decrypt the keyring encryption key from state. * @@ -2075,74 +2077,191 @@ export class SeedlessOnboardingController< pwEncKey: Uint8Array; }): Promise { await this.#withVaultLock(async () => { - const serializedVaultData = serializeVaultData(vaultData); + const updatedState = await this.#createUpdatedVaultState({ + password, + vaultData, + pwEncKey, + }); - const { vaultEncryptionKey, vaultEncryptionSalt, vault } = this.state; + // update the state with the updated vault data + this.update((state) => { + state.vault = updatedState.vault; + state.vaultEncryptionKey = updatedState.vaultEncryptionKey; + state.vaultEncryptionSalt = updatedState.vaultEncryptionSalt; + state.encryptedSeedlessEncryptionKey = + updatedState.encryptedSeedlessEncryptionKey; + }); - const updatedState: Partial = { - vault, - vaultEncryptionKey, - vaultEncryptionSalt, - encryptedSeedlessEncryptionKey: - this.state.encryptedSeedlessEncryptionKey, - }; + // cache the vault data to avoid decrypting the vault data multiple times + this.#cachedDecryptedVaultData = vaultData; + }); + } - // if the password is provided (not undefined), encrypt the vault with the password - // We gonna prioritize the password encryption here, in case of the operation is `Change Password`. - // We don't wanna re-use the old encryption key from the state. - if (password !== undefined) { - assertIsValidPassword(password); - - // Note that vault encryption using the password is a very costly operation as it involves deriving the encryption key - // from the password using an intentionally slow key derivation function. - // We should make sure that we only call it very intentionally. - const { vault: updatedEncVault, exportedKeyString } = - await this.#vaultEncryptor.encryptWithDetail( - password, - serializedVaultData, - ); + /** + * Create the updated vault state without persisting it. + * + * This method must be called while the vault lock is held. Keeping vault + * encryption separate from the state update allows password-change flows to + * combine the vault fields with their other state changes in one update. + * + * @param params - The parameters for updating the vault. + * @param params.password - The optional password to encrypt the vault. + * @param params.vaultData - The raw vault data to update the vault with. + * @param params.pwEncKey - The global password encryption key. + * @returns The prepared vault state. + */ + async #createUpdatedVaultState({ + password, + vaultData, + pwEncKey, + }: { + password?: string; + vaultData: DeserializedVaultData; + pwEncKey: Uint8Array; + }): Promise { + const serializedVaultData = serializeVaultData(vaultData); - updatedState.vault = updatedEncVault; - updatedState.vaultEncryptionKey = exportedKeyString; - updatedState.vaultEncryptionSalt = JSON.parse(updatedEncVault).salt; - - // encrypt the seedless encryption key with the password encryption key from TOPRF network - updatedState.encryptedSeedlessEncryptionKey = - this.#encryptSeedlessEncryptionKey(exportedKeyString, pwEncKey); - } else if (vaultEncryptionKey && vaultEncryptionSalt) { - const encryptionKey = - await this.#vaultEncryptor.importKey(vaultEncryptionKey); - const updatedEncVault = await this.#vaultEncryptor.encryptWithKey( - encryptionKey, + const { vaultEncryptionKey, vaultEncryptionSalt, vault } = this.state; + + const updatedState: UpdatedVaultState = { + vault, + vaultEncryptionKey, + vaultEncryptionSalt, + encryptedSeedlessEncryptionKey: this.state.encryptedSeedlessEncryptionKey, + }; + + // if the password is provided (not undefined), encrypt the vault with the password + // We gonna prioritize the password encryption here, in case of the operation is `Change Password`. + // We don't wanna re-use the old encryption key from the state. + if (password !== undefined) { + assertIsValidPassword(password); + + // Note that vault encryption using the password is a very costly operation as it involves deriving the encryption key + // from the password using an intentionally slow key derivation function. + // We should make sure that we only call it very intentionally. + const { vault: updatedEncVault, exportedKeyString } = + await this.#vaultEncryptor.encryptWithDetail( + password, serializedVaultData, ); - // NOTE: Referenced from keyring-controller! - // We need to include the salt used to derive the encryption key, to be able to derive it from password again. - updatedEncVault.salt = vaultEncryptionSalt; + updatedState.vault = updatedEncVault; + updatedState.vaultEncryptionKey = exportedKeyString; + updatedState.vaultEncryptionSalt = JSON.parse(updatedEncVault).salt; + + // encrypt the seedless encryption key with the password encryption key from TOPRF network + updatedState.encryptedSeedlessEncryptionKey = + this.#encryptSeedlessEncryptionKey(exportedKeyString, pwEncKey); + } else if (vaultEncryptionKey && vaultEncryptionSalt) { + const encryptionKey = + await this.#vaultEncryptor.importKey(vaultEncryptionKey); + const updatedEncVault = await this.#vaultEncryptor.encryptWithKey( + encryptionKey, + serializedVaultData, + ); - updatedState.vault = JSON.stringify(updatedEncVault); - updatedState.vaultEncryptionKey = vaultEncryptionKey; - updatedState.vaultEncryptionSalt = vaultEncryptionSalt; - } else { - // neither password nor encryption key is provided - throw new Error( - SeedlessOnboardingControllerErrorMessage.MissingCredentials, - ); - } + // NOTE: Referenced from keyring-controller! + // We need to include the salt used to derive the encryption key, to be able to derive it from password again. + updatedEncVault.salt = vaultEncryptionSalt; + + updatedState.vault = JSON.stringify(updatedEncVault); + updatedState.vaultEncryptionKey = vaultEncryptionKey; + updatedState.vaultEncryptionSalt = vaultEncryptionSalt; + } else { + // neither password nor encryption key is provided + throw new Error( + SeedlessOnboardingControllerErrorMessage.MissingCredentials, + ); + } + + return updatedState; + } + + /** + * Persist the local Seedless state for a password change in one update. + * + * This is intentionally separate from the generic vault creation path. The + * password-change boundary includes the vault, authentication public key, + * Keyring encryption key, and lifecycle phase. + * + * @param params - The password-change state to persist. + * @param params.password - The password to encrypt the vault with. + * @param params.encKey - The TOPRF encryption key. + * @param params.pwEncKey - The TOPRF password encryption key. + * @param params.authKeyPair - The TOPRF authentication key pair. + * @param params.keyringEncryptionKey - The decrypted Keyring encryption key. + * @param params.phase - The lifecycle phase to persist. + */ + async #commitPasswordChangeState({ + password, + encKey, + pwEncKey, + authKeyPair, + keyringEncryptionKey, + phase, + }: { + password: string; + encKey: Uint8Array; + pwEncKey: Uint8Array; + authKeyPair: KeyPair; + keyringEncryptionKey?: string; + phase: SeedlessPasswordChangePhase; + }): Promise { + this.#assertIsAuthenticatedUser(this.state); + + const { accessToken, revokeToken } = + await this.#getAccessTokenAndRevokeToken(password); + const vaultData: DeserializedVaultData = { + toprfAuthKeyPair: authKeyPair, + toprfEncryptionKey: encKey, + toprfPwEncryptionKey: pwEncKey, + revokeToken, + accessToken, + }; + + await this.#withVaultLock(async () => { + const updatedVaultState = await this.#createUpdatedVaultState({ + password, + vaultData, + pwEncKey, + }); + const encryptedKeyringEncryptionKey = + keyringEncryptionKey === undefined + ? undefined + : this.#encryptKeyringEncryptionKey(keyringEncryptionKey, pwEncKey); - // update the state with the updated vault data this.update((state) => { - state.vault = updatedState.vault; - state.vaultEncryptionKey = updatedState.vaultEncryptionKey; - state.vaultEncryptionSalt = updatedState.vaultEncryptionSalt; + state.vault = updatedVaultState.vault; + state.vaultEncryptionKey = updatedVaultState.vaultEncryptionKey; + state.vaultEncryptionSalt = updatedVaultState.vaultEncryptionSalt; state.encryptedSeedlessEncryptionKey = - updatedState.encryptedSeedlessEncryptionKey; + updatedVaultState.encryptedSeedlessEncryptionKey; + state.authPubKey = bytesToBase64(authKeyPair.pk); + if (encryptedKeyringEncryptionKey !== undefined) { + state.encryptedKeyringEncryptionKey = encryptedKeyringEncryptionKey; + } + state.passwordChangePhase = phase; }); - // cache the vault data to avoid decrypting the vault data multiple times this.#cachedDecryptedVaultData = vaultData; }); + + this.#setUnlocked(); + } + + /** + * Encrypt the Keyring encryption key with the TOPRF password encryption key. + * + * @param keyringEncryptionKey - The Keyring encryption key. + * @param pwEncKey - The TOPRF password encryption key. + * @returns The encrypted Keyring encryption key in base64 format. + */ + #encryptKeyringEncryptionKey( + keyringEncryptionKey: string, + pwEncKey: Uint8Array, + ): string { + const aes = managedNonce(gcm)(pwEncKey); + return bytesToBase64(aes.encrypt(utf8ToBytes(keyringEncryptionKey))); } /** @@ -2238,6 +2357,292 @@ export class SeedlessOnboardingController< return await withLock(this.#vaultOperationMutex, callback); } + /** + * Persist a password-change phase boundary to controller state. + * + * The phase is a recovery signal only; it is persisted through the normal + * controller state-change flow (`persist: true` metadata). It is not proof + * that a remote or local operation completed — recovery must always + * re-verify actual remote and local state. + * + * Must be called while the controller lock is held. + * + * @param phase - The phase to persist, or `undefined` to clear (no change in + * progress). + */ + #writePasswordChangePhase( + phase: SeedlessPasswordChangePhase | undefined, + ): void { + this.update((state) => { + state.passwordChangePhase = phase; + }); + } + + /** + * Clear the password-change lifecycle. + * + * Used after a definitive remote failure (server did not commit) or once + * Keyring encryption-key synchronization is verified and all required local + * writes have succeeded. The controller clears the phase so the next unlock + * is normal. + * + * @returns A promise that resolves once the lifecycle has been cleared. + */ + async clearPasswordChangePhase(): Promise { + await this.#withControllerLock(async () => { + if (this.state.passwordChangePhase === undefined) { + return; + } + this.#writePasswordChangePhase(undefined); + }); + } + + /** + * Mark the password-change lifecycle as `KEY_SYNC_PENDING`. + * + * Called by the client coordinator before it synchronizes the current + * Keyring encryption key to Seedless. The controller only records the + * boundary; it does not perform or verify synchronization. + * + * @returns A promise that resolves once the phase has been persisted. + */ + async markPasswordChangeKeySyncPending(): Promise { + await this.#withControllerLock(async () => { + if ( + this.state.passwordChangePhase === + SeedlessPasswordChangePhase.KeySyncPending + ) { + return; + } + this.#writePasswordChangePhase( + SeedlessPasswordChangePhase.KeySyncPending, + ); + }); + } + + /** + * Resolve the current password-sync state without consuming a password. + * + * Merges the legacy `checkIsPasswordOutdated` read with password-change + * recovery routing, so the client makes a single call at unlock (both on + * page render and on password submit) and routes UI from the returned status. + * + * Phase handling: + * - No phase (`undefined`): run the authoritative outdated check. `skipCache` + * is honored, so the client can read from cache on render and force a remote + * call on submit. Returns `InSync` or `PasswordOutdated` (another device + * changed the remote password). + * - `SEEDLESS_CHANGE_PENDING`: the remote outcome is ambiguous, so `skipCache` + * is ignored and a remote check is forced. Clears the phase (remote did not + * commit) or advances to `SEEDLESS_COMMITTED` (remote committed). Returns + * `InSync` or `EnterNewPassword`. + * - Other phases: return the next recovery step without mutating state. + * + * This method does not consume a password; the client prompts for the + * correct password and then calls `reconcilePassword`. + * + * @param options - The options. + * @param options.skipCache - Whether to bypass the outdated cache. Ignored + * for `SEEDLESS_CHANGE_PENDING`, which always forces a remote check. + * @returns The sync/recovery resolution. On any failure the last known phase + * is preserved and `PasswordSyncStatus.Unknown` is returned. + */ + async resolvePasswordSyncState(options?: { + skipCache?: boolean; + }): Promise { + // The phase snapshot and any resulting check or transition must share the + // controller lock. Otherwise a concurrent password change can advance the + // phase after it is read and before this method acts on it. + return await this.#withControllerLock(async () => { + const phase = this.state.passwordChangePhase; + switch (phase) { + case undefined: { + try { + const outdated = await this.#checkIsPasswordOutdated({ + skipCache: options?.skipCache, + }); + return outdated + ? PasswordSyncStatus.PasswordOutdated + : PasswordSyncStatus.InSync; + } catch { + // Remote state could not be established. Keep the wallet locked. + return PasswordSyncStatus.Unknown; + } + } + case SeedlessPasswordChangePhase.SeedlessChangePending: { + try { + // Remote outcome is ambiguous; force an authoritative remote + // check regardless of `skipCache`. + const outdated = await this.#checkIsPasswordOutdated({ + skipCache: true, + }); + if (!outdated) { + // Remote did not commit. Clear the phase; unlock with the old + // password normally. + this.#writePasswordChangePhase(undefined); + return PasswordSyncStatus.InSync; + } + // Remote committed. Advance so recovery reconciles the local + // Seedless side with the new password. + this.#writePasswordChangePhase( + SeedlessPasswordChangePhase.SeedlessCommitted, + ); + return PasswordSyncStatus.EnterNewPassword; + } catch { + // Remote state could not be established. Preserve the phase and + // keep the wallet locked. + return PasswordSyncStatus.Unknown; + } + } + case SeedlessPasswordChangePhase.SeedlessCommitted: + return PasswordSyncStatus.EnterNewPassword; + case SeedlessPasswordChangePhase.LocalKeyringPending: + return PasswordSyncStatus.ReconcileKeyring; + default: + // Terminal phases (KEY_SYNC_PENDING, UNKNOWN) and any unrecognized + // persisted value share routing. + return this.#statusForTerminalPhase(phase); + } + }); + } + + /** + * Reconcile the local Seedless password with the remote password. + * + * For `SEEDLESS_COMMITTED` or `LOCAL_KEYRING_PENDING` it re-runs the existing + * password-sync flow (chain unlock + local vault rewrite) + * with the new password and advances the phase to `LOCAL_KEYRING_PENDING`. + * These operations are idempotent, so re-running them is safe whether or not + * the local Seedless vault was already rewritten. The controller is left + * unlocked. + * + * For no phase (`undefined`) it re-checks whether the remote password is + * outdated. If it is, it runs the same password-sync flow, advances to + * `LOCAL_KEYRING_PENDING`, and returns `ReconcileKeyring` so the client can + * reconcile the local Keyring (e.g. after another device changed the remote + * password). If the remote password is not outdated it is a no-op. + * + * The client remains responsible for the Keyring side (classifying the local + * Keyring via `KeyringController:verifyPassword` and running the old-Keyring + * or new-Keyring branch), because this controller does not depend on + * `KeyringController`. See + * [0003](./docs/0003-controller-owned-password-change-recovery-plan.md). + * + * @param params - The reconciliation parameters. + * @param params.globalPassword - The current global password. + * @returns The reconciliation result. On any failure the last known phase is + * preserved and `PasswordSyncStatus.Unknown` is returned. + */ + async reconcilePassword({ + globalPassword, + }: { + globalPassword: string; + }): Promise { + return await this.#withControllerLock(async () => { + const phase = this.state.passwordChangePhase; + switch (phase) { + case SeedlessPasswordChangePhase.SeedlessChangePending: + // Remote state must be resolved first via + // resolvePasswordSyncState. + return PasswordSyncStatus.Unknown; + case SeedlessPasswordChangePhase.SeedlessCommitted: + case SeedlessPasswordChangePhase.LocalKeyringPending: { + try { + // Re-run the password-sync flow with the new password. This + // unlocks the controller and rewrites the local Seedless vault; + // both operations are idempotent if the vault is already synced. + await this.#runPasswordSyncFlow(globalPassword); + return PasswordSyncStatus.ReconcileKeyring; + } catch { + // Reconciliation failed (e.g. wrong password or transient + // remote error). Preserve the phase and keep the wallet locked. + return PasswordSyncStatus.Unknown; + } + } + case undefined: { + // No local password-change lifecycle is in flight. Another device + // may still have changed the remote password, so re-check and sync + // the Seedless side if it is outdated. A phase is then recorded so + // the client reconciles the local Keyring. + try { + const outdated = await this.#checkIsPasswordOutdated({ + skipCache: true, + }); + if (!outdated) { + return PasswordSyncStatus.InSync; + } + // The remote password is known to be newer. Record that boundary + // before starting the local Seedless rewrite so an interrupted + // flow remains recoverable. + this.#writePasswordChangePhase( + SeedlessPasswordChangePhase.SeedlessCommitted, + ); + await this.#runPasswordSyncFlow(globalPassword); + return PasswordSyncStatus.ReconcileKeyring; + } catch { + // Sync failed (e.g. wrong password or transient remote error). + // Keep the wallet locked. + return PasswordSyncStatus.Unknown; + } + } + default: + // Terminal phases (KEY_SYNC_PENDING, UNKNOWN) and any unrecognized + // persisted value share routing. + return this.#statusForTerminalPhase(phase); + } + }); + } + + /** + * Re-run the password-sync flow (chain unlock + local vault rewrite) with + * the supplied password. + * + * Both operations are idempotent if the local Seedless vault is already + * synced, so this is safe to re-run during recovery or a plain + * another-device sync. The controller is left unlocked. Caller must hold + * the controller lock. + * + * @param globalPassword - The current global password. + */ + async #runPasswordSyncFlow(globalPassword: string): Promise { + await this.#executeWithTokenRefresh(async () => { + const currentDeviceAuthPubKey = this.#recoverAuthPubKey(); + await this.#submitGlobalPassword({ + targetAuthPubKey: currentDeviceAuthPubKey, + globalPassword, + maxKeyChainLength: 5, + }); + }, 'submitGlobalPassword'); + await this.#executeWithTokenRefresh( + async () => await this.#syncLatestGlobalPasswordInner(globalPassword), + 'syncLatestGlobalPassword', + ); + } + + /** + * Return the recovery status for phases that require no Seedless-side + * mutation. Shared by `resolvePasswordSyncState` (read) and + * `reconcilePassword` (apply) so both route the terminal phases + * identically. + * + * @param phase - The persisted password-change phase. + * @returns The status for the phase. An unrecognized persisted value is + * treated as no change in progress and returns `InSync`. + */ + #statusForTerminalPhase( + phase: SeedlessPasswordChangePhase, + ): PasswordSyncStatus { + switch (phase) { + case SeedlessPasswordChangePhase.KeySyncPending: + return PasswordSyncStatus.SyncKey; + case SeedlessPasswordChangePhase.Unknown: + return PasswordSyncStatus.Unknown; + default: + // An unrecognized persisted phase is treated as no change in progress. + return PasswordSyncStatus.InSync; + } + } + /** * Parse and deserialize the authentication data from the vault. * @@ -2300,18 +2705,42 @@ export class SeedlessOnboardingController< * * @param options - The options for asserting the password is in sync. * @param options.skipCache - Whether to skip the cache check. - * @param options.skipLock - Whether to skip the lock acquisition. (to prevent deadlock in case the caller already acquired the lock) + * @param options.skipPhaseCheck - Whether to skip the `passwordChangePhase` guard. Only `changePassword` should set this: it guards concurrency itself at entry and writes the phase before its token-refresh retry, so its own internal assert must not be blocked by the phase it just wrote. * @returns The global auth public key and the latest key index. * @throws If the password is outdated. */ async #assertPasswordInSync(options?: { skipCache?: boolean; - skipLock?: boolean; + /** + * Skip the `passwordChangePhase` guard. Only `changePassword` should set + * this: it guards concurrency itself at entry and writes the phase + * before its token-refresh retry, so its own internal assert must not + * be blocked by the phase it just wrote. + */ + skipPhaseCheck?: boolean; }): Promise<{ authPubKey: SEC1EncodedPublicKey; latestKeyIndex: number; }> { this.#assertIsAuthenticatedUser(this.state); + + // Block TOPRF operations while a password change is unresolved. The + // controller mutex serializes in-process calls, but a previous change may + // have left a persisted phase after a crash. Running a fresh TOPRF + // operation against ambiguous state could corrupt recovery. Recovery + // itself bypasses this assert (it calls the password-sync primitives + // directly), so this guard does not block reconciliation. `changePassword` + // passes `skipPhaseCheck` because it writes the phase before its own + // token-refresh retry and already guards concurrency at entry. + if ( + !options?.skipPhaseCheck && + this.state.passwordChangePhase !== undefined + ) { + throw new SeedlessOnboardingError( + SeedlessOnboardingControllerErrorMessage.PasswordChangeInProgress, + ); + } + const { nodeAuthTokens, authConnectionId, @@ -2335,7 +2764,7 @@ export class SeedlessOnboardingController< }, ); }); - const isPasswordOutdated = await this.checkIsPasswordOutdated({ + const isPasswordOutdated = await this.#checkIsPasswordOutdated({ ...options, globalAuthPubKey: authPubKey, }); diff --git a/packages/seedless-onboarding-controller/src/constants.ts b/packages/seedless-onboarding-controller/src/constants.ts index e89287ef7cd..bbc3b51ae79 100644 --- a/packages/seedless-onboarding-controller/src/constants.ts +++ b/packages/seedless-onboarding-controller/src/constants.ts @@ -25,6 +25,52 @@ export enum SeedlessOnboardingMigrationVersion { V1 = 1, } +/** + * The lifecycle phase of a Seedless password-change operation. + * + * Used as a recovery signal only — it is not proof that a remote or local + * operation completed. Recovery must always verify actual remote and local + * state before acting on the phase. + */ +export enum SeedlessPasswordChangePhase { + /** A password change has started but the remote Seedless result is not yet confirmed. */ + SeedlessChangePending = 'SEEDLESS_CHANGE_PENDING', + /** The remote Seedless password change is confirmed committed. */ + SeedlessCommitted = 'SEEDLESS_COMMITTED', + /** The local Seedless vault has been rewritten with the new password. */ + LocalKeyringPending = 'LOCAL_KEYRING_PENDING', + /** The local Keyring encryption key has been stored; awaiting final verification. */ + KeySyncPending = 'KEY_SYNC_PENDING', + /** The result of one or more steps could not be established. */ + Unknown = 'UNKNOWN', +} + +/** + * The next step for the client after a password-sync or password-change + * recovery check. Returned by `resolvePasswordSyncState` (read + resolve, no + * password) and `reconcilePassword` (apply, with password). + * + * Covers both an interrupted local password change and an another-device + * password change. The controller owns Seedless-side sequencing; the client + * owns the Keyring-side steps (it must call `KeyringController` directly) + * and UI routing based on this status. See + * [0002](./docs/0002-password-change-recovery-flow.md). + */ +export enum PasswordSyncStatus { + /** The local and remote passwords are synchronized; no recovery action is needed. Unlock normally. */ + InSync = 'in-sync', + /** No lifecycle is in flight but the remote password changed (e.g. another device changed it). Prompt for the new password, then call `reconcilePassword`. */ + PasswordOutdated = 'password-outdated', + /** Remote committed (or the local Seedless side still needs the new password). Prompt for the new password, then call `reconcilePassword`. */ + EnterNewPassword = 'enter-new-password', + /** The Seedless side is reconciled (phase is `LOCAL_KEYRING_PENDING`). The client must cryptographically classify the local Keyring and run the old/new branch. */ + ReconcileKeyring = 'reconcile-keyring', + /** Phase is `KEY_SYNC_PENDING`. The client must export, store, and sync the current Keyring encryption key, then call `clearPasswordChangePhase`. */ + SyncKey = 'sync-key', + /** The remote or local state could not be established. Keep the wallet locked. The last known phase is preserved. */ + Unknown = 'unknown', +} + export enum SeedlessOnboardingControllerErrorMessage { ControllerLocked = `${controllerName} - The operation cannot be completed while the controller is locked.`, VaultLocked = `${controllerName} - The operation cannot be completed while the vault is locked.`, @@ -51,6 +97,7 @@ export enum SeedlessOnboardingControllerErrorMessage { NoSecretDataFound = `${controllerName} - No secret data found`, InvalidPrimarySecretDataType = `${controllerName} - Primary secret data must be of type mnemonic.`, FailedToChangePassword = `${controllerName} - Failed to change password`, + PasswordChangeInProgress = `${controllerName} - A password change is already in progress; recovery must finish before starting a new one`, TooManyLoginAttempts = `${controllerName} - Too many login attempts`, IncorrectPassword = `${controllerName} - Incorrect password`, OutdatedPassword = `${controllerName} - Outdated password`, diff --git a/packages/seedless-onboarding-controller/src/index.ts b/packages/seedless-onboarding-controller/src/index.ts index cf220239b06..7f9289db5cb 100644 --- a/packages/seedless-onboarding-controller/src/index.ts +++ b/packages/seedless-onboarding-controller/src/index.ts @@ -18,14 +18,13 @@ export type { SeedlessOnboardingControllerAddNewSecretDataAction, SeedlessOnboardingControllerFetchAllSecretDataAction, SeedlessOnboardingControllerChangePasswordAction, + SeedlessOnboardingControllerClearPasswordChangePhaseAction, + SeedlessOnboardingControllerMarkPasswordChangeKeySyncPendingAction, SeedlessOnboardingControllerUpdateBackupMetadataStateAction, SeedlessOnboardingControllerVerifyVaultPasswordAction, SeedlessOnboardingControllerGetSecretDataBackupStateAction, SeedlessOnboardingControllerSubmitPasswordAction, SeedlessOnboardingControllerSetLockedAction, - SeedlessOnboardingControllerSyncLatestGlobalPasswordAction, - SeedlessOnboardingControllerSubmitGlobalPasswordAction, - SeedlessOnboardingControllerCheckIsPasswordOutdatedAction, SeedlessOnboardingControllerGetIsUserAuthenticatedAction, SeedlessOnboardingControllerClearStateAction, SeedlessOnboardingControllerStoreKeyringEncryptionKeyAction, @@ -38,6 +37,8 @@ export type { SeedlessOnboardingControllerCheckMetadataAccessTokenExpiredAction, SeedlessOnboardingControllerCheckAccessTokenExpiredAction, SeedlessOnboardingControllerRunMigrationsAction, + SeedlessOnboardingControllerResolvePasswordSyncStateAction, + SeedlessOnboardingControllerReconcilePasswordAction, } from './SeedlessOnboardingController-method-action-types.js'; export type { AuthenticatedUserDetails, @@ -53,6 +54,8 @@ export { SeedlessOnboardingMigrationVersion, AuthConnection, SecretType, + SeedlessPasswordChangePhase, + PasswordSyncStatus, } from './constants.js'; export { SecretMetadata } from './SecretMetadata.js'; export { diff --git a/packages/seedless-onboarding-controller/src/types.ts b/packages/seedless-onboarding-controller/src/types.ts index f69c8f1d92a..4aab1ccf6cb 100644 --- a/packages/seedless-onboarding-controller/src/types.ts +++ b/packages/seedless-onboarding-controller/src/types.ts @@ -5,7 +5,11 @@ import type { } from '@metamask/toprf-secure-backup'; import type { MutexInterface } from 'async-mutex'; -import type { AuthConnection, SecretType } from './constants.js'; +import type { + AuthConnection, + SecretType, + SeedlessPasswordChangePhase, +} from './constants.js'; /** * The backup state of the secret data. @@ -106,6 +110,7 @@ export type InvalidPrimarySecretDataTypeErrorData = ( )[]; // State + export type SeedlessOnboardingControllerState = Partial & Partial & { @@ -190,6 +195,12 @@ export type SeedlessOnboardingControllerState = * Used to prevent re-running migrations. */ migrationVersion: number; + + /** + * The persisted last-known phase of an in-progress or unresolved + * password-change operation. Missing or `undefined` means `IDLE`. + */ + passwordChangePhase?: SeedlessPasswordChangePhase; }; /** diff --git a/packages/seedless-onboarding-controller/src/utils.test.ts b/packages/seedless-onboarding-controller/src/utils.test.ts index d1a922d3b5d..4f1ed107ea6 100644 --- a/packages/seedless-onboarding-controller/src/utils.test.ts +++ b/packages/seedless-onboarding-controller/src/utils.test.ts @@ -7,9 +7,9 @@ import { SecretType } from './constants.js'; import { SecretMetadata } from './SecretMetadata.js'; import type { DecodedNodeAuthToken } from './types.js'; import { - decodeNodeAuthToken, - decodeJWTToken, compareAndGetLatestToken, + decodeJWTToken, + decodeNodeAuthToken, getInvalidPrimarySecretDataTypeErrorData, getSecretTypeFromDataType, } from './utils.js';