From 4fd8b3c532b27d1d63784635b23893fca38f56fb Mon Sep 17 00:00:00 2001 From: "Jens L." Date: Tue, 24 Sep 2024 18:39:05 +0200 Subject: [PATCH] sources/ldap: fix mapping check, fix debug endpoint (#11442) * run connectivity check always Signed-off-by: Jens Langhammer * don't run sync if either sync_ option is enabled and no mappings are set Signed-off-by: Jens Langhammer * misc label fix Signed-off-by: Jens Langhammer * misc writing changse Signed-off-by: Jens Langhammer * add api validation Signed-off-by: Jens Langhammer * fix debug endpoint Signed-off-by: Jens Langhammer --------- Signed-off-by: Jens Langhammer --- authentik/sources/ldap/api.py | 37 +++++++++++++++----- authentik/sources/ldap/signals.py | 9 +++-- authentik/sources/ldap/tests/test_api.py | 32 +++++++++++++++++ web/src/admin/sources/ldap/LDAPSourceForm.ts | 2 +- website/docs/sources/ldap/index.md | 12 ++++--- 5 files changed, 74 insertions(+), 18 deletions(-) diff --git a/authentik/sources/ldap/api.py b/authentik/sources/ldap/api.py index be7625c0a9..89ef164cd7 100644 --- a/authentik/sources/ldap/api.py +++ b/authentik/sources/ldap/api.py @@ -3,6 +3,7 @@ from typing import Any from django.core.cache import cache +from django.utils.translation import gettext_lazy as _ from drf_spectacular.utils import extend_schema, inline_serializer from guardian.shortcuts import get_objects_for_user from rest_framework.decorators import action @@ -39,9 +40,8 @@ class LDAPSourceSerializer(SourceSerializer): """Get cached source connectivity""" return cache.get(CACHE_KEY_STATUS + source.slug, None) - def validate(self, attrs: dict[str, Any]) -> dict[str, Any]: + def validate_sync_users_password(self, sync_users_password: bool) -> bool: """Check that only a single source has password_sync on""" - sync_users_password = attrs.get("sync_users_password", True) if sync_users_password: sources = LDAPSource.objects.filter(sync_users_password=True) if self.instance: @@ -49,11 +49,31 @@ class LDAPSourceSerializer(SourceSerializer): if sources.exists(): raise ValidationError( { - "sync_users_password": ( + "sync_users_password": _( "Only a single LDAP Source with password synchronization is allowed" ) } ) + return sync_users_password + + def validate(self, attrs: dict[str, Any]) -> dict[str, Any]: + """Validate property mappings with sync_ flags""" + types = ["user", "group"] + for type in types: + toggle_value = attrs.get(f"sync_{type}s", False) + mappings_field = f"{type}_property_mappings" + mappings_value = attrs.get(mappings_field, []) + if toggle_value and len(mappings_value) == 0: + raise ValidationError( + { + mappings_field: _( + ( + "When 'Sync {type}s' is enabled, '{type}s property " + "mappings' cannot be empty." + ).format(type=type) + ) + } + ) return super().validate(attrs) class Meta: @@ -166,11 +186,12 @@ class LDAPSourceViewSet(UsedByMixin, ModelViewSet): for sync_class in SYNC_CLASSES: class_name = sync_class.name() all_objects.setdefault(class_name, []) - for obj in sync_class(source).get_objects(size_limit=10): - obj: dict - obj.pop("raw_attributes", None) - obj.pop("raw_dn", None) - all_objects[class_name].append(obj) + for page in sync_class(source).get_objects(size_limit=10): + for obj in page: + obj: dict + obj.pop("raw_attributes", None) + obj.pop("raw_dn", None) + all_objects[class_name].append(obj) return Response(data=all_objects) diff --git a/authentik/sources/ldap/signals.py b/authentik/sources/ldap/signals.py index f53f4ad7f1..a5c869f150 100644 --- a/authentik/sources/ldap/signals.py +++ b/authentik/sources/ldap/signals.py @@ -26,17 +26,16 @@ def sync_ldap_source_on_save(sender, instance: LDAPSource, **_): """Ensure that source is synced on save (if enabled)""" if not instance.enabled: return + ldap_connectivity_check.delay(instance.pk) # Don't sync sources when they don't have any property mappings. This will only happen if: # - the user forgets to set them or # - the source is newly created, this is the first save event # and the mappings are created with an m2m event - if ( - not instance.user_property_mappings.exists() - or not instance.group_property_mappings.exists() - ): + if instance.sync_users and not instance.user_property_mappings.exists(): + return + if instance.sync_groups and not instance.group_property_mappings.exists(): return ldap_sync_single.delay(instance.pk) - ldap_connectivity_check.delay(instance.pk) @receiver(password_validate) diff --git a/authentik/sources/ldap/tests/test_api.py b/authentik/sources/ldap/tests/test_api.py index e7dcf42320..778d58ad80 100644 --- a/authentik/sources/ldap/tests/test_api.py +++ b/authentik/sources/ldap/tests/test_api.py @@ -50,3 +50,35 @@ class LDAPAPITests(APITestCase): } ) self.assertFalse(serializer.is_valid()) + + def test_sync_users_mapping_empty(self): + """Check that when sync_users is enabled, property mappings must be set""" + serializer = LDAPSourceSerializer( + data={ + "name": "foo", + "slug": " foo", + "server_uri": "ldaps://1.2.3.4", + "bind_cn": "", + "bind_password": LDAP_PASSWORD, + "base_dn": "dc=foo", + "sync_users": True, + "user_property_mappings": [], + } + ) + self.assertFalse(serializer.is_valid()) + + def test_sync_groups_mapping_empty(self): + """Check that when sync_groups is enabled, property mappings must be set""" + serializer = LDAPSourceSerializer( + data={ + "name": "foo", + "slug": " foo", + "server_uri": "ldaps://1.2.3.4", + "bind_cn": "", + "bind_password": LDAP_PASSWORD, + "base_dn": "dc=foo", + "sync_groups": True, + "group_property_mappings": [], + } + ) + self.assertFalse(serializer.is_valid()) diff --git a/web/src/admin/sources/ldap/LDAPSourceForm.ts b/web/src/admin/sources/ldap/LDAPSourceForm.ts index 3004abec19..ecca7d4a7d 100644 --- a/web/src/admin/sources/ldap/LDAPSourceForm.ts +++ b/web/src/admin/sources/ldap/LDAPSourceForm.ts @@ -327,7 +327,7 @@ export class LDAPSourceForm extends BaseSourceForm { ${msg("Additional settings")}
- + => { const args: CoreGroupsListRequest = { diff --git a/website/docs/sources/ldap/index.md b/website/docs/sources/ldap/index.md index 1cb2456056..0fcc59253c 100644 --- a/website/docs/sources/ldap/index.md +++ b/website/docs/sources/ldap/index.md @@ -18,13 +18,13 @@ To create or edit a source in authentik, open the Admin interface and navigate t - **Update internal password on login**: When the user logs in to authentik using the LDAP password backend, the password is stored as a hashed value in authentik. Toggle off (default setting) if you do not want to store the hashed passwords in authentik. -- **Synch User**: Enable or disable user synchronization between authentik and the LDAP source. +- **Sync users**: Enable or disable user synchronization between authentik and the LDAP source. - **User password writeback**: Enable this option if you want to write password changes that are made in authentik back to LDAP. -- **Synch groups**: Enable/disable group synchronization. Groups are synced in the background every 5 minutes. +- **Sync groups**: Enable/disable group synchronization. Groups are synced in the background every 5 minutes. -- **Sync parent group**: Optionally set this group as the parent group for all synced groups. An example use case of this would be to import Active Directory groups under a root `imported-from-ad` group. +- **Parent group**: Optionally set this group as the parent group for all synced groups. An example use case of this would be to import Active Directory groups under a root `imported-from-ad` group. #### Connection settings @@ -45,7 +45,11 @@ To create or edit a source in authentik, open the Admin interface and navigate t #### LDAP Attribute mapping -- **User Property mappings** and **Group Property Mappings**: Define which LDAP properties map to which authentik properties. The default set of property mappings is generated for Active Directory. See also our documentation on [property mappings](#ldap-source-property-mappings). +- **User Property Mappings** and **Group Property Mappings**: Define which LDAP properties map to which authentik properties. The default set of property mappings is generated for Active Directory. See also our documentation on [property mappings](#ldap-source-property-mappings). + + :::warning + When the **Sync users** and or the **Sync groups** options are enabled, their respective property mapping options must have at least one mapping selected, otherwise the sync will not start. + ::: #### Additional Settings