New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[dashboard] support connect via SSH #9003
Conversation
@gtsiolis UX is slightly different from the design, because the connection of a single command requires many preconditions, please take a look |
@loujaybee Manual connections are the most compatible way to connect, and users can use any ssh client, such as The one-command connection has many prerequisites, such as sshpass in this case, which needs to be installed separately and is not included in the operating system by default, and the installation method is slightly different for each operating system |
Looking at this now! 👀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@iQQBot Thanks for pushing this forward! 🏀
UX changes using the modal look good.
However, I left some comments about UX and security practices here as these could affect the adoption of this feature.
Long-term exposing the password like this seems unessasary and can be avoided. I'd like to suggest the following approaches but I'm not sure if these will increase significantly the scope of this PR:
ssh
, curl
, or something similar that could allow use to have a single command to copy and paster as well as authenticate via browser. Thus, avoiding and dropping all together sshpass
support.
Looping in @akosyakov and @loujaybee to decide whether we'd like to a) ship this as is and iterate or b) step back and go with one of the
), | ||
}, | ||
{ | ||
key: "sshpass", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: While this seems efficient do we really want to promote tools like this? Are you regularly using this, @iQQBot? I could be wrong but I assume many users will be against installing tools like this simply to use SSH access.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've never actually used it, I've only seen it used in a demo by @csweichel 😂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sshpass is the common way to pass credentials to SSH on the commandline, i.e. it's not an uncommon tool. That said, it might not be installed everywhere.
In both cases: manual and sshpass we should provide documentation that explains how this ought to be used.
I'd almost argue for removing the sshpass for now, but offering a single commandline is just too sweet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We actually support workspaceID#ownerToken@ideUrl for single command connections
But this has to be based on the fact that the user implicitly provides a priavate_key which is the default id_rsa for authentication, we don't authenticate the user's private key, we just skip the step of having to enter a password through this mechanism. That said, if the user does not have any default privatekey, then the user will still be asked to enter the password.
We can change the behavior of the SSHGateway so that it can log in with any password if it provides full authentication information like workspaceID#ownerToken, but users must also be told this
#9003 (comment) And with a program like this, the user will be confused as to how the input should be done
To ease the burden of education in this area, I chose to provide sshpass and host
username
password
respectively.
CC @csweichel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, that's a neat hack :)
It is also a bit indirect and harder to understand.
Frankly I'd vote for going for the manual/sshpass options as they stand and make sure we have really good documentation for both cases in place.
@gtsiolis Thanks for your suggestion
Unfortunately, native ssh does not support passing passwords, much less browser authentication
Maybe it can be a solution, but we need to consider
|
💯
🛹 🛹 🛹 🛹 🛹 🛹 🛹 🛹 🛹
We could offer some Security-minded users would not execute anything
This is definitely where we want to go. It's also a considerable change, we'd need to
|
Yes, it is a good program, but I think we need to consider I mention above
|
I would start with something simple like single |
Who can help me write this warning text? Do we need to explain to the user why a default private key is needed? @akosyakov #9003 (comment) I think these situations still need to be considered, if the user is a professional user that is no problem, but if the user is not a professional user they will be very confused |
I remember you mentioned also that ssh supports an option to use only password based auth without keys? i.e. will disabling of
I am not sure yet what our users are going to do with it. We could do later it too, no? We should get out something simple and run some usability tests. |
Yes, ssh supports password-only authentication, but the ssh command does not support passing passwords, which is the biggest problem, If you need to pass a password, you must use a tool like sshpass. |
ok, could we then instead provide a warning which says that a user should have at least one private key and point to some docs how to create it (maybe there is some nice section on ssh.com)? |
@loujaybee we already got this event by {"jsonrpc":"2.0","id":14,"method":"trackEvent","params":{"anonymousId":"da439578-2c6c-42f8-80bc-ae6bfa1ca450","event":"dashboard_clicked","properties":{"path":"/workspaces","label":"Actions","button_type":"primary"}}} Do we need to add a separate analytics tracking event for this? |
@gtsiolis I follow your suggestion, can you please have a look again? |
@iQQBot new analytics events have to be added here: https://www.notion.so/gitpod/Tracking-Plan-ed27d1492da745b9b31711dd73dcbe57#6c68c725b7d744ebb0bd1593f37ed623 |
I think it is alright to reuse events if they are stable, but we have to update the epic to have proper info about how we can verify that a feature got discovered and used. It would be also cool to have a dashboard for it in Mixpanel. cc @loujaybee |
/werft run 👍 started the job as gitpod-build-pd-ssh-copy.12 |
@iQQBot ah yep, I think that tracking event should work, thanks! 🚀 |
fyi: Can not open new workspaces in the preview environment. Cross-posted internally, see relevant discussion (internal). |
/werft run 👍 started the job as gitpod-build-pd-ssh-copy.15 |
<p className="text-gray-500 whitespace-normal text-base"> | ||
The following shell command can be used to SSH into this workspace. | ||
</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: What do you think of
BEFORE | AFTER (:a:) | AFTER (:b:) |
---|---|---|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
go for A
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good! Impeccable agreements! 🤝
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, @iQQBot! Left one last comment above.
@gtsiolis Already change, please take a look again |
Can someone from @gitpod-io/engineering-webapp have a review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
Now that we have the new SSH gateway and the simpler authentication flow, it is possible that a user can copy/paste an SSH command onto their local machine to facilitate quicker/easier access to a Gitpod workspace.
This PR introduces a way to direct connect ssh to workspace
Related Issue(s)
Fixes #7713
How to test
start a workspace in preview environment, you can see Connect via SSH in workspace list and workspace start page(desktop IDE)
Release Notes
Documentation