Additional RPCs for dapps accounts management

ethcore/src/account_provider/mod.rs

@@ -18,7 +18,7 @@
 
 mod stores;
 
-use self::stores::{AddressBook, DappsSettingsStore};
+use self::stores::{AddressBook, DappsSettingsStore, NewDappsPolicy};
 
 use std::fmt;
 use std::collections::HashMap;
@@ -167,10 +167,49 @@ impl AccountProvider {
 		Ok(accounts)
 	}
 
+	/// Sets a whitelist of accounts exposed for unknown dapps.
+	/// `None` means that all accounts will be visible.
+	pub fn set_new_dapps_whitelist(&self, accounts: Option<Vec<Address>>) -> Result<(), Error> {
+		self.dapps_settings.write().set_policy(match accounts {
+			None => NewDappsPolicy::AllAccounts,
+			Some(accounts) => NewDappsPolicy::Whitelist(accounts),
+		});
+		Ok(())
+	}
+
+	/// Gets a whitelist of accounts exposed for unknown dapps.
+	/// `None` means that all accounts will be visible.
+	pub fn new_dapps_whitelist(&self) -> Result<Option<Vec<Address>>, Error> {
+		Ok(match self.dapps_settings.read().policy() {
+			NewDappsPolicy::AllAccounts => None,
+			NewDappsPolicy::Whitelist(accounts) => Some(accounts),
+		})
+	}
+
+	/// Gets a list of dapps recently requesting accounts.
+	pub fn recent_dapps(&self) -> Result<Vec<DappId>, Error> {
+		Ok(self.dapps_settings.read().recent_dapps())
+	}
+
+	/// Marks dapp as recently used.
+	pub fn note_dapp_used(&self, dapp: DappId) -> Result<(), Error> {
+		let mut dapps = self.dapps_settings.write();
+		dapps.mark_dapp_used(dapp.clone());
+		Ok(())
+	}
+
 	/// Gets addresses visile for dapp.
 	pub fn dapps_addresses(&self, dapp: DappId) -> Result<Vec<Address>, Error> {
-		let accounts = self.dapps_settings.read().get();
+		let dapps = self.dapps_settings.read();
-		Ok(accounts.get(&dapp).map(|settings| settings.accounts.clone()).unwrap_or_else(Vec::new))
+
+		let accounts = dapps.settings().get(&dapp).map(|settings| settings.accounts.clone());
+		match accounts {
+			Some(accounts) => Ok(accounts),
+			None => match dapps.policy() {
+				NewDappsPolicy::AllAccounts => self.accounts(),
+				NewDappsPolicy::Whitelist(accounts) => Ok(accounts),
+			}
+		}
 	}
 
 	/// Sets addresses visile for dapp.
@@ -386,4 +425,17 @@ mod tests {
 		// then
 		assert_eq!(ap.dapps_addresses(app.clone()).unwrap(), vec![1.into(), 2.into()]);
 	}
+
+	#[test]
+	fn should_set_dapps_policy() {
+		// given
+		let ap = AccountProvider::transient_provider();
+		let address = ap.new_account("test").unwrap();
+		// Default policy should be to return all
+		assert_eq!(ap.dapps_addresses("app1".into()).unwrap(), vec![address]);
+
+		// change policy
+		ap.set_new_dapps_whitelist(Some(vec![1.into()])).unwrap();
+		assert_eq!(ap.dapps_addresses("app1".into()).unwrap(), vec![1.into()]);
+	}
 }

ethcore/src/account_provider/stores.rs

@@ -17,11 +17,11 @@
 //! Address Book and Dapps Settings Store
 
 use std::{fs, fmt, hash, ops};
-use std::collections::HashMap;
+use std::collections::{HashMap, VecDeque};
 use std::path::PathBuf;
 
 use ethstore::ethkey::Address;
-use ethjson::misc::{AccountMeta, DappsSettings as JsonSettings};
+use ethjson::misc::{AccountMeta, DappsSettings as JsonSettings, NewDappsPolicy as JsonNewDappsPolicy};
 use account_provider::DappId;
 
 /// Disk-backed map from Address to String. Uses JSON.
@@ -105,43 +105,106 @@ impl From<DappsSettings> for JsonSettings {
 	}
 }
 
+/// Dapps user settings
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub enum NewDappsPolicy {
+	AllAccounts,
+	Whitelist(Vec<Address>),
+}
+
+impl From<JsonNewDappsPolicy> for NewDappsPolicy {
+	fn from(s: JsonNewDappsPolicy) -> Self {
+		match s {
+			JsonNewDappsPolicy::AllAccounts => NewDappsPolicy::AllAccounts,
+			JsonNewDappsPolicy::Whitelist(accounts) => NewDappsPolicy::Whitelist(
+				accounts.into_iter().map(Into::into).collect()
+			),
+		}
+	}
+}
+
+impl From<NewDappsPolicy> for JsonNewDappsPolicy {
+	fn from(s: NewDappsPolicy) -> Self {
+		match s {
+			NewDappsPolicy::AllAccounts => JsonNewDappsPolicy::AllAccounts,
+			NewDappsPolicy::Whitelist(accounts) => JsonNewDappsPolicy::Whitelist(
+				accounts.into_iter().map(Into::into).collect()
+			),
+		}
+	}
+}
+
+const MAX_RECENT_DAPPS: usize = 10;
+
 /// Disk-backed map from DappId to Settings. Uses JSON.
 pub struct DappsSettingsStore {
-	cache: DiskMap<DappId, DappsSettings>,
+	/// Dapps Settings
+	settings: DiskMap<DappId, DappsSettings>,
+	/// New Dapps Policy
+	policy: DiskMap<String, NewDappsPolicy>,
+	/// Recently Accessed Dapps (transient)
+	recent: VecDeque<DappId>,
 }
 
 impl DappsSettingsStore {
 	/// Creates new store at given directory path.
 	pub fn new(path: String) -> Self {
 		let mut r = DappsSettingsStore {
-			cache: DiskMap::new(path, "dapps_accounts.json".into())
+			settings: DiskMap::new(path.clone(), "dapps_accounts.json".into()),
+			policy: DiskMap::new(path.clone(), "dapps_policy.json".into()),
+			recent: VecDeque::with_capacity(MAX_RECENT_DAPPS),
 		};
-		r.cache.revert(JsonSettings::read_dapps_settings);
+		r.settings.revert(JsonSettings::read_dapps_settings);
+		r.policy.revert(JsonNewDappsPolicy::read_new_dapps_policy);
 		r
 	}
 
 	/// Creates transient store (no changes are saved to disk).
 	pub fn transient() -> Self {
 		DappsSettingsStore {
-			cache: DiskMap::transient()
+			settings: DiskMap::transient(),
+			policy: DiskMap::transient(),
+			recent: VecDeque::with_capacity(MAX_RECENT_DAPPS),
 		}
 	}
 
 	/// Get copy of the dapps settings
-	pub fn get(&self) -> HashMap<DappId, DappsSettings> {
+	pub fn settings(&self) -> HashMap<DappId, DappsSettings> {
-		self.cache.clone()
+		self.settings.clone()
 	}
 
-	fn save(&self) {
+	/// Returns current new dapps policy
-		self.cache.save(JsonSettings::write_dapps_settings)
+	pub fn policy(&self) -> NewDappsPolicy {
+		self.policy.get("default").cloned().unwrap_or(NewDappsPolicy::AllAccounts)
 	}
 
+	/// Returns recent dapps (in order of last request)
+	pub fn recent_dapps(&self) -> Vec<DappId> {
+		self.recent.iter().cloned().collect()
+	}
+
+	/// Marks recent dapp as used
+	pub fn mark_dapp_used(&mut self, dapp: DappId) {
+		self.recent.retain(|id| id != &dapp);
+		self.recent.push_front(dapp);
+		while self.recent.len() > MAX_RECENT_DAPPS {
+			self.recent.pop_back();
+		}
+	}
+
+	/// Sets current new dapps policy
+	pub fn set_policy(&mut self, policy: NewDappsPolicy) {
+		self.policy.insert("default".into(), policy);
+		self.policy.save(JsonNewDappsPolicy::write_new_dapps_policy);
+	}
+
+	/// Sets accounts for specific dapp.
 	pub fn set_accounts(&mut self, id: DappId, accounts: Vec<Address>) {
 		{
-			let mut settings = self.cache.entry(id).or_insert_with(DappsSettings::default);
+			let mut settings = self.settings.entry(id).or_insert_with(DappsSettings::default);
 			settings.accounts = accounts;
 		}
-		self.save();
+		self.settings.save(JsonSettings::write_dapps_settings);
 	}
 }
 
@@ -216,7 +279,7 @@ impl<K: hash::Hash + Eq, V> DiskMap<K, V> {
 
 #[cfg(test)]
 mod tests {
-	use super::{AddressBook, DappsSettingsStore, DappsSettings};
+	use super::{AddressBook, DappsSettingsStore, DappsSettings, NewDappsPolicy};
 	use std::collections::HashMap;
 	use ethjson::misc::AccountMeta;
 	use devtools::RandomTempPath;
@@ -232,25 +295,6 @@ mod tests {
 		assert_eq!(b.get(), hash_map![1.into() => AccountMeta{name: "One".to_owned(), meta: "{1:1}".to_owned(), uuid: None}]);
 	}
 
-	#[test]
-	fn should_save_and_reload_dapps_settings() {
-		// given
-		let temp = RandomTempPath::create_dir();
-		let path = temp.as_str().to_owned();
-		let mut b = DappsSettingsStore::new(path.clone());
-
-		// when
-		b.set_accounts("dappOne".into(), vec![1.into(), 2.into()]);
-
-		// then
-		let b = DappsSettingsStore::new(path);
-		assert_eq!(b.get(), hash_map![
-			"dappOne".into() => DappsSettings {
-				accounts: vec![1.into(), 2.into()],
-			}
-		]);
-	}
-
 	#[test]
 	fn should_remove_address() {
 		let temp = RandomTempPath::create_dir();
@@ -268,4 +312,57 @@ mod tests {
 			3.into() => AccountMeta{name: "Three".to_owned(), meta: "{}".to_owned(), uuid: None}
 		]);
 	}
+
+	#[test]
+	fn should_save_and_reload_dapps_settings() {
+		// given
+		let temp = RandomTempPath::create_dir();
+		let path = temp.as_str().to_owned();
+		let mut b = DappsSettingsStore::new(path.clone());
+
+		// when
+		b.set_accounts("dappOne".into(), vec![1.into(), 2.into()]);
+
+		// then
+		let b = DappsSettingsStore::new(path);
+		assert_eq!(b.settings(), hash_map![
+			"dappOne".into() => DappsSettings {
+				accounts: vec![1.into(), 2.into()],
+			}
+		]);
+	}
+
+	#[test]
+	fn should_maintain_a_list_of_recent_dapps() {
+		let mut store = DappsSettingsStore::transient();
+		assert!(store.recent_dapps().is_empty(), "Initially recent dapps should be empty.");
+
+		store.mark_dapp_used("dapp1".into());
+		assert_eq!(store.recent_dapps(), vec!["dapp1".to_owned()]);
+
+		store.mark_dapp_used("dapp2".into());
+		assert_eq!(store.recent_dapps(), vec!["dapp2".to_owned(), "dapp1".to_owned()]);
+
+		store.mark_dapp_used("dapp1".into());
+		assert_eq!(store.recent_dapps(), vec!["dapp1".to_owned(), "dapp2".to_owned()]);
+	}
+
+	#[test]
+	fn should_store_dapps_policy() {
+		// given
+		let temp = RandomTempPath::create_dir();
+		let path = temp.as_str().to_owned();
+		let mut store = DappsSettingsStore::new(path.clone());
+		// Test default policy
+		assert_eq!(store.policy(), NewDappsPolicy::AllAccounts);
+
+		// when
+		store.set_policy(NewDappsPolicy::Whitelist(vec![1.into(), 2.into()]));
+
+		// then
+		let store = DappsSettingsStore::new(path);
+		assert_eq!(store.policy.clone(), hash_map![
+			"default".into() => NewDappsPolicy::Whitelist(vec![1.into(), 2.into()])
+		]);
+	}
 }

json/src/misc/dapps_settings.rs

@@ -49,3 +49,32 @@ impl DappsSettings {
 		serde_json::to_writer(writer, &m.iter().map(|(a, m)| (a.clone().into(), m.clone().into())).collect::<HashMap<DappId, DappsSettings>>())
 	}
 }
+
+/// Accounts policy for new dapps.
+#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
+pub enum NewDappsPolicy {
+	/// All accounts are exposed by default.
+	AllAccounts,
+	/// Only accounts listed here are exposed by default for new dapps.
+	Whitelist(Vec<hash::Address>),
+}
+
+impl NewDappsPolicy {
+	/// Read a hash map of `String -> NewDappsPolicy`
+	pub fn read_new_dapps_policy<R, S>(reader: R) -> Result<HashMap<String, S>, serde_json::Error> where
+		R: io::Read,
+		S: From<NewDappsPolicy> + Clone,
+	{
+		serde_json::from_reader(reader).map(|ok: HashMap<String, NewDappsPolicy>|
+			ok.into_iter().map(|(a, m)| (a.into(), m.into())).collect()
+		)
+	}
+
+	/// Write a hash map of `String -> NewDappsPolicy`
+	pub fn write_new_dapps_policy<W, S>(m: &HashMap<String, S>, writer: &mut W) -> Result<(), serde_json::Error> where
+		W: io::Write,
+		S: Into<NewDappsPolicy> + Clone,
+	{
+		serde_json::to_writer(writer, &m.iter().map(|(a, m)| (a.clone().into(), m.clone().into())).collect::<HashMap<String, NewDappsPolicy>>())
+	}
+}

json/src/misc/mod.rs

@@ -19,5 +19,5 @@
 mod account_meta;
 mod dapps_settings;
 
-pub use self::dapps_settings::DappsSettings;
+pub use self::dapps_settings::{DappsSettings, NewDappsPolicy};
 pub use self::account_meta::AccountMeta;

rpc/src/v1/impls/eth.rs

@@ -340,7 +340,11 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM> Eth for EthClient<C, SN, S, M, EM> where
 		let dapp = id.0;
 
 		let store = take_weak!(self.accounts);
-		let accounts = try!(store.dapps_addresses(dapp.into()).map_err(|e| errors::internal("Could not fetch accounts.", e)));
+		let accounts = try!(store
+			.note_dapp_used(dapp.clone().into())
+			.and_then(|_| store.dapps_addresses(dapp.into()))
+			.map_err(|e| errors::internal("Could not fetch accounts.", e))
+		);
 
 		Ok(accounts.into_iter().map(Into::into).collect())
 	}

rpc/src/v1/impls/parity_accounts.rs

@@ -164,19 +164,51 @@ impl<C: 'static> ParityAccounts for ParityAccountsClient<C> where C: MiningBlock
 
 	fn set_dapps_addresses(&self, dapp: DappId, addresses: Vec<RpcH160>) -> Result<bool, Error> {
 		let store = take_weak!(self.accounts);
-		let addresses = addresses.into_iter().map(Into::into).collect();
 
-		store.set_dapps_addresses(dapp.into(), addresses)
+		store.set_dapps_addresses(dapp.into(), into_vec(addresses))
 			.map_err(|e| errors::account("Couldn't set dapps addresses.", e))
 			.map(|_| true)
 	}
 
+	fn dapps_addresses(&self, dapp: DappId) -> Result<Vec<RpcH160>, Error> {
+		let store = take_weak!(self.accounts);
+
+		store.dapps_addresses(dapp.into())
+			.map_err(|e| errors::account("Couldn't get dapps addresses.", e))
+			.map(into_vec)
+	}
+
+	fn set_new_dapps_whitelist(&self, whitelist: Option<Vec<RpcH160>>) -> Result<bool, Error> {
+		let store = take_weak!(self.accounts);
+
+		store
+			.set_new_dapps_whitelist(whitelist.map(into_vec))
+			.map_err(|e| errors::account("Couldn't set dapps whitelist.", e))
+			.map(|_| true)
+	}
+
+	fn new_dapps_whitelist(&self) -> Result<Option<Vec<RpcH160>>, Error> {
+		let store = take_weak!(self.accounts);
+
+		store.new_dapps_whitelist()
+			.map_err(|e| errors::account("Couldn't get dapps whitelist.", e))
+			.map(|accounts| accounts.map(into_vec))
+	}
+
+	fn recent_dapps(&self) -> Result<Vec<DappId>, Error> {
+		let store = take_weak!(self.accounts);
+
+		store.recent_dapps()
+			.map_err(|e| errors::account("Couldn't get recent dapps.", e))
+			.map(into_vec)
+	}
+
 	fn import_geth_accounts(&self, addresses: Vec<RpcH160>) -> Result<Vec<RpcH160>, Error> {
 		let store = take_weak!(self.accounts);
 
 		store
-			.import_geth_accounts(addresses.into_iter().map(Into::into).collect(), false)
+			.import_geth_accounts(into_vec(addresses), false)
-			.map(|imported| imported.into_iter().map(Into::into).collect())
+			.map(into_vec)
 			.map_err(|e| errors::account("Couldn't import Geth accounts", e))
 	}
 
@@ -184,10 +216,12 @@ impl<C: 'static> ParityAccounts for ParityAccountsClient<C> where C: MiningBlock
 		try!(self.active());
 		let store = take_weak!(self.accounts);
 
-		Ok(store.list_geth_accounts(false)
+		Ok(into_vec(store.list_geth_accounts(false)))
-			.into_iter()
-			.map(Into::into)
-			.collect()
-		)
 	}
 }
+
+fn into_vec<A, B>(a: Vec<A>) -> Vec<B> where
+	A: Into<B>
+{
+	a.into_iter().map(Into::into).collect()
+}

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

@@ -354,11 +354,17 @@ fn rpc_eth_gas_price() {
 #[test]
 fn rpc_eth_accounts() {
 	let tester = EthTester::default();
-	let _address = tester.accounts_provider.new_account("").unwrap();
+	let address = tester.accounts_provider.new_account("").unwrap();
 
+	// with default policy it should return the account
+	let request = r#"{"jsonrpc": "2.0", "method": "eth_accounts", "params": [], "id": 1}"#;
+	let response = r#"{"jsonrpc":"2.0","result":[""#.to_owned() + &format!("0x{:?}", address) + r#""],"id":1}"#;
+	assert_eq!(tester.io.handle_request_sync(request), Some(response.to_owned()));
+
+	tester.accounts_provider.set_new_dapps_whitelist(Some(vec![1.into()])).unwrap();
 	// even with some account it should return empty list (no dapp detected)
 	let request = r#"{"jsonrpc": "2.0", "method": "eth_accounts", "params": [], "id": 1}"#;
-	let response = r#"{"jsonrpc":"2.0","result":[],"id":1}"#;
+	let response = r#"{"jsonrpc":"2.0","result":["0x0000000000000000000000000000000000000001"],"id":1}"#;
 	assert_eq!(tester.io.handle_request_sync(request), Some(response.to_owned()));
 
 	// when we add visible address it should return that.

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

@@ -117,7 +117,7 @@ fn should_be_able_to_set_meta() {
 }
 
 #[test]
-fn rpc_parity_set_dapps_accounts() {
+fn rpc_parity_set_and_get_dapps_accounts() {
 	// given
 	let tester = setup();
 	assert_eq!(tester.accounts.dapps_addresses("app1".into()).unwrap(), vec![]);
@@ -129,6 +129,52 @@ fn rpc_parity_set_dapps_accounts() {
 
 	// then
 	assert_eq!(tester.accounts.dapps_addresses("app1".into()).unwrap(), vec![10.into()]);
+	let request = r#"{"jsonrpc": "2.0", "method": "parity_getDappsAddresses","params":["app1"], "id": 1}"#;
+	let response = r#"{"jsonrpc":"2.0","result":["0x000000000000000000000000000000000000000a"],"id":1}"#;
+	assert_eq!(tester.io.handle_request_sync(request), Some(response.to_owned()));
+}
+
+#[test]
+fn rpc_parity_set_and_get_new_dapps_whitelist() {
+	// given
+	let tester = setup();
+
+	// when set to whitelist
+	let request = r#"{"jsonrpc": "2.0", "method": "parity_setNewDappsWhitelist","params":[["0x000000000000000000000000000000000000000a"]], "id": 1}"#;
+	let response = r#"{"jsonrpc":"2.0","result":true,"id":1}"#;
+	assert_eq!(tester.io.handle_request_sync(request), Some(response.to_owned()));
+
+	// then
+	assert_eq!(tester.accounts.new_dapps_whitelist().unwrap(), Some(vec![10.into()]));
+	let request = r#"{"jsonrpc": "2.0", "method": "parity_getNewDappsWhitelist","params":[], "id": 1}"#;
+	let response = r#"{"jsonrpc":"2.0","result":["0x000000000000000000000000000000000000000a"],"id":1}"#;
+	assert_eq!(tester.io.handle_request_sync(request), Some(response.to_owned()));
+
+	// when set to empty
+	let request = r#"{"jsonrpc": "2.0", "method": "parity_setNewDappsWhitelist","params":[null], "id": 1}"#;
+	let response = r#"{"jsonrpc":"2.0","result":true,"id":1}"#;
+	assert_eq!(tester.io.handle_request_sync(request), Some(response.to_owned()));
+
+	// then
+	assert_eq!(tester.accounts.new_dapps_whitelist().unwrap(), None);
+	let request = r#"{"jsonrpc": "2.0", "method": "parity_getNewDappsWhitelist","params":[], "id": 1}"#;
+	let response = r#"{"jsonrpc":"2.0","result":null,"id":1}"#;
+	assert_eq!(tester.io.handle_request_sync(request), Some(response.to_owned()));
+}
+
+#[test]
+fn rpc_parity_recent_dapps() {
+	// given
+	let tester = setup();
+
+	// when
+	// trigger dapp usage
+	tester.accounts.note_dapp_used("dapp1".into()).unwrap();
+
+	// then
+	let request = r#"{"jsonrpc": "2.0", "method": "parity_listRecentDapps","params":[], "id": 1}"#;
+	let response = r#"{"jsonrpc":"2.0","result":["dapp1"],"id":1}"#;
+	assert_eq!(tester.io.handle_request_sync(request), Some(response.to_owned()));
 }
 
 #[test]

rpc/src/v1/traits/parity_accounts.rs

@@ -79,6 +79,24 @@ build_rpc_trait! {
 		#[rpc(name = "parity_setDappsAddresses")]
 		fn set_dapps_addresses(&self, DappId, Vec<H160>) -> Result<bool, Error>;
 
+		/// Gets accounts exposed for particular dapp.
+		#[rpc(name = "parity_getDappsAddresses")]
+		fn dapps_addresses(&self, DappId) -> Result<Vec<H160>, Error>;
+
+		/// Sets accounts exposed for new dapps.
+		/// `None` means that all accounts will be exposed.
+		#[rpc(name = "parity_setNewDappsWhitelist")]
+		fn set_new_dapps_whitelist(&self, Option<Vec<H160>>) -> Result<bool, Error>;
+
+		/// Gets accounts exposed for new dapps.
+		/// `None` means that all accounts will be exposed.
+		#[rpc(name = "parity_getNewDappsWhitelist")]
+		fn new_dapps_whitelist(&self) -> Result<Option<Vec<H160>>, Error>;
+
+		/// Sets accounts exposed for particular dapp.
+		#[rpc(name = "parity_listRecentDapps")]
+		fn recent_dapps(&self) -> Result<Vec<DappId>, Error>;
+
 		/// Imports a number of Geth accounts, with the list provided as the argument.
 		#[rpc(name = "parity_importGethAccounts")]
 		fn import_geth_accounts(&self, Vec<H160>) -> Result<Vec<H160>, Error>;

rpc/src/v1/types/dapp_id.rs

@@ -26,6 +26,12 @@ impl Into<String> for DappId {
 	}
 }
 
+impl From<String> for DappId {
+	fn from(s: String) -> Self {
+		DappId(s)
+	}
+}
+
 #[cfg(test)]
 mod tests {