sassoftware / vscode-sas-extension

This SAS Extension for Visual Studio Code provides support for the SAS language, including features such as SAS syntax highlighting, code completion, hover help, code folding, outline, SAS code snippets and run SAS code.
https://sassoftware.github.io/vscode-sas-extension/
Apache License 2.0
117 stars 47 forks source link

feat: username and password support for SSH connection type #1126

Open smorrisj opened 2 months ago

smorrisj commented 2 months ago

Summary

Todos:

Testing

Connection Profile Prompts:

Auth Refactor: Users can successfully authenticate to the server to run sas programs using the following auth methods:

Work Directory Detection

Keepalive Changes:

smorrisj commented 1 month ago

Pushed a super rough draft of auth handler logic this morning. Very rough around the edges but wanted to get thoughts down in some (semi) organized manner. Will further test and refine as time allows.

chunky commented 1 month ago

First up, thank you for working on this. It'll be a big improvement for us.

A few random thoughts:

  1. You can find a conversation I had with a maintainer of ssh2 about authhandlers here: https://github.com/mscdex/ssh2/issues/1400 . Things that might be relevant from your comments and code so far:
    1. The authhandler is only called with a list of methods that would work for that server
    2. "agent" and "private key" are different, in the authhandler callback.
    3. Note this from the ssh2 doc you link: "Valid method names are: ''none', 'password', 'publickey', 'agent', 'keyboard-interactive', 'hostbased'"
  2. I feel pretty strongly that attempting all non-interactive options on the table first, and only then trying the interactive ones is the right choice. If agent would be accepted, and I have a running agent, then obviously that should be tried before asking users for a password, for example.
  3. Don't forget keyboard-interactive, as separate from password. Some server configurations ask for "keyboard-interactive" authentication and ask you for a password in a message, rather than asking for "password" authentication. [set sshd config "ChallengeResponseAuthentication yes" to see this]
  4. Affording use of an unshielded private key as a separate config item, and proffering it if "private key" method is available, would really help
  5. I have no strong feelings about password storage in secrets. The main concern is, users have to rotate passwords regularly; what happens if the password in storage is no longer the same one that works for the server? Honestly for the first pass, asking for it each time doesn't seem unreasonable.
    1. users can set up public key auth if they don't like it

From your top message: "Users should still be able to specify an identify file for keys that have a passphrase." ; do you mean "for keys that don't have a passphrase"? For keys that do, you would need to develop the ability to de-shield them after loading them from disk, to pass to the private key method. Probably safer to just expect users to set up ssh-agent, if that's where they're at. [although it's true that that is unfortunate on windows]

smorrisj commented 1 month ago

First up, thank you for working on this. It'll be a big improvement for us.

A few random thoughts:

  1. You can find a conversation I had with a maintainer of ssh2 about authhandlers here: Request: Delay client password entry until password actually needed mscdex/ssh2#1400 . Things that might be relevant from your comments and code so far:

    1. The authhandler is only called with a list of methods that would work for that server
    2. "agent" and "private key" are different, in the authhandler callback.
    3. Note this from the ssh2 doc you link: "Valid method names are: ''none', 'password', 'publickey', 'agent', 'keyboard-interactive', 'hostbased'"
  2. I feel pretty strongly that attempting all non-interactive options on the table first, and only then trying the interactive ones is the right choice. If agent would be accepted, and I have a running agent, then obviously that should be tried before asking users for a password, for example.
  3. Don't forget keyboard-interactive, as separate from password. Some server configurations ask for "keyboard-interactive" authentication and ask you for a password in a message, rather than asking for "password" authentication. [set sshd config "ChallengeResponseAuthentication yes" to see this]
  4. Affording use of an unshielded private key as a separate config item, and proffering it if "private key" method is available, would really help
  5. I have no strong feelings about password storage in secrets. The main concern is, users have to rotate passwords regularly; what happens if the password in storage is no longer the same one that works for the server? Honestly for the first pass, asking for it each time doesn't seem unreasonable.

    1. users can set up public key auth if they don't like it

From your top message: "Users should still be able to specify an identify file for keys that have a passphrase." ; do you mean "for keys that don't have a passphrase"? For keys that do, you would need to develop the ability to de-shield them after loading them from disk, to pass to the private key method. Probably safer to just expect users to set up ssh-agent, if that's where they're at. [although it's true that that is unfortunate on windows]

All good feedback. I generally agree with thoughts above. I've put comments around changes that I think support these items. On the issue of whether or not to store the passwords in secrets, we have some precedent for doing this in other connection types. I do agree though that generally for ssh interaction with other solutions, anytime passwordless or passphraseless interactions are intended, that I've seen docs push users towards agent or public key.

jbreitman commented 1 month ago

I would like to see the authentication method gssapi-with-mic added. This will be beneficial to those in corporate environments where Kerberos Tickets enable you to access other resources such as file systems, databases, APIs, etc ... while eliminating the need for the person to enter their credentials.

chunky commented 1 month ago

If you can figure it out, yes, the kerberos SSO stuff is great where available. That was another thing I couldn't figure out. It works on some of our servers. Another of those "if it works it's magical and absolutely preferred".

I think the distinction between "agent" and "public key" is a manifestation from the ssh2 library, not the server. In practice it might mean taking two bites at the key-based apple, which is explicitly afforded by the Auth handler mechanism

smorrisj commented 1 month ago

I think the distinction between "agent" and "public key" is a manifestation from the ssh2 library, not the server. In practice it might mean taking two bites at the key-based apple, which is explicitly afforded by the Auth handler mechanism

Yea that's what the current changeset has, inside of "publickey" we'd look for whether or not the user has set their keyfile on the connection profile, if so we'd use that file, otherwise we'd defer to agent.

I would like to see the authentication method gssapi-with-mic added. This will be beneficial to those in corporate environments where Kerberos Tickets enable you to access other resources such as file systems, databases, APIs, etc ... while eliminating the need for the person to enter their credentials.

One challenge is that the ssh2 library doesnt support GSSAPI methods yet: https://github.com/mscdex/ssh2/issues/333

chunky commented 1 month ago

@smorrisj I missed this comment from earlier: "I'm seeing the auth method for agent and public key coming back from the server simply as "publickey". I'm wondering if the library is adding in agent based on set config options?"

From the server's perspective, these are the same thing; it just wants to do a key-based handshake. But on the client, they're different beasts. With ssh2's 'agent' they go get it from a daemon running on a socket, while 'publickey' is just a thing where you hand ssh2 the ready-to-use bytes.

Zhirong2022 commented 6 days ago

1.Create a SSH connection profile and specify an invalid privateKeyFilePath, run some code, it will prompt error message as expected. 2.Update the profile to correct the file path, run code, it has the same error as before. The only way to restart VSC to use the updated profile.

Zhirong2022 commented 5 days ago

1.Create a SSH connection profile, specify privateKeyFilePath field in connection profile (with a passphrase). 2.Run some code, it will ask the user to enter the passphrase for the private key 3.Cancel the dialog or type an invalid password in such scenario Result: It keeps showing 'Connecting to SAS session' until it gets error message 'Could not connect to the SAS server.' Suggestion: If the user cancel or type with an invalid password, it can stop the process right now without having to wait for some time.

Zhirong2022 commented 5 days ago

It cannot create the profile successfully if leave the 'Enter the local private key file path' as blank. For others fields have been filled correctly.

smorrisj commented 3 days ago

It cannot create the profile successfully if leave the 'Enter the local private key file path' as blank. For others fields have been filled correctly.

Fixed in https://github.com/sassoftware/vscode-sas-extension/pull/1126/commits/d3c9cd20a7ed32da89318826b0ede0239c067780

smorrisj commented 1 hour ago

1.Create a SSH connection profile and specify an invalid privateKeyFilePath, run some code, it will prompt error message as expected. 2.Update the profile to correct the file path, run code, it has the same error as before. The only way to restart VSC to use the updated profile.

Fixed in 6d28a43

smorrisj commented 1 hour ago

1.Create a SSH connection profile, specify privateKeyFilePath field in connection profile (with a passphrase). 2.Run some code, it will ask the user to enter the passphrase for the private key 3.Cancel the dialog or type an invalid password in such scenario Result: It keeps showing 'Connecting to SAS session' until it gets error message 'Could not connect to the SAS server.' Suggestion: If the user cancel or type with an invalid password, it can stop the process right now without having to wait for some time.

In this setup, we're trying as much as possible to let the SSH client and server negotiate auth based on the server configuration. If you're seeing that message, it means that the SSH client received a disconnect from the server side to close the connection before auth could finish. This can happen in a number of auth setups, for example, if max auth tries are exceeded etc. I think to support the largest number of customer configured auth setups, we should keep this the way it is.