From e3b5a6040b006e779747760e34cf30eed2c0c244 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dipl=2E=20Ing=2E=20P=C3=A9ter=20Varkoly?=
 <pvarkoly@cephalix.eu>
Date: Fri, 30 May 2025 17:04:52 +0200
Subject: [PATCH 1/2] Implementing the evalutaion of indirect group membership.
 Now member or uniqueMember can also be used for the group membership.

---
 config                |  7 +++--
 radicale/auth/ldap.py | 73 ++++++++++++++++++++++++++++---------------
 radicale/config.py    | 18 ++++++-----
 3 files changed, 64 insertions(+), 34 deletions(-)

diff --git a/config b/config
index 8f536958..9b570dcf 100644
--- a/config
+++ b/config
@@ -92,8 +92,11 @@
 # Path of the file containing password of the reader DN
 #ldap_secret_file = /run/secrets/ldap_password
 
-# the attribute to read the group memberships from in the user's LDAP entry (default: not set)
-#ldap_groups_attribute = memberOf
+# The attribute to read the group memberships. This can be memberOf from the user's LDAP entry. member or uniqueMember can also be used. In this case an additional ldap search will be executed to find the groups where the user is member of.
+#ldap_groups_attribute =
+
+# The base dn to find the groups. Will be used only if ldap_groups_attribute is member or uniqueMember. If not given ldap_base will be used.
+#ldap_groups_base =
 
 # The filter to find the DN of the user. This filter must contain a python-style placeholder for the login
 #ldap_filter = (&(objectClass=person)(uid={0}))
diff --git a/radicale/auth/ldap.py b/radicale/auth/ldap.py
index 3ca4ddb3..9e027cc8 100644
--- a/radicale/auth/ldap.py
+++ b/radicale/auth/ldap.py
@@ -18,13 +18,14 @@
 Authentication backend that checks credentials with a LDAP server.
 Following parameters are needed in the configuration:
    ldap_uri            The LDAP URL to the server like ldap://localhost
-   ldap_base           The baseDN of the LDAP server
+   ldap_base           The baseDN of the LDAP server searching for users.
    ldap_reader_dn      The DN of a LDAP user with read access to get the user accounts
    ldap_secret         The password of the ldap_reader_dn
    ldap_secret_file    The path of the file containing the password of the ldap_reader_dn
    ldap_filter         The search filter to find the user to authenticate by the username
    ldap_user_attribute The attribute to be used as username after authentication
    ldap_groups_attribute The attribute containing group memberships in the LDAP user entry
+   ldap_groups_base    The baseDN of the LDAP server searching for groups.
 Following parameters controls SSL connections:
    ldap_use_ssl        If ssl encryption should be used (to be deprecated)
    ldap_security    The encryption mode to be used: *none*|tls|starttls
@@ -78,6 +79,9 @@ class Auth(auth.BaseAuth):
         self._ldap_filter = configuration.get("auth", "ldap_filter")
         self._ldap_user_attr = configuration.get("auth", "ldap_user_attribute")
         self._ldap_groups_attr = configuration.get("auth", "ldap_groups_attribute")
+        self._ldap_groups_base = configuration.get("auth", "ldap_groups_base")
+        if self._ldap_groups_base == "":
+            self._ldap_groups_base = self._ldap_base
         ldap_secret_file_path = configuration.get("auth", "ldap_secret_file")
         if ldap_secret_file_path:
             with open(ldap_secret_file_path, 'r') as file:
@@ -130,8 +134,8 @@ class Auth(auth.BaseAuth):
             else:
                 logger.info("auth.ldap_ssl_ca_file     : (not provided)")
         """Extend attributes to to be returned in the user query"""
-        if self._ldap_groups_attr:
-            self._ldap_attributes.append(self._ldap_groups_attr)
+        if self._ldap_groups_attr == "memberOf":
+            self._ldap_attributes.append("memberOf")
         if self._ldap_user_attr:
             self._ldap_attributes.append(self._ldap_user_attr)
         logger.info("ldap_attributes           : %r" % self._ldap_attributes)
@@ -172,17 +176,25 @@ class Auth(auth.BaseAuth):
             conn.set_option(self.ldap.OPT_REFERRALS, 0)
             conn.simple_bind_s(user_dn, password)
             tmp: list[str] = []
-            if self._ldap_groups_attr:
-                tmp = []
-                for g in user_entry[1][self._ldap_groups_attr]:
-                    """Get group g's RDN's attribute value"""
-                    try:
-                        rdns = self.ldap.dn.explode_dn(g, notypes=True)
-                        tmp.append(rdns[0])
-                    except Exception:
-                        tmp.append(g.decode('utf8'))
-                self._ldap_groups = set(tmp)
-                logger.debug("_login2 LDAP groups of user: %s", ",".join(self._ldap_groups))
+            gdns: list[str] = []
+            if self._ldap_groups_attr == "memberOf":
+                gdns = user_entry[1][self._ldap_groups_attr]
+            elif self._ldap_groups_attr == "member" or self._ldap_groups_attr == "uniqueMember":
+                res = conn.search_s(
+                    self._ldap_groups_base,
+                    self.ldap.SCOPE_SUBTREE,
+                    filterstr="({0}={1})".format(self._ldap_groups_attr,user_dn),
+                    attrlist=self._ldap_attributes
+                )
+            for g in gdns:
+                """Get group g's RDN's attribute value"""
+                try:
+                    rdns = self.ldap.dn.explode_dn(g, notypes=True)
+                    tmp.append(rdns[0])
+                except Exception:
+                    tmp.append(g.decode('utf8'))
+            self._ldap_groups = set(tmp)
+            logger.debug("_login2 LDAP groups of user: %s", ",".join(self._ldap_groups))
             if self._ldap_user_attr:
                 if user_entry[1][self._ldap_user_attr]:
                     tmplogin = user_entry[1][self._ldap_user_attr][0]
@@ -226,7 +238,7 @@ class Auth(auth.BaseAuth):
         except Exception as e:
             logger.debug(f"_login3 error 1 {e} (reader)")
             pass
-        if not conn.bind():
+        if not conn.bind(read_server_info=False):
             logger.debug("_login3 cannot bind (reader)")
             raise RuntimeError("Unable to read from LDAP server")
         logger.debug(f"_login3 bind as {self._ldap_reader_dn}")
@@ -257,19 +269,30 @@ class Auth(auth.BaseAuth):
                     conn.start_tls()
             except self.ldap3.core.exceptions.LDAPStartTLSError as e:
                 raise RuntimeError(f"_login3 StartTLS Error: {e}")
-            if not conn.bind():
+            if not conn.bind(read_server_info=False):
                 logger.debug(f"_login3 user '{login}' cannot be found")
                 return ""
             tmp: list[str] = []
-            if self._ldap_groups_attr:
-                tmp = []
-                for g in user_entry['attributes'][self._ldap_groups_attr]:
-                    """Get group g's RDN's attribute value"""
-                    try:
-                        rdns = self.ldap3.utils.dn.parse_dn(g)
-                        tmp.append(rdns[0][1])
-                    except Exception:
-                        tmp.append(g)
+            gdns: list[str] = []
+            """Let's collect the groups of the user."""
+            if self._ldap_groups_attr == "memberOf":
+                gdns = user_entry['attributes']['memberOf']
+            elif self._ldap_groups_attr == "member" or self._ldap_groups_attr == "uniqueMember":
+                conn.search(
+                    search_base=self._ldap_groups_base,
+                    search_filter="({0}={1})".format(self._ldap_groups_attr,user_dn),
+                    search_scope=self.ldap3.SUBTREE,
+                    attributes="dn"
+                )
+                for group in conn.response:
+                    gdns.append(group['dn'])
+            for g in gdns:
+                """Get group g's RDN's attribute value"""
+                try:
+                    rdns = self.ldap3.utils.dn.parse_dn(g)
+                    tmp.append(rdns[0][1])
+                except Exception:
+                    tmp.append(g)
                 self._ldap_groups = set(tmp)
                 logger.debug("_login3 LDAP groups of user: %s", ",".join(self._ldap_groups))
             if self._ldap_user_attr:
diff --git a/radicale/config.py b/radicale/config.py
index c4f5fbe7..0059e883 100644
--- a/radicale/config.py
+++ b/radicale/config.py
@@ -269,31 +269,35 @@ DEFAULT_CONFIG_SCHEMA: types.CONFIG_SCHEMA = OrderedDict([
             "type": str}),
         ("ldap_base", {
             "value": "",
-            "help": "LDAP base DN of the ldap server",
+            "help": "The base DN of the ldap server where the user can be find.",
             "type": str}),
         ("ldap_reader_dn", {
             "value": "",
-            "help": "the DN of a ldap user with read access to get the user accounts",
+            "help": "The DN of a ldap user with read access to get the user accounts",
             "type": str}),
         ("ldap_secret", {
             "value": "",
-            "help": "the password of the ldap_reader_dn",
+            "help": "The password of the ldap_reader_dn",
             "type": str}),
         ("ldap_secret_file", {
             "value": "",
-            "help": "path of the file containing the password of the ldap_reader_dn",
+            "help": "Path of the file containing the password of the ldap_reader_dn",
             "type": str}),
         ("ldap_filter", {
             "value": "(cn={0})",
-            "help": "the search filter to find the user DN to authenticate by the username",
+            "help": "The search filter to find the user DN to authenticate by the username",
             "type": str}),
         ("ldap_user_attribute", {
             "value": "",
-            "help": "the attribute to be used as username after authentication",
+            "help": "The attribute to be used as username after authentication",
             "type": str}),
         ("ldap_groups_attribute", {
             "value": "",
-            "help": "attribute to read the group memberships from",
+            "help": "Attribute to read the group memberships from. Valid values are memberOf, member or uniqueMember. If no value is given group memebership will be ignored.",
+            "type": str}),
+        ("ldap_groups_base_dn", {
+            "value": "",
+            "help": "The base dn to find the groups. Necessary only if ldap_groups attribute is member or uniqueMember. If not given ldap_base will be used.",
             "type": str}),
         ("ldap_use_ssl", {
             "value": "False",

From 5b20813bd70d753a3fb8d3f0b685da55ffc414f4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dipl=2E=20Ing=2E=20P=C3=A9ter=20Varkoly?=
 <pvarkoly@cephalix.eu>
Date: Fri, 30 May 2025 18:38:45 +0200
Subject: [PATCH 2/2] Adapt code to the review. Adapt includes to satisfy
 isort.

---
 config                             |  7 +++++--
 radicale/app/proppatch.py          |  1 -
 radicale/auth/htpasswd.py          |  1 -
 radicale/auth/ldap.py              | 23 ++++++++++++-----------
 radicale/config.py                 |  8 ++++++--
 radicale/hook/rabbitmq/__init__.py |  1 -
 radicale/tests/__init__.py         |  2 +-
 radicale/tests/test_auth.py        |  1 -
 radicale/tests/test_base.py        |  2 +-
 radicale/tests/test_config.py      |  1 -
 radicale/tests/test_server.py      |  1 -
 radicale/tests/test_storage.py     |  1 -
 12 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/config b/config
index 9b570dcf..27f2bda3 100644
--- a/config
+++ b/config
@@ -92,10 +92,13 @@
 # Path of the file containing password of the reader DN
 #ldap_secret_file = /run/secrets/ldap_password
 
-# The attribute to read the group memberships. This can be memberOf from the user's LDAP entry. member or uniqueMember can also be used. In this case an additional ldap search will be executed to find the groups where the user is member of.
+# The attribute in user entry to read the group memberships.
 #ldap_groups_attribute =
 
-# The base dn to find the groups. Will be used only if ldap_groups_attribute is member or uniqueMember. If not given ldap_base will be used.
+# The attribute in group entries to read the group memberships.
+#ldap_group_members_attribute =
+
+# The base dn to find the groups. Necessary only if ldap_group_members_attribute is defined and other then ldap_base.
 #ldap_groups_base =
 
 # The filter to find the DN of the user. This filter must contain a python-style placeholder for the login
diff --git a/radicale/app/proppatch.py b/radicale/app/proppatch.py
index 76b4a1a1..c894850c 100644
--- a/radicale/app/proppatch.py
+++ b/radicale/app/proppatch.py
@@ -27,7 +27,6 @@ from http import client
 from typing import Dict, Optional, cast
 
 import defusedxml.ElementTree as DefusedET
-
 import radicale.item as radicale_item
 from radicale import httputils, storage, types, xmlutils
 from radicale.app.base import Access, ApplicationBase
diff --git a/radicale/auth/htpasswd.py b/radicale/auth/htpasswd.py
index dd66dfbd..0615dcbb 100644
--- a/radicale/auth/htpasswd.py
+++ b/radicale/auth/htpasswd.py
@@ -60,7 +60,6 @@ import time
 from typing import Any, Tuple
 
 from passlib.hash import apr_md5_crypt, sha256_crypt, sha512_crypt
-
 from radicale import auth, config, logger
 
 
diff --git a/radicale/auth/ldap.py b/radicale/auth/ldap.py
index 9e027cc8..2e6635b6 100644
--- a/radicale/auth/ldap.py
+++ b/radicale/auth/ldap.py
@@ -79,6 +79,7 @@ class Auth(auth.BaseAuth):
         self._ldap_filter = configuration.get("auth", "ldap_filter")
         self._ldap_user_attr = configuration.get("auth", "ldap_user_attribute")
         self._ldap_groups_attr = configuration.get("auth", "ldap_groups_attribute")
+        self._ldap_group_members_attr = configuration.get("auth", "ldap_group_members_attribute")
         self._ldap_groups_base = configuration.get("auth", "ldap_groups_base")
         if self._ldap_groups_base == "":
             self._ldap_groups_base = self._ldap_base
@@ -134,8 +135,8 @@ class Auth(auth.BaseAuth):
             else:
                 logger.info("auth.ldap_ssl_ca_file     : (not provided)")
         """Extend attributes to to be returned in the user query"""
-        if self._ldap_groups_attr == "memberOf":
-            self._ldap_attributes.append("memberOf")
+        if self._ldap_groups_attr:
+            self._ldap_attributes.append(self._ldap_groups_attr)
         if self._ldap_user_attr:
             self._ldap_attributes.append(self._ldap_user_attr)
         logger.info("ldap_attributes           : %r" % self._ldap_attributes)
@@ -177,13 +178,13 @@ class Auth(auth.BaseAuth):
             conn.simple_bind_s(user_dn, password)
             tmp: list[str] = []
             gdns: list[str] = []
-            if self._ldap_groups_attr == "memberOf":
+            if self._ldap_groups_attr:
                 gdns = user_entry[1][self._ldap_groups_attr]
-            elif self._ldap_groups_attr == "member" or self._ldap_groups_attr == "uniqueMember":
+            elif self._ldap_group_members_attr:
                 res = conn.search_s(
                     self._ldap_groups_base,
                     self.ldap.SCOPE_SUBTREE,
-                    filterstr="({0}={1})".format(self._ldap_groups_attr,user_dn),
+                    filterstr="({0}={1})".format(self._ldap_group_members_attr,user_dn),
                     attrlist=self._ldap_attributes
                 )
             for g in gdns:
@@ -275,12 +276,12 @@ class Auth(auth.BaseAuth):
             tmp: list[str] = []
             gdns: list[str] = []
             """Let's collect the groups of the user."""
-            if self._ldap_groups_attr == "memberOf":
-                gdns = user_entry['attributes']['memberOf']
-            elif self._ldap_groups_attr == "member" or self._ldap_groups_attr == "uniqueMember":
+            if self._ldap_groups_attr:
+                gdns = user_entry['attributes'][self._ldap_groups_attr]
+            elif self._ldap_group_members_attr:
                 conn.search(
                     search_base=self._ldap_groups_base,
-                    search_filter="({0}={1})".format(self._ldap_groups_attr,user_dn),
+                    search_filter="({0}={1})".format(self._ldap_group_members_attr,user_dn),
                     search_scope=self.ldap3.SUBTREE,
                     attributes="dn"
                 )
@@ -293,8 +294,8 @@ class Auth(auth.BaseAuth):
                     tmp.append(rdns[0][1])
                 except Exception:
                     tmp.append(g)
-                self._ldap_groups = set(tmp)
-                logger.debug("_login3 LDAP groups of user: %s", ",".join(self._ldap_groups))
+            self._ldap_groups = set(tmp)
+            logger.debug("_login3 LDAP groups of user: %s", ",".join(self._ldap_groups))
             if self._ldap_user_attr:
                 if user_entry['attributes'][self._ldap_user_attr]:
                     if isinstance(user_entry['attributes'][self._ldap_user_attr], list):
diff --git a/radicale/config.py b/radicale/config.py
index 0059e883..90b3a9d5 100644
--- a/radicale/config.py
+++ b/radicale/config.py
@@ -293,11 +293,15 @@ DEFAULT_CONFIG_SCHEMA: types.CONFIG_SCHEMA = OrderedDict([
             "type": str}),
         ("ldap_groups_attribute", {
             "value": "",
-            "help": "Attribute to read the group memberships from. Valid values are memberOf, member or uniqueMember. If no value is given group memebership will be ignored.",
+            "help": "Attribute in the user entry to read the group memberships from.",
+            "type": str}),
+        ("ldap_group_members_attribute", {
+            "value": "",
+            "help": "Attribute in the group entries to read the group memberships from.",
             "type": str}),
         ("ldap_groups_base_dn", {
             "value": "",
-            "help": "The base dn to find the groups. Necessary only if ldap_groups attribute is member or uniqueMember. If not given ldap_base will be used.",
+            "help": "The base dn to find the groups. Necessary only if ldap_group_members_attribute is defined and other then ldap_base.",
             "type": str}),
         ("ldap_use_ssl", {
             "value": "False",
diff --git a/radicale/hook/rabbitmq/__init__.py b/radicale/hook/rabbitmq/__init__.py
index 2323ed43..5b1ec659 100644
--- a/radicale/hook/rabbitmq/__init__.py
+++ b/radicale/hook/rabbitmq/__init__.py
@@ -1,6 +1,5 @@
 import pika
 from pika.exceptions import ChannelWrongStateError, StreamLostError
-
 from radicale import hook
 from radicale.hook import HookNotificationItem
 from radicale.log import logger
diff --git a/radicale/tests/__init__.py b/radicale/tests/__init__.py
index e5ecb1f9..23df6d54 100644
--- a/radicale/tests/__init__.py
+++ b/radicale/tests/__init__.py
@@ -31,9 +31,9 @@ from io import BytesIO
 from typing import Any, Dict, List, Optional, Tuple, Union
 from urllib.parse import quote
 
-import defusedxml.ElementTree as DefusedET
 import vobject
 
+import defusedxml.ElementTree as DefusedET
 import radicale
 from radicale import app, config, types, xmlutils
 
diff --git a/radicale/tests/test_auth.py b/radicale/tests/test_auth.py
index 88cd3ea4..9036c500 100644
--- a/radicale/tests/test_auth.py
+++ b/radicale/tests/test_auth.py
@@ -29,7 +29,6 @@ import sys
 from typing import Iterable, Tuple, Union
 
 import pytest
-
 from radicale import xmlutils
 from radicale.tests import BaseTest
 
diff --git a/radicale/tests/test_base.py b/radicale/tests/test_base.py
index d2b26949..8cd3279a 100644
--- a/radicale/tests/test_base.py
+++ b/radicale/tests/test_base.py
@@ -26,9 +26,9 @@ import os
 import posixpath
 from typing import Any, Callable, ClassVar, Iterable, List, Optional, Tuple
 
-import defusedxml.ElementTree as DefusedET
 import vobject
 
+import defusedxml.ElementTree as DefusedET
 from radicale import storage, xmlutils
 from radicale.tests import RESPONSES, BaseTest
 from radicale.tests.helpers import get_file_content
diff --git a/radicale/tests/test_config.py b/radicale/tests/test_config.py
index 92ece9a6..7b46a601 100644
--- a/radicale/tests/test_config.py
+++ b/radicale/tests/test_config.py
@@ -21,7 +21,6 @@ from configparser import RawConfigParser
 from typing import List, Tuple
 
 import pytest
-
 from radicale import config, types
 from radicale.tests.helpers import configuration_to_dict
 
diff --git a/radicale/tests/test_server.py b/radicale/tests/test_server.py
index b344dddf..d1887b63 100644
--- a/radicale/tests/test_server.py
+++ b/radicale/tests/test_server.py
@@ -34,7 +34,6 @@ from urllib import request
 from urllib.error import HTTPError, URLError
 
 import pytest
-
 from radicale import config, server
 from radicale.tests import BaseTest
 from radicale.tests.helpers import configuration_to_dict, get_file_path
diff --git a/radicale/tests/test_storage.py b/radicale/tests/test_storage.py
index 2fcfe717..0e3ab8c6 100644
--- a/radicale/tests/test_storage.py
+++ b/radicale/tests/test_storage.py
@@ -26,7 +26,6 @@ import shutil
 from typing import ClassVar, cast
 
 import pytest
-
 import radicale.tests.custom.storage_simple_sync
 from radicale.tests import BaseTest
 from radicale.tests.helpers import get_file_content