Skip to content

seedless controller: store encrypted password locally #5988

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

matthiasgeihs
Copy link
Contributor

@matthiasgeihs matthiasgeihs commented Jun 15, 2025

Explanation

Resolves https://github.com/MetaMask/decisions/pull/85. Implements Option 7.

Stores encrypted password locally instead of writing it to the backup store.
Resolves an audit finding as mentioned in the above referenced ADR.

References

Changelog

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed, highlighting breaking changes as necessary
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

@matthiasgeihs matthiasgeihs force-pushed the mg/seedless-controller/store-encrypted-password branch from a91643a to fde625f Compare June 15, 2025 18:52
@matthiasgeihs matthiasgeihs force-pushed the mg/seedless-controller/store-encrypted-password branch from fde625f to d327019 Compare June 15, 2025 19:09
@matthiasgeihs
Copy link
Contributor Author

@metamaskbot publish-preview

@matthiasgeihs matthiasgeihs changed the title (wip) store encrypted password locally seedless controller: store encrypted password locally Jun 15, 2025
@matthiasgeihs matthiasgeihs marked this pull request as ready for review June 15, 2025 19:16
@matthiasgeihs matthiasgeihs requested a review from a team as a code owner June 15, 2025 19:16
Copy link
Contributor

Preview builds have been published. See these instructions for more information about preview builds.

Expand for full list of packages and versions.
{
  "@metamask-previews/account-tree-controller": "0.1.1-preview-d327019",
  "@metamask-previews/accounts-controller": "30.0.0-preview-d327019",
  "@metamask-previews/address-book-controller": "6.1.0-preview-d327019",
  "@metamask-previews/announcement-controller": "7.0.3-preview-d327019",
  "@metamask-previews/app-metadata-controller": "1.0.0-preview-d327019",
  "@metamask-previews/approval-controller": "7.1.3-preview-d327019",
  "@metamask-previews/assets-controllers": "68.2.0-preview-d327019",
  "@metamask-previews/base-controller": "8.0.1-preview-d327019",
  "@metamask-previews/bridge-controller": "32.1.2-preview-d327019",
  "@metamask-previews/bridge-status-controller": "29.1.1-preview-d327019",
  "@metamask-previews/build-utils": "3.0.3-preview-d327019",
  "@metamask-previews/chain-agnostic-permission": "0.7.1-preview-d327019",
  "@metamask-previews/composable-controller": "11.0.0-preview-d327019",
  "@metamask-previews/controller-utils": "11.10.0-preview-d327019",
  "@metamask-previews/delegation-controller": "0.4.0-preview-d327019",
  "@metamask-previews/earn-controller": "1.1.1-preview-d327019",
  "@metamask-previews/eip1193-permission-middleware": "0.1.0-preview-d327019",
  "@metamask-previews/ens-controller": "16.0.0-preview-d327019",
  "@metamask-previews/error-reporting-service": "1.0.0-preview-d327019",
  "@metamask-previews/eth-json-rpc-provider": "4.1.8-preview-d327019",
  "@metamask-previews/foundryup": "1.0.0-preview-d327019",
  "@metamask-previews/gas-fee-controller": "23.0.0-preview-d327019",
  "@metamask-previews/json-rpc-engine": "10.0.3-preview-d327019",
  "@metamask-previews/json-rpc-middleware-stream": "8.0.7-preview-d327019",
  "@metamask-previews/keyring-controller": "22.0.2-preview-d327019",
  "@metamask-previews/logging-controller": "6.0.4-preview-d327019",
  "@metamask-previews/message-manager": "12.0.1-preview-d327019",
  "@metamask-previews/multichain-api-middleware": "0.4.0-preview-d327019",
  "@metamask-previews/multichain-network-controller": "0.8.0-preview-d327019",
  "@metamask-previews/multichain-transactions-controller": "2.0.0-preview-d327019",
  "@metamask-previews/name-controller": "8.0.3-preview-d327019",
  "@metamask-previews/network-controller": "23.6.0-preview-d327019",
  "@metamask-previews/notification-services-controller": "10.0.0-preview-d327019",
  "@metamask-previews/permission-controller": "11.0.6-preview-d327019",
  "@metamask-previews/permission-log-controller": "3.0.3-preview-d327019",
  "@metamask-previews/phishing-controller": "12.6.0-preview-d327019",
  "@metamask-previews/polling-controller": "13.0.0-preview-d327019",
  "@metamask-previews/preferences-controller": "18.1.0-preview-d327019",
  "@metamask-previews/profile-sync-controller": "17.1.0-preview-d327019",
  "@metamask-previews/queued-request-controller": "10.0.0-preview-d327019",
  "@metamask-previews/rate-limit-controller": "6.0.3-preview-d327019",
  "@metamask-previews/remote-feature-flag-controller": "1.6.0-preview-d327019",
  "@metamask-previews/sample-controllers": "0.1.0-preview-d327019",
  "@metamask-previews/seedless-onboarding-controller": "1.0.0-preview-d327019",
  "@metamask-previews/selected-network-controller": "22.1.0-preview-d327019",
  "@metamask-previews/signature-controller": "30.0.0-preview-d327019",
  "@metamask-previews/token-search-discovery-controller": "3.3.0-preview-d327019",
  "@metamask-previews/transaction-controller": "57.3.0-preview-d327019",
  "@metamask-previews/user-operation-controller": "36.0.0-preview-d327019"
}

@matthiasgeihs matthiasgeihs requested a review from a team as a code owner June 15, 2025 19:19
@matthiasgeihs matthiasgeihs force-pushed the mg/seedless-controller/store-encrypted-password branch from 8bb56c8 to 22b6fd0 Compare June 15, 2025 19:20
@@ -119,6 +122,10 @@ const seedlessOnboardingMetadata: StateMetadata<SeedlessOnboardingControllerStat
persist: false,
anonymous: true,
},
encryptedPassword: {
persist: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I know why do we need to persist this?

Copy link
Contributor Author

@matthiasgeihs matthiasgeihs Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because you need it when the keyring vault is locked.

let's say you are locked out of your device.
now you get the message, password changed.

you enter new password.
the seedless controller will fetch the encryption key that was used to encrypt your old password here.
will load the encryptedPassword, decrypt it, and return it.

so now other controller can log the user in.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Thanks for the explanation.

@himanshuchawla009
Copy link
Contributor

we should close this PR since we are going with option 6 @matthiasgeihs , i am changing the status of this to draft meanwhile to prevent the merge.

@himanshuchawla009 himanshuchawla009 marked this pull request as draft June 22, 2025 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants