From 028b6f6cb7dc3de4414bdac2b992542866c38b81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20P=C3=A9pin?= Date: Tue, 16 Jun 2020 17:21:59 +0200 Subject: [PATCH 1/2] Switch to python-ldap (instead of ldap3) --- gestioncof/tests/test_views.py | 7 +++--- requirements-prod.txt | 2 +- shared/tests/mixins.py | 41 +++++++++++++++++----------------- shared/views/autocomplete.py | 22 ++++++++++++------ 4 files changed, 41 insertions(+), 31 deletions(-) diff --git a/gestioncof/tests/test_views.py b/gestioncof/tests/test_views.py index d522a648..09e86860 100644 --- a/gestioncof/tests/test_views.py +++ b/gestioncof/tests/test_views.py @@ -317,10 +317,11 @@ class RegistrationAutocompleteViewTests(MockLDAPMixin, ViewTestCaseMixin, TestCa self._test("aa bb", [], [], [Clipper("uid", "first last")]) - mock_ldap.search.assert_called_once_with( + mock_ldap.ldap_obj.search_s.assert_called_once_with( "dc=spi,dc=ens,dc=fr", + mock_ldap.SCOPE_SUBTREE, "(&(|(cn=*aa*)(uid=*aa*))(|(cn=*bb*)(uid=*bb*)))", - attributes=["cn", "uid"], + ["cn", "uid"], ) def test_clipper_escaped(self): @@ -328,7 +329,7 @@ class RegistrationAutocompleteViewTests(MockLDAPMixin, ViewTestCaseMixin, TestCa self._test("; & | (", [], [], []) - mock_ldap.search.assert_not_called() + mock_ldap.ldap_obj.search_s.assert_not_called() def test_clipper_no_duplicate(self): self.mockLDAP([("uid", "abc")]) diff --git a/requirements-prod.txt b/requirements-prod.txt index e08ac120..a137dd67 100644 --- a/requirements-prod.txt +++ b/requirements-prod.txt @@ -11,4 +11,4 @@ asgiref==1.1.1 daphne==1.3.0 # ldap bindings -ldap3 +python-ldap diff --git a/shared/tests/mixins.py b/shared/tests/mixins.py index 8a00480e..030b3d5c 100644 --- a/shared/tests/mixins.py +++ b/shared/tests/mixins.py @@ -22,32 +22,33 @@ class MockLDAPMixin: appeler `with Connection(*args, **kwargs) as foo` pour que le test fonctionne. """ + class MockLDAPModule: + SCOPE_SUBTREE = None # whatever + + def __init__(self, ldap_obj): + self.ldap_obj = ldap_obj + + def initialize(self, *args): + """Always return the same ldap object.""" + return self.ldap_obj + def mockLDAP(self, results): - class Elt: - def __init__(self, value): - self.value = value + entries = [ + ("whatever", {"cn": [name.encode("utf-8")], "uid": [uid.encode("utf-8")]}) + for uid, name in results + ] + # Mock ldap object whose `search_s` method always returns the same results. + mock_ldap_obj = mock.Mock() + mock_ldap_obj.search_s = mock.Mock(return_value=entries) - class Entry: - def __init__(self, **kwargs): - for k, v in kwargs.items(): - setattr(self, k, Elt(v)) + # Mock ldap module whose `initialize_method` always return the same ldap object. + mock_ldap_module = self.MockLDAPModule(mock_ldap_obj) - results_as_ldap = [Entry(uid=uid, cn=name) for uid, name in results] - - mock_connection = mock.MagicMock() - mock_connection.entries = results_as_ldap - - # Connection is used as a context manager. - mock_context_manager = mock.MagicMock() - mock_context_manager.return_value.__enter__.return_value = mock_connection - - patcher = mock.patch( - "shared.views.autocomplete.Connection", new=mock_context_manager - ) + patcher = mock.patch("shared.views.autocomplete.ldap", new=mock_ldap_module) patcher.start() self.addCleanup(patcher.stop) - return mock_connection + return mock_ldap_module class CSVResponseMixin: diff --git a/shared/views/autocomplete.py b/shared/views/autocomplete.py index af5e3980..168abc4b 100644 --- a/shared/views/autocomplete.py +++ b/shared/views/autocomplete.py @@ -5,11 +5,11 @@ from django.conf import settings from django.db.models import Q if getattr(settings, "LDAP_SERVER_URL", None): - from ldap3 import Connection + import ldap else: # shared.tests.testcases.TestCaseMixin.mockLDAP needs - # Connection to be defined - Connection = None + # an ldap object to be in the scope + ldap = None class SearchUnit: @@ -125,12 +125,20 @@ class LDAPSearch(SearchUnit): query = self.get_ldap_query(keywords) - if Connection is None or query == "(&)": + if ldap is None or query == "(&)": return [] - with Connection(self.ldap_server_url) as conn: - conn.search(self.domain_component, query, attributes=self.search_fields) - return [Clipper(entry.uid.value, entry.cn.value) for entry in conn.entries] + ldap_obj = ldap.initialize(self.ldap_server_url) + res = ldap_obj.search_s( + self.domain_component, ldap.SCOPE_SUBTREE, query, self.search_fields + ) + return [ + Clipper( + clipper=attrs["uid"][0].decode("utf-8"), + fullname=attrs["cn"][0].decode("utf-8"), + ) + for (_, attrs) in res + ] # --- From b9ba0a38296f8297305f6f20e9a55fe660a6f0e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20P=C3=A9pin?= Date: Sat, 20 Jun 2020 17:49:56 +0200 Subject: [PATCH 2/2] Add missing ldap system dependencies to CI config --- .gitlab-ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 6bb31a5f..810c1132 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -22,7 +22,7 @@ test: stage: test before_script: - mkdir -p vendor/{pip,apt} - - apt-get update -q && apt-get -o dir::cache::archives="vendor/apt" install -yqq postgresql-client + - apt-get update -q && apt-get -o dir::cache::archives="vendor/apt" install -yqq postgresql-client libldap2-dev libsasl2-dev - sed -E 's/^REDIS_HOST.*/REDIS_HOST = "redis"/' cof/settings/secret_example.py > cof/settings/secret.py - sed -i.bak -E 's;^REDIS_PASSWD = .*$;REDIS_PASSWD = "";' cof/settings/secret.py # Remove the old test database if it has not been done yet @@ -64,7 +64,7 @@ migration_checks: stage: test before_script: - mkdir -p vendor/{pip,apt} - - apt-get update -q && apt-get -o dir::cache::archives="vendor/apt" install -yqq postgresql-client + - apt-get update -q && apt-get -o dir::cache::archives="vendor/apt" install -yqq postgresql-client libldap2-dev libsasl2-dev - cp cof/settings/secret_example.py cof/settings/secret.py - pip install --upgrade -r requirements-prod.txt - python --version