diff --git a/aleksis/apps/ldap/preferences.py b/aleksis/apps/ldap/preferences.py
index 53f6ed332c37eaeaff9f427d8494b77ea18a34d1..1fee28579d82fe9235b7f90afb4539660e6b48d8 100644
--- a/aleksis/apps/ldap/preferences.py
+++ b/aleksis/apps/ldap/preferences.py
@@ -35,21 +35,6 @@ class LDAPSyncCreateMissingPersons(BooleanPreference):
verbose_name = _("Create missing persons for LDAP users")
-@site_preferences_registry.register
-class LDAPMatchingFields(ChoicePreference):
- section = ldap
- name = "matching_fields"
- default = ""
- required = False
- verbose_name = _("LDAP sync matching fields")
- choices = [
- ("", "-----"),
- ("match-email", _("Match only on email")),
- ("match-name", _("Match only on name")),
- ("match-email-name", _("Match on email and name")),
- ]
-
-
@site_preferences_registry.register
class EnableLDAPGroupSync(BooleanPreference):
section = ldap
diff --git a/aleksis/apps/ldap/util/ldap_sync.py b/aleksis/apps/ldap/util/ldap_sync.py
index 9ecef8b371ba02a9d2fedf1b82c10637c878acb4..3a826269124b2972f75782c5ee9cfed751b6978a 100644
--- a/aleksis/apps/ldap/util/ldap_sync.py
+++ b/aleksis/apps/ldap/util/ldap_sync.py
@@ -10,7 +10,7 @@ from django.db.models.fields.files import FileField
from django.utils.text import slugify
from django.utils.translation import gettext as _
-from dynamic_preferences.types import StringPreference
+from dynamic_preferences.types import MultipleChoicePreference, StringPreference
from tqdm import tqdm
from aleksis.core.registries import site_preferences_registry
@@ -38,7 +38,7 @@ def ldap_field_to_filename(dn, fieldname):
return f"{slugify(dn)}__{slugify(fieldname)}"
-def from_ldap(value, instance, field, dn, ldap_field):
+def from_ldap(value, field, dn, ldap_field, instance=None):
"""Convert an LDAP value to the Python type of the target field.
This conversion is prone to error because LDAP deliberately breaks
@@ -50,7 +50,7 @@ def from_ldap(value, instance, field, dn, ldap_field):
if isinstance(field, (fields.DateField, fields.DateTimeField)):
# Be opportunistic, but keep old value if conversion fails
value = datetime_from_ldap(value) or value
- elif isinstance(field, FileField):
+ elif isinstance(field, FileField) and instance is not None:
name = ldap_field_to_filename(dn, ldap_field)
content = File(io.BytesIO(value))
@@ -101,6 +101,15 @@ def update_dynamic_preferences():
required = False
default = ""
+ @site_preferences_registry.register
+ class LDAPMatchingFields(MultipleChoicePreference):
+ section = section_ldap
+ name = "matching_fields"
+ default = []
+ required = False
+ verbose_name = _("LDAP sync matching fields")
+ choices = [(field.name, field.name) for field in Person.syncable_fields()]
+
def apply_templates(value, patterns, templates, separator="|"):
"""Regex-replace patterns in value in order."""
@@ -119,6 +128,41 @@ def apply_templates(value, patterns, templates, separator="|"):
return value
+def get_ldap_value_for_field(model, field, attrs, dn, instance=None, allow_missing=False):
+ """Get the value of a field in LDAP attributes.
+
+ Looks at the site preference for sync fields to determine which LDAP field is
+ associated with the model field, then gets this attribute and pythonises it.
+
+ Raises KeyError if the desired field is not in the LDAP entry.
+ Raises AttributeError if the requested field is not configured to be synced.
+ """
+ setting_name = "ldap__" + setting_name_from_field(model, field)
+
+ # Try sync if preference for this field is non-empty
+ ldap_field = get_site_preferences()[setting_name].lower()
+
+ if not ldap_field:
+ raise AttributeError(f"Field {field.name} not configured to be synced.")
+
+ if ldap_field in attrs:
+ value = attrs[ldap_field][0]
+
+ # Apply regex replace from config
+ patterns = get_site_preferences()[setting_name + "_re"]
+ templates = get_site_preferences()[setting_name + "_replace"]
+ value = apply_templates(value, patterns, templates)
+
+ # Opportunistically convert LDAP string value to Python object
+ value = from_ldap(value, field, dn, ldap_field, instance)
+
+ return value
+ else:
+ if allow_missing:
+ logger.warn(f"Field {ldap_field} not in attributes of {dn}")
+ else:
+ raise KeyError(f"Field {ldap_field} not in attributes of {dn}")
+
@transaction.atomic
def ldap_sync_user_on_login(sender, instance, created, **kwargs):
@@ -190,14 +234,25 @@ def ldap_sync_from_user(user, dn, attrs):
# Build filter criteria depending on config
matches = {}
defaults = {}
- if "-email" in get_site_preferences()["ldap__matching_fields"]:
- matches["email"] = user.email
- defaults["first_name"] = user.first_name
- defaults["last_name"] = user.last_name
- if "-name" in get_site_preferences()["ldap__matching_fields"]:
- matches["first_name"] = user.first_name
- matches["last_name"] = user.last_name
- defaults["email"] = user.email
+
+ # Match on all fields selected in preferences
+ fields_map = {f.name: f for f in Person.syncable_fields()}
+ for field_name in get_site_preferences()["ldap__matching_fields"]:
+ try:
+ value = get_ldap_value_for_field(Person, fields_map[field_name], attrs, dn)
+ except KeyError:
+ # Field is not set in LDAP, match on remaining fields
+ continue
+
+ matches[field_name] = value
+
+ if not matches:
+ raise KeyError(f"No matching fields found for {dn}")
+
+ # Pre-fill all mandatory non-matching fields from User object
+ for missing_key in ("first_name", "last_name", "email"):
+ if missing_key not in matches:
+ defaults[missing_key] = getattr(user, missing_key)
if get_site_preferences()["ldap__create_missing_persons"]:
person, created = Person.objects.get_or_create(**matches, defaults=defaults)
@@ -217,23 +272,14 @@ def ldap_sync_from_user(user, dn, attrs):
# Synchronise additional fields if enabled
for field in Person.syncable_fields():
- setting_name = "ldap__" + setting_name_from_field(Person, field)
-
- # Try sync if constance setting for this field is non-empty
- ldap_field = get_site_preferences()[setting_name].lower()
- if ldap_field and ldap_field in attrs:
- value = attrs[ldap_field][0]
-
- # Apply regex replace from config
- patterns = get_site_preferences()[setting_name + "_re"]
- templates = get_site_preferences()[setting_name + "_replace"]
- value = apply_templates(value, patterns, templates)
-
- # Opportunistically convert LDAP string value to Python object
- value = from_ldap(value, person, field, dn, ldap_field)
+ try:
+ value = get_ldap_value_for_field(Person, field, attrs, dn, person, allow_missing=True)
+ except AttributeError:
+ # A syncable field is not configured to sync
+ continue
- setattr(person, field.name, value)
- logger.debug(f"Field {field.name} set to {value} for {person}")
+ setattr(person, field.name, value)
+ logger.debug(f"Field {field.name} set to {value} for {person}")
person.save()
return person
@@ -349,7 +395,7 @@ def mass_ldap_import():
except Person.MultipleObjectsReturned:
logger.error(f"More than one matching person for user {user.username}")
continue
- except (DataError, IntegrityError, ValueError) as e:
+ except (DataError, IntegrityError, KeyError, ValueError) as e:
logger.error(f"Data error while synchronising user {user.username}:\n{e}")
continue
else: