Skip to content

Commit

Permalink
Add CLI error where multiple devices match the same short dev id
Browse files Browse the repository at this point in the history
  • Loading branch information
FirelightFlagboy committed Aug 21, 2024
1 parent 9349757 commit 9415f35
Show file tree
Hide file tree
Showing 6 changed files with 162 additions and 14 deletions.
2 changes: 1 addition & 1 deletion cli/src/commands/import_recovery_device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 };

Expand Down
17 changes: 10 additions & 7 deletions cli/src/testenv_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
};

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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<LocalDevice>,
author: Arc<LocalDevice>,
initial_profile: UserProfile,
Expand All @@ -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<LocalDevice>,
author: Arc<LocalDevice>,
now: DateTime,
Expand Down
147 changes: 146 additions & 1 deletion cli/tests/integration/device_option.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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("[email protected]", "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<LocalDevice>,
client_config: libparsec::ClientConfig,
dev_id_prefix: &str,
) -> anyhow::Result<Arc<LocalDevice>> {
// 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("[email protected]", "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)
}
4 changes: 2 additions & 2 deletions libparsec/crates/client/src/invite/claimer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
}

Expand Down Expand Up @@ -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,
)
}

Expand Down
4 changes: 2 additions & 2 deletions libparsec/crates/platform_device_loader/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion libparsec/src/invite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
};
Expand Down

0 comments on commit 9415f35

Please sign in to comment.