Don't prompt for password if a ssh key is set for that host (#186)

* feat: don't ask password if a ssh key is set for that host

* fix: User from ssh config was not used as it should

* fix: forgot a file

CHANGELOG.md

@@ -42,6 +42,8 @@ Released on ??
 - [Issue 153](https://github.com/veeso/termscp/issues/153): show a loading message when loading directory's content
 - [Issue 176](https://github.com/veeso/termscp/issues/176): debug log is now written to CACHE_DIR
 - [Issue 173](https://github.com/veeso/termscp/issues/173): allow unknown fields in ssh2 configuration file
+- [Issue 175](https://github.com/veeso/termscp/issues/175): don't prompt for password if a ssh key is set for that host
+- Fixed an issue that didn't use the `User` specified in ssh2-config
 
 ## 0.11.3
 

src/activity_manager.rs

@@ -7,11 +7,14 @@
 use std::path::{Path, PathBuf};
 use std::time::Duration;
 
-use crate::filetransfer::FileTransferParams;
+use remotefs_ssh::SshKeyStorage as SshKeyStorageTrait;
+
+use crate::filetransfer::{FileTransferParams, FileTransferProtocol};
 use crate::host::{HostError, Localhost};
 use crate::system::bookmarks_client::BookmarksClient;
 use crate::system::config_client::ConfigClient;
 use crate::system::environment;
+use crate::system::sshkey_storage::SshKeyStorage;
 use crate::system::theme_provider::ThemeProvider;
 use crate::ui::activities::auth::AuthActivity;
 use crate::ui::activities::filetransfer::FileTransferActivity;
@@ -71,24 +74,59 @@ impl ActivityManager {
         if params.password_missing() {
             if let Some(password) = password {
                 params.set_default_secret(password.to_string());
+            } else if matches!(
+                params.protocol,
+                FileTransferProtocol::Scp | FileTransferProtocol::Sftp,
+            ) && params.params.generic_params().is_some()
+            {
+                // * if protocol is SCP or SFTP check whether a SSH key is registered for this remote, in case not ask password
+                let storage = SshKeyStorage::from(self.context.as_ref().unwrap().config());
+                let generic_params = params.params.generic_params().unwrap();
+                if storage
+                    .resolve(
+                        &generic_params.address,
+                        &generic_params
+                            .username
+                            .clone()
+                            .unwrap_or(whoami::username()),
+                    )
+                    .is_none()
+                {
+                    debug!(
+                        "storage could not find any suitable key for {}... prompting for password",
+                        generic_params.address
+                    );
+                    self.prompt_password(&mut params)?;
                 } else {
+                    debug!(
+                        "a key is already set for {}; password is not required",
+                        generic_params.address
+                    );
+                }
+            } else {
+                self.prompt_password(&mut params)?;
+            }
+        }
+        // Put params into the context
+        self.context.as_mut().unwrap().set_ftparams(params);
+        Ok(())
+    }
+
+    /// Prompt user for password to set into params.
+    fn prompt_password(&self, params: &mut FileTransferParams) -> Result<(), String> {
         match tty::read_secret_from_tty("Password: ") {
-                    Err(err) => return Err(format!("Could not read password: {err}")),
+            Err(err) => Err(format!("Could not read password: {err}")),
             Ok(Some(secret)) => {
                 debug!(
                     "Read password from tty: {}",
                     fmt::shadow_password(secret.as_str())
                 );
                 params.set_default_secret(secret);
-                    }
-                    Ok(None) => {}
-                }
-            }
-        }
-        // Put params into the context
-        self.context.as_mut().unwrap().set_ftparams(params);
                 Ok(())
             }
+            Ok(None) => Ok(()),
+        }
+    }
 
     /// Resolve provided bookmark name and set it as file transfer params.
     /// Returns error if bookmark is not found

src/filetransfer/builder.rs

@@ -20,6 +20,7 @@ use super::params::{AwsS3Params, GenericProtocolParams, SmbParams};
 use super::{FileTransferProtocol, ProtocolParams};
 use crate::system::config_client::ConfigClient;
 use crate::system::sshkey_storage::SshKeyStorage;
+use crate::utils::ssh as ssh_utils;
 
 /// Remotefs builder
 pub struct Builder;
@@ -155,11 +156,30 @@ impl Builder {
 
     /// Build ssh options from generic protocol params and client configuration
     fn build_ssh_opts(params: GenericProtocolParams, config_client: &ConfigClient) -> SshOpts {
-        let mut opts = SshOpts::new(params.address)
+        let mut opts = SshOpts::new(params.address.clone())
             .key_storage(Box::new(Self::make_ssh_storage(config_client)))
             .port(params.port);
+        //*  get username. Case 1 provided in params
         if let Some(username) = params.username {
             opts = opts.username(username);
+        } else if let Some(ssh_config) = config_client.get_ssh_config().and_then(|x| {
+            //* case 2: found in ssh2 config
+            debug!("reading ssh config at {}", x);
+            ssh_utils::parse_ssh2_config(x).ok()
+        }) {
+            debug!("no username was provided, checking whether a user is set for this host");
+            if let Some(username) = ssh_config.query(&params.address).user {
+                debug!("found username from config: {username}");
+                opts = opts.username(username);
+            } else {
+                //* case 3: use system username; can't be None
+                debug!("no username was provided, using current username");
+                opts = opts.username(whoami::username());
+            }
+        } else {
+            //* case 3: use system username; can't be None
+            debug!("no username was provided, using current username");
+            opts = opts.username(whoami::username());
         }
         if let Some(password) = params.password {
             opts = opts.password(password);

src/filetransfer/params.rs

@@ -107,7 +107,6 @@ impl Default for ProtocolParams {
 }
 
 impl ProtocolParams {
-    #[cfg(test)]
     /// Retrieve generic parameters from protocol params if any
     pub fn generic_params(&self) -> Option<&GenericProtocolParams> {
         match self {

src/system/sshkey_storage.rs

@@ -7,10 +7,11 @@ use std::collections::HashMap;
 use std::path::{Path, PathBuf};
 
 // Ext
-use remotefs_ssh::{SshConfigParseRule, SshKeyStorage as SshKeyStorageTrait};
+use remotefs_ssh::SshKeyStorage as SshKeyStorageTrait;
 use ssh2_config::SshConfig;
 
 use super::config_client::ConfigClient;
+use crate::utils::ssh as ssh_utils;
 
 #[derive(Default)]
 pub struct SshKeyStorage {
@@ -34,19 +35,6 @@ impl SshKeyStorage {
         self.hosts.insert(key, p);
     }
 
-    /// Parse ssh2 config
-    fn parse_ssh2_config(path: &str) -> Result<SshConfig, String> {
-        use std::fs::File;
-        use std::io::BufReader;
-
-        let mut reader = File::open(path)
-            .map_err(|e| format!("failed to open {path}: {e}"))
-            .map(BufReader::new)?;
-        SshConfig::default()
-            .parse(&mut reader, SshConfigParseRule::ALLOW_UNKNOWN_FIELDS)
-            .map_err(|e| format!("Failed to parse ssh2 config: {e}"))
-    }
-
     /// Resolve host via termscp ssh keys storage
     fn resolve_host_in_termscp_storage(&self, host: &str, username: &str) -> Option<&Path> {
         let key: String = Self::make_mapkey(host, username);
@@ -89,7 +77,7 @@ impl From<&ConfigClient> for SshKeyStorage {
         // read ssh2 config
         let ssh_config = cfg_client.get_ssh_config().and_then(|x| {
             debug!("reading ssh config at {}", x);
-            Self::parse_ssh2_config(x).ok()
+            ssh_utils::parse_ssh2_config(x).ok()
         });
         let mut hosts: HashMap<String, PathBuf> =
             HashMap::with_capacity(cfg_client.iter_ssh_keys().count());

src/ui/activities/filetransfer/view.rs

@@ -153,7 +153,16 @@ impl FileTransferActivity {
             self.app.view(&Id::StatusBarLocal, f, status_bar_chunks[0]);
             self.app.view(&Id::StatusBarRemote, f, status_bar_chunks[1]);
             // @! Draw popups
-            if self.app.mounted(&Id::CopyPopup) {
+            if self.app.mounted(&Id::FatalPopup) {
+                let popup = Popup(
+                    Size::Percentage(50),
+                    self.calc_popup_height(Id::FatalPopup, f.size().width, f.size().height),
+                )
+                .draw_in(f.size());
+                f.render_widget(Clear, popup);
+                // make popup
+                self.app.view(&Id::FatalPopup, f, popup);
+            } else if self.app.mounted(&Id::CopyPopup) {
                 let popup = Popup(Size::Percentage(40), Size::Unit(3)).draw_in(f.size());
                 f.render_widget(Clear, popup);
                 // make popup
@@ -292,15 +301,6 @@ impl FileTransferActivity {
                 f.render_widget(Clear, popup);
                 // make popup
                 self.app.view(&Id::ErrorPopup, f, popup);
-            } else if self.app.mounted(&Id::FatalPopup) {
-                let popup = Popup(
-                    Size::Percentage(50),
-                    self.calc_popup_height(Id::FatalPopup, f.size().width, f.size().height),
-                )
-                .draw_in(f.size());
-                f.render_widget(Clear, popup);
-                // make popup
-                self.app.view(&Id::FatalPopup, f, popup);
             } else if self.app.mounted(&Id::WaitPopup) {
                 let popup = Popup(Size::Percentage(50), Size::Unit(3)).draw_in(f.size());
                 f.render_widget(Clear, popup);
@@ -360,6 +360,7 @@ impl FileTransferActivity {
     }
 
     pub(super) fn mount_fatal<S: AsRef<str>>(&mut self, text: S) {
+        self.umount_wait();
         // Mount
         let error_color = self.theme().misc_error_dialog;
         assert!(self

src/utils/mod.rs

@@ -9,6 +9,7 @@ pub mod fmt;
 pub mod parser;
 pub mod path;
 pub mod random;
+pub mod ssh;
 pub mod string;
 pub mod tty;
 pub mod ui;

src/utils/parser.rs

@@ -196,16 +196,7 @@ fn parse_generic_remote_opt(
     match REMOTE_GENERIC_OPT_REGEX.captures(s) {
         Some(groups) => {
             // Match user
-            let username: Option<String> = match groups.get(1) {
+            let username = groups.get(1).map(|x| x.as_str().to_string());
-                Some(group) => Some(group.as_str().to_string()),
-                None => match protocol {
-                    // If group is empty, set to current user
-                    FileTransferProtocol::Scp | FileTransferProtocol::Sftp => {
-                        Some(whoami::username())
-                    }
-                    _ => None,
-                },
-            };
             // Get address
             let address: String = match groups.get(2) {
                 Some(group) => group.as_str().to_string(),
@@ -437,7 +428,7 @@ mod tests {
         assert_eq!(result.protocol, FileTransferProtocol::Sftp);
         assert_eq!(params.address, String::from("172.26.104.1"));
         assert_eq!(params.port, 22);
-        assert!(params.username.is_some());
+        assert!(params.username.is_none());
         // User case
         let result: FileTransferParams = parse_remote_opt(&String::from("root@172.26.104.1"))
             .ok()
@@ -472,7 +463,7 @@ mod tests {
         assert_eq!(result.protocol, FileTransferProtocol::Sftp);
         assert_eq!(params.address, String::from("172.26.104.1"));
         assert_eq!(params.port, 4022);
-        assert!(params.username.is_some());
+        assert!(params.username.is_none());
         assert!(result.entry_directory.is_none());
         // Protocol
         let result: FileTransferParams = parse_remote_opt(&String::from("ftp://172.26.104.1"))
@@ -492,7 +483,7 @@ mod tests {
         assert_eq!(result.protocol, FileTransferProtocol::Sftp);
         assert_eq!(params.address, String::from("172.26.104.1"));
         assert_eq!(params.port, 22); // Fallback to sftp default
-        assert!(params.username.is_some()); // Doesn't fall back
+        assert!(params.username.is_none()); // Doesn't fall back
         assert!(result.entry_directory.is_none());
         let result: FileTransferParams = parse_remote_opt(&String::from("scp://172.26.104.1"))
             .ok()
@@ -501,7 +492,7 @@ mod tests {
         assert_eq!(result.protocol, FileTransferProtocol::Scp);
         assert_eq!(params.address, String::from("172.26.104.1"));
         assert_eq!(params.port, 22); // Fallback to scp default
-        assert!(params.username.is_some()); // Doesn't fall back
+        assert!(params.username.is_none()); // Doesn't fall back
         assert!(result.entry_directory.is_none());
         // Protocol + user
         let result: FileTransferParams =
@@ -539,7 +530,7 @@ mod tests {
         assert_eq!(result.protocol, FileTransferProtocol::Sftp);
         assert_eq!(params.address, String::from("172.26.104.1"));
         assert_eq!(params.port, 22);
-        assert!(params.username.is_some());
+        assert!(params.username.is_none());
         assert_eq!(result.entry_directory.unwrap(), PathBuf::from("home"));
         // All together now
         let result: FileTransferParams =

src/utils/ssh.rs

@@ -0,0 +1,45 @@
+use ssh2_config::{ParseRule, SshConfig};
+
+pub fn parse_ssh2_config(path: &str) -> Result<SshConfig, String> {
+    use std::fs::File;
+    use std::io::BufReader;
+
+    let mut reader = File::open(path)
+        .map_err(|e| format!("failed to open {path}: {e}"))
+        .map(BufReader::new)?;
+    SshConfig::default()
+        .parse(&mut reader, ParseRule::ALLOW_UNKNOWN_FIELDS)
+        .map_err(|e| format!("Failed to parse ssh2 config: {e}"))
+}
+
+#[cfg(test)]
+mod test {
+
+    use crate::utils::{ssh::parse_ssh2_config, test_helpers};
+
+    #[test]
+    fn should_parse_ssh2_config() {
+        let rsa_key = test_helpers::create_sample_file_with_content("ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDErJhQxEI0+VvhlXVUyh+vMCm7aXfCA/g633AG8ezD/5EylwchtAr2JCoBWnxn4zV8nI9dMqOgm0jO4IsXpKOjQojv+0VOH7I+cDlBg0tk4hFlvyyS6YviDAfDDln3jYUM+5QNDfQLaZlH2WvcJ3mkDxLVlI9MBX1BAeSmChLxwAvxALp2ncImNQLzDO9eHcig3dtMrEKkzXQowRW5Y7eUzg2+vvVq4H2DOjWwUndvB5sJkhEfTUVE7ID8ZdGJo60kUb/02dZYj+IbkAnMCsqktk0cg/4XFX82hEfRYFeb1arkysFisPU1DOb6QielL/axeTebVplaouYcXY0pFdJt root@8c50fd4c345a");
+        let ssh_config_file = test_helpers::create_sample_file_with_content(format!(
+            r#"
+Host test
+        HostName 127.0.0.1
+        Port 2222
+        User test
+        IdentityFile {}
+        StrictHostKeyChecking no
+        UserKnownHostsFile /dev/null
+"#,
+            rsa_key.path().display()
+        ));
+
+        assert!(parse_ssh2_config(
+            ssh_config_file
+                .path()
+                .to_string_lossy()
+                .to_string()
+                .as_str()
+        )
+        .is_ok());
+    }
+}