fix: LDAP user group removal

CHANGES.rst

@@ -1,3 +1,8 @@
+
+Fixed
+^^^^^
+- LDAP user group removal.
+
 [0.0.48] - 2024-04-08
 ---------------------
 

canaille/backends/ldap/ldapobject.py

@@ -444,6 +444,7 @@ class LDAPObject(BackendModel, metaclass=LDAPObjectMetaclass):
                 for name, value in self.changes.items()
                 if (
                     value is None
+                    or value == []
                     or (isinstance(value, list) and len(value) == 1 and not value[0])
                 )
                 and name in self.state
@@ -453,6 +454,7 @@ class LDAPObject(BackendModel, metaclass=LDAPObjectMetaclass):
                 for name, value in self.changes.items()
                 if name not in deletions and self.state.get(name) != value
             }
+            print(deletions, changes)
             formatted_changes = python_attrs_to_ldap(changes, null_allowed=False)
             modlist = [(ldap.MOD_DELETE, name, None) for name in deletions] + [
                 (ldap.MOD_REPLACE, name, values)

canaille/backends/ldap/models.py

@@ -49,16 +49,21 @@ class User(canaille.core.models.User, LDAPObject):
 
     def save(self, *args, **kwargs):
         group_attr = self.python_attribute_to_ldap("groups")
-        new_groups = self.changes.get(group_attr)
-        if not new_groups:
+        if group_attr not in self.changes:
             return super().save(*args, **kwargs)
 
+        # The LDAP attribute memberOf cannot directly be edited,
+        # so this is needed to update the Group.member attribute
+        # instead.
         old_groups = self.state.get(group_attr) or []
-        new_groups = [v if isinstance(v, Group) else Group.get(v) for v in new_groups]
+        new_groups = [
+            value if isinstance(value, Group) else Group.get(value)
+            for value in self.changes[group_attr]
+        ]
         to_add = set(new_groups) - set(old_groups)
         to_del = set(old_groups) - set(new_groups)
-
         del self.changes[group_attr]
+
         super().save(*args, **kwargs)
 
         for group in to_add:
@@ -66,6 +71,11 @@ class User(canaille.core.models.User, LDAPObject):
             group.save()
 
         for group in to_del:
+            # LDAP groups cannot be empty because groupOfNames.member
+            # is a MUST attribute.
+            # https://www.rfc-editor.org/rfc/rfc2256.html#section-7.10
+            # TODO: properly manage the situation where one wants to
+            # remove the last member of a group
             group.members = [member for member in group.members if member != self]
             group.save()
 

canaille/core/endpoints/account.py

@@ -753,7 +753,7 @@ def profile_settings_edit(editor, edited_user):
         if hasattr(edited_user, k) and k in available_fields
     }
 
-    data["groups"] = [g.id for g in edited_user.groups]
+    data["groups"] = [group.id for group in edited_user.groups]
 
     form = build_profile_form(
         editor.writable_fields & available_fields,
@@ -761,7 +761,6 @@ def profile_settings_edit(editor, edited_user):
         edited_user,
     )
     form.process(CombinedMultiDict((request.files, request.form)) or None, data=data)
-
     if (
         request.form
         and request.form.get("action") == "edit-settings"

tests/core/test_profile_settings.py

@@ -37,6 +37,24 @@ def test_edition(testclient, logged_user, admin, foo_group, bar_group, backend):
     logged_user.save()
 
 
+def test_group_removal(testclient, logged_admin, user, foo_group, backend):
+    foo_group.members = [user, logged_admin]
+    foo_group.save()
+    user.reload()
+    assert foo_group in user.groups
+
+    res = testclient.get("/profile/user/settings", status=200)
+    res.form["groups"] = []
+    res = res.form.submit(name="action", value="edit-settings")
+    assert res.flashes == [("success", "Profile updated successfully.")]
+
+    user.reload()
+    assert foo_group not in user.groups
+
+    foo_group.reload()
+    assert foo_group.members == [logged_admin]
+
+
 def test_profile_settings_edition_dynamic_validation(testclient, logged_admin):
     res = testclient.get("/profile/admin/settings")
     res = testclient.post(