Repository: replydev/cotp (language: rust)

Issue:
Error editing entry
Thank you very much for this app, I find it very convenient and use it a lot.

Now to the problem, when I try to edit existing entry I receive following error:
```bash
[0] $ cotp edit -i 8 -l withdraw
Password: 
thread 'main' panicked at 'Mismatch between definition and access of `index`. Could not downcast to TypeId { t: 13834754221672687376 }, need to downcast to TypeId { t: 8562976148318468642 }
', $HOME/.cargo/registry/src/github.com-1ecc6299db9ec823/clap-4.1.1/src/parser/error.rs:30:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Aborted
```

Here is the same thing with RUST_BACKTRACE=full
```bash
[0] $ RUST_BACKTRACE=full cotp edit -i 8 -l withdraw
Password: 
thread 'main' panicked at 'Mismatch between definition and access of `index`. Could not downcast to TypeId { t: 13834754221672687376 }, need to downcast to TypeId { t: 8562976148318468642 }
', /home/grbit/.cargo/registry/src/github.com-1ecc6299db9ec823/clap-4.1.1/src/parser/error.rs:30:9
stack backtrace:
   0:     0x5594bcd2e469 - <unknown>
   1:     0x5594bcccebbe - <unknown>
   2:     0x5594bcd06644 - <unknown>
   3:     0x5594bcd2f4ff - <unknown>
   4:     0x5594bcd2f0ff - <unknown>
   5:     0x5594bcd30168 - <unknown>
   6:     0x5594bcd2fc52 - <unknown>
   7:     0x5594bcd2fbbc - <unknown>
   8:     0x5594bcd2fb91 - <unknown>
   9:     0x5594bcc5c3f2 - <unknown>
  10:     0x5594bcc94bd4 - <unknown>
  11:     0x5594bcc65f55 - <unknown>
  12:     0x5594bcc9a8c7 - <unknown>
  13:     0x7fb8ad269d0a - __libc_start_main
  14:     0x5594bcc61bfa - <unknown>
  15:                0x0 - <unknown>
Aborted
```

Maybe I'm doing something in a wrong way, then please explain me how to edit entries in my cotp database.


Discussion / hints:
Thank you for reporting! I can replicate and fix will come very soon

Fix patch:
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index ecb9b9d9..8d7cfb49 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -38,3 +38,11 @@ jobs:
 
       - name: Run cargo test
         run: cargo test
+
+      - name: Run cargo build
+        run: cargo build
+
+      - uses: actions/upload-artifact@v3.1.2
+        with:
+          name: bins-${{ matrix.os }}
+          path: target/debug/cotp*
diff --git a/Cargo.lock b/Cargo.lock
index 4574144d..e58c3268 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -318,7 +318,7 @@ dependencies = [
 
 [[package]]
 name = "cotp"
-version = "1.2.4"
+version = "1.2.5"
 dependencies = [
  "aes-gcm",
  "base64 0.21.1",
diff --git a/Cargo.toml b/Cargo.toml
index 8518e76c..e1fc9d1f 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "cotp"
-version = "1.2.4"
+version = "1.2.5"
 authors = ["replydev <commoncargo@tutanota.com>"]
 edition = "2021"
 description = "Trustworthy, encrypted, command-line TOTP/HOTP authenticator app with import functionality."
diff --git a/src/args.rs b/src/args.rs
index 348cc530..90b0250f 100644
--- a/src/args.rs
+++ b/src/args.rs
@@ -128,6 +128,7 @@ pub fn get_matches() -> ArgMatches {
                         .long("index")
                         .help("OTP Code index")
                         .num_args(1)
+                        .value_parser(value_parser!(usize))
                         .required(true),
                 )
                 .arg(
@@ -136,7 +137,7 @@ pub fn get_matches() -> ArgMatches {
                         .long("issuer")
                         .help("OTP Code issuer")
                         .num_args(1)
-                        .required_unless_present_any(["label", "algorithm", "digits", "counter"]),
+                        .required_unless_present_any(["label", "algorithm", "digits", "counter", "pin", "change-secret"]),
                 )
                 .arg(
                     Arg::new("label")
@@ -144,7 +145,7 @@ pub fn get_matches() -> ArgMatches {
                         .long("label")
                         .help("OTP Code label")
                         .num_args(1)
-                        .required_unless_present_any(["issuer", "algorithm", "digits", "counter"]),
+                        .required_unless_present_any(["issuer", "algorithm", "digits", "counter", "pin", "change-secret"]),
                 )
                 .arg(
                     Arg::new("algorithm")
@@ -152,7 +153,7 @@ pub fn get_matches() -> ArgMatches {
                         .long("algorithm")
                         .help("OTP Code algorithm")
                         .num_args(1)
-                        .required_unless_present_any(["label", "issuer", "digits", "counter"])
+                        .required_unless_present_any(["label", "issuer", "digits", "counter", "pin", "change-secret"])
                         .value_parser(["SHA1", "SHA256", "SHA512"]),
                 )
                 .arg(
@@ -162,7 +163,7 @@ pub fn get_matches() -> ArgMatches {
                         .help("OTP Code digits")
                         .num_args(1)
                         .value_parser(value_parser!(u64))
-                        .required_unless_present_any(["label", "algorithm", "issuer", "counter"]),
+                        .required_unless_present_any(["label", "algorithm", "issuer", "counter", "pin", "change-secret"]),
                 )
                 .arg(
                     Arg::new("period")
@@ -171,7 +172,7 @@ pub fn get_matches() -> ArgMatches {
                         .help("OTP Code period")
                         .num_args(1)
                         .value_parser(value_parser!(u64))
-                        .required_unless_present_any(["label", "algorithm", "issuer", "counter"]),
+                        .required_unless_present_any(["label", "algorithm", "issuer", "counter", "pin", "change-secret"]),
                 )
                 .arg(
                     Arg::new("counter")
@@ -180,14 +181,23 @@ pub fn get_matches() -> ArgMatches {
                         .help("HOTP code counter (only for HOTP codes)")
                         .num_args(1)
                         .value_parser(value_parser!(u64))
-                        .required_unless_present_any(["label", "algorithm", "issuer", "digits"]),
+                        .required_unless_present_any(["label", "algorithm", "issuer", "digits", "pin", "change-secret"]),
+                )
+                .arg(
+                    Arg::new("pin")
+                        .short('p')
+                        .long("pin")
+                        .help("Code pin (for Yandex and MOTP)")
+                        .num_args(1)
+                        .required_unless_present_any(["label", "algorithm", "issuer", "digits", "counter", "change-secret"]),
                 )
                 .arg(
                     Arg::new("change-secret")
                         .short('k')
                         .long("change-secret")
                         .help("Change the OTP code secret")
-                        .action(ArgAction::SetTrue),
+                        .action(ArgAction::SetTrue)
+                        .required_unless_present_any(["label", "algorithm", "issuer", "digits", "counter", "pin"]),
                 ),
         )
         .subcommand(
diff --git a/src/argument_functions.rs b/src/argument_functions.rs
index 89847669..f9620b9b 100644
--- a/src/argument_functions.rs
+++ b/src/argument_functions.rs
@@ -99,44 +99,59 @@ pub fn edit(matches: &ArgMatches, database: &mut OTPDatabase) -> Result<String,
     };
 
     let index = *matches.get_one::<usize>("index").unwrap();
-    let otp_element: Option<&OTPElement> = database.get_element(index);
 
-    let issuer = matches.get_one::<String>("issuer").cloned();
-    let label = matches.get_one::<String>("label").cloned();
-    let digits = matches.get_one::<u64>("usize").copied();
-    let period = matches.get_one::<u64>("period").copied();
-    let counter = matches.get_one::<u64>("counter").copied();
-    let pin = matches.get_one::<String>("label").cloned();
-
-    match otp_element {
-        Some(v) => {
-            let mut element = v.clone();
+    if let Some(real_index) = index
+        // User provides row number from dashboard which is equal to the array index plus one
+        .checked_sub(1)
+    {
+        if real_index >= database.elements_ref().len() {
+            return Err(format!("{index} is an invalid index"));
+        }
 
-            if let Some(v) = issuer {
-                element.issuer = v;
-            }
-            if let Some(v) = label {
-                element.label = v;
-            }
-            if let Some(v) = digits {
-                element.digits = v;
-            }
-            if let Some(v) = period {
-                element.period = v;
+        let otp_element: Option<&OTPElement> = database.get_element(real_index);
+
+        let issuer = matches.get_one::<String>("issuer").cloned();
+        let label = matches.get_one::<String>("label").cloned();
+        let digits = matches.get_one::<u64>("digits").copied();
+        let period = matches.get_one::<u64>("period").copied();
+        let counter = matches.get_one::<u64>("counter").copied();
+        let pin = matches.get_one::<String>("pin").cloned();
+
+        match otp_element {
+            Some(v) => {
+                let mut element = v.clone();
+
+                if let Some(v) = issuer {
+                    element.issuer = v;
+                }
+                if let Some(v) = label {
+                    element.label = v;
+                }
+                if let Some(v) = digits {
+                    element.digits = v;
+                }
+                if let Some(v) = period {
+                    element.period = v;
+                }
+                if counter.is_some() {
+                    element.counter = counter;
+                }
+                if pin.is_some() {
+                    element.pin = pin;
+                }
+                if secret.is_some() {
+                    element.secret = secret.clone().unwrap();
+                }
+                database.edit_element(real_index, element);
             }
-            if counter.is_some() {
-                element.counter = counter;
-            }
-            if pin.is_some() {
-                element.pin = pin;
-            }
-            database.edit_element(index, element);
+            None => return Err(format!("No element found at index {index}")),
         }
-        None => return Err(format!("No element found at index {index}")),
-    }
 
-    secret.zeroize();
-    Ok(String::from("Success."))
+        secret.zeroize();
+        Ok(String::from("Success."))
+    } else {
+        Err(format! {"{index} is an invalid index"})
+    }
 }
 
 pub fn export(matches: &ArgMatches, database: &mut OTPDatabase) -> Result<String, String> {
diff --git a/src/otp/algorithms/yandex_otp_maker.rs b/src/otp/algorithms/yandex_otp_maker.rs
index 298e8eec..2ac7a51a 100644
--- a/src/otp/algorithms/yandex_otp_maker.rs
+++ b/src/otp/algorithms/yandex_otp_maker.rs
@@ -70,6 +70,10 @@ where
         Err(e) => return Err(OtpError::SecretEncoding(e.kind, e.position)),
     };
 
+    if decoded_secret.len() < SECRET_LENGHT {
+        return Err(OtpError::ShortSecret);
+    }
+
     let parsed_secret = &decoded_secret.as_slice()[0..SECRET_LENGHT];
 
     let mut pin_with_secret: Vec<u8> = Vec::with_capacity(pin.as_bytes().len() + SECRET_LENGHT);
diff --git a/src/otp/otp_element.rs b/src/otp/otp_element.rs
index faa0608e..b601401f 100644
--- a/src/otp/otp_element.rs
+++ b/src/otp/otp_element.rs
@@ -138,8 +138,11 @@ impl OTPDatabase {
     }
 
     pub fn sort(&mut self) {
-        self.elements
-            .sort_unstable_by(|c1, c2| c1.issuer.cmp(&c2.issuer))
+        self.elements.sort_unstable_by(|c1, c2| {
+            c1.issuer
+                .to_ascii_lowercase()
+                .cmp(&c2.issuer.to_ascii_lowercase())
+        })
     }
 }
 
diff --git a/src/otp/otp_error.rs b/src/otp/otp_error.rs
index 7d855430..269304be 100644
--- a/src/otp/otp_error.rs
+++ b/src/otp/otp_error.rs
@@ -5,6 +5,7 @@ use std::fmt::{Display, Formatter};
 pub enum OtpError {
     SecretEncoding(DecodeKind, usize), // Secret encoding error, of given kind at give position
     MissingPin,                        // Missing Pin for Yandex / MOTP Codes
+    ShortSecret,                       // Short secret for Yandex codes
     MissingCounter,                    // Missing counter for HOTP codes
     InvalidOffset,                     // Invalid offset
     InvalidDigest,                     // Invalid digest
@@ -20,6 +21,7 @@ impl Display for OtpError {
             OtpError::MissingCounter => f.write_str("Missing counter value"),
             OtpError::InvalidDigest => f.write_str("Invalid digest"),
             OtpError::InvalidOffset => f.write_str("Invalid offset"),
+            OtpError::ShortSecret => f.write_str("Secret length less than 16 bytes"),
         }
     }
 }


Test patch: