From 413c0f7abd73c65c222dac4e74cfb0df49ac9c17 Mon Sep 17 00:00:00 2001 From: Paul Fariello Date: Tue, 13 Sep 2016 16:17:28 +0200 Subject: [PATCH 01/17] Add ldap storage Current implementation is read only. Not sure if ldap storage is supposed to do vcard conversion. --- vdirsyncer/cli/utils.py | 3 +- vdirsyncer/storage/ldap.py | 73 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 vdirsyncer/storage/ldap.py diff --git a/vdirsyncer/cli/utils.py b/vdirsyncer/cli/utils.py index 80249c4bc..9aa6f74fe 100644 --- a/vdirsyncer/cli/utils.py +++ b/vdirsyncer/cli/utils.py @@ -47,7 +47,8 @@ def __init__(self): remotestorage_calendars=( 'vdirsyncer.storage.remotestorage.RemoteStorageCalendars'), google_calendar='vdirsyncer.storage.google.GoogleCalendarStorage', - google_contacts='vdirsyncer.storage.google.GoogleContactsStorage' + google_contacts='vdirsyncer.storage.google.GoogleContactsStorage', + ldap='vdirsyncer.storage.ldap.LDAPStorage' ) def __getitem__(self, name): diff --git a/vdirsyncer/storage/ldap.py b/vdirsyncer/storage/ldap.py new file mode 100644 index 000000000..99a7d1e49 --- /dev/null +++ b/vdirsyncer/storage/ldap.py @@ -0,0 +1,73 @@ +# -*- coding: utf-8 -*- +import ldap3 +import logging + +from .base import Storage, Item + +ldap_logger = logging.getLogger(__name__) + +class LDAPStorage(Storage): + + __doc__ = ''' + :param uri: LDAP URI + :param search_base: search base + :param bind: bind dn + :param password: bind password + :param filter: filter + ''' + storage_name = 'ldap' + read_only = True + fileext = '.vcf' + item_mimetype = 'text/vcard' + + def __init__(self, uri=None, search_base=None, bind=None, password=None, filter='(objectClass=*)', **kwargs): + super(LDAPStorage, self).__init__(**kwargs) + self.search_base = search_base + self.filter = filter + self.server = ldap3.Server(uri) + if bind: + self.conn = ldap3.Connection(self.server, user=bind, password=password) + else: + self.conn = ldap3.Connection(self.server) + self.conn.bind() + self.conn.start_tls() + ldap_logger.debug('Connected to: {}'.format(self.conn)) + + def list(self): + ''' + :returns: list of (href, etag) + ''' + ldap_logger.debug('Search on {self.search_base} with filter {self.filter}'.format(self=self)) + self.conn.search(self.search_base, self.filter, attributes=["whenChanged"]) + for entry in self.conn.entries: + ldap_logger.debug('Found {}'.format(entry.entry_get_dn())) + href = entry.entry_get_dn() + etag = str(entry.whenChanged) + yield href, etag + + def get(self, href): + self.conn.search(href, self.filter, + attributes=["whenChanged", "cn", "sn", "givenName", "displayName", "telephoneNumber", "mobile", "mail"]) + + if not self.conn.entries[0]: + raise exceptions.NotFoundError(href) + + entry = self.conn.entries[0] + etag = str(entry.whenChanged) + + vcard = "BEGIN:VCARD\r\n" + vcard += "VERSION:3.0\r\n" + vcard += "FN;CHARSET=UTF-8:{cn}\r\n".format(cn=entry.cn) + if getattr(entry, 'sn', None): + vcard += "N;CHARSET=UTF-8:{sn};{givenName}\r\n".format(givenName=entry.givenName, sn=entry.sn) + if getattr(entry, 'telephoneNumber', None): + vcard += "TEL;WORK;VOICE:{tel}\r\n".format(tel=entry.telephoneNumber) + if getattr(entry, 'mobile', None): + vcard += "TEL;CELL;VOICE:{mobile}\r\n".format(mobile=entry.mobile) + if getattr(entry, 'mail', None): + vcard += "EMAIL;INTERNET:{email}\r\n".format(email=entry.mail.value.strip()) + vcard += "END:VCARD" + + item = Item(vcard) + + return item, etag From 69698dae0945ecbf57a570aff33e91a220f792b8 Mon Sep 17 00:00:00 2001 From: Paul Fariello Date: Tue, 13 Sep 2016 16:48:01 +0200 Subject: [PATCH 02/17] pep8 conformance --- vdirsyncer/storage/ldap.py | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/vdirsyncer/storage/ldap.py b/vdirsyncer/storage/ldap.py index 99a7d1e49..65fca73a6 100644 --- a/vdirsyncer/storage/ldap.py +++ b/vdirsyncer/storage/ldap.py @@ -1,11 +1,13 @@ # -*- coding: utf-8 -*- -import ldap3 import logging +import ldap3 -from .base import Storage, Item +from .base import Item, Storage +from .. import exceptions ldap_logger = logging.getLogger(__name__) + class LDAPStorage(Storage): __doc__ = ''' @@ -20,13 +22,15 @@ class LDAPStorage(Storage): fileext = '.vcf' item_mimetype = 'text/vcard' - def __init__(self, uri=None, search_base=None, bind=None, password=None, filter='(objectClass=*)', **kwargs): + def __init__(self, uri=None, search_base=None, bind=None, password=None, + filter='(objectClass=*)', **kwargs): super(LDAPStorage, self).__init__(**kwargs) self.search_base = search_base self.filter = filter self.server = ldap3.Server(uri) if bind: - self.conn = ldap3.Connection(self.server, user=bind, password=password) + self.conn = ldap3.Connection(self.server, user=bind, + password=password) else: self.conn = ldap3.Connection(self.server) self.conn.bind() @@ -37,8 +41,10 @@ def list(self): ''' :returns: list of (href, etag) ''' - ldap_logger.debug('Search on {self.search_base} with filter {self.filter}'.format(self=self)) - self.conn.search(self.search_base, self.filter, attributes=["whenChanged"]) + ldap_logger.debug('Search on {self.search_base} with filter' + '{self.filter}'.format(self=self)) + self.conn.search(self.search_base, self.filter, + attributes=["whenChanged"]) for entry in self.conn.entries: ldap_logger.debug('Found {}'.format(entry.entry_get_dn())) href = entry.entry_get_dn() @@ -47,7 +53,9 @@ def list(self): def get(self, href): self.conn.search(href, self.filter, - attributes=["whenChanged", "cn", "sn", "givenName", "displayName", "telephoneNumber", "mobile", "mail"]) + attributes=["whenChanged", "cn", "sn", "givenName", + "displayName", "telephoneNumber", + "mobile", "mail"]) if not self.conn.entries[0]: raise exceptions.NotFoundError(href) @@ -59,13 +67,16 @@ def get(self, href): vcard += "VERSION:3.0\r\n" vcard += "FN;CHARSET=UTF-8:{cn}\r\n".format(cn=entry.cn) if getattr(entry, 'sn', None): - vcard += "N;CHARSET=UTF-8:{sn};{givenName}\r\n".format(givenName=entry.givenName, sn=entry.sn) + vcard += "N;CHARSET=UTF-8:{sn};{givenName}\r\n".format( + givenName=entry.givenName, sn=entry.sn) if getattr(entry, 'telephoneNumber', None): - vcard += "TEL;WORK;VOICE:{tel}\r\n".format(tel=entry.telephoneNumber) + vcard += "TEL;WORK;VOICE:{tel}\r\n".format( + tel=entry.telephoneNumber) if getattr(entry, 'mobile', None): vcard += "TEL;CELL;VOICE:{mobile}\r\n".format(mobile=entry.mobile) if getattr(entry, 'mail', None): - vcard += "EMAIL;INTERNET:{email}\r\n".format(email=entry.mail.value.strip()) + vcard += "EMAIL;INTERNET:{email}\r\n".format( + email=entry.mail.value.strip()) vcard += "END:VCARD" item = Item(vcard) From 3b0c4a6b5e356de61c212aafba8bc2d71024b8ce Mon Sep 17 00:00:00 2001 From: Paul Fariello Date: Tue, 13 Sep 2016 17:44:52 +0200 Subject: [PATCH 03/17] Use vobject for vcard conversion --- vdirsyncer/storage/ldap.py | 44 +++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/vdirsyncer/storage/ldap.py b/vdirsyncer/storage/ldap.py index 65fca73a6..fb37f2a08 100644 --- a/vdirsyncer/storage/ldap.py +++ b/vdirsyncer/storage/ldap.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- import logging import ldap3 +import vobject from .base import Item, Storage from .. import exceptions @@ -23,7 +24,9 @@ class LDAPStorage(Storage): item_mimetype = 'text/vcard' def __init__(self, uri=None, search_base=None, bind=None, password=None, - filter='(objectClass=*)', **kwargs): + filter='(&(objectCategory=person)(objectClass=user)' + '(sn=*)(givenName=*))', + **kwargs): super(LDAPStorage, self).__init__(**kwargs) self.search_base = search_base self.filter = filter @@ -55,7 +58,8 @@ def get(self, href): self.conn.search(href, self.filter, attributes=["whenChanged", "cn", "sn", "givenName", "displayName", "telephoneNumber", - "mobile", "mail"]) + "mobile", "facsimileTelephoneNumber", + "mail", "title"]) if not self.conn.entries[0]: raise exceptions.NotFoundError(href) @@ -63,22 +67,32 @@ def get(self, href): entry = self.conn.entries[0] etag = str(entry.whenChanged) - vcard = "BEGIN:VCARD\r\n" - vcard += "VERSION:3.0\r\n" - vcard += "FN;CHARSET=UTF-8:{cn}\r\n".format(cn=entry.cn) - if getattr(entry, 'sn', None): - vcard += "N;CHARSET=UTF-8:{sn};{givenName}\r\n".format( - givenName=entry.givenName, sn=entry.sn) + vcard = vobject.vCard() + vo = vcard.add('fn') + vo.value = str(entry.cn) + vo = vcard.add('n') + vo.value = vobject.vcard.Name(family=str(entry.sn), + given=str(entry.givenName)) if getattr(entry, 'telephoneNumber', None): - vcard += "TEL;WORK;VOICE:{tel}\r\n".format( - tel=entry.telephoneNumber) + vo = vcard.add('tel') + vo.value = str(entry.telephoneNumber) + vo.type_param = 'WORK' if getattr(entry, 'mobile', None): - vcard += "TEL;CELL;VOICE:{mobile}\r\n".format(mobile=entry.mobile) + vo = vcard.add('tel') + vo.value = str(entry.mobile) + vo.type_param = 'CELL' + if getattr(entry, 'facsimileTelephoneNumber', None): + vo = vcard.add('tel') + vo.value = str(entry.facsimileTelephoneNumber) + vo.type_param = 'FAX' if getattr(entry, 'mail', None): - vcard += "EMAIL;INTERNET:{email}\r\n".format( - email=entry.mail.value.strip()) - vcard += "END:VCARD" + vo = vcard.add('email') + vo.value = str(entry.mail) + vo.type_param = 'INTERNET' + if getattr(entry, 'title', None): + vo = vcard.add('title') + vo.value = str(entry.title) - item = Item(vcard) + item = Item(vcard.serialize()) return item, etag From 55e5ff65d1e9b5e1dd3ec0071a95eebb272e2018 Mon Sep 17 00:00:00 2001 From: Paul Fariello Date: Tue, 13 Sep 2016 17:45:02 +0200 Subject: [PATCH 04/17] Add ldap3 and vobject dependencies --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index bf0dcfbd2..3d9d2b203 100644 --- a/setup.py +++ b/setup.py @@ -69,6 +69,7 @@ def run(self): extras_require={ 'remotestorage': ['requests-oauthlib'], 'google': ['requests-oauthlib'], + 'ldap': ['ldap3', 'vobject'], }, # Build dependencies From 0959a1641ea4f9e9691fafd9cc0c8b1932f38004 Mon Sep 17 00:00:00 2001 From: Paul Fariello Date: Tue, 13 Sep 2016 17:48:13 +0200 Subject: [PATCH 05/17] Remove superfluous __doc__ attribute --- vdirsyncer/storage/ldap.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/vdirsyncer/storage/ldap.py b/vdirsyncer/storage/ldap.py index fb37f2a08..4a2d0f69a 100644 --- a/vdirsyncer/storage/ldap.py +++ b/vdirsyncer/storage/ldap.py @@ -10,8 +10,7 @@ class LDAPStorage(Storage): - - __doc__ = ''' + ''' :param uri: LDAP URI :param search_base: search base :param bind: bind dn From df3b6ff34a71476ddd332cdc8be14797d2654767 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Tue, 13 Sep 2016 21:03:01 +0200 Subject: [PATCH 06/17] Add simple LDAP test --- test-requirements.txt | 1 + tests/storage/test_ldap.py | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 tests/storage/test_ldap.py diff --git a/test-requirements.txt b/test-requirements.txt index 010a782e2..df7ebca08 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -2,3 +2,4 @@ hypothesis>=3.1 pytest pytest-localserver pytest-subtesthack +mockldap diff --git a/tests/storage/test_ldap.py b/tests/storage/test_ldap.py new file mode 100644 index 000000000..dc9ffbcd0 --- /dev/null +++ b/tests/storage/test_ldap.py @@ -0,0 +1,25 @@ +# -*- coding: utf-8 -*- + +from mockldap import MockLdap +import pytest + +from vdirsyncer.storage.ldap import LDAPStorage + +from . import StorageTests + + +class TestLDAPStorage(StorageTests): + storage_class = LDAPStorage + supports_collections = False + + @pytest.fixture + def get_storage_args(self, request): + uri = 'ldap://localhost' + mockldap = MockLdap({}) + mockldap.start() + ldapobj = mockldap[uri] + request.addfinalizer(mockldap.stop) + + def inner(collection='test'): + return {'uri': uri} + return inner From 3a08a00fd7ce0d8765dd054e1a86f79b3a050545 Mon Sep 17 00:00:00 2001 From: Paul Fariello Date: Wed, 14 Sep 2016 18:12:32 +0200 Subject: [PATCH 07/17] Use ldap3 internal mocking for tests --- test-requirements.txt | 1 - tests/storage/test_ldap.py | 14 +++++++------- vdirsyncer/storage/ldap.py | 15 +++++++++------ 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/test-requirements.txt b/test-requirements.txt index df7ebca08..010a782e2 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -2,4 +2,3 @@ hypothesis>=3.1 pytest pytest-localserver pytest-subtesthack -mockldap diff --git a/tests/storage/test_ldap.py b/tests/storage/test_ldap.py index dc9ffbcd0..7413f49cf 100644 --- a/tests/storage/test_ldap.py +++ b/tests/storage/test_ldap.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- -from mockldap import MockLdap +import ldap3 import pytest from vdirsyncer.storage.ldap import LDAPStorage @@ -13,13 +13,13 @@ class TestLDAPStorage(StorageTests): supports_collections = False @pytest.fixture - def get_storage_args(self, request): + def get_storage_args(self): uri = 'ldap://localhost' - mockldap = MockLdap({}) - mockldap.start() - ldapobj = mockldap[uri] - request.addfinalizer(mockldap.stop) + server = ldap3.Server('fake') + conn = ldap3.Connection(server, client_strategy=ldap3.MOCK_SYNC) + + conn.strategy.add_entry('cn=user0,ou=test,o=lab', {'userPassword': 'test0000', 'sn': 'user0_sn', 'revision': 0}) def inner(collection='test'): - return {'uri': uri} + return {'uri': uri, 'conn': conn} return inner diff --git a/vdirsyncer/storage/ldap.py b/vdirsyncer/storage/ldap.py index 4a2d0f69a..f58252d30 100644 --- a/vdirsyncer/storage/ldap.py +++ b/vdirsyncer/storage/ldap.py @@ -25,16 +25,19 @@ class LDAPStorage(Storage): def __init__(self, uri=None, search_base=None, bind=None, password=None, filter='(&(objectCategory=person)(objectClass=user)' '(sn=*)(givenName=*))', + conn=None, **kwargs): super(LDAPStorage, self).__init__(**kwargs) self.search_base = search_base self.filter = filter - self.server = ldap3.Server(uri) - if bind: - self.conn = ldap3.Connection(self.server, user=bind, - password=password) - else: - self.conn = ldap3.Connection(self.server) + self.conn = conn + if not self.conn: + server = ldap3.Server(uri) + if bind: + self.conn = ldap3.Connection(server, user=bind, + password=password) + else: + self.conn = ldap3.Connection(server) self.conn.bind() self.conn.start_tls() ldap_logger.debug('Connected to: {}'.format(self.conn)) From 400ba9019cb0799ae8927c30bea28f6032bc3d80 Mon Sep 17 00:00:00 2001 From: Paul Fariello Date: Wed, 14 Sep 2016 18:16:50 +0200 Subject: [PATCH 08/17] Fallback on item hash if whenChanged is not available --- vdirsyncer/storage/ldap.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/vdirsyncer/storage/ldap.py b/vdirsyncer/storage/ldap.py index f58252d30..2d7dce143 100644 --- a/vdirsyncer/storage/ldap.py +++ b/vdirsyncer/storage/ldap.py @@ -53,7 +53,12 @@ def list(self): for entry in self.conn.entries: ldap_logger.debug('Found {}'.format(entry.entry_get_dn())) href = entry.entry_get_dn() - etag = str(entry.whenChanged) + if getattr(entry, 'whenChanged'): + etag = str(entry.whenChanged) + else: + entry = self.get(href) + etag = item.hash + yield href, etag def get(self, href): From 9299f1166cc1311303fb0cd0c4d9394d0c2dddb3 Mon Sep 17 00:00:00 2001 From: Paul Fariello Date: Wed, 14 Sep 2016 18:18:19 +0200 Subject: [PATCH 09/17] Add default value for ldap uri --- vdirsyncer/storage/ldap.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vdirsyncer/storage/ldap.py b/vdirsyncer/storage/ldap.py index 2d7dce143..e6f91e7b4 100644 --- a/vdirsyncer/storage/ldap.py +++ b/vdirsyncer/storage/ldap.py @@ -22,7 +22,7 @@ class LDAPStorage(Storage): fileext = '.vcf' item_mimetype = 'text/vcard' - def __init__(self, uri=None, search_base=None, bind=None, password=None, + def __init__(self, uri='ldap://localhost', search_base=None, bind=None, password=None, filter='(&(objectCategory=person)(objectClass=user)' '(sn=*)(givenName=*))', conn=None, From 88cc171ffef0b2bf138b4bd1c02f1ebb828fc160 Mon Sep 17 00:00:00 2001 From: Paul Fariello Date: Wed, 14 Sep 2016 18:37:08 +0200 Subject: [PATCH 10/17] Add default value for base dn --- vdirsyncer/storage/ldap.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/vdirsyncer/storage/ldap.py b/vdirsyncer/storage/ldap.py index e6f91e7b4..bd8ceeef7 100644 --- a/vdirsyncer/storage/ldap.py +++ b/vdirsyncer/storage/ldap.py @@ -31,8 +31,8 @@ def __init__(self, uri='ldap://localhost', search_base=None, bind=None, password self.search_base = search_base self.filter = filter self.conn = conn - if not self.conn: - server = ldap3.Server(uri) + if self.conn is None: + server = ldap3.Server(uri, get_info=ldap3.DSA) if bind: self.conn = ldap3.Connection(server, user=bind, password=password) @@ -42,6 +42,10 @@ def __init__(self, uri='ldap://localhost', search_base=None, bind=None, password self.conn.start_tls() ldap_logger.debug('Connected to: {}'.format(self.conn)) + if self.search_base is None: + # Fallback to default root entry + self.search_base = server.info.naming_contexts[0] + def list(self): ''' :returns: list of (href, etag) From f812edd0d167471b65376899a4795aea940ae484 Mon Sep 17 00:00:00 2001 From: Paul Fariello Date: Thu, 15 Sep 2016 09:04:01 +0200 Subject: [PATCH 11/17] Add missing dependency in test requirements --- test-requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/test-requirements.txt b/test-requirements.txt index 010a782e2..25fe2b8f6 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -2,3 +2,4 @@ hypothesis>=3.1 pytest pytest-localserver pytest-subtesthack +ldap3 From 51ec702ea0640baf36df0615ea1fa429929f81e0 Mon Sep 17 00:00:00 2001 From: Paul Fariello Date: Thu, 15 Sep 2016 09:05:24 +0200 Subject: [PATCH 12/17] Rename ldap URI into URL --- tests/storage/test_ldap.py | 4 ++-- vdirsyncer/storage/ldap.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/storage/test_ldap.py b/tests/storage/test_ldap.py index 7413f49cf..5088a6bf6 100644 --- a/tests/storage/test_ldap.py +++ b/tests/storage/test_ldap.py @@ -14,12 +14,12 @@ class TestLDAPStorage(StorageTests): @pytest.fixture def get_storage_args(self): - uri = 'ldap://localhost' + url = 'ldap://localhost' server = ldap3.Server('fake') conn = ldap3.Connection(server, client_strategy=ldap3.MOCK_SYNC) conn.strategy.add_entry('cn=user0,ou=test,o=lab', {'userPassword': 'test0000', 'sn': 'user0_sn', 'revision': 0}) def inner(collection='test'): - return {'uri': uri, 'conn': conn} + return {'url': url, 'conn': conn} return inner diff --git a/vdirsyncer/storage/ldap.py b/vdirsyncer/storage/ldap.py index bd8ceeef7..17635b3ae 100644 --- a/vdirsyncer/storage/ldap.py +++ b/vdirsyncer/storage/ldap.py @@ -11,7 +11,7 @@ class LDAPStorage(Storage): ''' - :param uri: LDAP URI + :param url: LDAP URL :param search_base: search base :param bind: bind dn :param password: bind password @@ -22,7 +22,7 @@ class LDAPStorage(Storage): fileext = '.vcf' item_mimetype = 'text/vcard' - def __init__(self, uri='ldap://localhost', search_base=None, bind=None, password=None, + def __init__(self, url='ldap://localhost', search_base=None, bind=None, password=None, filter='(&(objectCategory=person)(objectClass=user)' '(sn=*)(givenName=*))', conn=None, @@ -32,7 +32,7 @@ def __init__(self, uri='ldap://localhost', search_base=None, bind=None, password self.filter = filter self.conn = conn if self.conn is None: - server = ldap3.Server(uri, get_info=ldap3.DSA) + server = ldap3.Server(url, get_info=ldap3.DSA) if bind: self.conn = ldap3.Connection(server, user=bind, password=password) From 626dcf012fd6d0cd30103f25f3d1101472b04db4 Mon Sep 17 00:00:00 2001 From: Paul Fariello Date: Wed, 19 Oct 2016 22:47:39 +0200 Subject: [PATCH 13/17] Add update and upload method --- tests/storage/test_ldap.py | 2 +- vdirsyncer/storage/ldap.py | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/storage/test_ldap.py b/tests/storage/test_ldap.py index 5088a6bf6..31e631684 100644 --- a/tests/storage/test_ldap.py +++ b/tests/storage/test_ldap.py @@ -21,5 +21,5 @@ def get_storage_args(self): conn.strategy.add_entry('cn=user0,ou=test,o=lab', {'userPassword': 'test0000', 'sn': 'user0_sn', 'revision': 0}) def inner(collection='test'): - return {'url': url, 'conn': conn} + return {'url': url, 'conn': conn, 'search_base': 'ou=test,o=lab'} return inner diff --git a/vdirsyncer/storage/ldap.py b/vdirsyncer/storage/ldap.py index 17635b3ae..564296a13 100644 --- a/vdirsyncer/storage/ldap.py +++ b/vdirsyncer/storage/ldap.py @@ -18,7 +18,6 @@ class LDAPStorage(Storage): :param filter: filter ''' storage_name = 'ldap' - read_only = True fileext = '.vcf' item_mimetype = 'text/vcard' @@ -107,3 +106,11 @@ def get(self, href): item = Item(vcard.serialize()) return item, etag + + def upload(self, item): + vcard = vobject.readOne(item.raw) + self.conn.strategy.add_entry('cn={},ou=test,o=lab'.format(vcard.fn), vcard) + + def update(self, href, item, etag): + vcard = vobject.readOne(item.raw) + self.conn.strategy.add_entry(href, vcard) From 52eb20ed50721241db6aaec6deacfdef065d9255 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Wed, 19 Oct 2016 23:38:33 +0200 Subject: [PATCH 14/17] ldap: Hide test args from docs, fix testing --- tests/storage/test_ldap.py | 2 +- vdirsyncer/storage/ldap.py | 13 +++++++------ vdirsyncer/utils/__init__.py | 8 ++++++-- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/tests/storage/test_ldap.py b/tests/storage/test_ldap.py index 31e631684..5e46fc941 100644 --- a/tests/storage/test_ldap.py +++ b/tests/storage/test_ldap.py @@ -21,5 +21,5 @@ def get_storage_args(self): conn.strategy.add_entry('cn=user0,ou=test,o=lab', {'userPassword': 'test0000', 'sn': 'user0_sn', 'revision': 0}) def inner(collection='test'): - return {'url': url, 'conn': conn, 'search_base': 'ou=test,o=lab'} + return {'url': url, '_conn': conn, 'search_base': 'ou=test,o=lab'} return inner diff --git a/vdirsyncer/storage/ldap.py b/vdirsyncer/storage/ldap.py index 564296a13..882ed8426 100644 --- a/vdirsyncer/storage/ldap.py +++ b/vdirsyncer/storage/ldap.py @@ -21,15 +21,15 @@ class LDAPStorage(Storage): fileext = '.vcf' item_mimetype = 'text/vcard' - def __init__(self, url='ldap://localhost', search_base=None, bind=None, password=None, + def __init__(self, url='ldap://localhost', search_base=None, bind=None, + password=None, filter='(&(objectCategory=person)(objectClass=user)' '(sn=*)(givenName=*))', - conn=None, - **kwargs): + _conn=None, **kwargs): super(LDAPStorage, self).__init__(**kwargs) self.search_base = search_base self.filter = filter - self.conn = conn + self.conn = _conn if self.conn is None: server = ldap3.Server(url, get_info=ldap3.DSA) if bind: @@ -37,8 +37,9 @@ def __init__(self, url='ldap://localhost', search_base=None, bind=None, password password=password) else: self.conn = ldap3.Connection(server) - self.conn.bind() - self.conn.start_tls() + self.conn.bind() + self.conn.start_tls() + ldap_logger.debug('Connected to: {}'.format(self.conn)) if self.search_base is None: diff --git a/vdirsyncer/utils/__init__.py b/vdirsyncer/utils/__init__.py index e1a0acbef..4f2b8438a 100644 --- a/vdirsyncer/utils/__init__.py +++ b/vdirsyncer/utils/__init__.py @@ -104,10 +104,14 @@ class can take, and ``required`` is the subset of arguments the class requires. ''' all, required = set(), set() + + def _filter_args(args): + return (a for a in args if not a.startswith('_')) + for spec in get_storage_init_specs(cls, stop_at=stop_at): - all.update(spec.args[1:]) + all.update(_filter_args(spec.args[1:])) last = -len(spec.defaults) if spec.defaults else len(spec.args) - required.update(spec.args[1:last]) + required.update(_filter_args(spec.args[1:last])) return all, required From f4e5f046d8efab9a0e9ec357dcbc5f0c82708231 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Wed, 19 Oct 2016 23:43:29 +0200 Subject: [PATCH 15/17] Only test vcards --- tests/storage/test_ldap.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/storage/test_ldap.py b/tests/storage/test_ldap.py index 5e46fc941..3634d2925 100644 --- a/tests/storage/test_ldap.py +++ b/tests/storage/test_ldap.py @@ -12,6 +12,10 @@ class TestLDAPStorage(StorageTests): storage_class = LDAPStorage supports_collections = False + @pytest.fixture(params=['VCARD']) + def item_type(self, request): + return request.param + @pytest.fixture def get_storage_args(self): url = 'ldap://localhost' From 3a5a5d66a023bb0789171ded15d91cdb91bc026a Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Sat, 22 Oct 2016 15:44:29 +0200 Subject: [PATCH 16/17] Fix test requirements --- Makefile | 5 ++--- test-requirements.txt | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 1e9343f64..00cce2ca0 100644 --- a/Makefile +++ b/Makefile @@ -68,10 +68,9 @@ release: python setup.py sdist bdist_wheel upload install-dev: - set -xe && if [ "$$REMOTESTORAGE_SERVER" != "skip" ]; then \ + pip install -e .[ldap] + if [ "$$REMOTESTORAGE_SERVER" != "skip" ]; then \ pip install -e .[remotestorage]; \ - else \ - pip install -e .; \ fi set -xe && if [ "$$REQUIREMENTS" = "devel" ]; then \ pip install -U --force-reinstall \ diff --git a/test-requirements.txt b/test-requirements.txt index 25fe2b8f6..010a782e2 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -2,4 +2,3 @@ hypothesis>=3.1 pytest pytest-localserver pytest-subtesthack -ldap3 From 2ff778b494f5b9fe70bf37316f4417f6d0af0ade Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Sat, 22 Oct 2016 16:54:03 +0200 Subject: [PATCH 17/17] Stylefixes --- tests/storage/test_ldap.py | 5 ++++- vdirsyncer/storage/ldap.py | 5 +++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/storage/test_ldap.py b/tests/storage/test_ldap.py index 3634d2925..87844e6de 100644 --- a/tests/storage/test_ldap.py +++ b/tests/storage/test_ldap.py @@ -22,7 +22,10 @@ def get_storage_args(self): server = ldap3.Server('fake') conn = ldap3.Connection(server, client_strategy=ldap3.MOCK_SYNC) - conn.strategy.add_entry('cn=user0,ou=test,o=lab', {'userPassword': 'test0000', 'sn': 'user0_sn', 'revision': 0}) + conn.strategy.add_entry( + 'cn=user0,ou=test,o=lab', + {'userPassword': 'test0000', 'sn': 'user0_sn', 'revision': 0} + ) def inner(collection='test'): return {'url': url, '_conn': conn, 'search_base': 'ou=test,o=lab'} diff --git a/vdirsyncer/storage/ldap.py b/vdirsyncer/storage/ldap.py index 882ed8426..5ca7a50e4 100644 --- a/vdirsyncer/storage/ldap.py +++ b/vdirsyncer/storage/ldap.py @@ -60,7 +60,7 @@ def list(self): if getattr(entry, 'whenChanged'): etag = str(entry.whenChanged) else: - entry = self.get(href) + item = self.get(href) etag = item.hash yield href, etag @@ -110,7 +110,8 @@ def get(self, href): def upload(self, item): vcard = vobject.readOne(item.raw) - self.conn.strategy.add_entry('cn={},ou=test,o=lab'.format(vcard.fn), vcard) + self.conn.strategy.add_entry('cn={},ou=test,o=lab'.format(vcard.fn), + vcard) def update(self, href, item, etag): vcard = vobject.readOne(item.raw)