Refactor LDAP import to separate user and group import

aleksis/apps/ldap/apps.py

@@ -3,7 +3,7 @@ from django.db.models.signals import post_save
 
 from aleksis.core.util.apps import AppConfig
 
-from .util.ldap_sync import ldap_sync_from_user, update_constance_config_fields
+from .util.ldap_sync import ldap_sync_user_on_login, update_constance_config_fields
 
 class LDAPConfig(AppConfig):
     name = "aleksis.apps.ldap"
@@ -15,4 +15,4 @@ class LDAPConfig(AppConfig):
         update_constance_config_fields()
 
         User = get_user_model()
-        post_save.connect(ldap_sync_from_user, sender=User)
+        post_save.connect(ldap_sync_user_on_login, sender=User)

aleksis/apps/ldap/util/ldap_sync.py

@@ -88,7 +88,7 @@ def apply_templates(value, patterns, templates, separator="|"):
 
 
 @transaction.atomic
-def ldap_sync_from_user(sender, instance, created, **kwargs):
+def ldap_sync_user_on_login(sender, instance, created, **kwargs):
     """ Synchronise Person meta-data and groups from ldap_user on User update. """
 
     # Semaphore to guard recursive saves within this signal
@@ -97,113 +97,26 @@ def ldap_sync_from_user(sender, instance, created, **kwargs):
     instance._skip_signal = True
 
     Person = apps.get_model("core", "Person")
-    Group = apps.get_model("core", "Group")
 
     if config.ENABLE_LDAP_SYNC and (created or config.LDAP_SYNC_ON_UPDATE) and hasattr(instance, "ldap_user"):
-        # Check if there is an existing person connected to the user.
-        if Person.objects.filter(user=instance).exists():
-            person = instance.person
-            logger.info("Existing person %s already linked to user %s" % (str(person), instance.username))
-        else:
-            # Build filter criteria depending on config
-            matches = {}
-            if "-email" in config.LDAP_MATCHING_FIELDS:
-                matches["email"] = instance.email
-            if "-name" in config.LDAP_MATCHING_FIELDS:
-                matches["first_name"] = instance.first_name
-                matches["last_name"] = instance.last_name
-
-            try:
-                with transaction.atomic():
-                    person = Person.objects.get(**matches)
-            except Person.DoesNotExist:
-                # Bail out of further processing
-                logger.warn("No matching person for user %s" % instance.username)
-                return
-            except Person.MultipleObjectsReturned:
-                # Bail out of further processing
-                logger.error("More than one matching person for user %s" % instance.username)
-                return
-            else:
-                person.user = instance
-                logger.info("Matching person %s linked to user %s" % (str(person), instance.username))
-
-        # Synchronise additional fields if enabled
-        for field in syncable_fields(Person):
-            setting_name = setting_name_from_field(Person, field)
-
-            # Try sync if constance setting for this field is non-empty
-            ldap_field = getattr(config, setting_name, "").lower()
-            if ldap_field and ldap_field in instance.ldap_user.attrs.data:
-                value = instance.ldap_user.attrs.data[ldap_field][0]
-
-                # Apply regex replace from config
-                patterns = getattr(config, setting_name + "_RE", "")
-                templates = getattr(config, setting_name + "_REPLACE", "")
-                value = apply_templates(value, patterns, templates)
-
-                # Opportunistically convert LDAP string value to Python object
-                value = from_ldap(value, field)
-
-                setattr(person, field.name, value)
-                logger.debug("Field %s set to %s for %s" % (field.name, str(value), str(person)))
+        try:
+            with transaction.atomic():
+                person = ldap_sync_from_user(instance, instance.ldap_user.dn, instance.ldap_user.attrs.data)
+        except Person.DoesNotExist:
+            logger.warn("No matching person for user %s" % user.username)
+            return
+        except Person.MultipleObjectsReturned:
+            logger.error("More than one matching person for user %s" % user.username)
+            return
+        except (DataError, IntegrityError, ValueError) as e:
+            logger.error("Data error while synchronising user %s:\n%s" % (user.username, str(e)))
+            return
 
         if config.ENABLE_LDAP_GROUP_SYNC:
-            # Resolve Group objects from LDAP group objects
-            group_objects = []
+            # Get groups from LDAP
             groups = instance.ldap_user._get_groups()
             group_infos = list(groups._get_group_infos())
-            for ldap_group in group_infos:
-                # Skip group if one of the name fields is missing
-                if config.LDAP_GROUP_SYNC_FIELD_SHORT_NAME not in ldap_group[1]:
-                    logger.error("LDAP group with DN %s does not have field %s" % (
-                        ldap_group[0],
-                        config.LDAP_GROUP_SYNC_FIELD_SHORT_NAME
-                    ))
-                    continue
-                if config.LDAP_GROUP_SYNC_FIELD_NAME not in ldap_group[1]:
-                    logger.error("LDAP group with DN %s does not have field %s" % (
-                        ldap_group[0],
-                        config.LDAP_GROUP_SYNC_FIELD_NAME
-                    ))
-                    continue
-
-                # Apply regex replace from config
-                short_name = apply_templates(
-                    ldap_group[1][config.LDAP_GROUP_SYNC_FIELD_SHORT_NAME][0],
-                    config.LDAP_GROUP_SYNC_FIELD_SHORT_NAME_RE,
-                    config.LDAP_GROUP_SYNC_FIELD_SHORT_NAME_REPLACE
-                )
-                name = apply_templates(
-                    ldap_group[1][config.LDAP_GROUP_SYNC_FIELD_NAME][0],
-                    config.LDAP_GROUP_SYNC_FIELD_NAME_RE,
-                    config.LDAP_GROUP_SYNC_FIELD_NAME_REPLACE
-                )
-
-                # Shorten names to fit into model fields
-                short_name = short_name[:Group._meta.get_field("short_name").max_length]
-                name = name[:Group._meta.get_field("name").max_length]
-
-                try:
-                    with transaction.atomic():
-                        group, created = Group.objects.update_or_create(
-                            import_ref = ldap_group[0],
-                            defaults = {
-                                "short_name": short_name,
-                                "name": name
-                            }
-                        )
-                except IntegrityError:
-                    logger.error("Integrity error while trying to import LDAP group %s" % ldap_group[0])
-                    continue
-                else:
-                    logger.info("%s LDAP group %s for Django group %s" % (
-                        ("Created" if created else "Updated"),
-                        ldap_group[1][config.LDAP_GROUP_SYNC_FIELD_NAME][0],
-                        name
-                    ))
-
-                group_objects.append(group)
+            group_objects = get_ldap_groups(group_infos)
 
             # Replace linked groups of logged-in user completely
             person.member_of.set(group_objects)
@@ -220,30 +133,158 @@ def ldap_sync_from_user(sender, instance, created, **kwargs):
     del instance._skip_signal
 
 
+@transaction.atomic
+def ldap_sync_from_user(user):
+    """ Synchronise person information from a User object (with ldap_user) to Django """
+
+    Person = apps.get_model("core", "Person")
+
+    # Check if there is an existing person connected to the user.
+    if Person.objects.filter(user=user).exists():
+        person = user.person
+        logger.info("Existing person %s already linked to user %s" % (str(person), user.username))
+    else:
+        # Build filter criteria depending on config
+        matches = {}
+        if "-email" in config.LDAP_MATCHING_FIELDS:
+            matches["email"] = user.email
+        if "-name" in config.LDAP_MATCHING_FIELDS:
+            matches["first_name"] = user.first_name
+            matches["last_name"] = user.last_name
+
+        try:
+            with transaction.atomic():
+                person = Person.objects.get(**matches)
+        except Person.DoesNotExist:
+            # Bail out of further processing
+            logger.warn("No matching person for user %s" % user.username)
+            return
+        except Person.MultipleObjectsReturned:
+            # Bail out of further processing
+            logger.error("More than one matching person for user %s" % user.username)
+            return
+        else:
+            person.user = user
+            logger.info("Matching person %s linked to user %s" % (str(person), user.username))
+
+    # Synchronise additional fields if enabled
+    for field in syncable_fields(Person):
+        setting_name = setting_name_from_field(Person, field)
+
+        # Try sync if constance setting for this field is non-empty
+        ldap_field = getattr(config, setting_name, "").lower()
+        if ldap_field and ldap_field in user.ldap_user.attrs.data:
+            value = user.ldap_user.attrs.data[ldap_field][0]
+
+            # Apply regex replace from config
+            patterns = getattr(config, setting_name + "_RE", "")
+            templates = getattr(config, setting_name + "_REPLACE", "")
+            value = apply_templates(value, patterns, templates)
+
+            # Opportunistically convert LDAP string value to Python object
+            value = from_ldap(value, field)
+
+            setattr(person, field.name, value)
+            logger.debug("Field %s set to %s for %s" % (field.name, str(value), str(person)))
+
+    return person
+
+
+@transaction.atomic
+def ldap_sync_from_groups(group_infos):
+    """ Synchronise group information from LDAP results to Django """
+
+    Group = apps.get_model("core", "Group")
+
+    # Resolve Group objects from LDAP group objects
+    group_objects = []
+    for ldap_group in group_infos:
+        # Skip group if one of the name fields is missing
+        if config.LDAP_GROUP_SYNC_FIELD_SHORT_NAME not in ldap_group[1]:
+            logger.error("LDAP group with DN %s does not have field %s" % (
+                ldap_group[0],
+                config.LDAP_GROUP_SYNC_FIELD_SHORT_NAME
+            ))
+            continue
+        if config.LDAP_GROUP_SYNC_FIELD_NAME not in ldap_group[1]:
+            logger.error("LDAP group with DN %s does not have field %s" % (
+                ldap_group[0],
+                config.LDAP_GROUP_SYNC_FIELD_NAME
+            ))
+            continue
+
+        # Apply regex replace from config
+        short_name = apply_templates(
+            ldap_group[1][config.LDAP_GROUP_SYNC_FIELD_SHORT_NAME][0],
+            config.LDAP_GROUP_SYNC_FIELD_SHORT_NAME_RE,
+            config.LDAP_GROUP_SYNC_FIELD_SHORT_NAME_REPLACE
+        )
+        name = apply_templates(
+            ldap_group[1][config.LDAP_GROUP_SYNC_FIELD_NAME][0],
+            config.LDAP_GROUP_SYNC_FIELD_NAME_RE,
+            config.LDAP_GROUP_SYNC_FIELD_NAME_REPLACE
+        )
+
+        # Shorten names to fit into model fields
+        short_name = short_name[:Group._meta.get_field("short_name").max_length]
+        name = name[:Group._meta.get_field("name").max_length]
+
+        try:
+            with transaction.atomic():
+                group, created = Group.objects.update_or_create(
+                    import_ref = ldap_group[0],
+                    defaults = {
+                        "short_name": short_name,
+                        "name": name
+                    }
+                )
+        except IntegrityError:
+            logger.error("Integrity error while trying to import LDAP group %s" % ldap_group[0])
+            continue
+        else:
+            logger.info("%s LDAP group %s for Django group %s" % (
+                ("Created" if created else "Updated"),
+                ldap_group[1][config.LDAP_GROUP_SYNC_FIELD_NAME][0],
+                name
+            ))
+
+        group_objects.append(group)
+
+    return group_objects
+
+
 @transaction.atomic
 def mass_ldap_import():
     """ Utility code for mass import from ldap """
 
     from django_auth_ldap.backend import LDAPBackend, _LDAPUser  # noqa
 
-    # Abuse pre-configured search object to find all users by letting * pass
+    # Abuse pre-configured search object as general LDAP interface
     backend = LDAPBackend()
-    res = backend.settings.USER_SEARCH.execute(_LDAPUser(backend, "").connection, {"user": "*"}, escape=False)
+    connection = _LDAPUser(backend, "").connection
+
+    # Synchronise all groups first
+    if config.ENABLE_LDAP_GROUP_SYNC:
+        ldap_groups = backend.settings.GROUP_SEARCH.execute(connection)
+        group_objects = ldap_sync_from_groups(ldap_groups)
 
     # Guess LDAP username field from user filter
     uid_field = re.search(r"([a-zA-Z]+)=%\(user\)s", backend.settings.USER_SEARCH.filterstr).group(1)
 
-    uids = [entry[1][uid_field][0] for entry in res]
+    # Synchronise user data for all found users
+    ldap_users = backend.settings.USER_SEARCH.execute(connection, {"user": "*"}, escape=False)
+    for dn, attrs in ldap_users:
+        uid = attrs[uid_field][0]
 
-    for uid in uids:
         # Prepare an empty LDAPUser object with the target username
         ldap_user = _LDAPUser(backend, username=uid)
 
         # Find out whether the User object would be created, but do not save
-        _, created = backend.get_or_build_user(uid, backend.ldap_to_django_username(ldap_user))
-        logger.info("Will %s user %s in Django" % ("create" if created else "update", uid))
+        user, created = backend.get_or_build_user(uid, ldap_user)
+        user.ldap_user = ldap_user
 
-        # Run creation and/or population, like the auth backend would
-        ldap_user._get_or_create_user(force_populate=config.LDAP_SYNC_ON_UPDATE)
+        if created or config.LDAP_SYNC_ON_UPDATE:
+            logger.info("Will %s user %s in Django" % ("create" if created else "update", uid))
+            person = ldap_sync_from_user(user)
 
         logger.info("Successfully imported user %s" % uid)