From 1ca8feb5fcd7070a1825ef28c7b2c4baaa2b245a Mon Sep 17 00:00:00 2001 From: Jens L Date: Mon, 10 Apr 2023 21:55:56 +0200 Subject: [PATCH] sources/ldap: make schema optional (#5213) * sources/ldap: make schema optional Signed-off-by: Jens Langhammer * create one connection and re-use it Signed-off-by: Jens Langhammer * use magicmock Signed-off-by: Jens Langhammer --------- Signed-off-by: Jens Langhammer --- authentik/blueprints/v1/tasks.py | 2 +- authentik/root/test_runner.py | 2 +- authentik/sources/ldap/auth.py | 14 +++---- authentik/sources/ldap/models.py | 38 +++++++++++++------ authentik/sources/ldap/password.py | 11 +++--- authentik/sources/ldap/sync/base.py | 3 ++ authentik/sources/ldap/sync/groups.py | 2 +- authentik/sources/ldap/sync/membership.py | 2 +- authentik/sources/ldap/sync/users.py | 2 +- authentik/sources/ldap/tests/test_auth.py | 6 +-- authentik/sources/ldap/tests/test_password.py | 4 +- authentik/sources/ldap/tests/test_sync.py | 24 ++++++------ 12 files changed, 63 insertions(+), 47 deletions(-) diff --git a/authentik/blueprints/v1/tasks.py b/authentik/blueprints/v1/tasks.py index 6224b8ea7..41733b0b9 100644 --- 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 index 6f07ca4ec..938c9d614 100644 --- a/authentik/root/test_runner.py +++ b/authentik/root/test_runner.py @@ -16,7 +16,7 @@ class PytestTestRunner: # pragma: no cover 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 index 6a9bbd035..e312fd3fc 100644 --- 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 @@ class LDAPBackend(InbuiltBackend): # 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 index 759747c89..4cbddd582 100644 --- 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 @@ class LDAPSource(Source): return LDAPSourceSerializer - @property - def server(self) -> Server: + def server(self, **kwargs) -> Server: """Get LDAP Server/ServerPool""" servers = [] tls_kwargs = {} @@ -113,32 +114,45 @@ class LDAPSource(Source): 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 index 42946abf3..11822c78f 100644 --- 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 @@ class LDAPPasswordChanger: """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 @@ class LDAPPasswordChanger: 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 index ebdfdede4..a2c562964 100644 --- a/authentik/sources/ldap/sync/base.py +++ b/authentik/sources/ldap/sync/base.py @@ -3,6 +3,7 @@ from typing import Any, Generator 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 index 143c8a377..2508bd979 100644 --- 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 index a24cead56..1bb0c8515 100644 --- a/authentik/sources/ldap/sync/membership.py +++ b/authentik/sources/ldap/sync/membership.py @@ -20,7 +20,7 @@ class MembershipLDAPSynchronizer(BaseLDAPSynchronizer): 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 index 739a09619..053fd6142 100644 --- 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 index 39fcfca43..3ba6ae386 100644 --- 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 @@ class LDAPSyncTests(TestCase): | 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 @@ class LDAPSyncTests(TestCase): ) ) 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 index 3b126a6be..aa0101679 100644 --- 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.password import LDAPPasswordChanger 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 index f190a4437..d9e03d0b5 100644 --- 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 @@ class LDAPSyncTests(TestCase): ) 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 @@ class LDAPSyncTests(TestCase): ) ) 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 @@ class LDAPSyncTests(TestCase): ) ) 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 @@ class LDAPSyncTests(TestCase): 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 @@ class LDAPSyncTests(TestCase): 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 @@ class LDAPSyncTests(TestCase): 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 @@ class LDAPSyncTests(TestCase): ) ) 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 @@ class LDAPSyncTests(TestCase): ) ) 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()