Skip to content

Commit

Permalink
fix(dre): Do not try to get auth parameters for ic-admin get-* comman…
Browse files Browse the repository at this point in the history
…ds (#508)
  • Loading branch information
sasa-tomic authored Jun 20, 2024
1 parent 48855c1 commit 3087304
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 35 deletions.
53 changes: 34 additions & 19 deletions rs/cli/src/detect_neuron.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,12 @@ impl Neuron {
}
}

pub async fn get_auth(&self) -> anyhow::Result<Auth> {
pub async fn get_auth(&self, allow_auth: bool) -> anyhow::Result<Auth> {
if !allow_auth {
// This is used for the `get-*` commands, which don't accept authentification parameters
return Ok(Auth::None);
}

if let Some(auth) = &*self.auth_cache.borrow() {
return Ok(auth.clone());
};
Expand Down Expand Up @@ -92,27 +97,34 @@ impl Neuron {
if let Some(neuron_id) = *self.neuron_id.borrow() {
return Ok(neuron_id);
};
let neuron_id = auto_detect_neuron_id(self.network.get_nns_urls(), self.get_auth().await?).await?;
let neuron_id = auto_detect_neuron_id(self.network.get_nns_urls(), self.get_auth(true).await?).await?;
self.neuron_id.replace(Some(neuron_id));
Ok(neuron_id)
}

/// Returns the arguments to pass to the ic-admin CLI for this neuron
/// If require_auth is true, it will panic if the auth method could not be detected
/// This is useful to check if the auth detection work correctly even without
/// submitting a proposal.
pub async fn as_arg_vec(&self, require_auth: bool) -> anyhow::Result<Vec<String>> {
// Auth required, try to find valid neuron id using HSM or with the private key
/// Returns the arguments to pass to the ic-admin CLI for this neuron.
/// If require_auth is true, it will panic if the auth method could not be detected.
/// if allow_auth is false, it will return an empty vector in any case.
/// if allow_auth is true, it will try to detect the auth parameters: method and neuron id.
/// Detection of the auth parameters is useful to check if the auth detection works correctly without
/// actually submitting a proposal.
pub async fn as_arg_vec(&self, require_auth: bool, allow_auth: bool) -> anyhow::Result<Vec<String>> {
// If auth may be provided (allow_auth), search for valid neuron id using HSM or with the private key
// `allow_auth` is set to false for ic-admin `get-*` commands, since they don't accept auth
// If private key is provided, use it without checking
let auth = match self.get_auth().await {
Ok(auth) => auth,
Err(e) => {
if require_auth {
return Err(anyhow::anyhow!(e));
} else {
return Ok(vec![]);
let auth = if allow_auth {
match self.get_auth(allow_auth).await {
Ok(auth) => auth,
Err(e) => {
if require_auth {
return Err(anyhow::anyhow!(e));
} else {
return Ok(vec![]);
}
}
}
} else {
Auth::None
};
let neuron_id = match auto_detect_neuron_id(self.network.get_nns_urls(), auth).await {
Ok(neuron_id) => neuron_id,
Expand Down Expand Up @@ -171,10 +183,13 @@ pub fn get_pkcs11_ctx() -> anyhow::Result<Pkcs11> {
impl Auth {
/// Returns the arguments to pass to the ic-admin CLI for the given auth method
/// If require_auth is true, it will panic if the auth method is Auth::None
/// Otherwise, it will return an empty vector if the auth method is Auth::None
/// This is useful to check if the auth detection work correctly even without
/// submitting a proposal
pub fn as_arg_vec(&self, require_auth: bool) -> Vec<String> {
/// If allow_auth is false, it will return an empty vector in any case.
/// If allow_auth is true, it will return the arguments for the set auth method.
/// Checking the auth parameters is useful to validate working auth detection.
pub fn as_arg_vec(&self, require_auth: bool, allow_auth: bool) -> Vec<String> {
if !allow_auth {
return vec![];
}
match self {
Auth::Hsm { pin, slot, key_id } => vec![
"--use-hsm".to_string(),
Expand Down
2 changes: 1 addition & 1 deletion rs/cli/src/general.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ pub async fn vote_on_proposals(
dry_run: bool,
sleep: Duration,
) -> anyhow::Result<()> {
let client: GovernanceCanisterWrapper = match &neuron.get_auth().await? {
let client: GovernanceCanisterWrapper = match &neuron.get_auth(true).await? {
Auth::Hsm { pin, slot, key_id } => CanisterClient::from_hsm(pin.to_string(), *slot, key_id.to_string(), &nns_urls[0])?.into(),
Auth::Keyfile { path } => CanisterClient::from_key_file(path.into(), &nns_urls[0])?.into(),
Auth::None => CanisterClient::from_anonymous(&nns_urls[0])?.into(),
Expand Down
31 changes: 16 additions & 15 deletions rs/cli/src/ic_admin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,8 @@ impl IcAdminWrapper {
}
}

async fn print_ic_admin_command_line(&self, cmd: &Command, require_auth: bool) {
let auth = match self.neuron.get_auth().await {
async fn print_ic_admin_command_line(&self, cmd: &Command, require_auth: bool, allow_auth: bool) {
let auth = match self.neuron.get_auth(allow_auth).await {
Ok(auth) => auth,
Err(e) => {
if require_auth {
Expand Down Expand Up @@ -215,7 +215,7 @@ impl IcAdminWrapper {
);
}

async fn _exec(&self, cmd: ProposeCommand, opts: ProposeOptions, as_simulation: bool) -> anyhow::Result<String> {
async fn _exec(&self, cmd: ProposeCommand, opts: ProposeOptions, as_simulation: bool, allow_auth: bool) -> anyhow::Result<String> {
if let Some(summary) = opts.clone().summary {
let summary_count = summary.chars().count();
if summary_count > MAX_SUMMARY_CHAR_COUNT {
Expand Down Expand Up @@ -246,12 +246,13 @@ impl IcAdminWrapper {
]
})
.unwrap_or_default(),
self.neuron.as_arg_vec(with_auth).await?,
self.neuron.as_arg_vec(with_auth, allow_auth).await?,
cmd.args(),
]
.concat()
.as_slice(),
with_auth,
allow_auth,
false,
)
.await
Expand All @@ -260,34 +261,34 @@ impl IcAdminWrapper {
pub async fn propose_run(&self, cmd: ProposeCommand, opts: ProposeOptions, dry_run: bool) -> anyhow::Result<String> {
// Dry run, or --help executions run immediately and do not proceed.
if dry_run || cmd.args().contains(&String::from("--help")) || cmd.args().contains(&String::from("--dry-run")) {
return self._exec(cmd, opts, true).await;
return self._exec(cmd, opts, true, true).await;
}

// If --yes was not specified, ask the user if they want to proceed
if !self.proceed_without_confirmation {
self._exec(cmd.clone(), opts.clone(), true).await?;
self._exec(cmd.clone(), opts.clone(), true, true).await?;
}

if self.proceed_without_confirmation || Confirm::new().with_prompt("Do you want to continue?").default(false).interact()? {
// User confirmed the desire to submit the proposal and no obvious problems were
// found. Proceeding!
self._exec(cmd, opts, false).await
self._exec(cmd, opts, false, true).await
} else {
Err(anyhow::anyhow!("Action aborted"))
}
}

async fn _run_ic_admin_with_args(&self, ic_admin_args: &[String], with_auth: bool, silent: bool) -> anyhow::Result<String> {
async fn _run_ic_admin_with_args(&self, ic_admin_args: &[String], require_auth: bool, allow_auth: bool, silent: bool) -> anyhow::Result<String> {
let ic_admin_path = self.ic_admin_bin_path.clone().unwrap_or_else(|| "ic-admin".to_string());
let mut cmd = Command::new(ic_admin_path);
let auth_options = self.neuron.get_auth().await?.as_arg_vec(with_auth);
let auth_options = self.neuron.get_auth(allow_auth).await?.as_arg_vec(require_auth, allow_auth);
let root_options = [auth_options, vec!["--nns-urls".to_string(), self.network.get_nns_urls_string()]].concat();
let cmd = cmd.args([&root_options, ic_admin_args].concat());

if silent {
cmd.stderr(Stdio::piped());
} else {
self.print_ic_admin_command_line(cmd, with_auth).await;
self.print_ic_admin_command_line(cmd, require_auth, allow_auth).await;
}
cmd.stdout(Stdio::piped());

Expand Down Expand Up @@ -331,9 +332,9 @@ impl IcAdminWrapper {
}
}

pub async fn run(&self, command: &str, args: &[String], with_auth: bool, silent: bool) -> anyhow::Result<String> {
pub async fn run(&self, command: &str, args: &[String], require_auth: bool, allow_auth: bool, silent: bool) -> anyhow::Result<String> {
let ic_admin_args = [&[command.to_string()], args].concat();
self._run_ic_admin_with_args(&ic_admin_args, with_auth, silent).await
self._run_ic_admin_with_args(&ic_admin_args, require_auth, allow_auth, silent).await
}

/// Run ic-admin and parse sub-commands that it lists with "--help",
Expand Down Expand Up @@ -410,7 +411,7 @@ impl IcAdminWrapper {
};

let stdout = self
.run(&args[0], &args.iter().skip(1).cloned().collect::<Vec<_>>(), false, silent)
.run(&args[0], &args.iter().skip(1).cloned().collect::<Vec<_>>(), false, false, silent)
.await?;
Ok(stdout)
}
Expand Down Expand Up @@ -1138,13 +1139,13 @@ oSMDIQBa2NLmSmaqjDXej4rrJEuEhKIz7/pGXpxztViWhB+X9Q==
]
})
.unwrap_or_default(),
cli.neuron.get_auth().await?.as_arg_vec(true),
cli.neuron.get_auth(true).await?.as_arg_vec(true, true),
cmd.args(),
]
.concat()
.to_vec();
let out = with_ic_admin(Default::default(), async {
cli.run(&cmd.get_command_name(), &vector, true, false)
cli.run(&cmd.get_command_name(), &vector, true, true, false)
.await
.map_err(|e| anyhow::anyhow!(e))
})
Expand Down

0 comments on commit 3087304

Please sign in to comment.