Repository: pypa/twine
Pull request #208 (2016-08-29 14:29:51)

Resolve repository password from the keyring, allowing it to be store…
…d securely and following conventions use by setuptools.upload.

sigmavirus24: Hi @jaraco 

Could you include some tests here for the behaviour of the functions you've added/modified? Also, can you update our setup.{py,cfg} to add an optional dependency on Keyring based on an extra?


jaraco: Okay. Should be good to go now.

Patch:
diff --git a/docs/changelog.rst b/docs/changelog.rst
index 67284ce..4b70310 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -4,6 +4,12 @@
 Changelog
 =========
 
+* :release:`1.9.0 <...>`
+
+  * Twine will now resolve passwords using the
+    `keyring <https://pypi.org/projects/keyring>`_ if available.
+    Module can be required with the ``keyring`` extra.
+
 * :release:`1.8.1 <2016-08-09>`
 
   * Check if a package exists if the URL is one of:
diff --git a/setup.cfg b/setup.cfg
index e2da31c..c8ad19e 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -15,3 +15,4 @@ requires-dist =
     setuptools >= 0.7.0
     argparse; python_version == '2.6'
     pyblake2; extra == 'with-blake2'
+    keyring; extra == 'keyring'
diff --git a/setup.py b/setup.py
index b431994..bc4a7f3 100644
--- a/setup.py
+++ b/setup.py
@@ -81,6 +81,9 @@ setup(
     extras_require={
         'with-blake2': [
             'pyblake2',
-        ]
+        ],
+        'keyring': [
+            'keyring',
+        ],
     },
 )
diff --git a/twine/commands/upload.py b/twine/commands/upload.py
index ca0be08..3326296 100644
--- a/twine/commands/upload.py
+++ b/twine/commands/upload.py
@@ -99,7 +99,9 @@ def upload(dists, repository, sign, identity, username, password, comment,
     print("Uploading distributions to {0}".format(config["repository"]))
 
     username = utils.get_username(username, config)
-    password = utils.get_password(password, config)
+    password = utils.get_password(
+        config["repository"], username, password, config,
+    )
     ca_cert = utils.get_cacert(cert, config)
     client_cert = utils.get_clientcert(client_cert, config)
 
diff --git a/twine/utils.py b/twine/utils.py
index b0a355b..f70c3e4 100644
--- a/twine/utils.py
+++ b/twine/utils.py
@@ -184,18 +184,30 @@ def password_prompt(prompt_text):  # Always expects unicode for our own sanity
     # Workaround for https://github.com/pypa/twine/issues/116
     if os.name == 'nt' and sys.version_info < (3, 0):
         prompt = prompt_text.encode('utf8')
-    return functools.partial(getpass.getpass, prompt=prompt)
+    return getpass.getpass(prompt)
+
+
+def get_password_from_keyring(system, username):
+    try:
+        import keyring
+    except ImportError:
+        return
+
+    return keyring.get_password(system, username)
+
+
+def password_from_keyring_or_prompt(system, username):
+    return (
+        get_password_from_keyring(system, username)
+        or password_prompt('Enter your password: ')
+    )
+
 
 get_username = functools.partial(
     get_userpass_value,
     key='username',
     prompt_strategy=functools.partial(input_func, 'Enter your username: '),
 )
-get_password = functools.partial(
-    get_userpass_value,
-    key='password',
-    prompt_strategy=password_prompt('Enter your password: '),
-)
 get_cacert = functools.partial(
     get_userpass_value,
     key='ca_cert',
@@ -222,3 +234,16 @@ class EnvironmentDefault(argparse.Action):
 
     def __call__(self, parser, namespace, values, option_string=None):
         setattr(namespace, self.dest, values)
+
+
+def get_password(system, username, cli_value, config):
+    return get_userpass_value(
+        cli_value,
+        config,
+        key='password',
+        prompt_strategy=functools.partial(
+            password_from_keyring_or_prompt,
+            system,
+            username,
+        ),
+    )


Test patch:
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 4c4d116..c7d8214 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -14,9 +14,15 @@
 from __future__ import absolute_import, division, print_function
 from __future__ import unicode_literals
 
+import sys
 import os.path
 import textwrap
 
+try:
+    import builtins
+except ImportError:
+    import __builtin__ as builtins
+
 import pytest
 
 from twine import utils
@@ -172,3 +178,50 @@ def test_default_to_environment_action(env_name, default, environ, expected):
         )
     assert action.env == env_name
     assert action.default == expected
+
+
+def test_get_password_keyring_overrides_prompt(monkeypatch):
+    class MockKeyring:
+        @staticmethod
+        def get_password(system, user):
+            return '{user}@{system} sekure pa55word'.format(**locals())
+
+    monkeypatch.setitem(sys.modules, 'keyring', MockKeyring)
+
+    pw = utils.get_password('system', 'user', None, {})
+    assert pw == 'user@system sekure pa55word'
+
+
+def test_get_password_keyring_defers_to_prompt(monkeypatch):
+    monkeypatch.setattr(utils, 'password_prompt', lambda prompt: 'entered pw')
+
+    class MockKeyring:
+        @staticmethod
+        def get_password(system, user):
+            return
+
+    monkeypatch.setitem(sys.modules, 'keyring', MockKeyring)
+
+    pw = utils.get_password('system', 'user', None, {})
+    assert pw == 'entered pw'
+
+
+@pytest.fixture
+def keyring_missing(monkeypatch):
+    """
+    Simulate that 'import keyring' raises an ImportError
+    """
+    real_import = builtins.__import__
+
+    def my_import(name, *args, **kwargs):
+        if name == 'keyring':
+            raise ImportError
+        return real_import(name, *args, **kwargs)
+    monkeypatch.setattr(builtins, '__import__', my_import)
+
+
+def test_get_password_keyring_missing_prompts(monkeypatch, keyring_missing):
+    monkeypatch.setattr(utils, 'password_prompt', lambda prompt: 'entered pw')
+
+    pw = utils.get_password('system', 'user', None, {})
+    assert pw == 'entered pw'