-
-
Notifications
You must be signed in to change notification settings - Fork 234
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
base: main
Are you sure you want to change the base?
seedless controller: store encrypted password locally #5988
Conversation
a91643a
to
fde625f
Compare
fde625f
to
d327019
Compare
@metamaskbot publish-preview |
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions.
|
8bb56c8
to
22b6fd0
Compare
@@ -119,6 +122,10 @@ const seedlessOnboardingMetadata: StateMetadata<SeedlessOnboardingControllerStat | |||
persist: false, | |||
anonymous: true, | |||
}, | |||
encryptedPassword: { | |||
persist: true, |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
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. |
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