Merge pull request #230 from replydev/fix/broken_edit_command

replydev · web-flow · commit 112d40643695 · 2023-05-23T10:35:47.000+02:00
Fix/broken_edit_command
diff --git 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
diff --git 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
@@ -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,23 +137,23 @@ 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")
                         .short('l')
                         .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")
                         .short('a')
                         .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
@@ -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
@@ -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
@@ -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
@@ -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"),
         }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -138,8 +138,11 @@ impl OTPDatabase {`
`138`	`138`	`}`
`139`	`139`
`140`	`140`	`pub fn sort(&mut self) {`
`141`		`- self.elements`
`142`		`- .sort_unstable_by(\|c1, c2\| c1.issuer.cmp(&c2.issuer))`
	`141`	`+ self.elements.sort_unstable_by(\|c1, c2\| {`
	`142`	`+ c1.issuer`
	`143`	`+ .to_ascii_lowercase()`
	`144`	`+ .cmp(&c2.issuer.to_ascii_lowercase())`
	`145`	`+ })`
`143`	`146`	`}`
`144`	`147`	`}`
`145`	`148`
Original file line number	Diff line number	Diff line change
`@@ -5,6 +5,7 @@ use std::fmt::{Display, Formatter};`
`5`	`5`	`pub enum OtpError {`
`6`	`6`	`SecretEncoding(DecodeKind, usize), // Secret encoding error, of given kind at give position`
`7`	`7`	`MissingPin, // Missing Pin for Yandex / MOTP Codes`
	`8`	`+ ShortSecret, // Short secret for Yandex codes`
`8`	`9`	`MissingCounter, // Missing counter for HOTP codes`
`9`	`10`	`InvalidOffset, // Invalid offset`
`10`	`11`	`InvalidDigest, // Invalid digest`
`@@ -20,6 +21,7 @@ impl Display for OtpError {`
`20`	`21`	`OtpError::MissingCounter => f.write_str("Missing counter value"),`
`21`	`22`	`OtpError::InvalidDigest => f.write_str("Invalid digest"),`
`22`	`23`	`OtpError::InvalidOffset => f.write_str("Invalid offset"),`
	`24`	`+ OtpError::ShortSecret => f.write_str("Secret length less than 16 bytes"),`
`23`	`25`	`}`
`24`	`26`	`}`
`25`	`27`	`}`