From 818f764583a8700c975af15245cff11d959d50a5 Mon Sep 17 00:00:00 2001 From: Aurelia <56112063+AureliaDolo@users.noreply.github.com> Date: Tue, 10 Dec 2024 17:23:10 +0100 Subject: [PATCH] Cli shared recovery create - ask for threshold if not provided. (#9127) * Cli shared recovery create - ask for threshold if not provided. * CLI shared recovery create - add test for interactive threshold * [CLI] rework spinners and display for shared recovery. --- .cspell/custom-words.txt | 1 + Cargo.lock | 78 ++++++++++++++++++- Cargo.toml | 2 + cli/Cargo.toml | 2 + cli/src/commands/shared_recovery/create.rs | 27 +++++-- cli/src/commands/shared_recovery/delete.rs | 8 +- cli/src/commands/shared_recovery/info.rs | 11 ++- cli/src/commands/shared_recovery/list.rs | 15 ++-- cli/src/utils.rs | 1 + .../integration/shared_recovery/create.rs | 33 +++++++- .../integration/shared_recovery/delete.rs | 41 +--------- cli/tests/integration/shared_recovery/list.rs | 20 +---- cli/tests/integration/shared_recovery/mod.rs | 52 +++++++++++++ 13 files changed, 213 insertions(+), 78 deletions(-) diff --git a/.cspell/custom-words.txt b/.cspell/custom-words.txt index 261e5a58914..34c60ee9bd3 100644 --- a/.cspell/custom-words.txt +++ b/.cspell/custom-words.txt @@ -562,6 +562,7 @@ Reqwest rerunfailures reseted retrier +rexpect Richcmp Rlib rmvb diff --git a/Cargo.lock b/Cargo.lock index ac92bdea8fc..e46616e6214 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -507,6 +507,12 @@ dependencies = [ "memchr", ] +[[package]] +name = "comma" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "55b672471b4e9f9e95499ea597ff64941a309b2cdbffcc46f2cc5e2d971fd335" + [[package]] name = "concurrent-queue" version = "2.4.0" @@ -516,6 +522,19 @@ dependencies = [ "crossbeam-utils", ] +[[package]] +name = "console" +version = "0.15.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0e1f83fc076bd6dd27517eacdf25fef6c4dfe5f1d7448bafaaf3a26f13b5e4eb" +dependencies = [ + "encode_unicode", + "lazy_static", + "libc", + "unicode-width", + "windows-sys 0.52.0", +] + [[package]] name = "console_error_panic_hook" version = "0.1.6" @@ -745,7 +764,7 @@ version = "3.4.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "90eeab0aa92f3f9b4e87f258c72b139c207d251f9cbc1080a0086b86a8870dd3" dependencies = [ - "nix", + "nix 0.29.0", "windows-sys 0.59.0", ] @@ -895,6 +914,17 @@ dependencies = [ "serde", ] +[[package]] +name = "dialoguer" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "658bce805d770f407bc62102fca7c2c64ceef2fbcb2b8bd19d2765ce093980de" +dependencies = [ + "console", + "shell-words", + "thiserror", +] + [[package]] name = "diff" version = "0.1.13" @@ -1019,6 +1049,12 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "encode_unicode" +version = "0.3.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a357d28ed41a50f9c765dbfe56cbc04a64e53e5fc58ba79fbc34c10ef3df831f" + [[package]] name = "env_logger" version = "0.10.2" @@ -1194,7 +1230,7 @@ dependencies = [ "libc", "log", "memchr", - "nix", + "nix 0.29.0", "page_size", "pkg-config", "smallvec", @@ -2581,6 +2617,17 @@ dependencies = [ "syn-mid", ] +[[package]] +name = "nix" +version = "0.27.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2eb04e9c688eff1c89d72b407f168cf79bb9e867a9d3323ed6c01519eb9cc053" +dependencies = [ + "bitflags 2.6.0", + "cfg-if 1.0.0", + "libc", +] + [[package]] name = "nix" version = "0.29.0" @@ -2861,6 +2908,7 @@ dependencies = [ "assert_cmd", "clap", "criterion", + "dialoguer", "env_logger", "itertools 0.12.1", "libparsec", @@ -2871,6 +2919,7 @@ dependencies = [ "log", "predicates", "reqwest", + "rexpect", "rpassword", "rstest", "serde", @@ -3435,6 +3484,19 @@ dependencies = [ "thiserror", ] +[[package]] +name = "rexpect" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c020234fb542618dc3e3d43724e9d93f87e1db74040a76a8c4e830220fb9b20d" +dependencies = [ + "comma", + "nix 0.27.1", + "regex", + "tempfile", + "thiserror", +] + [[package]] name = "rmp" version = "0.8.14" @@ -3947,6 +4009,12 @@ dependencies = [ "zeroize", ] +[[package]] +name = "shell-words" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "24188a676b6ae68c3b2cb3a01be17fbf7240ce009799bb56d5b1409051e78fde" + [[package]] name = "signature" version = "1.6.4" @@ -4579,6 +4647,12 @@ version = "1.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d4c87d22b6e3f4a18d4d40ef354e97c90fcb14dd91d7dc0aa9d8a1172ebf7202" +[[package]] +name = "unicode-width" +version = "0.1.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7dd6e30e90baa6f72411720665d41d89b9a3d039dc45b8faea1ddd07f617f6af" + [[package]] name = "unicode_categories" version = "0.1.1" diff --git a/Cargo.toml b/Cargo.toml index a4f7b1f31f5..9923ed030be 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -123,6 +123,7 @@ crypto_secretbox = { version = "0.1.1", default-features = false } ctrlc = { version = "3.4.5", default-features = false } # You need base64&co ? This is the crate you need ! data-encoding = { version = "2.6.0", default-features = false } +dialoguer = { version = "0.11.0", default-features = false } digest = { version = "0.10.7", default-features = false } dirs = { version = "5.0.1", default-features = false } ed25519-dalek = { version = "2.1.1", default-features = false } @@ -172,6 +173,7 @@ regex = { version = "1.11.1", default-features = false } regex-syntax = { version = "0.8.4", default-features = false } reqwest = { version = "0.12.9", default-features = false } reqwest-eventsource = { version = "0.6.0", default-features = false } +rexpect = "0.6.0" rmp-serde = { version = "1.3.0", default-features = false } rpassword = { version = "7.3.1", default-features = false } rsa = { version = "0.8.2", default-features = false } diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 4b15b1e51dc..2297c340f67 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -28,6 +28,7 @@ libparsec_platform_ipc = { workspace = true } anyhow = { workspace = true } clap = { workspace = true, features = ["default", "derive", "env"] } +dialoguer = { workspace = true } env_logger = { workspace = true, features = ["auto-color", "humantime", "regex"] } log = { workspace = true } reqwest = { workspace = true, features = ["json"] } @@ -45,6 +46,7 @@ libparsec = { workspace = true, features = ["cli-tests"] } assert_cmd = { workspace = true } predicates = { workspace = true, features = ["regex"] } +rexpect = { workspace = true } rstest = { workspace = true } uuid = { workspace = true, features = ["v6", "std", "rng"] } diff --git a/cli/src/commands/shared_recovery/create.rs b/cli/src/commands/shared_recovery/create.rs index 1aec685ce45..59599e65665 100644 --- a/cli/src/commands/shared_recovery/create.rs +++ b/cli/src/commands/shared_recovery/create.rs @@ -1,8 +1,9 @@ use std::{collections::HashMap, num::NonZeroU8}; +use dialoguer::Input; use libparsec::{UserID, UserProfile}; -use crate::utils::{start_spinner, StartedClient}; +use crate::utils::{start_spinner, StartedClient, GREEN_CHECKMARK}; // TODO: should provide the recipients and their share count as a single parameter // e.g. `--recipients=foo@example.com=2,bar@example.com=3` @@ -20,8 +21,8 @@ crate::clap_parser_with_shared_opts_builder!( #[arg(short, long, requires = "recipients", num_args = 1..=255)] weights: Option>, /// Threshold number of shares required to proceed with recovery. - #[arg(short, long)] - threshold: NonZeroU8, + #[arg(short, long, requires = "recipients")] + threshold: Option, } ); @@ -36,8 +37,9 @@ pub async fn create_shared_recovery(args: Args, client: &StartedClient) -> anyho } = args; { - let _spinner = start_spinner("Poll server for new certificates".into()); + let mut spinner = start_spinner("Poll server for new certificates".into()); client.poll_server_for_new_certificates().await?; + spinner.stop_with_symbol(GREEN_CHECKMARK); } let mut handle = start_spinner("Creating shared recovery setup".into()); @@ -81,12 +83,27 @@ pub async fn create_shared_recovery(args: Args, client: &StartedClient) -> anyho .map(|user_id| (user_id, weight)) .collect() }; + // we must stop the handle here to avoid messing up with the threshold choice + handle.stop_with_newline(); + let threshold = if let Some(t) = threshold { + t + } else { + // note that this is a blocking call + Input::::new() + .with_prompt(format!( + "Choose a threshold between 1 and {}\nThe threshold is the minimum number of recipients that one must gather to recover the account", + per_recipient_shares.len() + )) .interact_text()? + }; + let mut handle = start_spinner("Creating shared recovery setup".into()); client .setup_shamir_recovery(per_recipient_shares, threshold) .await?; - handle.stop_with_message("Shared recovery setup has been created".into()); + handle.stop_with_message(format!( + "{GREEN_CHECKMARK} Shared recovery setup has been created" + )); client.stop().await; diff --git a/cli/src/commands/shared_recovery/delete.rs b/cli/src/commands/shared_recovery/delete.rs index dcfcae76d14..9cc6028c513 100644 --- a/cli/src/commands/shared_recovery/delete.rs +++ b/cli/src/commands/shared_recovery/delete.rs @@ -16,13 +16,17 @@ pub async fn main(args: Args) -> anyhow::Result<()> { let client = load_client(&config_dir, device, password_stdin).await?; { - let _spinner = start_spinner("Poll server for new certificates".into()); + let mut spinner = start_spinner("Poll server for new certificates".into()); client.poll_server_for_new_certificates().await?; + spinner.stop_with_symbol(GREEN_CHECKMARK); } + let mut handle = start_spinner("Deleting shared recovery setup".into()); client.delete_shamir_recovery().await?; - println!("Deleted shared recovery for {}", client.user_id()); + handle.stop_with_message(format!( + "{GREEN_CHECKMARK} Shared recovery setup has been deleted" + )); client.stop().await; diff --git a/cli/src/commands/shared_recovery/info.rs b/cli/src/commands/shared_recovery/info.rs index b986befaca4..aa79d07ac3f 100644 --- a/cli/src/commands/shared_recovery/info.rs +++ b/cli/src/commands/shared_recovery/info.rs @@ -18,8 +18,9 @@ pub async fn main(args: Args) -> anyhow::Result<()> { let client = load_client(&config_dir, device, password_stdin).await?; { - let _spinner = start_spinner("Poll server for new certificates".into()); + let mut spinner = start_spinner("Poll server for new certificates".into()); client.poll_server_for_new_certificates().await?; + spinner.stop_with_symbol(GREEN_CHECKMARK); } let info = client.get_self_shamir_recovery().await?; @@ -33,15 +34,17 @@ pub async fn main(args: Args) -> anyhow::Result<()> { .. } => println!("{RED}Deleted{RESET} shared recovery - deleted by {deleted_by} on {deleted_on}"), libparsec_client::SelfShamirRecoveryInfo::SetupAllValid { - .. - } => println!("Shared recovery {GREEN}set up{RESET}"), + per_recipient_shares, threshold, .. + } => println!("Shared recovery {GREEN}set up{RESET} with threshold {threshold}\n{}", per_recipient_shares.iter().map(|(recipient, share)| { + format!("• User {recipient} has {share} share(s)") // TODO: special case if there is only on share + }).join("\n")), libparsec_client::SelfShamirRecoveryInfo::SetupWithRevokedRecipients { threshold, per_recipient_shares, revoked_recipients, .. } => println!( - "Shared recovery for {YELLOW}set up{RESET} - contains revoked recipients: {revoked} ({revoked_len} out of {total} total recipients, with threshold {threshold})", + "Shared recovery {YELLOW}set up{RESET} - contains revoked recipients: {revoked} ({revoked_len} out of {total} total recipients, with threshold {threshold})", revoked = revoked_recipients.iter().join(", "), revoked_len = revoked_recipients.len(), total = per_recipient_shares.len()), diff --git a/cli/src/commands/shared_recovery/list.rs b/cli/src/commands/shared_recovery/list.rs index 484030faf02..3e7ec37ffc6 100644 --- a/cli/src/commands/shared_recovery/list.rs +++ b/cli/src/commands/shared_recovery/list.rs @@ -18,8 +18,9 @@ pub async fn main(args: Args) -> anyhow::Result<()> { let client = load_client(&config_dir, device, password_stdin).await?; { - let _spinner = start_spinner("Poll server for new certificates".into()); + let mut spinner = start_spinner("Poll server for new certificates".into()); client.poll_server_for_new_certificates().await?; + spinner.stop_with_symbol(GREEN_CHECKMARK); } let res = client.list_shamir_recoveries_for_others().await?; @@ -38,16 +39,18 @@ pub async fn main(args: Args) -> anyhow::Result<()> { deleted_on, deleted_by, .. - } => println!("Deleted shared recovery for {RED}{user_id}{RESET} - deleted by {deleted_by} on {deleted_on}"), + } => println!("• Deleted shared recovery for {RED}{user_id}{RESET} - deleted by {deleted_by} on {deleted_on}"), libparsec_client::OtherShamirRecoveryInfo::SetupAllValid { - user_id,.. - } => println!("Shared recovery for {GREEN}{user_id}{RESET}"), + user_id, threshold, per_recipient_shares,.. + } => println!("Shared recovery for {GREEN}{user_id}{RESET} with threshold {threshold}\n{}", per_recipient_shares.iter().map(|(recipient, share)| { + format!("\t• User {recipient} has {share} share(s)") // TODO: special case if there is only on share + }).join("\n")), libparsec_client::OtherShamirRecoveryInfo::SetupWithRevokedRecipients { user_id, threshold, per_recipient_shares, revoked_recipients,.. - } => println!("Shared recovery for {YELLOW}{user_id}{RESET} - contains revoked recipients: {revoked} ({revoked_len} out of {total} total recipients, with threshold {threshold})", + } => println!("• Shared recovery for {YELLOW}{user_id}{RESET} - contains revoked recipients: {revoked} ({revoked_len} out of {total} total recipients, with threshold {threshold})", revoked = revoked_recipients.iter().join(", "), revoked_len = revoked_recipients.len(), total = per_recipient_shares.len()), @@ -56,7 +59,7 @@ pub async fn main(args: Args) -> anyhow::Result<()> { threshold, per_recipient_shares, revoked_recipients,.. - } => println!("Unusable shared recovery for {RED}{user_id}{RESET} - contains revoked recipients: {revoked} ({revoked_len} out of {total} total recipients, with threshold {threshold})", + } => println!("• Unusable shared recovery for {RED}{user_id}{RESET} - contains revoked recipients: {revoked} ({revoked_len} out of {total} total recipients, with threshold {threshold})", revoked = revoked_recipients.iter().join(", "), revoked_len = revoked_recipients.len(), total = per_recipient_shares.len()), diff --git a/cli/src/utils.rs b/cli/src/utils.rs index d97937f44c8..b0c70500d00 100644 --- a/cli/src/utils.rs +++ b/cli/src/utils.rs @@ -24,6 +24,7 @@ pub const GREEN: &str = "\x1B[92m"; pub const RED: &str = "\x1B[91m"; pub const RESET: &str = "\x1B[39m"; pub const YELLOW: &str = "\x1B[33m"; +pub const GREEN_CHECKMARK: &str = "\x1B[92m🗸\x1B[39m"; pub fn format_devices( devices: &[AvailableDevice], diff --git a/cli/tests/integration/shared_recovery/create.rs b/cli/tests/integration/shared_recovery/create.rs index 9168b358c81..6d4fcc67e66 100644 --- a/cli/tests/integration/shared_recovery/create.rs +++ b/cli/tests/integration/shared_recovery/create.rs @@ -1,8 +1,8 @@ use libparsec::{tmp_path, TmpPath}; -use crate::testenv_utils::{TestOrganization, DEFAULT_DEVICE_PASSWORD}; - use crate::integration_tests::bootstrap_cli_test; +use crate::testenv_utils::{TestOrganization, DEFAULT_DEVICE_PASSWORD}; +use rexpect::session::spawn; #[rstest::rstest] #[tokio::test] @@ -84,3 +84,32 @@ async fn create_shared_recovery_inexistent_email(tmp_path: TmpPath) { ) .stderr(predicates::str::contains("A user is missing")); } + +#[rstest::rstest] +#[tokio::test] +async fn create_shared_recovery_default(tmp_path: TmpPath) { + let (_, TestOrganization { bob, .. }, _) = bootstrap_cli_test(&tmp_path).await.unwrap(); + let mut cmd = assert_cmd::Command::cargo_bin("parsec-cli").unwrap(); + cmd.args([ + "shared-recovery", + "create", + "--device", + &bob.device_id.hex(), + ]); + + let program = cmd.get_program().to_str().unwrap().to_string(); + let program = cmd + .get_args() + .fold(program, |acc, s| format!("{acc} {s:?}")); + + let mut p = spawn(&dbg!(program), Some(1000)).unwrap(); + + p.exp_string("Enter password for the device:").unwrap(); + p.send_line(DEFAULT_DEVICE_PASSWORD).unwrap(); + + p.exp_regex(".*The threshold is the minimum number of recipients that one must gather to recover the account:.*").unwrap(); + p.send_line("1").unwrap(); + p.exp_regex(".*Shared recovery setup has been created.*") + .unwrap(); + p.exp_eof().unwrap(); +} diff --git a/cli/tests/integration/shared_recovery/delete.rs b/cli/tests/integration/shared_recovery/delete.rs index 4bd39cadb48..17efba91856 100644 --- a/cli/tests/integration/shared_recovery/delete.rs +++ b/cli/tests/integration/shared_recovery/delete.rs @@ -1,5 +1,6 @@ use libparsec::{tmp_path, TmpPath}; +use crate::integration_tests::shared_recovery::setup_shared_recovery; use crate::testenv_utils::{TestOrganization, DEFAULT_DEVICE_PASSWORD}; use crate::utils::*; @@ -16,45 +17,7 @@ async fn remove_shared_recovery_ok(tmp_path: TmpPath) { _, ) = bootstrap_cli_test(&tmp_path).await.unwrap(); - crate::assert_cmd_success!( - with_password = DEFAULT_DEVICE_PASSWORD, - "shared-recovery", - "info", - "--device", - &alice.device_id.hex() - ) - .stdout(predicates::str::contains(format!( - "Shared recovery {RED}never setup{RESET}" - ))); - crate::assert_cmd_success!( - with_password = DEFAULT_DEVICE_PASSWORD, - "shared-recovery", - "create", - "--device", - &alice.device_id.hex(), - "--recipients", - &bob.human_handle.email(), - &toto.human_handle.email(), - "--weights", - "1", - "1", - "--threshold", - "1" - ) - .stdout(predicates::str::contains( - "Shared recovery setup has been created", - )); - - crate::assert_cmd_success!( - with_password = DEFAULT_DEVICE_PASSWORD, - "shared-recovery", - "info", - "--device", - &alice.device_id.hex() - ) - .stdout(predicates::str::contains(format!( - "Shared recovery {GREEN}set up{RESET}" - ))); + setup_shared_recovery(&alice, &bob, &toto); crate::assert_cmd_success!( with_password = DEFAULT_DEVICE_PASSWORD, diff --git a/cli/tests/integration/shared_recovery/list.rs b/cli/tests/integration/shared_recovery/list.rs index 3463b202658..2129aba4c63 100644 --- a/cli/tests/integration/shared_recovery/list.rs +++ b/cli/tests/integration/shared_recovery/list.rs @@ -1,5 +1,6 @@ use libparsec::{tmp_path, TmpPath}; +use crate::integration_tests::shared_recovery::setup_shared_recovery; use crate::testenv_utils::{TestOrganization, DEFAULT_DEVICE_PASSWORD}; use crate::utils::*; @@ -25,24 +26,7 @@ async fn list_shared_recovery_ok(tmp_path: TmpPath) { ) .stdout(predicates::str::contains("No shared recovery found")); - crate::assert_cmd_success!( - with_password = DEFAULT_DEVICE_PASSWORD, - "shared-recovery", - "create", - "--device", - &alice.device_id.hex(), - "--recipients", - &bob.human_handle.email(), - &toto.human_handle.email(), - "--weights", - "1", - "1", - "--threshold", - "1" - ) - .stdout(predicates::str::contains( - "Shared recovery setup has been created", - )); + setup_shared_recovery(&alice, &bob, &toto); crate::assert_cmd_success!( with_password = DEFAULT_DEVICE_PASSWORD, diff --git a/cli/tests/integration/shared_recovery/mod.rs b/cli/tests/integration/shared_recovery/mod.rs index b6ce6518194..d6288b5d12f 100644 --- a/cli/tests/integration/shared_recovery/mod.rs +++ b/cli/tests/integration/shared_recovery/mod.rs @@ -1,4 +1,56 @@ +use crate::utils::*; +use libparsec::LocalDevice; +use std::sync::Arc; + +use crate::testenv_utils::DEFAULT_DEVICE_PASSWORD; + mod create; mod delete; mod info; mod list; + +fn setup_shared_recovery( + alice: &Arc, + bob: &Arc, + toto: &Arc, +) { + crate::assert_cmd_success!( + with_password = DEFAULT_DEVICE_PASSWORD, + "shared-recovery", + "info", + "--device", + &alice.device_id.hex() + ) + .stdout(predicates::str::contains(format!( + "Shared recovery {RED}never setup{RESET}" + ))); + crate::assert_cmd_success!( + with_password = DEFAULT_DEVICE_PASSWORD, + "shared-recovery", + "create", + "--device", + &alice.device_id.hex(), + "--recipients", + &bob.human_handle.email(), + &toto.human_handle.email(), + "--weights", + "1", + "1", + "--threshold", + "1" + ) + .stdout(predicates::str::contains( + "Shared recovery setup has been created", + )); + + crate::assert_cmd_success!( + with_password = DEFAULT_DEVICE_PASSWORD, + "shared-recovery", + "info", + "--device", + &alice.device_id.hex() + ) + .stdout(predicates::str::contains(format!( + "Shared recovery {GREEN}set up{RESET}" + ))); +}