Merge pull request #147 from stopthatcow/feature/99_credential_storage

Update credential storage keys and support updating credentials when they fail

setup.py

@@ -62,8 +62,6 @@
                       'autopep8>=1.3.4',               # MIT
                       'docformatter>=1.0',             # Expat
                       'semantic_version>=2.6.0',       # BSD
-                      'gcovr>=3.4',                    # BSD
-                      'teamcity-messages>=1.21',       # Apache 2.0
                       'future>=0.16.0',                # MIT
                       'futures>=3.2.0',                # PSF
                       'inquirer>=2.2.0',               # MIT

tests/conftest.py

@@ -111,7 +111,7 @@ def git_repo_with_out_of_date_local_origin(git_repo):
 
 @contextlib.contextmanager
 def working_directory(path):
-    """Changes the working directory to the given path back to its previous value on exit"""
+    """Changes the working directory to the given path back to its previous value on exit."""
     prev_cwd = os.getcwd()
     os.chdir(path)
     try:
@@ -121,7 +121,7 @@ def working_directory(path):
 
 
 def dict_to_obj(dictionary):
-    """Creates a object from a dictionary"""
+    """Creates a object from a dictionary."""
     class Struct(object):
 
         def __init__(self, d):

tests/test_credential_helper.py

@@ -10,10 +10,10 @@
 def mock_get_pass(component, name):
     returns = {
         'component':
-        {
-            'component_user': 'user',
-            'component_password': 'password'
-        }
+            {
+                'username': 'user',
+                'password': 'password'
+            }
     }
     return returns[component][name]
 
@@ -33,8 +33,8 @@ def test_user_pass_credentials_prompt(mocker):
     mocker.patch('click.confirm', return_value=True)
     creds = zazu.credential_helper.get_user_pass_credentials('component', use_saved=False)
     assert creds == ('new_user', 'new_password')
-    zazu.util.prompt.assert_called_once_with('component username', expected_type=str)
-    click.prompt.assert_called_once_with('component password', type=str, hide_input=True)
+    zazu.util.prompt.assert_called_once_with('Username for component', expected_type=str)
+    click.prompt.assert_called_once_with('Password for new_user at component', type=str, hide_input=True)
     click.confirm.assert_called_once_with('Do you want to save these credentials?', default=True)
-    assert keyring.set_password.call_args_list[0][0] == ('component', 'component_user', 'new_user')
-    assert keyring.set_password.call_args_list[1][0] == ('component', 'component_password', 'new_password')
+    assert keyring.set_password.call_args_list[0][0] == ('component', 'username', 'new_user')
+    assert keyring.set_password.call_args_list[1][0] == ('component', 'password', 'new_password')

tests/test_github_code_reviewer.py

@@ -109,7 +109,6 @@ def test_github_issue_tracker(mocker):
     mocker.patch('zazu.github_helper.make_gh', return_value=MockGitHub())
     uut = zazu.plugins.github_code_reviewer.GitHubCodeReviewer.from_config({'owner': 'foo',
                                                                             'repo': 'bar'})
-    assert uut._base_url == 'https://github.com/foo/bar'
     assert uut._owner == 'foo'
     assert uut._repo == 'bar'
     uut.connect()

tests/test_github_helper.py

@@ -28,6 +28,22 @@ def test_make_gh_with_no_credentials(mocker):
     github.Github.assert_called_once_with('token')
 
 
+def test_make_gh_with_bad_token(mocker):
+    def side_effect(token):
+        if token == 'token':
+            raise github.BadCredentialsException('status', 'data')
+        return token
+    mocker.patch('keyring.get_password', return_value='token')
+    mocker.patch('keyring.set_password')
+    mocker.patch('zazu.github_helper.make_gh_token', return_value='token2')
+    mocker.patch('github.Github', side_effect=side_effect)
+    zazu.github_helper.make_gh()
+    calls = github.Github.call_args_list
+    assert github.Github.call_count == 2
+    assert calls[0] == mocker.call('token')
+    assert calls[1] == mocker.call('token2')
+
+
 class MockResponce(object):
 
     def __init__(self, status_code, json=None):

tests/test_github_issue_tracker.py

@@ -88,7 +88,6 @@ def test_from_config(git_repo):
                                                                                 'repo': 'zazu'})
         assert uut._owner == 'stopthatcow'
         assert uut._repo == 'zazu'
-        assert uut._base_url == 'https://github.com/stopthatcow/zazu'
         assert not uut.default_project()
         assert ['issue'] == uut.issue_types()
         assert [] == uut.issue_components()
@@ -99,7 +98,6 @@ def test_from_config_from_origin(repo_with_github_as_origin):
         uut = zazu.plugins.github_issue_tracker.GitHubIssueTracker.from_config({})
         assert uut._owner == 'stopthatcow'
         assert uut._repo == 'zazu'
-        assert uut._base_url == 'https://github.com/stopthatcow/zazu'
         assert not uut.default_project()
         assert ['issue'] == uut.issue_types()
         assert [] == uut.issue_components()

tests/test_jira_issue_tracker.py

@@ -52,17 +52,39 @@ def get_mock_issue_no_description(id):
     return mock_issue_no_description
 
 
+JIRA_ADDRESS = 'https://jira'
+
+
 def test_jira_issue_tracker(mocker):
     mocker.patch('zazu.credential_helper.get_user_pass_credentials', return_value=('user', 'pass'))
     mocker.patch('jira.JIRA', autospec=True)
-    uut = zazu.plugins.jira_issue_tracker.JiraIssueTracker('https://jira', 'ZZ', ['comp'])
+    uut = zazu.plugins.jira_issue_tracker.JiraIssueTracker(JIRA_ADDRESS, 'ZZ', ['comp'])
     uut.connect()
     assert uut.default_project() == 'ZZ'
     assert uut.issue_components() == ['comp']
     assert uut.issue_types() == ['Task', 'Bug', 'Story']
     assert uut.issue('ZZ-1')
 
 
+def test_jira_issue_tracker_bad_credentials(mocker):
+    mocker.patch('zazu.credential_helper.get_user_pass_credentials', side_effect=[('user', 'pass'), ('user', 'pass2')])
+    mocker.patch('jira.JIRA', autospec=True, side_effect=[jira.JIRAError(status_code=401), object()])
+    uut = zazu.plugins.jira_issue_tracker.JiraIssueTracker(JIRA_ADDRESS, 'ZZ', ['comp'])
+    uut.connect()
+    calls = zazu.credential_helper.get_user_pass_credentials.call_args_list
+    assert calls[0] == mocker.call(JIRA_ADDRESS, use_saved=True)
+    assert calls[1] == mocker.call(JIRA_ADDRESS, use_saved=False)
+
+
+def test_jira_issue_tracker_exception(mocker):
+    mocker.patch('zazu.credential_helper.get_user_pass_credentials', return_value=('user', 'pass'))
+    mocker.patch('jira.JIRA', autospec=True, side_effect=jira.JIRAError(status_code=400))
+    uut = zazu.plugins.jira_issue_tracker.JiraIssueTracker(JIRA_ADDRESS, 'ZZ', ['comp'])
+    with pytest.raises(zazu.issue_tracker.IssueTrackerError) as e:
+        uut.connect()
+        assert '400' in str(e.value)
+
+
 def test_jira_issue_tracker_no_description(mocker, mocked_jira_issue_tracker):
     mocked_jira_issue_tracker._jira_handle.issue = mocker.Mock(wraps=get_mock_issue_no_description)
     assert mocked_jira_issue_tracker.issue('ZZ-1').description == ''

zazu.yaml

@@ -25,6 +25,11 @@ style:
       - type: autopep8
         options:
           - "--max-line-length=150"
+      - type: docformatter
+        options:
+          - "--wrap-summaries=0"
+          - "--wrap-descriptions=0"
+          - "--blank"
       # Fix common misspellings.
       - type: generic
         command: sed

zazu/config.py

@@ -30,6 +30,7 @@ def __init__(self, name, subclass):
         Args:
             name (str): the name of the plugin type.
             subclass (type): subclasses of this type will be loaded as potential plugins.
+
         """
         self._subclass = subclass
         self._name = name
@@ -54,7 +55,7 @@ def from_config(self, config):
 code_reviewer_factory = PluginFactory('codeReviewer', zazu.code_reviewer.CodeReviewer)
 
 
-def scm_host_factory(config):
+def scm_host_factory(user_config, config):
     """Make and initialize the ScmHosts from the config."""
     hosts = {}
     default_host = ''
@@ -170,6 +171,31 @@ def user_config_filepath():
     return os.path.join(os.path.expanduser('~'), '.zazuconfig.yaml')
 
 
+class ConfigFile(object):
+    """Holds a parsed config file and can write changes to disk."""
+
+    def __init__(self, path):
+        """Store path and read the contents from disk if it exists."""
+        self._path = path
+        self.dict = {}
+        if self.exists():
+            self.read()
+
+    def exists(self):
+        """Return True if the path exists."""
+        return os.path.isfile(self._path)
+
+    def read(self):
+        """Read config file from disk."""
+        self.dict = load_yaml_file(self._path)
+
+    def write(self):
+        """Write config file to disk."""
+        yaml = ruamel.yaml.YAML()
+        with open(self._path, 'w') as f:
+            yaml.dump(self.dict, f)
+
+
 class Config(object):
     """Hold all zazu configuration info."""
 
@@ -188,7 +214,6 @@ def __init__(self, repo_root):
         self._project_config = None
         self._user_config = None
         self._stylers = None
-        self._tc = None
 
     def issue_tracker(self):
         """Lazily create a IssueTracker object."""
@@ -236,7 +261,7 @@ def scm_host_config(self):
     def scm_hosts(self):
         """Lazily create and return scm host list."""
         if self._scm_hosts is None:
-            self._scm_hosts, self._default_scm_host = scm_host_factory(self.scm_host_config())
+            self._scm_hosts, self._default_scm_host = scm_host_factory(self.user_config(), self.scm_host_config())
         return self._scm_hosts
 
     def default_scm_host(self):
@@ -275,11 +300,7 @@ def project_config(self):
     def user_config(self):
         """Parse and return the global zazu yaml configuration file."""
         if self._user_config is None:
-            user_config_path = user_config_filepath()
-            try:
-                self._user_config = load_yaml_file(user_config_path)
-            except IOError:
-                self._user_config = {}
+            self._user_config = ConfigFile(user_config_filepath()).dict
         return self._user_config
 
     def stylers(self):
@@ -289,13 +310,15 @@ def stylers(self):
         return self._stylers
 
     def develop_branch_name(self):
+        """Get the branch name for develop branch."""
         try:
             return self.project_config()['branches']['develop']
         except (click.ClickException, KeyError):
             pass
         return 'develop'
 
     def master_branch_name(self):
+        """Get the branch name for master branch."""
         try:
             return self.project_config()['branches']['master']
         except (click.ClickException, KeyError):
@@ -356,7 +379,9 @@ def config(ctx, list, add, unset, show_origin, param_name, param_value):
     user_config_path = user_config_filepath()
     maybe_write_default_user_config(user_config_path)
 
-    config_dict = load_yaml_file(user_config_path)
+    config_file = ConfigFile(user_config_path)
+    config_dict = config_file.dict
+
     write_config = False
 
     if list or show_origin:
@@ -387,6 +412,4 @@ def config(ctx, list, add, unset, show_origin, param_name, param_value):
 
     if write_config:
         # Update config file.
-        yaml = ruamel.yaml.YAML()
-        with open(user_config_path, 'w') as f:
-            yaml.dump(config_dict, f)
+        config_file.write()

zazu/credential_helper.py

@@ -10,21 +10,39 @@
 __copyright__ = 'Copyright 2016'
 
 
-def get_user_pass_credentials(component, use_saved=True):
+def get_user_pass_credentials(url, use_saved=True, offer_to_save=True):
     """Retrieve a stored user/password for a named component or offers to store a new set."""
-    keyring_user = component.lower() + '_user'
-    keyring_password = component.lower() + '_password'
-    user = None
+    username = None
     password = None
     if use_saved:
-        user = keyring.get_password(component, keyring_user)
-        password = keyring.get_password(component, keyring_password)
-    if user is None or password is None:
-        user = zazu.util.prompt('{} username'.format(component), expected_type=str)
-        password = click.prompt('{} password'.format(
-            component), type=str, hide_input=True)
-        if click.confirm('Do you want to save these credentials?', default=True):
-            keyring.set_password(component, keyring_user, user)
-            keyring.set_password(component, keyring_password, password)
+        username = get_user(url)
+        password = get_password(url)
+    if password is None or username is None:
+        username = zazu.util.prompt('Username for {}'.format(url), expected_type=str)
+        password = click.prompt('Password for {} at {}'.format(
+            username, url), type=str, hide_input=True)
+        if offer_to_save and click.confirm('Do you want to save these credentials?', default=True):
+            set_user(url, username)
+            set_password(url, password)
             click.echo('saved.')
-    return user, password
+    return username, password
+
+
+def get_user(url):
+    """Get the stored username for a given URL from the keychain."""
+    return keyring.get_password(url, 'username')
+
+
+def set_user(url, username):
+    """Store a username to the keychain for a given URL."""
+    return keyring.set_password(url, 'username', username)
+
+
+def get_password(url):
+    """Get the stored password for a given URL from the keychain."""
+    return keyring.get_password(url, 'password')
+
+
+def set_password(url, password):
+    """Store a password to the keychain for a given URL."""
+    return keyring.set_password(url, 'password', password)

zazu/dev/commands.py

@@ -26,6 +26,7 @@ def __init__(self, type, id, description=''):
             type (str): the issue type.
             id (str): the issue tracker id.
             description (str): a brief description (used for the branch name only).
+
         """
         self.type = type
         self.id = id

zazu/github_helper.py

@@ -14,9 +14,11 @@
 __copyright__ = 'Copyright 2016'
 
 
-def make_gh_token():
+GITHUB_API_URL = 'https://api.github.com'
+
+
+def make_gh_token(api_url=GITHUB_API_URL):
     """Make new GitHub token."""
-    api_url = 'https://api.github.com'
     add_auth = {
         'scopes': [
             'repo'
@@ -25,8 +27,7 @@ def make_gh_token():
     }
     token = None
     while token is None:
-        user = zazu.util.prompt('GitHub username', expected_type=str)
-        password = click.prompt('GitHub password', type=str, hide_input=True)
+        user, password = zazu.credential_helper.get_user_pass_credentials(api_url, offer_to_save=False)
         r = requests.post('{}/authorizations'.format(api_url), json=add_auth, auth=(user, password))
         if r.status_code == 401:
             if 'Must specify two-factor authentication OTP code.' in r.json()['message']:
@@ -46,15 +47,24 @@ def make_gh_token():
     return token
 
 
-def make_gh():
+def make_gh(api_url=GITHUB_API_URL):
     """Make github object with token from the keychain."""
     import keyring  # For some reason this doesn't play nicely with threads on lazy import.
-    token = keyring.get_password('https://api.github.com', 'token')
+    gh = None
+    token = keyring.get_password(api_url, 'token')
     if token is None:
         click.echo('No saved GitHub token found in keychain, lets add one...')
-        token = make_gh_token()
-        keyring.set_password('https://api.github.com', 'token', token)
-    gh = github.Github(token)
+    while gh is None:
+        try:
+            if token is None:
+                token = make_gh_token(api_url)
+                gh = github.Github(token)
+                keyring.set_password(api_url, 'token', token)
+            else:
+                gh = github.Github(token)
+        except github.BadCredentialsException:
+            click.echo("GitHub token rejected, you will need to create a new token.")
+            token = None
     return gh
 
 

zazu/plugins/autopep8_styler.py

@@ -21,5 +21,5 @@ def type():
 
     @staticmethod
     def required_options():
-        """Options required to make autopep8 take input from stdin."""
+        """Get options required to make autopep8 take input from stdin."""
         return ['-']

zazu/plugins/docformatter_styler.py

@@ -21,5 +21,5 @@ def type():
 
     @staticmethod
     def required_options():
-        """Options required to make docformatter use stdin."""
+        """Get options required to make docformatter use stdin."""
         return ['-']