only add transactions to signing-queue if it is enabled (#11272)

* only add transactions to signing-queue if it is enabled

* Update rpc/src/v1/helpers/errors.rs

Co-Authored-By: David <dvdplm@gmail.com>

* use errors::codes::ACCOUNT_LOCKED

* bail early if account isn't unlocked

* use errors::signing

* Update rpc/src/v1/helpers/errors.rs

* Update rpc/src/v1/helpers/errors.rs

* test

* adds cli flag to enable signing queue.

* use helper method siginig_queue_disabled instead of accounts::SignError

* fix typo, use raw i64

* fixed tests

parity/cli/mod.rs

@@ -348,6 +348,10 @@ usage! {
 			"--unlock=[ACCOUNTS]",
 			"Unlock ACCOUNTS for the duration of the execution. ACCOUNTS is a comma-delimited list of addresses.",
 
+			ARG arg_enable_signing_queue: (bool) = false, or |c: &Config| c.account.as_ref()?.enable_signing_queue,
+			"--enable-signing-queue=[BOOLEAN]",
+			"Enables the signing queue for external transaction signing either via CLI or personal_unlockAccount, turned off by default.",
+
 			ARG arg_password: (Vec<String>) = Vec::new(), or |c: &Config| c.account.as_ref()?.password.clone(),
 			"--password=[FILE]...",
 			"Provide a file containing a password for unlocking an account. Leading and trailing whitespace is trimmed.",
@@ -1194,6 +1198,7 @@ struct Operating {
 #[serde(deny_unknown_fields)]
 struct Account {
 	unlock: Option<Vec<String>>,
+	enable_signing_queue: Option<bool>,
 	password: Option<Vec<String>>,
 	keys_iterations: Option<u32>,
 	refresh_time: Option<u64>,
@@ -1728,6 +1733,7 @@ mod tests {
 			arg_restore_file: None,
 			arg_tools_hash_file: None,
 
+			arg_enable_signing_queue: false,
 			arg_signer_sign_id: None,
 			arg_signer_reject_id: None,
 			arg_dapp_path: None,
@@ -2045,6 +2051,7 @@ mod tests {
 				_legacy_public_node: None,
 			}),
 			account: Some(Account {
+				enable_signing_queue: None,
 				unlock: Some(vec!["0x1".into(), "0x2".into(), "0x3".into()]),
 				password: Some(vec!["passwdfile path".into()]),
 				keys_iterations: None,

parity/configuration.rs

@@ -898,7 +898,7 @@ impl Configuration {
 	fn ws_config(&self) -> Result<WsConfiguration, String> {
 		let support_token_api =
 			// enabled when not unlocking
-			self.args.arg_unlock.is_none();
+			self.args.arg_unlock.is_none() && self.args.arg_enable_signing_queue;
 
 		let conf = WsConfiguration {
 			enabled: self.ws_enabled(),
@@ -1388,7 +1388,7 @@ mod tests {
 			origins: Some(vec!["parity://*".into(),"chrome-extension://*".into(), "moz-extension://*".into()]),
 			hosts: Some(vec![]),
 			signer_path: expected.into(),
-			support_token_api: true,
+			support_token_api: false,
 			max_connections: 100,
 		}, LogConfig {
 			color: !cfg!(windows),

parity/rpc.rs

@@ -119,7 +119,7 @@ impl Default for WsConfiguration {
 			origins: Some(vec!["parity://*".into(),"chrome-extension://*".into(), "moz-extension://*".into()]),
 			hosts: Some(Vec::new()),
 			signer_path: replace_home(&data_dir, "$BASE/signer").into(),
-			support_token_api: true,
+			support_token_api: false,
 		}
 	}
 }

rpc/src/v1/helpers/errors.rs

@@ -380,6 +380,16 @@ pub fn invalid_call_data<T: fmt::Display>(error: T) -> Error {
 	}
 }
 
+pub fn signing_queue_disabled() -> Error {
+	Error {
+		code: ErrorCode::ServerError(-32020),
+		message: "Your account is locked and the signing queue is disabled. \
+		You can either Unlock the account via CLI, personal_unlockAccount or \
+		enable the signing queue to use Trusted Signer.".into(),
+		data: None,
+	}
+}
+
 #[cfg(any(test, feature = "accounts"))]
 pub fn signing(error: ::accounts::SignError) -> Error {
 	Error {

rpc/src/v1/impls/signing.rs

@@ -114,6 +114,12 @@ impl<D: Dispatcher + 'static> SigningQueueClient<D> {
 
 	fn dispatch(&self, payload: RpcConfirmationPayload, origin: Origin) -> BoxFuture<DispatchResult> {
 		let default_account = self.accounts.default_account();
+		let from = &payload.sender().unwrap_or(&default_account);
+		// bail early if the account isn't unlocked
+		if !self.accounts.is_unlocked(from) && !self.signer.is_enabled() {
+			return Box::new(future::done(Err(errors::signing_queue_disabled())))
+		}
+
 		let accounts = self.accounts.clone();
 		let dispatcher = self.dispatcher.clone();
 		let signer = self.signer.clone();
@@ -127,7 +133,9 @@ impl<D: Dispatcher + 'static> SigningQueueClient<D> {
 				} else {
 					Either::B(future::done(
 						signer.add_request(payload, origin)
-							.map(|(id, future)| DispatchResult::Future(id, future))
+							.map(|(id, future)| {
+								DispatchResult::Future(id, future)
+							})
 							.map_err(|_| errors::request_rejected_limit())
 					))
 				}

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

@@ -50,10 +50,10 @@ struct SigningTester {
 	pub io: IoHandler<Metadata>,
 }
 
-impl Default for SigningTester {
-	fn default() -> Self {
+impl SigningTester {
+	fn new(signing_queue_enabled: bool) ->Self {
 		let runtime = Runtime::with_thread_count(1);
-		let signer = Arc::new(SignerService::new_test(false));
+		let signer = Arc::new(SignerService::new_test(signing_queue_enabled));
 		let client = Arc::new(TestBlockChainClient::default());
 		let miner = Arc::new(TestMinerService::default());
 		let accounts = Arc::new(AccountProvider::transient_provider());
@@ -81,15 +81,15 @@ impl Default for SigningTester {
 	}
 }
 
-fn eth_signing() -> SigningTester {
-	SigningTester::default()
+fn eth_signing(signing_queue_enabled: bool) -> SigningTester {
+	SigningTester::new(signing_queue_enabled)
 }
 
 #[test]
 fn rpc_eth_sign() {
 	use rustc_hex::FromHex;
 
-	let tester = eth_signing();
+	let tester = eth_signing(true);
 
 	let account = tester.accounts.insert_account(Secret::from([69u8; 32]), &"abcd".into()).unwrap();
 	tester.accounts.unlock_account_permanently(account, "abcd".into()).unwrap();
@@ -112,7 +112,7 @@ fn rpc_eth_sign() {
 #[test]
 fn should_add_sign_to_queue() {
 	// given
-	let tester = eth_signing();
+	let tester = eth_signing(true);
 	let address = Address::random();
 	assert_eq!(tester.signer.requests().len(), 0);
 
@@ -150,7 +150,7 @@ fn should_add_sign_to_queue() {
 #[test]
 fn should_post_sign_to_queue() {
 	// given
-	let tester = eth_signing();
+	let tester = eth_signing(true);
 	let address = Address::random();
 	assert_eq!(tester.signer.requests().len(), 0);
 
@@ -174,7 +174,7 @@ fn should_post_sign_to_queue() {
 #[test]
 fn should_check_status_of_request() {
 	// given
-	let tester = eth_signing();
+	let tester = eth_signing(true);
 	let address = Address::random();
 	let request = r#"{
 		"jsonrpc": "2.0",
@@ -203,7 +203,7 @@ fn should_check_status_of_request() {
 #[test]
 fn should_check_status_of_request_when_its_resolved() {
 	// given
-	let tester = eth_signing();
+	let tester = eth_signing(true);
 	let address = Address::random();
 	let request = r#"{
 		"jsonrpc": "2.0",
@@ -234,10 +234,56 @@ fn should_check_status_of_request_when_its_resolved() {
 	assert_eq!(tester.io.handle_request_sync(&request), Some(response.to_owned()));
 }
 
+#[test]
+fn eth_sign_locked_account() {
+	let secret = "8a283037bb19c4fed7b1c569e40c7dcff366165eb869110a1b11532963eb9cb2".parse().unwrap();
+	let tester = eth_signing(false);
+	let address = tester.accounts.insert_account(secret, &"".into()).unwrap();
+
+	let req_send_trans = r#"{
+		"jsonrpc": "2.0",
+		"method": "eth_sendTransaction",
+		"params": [{
+			"from": ""#.to_owned() + format!("0x{:x}", address).as_ref() + r#"",
+			"to": "0xd46e8dd67c5d32be8058bb8eb970870f07244567",
+			"gas": "0x30000",
+			"gasPrice": "0x1",
+			"value": "0x9184e72a"
+		}],
+		"id": 16
+	}"#;
+
+	// expected error response
+	let error_res = r#"{
+		"jsonrpc":"2.0",
+		"error":
+			{
+				"code":-32020,
+				"message":"Your account is locked and the signing queue is disabled.
+				 You can either Unlock the account via CLI, personal_unlockAccount or
+				 enable the signing queue to use Trusted Signer."
+			},
+		"id":16
+	}"#;
+	// dispatch the transaction, without unlocking the account.
+	assert_eq!(
+		error_res.replace("\t", "").replace("\n", ""),
+		tester.io.handle_request_sync(&req_send_trans).unwrap()
+	);
+	// unlock the account
+	tester.accounts.unlock_account_permanently(address, "".into()).unwrap();
+
+	// try again, this time account is unlocked.
+	assert_eq!(
+		r#"{"jsonrpc":"2.0","result":"0x70df76392fba654351714803d99a90be1d58d165352e0008e376427284314c88","id":16}"#,
+		tester.io.handle_request_sync(&req_send_trans).unwrap()
+	);
+}
+
 #[test]
 fn should_sign_if_account_is_unlocked() {
 	// given
-	let tester = eth_signing();
+	let tester = eth_signing(true);
 	let data = vec![5u8];
 	let acc = tester.accounts.insert_account(Secret::from([69u8; 32]), &"test".into()).unwrap();
 	tester.accounts.unlock_account_permanently(acc, "test".into()).unwrap();
@@ -260,7 +306,7 @@ fn should_sign_if_account_is_unlocked() {
 #[test]
 fn should_add_transaction_to_queue() {
 	// given
-	let tester = eth_signing();
+	let tester = eth_signing(true);
 	let address = Address::random();
 	assert_eq!(tester.signer.requests().len(), 0);
 
@@ -301,7 +347,7 @@ fn should_add_transaction_to_queue() {
 #[test]
 fn should_add_sign_transaction_to_the_queue() {
 	// given
-	let tester = eth_signing();
+	let tester = eth_signing(true);
 	let address = tester.accounts.new_account(&"test".into()).unwrap();
 
 	assert_eq!(tester.signer.requests().len(), 0);
@@ -380,7 +426,7 @@ fn should_add_sign_transaction_to_the_queue() {
 #[test]
 fn should_dispatch_transaction_if_account_is_unlock() {
 	// given
-	let tester = eth_signing();
+	let tester = eth_signing(true);
 	let acc = tester.accounts.new_account(&"test".into()).unwrap();
 	tester.accounts.unlock_account_permanently(acc, "test".into()).unwrap();
 
@@ -417,7 +463,7 @@ fn should_dispatch_transaction_if_account_is_unlock() {
 #[test]
 fn should_decrypt_message_if_account_is_unlocked() {
 	// given
-	let mut tester = eth_signing();
+	let mut tester = eth_signing(true);
 	let parity = parity::Dependencies::new();
 	tester.io.extend_with(parity.client(None).to_delegate());
 	let (address, public) = tester.accounts.new_account_and_public(&"test".into()).unwrap();
@@ -449,7 +495,7 @@ fn should_decrypt_message_if_account_is_unlocked() {
 #[test]
 fn should_add_decryption_to_the_queue() {
 	// given
-	let tester = eth_signing();
+	let tester = eth_signing(true);
 	let acc = Random.generate().unwrap();
 	assert_eq!(tester.signer.requests().len(), 0);
 
@@ -486,7 +532,7 @@ fn should_add_decryption_to_the_queue() {
 #[test]
 fn should_compose_transaction() {
 	// given
-	let tester = eth_signing();
+	let tester = eth_signing(true);
 	let acc = Random.generate().unwrap();
 	assert_eq!(tester.signer.requests().len(), 0);
 	let from = format!("{:x}", acc.address());

rpc/src/v1/types/confirmations.rs

@@ -229,6 +229,18 @@ impl From<helpers::ConfirmationPayload> for ConfirmationPayload {
 	}
 }
 
+impl ConfirmationPayload {
+	pub fn sender(&self) -> Option<&H160> {
+		match *self {
+			ConfirmationPayload::SendTransaction(ref request) => request.from.as_ref(),
+			ConfirmationPayload::SignTransaction(ref request) => request.from.as_ref(),
+			ConfirmationPayload::EthSignMessage(ref request) => Some(&request.address),
+			ConfirmationPayload::EIP191SignMessage(ref request) => Some(&request.address),
+			ConfirmationPayload::Decrypt(ref request) => Some(&request.address),
+		}
+	}
+}
+
 /// Possible modifications to the confirmed transaction sent by `Trusted Signer`
 #[derive(Debug, PartialEq, Serialize, Deserialize)]
 #[serde(deny_unknown_fields)]