diff --git a/docs/keyring.rst b/docs/keyring.rst index 74c4db5..1c4ccd8 100644 --- a/docs/keyring.rst +++ b/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. - -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/``. +*vdirsyncer* will use the hostname as key prefixed with ``vdirsyncer:`` when +saving and fetching, e.g. ``vdirsyncer:owncloud.example.com``. *keyring* support these keyrings: diff --git a/tests/utils/test_main.py b/tests/utils/test_main.py index 929a48a..3d7b7f2 100644 --- a/tests/utils/test_main.py +++ b/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, resources_to_test): +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 __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) - if not self.resources: - return password + assert resource == utils.password_key_prefix + hostname + 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' ] diff --git a/vdirsyncer/utils/__init__.py b/vdirsyncer/utils/__init__.py index 75bc384..20f9e00 100644 --- a/vdirsyncer/utils/__init__.py +++ b/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): - password = func(username, resource) - if password is not None: - logger.debug('Got password for {} from {}' - .format(username, func.__doc__)) - return password + if ctx: + password_cache = ctx.obj.setdefault('passwords', {}) - prompt = ('Server password for {} at the resource {}' - .format(username, resource)) - password = click.prompt(prompt, hide_input=True) + with _lock: + host = urlparse.urlsplit(resource).hostname + 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 \ - click.confirm('Save this password in the keyring?', default=False): - keyring.set_password(password_key_prefix + resource, - username, password) + prompt = ('Server password for {} at host {}'.format(username, host)) + password = click.prompt(prompt, hide_input=True) - 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 - 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 + return keyring.get_password(password_key_prefix + host, username) def request(method, url, data=None, headers=None, auth=None, verify=None,