Skip to content
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

Add SSH key generation & uploading to gh auth login flow #2892

Merged
merged 7 commits into from Feb 17, 2021
Merged

Conversation

mislav
Copy link
Contributor

@mislav mislav commented Feb 1, 2021

When "SSH" protocol is selected during gh auth login interactive mode, this offers to generate and upload an SSH key to the user's GitHub account.

The order of gh auth login questions was reshuffled to accommodate the new flow and to know beforehand which OAuth scopes to request (workflow for HTTPS mode, admin:public_key for SSH mode).

The new flow when no SSH keys exist locally:

$ gh auth login -h github.com
? What is your preferred protocol for Git operations?  [Use arrows to move, type to filter]
  HTTPS
> SSH
? Generate a new SSH key to add to your GitHub account? (Y/n)
? Enter the passphrase for your new SSH key *****
? How would you like to authenticate GitHub CLI?  [Use arrows to move, type to filter]
> Login with a web browser
  Paste an authentication token

! First copy your one-time code: ****-****
- Press Enter to open github.com in your browser...
✓ Authentication complete. Press Enter to continue...

- gh config set -h github.com git_protocol ssh
✓ Configured git protocol
✓ Uploaded the SSH key to your GitHub account: /Users/mislav/.ssh/id_ed25519.pub
✓ Logged in as mislav

The new flow when existing ~/.ssh/*.pub files (SSH public keys) were found locally:

$ gh auth login -h github.com
? What is your preferred protocol for Git operations?  [Use arrows to move, type to filter]
  HTTPS
> SSH
? Upload your SSH public key to your GitHub account?  [Use arrows to move, type to filter]
> /Users/mislav/.ssh/id_ed25519.pub
  Skip
? How would you like to authenticate GitHub CLI? Login with a web browser

! First copy your one-time code: ****-****
- Press Enter to open github.com in your browser...
✓ Authentication complete. Press Enter to continue...

- gh config set -h github.com git_protocol ssh
✓ Configured git protocol
✓ Uploaded the SSH key to your GitHub account: /Users/mislav/.ssh/id_ed25519.pub
✓ Logged in as mislav

Under the hood:

  • This approach goes out of its way to request only the minimum OAuth scopes necessary
  • We only offer to generate a SSH key when ssh-keygen is available on the system. On Windows, we try to locate ssh-keygen within the Git for Windows installation.
  • We select the ed25519 algorithm when generating SSH keys, which is also recommended by GitHub help. Some older ssh-keygen implementations might not support ed25519.
  • We avoid ever overwriting any existing key under the ~/.ssh directory.
  • Trying to re-upload an SSH key that is already in the user's account will succeed silently instead of yielding a HTTP 422 error.
  • Removes the need for overridable shared.ClientFromCfg function
    • Fixes the correct User-Agent header being used for API requests during gh auth

Questions for the team:

  • Does the prompt order & phrasing make sense?
  • Should gh auth refresh also have any of this functionality?

TODO:

  • Test coverage for ssh-keygen flow

Fixes #2627

@mislav mislav requested review from a team and ampinsk February 4, 2021 16:27
@mislav mislav added this to Backlog 🗒 in The GitHub CLI via automation Feb 4, 2021
@mislav mislav moved this from Backlog 🗒 to Needs review 🤔 in The GitHub CLI Feb 4, 2021
Copy link
Contributor

@vilmibm vilmibm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great ✨ marking request change for my one incredibly minor request about passphrases.

Comment on lines 81 to 86
// err = prompt.SurveyAskOne(&survey.Input{
// Message: "Enter the label for your new SSH key",
// }, &sshLabel)
// if err != nil {
// return "", fmt.Errorf("could not prompt: %w", err)
// }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, good call. I was on the fence about leaving it in, but it's entirely optional for SSH keys in general and adds yet another prompt in this command that already has a dozen prompts. Best to avoid it.

pkg/cmd/auth/shared/ssh_keys.go Outdated Show resolved Hide resolved
@ampinsk
Copy link
Contributor

ampinsk commented Feb 4, 2021

Re: this part of the existing ssh key flow:

? Upload your SSH public key to your GitHub account?  [Use arrows to move, type to filter]
> /Users/mislav/.ssh/id_ed25519.pub
  Skip

Should we offer to let you create a new one here? Or is that unnecessary?

Co-authored-by: Amanda Pinsker <ampinsk@github.com>
@mislav
Copy link
Contributor Author

mislav commented Feb 5, 2021

Should we offer to let you create a new one here? Or is that unnecessary?

That's a great idea which I hadn't at all considered. When the user already has some SSH keys of their own, I assumed that they wanted to reuse one of them for GitHub because that's what I would have done, but it's entirely possible that they might want to generate a new one for GitHub.

Here's a catch though. Since some keys already exist, when generating a new one we'd have to also ask the person to name their new key on disk (to avoid clashes with existing keys). And, if the user names their key something other than id_dsa, id_ecdsa, id_ed25519, or id_rsa (i.e. the default key names), then they would have to add additional lines to their ~/.ssh/config to wire up their key for use with GitHub:

Host github.com
  IdentityFile ~/.ssh/<my-new-key>

We could either tell the user they have to do this, or do it for them, but I'm a bit wary of editing people's ~/.ssh/config. @vilmibm do you have opinions on how best to handle this?

@vilmibm
Copy link
Contributor

vilmibm commented Feb 5, 2021

@mislav @ampinsk

Offering to generate new keys at that step makes sense but I think can be punted on. The kind of folks that tend to generate new keypairs per remote are more security conscious and I feel like our SSH helpers here are for people less familiar with SSH. Given that and the complexities you mentioned (ensuring we don't overwrite and potentially handling the ssh config) I think we should leave things as they are in the PR.

Regarding updating the SSH config, the cleanest way that I can think of is converting a user's ssh config to .ssh/config.d and adding a file there but I'd be uncomfortable with that level of manipulation of a user's .ssh directory.

Copy link
Contributor

@vilmibm vilmibm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

The GitHub CLI automation moved this from Needs review 🤔 to Needs to be merged 🎉 Feb 5, 2021
@mislav mislav merged commit 3a224b7 into trunk Feb 17, 2021
The GitHub CLI automation moved this from Needs to be merged 🎉 to Pending Release 🥚 Feb 17, 2021
@mislav mislav deleted the auth-with-ssh branch February 17, 2021 16:07
@github-actions github-actions bot moved this from Pending Release 🥚 to Done 💤 in The GitHub CLI Feb 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
The GitHub CLI
  
Done 💤
Development

Successfully merging this pull request may close these issues.

After logging in with GitHub CLI, offer to generate and/or upload user's SSH keys
3 participants