Update User group when save is called

canaille/conf/config.sample.toml

@@ -87,10 +87,6 @@ GROUP_BASE = "ou=groups,dc=mydomain,dc=tld"
 # The attribute to use to identify a group
 # GROUP_NAME_ATTRIBUTE = "cn"
 
-# A filter to check if a user belongs to a group
-# A 'user' variable is available.
-# GROUP_USER_FILTER = "member={user.dn}"
-
 [ACL]
 # You can define access controls that define what users can do on canaille
 # An access control consists in a FILTER to match users, a list of PERMISSIONS
@@ -108,8 +104,8 @@ GROUP_BASE = "ou=groups,dc=mydomain,dc=tld"
 # Here are some examples
 #     FILTER = {user_name = 'admin'}
 #     FILTER =
-#       - {groups = 'admin'}
+#       - {groups = 'cn=admins,ou=groups,dc=mydomain,dc=tld'}
-#       - {groups = 'moderators'}
+#       - {groups = 'cn=moderators,ou=groups,dc=mydomain,dc=tld'}
 #
 # The 'PERMISSIONS' parameter that is an list of items the users in the access
 # control will be able to manage. 'PERMISSIONS' is optionnal. Values can be:
@@ -149,7 +145,7 @@ WRITE = [
 ]
 
 [ACL.ADMIN]
-FILTER = {groups = "admins"}
+FILTER = {groups = "cn=admins,ou=groups,dc=mydomain,dc=tld"}
 PERMISSIONS = [
     "manage_users",
     "manage_groups",

canaille/core/account.py

@@ -403,17 +403,11 @@ def profile_create(current_app, form):
     user.formatted_name = [f"{user.given_name[0]} {user.family_name[0]}".strip()]
     user.save()
 
-    if "groups" in form:
-        groups = [Group.get(group_id) for group_id in form["groups"].data]
-        for group in groups:
-            group.members = group.members + [user]
-            group.save()
-
     if form["password1"].data:
         user.set_password(form["password1"].data)
+        user.save()
 
     flash(_("User account creation succeed."), "success")
-    user.save()
 
     return user
 

canaille/core/models.py

@@ -32,13 +32,13 @@ class User(LDAPObject):
         "title": "title",
         "organization": "o",
         "last_modified": "modifyTimestamp",
+        "groups": "memberOf",
     }
 
     def __init__(self, *args, **kwargs):
         self.read = set()
         self.write = set()
         self.permissions = set()
-        self._groups = None
         super().__init__(*args, **kwargs)
 
     @classmethod
@@ -59,21 +59,17 @@ class User(LDAPObject):
         user = super().get(**kwargs)
         if user:
             user.load_permissions()
-            user.load_groups()
 
         return user
 
     @classmethod
     def acl_filter_to_ldap_filter(cls, filter_):
         if isinstance(filter_, dict):
-            return (
+            base = "".join(
-                "(&"
+                f"({cls.attribute_table.get(key, key)}={value})"
-                + "".join(
+                for key, value in filter_.items()
-                    f"({cls.attribute_table.get(key, key)}={value})"
-                    for key, value in filter_.items()
-                )
-                + ")"
             )
+            return f"(&{base})" if len(filter_) > 1 else base
 
         if isinstance(filter_, list):
             return (
@@ -84,15 +80,6 @@ class User(LDAPObject):
 
         return filter_
 
-    def load_groups(self):
-        group_filter = (
-            current_app.config["BACKENDS"]["LDAP"]
-            .get("GROUP_USER_FILTER", Group.DEFAULT_USER_FILTER)
-            .format(user=self)
-        )
-        escaped_group_filter = ldap.filter.escape_filter_chars(group_filter)
-        self._groups = Group.query(filter=escaped_group_filter)
-
     @classmethod
     def authenticate(cls, login, password, signin=False):
         user = User.get_from_login(login)
@@ -154,27 +141,32 @@ class User(LDAPObject):
     def reload(self):
         super().reload()
         self.load_permissions()
-        self.load_groups()
 
-    @property
+    def save(self, *args, **kwargs):
-    def groups(self):
+        group_attr = self.attribute_table.get("groups", "groups")
-        if self._groups is None:
+        new_groups = self.changes.get(group_attr)
-            self.load_groups()
+        if not new_groups:
-        return self._groups
+            return super().save(*args, **kwargs)
+
+        old_groups = self.attrs.get(group_attr) or []
+        new_groups = [
+            v if isinstance(v, Group) else Group.get(id=v) for v in new_groups
+        ]
+        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)
 
-    @groups.setter
-    def groups(self, values):
-        before = self._groups or []
-        after = [v if isinstance(v, Group) else Group.get(id=v) for v in values]
-        to_add = set(after) - set(before)
-        to_del = set(before) - set(after)
         for group in to_add:
             group.members = group.members + [self]
             group.save()
+
         for group in to_del:
             group.members = [member for member in group.members if member != self]
             group.save()
-        self._groups = after
+
+        self.attrs[group_attr] = new_groups
 
     def load_permissions(self):
         conn = self.ldap_connection()
@@ -224,7 +216,6 @@ class Group(LDAPObject):
     DEFAULT_OBJECT_CLASS = "groupOfNames"
     DEFAULT_ID_ATTRIBUTE = "cn"
     DEFAULT_NAME_ATTRIBUTE = "cn"
-    DEFAULT_USER_FILTER = "member={user.id}"
 
     attribute_table = {
         "id": "dn",

canaille/ldap_backend/ldapobject.py

@@ -326,8 +326,6 @@ class LDAPObject(metaclass=LDAPObjectMetaclass):
 
         if not filter:
             filter = ""
-        elif not filter.startswith("(") and not filter.endswith(")"):
-            filter = f"({filter})"
 
         ldapfilter = f"(&{class_filter}{arg_filter}{filter})"
         base = base or f"{cls.base},{cls.root_dn}"

demo/conf-docker/canaille.toml

@@ -88,10 +88,6 @@ GROUP_BASE = "ou=groups,dc=mydomain,dc=tld"
 # The attribute to use to identify a group
 # GROUP_NAME_ATTRIBUTE = "cn"
 
-# A filter to check if a user belongs to a group
-# A 'user' variable is available.
-# GROUP_USER_FILTER = "member={user.dn}"
-
 [ACL]
 # You can define access controls that define what users can do on canaille
 # An access control consists in a FILTER to match users, a list of PERMISSIONS
@@ -109,8 +105,8 @@ GROUP_BASE = "ou=groups,dc=mydomain,dc=tld"
 # Here are some examples
 #     FILTER = {user_name = 'admin'}
 #     FILTER =
-#       - {groups = 'admin'}
+#       - {groups = 'cn=admins,ou=groups,dc=mydomain,dc=tld'}
-#       - {groups = 'moderators'}
+#       - {groups = 'cn=moderators,ou=groups,dc=mydomain,dc=tld'}
 #
 # The 'PERMISSIONS' parameter that is an list of items the users in the access
 # control will be able to manage. 'PERMISSIONS' is optionnal. Values can be:
@@ -150,7 +146,7 @@ WRITE = [
 ]
 
 [ACL.ADMIN]
-FILTER = {groups = "admins"}
+FILTER = {groups = "cn=admins,ou=groups,dc=mydomain,dc=tld"}
 PERMISSIONS = [
     "manage_users",
     "manage_groups",

demo/conf/canaille.toml

@@ -88,10 +88,6 @@ GROUP_BASE = "ou=groups,dc=mydomain,dc=tld"
 # The attribute to use to identify a group
 # GROUP_NAME_ATTRIBUTE = "cn"
 
-# A filter to check if a user belongs to a group
-# A 'user' variable is available.
-# GROUP_USER_FILTER = "member={user.dn}"
-
 [ACL]
 # You can define access controls that define what users can do on canaille
 # An access control consists in a FILTER to match users, a list of PERMISSIONS
@@ -109,8 +105,8 @@ GROUP_BASE = "ou=groups,dc=mydomain,dc=tld"
 # Here are some examples
 #     FILTER = {user_name = 'admin'}
 #     FILTER =
-#       - {groups = 'admin'}
+#       - {groups = 'cn=admins,ou=groups,dc=mydomain,dc=tld'}
-#       - {groups = 'moderators'}
+#       - {groups = 'cn=moderators,ou=groups,dc=mydomain,dc=tld'}
 #
 # The 'PERMISSIONS' parameter that is an list of items the users in the access
 # control will be able to manage. 'PERMISSIONS' is optionnal. Values can be:
@@ -150,7 +146,7 @@ WRITE = [
 ]
 
 [ACL.ADMIN]
-FILTER = {groups = "admins"}
+FILTER = {groups = "cn=admins,ou=groups,dc=mydomain,dc=tld"}
 PERMISSIONS = [
     "manage_users",
     "manage_groups",

doc/configuration.rst

@@ -118,10 +118,6 @@ BACKENDS.LDAP
     *Optional.* The attribute to identify a group in the web interface.
     Defaults to ``cn``
 
-:GROUP_USER_FILTER:
-    *Optional.* A filter to check if a user belongs to a group. A 'user' variable is available.
-    Defaults to ``member={user.dn}``
-
 ACL
 ---
 You can define access controls that define what users can do on canaille

tests/conftest.py

@@ -238,6 +238,7 @@ def foo_group(app, user, slapd_connection):
         display_name="foo",
     )
     group.save()
+    user.reload()
     yield group
     group.delete()
 
@@ -249,6 +250,7 @@ def bar_group(app, admin, slapd_connection):
         display_name="bar",
     )
     group.save()
+    admin.reload()
     yield group
     group.delete()
 

tests/core/test_groups.py

@@ -62,6 +62,8 @@ def test_group_deletion(testclient, slapd_server, slapd_connection):
         display_name="foobar",
     )
     group.save()
+
+    user.reload()
     assert user.groups == [group]
 
     group.delete()
@@ -88,15 +90,17 @@ def test_group_list_search(testclient, logged_admin, foo_group, bar_group):
 
 def test_set_groups(app, user, foo_group, bar_group):
     assert user in foo_group.members
-    assert user.groups[0] == foo_group
+    assert user.groups == [foo_group]
 
     user.groups = [foo_group, bar_group]
+    user.save()
 
     bar_group.reload()
     assert user in bar_group.members
     assert user.groups[1] == bar_group
 
     user.groups = [foo_group]
+    user.save()
 
     foo_group.reload()
     bar_group.reload()
@@ -114,10 +118,12 @@ def test_set_groups_with_leading_space_in_user_id_attribute(app, foo_group):
     user.save()
 
     user.groups = [foo_group]
+    user.save()
 
     assert user in foo_group.members
 
     user.groups = []
+    user.save()
 
     foo_group.reload()
     assert user.id not in foo_group.members

tests/core/test_permissions.py

@@ -0,0 +1,7 @@
+def test_group_permissions(testclient, user, foo_group):
+    assert not user.can_manage_users
+
+    testclient.app.config["ACL"]["ADMIN"]["FILTER"] = {"groups": foo_group.id}
+    user.reload()
+
+    assert user.can_manage_users