EvotecIT / Transferetto

Small PowerShell module with FTPS/SFTP functionality
MIT License
55 stars 14 forks source link

Implement PrivateKeyfile authentication method for SSH and SFTP connections #2

Closed Szeraax closed 3 years ago

Szeraax commented 3 years ago

This issue relates to Connect-SFTP.ps1 and Connect-SSH.ps1.

Currently, you can only use Credential or cleartext for connecting to an SFTP server. The SftpClient ctor contains two methods for creating a new connection with key files. The one I think you will like the most is this:

Renci.SshNet.SftpClient new(string host, string username, Params Renci.SshNet.PrivateKeyFile[] keyFiles)

PrivateKeyFile can be created directly from a string and errors nicely if the path is not found or if the file listed in the path is not a valid keyfile (no contents, damaged keyfile, putty ppk, etc.)

The SFTP client can be constructed via this method:

[Renci.SshNet.SftpClient]::new($Server,$Username,[Renci.SshNet.PrivateKeyFile]".\private.key")

Note that we can also add support for (multiple keys) or (password AND private key) via multiple authentication methods as outlined here:

Renci.SshNet.SftpClient new(Renci.SshNet.ConnectionInfo connectionInfo)
Renci.SshNet.ConnectionInfo new(string host, string username, Params Renci.SshNet.AuthenticationMethod[] authenticationMethods)

As usual, the ps module thingy with [Environment]::CurrentDirectory vs working directory always applies.

Szeraax commented 3 years ago

Ok, I've added a little PR that MIGHT work. I have no tested in any fashion. I will not be in a position to test it until tomorrow, so i've labeled the PR as draft for now. Feel free to edit as desired (I removed all the mandatory parameters from the SFTP params just for consistency sake). If you test and have it working, feel free to merge.

PrzemyslawKlys commented 3 years ago

Thank you. I just woke up to this. I can't really test this as I don't have a server that supports keys, but from the code, it looks ok.

I think it's possible to use login, password, and private key right? So we need to address that possibility.

Szeraax commented 3 years ago

Any time that I need to use password and private key, I'm doing stuff interactively and it's on winscp. For automation, my stuff is all private key + passphrase or just private key. So I'm not sure if anyone else really cares about being able to use multiple authentication method for a single connection.

If it was me, id probably be skipping it until someone puts in an issue.

Szeraax commented 3 years ago

I will test the pr today when I get a minute

Szeraax commented 3 years ago

PR has been accepted and closed. Awesome.

As for try/catch for errors, I would expect that letting the Renci object handle that would be sufficient. But if you want to add more try/catch to it, I'm sure that it'll make things a little clearer.

My plan for this module is to try it out on my next FTP automation project, but I dunno when that will be. Once I start doing that, I'll be able to open more issues/PRs to flesh out some of the functionality that I expect from SFTP that may not be present currently.

PrzemyslawKlys commented 3 years ago

Great - I've mostly focused on testing ftps as this is what I use. The question is how sftpclient reacts if the wrong value is provided. If nothing happens here no need for try/catch.

https://github.com/EvotecIT/Transferetto/blob/eb1630b509d8a12df76316d586485048848fb62b/Public/Connect-SFTP.ps1#L28-L37

Szeraax commented 3 years ago

ya, it doesn't give you a broken SFTP object. It throws. I'd say leave it alone.

On Tue, Sep 14, 2021 at 12:30 PM Przemysław Kłys @.***> wrote:

Great - I've mostly focused on testing ftps as this is what I use. The question is how sftpclient reacts if the wrong value is provided. If nothing happens here no need for try/catch.

https://github.com/EvotecIT/Transferetto/blob/eb1630b509d8a12df76316d586485048848fb62b/Public/Connect-SFTP.ps1#L28-L37

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/EvotecIT/Transferetto/issues/2#issuecomment-919410691, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPUBTZAAKYR6HLNAEFSVUTUB6IEZANCNFSM5DAVNFDQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

PrzemyslawKlys commented 3 years ago

Well if it throws - it's going to be red. I prefer to have Warning by default, and if someone provides ErrorAction Stop - then throw an error and stop.

https://github.com/EvotecIT/Transferetto/blob/eb1630b509d8a12df76316d586485048848fb62b/Public/Connect-SFTP.ps1#L42-L53