Obtaining authentication from environment #463

Closed
pwaller opened this Issue Jan 15, 2014 · 28 comments

Comments

Projects
None yet
10 participants
@pwaller

pwaller commented Jan 15, 2014

I don't want to have a .config/hub file lying around on remote systems where I use hub. I'd prefer to communicate my password through the environment (since I can use ssh SendEnv, etc).

I've tried using GITHUB_USER and GITHUB_PASSWORD with a token generated from https://github.com/settings/applications, but when I try to send a pull request I get "404 not found". It works if I send a pull request by entering my credentials and using .config/hub. Any ideas?

@mislav

This comment has been minimized.

Show comment
Hide comment
@mislav

mislav Jan 15, 2014

Member

Your use case is not supported right now. The thing about GITHUB_PASSWORD is, it's just an alternative to typing your password in the terminal. hub will still try to exchange those credentials for an OAuth token.

Maybe we could support another variable such as GITHUB_TOKEN, and if it's set, use Basic Auth with your username and that token and never exchange that for OAuth. That would skip reading/writing to ~/.config/hub.

Member

mislav commented Jan 15, 2014

Your use case is not supported right now. The thing about GITHUB_PASSWORD is, it's just an alternative to typing your password in the terminal. hub will still try to exchange those credentials for an OAuth token.

Maybe we could support another variable such as GITHUB_TOKEN, and if it's set, use Basic Auth with your username and that token and never exchange that for OAuth. That would skip reading/writing to ~/.config/hub.

@pwaller

This comment has been minimized.

Show comment
Hide comment
@pwaller

pwaller Jan 15, 2014

👍

pwaller commented Jan 15, 2014

👍

@supermarin

This comment has been minimized.

Show comment
Hide comment
@mislav

This comment has been minimized.

Show comment
Hide comment
@mislav

mislav Jan 19, 2014

Member

@mneorr But the keychain client needs to be a compiled program in order to retrieve stuff securely, and I don't wanna go down that road. https://github.com/boxen/boxen/blob/23b0038eda0a67e172fcf6229ea7273ba29ffa18/src/keychain-helper.c

Member

mislav commented Jan 19, 2014

@mneorr But the keychain client needs to be a compiled program in order to retrieve stuff securely, and I don't wanna go down that road. https://github.com/boxen/boxen/blob/23b0038eda0a67e172fcf6229ea7273ba29ffa18/src/keychain-helper.c

@supermarin

This comment has been minimized.

Show comment
Hide comment
@supermarin

supermarin Jan 19, 2014

@mislav not necessary. There's a CLI tool (security) built into OSX. Try man security.

Sample getter for a github token:
security find-generic-password -wgs github.oauth_token

@mislav not necessary. There's a CLI tool (security) built into OSX. Try man security.

Sample getter for a github token:
security find-generic-password -wgs github.oauth_token

@supermarin

This comment has been minimized.

Show comment
Hide comment
@supermarin

supermarin Jan 19, 2014

Setter:
security add-generic-password -a mneorr -s github.oauth_token -w 1234567890abcdefghijklmnoprstuvwxyz

Setter:
security add-generic-password -a mneorr -s github.oauth_token -w 1234567890abcdefghijklmnoprstuvwxyz

@mislav

This comment has been minimized.

Show comment
Hide comment
@mislav

mislav Jan 19, 2014

Member

There's a CLI tool (security) built into OSX. Try man security.

…And if you use that route, now any rogue program can shell out and invoke the same command to obtain that password. That defeats the whole purpose of Keychain, as it becomes no more secure than storing it in plain text in a file.

The point of Keychain is that you allow a single binary application to store/retrieve some values. If another app tries to get those values, it will be blocked. If the original binary changes and tries to access those values, Keychain will warn you and ask for approval.

Member

mislav commented Jan 19, 2014

There's a CLI tool (security) built into OSX. Try man security.

…And if you use that route, now any rogue program can shell out and invoke the same command to obtain that password. That defeats the whole purpose of Keychain, as it becomes no more secure than storing it in plain text in a file.

The point of Keychain is that you allow a single binary application to store/retrieve some values. If another app tries to get those values, it will be blocked. If the original binary changes and tries to access those values, Keychain will warn you and ask for approval.

@supermarin

This comment has been minimized.

Show comment
Hide comment
@supermarin

supermarin Jan 19, 2014

Well, it's still better than getting your token checked in by accident together with your dotfiles.
~/.config doesn't sound like the right place to store a token.

In my case, fish stores stuff in ~/.config/fish, and that's how I've messed up checking it in.

Well, it's still better than getting your token checked in by accident together with your dotfiles.
~/.config doesn't sound like the right place to store a token.

In my case, fish stores stuff in ~/.config/fish, and that's how I've messed up checking it in.

@supermarin

This comment has been minimized.

Show comment
Hide comment
@supermarin

supermarin Jan 19, 2014

Probably add-generic-password isn't the right way but the quickest. After looking a bit more into it, add-internet-password doesn't seem that bad.

$ security add-internet-password                                                                                                                                      Users/musalj
Usage: add-internet-password [-a account] [-s server] [-w password] [options...] [-A|-T appPath] [keychain]
    -a  Specify account name (required)
    -c  Specify item creator (optional four-character code)
    -C  Specify item type (optional four-character code)
    -d  Specify security domain string (optional)
    -D  Specify kind (default is "Internet password")
    -j  Specify comment string (optional)
    -l  Specify label (if omitted, server name is used as default label)
    -p  Specify path string (optional)
    -P  Specify port number (optional)
    -r  Specify protocol (optional four-character SecProtocolType, e.g. "http", "ftp ")
    -s  Specify server name (required)
    -t  Specify authentication type (as a four-character SecAuthenticationType, default is "dflt")
    -w  Specify password to be added
    -A  Allow any application to access this item without warning (insecure, not recommended!)
    -T  Specify an application which may access this item (multiple -T options are allowed)
    -U  Update item if it already exists (if omitted, the item cannot already exist)

Probably add-generic-password isn't the right way but the quickest. After looking a bit more into it, add-internet-password doesn't seem that bad.

$ security add-internet-password                                                                                                                                      Users/musalj
Usage: add-internet-password [-a account] [-s server] [-w password] [options...] [-A|-T appPath] [keychain]
    -a  Specify account name (required)
    -c  Specify item creator (optional four-character code)
    -C  Specify item type (optional four-character code)
    -d  Specify security domain string (optional)
    -D  Specify kind (default is "Internet password")
    -j  Specify comment string (optional)
    -l  Specify label (if omitted, server name is used as default label)
    -p  Specify path string (optional)
    -P  Specify port number (optional)
    -r  Specify protocol (optional four-character SecProtocolType, e.g. "http", "ftp ")
    -s  Specify server name (required)
    -t  Specify authentication type (as a four-character SecAuthenticationType, default is "dflt")
    -w  Specify password to be added
    -A  Allow any application to access this item without warning (insecure, not recommended!)
    -T  Specify an application which may access this item (multiple -T options are allowed)
    -U  Update item if it already exists (if omitted, the item cannot already exist)
@pwaller

This comment has been minimized.

Show comment
Hide comment
@pwaller

pwaller Mar 21, 2015

Tidying up my personal issues list, so closing this. Please create a new issue if you're still interested in tracking it.

pwaller commented Mar 21, 2015

Tidying up my personal issues list, so closing this. Please create a new issue if you're still interested in tracking it.

@pwaller pwaller closed this Mar 21, 2015

@mislav

This comment has been minimized.

Show comment
Hide comment
@mislav

mislav Mar 23, 2015

Member

I would appreciate if you didn't close an old ticket when the issue is still unsolved. Forcing us to open another issue means that we lose the conversation thread as well as everyone who was subscribed to the updates for this issue.

Member

mislav commented Mar 23, 2015

I would appreciate if you didn't close an old ticket when the issue is still unsolved. Forcing us to open another issue means that we lose the conversation thread as well as everyone who was subscribed to the updates for this issue.

@xakraz

This comment has been minimized.

Show comment
Hide comment
@xakraz

xakraz Sep 3, 2015

👍 also for GITHUB_TOKEN

xakraz commented Sep 3, 2015

👍 also for GITHUB_TOKEN

@mislav mislav closed this in 27f2023 Sep 26, 2015

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Sep 26, 2015

Contributor

1c6d4b8 c23823d 9e6b7cb (merge commit: 2548707)

Contributor

parkr commented Sep 26, 2015

1c6d4b8 c23823d 9e6b7cb (merge commit: 2548707)

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Sep 26, 2015

Contributor

Thanks for picking this up, @mislav!

Contributor

parkr commented Sep 26, 2015

Thanks for picking this up, @mislav!

@corysimmons

This comment has been minimized.

Show comment
Hide comment
@corysimmons

corysimmons Jan 10, 2016

I'm not sure this is fixed. I had Hub installed from a while back and just went to try it out again and kept getting 401 errors. Apparently I had deleted my old token. I created a new one and added it, but I'm not sure what permissions that token needs, and it seems like bad design to make the user have to track down all this stuff.

Is it possible to test if the token is good and if it isn't generate/store a new one? Fringe case, I know, but it would've saved me a 10 minute headache. :^)

I'm not sure this is fixed. I had Hub installed from a while back and just went to try it out again and kept getting 401 errors. Apparently I had deleted my old token. I created a new one and added it, but I'm not sure what permissions that token needs, and it seems like bad design to make the user have to track down all this stuff.

Is it possible to test if the token is good and if it isn't generate/store a new one? Fringe case, I know, but it would've saved me a 10 minute headache. :^)

@mislav

This comment has been minimized.

Show comment
Hide comment
@mislav

mislav Jan 10, 2016

Member

@corysimmons No updates yet. Pull requests welcome!

Member

mislav commented Jan 10, 2016

@corysimmons No updates yet. Pull requests welcome!

@corysimmons

This comment has been minimized.

Show comment
Hide comment
@corysimmons

corysimmons Jan 10, 2016

I would but I don't know Go at all. Planning on learning it in the next few months. Will get back to you then. ;^)

I would but I don't know Go at all. Planning on learning it in the next few months. Will get back to you then. ;^)

@danielcompton

This comment has been minimized.

Show comment
Hide comment
@danielcompton

danielcompton Jan 11, 2016

I had the same issue today.

~/D/j/karma (master) $ hub fork
Error creating fork: Unauthorized (HTTP 401)
Bad credentials

A few things could make this easier to debug/fix:

  • Say where the bad credentials came from (~/.config/hub)
  • Say which user Hub was attempting to authenticate as
  • Include in the log message that this is a Hub error, not a Git error (maybe?)
  • Tell people that they can clear credentials and reauthenticate by deleting ~/.config/hub (or whatever is the correct way of doing this)

I had the same issue today.

~/D/j/karma (master) $ hub fork
Error creating fork: Unauthorized (HTTP 401)
Bad credentials

A few things could make this easier to debug/fix:

  • Say where the bad credentials came from (~/.config/hub)
  • Say which user Hub was attempting to authenticate as
  • Include in the log message that this is a Hub error, not a Git error (maybe?)
  • Tell people that they can clear credentials and reauthenticate by deleting ~/.config/hub (or whatever is the correct way of doing this)
@mislav

This comment has been minimized.

Show comment
Hide comment
@mislav

mislav Jan 14, 2016

Member

@corysimmons I didn't know Go at all as well until @jingweno (then just a fan) rewrote all of hub in Go and I've accepted it as a contribution 😉

@danielcompton Those are all great suggestions. I would like to implement some of your ideas together with a specialized command to handle (re)authentication. Then, the error message could suggest that you try to reauthenticate to obtain a fresh token.

Member

mislav commented Jan 14, 2016

@corysimmons I didn't know Go at all as well until @jingweno (then just a fan) rewrote all of hub in Go and I've accepted it as a contribution 😉

@danielcompton Those are all great suggestions. I would like to implement some of your ideas together with a specialized command to handle (re)authentication. Then, the error message could suggest that you try to reauthenticate to obtain a fresh token.

@corysimmons

This comment has been minimized.

Show comment
Hide comment
@corysimmons

corysimmons Jan 15, 2016

lol what kind of person submits a PR with an entire project rewritten in another language? What kind of person accepts that? You guys are crazy, or talented, or both. 👯

lol what kind of person submits a PR with an entire project rewritten in another language? What kind of person accepts that? You guys are crazy, or talented, or both. 👯

@mislav

This comment has been minimized.

Show comment
Hide comment
@mislav

mislav Jan 18, 2016

Member

lol what kind of person submits a PR with an entire project rewritten in another language? What kind of person accepts that?

Yeah it's pretty crazy, I know. 😄 The Go version was faster (I plateaued in terms of how fast I can make Ruby implementation be) and easier to distribute via prebuilt binaries.

Member

mislav commented Jan 18, 2016

lol what kind of person submits a PR with an entire project rewritten in another language? What kind of person accepts that?

Yeah it's pretty crazy, I know. 😄 The Go version was faster (I plateaued in terms of how fast I can make Ruby implementation be) and easier to distribute via prebuilt binaries.

@corysimmons

This comment has been minimized.

Show comment
Hide comment
@corysimmons

corysimmons Jan 18, 2016

Looks great. I'm excited to learn it. =)

Looks great. I'm excited to learn it. =)

@jingweno

This comment has been minimized.

Show comment
Hide comment
@jingweno

jingweno Jan 19, 2016

Collaborator

@calebthompson It's pretty crazy :). I documented why I did that. And @mislav helped a lot for the migration.

Collaborator

jingweno commented Jan 19, 2016

@calebthompson It's pretty crazy :). I documented why I did that. And @mislav helped a lot for the migration.

@calebthompson

This comment has been minimized.

Show comment
Hide comment
@calebthompson

calebthompson Jan 19, 2016

Contributor

Uh, hi. @corysimmons maybe?

Contributor

calebthompson commented Jan 19, 2016

Uh, hi. @corysimmons maybe?

@corysimmons

This comment has been minimized.

Show comment
Hide comment
@corysimmons

corysimmons Jan 19, 2016

@jingweno Yeah, read it the other day. Seems amazing.

@calebthompson Yeah I think he meant to ping me. =)

@jingweno Yeah, read it the other day. Seems amazing.

@calebthompson Yeah I think he meant to ping me. =)

greg-1-anderson added a commit to greg-1-anderson/hub that referenced this issue Nov 3, 2016

Add authentication instructions to README
Authentication options are discussed in #491 and #463; it would be good to have some of this information in the README as well.
@FranciscoBorges

This comment has been minimized.

Show comment
Hide comment
@FranciscoBorges

FranciscoBorges Jan 4, 2017

So originally the argument for not using OSX Keychain was that hub was not compiled. Hub now is written in Go and distributed in compiled form, would it be too difficult to just add Keychain support?

FWIW, I would be using 'hub' in a GitHub Enterprise environment where I must work with tokens, i.e. authenticating with user/pass directly with the GitHub server is never going to work.

So originally the argument for not using OSX Keychain was that hub was not compiled. Hub now is written in Go and distributed in compiled form, would it be too difficult to just add Keychain support?

FWIW, I would be using 'hub' in a GitHub Enterprise environment where I must work with tokens, i.e. authenticating with user/pass directly with the GitHub server is never going to work.

@mislav

This comment has been minimized.

Show comment
Hide comment
@mislav

mislav Jan 4, 2017

Member

@FranciscoBorges I'm thinking of adding support for git credential helper, and then you could configure hub to use git-credential-osxkeychain (ships with git) to store credentials in macOS Keychain. Would that be an acceptable solution? #1217

Member

mislav commented Jan 4, 2017

@FranciscoBorges I'm thinking of adding support for git credential helper, and then you could configure hub to use git-credential-osxkeychain (ships with git) to store credentials in macOS Keychain. Would that be an acceptable solution? #1217

@supermarin

This comment has been minimized.

Show comment
Hide comment
@supermarin

supermarin Jan 5, 2017

That sounds good to me

That sounds good to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment