cleanup and fix tests

Signed-off-by: Jens Langhammer <jens@goauthentik.io>

authentik/sources/ldap/migrations/0001_squashed_0012_auto_20210812_1703.py

@@ -153,7 +153,7 @@ class Migration(migrations.Migration):
                 (
                     "object_uniqueness_field",
                     models.TextField(
-                        default="objectSid", help_text="Field which contains a unique Identifier."
+                        default="entryDN", help_text="Field which contains a unique Identifier."
                     ),
                 ),
                 ("sync_groups", models.BooleanField(default=True)),

authentik/sources/ldap/models.py

@@ -88,7 +88,7 @@ class LDAPSource(Source):
         help_text=_("Consider Objects matching this filter to be Groups."),
     )
     object_uniqueness_field = models.TextField(
-        default="objectSid", help_text=_("Field which contains a unique Identifier.")
+        default="entryDN", help_text=_("Field which contains a unique Identifier.")
     )
 
     property_mappings_group = models.ManyToManyField(

authentik/sources/ldap/sync/base.py

@@ -47,6 +47,15 @@ class BaseLDAPSynchronizer:
         """UI name for the type of object this class synchronizes"""
         raise NotImplementedError
 
+    def get_unique_identifier(self, ldap_object: dict) -> str | None:
+        """Get unique identifier"""
+        attributes = ldap_object.get("attributes", {})
+        if self._source.object_uniqueness_field in attributes:
+            return flatten(attributes[self._source.object_uniqueness_field])
+        if self._source.object_uniqueness_field in ldap_object:
+            return flatten(ldap_object.get(self._source.object_uniqueness_field))
+        return None
+
     def sync_full(self):
         """Run full sync, this function should only be used in tests"""
         if not settings.TEST:  # noqa
@@ -134,20 +143,22 @@ class BaseLDAPSynchronizer:
                 cookie = None
             yield self._connection.response
 
-    def build_user_properties(self, user_dn: str, **kwargs) -> dict[str, Any]:
+    def build_user_properties(self, user_dn: str, uniq: str, **kwargs) -> dict[str, Any]:
         """Build attributes for User object based on property mappings."""
-        props = self._build_object_properties(user_dn, self._source.property_mappings, **kwargs)
+        props = self._build_object_properties(
+            user_dn, self._source.property_mappings, uniq, **kwargs
+        )
         props.setdefault("path", self._source.get_user_path())
         return props
 
-    def build_group_properties(self, group_dn: str, **kwargs) -> dict[str, Any]:
+    def build_group_properties(self, group_dn: str, uniq: str, **kwargs) -> dict[str, Any]:
         """Build attributes for Group object based on property mappings."""
         return self._build_object_properties(
-            group_dn, self._source.property_mappings_group, **kwargs
+            group_dn, self._source.property_mappings_group, uniq, **kwargs
         )
 
     def _build_object_properties(
-        self, object_dn: str, mappings: QuerySet, **kwargs
+        self, object_dn: str, mappings: QuerySet, uniq: str, **kwargs
     ) -> dict[str, dict[Any, Any]]:
         properties = {"attributes": {}}
         for mapping in mappings.all().select_subclasses():
@@ -180,10 +191,7 @@ class BaseLDAPSynchronizer:
                 ).save()
                 self._logger.warning("Mapping failed to evaluate", exc=exc, mapping=mapping)
                 continue
-        if self._source.object_uniqueness_field in kwargs:
+        properties["attributes"][LDAP_UNIQUENESS] = uniq
-            properties["attributes"][LDAP_UNIQUENESS] = flatten(
-                kwargs.get(self._source.object_uniqueness_field)
-            )
         properties["attributes"][LDAP_DISTINGUISHED_NAME] = object_dn
         return properties
 

authentik/sources/ldap/sync/groups.py

@@ -41,16 +41,16 @@ class GroupLDAPSynchronizer(BaseLDAPSynchronizer):
                 continue
             attributes = group.get("attributes", {})
             group_dn = flatten(flatten(group.get("entryDN", group.get("dn"))))
-            if self._source.object_uniqueness_field not in attributes:
+            uniq = self.get_unique_identifier(group)
+            if not uniq:
                 self.message(
                     f"Cannot find uniqueness field in attributes: '{group_dn}'",
                     attributes=attributes.keys(),
                     dn=group_dn,
                 )
                 continue
-            uniq = flatten(attributes[self._source.object_uniqueness_field])
             try:
-                defaults = self.build_group_properties(group_dn, **attributes)
+                defaults = self.build_group_properties(group_dn, uniq, **attributes)
                 defaults["parent"] = self._source.sync_parent_group
                 if "name" not in defaults:
                     raise IntegrityError("Name was not set by propertymappings")

authentik/sources/ldap/sync/membership.py

@@ -4,7 +4,7 @@ from collections.abc import Generator
 from typing import Any
 
 from django.db.models import Q
-from ldap3 import SUBTREE
+from ldap3 import ALL_ATTRIBUTES, ALL_OPERATIONAL_ATTRIBUTES, SUBTREE
 
 from authentik.core.models import Group, User
 from authentik.sources.ldap.auth import LDAP_DISTINGUISHED_NAME
@@ -33,11 +33,7 @@ class MembershipLDAPSynchronizer(BaseLDAPSynchronizer):
             search_base=self.base_dn_groups,
             search_filter=self._source.group_object_filter,
             search_scope=SUBTREE,
-            attributes=[
+            attributes=[ALL_ATTRIBUTES, ALL_OPERATIONAL_ATTRIBUTES],
-                self._source.group_membership_field,
-                self._source.object_uniqueness_field,
-                LDAP_DISTINGUISHED_NAME,
-            ],
             **kwargs,
         )
 
@@ -80,7 +76,7 @@ class MembershipLDAPSynchronizer(BaseLDAPSynchronizer):
     def get_group(self, group_dict: dict[str, Any]) -> Group | None:
         """Check if we fetched the group already, and if not cache it for later"""
         group_dn = group_dict.get("attributes", {}).get(LDAP_DISTINGUISHED_NAME, [])
-        group_uniq = group_dict.get("attributes", {}).get(self._source.object_uniqueness_field, [])
+        group_uniq = self.get_unique_identifier(group_dict)
         # group_uniq might be a single string or an array with (hopefully) a single string
         if isinstance(group_uniq, list):
             if len(group_uniq) < 1:

authentik/sources/ldap/sync/users.py

@@ -43,16 +43,16 @@ class UserLDAPSynchronizer(BaseLDAPSynchronizer):
                 continue
             attributes = user.get("attributes", {})
             user_dn = flatten(user.get("entryDN", user.get("dn")))
-            if self._source.object_uniqueness_field not in attributes:
+            uniq = self.get_unique_identifier(user)
+            if not uniq:
                 self.message(
                     f"Cannot find uniqueness field in attributes: '{user_dn}'",
                     attributes=attributes.keys(),
                     dn=user_dn,
                 )
                 continue
-            uniq = flatten(attributes[self._source.object_uniqueness_field])
             try:
-                defaults = self.build_user_properties(user_dn, **attributes)
+                defaults = self.build_user_properties(user_dn, uniq, **attributes)
                 self._logger.debug("Writing user with attributes", **defaults)
                 if "username" not in defaults:
                     raise IntegrityError("Username was not set by propertymappings")

authentik/sources/ldap/tests/mock_ad.py

@@ -41,7 +41,7 @@ def mock_ad_connection(password: str) -> Connection:
     connection.strategy.add_entry(
         "cn=group2,ou=groups,dc=goauthentik,dc=io",
         {
-            "name": "test-group",
+            "name": "test-group2",
             "objectClass": "group",
             "distinguishedName": "cn=group2,ou=groups,dc=goauthentik,dc=io",
         },
@@ -61,18 +61,6 @@ def mock_ad_connection(password: str) -> Connection:
             ),
         },
     )
-    # User without SID
-    connection.strategy.add_entry(
-        "cn=user1,ou=users,dc=goauthentik,dc=io",
-        {
-            "userPassword": "test1111",
-            "sAMAccountName": "user2_sn",
-            "name": "user1_sn",
-            "revision": 0,
-            "objectClass": "person",
-            "distinguishedName": "cn=user1,ou=users,dc=goauthentik,dc=io",
-        },
-    )
     # Duplicate users
     connection.strategy.add_entry(
         "cn=user2,ou=users,dc=goauthentik,dc=io",
@@ -87,7 +75,7 @@ def mock_ad_connection(password: str) -> Connection:
         },
     )
     connection.strategy.add_entry(
-        "cn=user3,ou=users,dc=goauthentik,dc=io",
+        "cn=user2,ou=users,dc=goauthentik,dc=io",
         {
             "userPassword": "test2222",
             "sAMAccountName": "user2_sn",
@@ -95,7 +83,7 @@ def mock_ad_connection(password: str) -> Connection:
             "revision": 0,
             "objectSid": "unique-test2222",
             "objectClass": "person",
-            "distinguishedName": "cn=user3,ou=users,dc=goauthentik,dc=io",
+            "distinguishedName": "cn=user2,ou=users,dc=goauthentik,dc=io",
         },
     )
     connection.bind()

authentik/sources/ldap/tests/test_sync.py

@@ -108,12 +108,7 @@ class LDAPSyncTests(TestCase):
         user = User.objects.create(
             username="user0_sn",
             attributes={
-                "ldap_uniq": (
+                "ldap_uniq": "cn=user0,ou=foo,ou=users,dc=goauthentik,dc=io",
-                    "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",
             },
         )

web/src/admin/sources/ldap/LDAPSourceForm.ts

@@ -470,7 +470,7 @@ export class LDAPSourceForm extends BaseSourceForm<LDAPSource> {
                     >
                         <input
                             type="text"
-                            value="${this.instance?.objectUniquenessField || "objectSid"}"
+                            value="${this.instance?.objectUniquenessField || "entryDN"}"
                             class="pf-c-form-control"
                             required
                         />