sources/ldap: make schema optional (#5213)

* sources/ldap: make schema optional Signed-off-by: Jens Langhammer <[email protected]> * create one connection and re-use it Signed-off-by: Jens Langhammer <[email protected]> * use magicmock Signed-off-by: Jens Langhammer <[email protected]> --------- Signed-off-by: Jens Langhammer <[email protected]>
goauthentik · Apr 10, 2023 · 1ca8feb · 1ca8feb
1 parent c1615d0
commit 1ca8feb
Show file tree

Hide file tree

Showing 12 changed files with 63 additions and 47 deletions.
diff --git a/authentik/blueprints/v1/tasks.py b/authentik/blueprints/v1/tasks.py
@@ -122,7 +122,7 @@ def blueprints_find():
         )
         blueprint.meta = from_dict(BlueprintMetadata, metadata) if metadata else None
         blueprints.append(blueprint)
-        LOGGER.info(
+        LOGGER.debug(
             "parsed & loaded blueprint",
             hash=file_hash,
             path=str(path),

diff --git a/authentik/root/test_runner.py b/authentik/root/test_runner.py
@@ -16,7 +16,7 @@ def __init__(self, verbosity=1, failfast=False, keepdb=False, **kwargs):
         self.failfast = failfast
         self.keepdb = keepdb
 
-        self.args = ["-vv"]
+        self.args = ["-vv", "--full-trace"]
         if self.failfast:
             self.args.append("--exitfirst")
         if self.keepdb:

diff --git a/authentik/sources/ldap/auth.py b/authentik/sources/ldap/auth.py
@@ -2,13 +2,12 @@
 from typing import Optional
 
 from django.http import HttpRequest
-from ldap3 import Connection
 from ldap3.core.exceptions import LDAPException, LDAPInvalidCredentialsResult
 from structlog.stdlib import get_logger
 
 from authentik.core.auth import InbuiltBackend
 from authentik.core.models import User
-from authentik.sources.ldap.models import LDAP_TIMEOUT, LDAPSource
+from authentik.sources.ldap.models import LDAPSource
 
 LOGGER = get_logger()
 LDAP_DISTINGUISHED_NAME = "distinguishedName"
@@ -58,12 +57,11 @@ def auth_user_by_bind(self, source: LDAPSource, user: User, password: str) -> Op
         # Try to bind as new user
         LOGGER.debug("Attempting Binding as user", user=user)
         try:
-            temp_connection = Connection(
-                source.server,
-                user=user.attributes.get(LDAP_DISTINGUISHED_NAME),
-                password=password,
-                raise_exceptions=True,
-                receive_timeout=LDAP_TIMEOUT,
+            temp_connection = source.connection(
+                connection_kwargs={
+                    "user": user.attributes.get(LDAP_DISTINGUISHED_NAME),
+                    "password": password,
+                }
             )
             temp_connection.bind()
             return user

diff --git a/authentik/sources/ldap/models.py b/authentik/sources/ldap/models.py
@@ -1,9 +1,11 @@
 """authentik LDAP Models"""
 from ssl import CERT_REQUIRED
+from typing import Optional
 
 from django.db import models
 from django.utils.translation import gettext_lazy as _
-from ldap3 import ALL, RANDOM, Connection, Server, ServerPool, Tls
+from ldap3 import ALL, NONE, RANDOM, Connection, Server, ServerPool, Tls
+from ldap3.core.exceptions import LDAPSchemaError
 from rest_framework.serializers import Serializer
 
 from authentik.core.models import Group, PropertyMapping, Source
@@ -103,8 +105,7 @@ def serializer(self) -> type[Serializer]:
 
         return LDAPSourceSerializer
 
-    @property
-    def server(self) -> Server:
+    def server(self, **kwargs) -> Server:
         """Get LDAP Server/ServerPool"""
         servers = []
         tls_kwargs = {}
@@ -113,32 +114,45 @@ def server(self) -> Server:
             tls_kwargs["validate"] = CERT_REQUIRED
         if ciphers := CONFIG.y("ldap.tls.ciphers", None):
             tls_kwargs["ciphers"] = ciphers.strip()
-        kwargs = {
+        server_kwargs = {
             "get_info": ALL,
             "connect_timeout": LDAP_TIMEOUT,
             "tls": Tls(**tls_kwargs),
         }
+        server_kwargs.update(kwargs)
         if "," in self.server_uri:
             for server in self.server_uri.split(","):
-                servers.append(Server(server, **kwargs))
+                servers.append(Server(server, **server_kwargs))
         else:
-            servers = [Server(self.server_uri, **kwargs)]
+            servers = [Server(self.server_uri, **server_kwargs)]
         return ServerPool(servers, RANDOM, active=True, exhaust=True)
 
-    @property
-    def connection(self) -> Connection:
+    def connection(
+        self, server_kwargs: Optional[dict] = None, connection_kwargs: Optional[dict] = None
+    ) -> Connection:
         """Get a fully connected and bound LDAP Connection"""
+        server_kwargs = server_kwargs or {}
+        connection_kwargs = connection_kwargs or {}
+        connection_kwargs.setdefault("user", self.bind_cn)
+        connection_kwargs.setdefault("password", self.bind_password)
         connection = Connection(
-            self.server,
+            self.server(**server_kwargs),
             raise_exceptions=True,
-            user=self.bind_cn,
-            password=self.bind_password,
             receive_timeout=LDAP_TIMEOUT,
+            **connection_kwargs,
         )
 
         if self.start_tls:
             connection.start_tls(read_server_info=False)
-        connection.bind()
+        try:
+            connection.bind()
+        except LDAPSchemaError as exc:
+            # Schema error, so try connecting without schema info
+            # See https://github.com/goauthentik/authentik/issues/4590
+            if server_kwargs.get("get_info", ALL) == NONE:
+                raise exc
+            server_kwargs["get_info"] = NONE
+            return self.connection(server_kwargs, connection_kwargs)
         return connection
 
     class Meta:

diff --git a/authentik/sources/ldap/password.py b/authentik/sources/ldap/password.py
@@ -47,10 +47,11 @@ class LDAPPasswordChanger:
 
     def __init__(self, source: LDAPSource) -> None:
         self._source = source
+        self._connection = source.connection()
 
     def get_domain_root_dn(self) -> str:
         """Attempt to get root DN via MS specific fields or generic LDAP fields"""
-        info = self._source.connection.server.info
+        info = self._connection.server.info
         if "rootDomainNamingContext" in info.other:
             return info.other["rootDomainNamingContext"][0]
         naming_contexts = info.naming_contexts
@@ -61,7 +62,7 @@ def check_ad_password_complexity_enabled(self) -> bool:
         """Check if DOMAIN_PASSWORD_COMPLEX is enabled"""
         root_dn = self.get_domain_root_dn()
         try:
-            root_attrs = self._source.connection.extend.standard.paged_search(
+            root_attrs = self._connection.extend.standard.paged_search(
                 search_base=root_dn,
                 search_filter="(objectClass=*)",
                 search_scope=BASE,
@@ -90,14 +91,14 @@ def change_password(self, user: User, password: str):
             LOGGER.info(f"User has no {LDAP_DISTINGUISHED_NAME} set.")
             return
         try:
-            self._source.connection.extend.microsoft.modify_password(user_dn, password)
+            self._connection.extend.microsoft.modify_password(user_dn, password)
         except LDAPAttributeError:
-            self._source.connection.extend.standard.modify_password(user_dn, new_password=password)
+            self._connection.extend.standard.modify_password(user_dn, new_password=password)
 
     def _ad_check_password_existing(self, password: str, user_dn: str) -> bool:
         """Check if a password contains sAMAccount or displayName"""
         users = list(
-            self._source.connection.extend.standard.paged_search(
+            self._connection.extend.standard.paged_search(
                 search_base=user_dn,
                 search_filter=self._source.user_object_filter,
                 search_scope=BASE,

diff --git a/authentik/sources/ldap/sync/base.py b/authentik/sources/ldap/sync/base.py
@@ -3,6 +3,7 @@
 
 from django.db.models.base import Model
 from django.db.models.query import QuerySet
+from ldap3 import Connection
 from structlog.stdlib import BoundLogger, get_logger
 
 from authentik.core.exceptions import PropertyMappingExpressionException
@@ -19,10 +20,12 @@ class BaseLDAPSynchronizer:
 
     _source: LDAPSource
     _logger: BoundLogger
+    _connection: Connection
     _messages: list[str]
 
     def __init__(self, source: LDAPSource):
         self._source = source
+        self._connection = source.connection()
         self._messages = []
         self._logger = get_logger().bind(source=source, syncer=self.__class__.__name__)
 

diff --git a/authentik/sources/ldap/sync/groups.py b/authentik/sources/ldap/sync/groups.py
@@ -14,7 +14,7 @@ class GroupLDAPSynchronizer(BaseLDAPSynchronizer):
     """Sync LDAP Users and groups into authentik"""
 
     def get_objects(self, **kwargs) -> Generator:
-        return self._source.connection.extend.standard.paged_search(
+        return self._connection.extend.standard.paged_search(
             search_base=self.base_dn_groups,
             search_filter=self._source.group_object_filter,
             search_scope=SUBTREE,

diff --git a/authentik/sources/ldap/sync/membership.py b/authentik/sources/ldap/sync/membership.py
@@ -20,7 +20,7 @@ def __init__(self, source: LDAPSource):
         self.group_cache: dict[str, Group] = {}
 
     def get_objects(self, **kwargs) -> Generator:
-        return self._source.connection.extend.standard.paged_search(
+        return self._connection.extend.standard.paged_search(
             search_base=self.base_dn_groups,
             search_filter=self._source.group_object_filter,
             search_scope=SUBTREE,

diff --git a/authentik/sources/ldap/sync/users.py b/authentik/sources/ldap/sync/users.py
@@ -16,7 +16,7 @@ class UserLDAPSynchronizer(BaseLDAPSynchronizer):
     """Sync LDAP Users into authentik"""
 
     def get_objects(self, **kwargs) -> Generator:
-        return self._source.connection.extend.standard.paged_search(
+        return self._connection.extend.standard.paged_search(
             search_base=self.base_dn_users,
             search_filter=self._source.user_object_filter,
             search_scope=SUBTREE,

diff --git a/authentik/sources/ldap/tests/test_auth.py b/authentik/sources/ldap/tests/test_auth.py
@@ -1,5 +1,5 @@
 """LDAP Source tests"""
-from unittest.mock import Mock, PropertyMock, patch
+from unittest.mock import MagicMock, Mock, patch
 
 from django.db.models import Q
 from django.test import TestCase
@@ -37,7 +37,7 @@ def test_auth_synced_user_ad(self):
                 | Q(managed__startswith="goauthentik.io/sources/ldap/ms-")
             )
         )
-        connection = PropertyMock(return_value=mock_ad_connection(LDAP_PASSWORD))
+        connection = MagicMock(return_value=mock_ad_connection(LDAP_PASSWORD))
         with patch("authentik.sources.ldap.models.LDAPSource.connection", connection):
             user_sync = UserLDAPSynchronizer(self.source)
             user_sync.sync()
@@ -64,7 +64,7 @@ def test_auth_synced_user_openldap(self):
             )
         )
         self.source.save()
-        connection = PropertyMock(return_value=mock_slapd_connection(LDAP_PASSWORD))
+        connection = MagicMock(return_value=mock_slapd_connection(LDAP_PASSWORD))
         with patch("authentik.sources.ldap.models.LDAPSource.connection", connection):
             user_sync = UserLDAPSynchronizer(self.source)
             user_sync.sync()

diff --git a/authentik/sources/ldap/tests/test_password.py b/authentik/sources/ldap/tests/test_password.py
@@ -1,5 +1,5 @@
 """LDAP Source tests"""
-from unittest.mock import PropertyMock, patch
+from unittest.mock import MagicMock, patch
 
 from django.test import TestCase
 
@@ -10,7 +10,7 @@
 from authentik.sources.ldap.tests.mock_ad import mock_ad_connection
 
 LDAP_PASSWORD = generate_key()
-LDAP_CONNECTION_PATCH = PropertyMock(return_value=mock_ad_connection(LDAP_PASSWORD))
+LDAP_CONNECTION_PATCH = MagicMock(return_value=mock_ad_connection(LDAP_PASSWORD))
 
 
 class LDAPPasswordTests(TestCase):

diff --git a/authentik/sources/ldap/tests/test_sync.py b/authentik/sources/ldap/tests/test_sync.py
@@ -1,5 +1,5 @@
 """LDAP Source tests"""
-from unittest.mock import PropertyMock, patch
+from unittest.mock import MagicMock, patch
 
 from django.db.models import Q
 from django.test import TestCase
@@ -48,7 +48,7 @@ def test_sync_error(self):
         )
         self.source.property_mappings.set([mapping])
         self.source.save()
-        connection = PropertyMock(return_value=mock_ad_connection(LDAP_PASSWORD))
+        connection = MagicMock(return_value=mock_ad_connection(LDAP_PASSWORD))
         with patch("authentik.sources.ldap.models.LDAPSource.connection", connection):
             user_sync = UserLDAPSynchronizer(self.source)
             user_sync.sync()
@@ -69,7 +69,7 @@ def test_sync_users_ad(self):
             )
         )
         self.source.save()
-        connection = PropertyMock(return_value=mock_ad_connection(LDAP_PASSWORD))
+        connection = MagicMock(return_value=mock_ad_connection(LDAP_PASSWORD))
 
         # Create the user beforehand so we can set attributes and check they aren't removed
         user = User.objects.create(
@@ -103,7 +103,7 @@ def test_sync_users_openldap(self):
             )
         )
         self.source.save()
-        connection = PropertyMock(return_value=mock_slapd_connection(LDAP_PASSWORD))
+        connection = MagicMock(return_value=mock_slapd_connection(LDAP_PASSWORD))
         with patch("authentik.sources.ldap.models.LDAPSource.connection", connection):
             user_sync = UserLDAPSynchronizer(self.source)
             user_sync.sync()
@@ -121,11 +121,11 @@ def test_sync_groups_ad(self):
         self.source.property_mappings_group.set(
             LDAPPropertyMapping.objects.filter(managed="goauthentik.io/sources/ldap/default-name")
         )
-        _user = create_test_admin_user()
-        parent_group = Group.objects.get(name=_user.username)
-        self.source.sync_parent_group = parent_group
-        connection = PropertyMock(return_value=mock_ad_connection(LDAP_PASSWORD))
+        connection = MagicMock(return_value=mock_ad_connection(LDAP_PASSWORD))
         with patch("authentik.sources.ldap.models.LDAPSource.connection", connection):
+            _user = create_test_admin_user()
+            parent_group = Group.objects.get(name=_user.username)
+            self.source.sync_parent_group = parent_group
             self.source.save()
             group_sync = GroupLDAPSynchronizer(self.source)
             group_sync.sync()
@@ -148,7 +148,7 @@ def test_sync_groups_openldap(self):
         self.source.property_mappings_group.set(
             LDAPPropertyMapping.objects.filter(managed="goauthentik.io/sources/ldap/openldap-cn")
         )
-        connection = PropertyMock(return_value=mock_slapd_connection(LDAP_PASSWORD))
+        connection = MagicMock(return_value=mock_slapd_connection(LDAP_PASSWORD))
         with patch("authentik.sources.ldap.models.LDAPSource.connection", connection):
             self.source.save()
             group_sync = GroupLDAPSynchronizer(self.source)
@@ -173,7 +173,7 @@ def test_sync_groups_openldap_posix_group(self):
         self.source.property_mappings_group.set(
             LDAPPropertyMapping.objects.filter(managed="goauthentik.io/sources/ldap/openldap-cn")
         )
-        connection = PropertyMock(return_value=mock_slapd_connection(LDAP_PASSWORD))
+        connection = MagicMock(return_value=mock_slapd_connection(LDAP_PASSWORD))
         with patch("authentik.sources.ldap.models.LDAPSource.connection", connection):
             self.source.save()
             user_sync = UserLDAPSynchronizer(self.source)
@@ -195,7 +195,7 @@ def test_tasks_ad(self):
             )
         )
         self.source.save()
-        connection = PropertyMock(return_value=mock_ad_connection(LDAP_PASSWORD))
+        connection = MagicMock(return_value=mock_ad_connection(LDAP_PASSWORD))
         with patch("authentik.sources.ldap.models.LDAPSource.connection", connection):
             ldap_sync_all.delay().get()
 
@@ -210,6 +210,6 @@ def test_tasks_openldap(self):
             )
         )
         self.source.save()
-        connection = PropertyMock(return_value=mock_slapd_connection(LDAP_PASSWORD))
+        connection = MagicMock(return_value=mock_slapd_connection(LDAP_PASSWORD))
         with patch("authentik.sources.ldap.models.LDAPSource.connection", connection):
             ldap_sync_all.delay().get()