-
-
Notifications
You must be signed in to change notification settings - Fork 129
Keep User Logged In: Export key for encrypted key login #152
Changes from all commits
c25efe0
e239774
3c4e88d
eb8b357
aed7c3b
97ebd51
4d62780
fb461b5
1ff9e9d
b200969
52605f9
645ecd9
8e8e771
6e840cc
76d1adf
ef8dc4f
2dafa98
751fbfe
db4dc1d
e141b84
2a2e53b
0ace5f5
a5fe1dc
e28b476
7cd4d40
dc73e95
a321458
e594170
1093c17
76c0123
72145ba
aedfbe1
46f39db
76aa90d
5ef3bb6
3694ce5
4769f37
a4e01d0
37af169
4021e0c
81cd50a
12d8365
a7a7bc7
2d4b51d
78412cb
9a0343e
0071c04
7907b34
9178496
88299ef
3848e04
c137eae
83e0973
a5fe207
aacd2a1
ad5169d
cc7ff86
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,7 @@ const { EventEmitter } = require('events'); | |
| const { Buffer } = require('buffer'); | ||
| const bip39 = require('@metamask/bip39'); | ||
| const ObservableStore = require('obs-store'); | ||
| const encryptor = require('browser-passworder'); | ||
| const encryptor = require('@metamask/browser-passworder'); | ||
| const { normalize: normalizeAddress } = require('eth-sig-util'); | ||
|
|
||
| const SimpleKeyring = require('eth-simple-keyring'); | ||
|
|
@@ -14,6 +14,7 @@ const KEYRINGS_TYPE_MAP = { | |
| HD_KEYRING: 'HD Key Tree', | ||
| SIMPLE_KEYRING: 'Simple Key Pair', | ||
| }; | ||
|
|
||
| /** | ||
| * Strip the hex prefix from an address, if present | ||
| * @param {string} address - The address that might be hex prefixed. | ||
|
|
@@ -40,12 +41,17 @@ class KeyringController extends EventEmitter { | |
| this.store = new ObservableStore(initState); | ||
| this.memStore = new ObservableStore({ | ||
| isUnlocked: false, | ||
| keyringTypes: this.keyringTypes.map((krt) => krt.type), | ||
| keyringTypes: this.keyringTypes.map((keyringType) => keyringType.type), | ||
| keyrings: [], | ||
| encryptionKey: null, | ||
| }); | ||
|
|
||
| this.encryptor = opts.encryptor || encryptor; | ||
| this.keyrings = []; | ||
|
|
||
| // This option allows the controller to cache an exported key | ||
| // for use in decrypting and encrypting data without password | ||
| this.cacheEncryptionKey = Boolean(opts.cacheEncryptionKey); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -143,9 +149,15 @@ class KeyringController extends EventEmitter { | |
| * @returns {Promise<Object>} A Promise that resolves to the state. | ||
| */ | ||
| async setLocked() { | ||
| delete this.password; | ||
|
|
||
| // set locked | ||
| this.password = null; | ||
| this.memStore.updateState({ isUnlocked: false }); | ||
| this.memStore.updateState({ | ||
| isUnlocked: false, | ||
| encryptionKey: null, | ||
| encryptionSalt: null, | ||
| }); | ||
|
|
||
| // remove keyrings | ||
| this.keyrings = []; | ||
| await this._updateMemStoreKeyrings(); | ||
|
|
@@ -168,6 +180,28 @@ class KeyringController extends EventEmitter { | |
| */ | ||
| async submitPassword(password) { | ||
| this.keyrings = await this.unlockKeyrings(password); | ||
|
|
||
| this.setUnlocked(); | ||
| this.fullUpdate(); | ||
| } | ||
|
|
||
| /** | ||
| * Submit Encryption Key | ||
| * | ||
| * Attempts to decrypt the current vault and load its keyrings | ||
| * into memory based on the vault and CryptoKey information | ||
| * | ||
| * @emits KeyringController#unlock | ||
| * @param {string} encryptionKey - The encrypted key information used to decrypt the vault | ||
| * @param {string} encryptionSalt - The salt used to generate the last key | ||
| * @returns {Promise<Object>} A Promise that resolves to the state. | ||
| */ | ||
| async submitEncryptionKey(encryptionKey, encryptionSalt) { | ||
| this.keyrings = await this.unlockKeyrings( | ||
| undefined, | ||
| encryptionKey, | ||
| encryptionSalt, | ||
| ); | ||
| this.setUnlocked(); | ||
| this.fullUpdate(); | ||
| } | ||
|
|
@@ -215,7 +249,6 @@ class KeyringController extends EventEmitter { | |
| this.keyrings.push(keyring); | ||
| await this.persistAllKeyrings(); | ||
|
|
||
| await this._updateMemStoreKeyrings(); | ||
| this.fullUpdate(); | ||
|
|
||
| return keyring; | ||
|
|
@@ -297,7 +330,6 @@ class KeyringController extends EventEmitter { | |
| }); | ||
|
|
||
| await this.persistAllKeyrings(); | ||
| await this._updateMemStoreKeyrings(); | ||
| this.fullUpdate(); | ||
| } | ||
|
|
||
|
|
@@ -347,7 +379,6 @@ class KeyringController extends EventEmitter { | |
| } | ||
|
|
||
| await this.persistAllKeyrings(); | ||
| await this._updateMemStoreKeyrings(); | ||
| this.fullUpdate(); | ||
| } | ||
|
|
||
|
|
@@ -513,6 +544,14 @@ class KeyringController extends EventEmitter { | |
| * @returns {Promise<boolean>} Resolves to true once keyrings are persisted. | ||
| */ | ||
| async persistAllKeyrings() { | ||
| const { encryptionKey, encryptionSalt } = this.memStore.getState(); | ||
|
|
||
| if (!this.password && !encryptionKey) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the difference between holding something in memory store vs class field? Why do we use/need both? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Putting the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't generally have a use for public class fields; the password field should be private. That's a change I was going to make in a separate PR at some point; it would be a breaking change because |
||
| throw new Error( | ||
| 'Cannot persist vault without password and encryption key', | ||
legobeat marked this conversation as resolved.
Show resolved
Hide resolved
darkwing marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ); | ||
| } | ||
|
|
||
| const serializedKeyrings = await Promise.all( | ||
| this.keyrings.map(async (keyring) => { | ||
| const [type, data] = await Promise.all([ | ||
|
|
@@ -522,11 +561,48 @@ class KeyringController extends EventEmitter { | |
| return { type, data }; | ||
| }), | ||
| ); | ||
| const encryptedString = await this.encryptor.encrypt( | ||
| this.password, | ||
| serializedKeyrings, | ||
| ); | ||
| this.store.updateState({ vault: encryptedString }); | ||
|
|
||
| let vault; | ||
| let newEncryptionKey; | ||
|
|
||
| if (this.cacheEncryptionKey) { | ||
| if (this.password) { | ||
| const { vault: newVault, exportedKeyString } = | ||
| await this.encryptor.encryptWithDetail( | ||
| this.password, | ||
| serializedKeyrings, | ||
| ); | ||
|
|
||
| vault = newVault; | ||
| newEncryptionKey = exportedKeyString; | ||
| } else if (encryptionKey) { | ||
darkwing marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| const key = await this.encryptor.importKey(encryptionKey); | ||
| const vaultJSON = await this.encryptor.encryptWithKey( | ||
| key, | ||
| serializedKeyrings, | ||
| ); | ||
| vaultJSON.salt = encryptionSalt; | ||
| vault = JSON.stringify(vaultJSON); | ||
| } | ||
| } else { | ||
| vault = await this.encryptor.encrypt(this.password, serializedKeyrings); | ||
| } | ||
|
|
||
| if (!vault) { | ||
| throw new Error('Cannot persist vault without vault information'); | ||
| } | ||
|
|
||
| this.store.updateState({ vault }); | ||
|
|
||
| // The keyring updates need to be announced before updating the encryptionKey | ||
| // so that the updated keyring gets propagated to the extension first. | ||
| // Not calling _updateMemStoreKeyrings results in the wrong account being selected | ||
| // in the extension. | ||
| await this._updateMemStoreKeyrings(); | ||
darkwing marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (newEncryptionKey) { | ||
| this.memStore.updateState({ encryptionKey: newEncryptionKey }); | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
|
|
@@ -537,17 +613,55 @@ class KeyringController extends EventEmitter { | |
| * initializing the persisted keyrings to RAM. | ||
| * | ||
| * @param {string} password - The keyring controller password. | ||
| * @param {string} encryptionKey - An exported key string to unlock keyrings with | ||
| * @param {string} encryptionSalt - The salt used to encrypt the vault | ||
| * @returns {Promise<Array<Keyring>>} The keyrings. | ||
| */ | ||
| async unlockKeyrings(password) { | ||
| async unlockKeyrings(password, encryptionKey, encryptionSalt) { | ||
| const encryptedVault = this.store.getState().vault; | ||
| if (!encryptedVault) { | ||
| throw new Error('Cannot unlock without a previous vault.'); | ||
| } | ||
|
|
||
| await this.clearKeyrings(); | ||
| const vault = await this.encryptor.decrypt(password, encryptedVault); | ||
| this.password = password; | ||
|
|
||
| let vault; | ||
|
|
||
| if (this.cacheEncryptionKey) { | ||
| if (password) { | ||
| const result = await this.encryptor.decryptWithDetail( | ||
| password, | ||
| encryptedVault, | ||
| ); | ||
| vault = result.vault; | ||
| this.password = password; | ||
|
|
||
| this.memStore.updateState({ | ||
| encryptionKey: result.exportedKeyString, | ||
| encryptionSalt: result.salt, | ||
| }); | ||
| } else { | ||
| const parsedEncryptedVault = JSON.parse(encryptedVault); | ||
|
|
||
| if (encryptionSalt !== parsedEncryptedVault.salt) { | ||
| throw new Error('Encryption key and salt provided are expired'); | ||
| } | ||
|
|
||
| const key = await this.encryptor.importKey(encryptionKey); | ||
| vault = await this.encryptor.decryptWithKey(key, parsedEncryptedVault); | ||
|
|
||
| // This call is required on the first call because encryptionKey | ||
| // is not yet inside the memStore | ||
| this.memStore.updateState({ | ||
| encryptionKey, | ||
| encryptionSalt, | ||
| }); | ||
| } | ||
| } else { | ||
| vault = await this.encryptor.decrypt(password, encryptedVault); | ||
| this.password = password; | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (nit) This if structure looks similar across both functions. I'd be tempted to create a function that accepts state and descriptively named functions as reactions to certain states. |
||
|
|
||
| await Promise.all(vault.map(this._restoreKeyring.bind(this))); | ||
| await this._updateMemStoreKeyrings(); | ||
| return this.keyrings; | ||
|
|
@@ -602,7 +716,7 @@ class KeyringController extends EventEmitter { | |
| * @returns {Keyring|undefined} The class, if it exists. | ||
| */ | ||
| getKeyringClassForType(type) { | ||
| return this.keyringTypes.find((kr) => kr.type === type); | ||
| return this.keyringTypes.find((keyring) => keyring.type === type); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -744,7 +858,6 @@ class KeyringController extends EventEmitter { | |
| if (keyring.forgetDevice) { | ||
| keyring.forgetDevice(); | ||
| this.persistAllKeyrings(); | ||
| this._updateMemStoreKeyrings.bind(this)(); | ||
| } else { | ||
| throw new Error( | ||
| `KeyringController - keyring does not have method "forgetDevice", keyring type: ${keyring.type}`, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.