#337 fixed LDAP authentication when user contains parentheses

Author: bugy

samples/ldap/bootstrap.ldif

@@ -0,0 +1,37 @@
+dn: ou=People,dc=script-server,dc=net
+objectClass: organizationalUnit
+ou: People
+
+dn: uid=user1,ou=People,dc=script-server,dc=net
+objectClass: inetOrgPerson
+objectClass: posixAccount
+cn: John Smith
+sn: Smith
+uid: user1
+uidNumber: 1000
+gidNumber: 1000
+homeDirectory: /home/user1
+userPassword: qwerty
+
+dn: uid=user with space,ou=People,dc=script-server,dc=net
+objectClass: inetOrgPerson
+cn: user with space
+sn: Uws
+uid: user with space
+userPassword: 123 456
+
+dn: uid=user (with brackets),ou=People,dc=script-server,dc=net
+objectClass: inetOrgPerson
+cn: user with brackets
+sn: UwB
+uid: user (with brackets)
+userPassword: 666
+
+dn: cn=all_users,dc=script-server,dc=net
+objectClass: posixGroup
+cn: all_users
+description: All users group
+gidNumber: 10000
+memberUid: user1
+memberUid: user with space
+memberUid: user (with brackets)

samples/ldap/start-ldap-docker.sh

@@ -0,0 +1,17 @@
+#!/bin/bash
+
+docker stop script-server-ldap
+docker rm script-server-ldap
+
+set -e
+
+docker run \
+--name script-server-ldap \
+--env LDAP_ORGANISATION="Script server" \
+--env LDAP_DOMAIN="script-server.net" \
+--env LDAP_ADMIN_PASSWORD="admin_passw" \
+--volume "$PWD"/bootstrap.ldif:/container/service/slapd/assets/config/bootstrap/ldif/50-bootstrap.ldif \
+--detach \
+osixia/openldap:1.4.0 \
+--copy-service \
+--loglevel debug

src/auth/auth_ldap.py

@@ -5,6 +5,7 @@
 
 from ldap3 import Connection, SIMPLE
 from ldap3.core.exceptions import LDAPAttributeError
+from ldap3.utils.conv import escape_filter_chars
 
 from auth import auth_base
 from model import model_helper
@@ -38,19 +39,21 @@ def _resolve_base_dn(full_username):
         return ''
 
 
-def _search(dn, search_filter, attributes, connection):
-    success = connection.search(dn, search_filter, attributes=attributes)
+def _search(dn, search_request, attributes, connection):
+    search_string = search_request.as_search_string()
+
+    success = connection.search(dn, search_string, attributes=attributes)
     if not success:
         if connection.last_error:
             LOGGER.warning('ldap search failed: ' + connection.last_error
-                           + '. dn:' + dn + ', filter: ' + search_filter)
+                           + '. dn:' + dn + ', filter: ' + search_string)
         return None
 
     return connection.entries
 
 
-def _load_multiple_entries_values(dn, search_filter, attribute_name, connection):
-    entries = _search(dn, search_filter, [attribute_name], connection)
+def _load_multiple_entries_values(dn, search_request, attribute_name, connection):
+    entries = _search(dn, search_request, [attribute_name], connection)
     if entries is None:
         return []
 
@@ -174,12 +177,13 @@ def _fetch_user_groups(self, user_dn, user_uid, connection):
 
         result = set()
 
-        result.update(_load_multiple_entries_values(base_dn, '(member=%s)' % user_dn, 'cn', connection))
+        result.update(
+            _load_multiple_entries_values(base_dn, SearchRequest('(member=%s)', user_dn), 'cn', connection))
 
         if user_uid:
             result.update(_load_multiple_entries_values(
                 base_dn,
-                '(&(objectClass=posixGroup)(memberUid=%s))' % user_uid,
+                SearchRequest('(&(objectClass=posixGroup)(memberUid=%s))', user_uid),
                 'cn',
                 connection))
 
@@ -191,23 +195,23 @@ def _get_user_ids(self, full_username, connection):
         username_lower = full_username.lower()
         if ',dc=' in username_lower:
             base_dn = username_lower
-            search_filter = '(objectClass=*)'
+            search_request = SearchRequest('(objectClass=*)')
         elif '@' in full_username:
-            search_filter = '(userPrincipalName=%s)' % full_username
+            search_request = SearchRequest('(userPrincipalName=%s)', full_username)
         elif '\\' in full_username:
             username_index = full_username.rfind('\\') + 1
             username = full_username[username_index:]
-            search_filter = '(sAMAccountName=%s)' % username
+            search_request = SearchRequest('(sAMAccountName=%s)', username)
         else:
             LOGGER.warning('Unsupported username pattern for ' + full_username)
             return full_username, None
 
-        entries = _search(base_dn, search_filter, ['uid'], connection)
+        entries = _search(base_dn, search_request, ['uid'], connection)
         if not entries:
             return full_username, None
 
         if len(entries) > 1:
-            LOGGER.warning('More than one user found by filter: ' + search_filter)
+            LOGGER.warning('More than one user found by filter: ' + str(search_request))
             return full_username, None
 
         entry = entries[0]
@@ -225,3 +229,15 @@ def _set_user_groups(self, user, groups):
 
         new_groups_content = json.dumps(self._user_groups, indent=2)
         file_utils.write_file(self._groups_file, new_groups_content)
+
+
+class SearchRequest:
+    def __init__(self, template, *variables) -> None:
+        escaped_vars = [escape_filter_chars(var) for var in variables]
+        self.search_string = template % tuple(escaped_vars)
+
+    def as_search_string(self):
+        return self.search_string
+
+    def __str__(self) -> str:
+        return self.as_search_string()

src/tests/auth_ldap_test.py

@@ -143,6 +143,24 @@ def test_load_multiple_groups_by_uid_when_dn_template(self):
         groups = self.auth_and_get_groups('user1', auth_wrapper)
         self.assertCountEqual(['group1', 'group2', 'group3'], groups)
 
+    def test_load_multiple_groups_by_uid_when_not_all_match(self):
+        auth_wrapper = _LdapAuthenticatorMockWrapper('cn=$username,cn=Users,dc=ldap,dc=test', 'dc=ldap,dc=test')
+        auth_wrapper.add_posix_user('user1', '1234', 'uid_X')
+        auth_wrapper.add_posix_group('group1', ['uid_X'])
+        auth_wrapper.add_posix_group('group2', ['uid_123'])
+        auth_wrapper.add_posix_group('group3', ['uid_X'])
+
+        groups = self.auth_and_get_groups('user1', auth_wrapper)
+        self.assertCountEqual(['group1', 'group3'], groups)
+
+    def test_load_single_group_by_uid_when_dn_has_parenthesss(self):
+        auth_wrapper = _LdapAuthenticatorMockWrapper('cn=$username,cn=Users,dc=ldap,dc=test', 'dc=ldap,dc=test')
+        auth_wrapper.add_posix_user('user (1)', '1234', 'uid (X)')
+        auth_wrapper.add_posix_group('group1', ['uid (X)'])
+
+        groups = self.auth_and_get_groups('user (1)', auth_wrapper)
+        self.assertCountEqual(['group1'], groups)
+
     def test_load_multiple_groups_by_member_and_uid_when_dn_template(self):
         auth_wrapper = _LdapAuthenticatorMockWrapper('cn=$username,cn=Users,dc=ldap,dc=test', 'dc=ldap,dc=test')
         auth_wrapper.add_posix_user('user1', '1234', 'uid_X')
@@ -169,6 +187,14 @@ def test_load_single_group_by_member_when_sam_account_template(self):
         groups = self.auth_and_get_groups('user1', auth_wrapper)
         self.assertEqual(['group1'], groups)
 
+    def test_load_single_group_by_member_when_sam_account_template_with_parentheses(self):
+        auth_wrapper = _LdapAuthenticatorMockWrapper('some_domain\\$username', 'dc=some_domain,dc=test')
+        auth_wrapper.add_user('User (Noname)', '1234', sAMAccountName='user (1)')
+        auth_wrapper.add_group('group1', ['User (Noname)'])
+
+        groups = self.auth_and_get_groups('user (1)', auth_wrapper)
+        self.assertEqual(['group1'], groups)
+
     def test_load_single_group_by_uid_when_sam_account_template(self):
         auth_wrapper = _LdapAuthenticatorMockWrapper('some_domain\\$username', 'dc=some_domain,dc=test')
         auth_wrapper.add_posix_user('User Noname', '1234', 'uid_X', sAMAccountName='user1')
@@ -185,6 +211,14 @@ def test_load_single_group_by_member_when_user_principal_template(self):
         groups = self.auth_and_get_groups('user1', auth_wrapper)
         self.assertEqual(['group1'], groups)
 
+    def test_load_single_group_by_member_when_user_principal_template_with_parentheses(self):
+        auth_wrapper = _LdapAuthenticatorMockWrapper('$username@buggy.net', 'dc=buggy,dc=net')
+        auth_wrapper.add_user('User (Noname)', '1234', userPrincipalName='user (1)@buggy.net')
+        auth_wrapper.add_group('group1', ['User (Noname)'])
+
+        groups = self.auth_and_get_groups('user (1)', auth_wrapper)
+        self.assertEqual(['group1'], groups)
+
     def test_cannot_load_group_when_same_principal_names(self):
         auth_wrapper = _LdapAuthenticatorMockWrapper('$username@buggy.net', 'dc=buggy,dc=net')
         auth_wrapper.add_user('user1', '1234', userPrincipalName='userX@buggy.net')