New password fetching

Fix #233

CHANGELOG.rst

@@ -9,11 +9,13 @@ Package maintainers and users who have to manually update their installation
 may want to subscribe to `GitHub's tag feed
 <https://github.com/untitaker/vdirsyncer/tags.atom>`_.
 
-Version 0.6.1
+Version 0.7.0
 =============
 
 - **Packagers:** New dependencies are ``click_threading``, ``click_log`` and
   ``click>=5.0``.
+- ``password_command`` is gone. Keyring support got completely overhauled. See
+  :ref:`keyring`.
 
 Version 0.6.0
 =============

docs/keyring.rst

@@ -1,54 +1,55 @@
-===============
-Keyring Support
-===============
+=================
+Storing passwords
+=================
 
-Vdirsyncer will try the following storages in that order if no password (but a
-username) is set in your config. If all of those methods fail, it will prompt
-for the password and store the password in the system keyring (if possible and
-wished).
+.. versionchanged:: 0.7.0
 
-Custom command
-==============
+   Password configuration got completely overhauled.
 
-.. versionadded:: 0.3.0
+Vdirsyncer can fetch passwords from a custom command or your system keyring if
+the keyring_ Python package is installed.
 
-A custom command/binary can be specified to retrieve the password for a
-username/hostname combination. See :ref:`general_config`.
+Command
+=======
 
-.. versionchanged:: 0.6.0
+Say you have the following configuration::
 
-    Setting a custom command now disables all other methods.
+    [storage foo]
+    type = caldav
+    url = ...
+    username = foo
+    password = bar
 
-netrc
-=====
+But it bugs you that the password is stored in cleartext in the config file.
+You can do this::
 
-Vdirsyncer can use ``~/.netrc`` for retrieving a password. An example
-``.netrc`` looks like this::
+    [storage foo]
+    type = caldav
+    url = ...
+    username = foo
+    password.fetch = ["command", "~/get-password.sh", "more", "args"]
 
-    machine owncloud.example.com
-    login foouser
-    password foopass
+You can fetch the username as well::
+
+    [storage foo]
+    type = caldav
+    url = ...
+    username.fetch = ["command", "~/get-username.sh"]
+    password.fetch = ["command", "~/get-password.sh"]
+
+Or really any kind of parameter in a storage section.
 
 System Keyring
 ==============
 
-Vdirsyncer can use your system's password storage, utilizing the keyring_
-library. Supported services include **OS X Keychain, Gnome Keyring, KDE Kwallet
-or the Windows Credential Vault**. For a full list see the library's
-documentation.
+While the command approach is quite flexible, it is often cumbersome to write a
+script fetching the system keyring.
 
-To use it, you must install the ``keyring`` Python package.
+Given that you have the keyring_ Python library installed, you can use::
 
-.. _keyring: https://bitbucket.org/kang/python-keyring-lib
+    [storage foo]
+    type = caldav
+    username = myusername
+    password.fetch = ["keyring", "myservicename", "myusername"]
 
-Storing the password
---------------------
-
-Vdirsyncer will use the hostname as key prefixed with ``vdirsyncer:``, e.g.
-``vdirsyncer:owncloud.example.com``.
-
-Changing the Password
----------------------
-
-If your password on the server changed or you misspelled it, you need to
-manually edit or delete the entry in your system keyring.
+.. _keyring: https://pypi.python.org/pypi/keyring

tests/cli/test_fetchparams.py

@@ -0,0 +1,54 @@
+# -*- coding: utf-8 -*-
+
+from textwrap import dedent
+
+import pytest
+
+
+class EmptyKeyring(object):
+    def get_password(self, *a, **kw):
+        return None
+
+
+@pytest.fixture(autouse=True)
+def empty_password_storages(monkeypatch):
+    monkeypatch.setattr('vdirsyncer.cli.fetchparams.keyring', EmptyKeyring())
+
+
+def test_get_password_from_command(tmpdir, runner):
+    runner.write_with_general(dedent('''
+        [pair foobar]
+        a = foo
+        b = bar
+        collections = ["a", "b", "c"]
+
+        [storage foo]
+        type = filesystem
+        path = {base}/foo/
+        fileext.fetch = ["command", "echo", ".txt"]
+
+        [storage bar]
+        type = filesystem
+        path = {base}/bar/
+        fileext.fetch = ["command", "echo", ".asdf"]
+    '''.format(base=str(tmpdir))))
+
+    foo = tmpdir.ensure('foo', dir=True)
+    foo.ensure('a', dir=True)
+    foo.ensure('b', dir=True)
+    foo.ensure('c', dir=True)
+    bar = tmpdir.ensure('bar', dir=True)
+    bar.ensure('a', dir=True)
+    bar.ensure('b', dir=True)
+    bar.ensure('c', dir=True)
+
+    result = runner.invoke(['discover'])
+    assert not result.exception
+    status = tmpdir.join('status').join('foobar.collections').read()
+    assert 'foo' in status
+    assert 'bar' in status
+    assert 'asdf' not in status
+    assert 'txt' not in status
+
+    result = runner.invoke(['sync'])
+    assert not result.exception

tests/utils/test_main.py

@@ -22,31 +22,11 @@ from vdirsyncer.cli.config import Config
 # imported
 import vdirsyncer.utils.compat  # noqa
 import vdirsyncer.utils.http  # noqa
-import vdirsyncer.utils.password  # noqa
 
 
 from .. import blow_up
 
 
-class EmptyNetrc(object):
-    def __init__(self, file=None):
-        self._file = file
-
-    def authenticators(self, hostname):
-        return None
-
-
-class EmptyKeyring(object):
-    def get_password(self, *a, **kw):
-        return None
-
-
-@pytest.fixture(autouse=True)
-def empty_password_storages(monkeypatch):
-    monkeypatch.setattr('netrc.netrc', EmptyNetrc)
-    monkeypatch.setattr(utils.password, 'keyring', EmptyKeyring())
-
-
 @pytest.fixture(autouse=True)
 def no_debug_output(request):
     logger = click_log.basic_config('vdirsyncer')
@@ -59,153 +39,6 @@ def no_debug_output(request):
     request.addfinalizer(teardown)
 
 
-def test_get_password_from_netrc(monkeypatch):
-    username = 'foouser'
-    password = 'foopass'
-    resource = 'http://example.com/path/to/whatever/'
-    hostname = 'example.com'
-
-    calls = []
-
-    class Netrc(object):
-        def authenticators(self, hostname):
-            calls.append(hostname)
-            return username, 'bogus', password
-
-    monkeypatch.setattr('netrc.netrc', Netrc)
-    monkeypatch.setattr('getpass.getpass', blow_up)
-
-    _password = utils.password.get_password(username, resource)
-    assert _password == password
-    assert calls == [hostname]
-
-
-def test_get_password_from_system_keyring(monkeypatch):
-    username = 'foouser'
-    password = 'foopass'
-    resource = 'http://example.com/path/to/whatever/'
-    hostname = 'example.com'
-
-    class KeyringMock(object):
-        def get_password(self, resource, _username):
-            assert _username == username
-            assert resource == utils.password.password_key_prefix + hostname
-            return password
-
-    monkeypatch.setattr(utils.password, 'keyring', KeyringMock())
-
-    monkeypatch.setattr('getpass.getpass', blow_up)
-
-    _password = utils.password.get_password(username, resource)
-    assert _password == password
-
-
-def test_get_password_from_command(tmpdir):
-    username = 'my_username'
-    resource = 'http://example.com'
-    password = 'testpassword'
-    filename = 'command.sh'
-
-    filepath = str(tmpdir) + '/' + filename
-    f = open(filepath, 'w')
-    f.write('#!/bin/sh\n'
-            '[ "$1" != "my_username" ] && exit 1\n'
-            '[ "$2" != "example.com" ] && exit 1\n'
-            'echo "{}"'.format(password))
-    f.close()
-
-    st = os.stat(filepath)
-    os.chmod(filepath, st.st_mode | stat.S_IEXEC)
-
-    @click.command()
-    @pass_context
-    def fake_app(ctx):
-        ctx.config = Config({'password_command': filepath}, {}, {})
-        _password = utils.password.get_password(username, resource)
-        assert _password == password
-
-    runner = CliRunner()
-    result = runner.invoke(fake_app)
-    assert not result.exception
-
-
-def test_get_password_from_prompt():
-    user = 'my_user'
-    resource = 'http://example.com'
-
-    @click.command()
-    def fake_app():
-        x = utils.password.get_password(user, resource)
-        click.echo('Password is {}'.format(x))
-
-    runner = CliRunner()
-    result = runner.invoke(fake_app, input='my_password\n\n')
-    assert not result.exception
-    assert result.output.splitlines() == [
-        'Server password for my_user at host example.com: ',
-        'Save this password in the keyring? [y/N]: ',
-        'Password is my_password',
-    ]
-
-
-def test_set_keyring_password(monkeypatch):
-    class KeyringMock(object):
-        def get_password(self, resource, username):
-            assert resource == \
-                utils.password.password_key_prefix + 'example.com'
-            assert username == 'foouser'
-            return None
-
-        def set_password(self, resource, username, password):
-            assert resource == \
-                utils.password.password_key_prefix + 'example.com'
-            assert username == 'foouser'
-            assert password == 'hunter2'
-
-    monkeypatch.setattr(utils.password, 'keyring', KeyringMock())
-
-    @click.command()
-    @pass_context
-    def fake_app(ctx):
-        x = utils.password.get_password('foouser', 'http://example.com/a/b')
-        click.echo('password is ' + x)
-
-    runner = CliRunner()
-    result = runner.invoke(fake_app, input='hunter2\ny\n')
-    assert not result.exception
-    assert result.output == (
-        'Server password for foouser at host example.com: \n'
-        'Save this password in the keyring? [y/N]: y\n'
-        'password is hunter2\n'
-    )
-
-
-def test_get_password_from_cache(monkeypatch):
-    user = 'my_user'
-    resource = 'http://example.com'
-
-    @click.command()
-    @pass_context
-    def fake_app(ctx):
-        x = utils.password.get_password(user, resource)
-        click.echo('Password is {}'.format(x))
-        monkeypatch.setattr(click, 'prompt', blow_up)
-
-        assert (user, 'example.com') in ctx.passwords
-        x = utils.password.get_password(user, resource)
-        click.echo('Password is {}'.format(x))
-
-    runner = CliRunner()
-    result = runner.invoke(fake_app, input='my_password\n')
-    assert not result.exception
-    assert result.output.splitlines() == [
-        'Server password for {} at host {}: '.format(user, 'example.com'),
-        'Save this password in the keyring? [y/N]: ',
-        'Password is my_password',
-        'Password is my_password'
-    ]
-
-
 def test_get_class_init_args():
     class Foobar(object):
         def __init__(self, foo, bar, baz=None):

vdirsyncer/cli/__init__.py

@@ -16,7 +16,7 @@ cli_logger = log.get(__name__)
 class AppContext(object):
     def __init__(self):
         self.config = None
-        self.passwords = {}
+        self.fetched_params = {}
         self.logger = None
 
 

vdirsyncer/cli/config.py

@@ -4,8 +4,9 @@ import string
 from itertools import chain
 
 from . import CliError, cli_logger
+from .fetchparams import expand_fetch_params
 from .. import PROJECT_HOME
-from ..utils import expand_path
+from ..utils import expand_path, cached_property
 from ..utils.compat import text_type
 
 try:
@@ -13,7 +14,7 @@ try:
 except ImportError:
     from configparser import RawConfigParser
 
-GENERAL_ALL = frozenset(['status_path', 'password_command'])
+GENERAL_ALL = frozenset(['status_path'])
 GENERAL_REQUIRED = frozenset(['status_path'])
 SECTION_NAME_CHARS = frozenset(chain(string.ascii_letters, string.digits, '_'))
 
@@ -206,8 +207,16 @@ class PairConfig(object):
         self.name_a = name_b
         self.options = pair_options
 
-        self.config_a = config.get_storage_args(name_a, pair_name=name)
-        self.config_b = config.get_storage_args(name_b, pair_name=name)
+        self.raw_config_a = config.get_storage_args(name_a, pair_name=name)
+        self.raw_config_b = config.get_storage_args(name_b, pair_name=name)
+
+    @cached_property
+    def config_a(self):
+        return expand_fetch_params(self.raw_config_a)
+
+    @cached_property
+    def config_b(self):
+        return expand_fetch_params(self.raw_config_b)
 
 
 class CollectionConfig(object):

vdirsyncer/cli/fetchparams.py

@@ -0,0 +1,90 @@
+# -*- coding: utf-8 -*-
+
+import click
+
+from . import AppContext
+from .. import exceptions, log
+from ..utils import expand_path
+
+SUFFIX = '.fetch'
+
+logger = log.get(__name__)
+
+try:
+    import keyring
+except ImportError:
+    keyring = None
+
+
+def expand_fetch_params(config):
+    config = dict(config)
+    for key in list(config):
+        if not key.endswith(SUFFIX):
+            continue
+
+        newkey = key[:-len(SUFFIX)]
+        if newkey in config:
+            raise ValueError('Can\'t set {} and {}.'.format(key, newkey))
+        config[newkey] = _fetch_value(config[key], key)
+        del config[key]
+
+    return config
+
+
+def _fetch_value(opts, key):
+    if not isinstance(opts, list):
+        raise ValueError('Invalid value for {}: Expected a list, found {!r}.'
+                         .format(key, opts))
+    if not opts:
+        raise ValueError('Expected list of length > 0.')
+
+    try:
+        ctx = click.get_current_context().find_object(AppContext)
+        if ctx is None:
+            raise RuntimeError()
+        password_cache = ctx.fetched_params
+    except RuntimeError:
+        password_cache = {}
+
+    cache_key = tuple(opts)
+    if cache_key in password_cache:
+        rv = password_cache[cache_key]
+        logger.debug('Found cached value for {!r}.'.format(opts))
+        if isinstance(rv, BaseException):
+            raise rv
+        return rv
+
+    strategy = opts[0]
+    logger.debug('Fetching value for {} with {} strategy.'
+                 .format(key, strategy))
+    try:
+        rv = STRATEGIES[strategy](*opts[1:])
+    except (click.Abort, KeyboardInterrupt) as e:
+        password_cache[cache_key] = e
+        raise
+    else:
+        password_cache[cache_key] = rv
+        return rv
+
+
+def _strategy_keyring(username, host):
+    if not keyring:
+        raise RuntimeError('Keyring package not available.')
+    return keyring.get_password(username, host)
+
+
+def _strategy_command(*command):
+    import subprocess
+    command = (expand_path(command[0]),) + command[1:]
+    try:
+        stdout = subprocess.check_output(command, universal_newlines=True)
+        return stdout.strip('\n')
+    except OSError as e:
+        raise exceptions.UserError('Failed to execute command: {}\n{}'
+                                   .format(' '.join(command), str(e)))
+
+
+STRATEGIES = {
+    'keyring': _strategy_keyring,
+    'command': _strategy_command
+}

vdirsyncer/storage/dav.py

@@ -299,10 +299,6 @@ class DavSession(object):
     def __init__(self, url, username='', password='', verify=True, auth=None,
                  useragent=USERAGENT, verify_fingerprint=None,
                  auth_cert=None):
-        if username and not password:
-            from ..utils.password import get_password
-            password = get_password(username, url)
-
         self._settings = {
             'auth': prepare_auth(auth, username, password),
             'cert': prepare_client_cert(auth_cert),

vdirsyncer/storage/filesystem.py

@@ -63,6 +63,7 @@ class FilesystemStorage(Storage):
             for collection in collections:
                 collection_path = os.path.join(path, collection)
                 if os.path.isdir(collection_path):
+                    print("COLLECTION", collection_path)
                     args = dict(collection=collection, path=collection_path,
                                 **kwargs)
                     yield args

vdirsyncer/storage/http.py

@@ -5,7 +5,6 @@ from .. import exceptions
 from ..utils import expand_path
 from ..utils.compat import iteritems, text_type, urlparse
 from ..utils.http import request
-from ..utils.password import get_password
 from ..utils.vobject import split_collection
 
 USERAGENT = 'vdirsyncer'
@@ -123,9 +122,6 @@ class HttpStorage(Storage):
                  **kwargs):
         super(HttpStorage, self).__init__(**kwargs)
 
-        if username and not password:
-            password = get_password(username, url)
-
         self._settings = {
             'auth': prepare_auth(auth, username, password),
             'cert': prepare_client_cert(auth_cert),

vdirsyncer/utils/password.py

@@ -1,143 +0,0 @@
-# -*- coding: utf-8 -*-
-
-import threading
-
-import click
-
-from . import expand_path
-from .compat import urlparse
-from .. import exceptions, log
-from ..cli import AppContext
-
-logger = log.get(__name__)
-password_key_prefix = 'vdirsyncer:'
-
-try:
-    import keyring
-except ImportError:
-    keyring = None
-
-
-def get_password(username, resource, _lock=threading.Lock()):
-    """tries to access saved password or asks user for it
-
-    will try the following in this order:
-        1. read password from netrc (and only the password, username
-           in netrc will be ignored)
-        2. read password from keyring (keyring needs to be installed)
-        3. read password from the command passed as password_command in the
-           general config section with username and host as parameters
-        4a ask user for the password
-         b save in keyring if installed and user agrees
-
-    :param username: user's name on the server
-    :type username: str/unicode
-    :param resource: a resource to which the user has access via password,
-                     it will be shortened to just the hostname. It is assumed
-                     that each unique username/hostname combination only ever
-                     uses the same password.
-    :type resource: str/unicode
-    :return: password
-    :rtype: str/unicode
-
-
-    """
-    # If no app is running, Click will automatically create an empty cache for
-    # us and discard it.
-    try:
-        ctx = click.get_current_context().find_object(AppContext)
-        if ctx is None:
-            raise RuntimeError()
-        password_cache = ctx.passwords
-    except RuntimeError:
-        password_cache = {}
-
-    def _password_from_cache(username, host):
-        '''internal cache'''
-        rv = password_cache.get((username, host), None)
-        if isinstance(rv, BaseException):
-            raise rv
-        return rv
-
-    with _lock:
-        try:
-            host = urlparse.urlsplit(resource).hostname
-            for func in (_password_from_cache, _password_from_command,
-                         _password_from_netrc, _password_from_keyring,
-                         _password_from_prompt):
-                password = func(username, host)
-                if password is not None:
-                    logger.debug('Got password for {} from {}'
-                                 .format(username, func.__doc__))
-                    break
-        except (click.Abort, KeyboardInterrupt) as e:
-            password_cache[(username, host)] = e
-            raise
-        else:
-            password_cache[(username, host)] = password
-            return password
-
-
-def _password_from_prompt(username, host):
-    '''prompt'''
-    prompt = ('Server password for {} at host {}'.format(username, host))
-    password = click.prompt(prompt, hide_input=True)
-    if keyring is not None and \
-       click.confirm('Save this password in the keyring?',
-                     default=False):
-        keyring.set_password(password_key_prefix + host,
-                             username, password)
-    return password
-
-
-def _password_from_netrc(username, host):
-    '''.netrc'''
-    from netrc import netrc
-
-    try:
-        netrc_user, account, password = \
-            netrc().authenticators(host) or (None, None, None)
-        if netrc_user == username:
-            return password
-    except IOError:
-        pass
-
-
-def _password_from_keyring(username, host):
-    '''system keyring'''
-    if keyring is None:
-        return None
-
-    return keyring.get_password(password_key_prefix + host, username)
-
-
-def _password_from_command(username, host):
-    '''command'''
-    import subprocess
-
-    try:
-        ctx = click.get_current_context()
-    except RuntimeError:
-        return None
-
-    ctx = ctx.find_object(AppContext)
-    if ctx is None or not ctx.config:
-        return None
-
-    try:
-        command = ctx.config.general['password_command'].split()
-    except KeyError:
-        return None
-
-    if not command:
-        return None
-
-    command[0] = expand_path(command[0])
-
-    try:
-        stdout = subprocess.check_output(command + [username, host],
-                                         universal_newlines=True)
-        return stdout.strip('\n')
-    except OSError as e:
-        raise exceptions.UserError('Failed to execute command: {}\n{}'.
-                                   format(' '.join(command), str(e)))