From 9415f3577ddded0861f78768f8fd154cf880b124 Mon Sep 17 00:00:00 2001 From: Firelight Flagboy Date: Tue, 13 Aug 2024 08:51:57 +0200 Subject: [PATCH] Add CLI error where multiple devices match the same short dev id --- cli/src/commands/import_recovery_device.rs | 2 +- cli/src/testenv_utils.rs | 17 +- cli/tests/integration/device_option.rs | 147 +++++++++++++++++- libparsec/crates/client/src/invite/claimer.rs | 4 +- .../crates/platform_device_loader/src/lib.rs | 4 +- libparsec/src/invite.rs | 2 +- 6 files changed, 162 insertions(+), 14 deletions(-) diff --git a/cli/src/commands/import_recovery_device.rs b/cli/src/commands/import_recovery_device.rs index 9e9201c4e6a..9db8760fd04 100644 --- a/cli/src/commands/import_recovery_device.rs +++ b/cli/src/commands/import_recovery_device.rs @@ -48,7 +48,7 @@ pub async fn import_recovery_device( } })?; - let key_file = libparsec::get_default_key_file(&config_dir, &device); + let key_file = libparsec::get_default_key_file(&config_dir, &device.device_id); let access = DeviceAccessStrategy::Password { key_file, password }; diff --git a/cli/src/testenv_utils.rs b/cli/src/testenv_utils.rs index 2e82444f15e..66176c77bdf 100644 --- a/cli/src/testenv_utils.rs +++ b/cli/src/testenv_utils.rs @@ -54,9 +54,10 @@ pub async fn initialize_test_organization( .await?; let access = DeviceAccessStrategy::Password { - key_file: client_config - .config_dir - .join(format!("devices/{}.keys", alice_device.device_id.hex())), + key_file: libparsec::get_default_key_file( + &client_config.config_dir, + &alice_device.device_id, + ), password: DEFAULT_DEVICE_PASSWORD.to_string().into(), }; @@ -146,7 +147,8 @@ async fn register_new_device( Err(e) => return Err(anyhow::anyhow!("{e}")), } - let key_file = libparsec::get_default_key_file(&client_config.config_dir, &new_device); + let key_file = + libparsec::get_default_key_file(&client_config.config_dir, &new_device.device_id); let access = DeviceAccessStrategy::Password { key_file, @@ -205,7 +207,8 @@ async fn register_new_user( Err(e) => return Err(anyhow::anyhow!("{e}")), } - let key_file = libparsec::get_default_key_file(&client_config.config_dir, &new_device); + let key_file = + libparsec::get_default_key_file(&client_config.config_dir, &new_device.device_id); let access = DeviceAccessStrategy::Password { key_file, @@ -217,7 +220,7 @@ async fn register_new_user( Ok(new_device) } -fn create_new_user( +pub(crate) fn create_new_user( new_device: Arc, author: Arc, initial_profile: UserProfile, @@ -242,7 +245,7 @@ fn create_new_user( (user_certificate, redacted_user_certificate) } -fn create_new_device( +pub(crate) fn create_new_device( new_device: Arc, author: Arc, now: DateTime, diff --git a/cli/tests/integration/device_option.rs b/cli/tests/integration/device_option.rs index 4448b3945a2..3ee0eaf9444 100644 --- a/cli/tests/integration/device_option.rs +++ b/cli/tests/integration/device_option.rs @@ -1,6 +1,6 @@ use std::sync::Arc; -use libparsec::{tmp_path, LocalDevice, TmpPath}; +use libparsec::{tmp_path, HumanHandle, LocalDevice, TmpPath}; use predicates::prelude::PredicateBooleanExt; use super::bootstrap_cli_test; @@ -93,3 +93,148 @@ async fn missing_option(tmp_path: TmpPath) { .and(predicates::str::contains(&available_devices_string_list)), ); } + +#[rstest::rstest] +#[tokio::test] +async fn multiple_device_found(tmp_path: TmpPath) { + let config = super::get_testenv_config(); + let url = super::new_environment(&tmp_path, None, config, false) + .await + .unwrap() + .unwrap(); + + let org_id = super::unique_org_id(); + let org_addr = crate::commands::create_organization::create_organization_req( + &org_id, + &url, + crate::testenv_utils::DEFAULT_ADMINISTRATION_TOKEN, + ) + .await + .unwrap(); + + let client_config = libparsec::ClientConfig::default(); + let first_available_device = + crate::commands::bootstrap_organization::bootstrap_organization_req( + client_config.clone(), + org_addr, + "dev1".parse().unwrap(), + libparsec::HumanHandle::new("first@dev1.com", "First").unwrap(), + crate::testenv_utils::DEFAULT_DEVICE_PASSWORD + .to_string() + .into(), + ) + .await + .unwrap(); + + let first_device = libparsec::load_device( + &client_config.config_dir, + &libparsec::DeviceAccessStrategy::Password { + key_file: libparsec::get_default_key_file( + &client_config.config_dir, + &first_available_device.device_id, + ), + password: crate::testenv_utils::DEFAULT_DEVICE_PASSWORD + .to_string() + .into(), + }, + ) + .await + .unwrap(); + + let first_auth_cmds = libparsec::AuthenticatedCmds::new( + &client_config.config_dir, + first_device.clone(), + libparsec::ProxyConfig::default(), + ) + .unwrap(); + + let first_dev_id_prefix = &first_device.device_id.hex()[..8]; + + let second_device = create_second_device( + &first_auth_cmds, + &first_device, + client_config, + first_dev_id_prefix, + ) + .await + .unwrap(); + + let mut devices = [first_device, second_device]; + devices.sort_by_cached_key(|d| d.device_id.hex()); + + let mut available_devices_string_list = String::new(); + format_device::<3>(&devices, &mut available_devices_string_list).unwrap(); + + crate::assert_cmd_failure!("list-users", "--device", first_dev_id_prefix).stderr( + predicates::str::contains(format!( + "Error: Multiple devices found for `{first_dev_id_prefix}`:" + )) + .and(predicates::str::contains(available_devices_string_list)), + ); +} + +async fn create_second_device( + cmds: &libparsec::AuthenticatedCmds, + author: &Arc, + client_config: libparsec::ClientConfig, + dev_id_prefix: &str, +) -> anyhow::Result> { + // Create a device id with the same prefix as the first device + let second_device_id = { + let new_id_hex = libparsec::DeviceID::default().hex(); + libparsec::DeviceID::from_hex(&format!( + "{}{}", + &dev_id_prefix, + &new_id_hex[dev_id_prefix.len()..] + )) + .map_err(anyhow::Error::msg) + }?; + + let second_device = Arc::new(LocalDevice::generate_new_device( + cmds.addr().clone(), + libparsec::UserProfile::Standard, + HumanHandle::new("second@dev2.com", "Second").map_err(anyhow::Error::msg)?, + "second".parse().map_err(anyhow::Error::msg)?, + None, + Some(second_device_id), + None, + None, + )); + + let now = author.now(); + let (user_cert, redacted_user_cert) = crate::testenv_utils::create_new_user( + second_device.clone(), + author.clone(), + libparsec::UserProfile::Standard, + now, + ); + let (dev_cert, redacted_dev_cert) = + crate::testenv_utils::create_new_device(second_device.clone(), author.clone(), now); + + match cmds + .send(libparsec::authenticated_cmds::latest::user_create::Req { + device_certificate: dev_cert, + redacted_device_certificate: redacted_dev_cert, + redacted_user_certificate: redacted_user_cert, + user_certificate: user_cert, + }) + .await? + { + libparsec::authenticated_cmds::latest::user_create::UserCreateRep::Ok => (), + invalid_rep => panic!("Cannot create user: {invalid_rep:?}"), + } + + let key_file = + libparsec::get_default_key_file(&client_config.config_dir, &second_device.device_id); + + let second_dev_access = libparsec::DeviceAccessStrategy::Password { + key_file, + password: crate::testenv_utils::DEFAULT_DEVICE_PASSWORD + .to_string() + .into(), + }; + + libparsec::save_device(std::path::Path::new(""), &second_dev_access, &second_device).await?; + + Ok(second_device) +} diff --git a/libparsec/crates/client/src/invite/claimer.rs b/libparsec/crates/client/src/invite/claimer.rs index 801a8460969..7953470287d 100644 --- a/libparsec/crates/client/src/invite/claimer.rs +++ b/libparsec/crates/client/src/invite/claimer.rs @@ -574,7 +574,7 @@ impl UserClaimFinalizeCtx { pub fn get_default_key_file(&self) -> PathBuf { libparsec_platform_device_loader::get_default_key_file( &self.config.config_dir, - &self.new_local_device, + &self.new_local_device.device_id, ) } @@ -611,7 +611,7 @@ impl DeviceClaimFinalizeCtx { pub fn get_default_key_file(&self) -> PathBuf { libparsec_platform_device_loader::get_default_key_file( &self.config.config_dir, - &self.new_local_device, + &self.new_local_device.device_id, ) } diff --git a/libparsec/crates/platform_device_loader/src/lib.rs b/libparsec/crates/platform_device_loader/src/lib.rs index 4d8d8b991af..7f94fbebe9a 100644 --- a/libparsec/crates/platform_device_loader/src/lib.rs +++ b/libparsec/crates/platform_device_loader/src/lib.rs @@ -88,12 +88,12 @@ pub fn get_default_mountpoint_base_dir() -> PathBuf { /// /// Note that the filename does not carry any intrinsic meaning. /// Here, we simply use the device ID (as it is a UUID) to avoid name collision. -pub fn get_default_key_file(config_dir: &Path, device: &LocalDevice) -> PathBuf { +pub fn get_default_key_file(config_dir: &Path, device_id: &DeviceID) -> PathBuf { let mut device_path = config_dir.to_path_buf(); device_path.push("devices"); - device_path.push(format!("{}.{DEVICE_FILE_EXT}", device.device_id.hex())); + device_path.push(format!("{}.{DEVICE_FILE_EXT}", device_id.hex())); device_path } diff --git a/libparsec/src/invite.rs b/libparsec/src/invite.rs index a1109c97c15..5e4efce5c68 100644 --- a/libparsec/src/invite.rs +++ b/libparsec/src/invite.rs @@ -127,7 +127,7 @@ pub async fn bootstrap_organization( let access = { let key_file = libparsec_platform_device_loader::get_default_key_file( &config.config_dir, - &finalize_ctx.new_local_device, + &finalize_ctx.new_local_device.device_id, ); save_strategy.into_access(key_file) };