Skip to content

Commit

Permalink
Only store password in config for GitHub Enterprise (due to Enterpris…
Browse files Browse the repository at this point in the history
…e limitations) (#79)

* stops storing passwords in cleartext in config
  • Loading branch information
nttibbetts authored and donnemartin committed Nov 12, 2016
1 parent 4661f56 commit e156d08
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 10 deletions.
24 changes: 16 additions & 8 deletions gitsome/config.py
Expand Up @@ -91,6 +91,8 @@ class Config(object):
:type user_pass: str
:param user_pass: The user's pass in ~/.gitsomeconfig.
This is only stored for GitHub Enterprise users since using only a
personal access token does not seem to be supported.
:type user_token: str
:param user_token: The user's token in ~/.gitsomeconfig.
Expand Down Expand Up @@ -285,22 +287,25 @@ def authenticate(self, enterprise=False,
if click.confirm(('Do you want to log in with a password [Y] or '
'a personal access token [n]?'),
default=True):
while not self.user_pass:
self.user_pass = self.getpass('Password: ')
login_kwargs.update({'password': self.user_pass})
user_pass = None
while not user_pass:
user_pass = self.getpass('Password: ')
login_kwargs.update({'password': user_pass})
try:
if not enterprise:
# Trade the user password for a personal access token.
# This does not seem to be available for Enterprise.
auth = self.authorize(
self.user_login,
self.user_pass,
user_pass,
scopes=['user', 'repo'],
note='gitsome',
note_url='https://github.com/donnemartin/gitsome',
two_factor_callback=self.request_two_factor_code
)
self.user_token = auth.token
else:
self.user_pass = user_pass
except (UnprocessableEntity, AuthenticationFailed):
click.secho('Error creating token.',
fg=self.clr_error)
Expand Down Expand Up @@ -616,10 +621,6 @@ def save_config(self):
parser.set(self.CONFIG_SECTION,
self.CONFIG_USER_LOGIN,
self.user_login)
if self.user_pass is not None:
parser.set(self.CONFIG_SECTION,
self.CONFIG_USER_PASS,
self.user_pass)
if self.user_token is not None:
parser.set(self.CONFIG_SECTION,
self.CONFIG_USER_TOKEN,
Expand All @@ -632,6 +633,13 @@ def save_config(self):
parser.set(self.CONFIG_SECTION,
self.CONFIG_ENTERPRISE_URL,
self.enterprise_url)
if self.user_pass is not None:
parser.set(self.CONFIG_SECTION,
self.CONFIG_USER_PASS,
self.user_pass)
else:
parser.remove_option(self.CONFIG_SECTION,
self.CONFIG_USER_PASS)
parser.set(self.CONFIG_SECTION,
self.CONFIG_VERIFY_SSL,
self.verify_ssl)
Expand Down
21 changes: 19 additions & 2 deletions tests/test_config.py
Expand Up @@ -49,6 +49,7 @@ def verify_login_pass(self, username=None, password=None,
verify=True):
assert username is not None
assert password is not None
assert password
assert two_factor_callback is not None
assert verify

Expand Down Expand Up @@ -104,6 +105,20 @@ def test_authenticate_cached_credentials_token(self):
assert self.github.config.user_login == 'foo'
assert self.github.config.user_token == 'bar'

def test_authenticate_cached_credentials_pass(self):
self.github.config.user_login = 'foo'
self.github.config.user_pass = 'bar'
self.github.config.save_config()
self.github.config.user_login = ''
self.github.config.user_pass = ''
self.github.config.api = None
config = self.github.config.get_github_config_path(
self.github.config.CONFIG)
parser = configparser.RawConfigParser()
self.github.config.authenticate_cached_credentials(config, parser)
assert self.github.config.user_login == 'foo'
assert self.github.config.user_pass is None

def test_authenticate_cached_credentials_token_enterprise(self):
self.github.config.user_login = 'foo'
self.github.config.user_token = 'bar'
Expand Down Expand Up @@ -159,14 +174,15 @@ def test_authenticate_token(self, mock_auth, mock_click_secho):
@mock.patch('gitsome.github.click.secho')
@mock.patch('gitsome.config.Config.authenticate_cached_credentials')
def test_authenticate_pass(self, mock_auth, mock_click_secho):
self.github.config.getpass.return_value = 'bar'
with mock.patch('click.confirm', return_value=True):
with mock.patch('builtins.input', return_value='foo'):
self.github.config.login = self.verify_login_pass
self.github.config.user_login = 'foo'
self.github.config.user_pass = 'bar'
self.github.config.authenticate(
enterprise=False,
overwrite=True)
assert self.github.config.user_pass is None

@mock.patch('gitsome.github.click.secho')
@mock.patch('gitsome.config.Config.authenticate_cached_credentials')
Expand All @@ -185,14 +201,15 @@ def test_authenticate_enterprise_token(self, mock_auth, mock_click_secho):
@mock.patch('gitsome.github.click.secho')
@mock.patch('gitsome.config.Config.authenticate_cached_credentials')
def test_authenticate_enterprise_pass(self, mock_auth, mock_click_secho):
self.github.config.getpass.return_value = 'bar'
with mock.patch('click.confirm', return_value=True):
with mock.patch('builtins.input', return_value='foo'):
self.github.config.user_login = 'foo'
self.github.config.user_pass = 'bar'
self.github.config.authenticate(
enterprise=True,
enterprise_auth=self.verify_login_pass_url_enterprise,
overwrite=True)
assert self.github.config.user_pass is not None

@mock.patch('gitsome.github.click.secho')
def test_check_auth_error(self, mock_click_secho):
Expand Down

0 comments on commit e156d08

Please sign in to comment.