Rewrite get_password

Only fetching by hostname, no bruteforce algorithm for system keyring

docs/keyring.rst

@@ -26,14 +26,8 @@ To use it, you must install keyring_.
 
 .. _keyring: https://bitbucket.org/kang/python-keyring-lib
 
-*vdirsyncer* will use the full resource URL as the key when saving.
+*vdirsyncer* will use the hostname as key prefixed with ``vdirsyncer:`` when
-
+saving and fetching, e.g. ``vdirsyncer:owncloud.example.com``.
-When retrieving the key, it will try to remove segments of the URL's path until
-it finds a password. For example, if you save a password under the key
-``vdirsyncer:http://example.com``, it will be used as a fallback for all
-resources on ``example.com``. If you additionally save a password under the key
-``vdirsyncer:http://example.com/special/``, that password will be used for all
-resources on ``example.com`` whose path starts with ``/special/``.
 
 *keyring* support these keyrings:
 

tests/utils/test_main.py

@@ -8,14 +8,31 @@
 '''
 
 import click
+
 from click.testing import CliRunner
 import pytest
 import vdirsyncer.utils as utils
+import vdirsyncer.doubleclick as doubleclick
 from vdirsyncer.utils.vobject import split_collection
 
 from .. import blow_up, normalize_item, SIMPLE_TEMPLATE, BARE_EVENT_TEMPLATE
 
 
+class EmptyNetrc(object):
+    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, 'keyring', EmptyKeyring())
+
+
 def test_parse_options():
     o = {
         'foo': 'yes',
@@ -67,31 +84,17 @@ def test_get_password_from_netrc(monkeypatch):
     assert calls == [hostname]
 
 
-@pytest.mark.parametrize('resources_to_test', range(1, 8))
+def test_get_password_from_system_keyring(monkeypatch):
-def test_get_password_from_system_keyring(monkeypatch, resources_to_test):
     username = 'foouser'
     password = 'foopass'
     resource = 'http://example.com/path/to/whatever/'
     hostname = 'example.com'
 
     class KeyringMock(object):
-        def __init__(self):
-            p = utils.password_key_prefix
-            self.resources = [
-                p + 'http://example.com/path/to/whatever/',
-                p + 'http://example.com/path/to/whatever',
-                p + 'http://example.com/path/to/',
-                p + 'http://example.com/path/to',
-                p + 'http://example.com/path/',
-                p + 'http://example.com/path',
-                p + 'http://example.com/',
-            ][:resources_to_test]
-
         def get_password(self, resource, _username):
             assert _username == username
-            assert resource == self.resources.pop(0)
+            assert resource == utils.password_key_prefix + hostname
-            if not self.resources:
+            return password
-                return password
 
     monkeypatch.setattr(utils, 'keyring', KeyringMock())
 
@@ -110,20 +113,9 @@ def test_get_password_from_system_keyring(monkeypatch, resources_to_test):
     assert netrc_calls == [hostname]
 
 
-def test_get_password_from_prompt(monkeypatch):
+def test_get_password_from_prompt():
     getpass_calls = []
 
-    class Netrc(object):
-        def authenticators(self, hostname):
-            return None
-
-    class Keyring(object):
-        def get_password(self, *a, **kw):
-            return None
-
-    monkeypatch.setattr('netrc.netrc', Netrc)
-    monkeypatch.setattr(utils, 'keyring', Keyring())
-
     user = 'my_user'
     resource = 'http://example.com'
 
@@ -136,8 +128,35 @@ def test_get_password_from_prompt(monkeypatch):
     result = runner.invoke(fake_app, input='my_password\n\n')
     assert not result.exception
     assert result.output.splitlines() == [
-        'Server password for {} at the resource {}: '.format(user, resource),
+        'Server password for {} at host {}: '.format(user, 'example.com'),
+        'Password is my_password'
+    ]
+
+
+def test_get_password_from_cache(monkeypatch):
+    user = 'my_user'
+    resource = 'http://example.com'
+
+    @doubleclick.click.command()
+    @doubleclick.click.pass_context
+    def fake_app(ctx):
+        ctx.obj = {}
+        x = utils.get_password(user, resource)
+        click.echo('Password is {}'.format(x))
+        monkeypatch.setattr(doubleclick.click, 'prompt', blow_up)
+
+        assert (user, 'example.com') in ctx.obj['passwords']
+        x = utils.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',
+        'debug: Got password for my_user from internal cache',
         'Password is my_password'
     ]
 

vdirsyncer/utils/__init__.py

@@ -8,11 +8,12 @@
 '''
 
 import os
+import threading
 
 import requests
 
 from .. import exceptions, log
-from ..doubleclick import click
+from ..doubleclick import click, ctx
 from .compat import iteritems, urlparse
 
 
@@ -88,7 +89,7 @@ def parse_options(items, section=None):
                              .format(section, key, e))
 
 
-def get_password(username, resource):
+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:
@@ -110,71 +111,58 @@ def get_password(username, resource):
 
 
     """
-    for func in (_password_from_netrc, _password_from_keyring):
+    if ctx:
-        password = func(username, resource)
+        password_cache = ctx.obj.setdefault('passwords', {})
-        if password is not None:
-            logger.debug('Got password for {} from {}'
-                         .format(username, func.__doc__))
-            return password
 
-    prompt = ('Server password for {} at the resource {}'
+    with _lock:
-              .format(username, resource))
+        host = urlparse.urlsplit(resource).hostname
-    password = click.prompt(prompt, hide_input=True)
+        for func in (_password_from_cache, _password_from_netrc,
+                     _password_from_keyring):
+            password = func(username, host)
+            if password is not None:
+                logger.debug('Got password for {} from {}'
+                             .format(username, func.__doc__))
+                return password
 
-    if keyring is not None and \
+        prompt = ('Server password for {} at host {}'.format(username, host))
-       click.confirm('Save this password in the keyring?', default=False):
+        password = click.prompt(prompt, hide_input=True)
-        keyring.set_password(password_key_prefix + resource,
-                             username, password)
 
-    return password
+        if ctx and func is not _password_from_cache:
+            password_cache[(username, host)] = password
+            if keyring is not None and \
+               click.confirm('Save this password in the keyring?',
+                             default=False):
+                keyring.set_password(password_key_prefix + resource,
+                                     username, password)
+
+        return password
 
 
-def _password_from_netrc(username, resource):
+def _password_from_cache(username, host):
+    '''internal cache'''
+    if ctx:
+        return ctx.obj['passwords'].get((username, host), None)
+
+
+def _password_from_netrc(username, host):
     '''.netrc'''
     from netrc import netrc
 
-    hostname = urlparse.urlsplit(resource).hostname
     try:
         netrc_user, account, password = \
-            netrc().authenticators(hostname) or (None, None, None)
+            netrc().authenticators(host) or (None, None, None)
         if netrc_user == username:
             return password
     except IOError:
         pass
 
 
-def _password_from_keyring(username, resource):
+def _password_from_keyring(username, host):
     '''system keyring'''
     if keyring is None:
         return None
 
-    key = resource
+    return keyring.get_password(password_key_prefix + host, username)
-    password = None
-
-    while True:
-        password = keyring.get_password(password_key_prefix + key, username)
-        if password is not None:
-            return password
-
-        parsed = urlparse.urlsplit(key)
-        path = parsed.path
-        if not path:
-            return None
-        elif path.endswith('/'):
-            path = path.rstrip('/')
-        else:
-            path = path.rsplit('/', 1)[0] + '/'
-
-        new_key = urlparse.urlunsplit((
-            parsed.scheme,
-            parsed.netloc,
-            path,
-            parsed.query,
-            parsed.fragment
-        ))
-        if new_key == key:
-            return None
-        key = new_key
 
 
 def request(method, url, data=None, headers=None, auth=None, verify=None,