Skip to content

Commit

Permalink
New password fetching
Browse files Browse the repository at this point in the history
Fix #233
  • Loading branch information
untitaker committed Sep 11, 2015
1 parent e198326 commit 3a4e421
Show file tree
Hide file tree
Showing 11 changed files with 201 additions and 362 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
=============
Expand Down
77 changes: 39 additions & 38 deletions docs/keyring.rst
Original file line number Diff line number Diff line change
@@ -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
==============

.. versionadded:: 0.3.0
Password configuration got completely overhauled.

A custom command/binary can be specified to retrieve the password for a
username/hostname combination. See :ref:`general_config`.
Vdirsyncer can fetch passwords from a custom command or your system keyring if
the keyring_ Python package is installed.

.. versionchanged:: 0.6.0
Command
=======

Setting a custom command now disables all other methods.
Say you have the following configuration::

netrc
=====
[storage foo]
type = caldav
url = ...
username = foo
password = bar

Vdirsyncer can use ``~/.netrc`` for retrieving a password. An example
``.netrc`` looks like this::
But it bugs you that the password is stored in cleartext in the config file.
You can do this::

machine owncloud.example.com
login foouser
password foopass
[storage foo]
type = caldav
url = ...
username = foo
password.fetch = ["command", "~/get-password.sh", "more", "args"]

System Keyring
==============
You can fetch the username as well::

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.
[storage foo]
type = caldav
url = ...
username.fetch = ["command", "~/get-username.sh"]
password.fetch = ["command", "~/get-password.sh"]

To use it, you must install the ``keyring`` Python package.
Or really any kind of parameter in a storage section.

.. _keyring: https://bitbucket.org/kang/python-keyring-lib
System Keyring
==============

Storing the password
--------------------
While the command approach is quite flexible, it is often cumbersome to write a
script fetching the system keyring.

Vdirsyncer will use the hostname as key prefixed with ``vdirsyncer:``, e.g.
``vdirsyncer:owncloud.example.com``.
Given that you have the keyring_ Python library installed, you can use::

Changing the Password
---------------------
[storage foo]
type = caldav
username = myusername
password.fetch = ["keyring", "myservicename", "myusername"]

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
54 changes: 54 additions & 0 deletions tests/cli/test_fetchparams.py
Original file line number Diff line number Diff line change
@@ -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
167 changes: 0 additions & 167 deletions tests/utils/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,31 +22,11 @@
# 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')
Expand All @@ -59,153 +39,6 @@ def teardown():
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):
Expand Down
2 changes: 1 addition & 1 deletion vdirsyncer/cli/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
class AppContext(object):
def __init__(self):
self.config = None
self.passwords = {}
self.fetched_params = {}
self.logger = None


Expand Down
17 changes: 13 additions & 4 deletions vdirsyncer/cli/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,17 @@
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:
from ConfigParser import RawConfigParser
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, '_'))

Expand Down Expand Up @@ -206,8 +207,16 @@ def __init__(self, config, name, name_a, name_b, pair_options):
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):
Expand Down
Loading

0 comments on commit 3a4e421

Please sign in to comment.