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

Don't store authentication token in ~/.config #191

Merged
merged 1 commit into from
Feb 4, 2014
Merged

Conversation

owenthereal
Copy link
Owner

Please don't make the same mistake as hub does.
~/.config folder is used by fish, cocoapods and many other tools.

It's a folder intended to store configuration files.
That means people are checking it in with their dotfiles repos.

Use the OS-level keychains, or require an ENV.

@calavera
Copy link
Contributor

calavera commented Feb 4, 2014

It's really something to consider, thanks @mneorr.

I guess storing the credentials in a keychain would be the optimal solution. We can abstract that logic into specific sources by GOARCH, we can have keychain_win.go, keychain_darwin.go and keychain_linux.go.

We're trying to be totally compatible with hub, which might require some kind of transformation from its configuration file to something like this.

@owenthereal
Copy link
Owner

@calavera I really like the idea of using native mechanism for storing credentials! And it's good use of Go build tag for this purpose 👍.

@mneorr You could specify the path to the config file by using the GH_CONFIG env var. This should solve your problem for now. I'll also document this on the README. I'm also thinking it may be a good idea to use the ~/.gh folder to store any gh related settings. This should have less conflict with any other tools.

@owenthereal owenthereal merged commit 2250509 into master Feb 4, 2014
@owenthereal owenthereal deleted the gh_config branch February 4, 2014 17:03
@supermarin
Copy link
Author

@jingweno i'm sad seeing this being closed, because documenting isn't a solution.
As long as default behavior sets the token in ~/.config, expect many committed tokens and exploits.

@owenthereal
Copy link
Owner

@mneorr I didn't close it. I merged the branch and github did that...

@owenthereal owenthereal restored the gh_config branch February 4, 2014 17:15
@owenthereal owenthereal deleted the gh_config branch February 4, 2014 17:15
@owenthereal
Copy link
Owner

Looks like with the new UI, i can't reopen an issue either...

@owenthereal
Copy link
Owner

screen shot 2014-02-04 at 9 23 07 am

This is all I got...no "reopen" button

@calavera
Copy link
Contributor

calavera commented Feb 4, 2014

it's because you turned the issue into a PR. PRs cannot be reopen.

@calavera
Copy link
Contributor

calavera commented Feb 4, 2014

We should definitely keep this open until we provide something else as a solution.

@owenthereal
Copy link
Owner

@calavera @mneorr Please move onto #192 for further discussion. Sorry for the inconvenience...

@owenthereal
Copy link
Owner

There's apparently something we could do to minimize misoperation like this: #193

@mislav
Copy link
Collaborator

mislav commented Feb 4, 2014

@mneorr We're accepting suggestions for better places to store the OAuth token in a cross platform way; OS X Keychain is cool but we unfortunately don't have it on Linux or Windows.

Credentials via environment variables is theoretically a good solution but in practice it's a nightmare to configure environment variables consistently for different kinds of execution contexts.

@supermarin
Copy link
Author

@mislav you can at least store it in .netrc instead of ~/.config/.

There are projects like GPG that could probably work on other platforms.
Not sure how helpful this is, but seems like there's some support on git level
http://stackoverflow.com/questions/6031214/git-how-to-use-netrc-file-on-windows-to-save-user-and-password

@wilmoore
Copy link
Contributor

Seems like the lowest common denominator is looking to environment variables:

  1. Each platform supports them.
  2. If a user wants to use a platform native solution like Keychain Access (mac) or Windows Registry, they have the option to export whatever is in those stores to an environment variable using command-line tools.

I don't love the idea that by default the tool writes sensitive information directly into a plain text file, even though the location can be configured.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants