From 7771c5d3d9bce749844298615d4fbf7293c6aa8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Thu, 12 May 2022 10:28:45 +0200 Subject: [PATCH] wallet: create password with multi-account by default --- .../src/operations/mixnet/account.rs | 7 +- .../src/wallet_storage/account_data.rs | 18 +- .../src-tauri/src/wallet_storage/mod.rs | 468 +++++++++++++++++- .../src-tauri/src/wallet_storage/password.rs | 12 + 4 files changed, 492 insertions(+), 13 deletions(-) diff --git a/nym-wallet/src-tauri/src/operations/mixnet/account.rs b/nym-wallet/src-tauri/src/operations/mixnet/account.rs index 0bf20ae202..0f17c2880c 100644 --- a/nym-wallet/src-tauri/src/operations/mixnet/account.rs +++ b/nym-wallet/src-tauri/src/operations/mixnet/account.rs @@ -372,7 +372,7 @@ pub fn create_password(mnemonic: &str, password: String) -> Result<(), BackendEr // Currently we only support a single, default, id in the wallet let id = wallet_storage::AccountId::new(DEFAULT_WALLET_ACCOUNT_ID.to_string()); let password = wallet_storage::UserPassword::new(password); - wallet_storage::store_wallet_login_information(mnemonic, hd_path, id, &password) + wallet_storage::store_wallet_login_multiple_information(mnemonic, hd_path, id, &password) } #[tauri::command] @@ -629,4 +629,9 @@ mod tests { ] ); } + + #[test] + fn decrypt_stored_wallet_multiple_for_sign_in() { + // WIP(JON): same as above but with file containing multiple accounts + } } diff --git a/nym-wallet/src-tauri/src/wallet_storage/account_data.rs b/nym-wallet/src-tauri/src/wallet_storage/account_data.rs index 205cf684df..418be34433 100644 --- a/nym-wallet/src-tauri/src/wallet_storage/account_data.rs +++ b/nym-wallet/src-tauri/src/wallet_storage/account_data.rs @@ -157,6 +157,10 @@ impl StoredLogin { Self::Mnemonic(MnemonicAccount { mnemonic, hd_path }) } + pub(crate) fn new_multiple_login() -> Self { + Self::Multiple(MultipleAccounts::empty()) + } + #[cfg(test)] pub(crate) fn as_mnemonic_account(&self) -> Option<&MnemonicAccount> { match self { @@ -194,7 +198,6 @@ impl MnemonicAccount { &self.mnemonic } - #[cfg(test)] pub(crate) fn hd_path(&self) -> &DerivationPath { &self.hd_path } @@ -239,6 +242,12 @@ pub(crate) struct MultipleAccounts { } impl MultipleAccounts { + pub(crate) fn empty() -> Self { + MultipleAccounts { + accounts: Vec::new(), + } + } + pub(crate) fn new(account: WalletAccount) -> Self { MultipleAccounts { accounts: vec![account], @@ -340,6 +349,13 @@ impl AccountData { AccountData::Mnemonic(account) => account.mnemonic(), } } + + #[cfg(test)] + pub(crate) fn hd_path(&self) -> &DerivationPath { + match self { + AccountData::Mnemonic(account) => account.hd_path(), + } + } } impl From for AccountData { diff --git a/nym-wallet/src-tauri/src/wallet_storage/mod.rs b/nym-wallet/src-tauri/src/wallet_storage/mod.rs index 9792c877b2..731547ee39 100644 --- a/nym-wallet/src-tauri/src/wallet_storage/mod.rs +++ b/nym-wallet/src-tauri/src/wallet_storage/mod.rs @@ -18,11 +18,12 @@ pub(crate) mod encryption; mod password; +/// The default wallet (top-level) login id. pub(crate) const DEFAULT_WALLET_ACCOUNT_ID: &str = "default"; /// When converting a single account entry to one that contains many, the first account will use /// this name. -//pub(crate) const DEFAULT_NAME_FIRST_ACCOUNT: &str = "Account 1"; +pub(crate) const DEFAULT_NAME_FIRST_ACCOUNT: &str = "Account 1"; fn get_storage_directory() -> Result { tauri::api::path::local_data_dir() @@ -73,6 +74,8 @@ pub(crate) fn load_existing_wallet_login_information_at_file( /// Encrypt `mnemonic` and store it together with `id`. It is stored at the top-level. /// Currently we enforce that we can only add entries with the same password as the other already /// existing entries. This is not unlikely to change in the future. +// DEPRECATED: consider making this cfg(test) only as it's still useful to verify being able to +// handle older formats pub(crate) fn store_wallet_login_information( mnemonic: bip39::Mnemonic, hd_path: DerivationPath, @@ -99,7 +102,8 @@ fn store_wallet_login_information_at_file( result => result?, }; - // Confirm that the given password also can unlock the other entries + // Confirm that the given password also can unlock the other entries. + // This is restriction we can relax in the future, but for now it's a sanity check. if !stored_wallet.password_can_decrypt_all(password) { return Err(BackendError::WalletDifferentPasswordDetected); } @@ -121,6 +125,62 @@ fn store_wallet_login_information_at_file( Ok(serde_json::to_writer_pretty(file, &stored_wallet)?) } +pub(crate) fn store_wallet_login_multiple_information( + mnemonic: bip39::Mnemonic, + hd_path: DerivationPath, + id: AccountId, + password: &UserPassword, +) -> Result<(), BackendError> { + // make sure the entire directory structure exists + let store_dir = get_storage_directory()?; + create_dir_all(&store_dir)?; + let filepath = store_dir.join(WALLET_INFO_FILENAME); + + store_wallet_login_multiple_information_at_file(filepath, mnemonic, hd_path, id, password) +} + +fn store_wallet_login_multiple_information_at_file( + filepath: PathBuf, + mnemonic: bip39::Mnemonic, + hd_path: DerivationPath, + id: AccountId, + password: &UserPassword, +) -> Result<(), BackendError> { + let mut stored_wallet = match load_existing_wallet_at_file(filepath.clone()) { + Err(BackendError::WalletFileNotFound) => StoredWallet::default(), + result => result?, + }; + + // Confirm that the given password also can unlock the other entries. + // This is restriction we can relax in the future, but for now it's a sanity check. + if !stored_wallet.password_can_decrypt_all(password) { + return Err(BackendError::WalletDifferentPasswordDetected); + } + + let mut new_accounts = account_data::MultipleAccounts::empty(); + new_accounts.add( + DEFAULT_NAME_FIRST_ACCOUNT.to_string().into(), + mnemonic, + hd_path, + )?; + + let new_login = StoredLogin::Multiple(new_accounts); + let new_encrypted_login = EncryptedLogin { + id, + account: encrypt_struct(&new_login, password)?, + }; + + stored_wallet.add_encrypted_login(new_encrypted_login)?; + + let file = OpenOptions::new() + .create(true) + .write(true) + .truncate(true) + .open(filepath)?; + + Ok(serde_json::to_writer_pretty(file, &stored_wallet)?) +} + /// Append an account to an already existing top-level encrypted account entry. /// If the existing top-level entry is just a single account, it will be converted to the first /// account in the list of accounts associated with the encrypted entry. The inner id for this @@ -343,6 +403,35 @@ mod tests { assert!(!login.account.ciphertext().is_empty()); } + #[test] + fn store_single_login_with_multi() { + let store_dir = tempdir().unwrap(); + let wallet_file = store_dir.path().join(WALLET_INFO_FILENAME); + + let dummy_account1 = bip39::Mnemonic::generate(24).unwrap(); + let cosmos_hd_path: DerivationPath = COSMOS_DERIVATION_PATH.parse().unwrap(); + let password = UserPassword::new("password".to_string()); + let id1 = AccountId::new("first".to_string()); + + store_wallet_login_multiple_information_at_file( + wallet_file.clone(), + dummy_account1, + cosmos_hd_path, + id1, + &password, + ) + .unwrap(); + + let stored_wallet = load_existing_wallet_at_file(wallet_file).unwrap(); + assert_eq!(stored_wallet.len(), 1); + + let login = stored_wallet.get_encrypted_login_by_index(0).unwrap(); + assert_eq!(login.id, AccountId::new("first".to_string())); + + // some actual ciphertext was saved + assert!(!login.account.ciphertext().is_empty()); + } + #[test] fn store_twice_for_the_same_id_fails() { let store_dir = tempdir().unwrap(); @@ -376,6 +465,39 @@ mod tests { )); } + #[test] + fn store_twice_for_the_same_id_fails_with_multiple() { + let store_dir = tempdir().unwrap(); + let wallet_file = store_dir.path().join(WALLET_INFO_FILENAME); + + let dummy_account1 = bip39::Mnemonic::generate(24).unwrap(); + let cosmos_hd_path: DerivationPath = COSMOS_DERIVATION_PATH.parse().unwrap(); + let password = UserPassword::new("password".to_string()); + let id1 = AccountId::new("first".to_string()); + + // Store the first login + store_wallet_login_multiple_information_at_file( + wallet_file.clone(), + dummy_account1.clone(), + cosmos_hd_path.clone(), + id1.clone(), + &password, + ) + .unwrap(); + + // and storing the same id again fails + assert!(matches!( + store_wallet_login_multiple_information_at_file( + wallet_file, + dummy_account1, + cosmos_hd_path, + id1, + &password, + ), + Err(BackendError::IdAlreadyExistsInWallet), + )); + } + #[test] fn load_with_wrong_password_fails() { let store_dir = tempdir().unwrap(); @@ -403,6 +525,33 @@ mod tests { )); } + #[test] + fn load_with_wrong_password_fails_with_multi() { + let store_dir = tempdir().unwrap(); + let wallet_file = store_dir.path().join(WALLET_INFO_FILENAME); + + let dummy_account1 = bip39::Mnemonic::generate(24).unwrap(); + let cosmos_hd_path: DerivationPath = COSMOS_DERIVATION_PATH.parse().unwrap(); + let password = UserPassword::new("password".to_string()); + let bad_password = UserPassword::new("bad-password".to_string()); + let id1 = AccountId::new("first".to_string()); + + store_wallet_login_multiple_information_at_file( + wallet_file.clone(), + dummy_account1, + cosmos_hd_path, + id1.clone(), + &password, + ) + .unwrap(); + + // Trying to load it with wrong password now fails + assert!(matches!( + load_existing_wallet_login_information_at_file(wallet_file, &id1, &bad_password), + Err(BackendError::DecryptionError), + )); + } + #[test] fn load_with_wrong_id_fails() { let store_dir = tempdir().unwrap(); @@ -430,6 +579,33 @@ mod tests { )); } + #[test] + fn load_with_wrong_id_fails_with_multi() { + let store_dir = tempdir().unwrap(); + let wallet_file = store_dir.path().join(WALLET_INFO_FILENAME); + + let dummy_account1 = bip39::Mnemonic::generate(24).unwrap(); + let cosmos_hd_path: DerivationPath = COSMOS_DERIVATION_PATH.parse().unwrap(); + let password = UserPassword::new("password".to_string()); + let id1 = AccountId::new("first".to_string()); + let id2 = AccountId::new("second".to_string()); + + store_wallet_login_multiple_information_at_file( + wallet_file.clone(), + dummy_account1, + cosmos_hd_path, + id1, + &password, + ) + .unwrap(); + + // Trying to load with the wrong id + assert!(matches!( + load_existing_wallet_login_information_at_file(wallet_file, &id2, &password), + Err(BackendError::NoSuchIdInWallet), + )); + } + #[test] fn store_and_load_a_single_login() { let store_dir = tempdir().unwrap(); @@ -456,6 +632,37 @@ mod tests { assert_eq!(&cosmos_hd_path, acc.hd_path()); } + #[test] + fn store_and_load_a_single_login_with_multi() { + let store_dir = tempdir().unwrap(); + let wallet_file = store_dir.path().join(WALLET_INFO_FILENAME); + + let dummy_account1 = bip39::Mnemonic::generate(24).unwrap(); + let cosmos_hd_path: DerivationPath = COSMOS_DERIVATION_PATH.parse().unwrap(); + let password = UserPassword::new("password".to_string()); + let id1 = AccountId::new("first".to_string()); + + store_wallet_login_multiple_information_at_file( + wallet_file.clone(), + dummy_account1.clone(), + cosmos_hd_path.clone(), + id1.clone(), + &password, + ) + .unwrap(); + + let loaded_login = + load_existing_wallet_login_information_at_file(wallet_file, &id1, &password).unwrap(); + let accounts = loaded_login.as_multiple_accounts().unwrap(); + assert_eq!(accounts.len(), 1); + let account = accounts + .get_account(&DEFAULT_NAME_FIRST_ACCOUNT.into()) + .unwrap(); + assert_eq!(account.id, DEFAULT_NAME_FIRST_ACCOUNT.into()); + assert_eq!(account.account.mnemonic(), &dummy_account1); + assert_eq!(account.account.hd_path(), &cosmos_hd_path); + } + #[test] fn store_a_second_login_with_a_different_password_fails() { let store_dir = tempdir().unwrap(); @@ -493,6 +700,43 @@ mod tests { )); } + #[test] + fn store_a_second_login_with_a_different_password_fails_with_multi() { + let store_dir = tempdir().unwrap(); + let wallet_file = store_dir.path().join(WALLET_INFO_FILENAME); + + let dummy_account1 = bip39::Mnemonic::generate(24).unwrap(); + let dummy_account2 = bip39::Mnemonic::generate(24).unwrap(); + let cosmos_hd_path: DerivationPath = COSMOS_DERIVATION_PATH.parse().unwrap(); + + let password = UserPassword::new("password".to_string()); + let bad_password = UserPassword::new("bad-password".to_string()); + + let id1 = AccountId::new("first".to_string()); + let id2 = AccountId::new("second".to_string()); + + store_wallet_login_multiple_information_at_file( + wallet_file.clone(), + dummy_account1, + cosmos_hd_path.clone(), + id1, + &password, + ) + .unwrap(); + + // Can't store a second login if you use different password + assert!(matches!( + store_wallet_login_multiple_information_at_file( + wallet_file, + dummy_account2, + cosmos_hd_path, + id2, + &bad_password + ), + Err(BackendError::WalletDifferentPasswordDetected), + )); + } + #[test] fn store_two_mnemonic_accounts_gives_different_salts_and_iv() { let store_dir = tempdir().unwrap(); @@ -605,6 +849,66 @@ mod tests { assert_eq!(&different_hd_path, acc2.hd_path()); } + #[test] + fn store_one_mnemonic_account_and_one_multi_account() { + let store_dir = tempdir().unwrap(); + let wallet_file = store_dir.path().join(WALLET_INFO_FILENAME); + + let dummy_account1 = bip39::Mnemonic::generate(24).unwrap(); + let dummy_account2 = bip39::Mnemonic::generate(24).unwrap(); + let cosmos_hd_path: DerivationPath = COSMOS_DERIVATION_PATH.parse().unwrap(); + let different_hd_path: DerivationPath = "m".parse().unwrap(); + + let password = UserPassword::new("password".to_string()); + + let id1 = AccountId::new("first".to_string()); + let id2 = AccountId::new("second".to_string()); + + // Store the first account + store_wallet_login_information_at_file( + wallet_file.clone(), + dummy_account1.clone(), + cosmos_hd_path.clone(), + id1.clone(), + &password, + ) + .unwrap(); + + let loaded_account = + load_existing_wallet_login_information_at_file(wallet_file.clone(), &id1, &password).unwrap(); + let acc = loaded_account.as_mnemonic_account().unwrap(); + assert_eq!(&dummy_account1, acc.mnemonic()); + assert_eq!(&cosmos_hd_path, acc.hd_path()); + + // Add an extra account + store_wallet_login_multiple_information_at_file( + wallet_file.clone(), + dummy_account2.clone(), + different_hd_path.clone(), + id2.clone(), + &password, + ) + .unwrap(); + + // first account should be unchanged + let loaded_account = + load_existing_wallet_login_information_at_file(wallet_file.clone(), &id1, &password).unwrap(); + let acc1 = loaded_account.as_mnemonic_account().unwrap(); + assert_eq!(&dummy_account1, acc1.mnemonic()); + assert_eq!(&cosmos_hd_path, acc1.hd_path()); + + let loaded_account = + load_existing_wallet_login_information_at_file(wallet_file, &id2, &password).unwrap(); + let acc2 = loaded_account.as_multiple_accounts().unwrap(); + assert_eq!(acc2.len(), 1); + let account = acc2 + .get_account(&DEFAULT_NAME_FIRST_ACCOUNT.into()) + .unwrap(); + assert_eq!(account.id, DEFAULT_NAME_FIRST_ACCOUNT.into()); + assert_eq!(account.account.mnemonic(), &dummy_account2); + assert_eq!(account.account.hd_path(), &different_hd_path); + } + #[test] fn remove_non_existent_id_fails() { let store_dir = tempdir().unwrap(); @@ -618,7 +922,7 @@ mod tests { let id1 = AccountId::new("first".to_string()); let id2 = AccountId::new("second".to_string()); - store_wallet_login_information_at_file( + store_wallet_login_multiple_information_at_file( wallet_file.clone(), dummy_account1, cosmos_hd_path, @@ -844,6 +1148,90 @@ mod tests { assert_eq!(accounts, &expected); } + #[test] + fn append_accounts_to_existing_login_with_multi() { + let store_dir = tempdir().unwrap(); + let wallet_file = store_dir.path().join(WALLET_INFO_FILENAME); + + let dummy_account1 = bip39::Mnemonic::generate(24).unwrap(); + let dummy_account2 = bip39::Mnemonic::generate(24).unwrap(); + let dummy_account3 = bip39::Mnemonic::generate(24).unwrap(); + let dummy_account4 = bip39::Mnemonic::generate(24).unwrap(); + let hd_path: DerivationPath = COSMOS_DERIVATION_PATH.parse().unwrap(); + + let password = UserPassword::new("password".to_string()); + + let id1 = AccountId::new("first".to_string()); + let id2 = AccountId::new("second".to_string()); + let id3 = AccountId::new("third".to_string()); + let id4 = AccountId::new("fourth".to_string()); + + store_wallet_login_multiple_information_at_file( + wallet_file.clone(), + dummy_account1.clone(), + hd_path.clone(), + id1.clone(), + &password, + ) + .unwrap(); + + store_wallet_login_multiple_information_at_file( + wallet_file.clone(), + dummy_account2.clone(), + hd_path.clone(), + id2.clone(), + &password, + ) + .unwrap(); + + // Add a third and fourth mnenonic grouped together with the second one + append_account_to_wallet_login_information_at_file( + wallet_file.clone(), + dummy_account3.clone(), + hd_path.clone(), + id2.clone(), + id3.clone(), + &password, + ) + .unwrap(); + append_account_to_wallet_login_information_at_file( + wallet_file.clone(), + dummy_account4.clone(), + hd_path.clone(), + id2.clone(), + id4.clone(), + &password, + ) + .unwrap(); + + // Check that we can load all four + let loaded_login = + load_existing_wallet_login_information_at_file(wallet_file.clone(), &id1, &password).unwrap(); + let accounts = loaded_login.as_multiple_accounts().unwrap(); + let expected = vec![WalletAccount::new_mnemonic_backed_account( + DEFAULT_NAME_FIRST_ACCOUNT.into(), + dummy_account1, + hd_path.clone(), + )] + .into(); + assert_eq!(accounts, &expected); + + let loaded_login = + load_existing_wallet_login_information_at_file(wallet_file, &id2, &password).unwrap(); + let accounts = loaded_login.as_multiple_accounts().unwrap(); + let expected = vec![ + WalletAccount::new_mnemonic_backed_account( + DEFAULT_NAME_FIRST_ACCOUNT.into(), + dummy_account2, + hd_path.clone(), + ), + WalletAccount::new_mnemonic_backed_account(id3, dummy_account3, hd_path.clone()), + WalletAccount::new_mnemonic_backed_account(id4, dummy_account4, hd_path), + ] + .into(); + assert_eq!(accounts, &expected); + } + #[test] fn delete_the_same_account_twice_for_a_login_fails() { let store_dir = tempdir().unwrap(); @@ -885,6 +1273,47 @@ mod tests { )); } + #[test] + fn delete_the_same_account_twice_for_a_login_fails_with_multi() { + let store_dir = tempdir().unwrap(); + let wallet_file = store_dir.path().join(WALLET_INFO_FILENAME); + + let dummy_account1 = bip39::Mnemonic::generate(24).unwrap(); + let dummy_account2 = bip39::Mnemonic::generate(24).unwrap(); + let cosmos_hd_path: DerivationPath = COSMOS_DERIVATION_PATH.parse().unwrap(); + + let password = UserPassword::new("password".to_string()); + + let id1 = AccountId::new("first".to_string()); + let id2 = AccountId::new("second".to_string()); + + store_wallet_login_multiple_information_at_file( + wallet_file.clone(), + dummy_account1, + cosmos_hd_path.clone(), + id1.clone(), + &password, + ) + .unwrap(); + + append_account_to_wallet_login_information_at_file( + wallet_file.clone(), + dummy_account2, + cosmos_hd_path, + id1.clone(), + id2.clone(), + &password, + ) + .unwrap(); + + remove_account_from_wallet_login_at_file(wallet_file.clone(), &id1, &id2, &password).unwrap(); + + assert!(matches!( + remove_account_from_wallet_login_at_file(wallet_file, &id1, &id2, &password), + Err(BackendError::NoSuchIdInWalletLoginEntry), + )); + } + #[test] fn delete_appended_account_doesnt_affect_others() { let store_dir = tempdir().unwrap(); @@ -957,7 +1386,7 @@ mod tests { let id1 = AccountId::new("first".to_string()); let id2 = AccountId::new("second".to_string()); - store_wallet_login_information_at_file( + store_wallet_login_multiple_information_at_file( wallet.clone(), dummy_account1, cosmos_hd_path.clone(), @@ -976,7 +1405,13 @@ mod tests { ) .unwrap(); - remove_account_from_wallet_login_at_file(wallet.clone(), &id1, &id1, &password).unwrap(); + remove_account_from_wallet_login_at_file( + wallet.clone(), + &id1, + &DEFAULT_NAME_FIRST_ACCOUNT.into(), + &password, + ) + .unwrap(); remove_account_from_wallet_login_at_file(wallet.clone(), &id1, &id2, &password).unwrap(); // The file should now be removed @@ -1005,7 +1440,7 @@ mod tests { let id2 = AccountId::new("second".to_string()); let id3 = AccountId::new("third".to_string()); - store_wallet_login_information_at_file( + store_wallet_login_multiple_information_at_file( wallet.clone(), dummy_account1, cosmos_hd_path.clone(), @@ -1024,7 +1459,7 @@ mod tests { ) .unwrap(); - store_wallet_login_information_at_file( + store_wallet_login_multiple_information_at_file( wallet.clone(), dummy_account3.clone(), cosmos_hd_path.clone(), @@ -1033,7 +1468,13 @@ mod tests { ) .unwrap(); - remove_account_from_wallet_login_at_file(wallet.clone(), &id1, &id1, &password).unwrap(); + remove_account_from_wallet_login_at_file( + wallet.clone(), + &id1, + &DEFAULT_NAME_FIRST_ACCOUNT.into(), + &password, + ) + .unwrap(); remove_account_from_wallet_login_at_file(wallet.clone(), &id1, &id2, &password).unwrap(); // And trying to load when the file is gone fails @@ -1045,9 +1486,14 @@ mod tests { // The other login is still there let loaded_account = load_existing_wallet_login_information_at_file(wallet, &id3, &password).unwrap(); - let acc3 = loaded_account.as_mnemonic_account().unwrap(); - assert_eq!(acc3.mnemonic(), &dummy_account3); - assert_eq!(acc3.hd_path(), &cosmos_hd_path); + let acc3 = loaded_account.as_multiple_accounts().unwrap(); + let expected = vec![WalletAccount::new_mnemonic_backed_account( + DEFAULT_NAME_FIRST_ACCOUNT.into(), + dummy_account3, + cosmos_hd_path, + )] + .into(); + assert_eq!(acc3, &expected); } #[test] diff --git a/nym-wallet/src-tauri/src/wallet_storage/password.rs b/nym-wallet/src-tauri/src/wallet_storage/password.rs index 016c5d3e5c..920032a5da 100644 --- a/nym-wallet/src-tauri/src/wallet_storage/password.rs +++ b/nym-wallet/src-tauri/src/wallet_storage/password.rs @@ -21,6 +21,18 @@ impl AsRef for AccountId { } } +impl From for AccountId { + fn from(id: String) -> Self { + Self::new(id) + } +} + +impl From<&str> for AccountId { + fn from(id: &str) -> Self { + Self::new(id.to_string()) + } +} + impl fmt::Display for AccountId { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}", self.0)