search

guyzmo / git-repo

Git-Repo: CLI utility to manage git services from your workspace
https://webchat.freenode.net/?channels=#git-repo
Other
840 stars 85 forks source link

GitHub 2FA support #39

Closed julian45 closed 8 years ago

julian45 commented 8 years ago

I have 2FA enabled on my GitHub account. When I ran git repo config, this happened:

Do you want to configure the github service?
    [Yn]> y
Please enter your credentials to connect to the service:
username> tcby45
password>
Fatal error: 401 Must specify two-factor authentication OTP code.

Could you please either include a 2FA check on the account (then a prompt for the code), or possibly ask whether to authenticate by personal access token or user/pass?

guyzmo commented 8 years ago

well, the idea is that when you enter login/password with github, it's actually creating a personal token for git-repo, it's NOT keeping your credentials.

I agree it's a bit too cryptic and needs more explanation, though. But if you don't want to share your credentials, just can edit ~/.gitconfig manually!

guyzmo commented 8 years ago

About github 2FA per se, it's definitely a good idea, but it's definitely low priority for me, way after implementing proper OAuth for bitbucket.

jandrusk commented 8 years ago

Just ran into the same thing and would like 2FA support as well.

guyzmo commented 8 years ago

how would you see the 2FA happening? Just for the first time authentication? Or each time you're using the CLI tool?

guyzmo commented 8 years ago

I'd be happy to see a proper usage use case for accepting the 2FA as a feature.

julian45 commented 8 years ago

With what you've described above, here's an ideal situation:

  1. User runs git repo config for the first time and elects to use GitHub.
  2. Wizard prompts user for username and password.
  3. Two options:

a. User with 2FA enabled sends username and password. Wizard receives X-GitHub-OTP: required; :2fa-type header (see here). User is prompted for OTP password, which is sent, in the X-GitHub-OTP header, along with another request containing the username and password. GitHub approves, and the wizard continues as normal. b. User without 2FA enabled sends username and password. GitHub approves, and the wizard continues as normal.

This way, anyone with 2FA enabled (i.e. those who have any sense of security) can take advantage of the wizard and git-repo as a whole.

guyzmo commented 8 years ago

oooh alright… we'll have to check with @sigmavirus24 how the sigmavirus24/github3.py library would handle 2FA, and I might make a patch for that.

guyzmo commented 8 years ago

looks like it's in the lib sigmavirus24/github3.py#167 so it's definitely possible to add support in the client.

guyzmo commented 8 years ago

I've just pushed a new branch that supports 2FA auth:

https://github.com/guyzmo/git-repo/tree/features/2FA_github

the only issue is that it crashes with a 422 Validation Failed when trying to create a token for the tool.

Please test and hack based on that commit 0c29743b596cc4be08adea2e788aa66938087aaf

julian45 commented 8 years ago

Downloaded and installed from that branch, got the following error: Fatal error: 'GitHub' object has no attribute 'token'

guyzmo commented 8 years ago

oh, I'm sorry, I forgot a bit of code when I made the commit ☺

Though it's still failing with the 422 error.

I guess that we need to patch the github3.py code so that it supports the two_factor_auth_callback setting as shown in the login() method.

The main difference in auth methods being that authorize() uses self.session.temporary_basic_auth(), whilst login() uses self.session.basic_auth(). Looking more closely at the code, the former is actually a wrapper around the second, implemented as a context manager.

As you can see, on the git-repo side, the code implementing auth is pretty minimalistic…

sigmavirus24 commented 8 years ago

self.session.basic_auth() does nothing with the 2FA callback work. You'll need to tell us you're going to use the 2FA callback (at the moment just doing gh.login(two_factor_callback=...) and then using authorize. It will still catch it for you

guyzmo commented 8 years ago

hi @sigmavirus24 🖖, very nice of you to join the discussion!

well, that's what I've actually implemented there, and for some reason it's failing with a 422.

So let's dig into that…

guyzmo commented 8 years ago

…ok I went too fast when I first tried.

The issue is that when there's already a key with the same name on github, it's failing with a 422 Unprocessable Entity, making some confusion.

So basically, the 2FA code is working! I'm not sure how I can make a meaningful regression test for that, though… ☹

guyzmo commented 8 years ago

ok, it's merged in devel. I'll roll it out with the next release.