From 12bef596581101d4250b859b4402918098736ef0 Mon Sep 17 00:00:00 2001 From: Florian Richter Date: Wed, 11 Mar 2015 18:02:53 +0100 Subject: [PATCH 1/5] Add .travis.yml --- .travis.yml | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 .travis.yml diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 0000000..d3dd8a9 --- /dev/null +++ b/.travis.yml @@ -0,0 +1,9 @@ +language: python +python: + - "3.3" + - "2.7" +install: + - "sudo apt-get update -qq" + - "sudo apt-get install -y slapd libldap2-dev" + - "pip install -r requirements.txt" +script: "python -m unittest -v tests" From 90ca6d0fb9177a2cf800e2e56fba39672d76463b Mon Sep 17 00:00:00 2001 From: Florian Richter Date: Tue, 17 Mar 2015 19:20:51 +0100 Subject: [PATCH 2/5] Show meaningful error when attribute types could not be fetched --- ldapom/connection.py | 3 +++ ldapom/error.py | 3 +++ 2 files changed, 6 insertions(+) diff --git a/ldapom/connection.py b/ldapom/connection.py index eb20f3d..8283cc5 100644 --- a/ldapom/connection.py +++ b/ldapom/connection.py @@ -161,6 +161,9 @@ def _fetch_attribute_types(self): scope=libldap.LDAP_SCOPE_BASE, search_filter="(objectClass=*)", retrieve_attributes=["attributeTypes"])) + if len(result) == 0: + raise error.LDAPCouldNotFetchAttributeTypes + # Decode the type definitions returned to strings attribute_type_definitions += map(compat._decode_utf8, result[0][1][compat._encode_utf8("attributeTypes")]) diff --git a/ldapom/error.py b/ldapom/error.py index 8aa4fe7..4b94657 100644 --- a/ldapom/error.py +++ b/ldapom/error.py @@ -26,3 +26,6 @@ class LDAPServerDownError(LDAPError): class LDAPAttributeNameNotFoundError(LDAPomError): pass + +class LDAPCouldNotFetchAttributeTypes(LDAPomError): + pass From 379c168642406367d78ffd86b7d79b8033c0b30e Mon Sep 17 00:00:00 2001 From: Florian Richter Date: Tue, 17 Mar 2015 19:22:27 +0100 Subject: [PATCH 3/5] Do not fetch attribute types on can_bind As the connection is only established to test authentication, attribute types are never used and we don't need to retrieve them --- ldapom/connection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ldapom/connection.py b/ldapom/connection.py index 8283cc5..b89fe72 100644 --- a/ldapom/connection.py +++ b/ldapom/connection.py @@ -206,7 +206,7 @@ def can_bind(self, bind_dn, bind_password): """ try: self.__class__(self._uri, self._base, bind_dn, bind_password, - self._cacertfile) + self._cacertfile, enable_attribute_type_mapping=False) except error.LDAPInvalidCredentialsError: return False return True From 8cdc507437ac78013775af6e03917334201b9473 Mon Sep 17 00:00:00 2001 From: Florian Richter Date: Tue, 17 Mar 2015 23:12:07 +0100 Subject: [PATCH 4/5] Return None for empty single-value attributes This corresponds better what happens for multi-value attributes, where an empty set is returned --- ldapom/entry.py | 2 +- tests.py | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/ldapom/entry.py b/ldapom/entry.py index 87d0f84..48b49bd 100644 --- a/ldapom/entry.py +++ b/ldapom/entry.py @@ -90,7 +90,7 @@ def __getattr__(self, name): setattr(self, name, set()) return self.get_attribute(name).values else: - raise AttributeError() + return None def __setattr__(self, name, value): """Set an attribute value. diff --git a/tests.py b/tests.py index 130b72c..44fbc77 100755 --- a/tests.py +++ b/tests.py @@ -122,7 +122,7 @@ def test_delete_attribute(self): entry = self.ldap_connection.get_entry( "cn=jack,dc=example,dc=com") - self.assertRaises(AttributeError, getattr, entry, "loginShell") + self.assertIsNone(entry.loginShell) def test_modify_single_value_attribute(self): entry = self.ldap_connection.get_entry( @@ -135,11 +135,11 @@ def test_modify_single_value_attribute(self): "cn=jack,dc=example,dc=com") self.assertEqual(entry.loginShell, "/bin/zsh") entry.loginShell = None - self.assertEqual(entry.loginShell, None) + self.assertIsNone(entry.loginShell) entry.save() entry.fetch() - self.assertRaises(AttributeError, getattr, entry, "loginShell") + self.assertIsNone(entry.loginShell) def test_modify_multi_value_attribute(self): entry = self.ldap_connection.get_entry( @@ -249,8 +249,7 @@ def test_entry_empty_multi_value_attribute(self): def test_entry_nonexistant_single_value_attribute(self): entry = self.ldap_connection.get_entry("cn=daniel,dc=example,dc=com") del entry.loginShell - with self.assertRaises(AttributeError): - entry.loginShell + self.assertIsNone(entry.loginShell) def test_retrieve_operational_attributes(self): # Test for https://github.com/HaDiNet/ldapom/issues/29 From a10b0f3fee336c3410b7fb74ef4e8df92e2fbd8e Mon Sep 17 00:00:00 2001 From: Florian Richter Date: Tue, 17 Mar 2015 23:13:10 +0100 Subject: [PATCH 5/5] Fix writes of unchanged empty attributes Some multi-value attributes were written although they where never changed. Accessing them created an empty set, which is different from the old value None. This unintented writing caused errors, when the user only has permission to read the attribute. The test-suite was updated to use a none-admin user for testing. --- ldapom/connection.py | 14 ++++++++++---- ldapom/entry.py | 1 + test_server/slapd.conf | 34 +++++++++++++++++++++------------- test_server/testdata.ldif | 13 +++++++++++++ tests.py | 23 ++++++++++++++++++----- 5 files changed, 63 insertions(+), 22 deletions(-) diff --git a/ldapom/connection.py b/ldapom/connection.py index b89fe72..bb8759c 100644 --- a/ldapom/connection.py +++ b/ldapom/connection.py @@ -24,6 +24,9 @@ LDAP_OPT_X_TLS_TRY = libldap.LDAP_OPT_X_TLS_TRY +def _filter_empty_attributes(attributes): + return {attr for attr in attributes if len(attr._values) > 0} + def handle_ldap_error(err): """Given an LDAP error code, raise an error if needed. @@ -397,17 +400,20 @@ def save(self, entry): if entry_exists: assert entry._fetched_attributes is not None - changed_attributes = entry._attributes - entry._fetched_attributes + # remove empty attributes to avoid unnecessary writes + attributes_before = _filter_empty_attributes(entry._fetched_attributes) + attributes_after = _filter_empty_attributes(entry._attributes) + changed_attributes = attributes_after - attributes_before # Deleted attributes are represented as empty attributes to the LDAP server. - deleted_attribute_names = (frozenset(a.name for a in entry._fetched_attributes) - - frozenset(a.name for a in entry._attributes)) + deleted_attribute_names = (frozenset(a.name for a in attributes_before) + - frozenset(a.name for a in attributes_after)) for deleted_name in deleted_attribute_names: deleted_attribute_type = self.get_attribute_type(deleted_name) changed_attributes.add(deleted_attribute_type(deleted_name)) else: # Don't try to save empty attributes as this fails if the entry does # not exist on the server yet. - changed_attributes = set(filter(lambda attr: len(attr._values) > 0, entry._attributes)) + changed_attributes = _filter_empty_attributes(entry._attributes) # Don't try to save an empty modification set if not changed_attributes: diff --git a/ldapom/entry.py b/ldapom/entry.py index 48b49bd..2f58dc5 100644 --- a/ldapom/entry.py +++ b/ldapom/entry.py @@ -87,6 +87,7 @@ def __getattr__(self, name): return attribute.values else: if attribute_type.multi_value: + # keep reference, so changes are tracked setattr(self, name, set()) return self.get_attribute(name).values else: diff --git a/test_server/slapd.conf b/test_server/slapd.conf index 3380340..23d71c3 100644 --- a/test_server/slapd.conf +++ b/test_server/slapd.conf @@ -29,19 +29,33 @@ pidfile slapd.pid # Allow authenticated users read access # Allow anonymous users to authenticate # Directives needed to implement policy: -# access to dn.base="" by * read -# access to dn.base="cn=Subschema" by * read -# access to * -# by self write -# by users read -# by anonymous auth -# +access to dn.base="" by * read +access to dn.base="cn=Subschema" by * read + # if no access controls are present, the default policy # allows anyone and everyone to read anything but restricts # updates to rootdn. (e.g., "access to * by * read") # # rootdn can always read and write EVERYTHING! +# Users can change some of their own attributes +access to attrs=userPassword + by self write + by anonymous auth + by dn="cn=admin,dc=example,dc=com" write + by * break + +# Daniel may not write to attribute givenName for test_entry_access_empty_attribute +access to attrs=givenName + by dn="cn=daniel,dc=example,dc=com" read + by dn="cn=admin,dc=example,dc=com" write + by * break + +access to * + by dn="cn=daniel,dc=example,dc=com" write + by dn="cn=admin,dc=example,dc=com" write + by * break + ####################################################################### # BDB database definitions ####################################################################### @@ -59,11 +73,5 @@ rootpw admin # Mode 700 recommended. directory ldapdata -# Users can change some of their own attributes (only if they are active) -#access to attrs=userPassword -# by self write -# by anonymous auth -# by * break - # Indices to maintain #index objectClass eq diff --git a/test_server/testdata.ldif b/test_server/testdata.ldif index 28c40da..f473218 100644 --- a/test_server/testdata.ldif +++ b/test_server/testdata.ldif @@ -12,6 +12,19 @@ gidNumber: 10001 objectClass: posixGroup memberUid: luke +# admin, example.com +dn: cn=admin,dc=example,dc=com +objectClass: person +objectClass: posixAccount +cn: admin +gidNumber: 11000 +homeDirectory: /home/jack +sn: admin +uid: admin +uidNumber: 11142 +userPassword: admin +loginShell: /bin/bash + # jack, example.com dn: cn=jack,dc=example,dc=com objectClass: person diff --git a/tests.py b/tests.py index 44fbc77..6c91563 100755 --- a/tests.py +++ b/tests.py @@ -20,8 +20,8 @@ def setUp(self): self.ldap_connection = ldapom.LDAPConnection( uri=self.ldap_server.ldapi_url(), base='dc=example,dc=com', - bind_dn='cn=admin,dc=example,dc=com', - bind_password='admin') + bind_dn='cn=daniel,dc=example,dc=com', + bind_password='daniel') def tearDown(self): self.ldap_server.stop() @@ -182,7 +182,7 @@ def test_dn_computed_properties(self): def test_search(self): result = self.ldap_connection.search("cn=*n*") - self.assertEqual(set(["Noël", "daniel"]), + self.assertEqual(set(["Noël", "daniel", "admin"]), set([next(iter(r.cn)) for r in result])) def test_search_empty_result(self): @@ -191,12 +191,12 @@ def test_search_empty_result(self): def test_rename(self): entry = self.ldap_connection.get_entry( - "cn=daniel,dc=example,dc=com") + "cn=jack,dc=example,dc=com") entry.rename("cn=dieter,dc=example,dc=com") self.assertTrue(entry.exists()) entry = self.ldap_connection.get_entry( - "cn=daniel,dc=example,dc=com") + "cn=jack,dc=example,dc=com") self.assertFalse(entry.exists()) entry = self.ldap_connection.get_entry( @@ -246,6 +246,19 @@ def test_entry_empty_multi_value_attribute(self): entry = self.ldap_connection.get_entry("cn=daniel,dc=example,dc=com") self.assertEqual({'superman'}, entry.description) + def test_entry_access_empty_attribute(self): + ldap_connection = ldapom.LDAPConnection( + uri=self.ldap_server.ldapi_url(), + base='dc=example,dc=com', + bind_dn='cn=daniel,dc=example,dc=com', + bind_password='daniel') + entry = ldap_connection.get_entry("cn=jack,dc=example,dc=com") + self.assertEqual(set(), entry.givenName) + + # Ensure that saving doesn't trigger a write to unchanged and empty givenName + entry.save() + self.assertEqual(set(), entry.givenName) + def test_entry_nonexistant_single_value_attribute(self): entry = self.ldap_connection.get_entry("cn=daniel,dc=example,dc=com") del entry.loginShell