Fix key.meta.vault for root dir keys && read vault.meta without vault key (#4482)

* fix vault for root && read vault meta without key

* support for old vaults (wthout meta field)

ethstore/src/dir/disk.rs

@@ -234,6 +234,10 @@ impl<T> VaultKeyDirectoryProvider for DiskDirectory<T> where T: KeyFileManager {
 			})
 			.collect())
 	}
+
+	fn vault_meta(&self, name: &str) -> Result<String, Error> {
+		VaultDiskDirectory::meta_at(&self.path, name)
+	}
 }
 
 impl KeyFileManager for DiskKeyFileManager {
@@ -242,7 +246,12 @@ impl KeyFileManager for DiskKeyFileManager {
 		Ok(SafeAccount::from_file(key_file, filename))
 	}
 
-	fn write<T>(&self, account: SafeAccount, writer: &mut T) -> Result<(), Error> where T: io::Write {
+	fn write<T>(&self, mut account: SafeAccount, writer: &mut T) -> Result<(), Error> where T: io::Write {
+		// when account is moved back to root directory from vault
+		// => remove vault field from meta
+		account.meta = json::remove_vault_name_from_json_meta(&account.meta)
+			.map_err(|err| Error::Custom(format!("{:?}", err)))?;
+
 		let key_file: json::KeyFile = account.into();
 		key_file.write(writer).map_err(|e| Error::Custom(format!("{:?}", e)))
 	}

ethstore/src/dir/mod.rs

@@ -72,6 +72,8 @@ pub trait VaultKeyDirectoryProvider {
 	fn open(&self, name: &str, key: VaultKey) -> Result<Box<VaultKeyDirectory>, Error>;
 	/// List all vaults
 	fn list_vaults(&self) -> Result<Vec<String>, Error>;
+	/// Get vault meta
+	fn vault_meta(&self, name: &str) -> Result<String, Error>;
 }
 
 /// Vault directory

ethstore/src/dir/vault.rs

@@ -67,11 +67,23 @@ impl VaultDiskDirectory {
 		}
 
 		// check that passed key matches vault file
-		let meta = read_vault_file(&vault_dir_path, &key)?;
+		let meta = read_vault_file(&vault_dir_path, Some(&key))?;
 
 		Ok(DiskDirectory::new(vault_dir_path, VaultKeyFileManager::new(name, key, &meta)))
 	}
 
+	/// Read vault meta without actually opening the vault
+	pub fn meta_at<P>(root: P, name: &str) -> Result<String, Error> where P: AsRef<Path> {
+		// check that vault directory exists
+		let vault_dir_path = make_vault_dir_path(root, name, true)?;
+		if !vault_dir_path.is_dir() {
+			return Err(Error::VaultNotFound);
+		}
+
+		// check that passed key matches vault file
+		read_vault_file(&vault_dir_path, None)
+	}
+
 	fn create_temp_vault(&self, key: VaultKey) -> Result<VaultDiskDirectory, Error> {
 		let original_path = self.path().expect("self is instance of DiskDirectory; DiskDirectory always returns path; qed");
 		let mut path: PathBuf = original_path.clone();
@@ -241,7 +253,7 @@ fn create_vault_file<P>(vault_dir_path: P, key: &VaultKey, meta: &str) -> Result
 }
 
 /// When vault is opened => we must check that password matches && read metadata
-fn read_vault_file<P>(vault_dir_path: P, key: &VaultKey) -> Result<String, Error> where P: AsRef<Path> {
+fn read_vault_file<P>(vault_dir_path: P, key: Option<&VaultKey>) -> Result<String, Error> where P: AsRef<Path> {
 	let mut vault_file_path: PathBuf = vault_dir_path.as_ref().into();
 	vault_file_path.push(VAULT_FILE_NAME);
 
@@ -250,11 +262,13 @@ fn read_vault_file<P>(vault_dir_path: P, key: &VaultKey) -> Result<String, Error
 	let vault_file_meta = vault_file_contents.meta.unwrap_or("{}".to_owned());
 	let vault_file_crypto: Crypto = vault_file_contents.crypto.into();
 
+	if let Some(key) = key {
 		let password_bytes = vault_file_crypto.decrypt(&key.password)?;
 		let password_hash = key.password.sha3();
 		if &*password_hash != password_bytes.as_slice() {
 			return Err(Error::InvalidPassword);
 		}
+	}
 
 	Ok(vault_file_meta)
 }
@@ -264,7 +278,7 @@ mod test {
 	use std::fs;
 	use std::io::Write;
 	use std::path::PathBuf;
-	use dir::{VaultKey, VaultKeyDirectory};
+	use dir::VaultKey;
 	use super::{VAULT_FILE_NAME, check_vault_name, make_vault_dir_path, create_vault_file, read_vault_file, VaultDiskDirectory};
 	use devtools::RandomTempPath;
 
@@ -333,7 +347,7 @@ mod test {
 		}
 
 		// when
-		let result = read_vault_file(&dir, &key);
+		let result = read_vault_file(&dir, Some(&key));
 
 		// then
 		assert!(result.is_ok());
@@ -349,7 +363,7 @@ mod test {
 		vault_file_path.push(VAULT_FILE_NAME);
 
 		// when
-		let result = read_vault_file(&dir, &key);
+		let result = read_vault_file(&dir, Some(&key));
 
 		// then
 		assert!(result.is_err());
@@ -362,7 +376,7 @@ mod test {
 		}
 
 		// when
-		let result = read_vault_file(&dir, &key);
+		let result = read_vault_file(&dir, Some(&key));
 
 		// then
 		assert!(result.is_err());
@@ -418,22 +432,4 @@ mod test {
 		// then
 		assert!(vault.is_err());
 	}
-
-	#[test]
-	fn vault_directory_can_preserve_meta() {
-		// given
-		let temp_path = RandomTempPath::new();
-		let key = VaultKey::new("password", 1024);
-		let dir: PathBuf = temp_path.as_path().into();
-		let vault = VaultDiskDirectory::create(&dir, "vault", key.clone()).unwrap();
-
-		// then
-		assert_eq!(vault.meta(), "{}".to_owned());
-		assert!(vault.set_meta("Hello, world!!!").is_ok());
-		assert_eq!(vault.meta(), "Hello, world!!!".to_owned());
-
-		// and when
-		let vault = VaultDiskDirectory::at(&dir, "vault", key.clone()).unwrap();
-		assert_eq!(vault.meta(), "Hello, world!!!".to_owned());
-	}
 }

ethstore/src/ethstore.rs

@@ -501,10 +501,17 @@ impl SimpleSecretStore for EthMultiStore {
 	}
 
 	fn get_vault_meta(&self, name: &str) -> Result<String, Error> {
+		// vault meta contains password hint
+		// => allow reading meta even if vault is not yet opened
 		self.vaults.lock()
 			.get(name)
+			.and_then(|v| Some(v.meta()))
 			.ok_or(Error::VaultNotFound)
-			.and_then(|v| Ok(v.meta()))
+			.or_else(|_| {
+				let vault_provider = self.dir.as_vault_provider().ok_or(Error::VaultsAreNotSupported)?;
+				vault_provider.vault_meta(name)
+			})
+			
 	}
 
 	fn set_vault_meta(&self, name: &str, meta: &str) -> Result<(), Error> {
@@ -861,4 +868,34 @@ mod tests {
 		assert!(opened_vaults.iter().any(|v| &*v == name1));
 		assert!(opened_vaults.iter().any(|v| &*v == name3));
 	}
+
+	#[test]
+	fn should_manage_vaults_meta() {
+		// given
+		let mut dir = RootDiskDirectoryGuard::new();
+		let store = EthStore::open(dir.key_dir.take().unwrap()).unwrap();
+		let name1 = "vault1"; let password1 = "password1";
+
+		// when
+		store.create_vault(name1, password1).unwrap();
+
+		// then
+		assert_eq!(store.get_vault_meta(name1).unwrap(), "{}".to_owned());
+		assert!(store.set_vault_meta(name1, "Hello, world!!!").is_ok());
+		assert_eq!(store.get_vault_meta(name1).unwrap(), "Hello, world!!!".to_owned());
+
+		// and when
+		store.close_vault(name1).unwrap();
+		store.open_vault(name1, password1).unwrap();
+
+		// then
+		assert_eq!(store.get_vault_meta(name1).unwrap(), "Hello, world!!!".to_owned());
+
+		// and when
+		store.close_vault(name1).unwrap();
+
+		// then
+		assert_eq!(store.get_vault_meta(name1).unwrap(), "Hello, world!!!".to_owned());
+		assert!(store.get_vault_meta("vault2").is_err());
+	}
 }

ethstore/src/json/vault_file.rs

@@ -81,7 +81,7 @@ impl Visitor for VaultFileVisitor {
 		loop {
 			match visitor.visit_key()? {
 				Some(VaultFileField::Crypto) => { crypto = Some(visitor.visit_value()?); },
-				Some(VaultFileField::Meta) => { meta = Some(visitor.visit_value()?); }
+				Some(VaultFileField::Meta) => { meta = visitor.visit_value().ok(); }, // meta is optional
 				None => { break; },
 			}
 		}
@@ -141,4 +141,29 @@ mod test {
 
 		assert_eq!(file, deserialized);
 	}
+
+	#[test]
+	fn to_and_from_json_no_meta() {
+		let file = VaultFile {
+			crypto: Crypto {
+				cipher: Cipher::Aes128Ctr(Aes128Ctr {
+					iv: "0155e3690be19fbfbecabcd440aa284b".into(),
+				}),
+				ciphertext: "4d6938a1f49b7782".into(),
+				kdf: Kdf::Pbkdf2(Pbkdf2 {
+					c: 1024,
+					dklen: 32,
+					prf: Prf::HmacSha256,
+					salt: "b6a9338a7ccd39288a86dba73bfecd9101b4f3db9c9830e7c76afdbd4f6872e5".into(),
+				}),
+				mac: "16381463ea11c6eb2239a9f339c2e780516d29d234ce30ac5f166f9080b5a262".into(),
+			},
+			meta: None,
+		};
+
+		let serialized = serde_json::to_string(&file).unwrap();
+		let deserialized = serde_json::from_str(&serialized).unwrap();
+
+		assert_eq!(file, deserialized);
+	}
 }

rpc/src/v1/tests/mocked/parity_accounts.rs

@@ -314,6 +314,14 @@ fn rpc_parity_vault_adds_vault_field_to_acount_meta() {
 	let response = format!(r#"{{"jsonrpc":"2.0","result":{{"0x{}":{{"meta":"{{\"vault\":\"vault1\"}}","name":"","uuid":"{}"}}}},"id":1}}"#, address1.hex(), uuid1);
 
 	assert_eq!(tester.io.handle_request_sync(request), Some(response.to_owned()));
+
+	// and then
+	assert!(tester.accounts.change_vault(address1, "").is_ok());
+
+	let request = r#"{"jsonrpc": "2.0", "method": "parity_allAccountsInfo", "params":[], "id": 1}"#;
+	let response = format!(r#"{{"jsonrpc":"2.0","result":{{"0x{}":{{"meta":"{{}}","name":"","uuid":"{}"}}}},"id":1}}"#, address1.hex(), uuid1);
+
+	assert_eq!(tester.io.handle_request_sync(request), Some(response.to_owned()));
 }
 
 #[test]
@@ -358,6 +366,14 @@ fn rpc_parity_get_set_vault_meta() {
 	let tester = setup_with_vaults_support(temp_path.as_str());
 
 	assert!(tester.accounts.create_vault("vault1", "password1").is_ok());
+
+	// when no meta set
+	let request = r#"{"jsonrpc": "2.0", "method": "parity_getVaultMeta", "params":["vault1"], "id": 1}"#;
+	let response = r#"{"jsonrpc":"2.0","result":"{}","id":1}"#;
+
+	assert_eq!(tester.io.handle_request_sync(request), Some(response.to_owned()));
+
+	// when meta set
 	assert!(tester.accounts.set_vault_meta("vault1", "vault1_meta").is_ok());
 
 	let request = r#"{"jsonrpc": "2.0", "method": "parity_getVaultMeta", "params":["vault1"], "id": 1}"#;
@@ -365,11 +381,13 @@ fn rpc_parity_get_set_vault_meta() {
 
 	assert_eq!(tester.io.handle_request_sync(request), Some(response.to_owned()));
 
+	// change meta
 	let request = r#"{"jsonrpc": "2.0", "method": "parity_setVaultMeta", "params":["vault1", "updated_vault1_meta"], "id": 1}"#;
 	let response = r#"{"jsonrpc":"2.0","result":true,"id":1}"#;
 
 	assert_eq!(tester.io.handle_request_sync(request), Some(response.to_owned()));
 
+	// query changed meta
 	let request = r#"{"jsonrpc": "2.0", "method": "parity_getVaultMeta", "params":["vault1"], "id": 1}"#;
 	let response = r#"{"jsonrpc":"2.0","result":"updated_vault1_meta","id":1}"#;