Remove pws set

CHANGELOG.md

@@ -6,6 +6,7 @@ Unreleased
 - Changed error reporting format to make up only a single line
 - Added the `pws add` subcommand to write to a new slot
 - Added the `pws update` subcommand to update an existing PWS slot
+- Removed the `pws set` subcommand – use `pws add` or `pws update` instead
 - Added the `--only-aes-key` option to the `reset` command to build a new AES
   key without performing a factory reset
 - Added `NITROCLI_RESOLVED_USB_PATH` environment variable to be used by

doc/nitrocli.1

@@ -1,4 +1,4 @@
-.TH NITROCLI 1 2021-04-18
+.TH NITROCLI 1 2021-04-20
 .SH NAME
 nitrocli \- access Nitrokey devices
 .SH SYNOPSIS
@@ -273,12 +273,6 @@ The fields are printed together with a label.
 Use the \fB\-\-quiet\fR option to suppress the labels and to only output the
 values stored in the PWS slot.
 .TP
-\fBnitrocli pws set \fIslot name login password\fR
-Set the content of a PWS slot.
-\fIslot\fR is the number of the slot to write.
-\fIname\fR, \fIlogin\fR, and \fIpassword\fR represent the data to write to the
-slot.
-.TP
 \fBnitrocli pws add \fR[\fB\-s\fR|\fB\-\-slot \fIslot\fR] \
 \fIname login password\fR
 Add a new PWS slot.
@@ -540,7 +534,8 @@ Change the configuration:
 
 .SS Password safe
 Configure a PWS slot:
-    $ \fBnitrocli pws set 0 example.org john.doe passw0rd\fR
+    $ \fBnitrocli pws add example.org john.doe passw0rd\fR
+    Added PWS slot 0
 
 Get the data from a slot:
     $ \fBnitrocli pws get 0\fR

src/args.rs

@@ -394,10 +394,6 @@ Command! {PwsCommand, [
   Get(PwsGetArgs) => |ctx, args: PwsGetArgs| {
     crate::commands::pws_get(ctx, args.slot, args.name, args.login, args.password, args.quiet)
   },
-  /// Writes a password safe slot
-  Set(PwsSetArgs) => |ctx, args: PwsSetArgs| {
-    crate::commands::pws_set(ctx, args.slot, &args.name, &args.login, &args.password)
-  },
   /// Adds a new password safe slot
   Add(PwsAddArgs) => |ctx, args: PwsAddArgs| {
     crate::commands::pws_add(ctx, &args.name, &args.login, &args.password, args.slot)
@@ -440,18 +436,6 @@ pub struct PwsGetArgs {
   pub slot: u8,
 }
 
-#[derive(Debug, PartialEq, structopt::StructOpt)]
-pub struct PwsSetArgs {
-  /// The PWS slot to write
-  pub slot: u8,
-  /// The name to store on the slot
-  pub name: String,
-  /// The login to store on the slot
-  pub login: String,
-  /// The password to store on the slot
-  pub password: String,
-}
-
 #[derive(Debug, PartialEq, structopt::StructOpt)]
 pub struct PwsAddArgs {
   /// The name to store on the slot

src/commands.rs

@@ -1032,21 +1032,6 @@ pub fn pws_get(
   })
 }
 
-/// Write a PWS slot.
-pub fn pws_set(
-  ctx: &mut Context<'_>,
-  slot: u8,
-  name: &str,
-  login: &str,
-  password: &str,
-) -> anyhow::Result<()> {
-  with_password_safe(ctx, |_ctx, mut pws| {
-    pws
-      .write_slot(slot, name, login, password)
-      .context("Failed to write PWS slot")
-  })
-}
-
 /// Add a new PWS slot.
 pub fn pws_add(
   ctx: &mut Context<'_>,

src/tests/pws.rs

@@ -5,6 +5,24 @@
 
 use super::*;
 
+fn clear_pws(model: nitrokey::Model) -> anyhow::Result<()> {
+  use nitrokey::GetPasswordSafe as _;
+
+  let mut manager = nitrokey::force_take()?;
+  let mut device = manager.connect_model(model)?;
+  let mut pws = device.get_password_safe(nitrokey::DEFAULT_USER_PIN)?;
+  let slots_to_clear: Vec<_> = pws
+    .get_slots()?
+    .into_iter()
+    .flatten()
+    .map(|s| s.index())
+    .collect();
+  for slot in slots_to_clear {
+    pws.erase_slot(slot)?;
+  }
+  Ok(())
+}
+
 fn assert_slot(
   model: nitrokey::Model,
   slot: u8,
@@ -18,17 +36,6 @@ fn assert_slot(
   Ok(())
 }
 
-#[test_device]
-fn set_invalid_slot(model: nitrokey::Model) {
-  let err = Nitrocli::new()
-    .model(model)
-    .handle(&["pws", "set", "100", "name", "login", "1234"])
-    .unwrap_err()
-    .to_string();
-
-  assert_eq!(err, "Failed to write PWS slot");
-}
-
 #[test_device]
 fn add_invalid_slot(model: nitrokey::Model) {
   let err = Nitrocli::new()
@@ -48,24 +55,28 @@ fn status(model: nitrokey::Model) -> anyhow::Result<()> {
   )
   .unwrap();
 
+  clear_pws(model)?;
+
   let mut ncli = Nitrocli::new().model(model);
   // Make sure that we have at least something to display by ensuring
   // that there are there is one slot programmed.
-  let _ = ncli.handle(&["pws", "set", "0", "the-name", "the-login", "123456"])?;
+  let _ = ncli.handle(&["pws", "add", "the-name", "the-login", "123456"])?;
 
   let out = ncli.handle(&["pws", "status"])?;
   assert!(re.is_match(&out), "{}", out);
   Ok(())
 }
 
 #[test_device]
-fn set_get(model: nitrokey::Model) -> anyhow::Result<()> {
+fn add_get(model: nitrokey::Model) -> anyhow::Result<()> {
   const NAME: &str = "dropbox";
   const LOGIN: &str = "d-e-s-o";
   const PASSWORD: &str = "my-secret-password";
 
+  clear_pws(model)?;
+
   let mut ncli = Nitrocli::new().model(model);
-  let _ = ncli.handle(&["pws", "set", "1", &NAME, &LOGIN, &PASSWORD])?;
+  let _ = ncli.handle(&["pws", "add", "--slot", "1", &NAME, &LOGIN, &PASSWORD])?;
 
   let out = ncli.handle(&["pws", "get", "1", "--quiet", "--name"])?;
   assert_eq!(out, format!("{}\n", NAME));
@@ -90,9 +101,12 @@ fn set_get(model: nitrokey::Model) -> anyhow::Result<()> {
 }
 
 #[test_device]
-fn set_empty(model: nitrokey::Model) -> anyhow::Result<()> {
+fn add_empty(model: nitrokey::Model) -> anyhow::Result<()> {
   let mut ncli = Nitrocli::new().model(model);
-  let _ = ncli.handle(&["pws", "set", "1", "", "", ""])?;
+
+  clear_pws(model)?;
+
+  let _ = ncli.handle(&["pws", "add", "--slot", "1", "", "", ""])?;
 
   let out = ncli.handle(&["pws", "get", "1", "--quiet", "--name"])?;
   assert_eq!(out, "\n");
@@ -111,13 +125,15 @@ fn set_empty(model: nitrokey::Model) -> anyhow::Result<()> {
 }
 
 #[test_device]
-fn set_reset_get(model: nitrokey::Model) -> anyhow::Result<()> {
+fn add_reset_get(model: nitrokey::Model) -> anyhow::Result<()> {
   const NAME: &str = "some/svc";
   const LOGIN: &str = "a\\user";
   const PASSWORD: &str = "!@&-)*(&+%^@";
 
+  clear_pws(model)?;
+
   let mut ncli = Nitrocli::new().model(model);
-  let _ = ncli.handle(&["pws", "set", "2", &NAME, &LOGIN, &PASSWORD])?;
+  let _ = ncli.handle(&["pws", "add", "--slot", "2", &NAME, &LOGIN, &PASSWORD])?;
 
   let out = ncli.handle(&["reset"])?;
   assert_eq!(out, "");
@@ -130,8 +146,11 @@ fn set_reset_get(model: nitrokey::Model) -> anyhow::Result<()> {
 
 #[test_device]
 fn clear(model: nitrokey::Model) -> anyhow::Result<()> {
+  clear_pws(model)?;
+
   let mut ncli = Nitrocli::new().model(model);
-  let _ = ncli.handle(&["pws", "set", "10", "clear-test", "some-login", "abcdef"])?;
+  let _ = ncli.handle(&["pws", "clear", "10"])?;
+  let _ = ncli.handle(&["pws", "add", "--slot", "10", "clear-test", "some-login", "abcdef"])?;
   let _ = ncli.handle(&["pws", "clear", "10"])?;
   let res = ncli.handle(&["pws", "get", "10"]);
 
@@ -142,9 +161,9 @@ fn clear(model: nitrokey::Model) -> anyhow::Result<()> {
 
 #[test_device]
 fn update_unprogrammed(model: nitrokey::Model) -> anyhow::Result<()> {
+  clear_pws(model)?;
+
   let mut ncli = Nitrocli::new().model(model);
-  let _ = ncli.handle(&["pws", "set", "10", "clear-test", "some-login", "abcdef"])?;
-  let _ = ncli.handle(&["pws", "clear", "10"])?;
   let res = ncli.handle(&["pws", "update", "10", "--name", "test"]);
 
   let err = res.unwrap_err().to_string();
@@ -174,10 +193,13 @@ fn update(model: nitrokey::Model) -> anyhow::Result<()> {
   const PASSWORD_BEFORE: &str = "password-before";
   const PASSWORD_AFTER: &str = "password-after";
 
+  clear_pws(model)?;
+
   let mut ncli = Nitrocli::new().model(model);
   let _ = ncli.handle(&[
     "pws",
-    "set",
+    "add",
+    "--slot",
     "10",
     NAME_BEFORE,
     LOGIN_BEFORE,
@@ -215,18 +237,17 @@ fn update(model: nitrokey::Model) -> anyhow::Result<()> {
 fn add_full(model: nitrokey::Model) -> anyhow::Result<()> {
   let mut ncli = Nitrocli::new().model(model);
 
+  clear_pws(model)?;
+
   // Fill all PWS slots
-  for slot in u8::MIN..=u8::MAX {
-    let res = ncli.handle(&["pws", "set", &slot.to_string(), "name", "login", "passw0rd"]);
-    match res {
-      Ok(_) => {}
-      Err(err) => match err.downcast::<nitrokey::Error>() {
-        Ok(err) => match err {
-          nitrokey::Error::LibraryError(nitrokey::LibraryError::InvalidSlot) => break,
-          err => anyhow::bail!(err),
-        },
-        Err(err) => anyhow::bail!(err),
-      },
+  {
+    use nitrokey::GetPasswordSafe as _;
+
+    let mut manager = nitrokey::force_take()?;
+    let mut device = manager.connect_model(model)?;
+    let mut pws = device.get_password_safe(nitrokey::DEFAULT_USER_PIN)?;
+    for slot in 0..pws.get_slot_count() {
+      pws.write_slot(slot, "name", "login", "passw0rd")?;
     }
   }
 
@@ -240,10 +261,12 @@ fn add_full(model: nitrokey::Model) -> anyhow::Result<()> {
 
 #[test_device]
 fn add_existing(model: nitrokey::Model) -> anyhow::Result<()> {
+  clear_pws(model)?;
+
   let mut ncli = Nitrocli::new().model(model);
 
   // Fill slot 0
-  let _ = ncli.handle(&["pws", "set", "0", "name0", "login0", "pass0rd"])?;
+  let _ = ncli.handle(&["pws", "add", "--slot", "0", "name0", "login0", "pass0rd"])?;
 
   // Try to add slot 0
   let res = ncli.handle(&["pws", "add", "--slot", "0", "name", "login", "passw0rd"]);
@@ -255,14 +278,13 @@ fn add_existing(model: nitrokey::Model) -> anyhow::Result<()> {
 
 #[test_device]
 fn add_slot(model: nitrokey::Model) -> anyhow::Result<()> {
+  clear_pws(model)?;
+
   let mut ncli = Nitrocli::new().model(model);
 
   // Fill slots 0 and 5
-  let _ = ncli.handle(&["pws", "set", "0", "name0", "login0", "passw0rd"])?;
-  let _ = ncli.handle(&["pws", "set", "5", "name5", "login5", "passw5rd"])?;
-
-  // Clear slot 1 (in case it was written to by other slots)
-  let _ = ncli.handle(&["pws", "clear", "1"])?;
+  let _ = ncli.handle(&["pws", "add", "--slot", "0", "name0", "login0", "passw0rd"])?;
+  let _ = ncli.handle(&["pws", "add", "--slot", "5", "name5", "login5", "passw5rd"])?;
 
   // Try to add slot 1
   let out = ncli.handle(&["pws", "add", "--slot", "1", "name1", "login1", "passw1rd"])?;
@@ -277,26 +299,13 @@ fn add_slot(model: nitrokey::Model) -> anyhow::Result<()> {
 
 #[test_device]
 fn add(model: nitrokey::Model) -> anyhow::Result<()> {
-  let mut ncli = Nitrocli::new().model(model);
+  clear_pws(model)?;
 
-  // Clear all PWS slots
-  for slot in u8::MIN..=u8::MAX {
-    let res = ncli.handle(&["pws", "clear", &slot.to_string()]);
-    match res {
-      Ok(_) => {}
-      Err(err) => match err.downcast::<nitrokey::Error>() {
-        Ok(err) => match err {
-          nitrokey::Error::LibraryError(nitrokey::LibraryError::InvalidSlot) => break,
-          err => anyhow::bail!(err),
-        },
-        Err(err) => anyhow::bail!(err),
-      },
-    }
-  }
+  let mut ncli = Nitrocli::new().model(model);
 
   // Fill slots 0 and 5
-  let _ = ncli.handle(&["pws", "set", "0", "name0", "login0", "pass0rd"])?;
-  let _ = ncli.handle(&["pws", "set", "5", "name5", "login5", "pass5rd"])?;
+  let _ = ncli.handle(&["pws", "add", "--slot", "0", "name0", "login0", "pass0rd"])?;
+  let _ = ncli.handle(&["pws", "add", "--slot", "5", "name5", "login5", "pass5rd"])?;
 
   // Try to add another one
   let out = ncli.handle(&["pws", "add", "name1", "login1", "passw1rd"])?;

src/tests/run.rs

@@ -75,7 +75,8 @@ fn help_options() {
   test(&["pws"]);
   test(&["pws", "clear"]);
   test(&["pws", "get"]);
-  test(&["pws", "set"]);
+  test(&["pws", "add"]);
+  test(&["pws", "update"]);
   test(&["pws", "status"]);
   test(&["reset"]);
   test(&["status"]);