sorces/ldap: fix user/group sync overwriting attributes instead of merging them

Signed-off-by: Jens Langhammer <jens.langhammer@beryju.org>

authentik/sources/ldap/sync/base.py

@@ -1,6 +1,8 @@
 """Sync LDAP Users and groups into authentik"""
 from typing import Any
 
+from deepmerge import always_merger
+from django.db.models.base import Model
 from django.db.models.query import QuerySet
 from structlog.stdlib import BoundLogger, get_logger
 
@@ -105,3 +107,24 @@ class BaseLDAPSynchronizer:
             )
         properties["attributes"][LDAP_DISTINGUISHED_NAME] = object_dn
         return properties
+
+    def update_or_create_attributes(
+        self,
+        obj: type[Model],
+        query: dict[str, Any],
+        data: dict[str, Any],
+    ) -> tuple[Model, bool]:
+        """Same as django's update_or_create but correctly update attributes by merging dicts"""
+        instance = obj.objects.filter(**query).first()
+        if not instance:
+            return (obj.objects.create(**data), True)
+        for key, value in data.items():
+            if key == "attributes":
+                continue
+            setattr(instance, key, value)
+        final_atttributes = {}
+        always_merger.merge(final_atttributes, instance.attributes)
+        always_merger.merge(final_atttributes, data.get("attributes", {}))
+        instance.attributes = final_atttributes
+        instance.save()
+        return (instance, False)

authentik/sources/ldap/sync/groups.py

@@ -43,12 +43,13 @@ class GroupLDAPSynchronizer(BaseLDAPSynchronizer):
                 # Special check for `users` field, as this is an M2M relation, and cannot be sync'd
                 if "users" in defaults:
                     del defaults["users"]
-                ak_group, created = Group.objects.update_or_create(
+                ak_group, created = self.update_or_create_attributes(
-                    **{
+                    Group,
+                    {
                         f"attributes__{LDAP_UNIQUENESS}": uniq,
                         "parent": self._source.sync_parent_group,
-                        "defaults": defaults,
+                    },
-                    }
+                    defaults,
                 )
             except (IntegrityError, FieldError) as exc:
                 Event.new(

authentik/sources/ldap/sync/users.py

@@ -42,11 +42,8 @@ class UserLDAPSynchronizer(BaseLDAPSynchronizer):
                 self._logger.debug("Creating user with attributes", **defaults)
                 if "username" not in defaults:
                     raise IntegrityError("Username was not set by propertymappings")
-                ak_user, created = User.objects.update_or_create(
+                ak_user, created = self.update_or_create_attributes(
-                    **{
+                    User, {f"attributes__{LDAP_UNIQUENESS}": uniq}, defaults
-                        f"attributes__{LDAP_UNIQUENESS}": uniq,
-                        "defaults": defaults,
-                    }
                 )
             except (IntegrityError, FieldError) as exc:
                 Event.new(

authentik/sources/ldap/tests/test_sync.py

@@ -69,10 +69,26 @@ class LDAPSyncTests(TestCase):
         )
         self.source.save()
         connection = PropertyMock(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(
+            username="user0_sn",
+            attributes={
+                "ldap_uniq": (
+                    "S-117-6648368-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-"
+                    "0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-"
+                    "0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-"
+                    "0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0-0"
+                ),
+                "foo": "bar,",
+            },
+        )
+
         with patch("authentik.sources.ldap.models.LDAPSource.connection", connection):
             user_sync = UserLDAPSynchronizer(self.source)
             user_sync.sync()
             user = User.objects.filter(username="user0_sn").first()
+            self.assertEqual(user.attributes["foo"], "bar")
             self.assertFalse(user.is_active)
             self.assertFalse(User.objects.filter(username="user1_sn").exists())