Skip to content

Commit 54a03e9

Browse files
authored
AAP-36738: enhance ldap filter validation (#727)
Use ldap-filter package to do LDAP filter validation rather than the regex we had before.
1 parent 30cf49a commit 54a03e9

File tree

4 files changed

+17
-6
lines changed

4 files changed

+17
-6
lines changed

ansible_base/authentication/authenticator_plugins/ldap.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from django_auth_ldap.backend import LDAPBackend
1111
from django_auth_ldap.backend import LDAPSettings as BaseLDAPSettings
1212
from django_auth_ldap.config import LDAPGroupType, LDAPSearchUnion
13+
from ldap_filter import Filter, ParseError
1314
from rest_framework.serializers import ValidationError
1415

1516
from ansible_base.authentication.authenticator_plugins.base import AbstractAuthenticatorPlugin, Authenticator, BaseAuthenticatorConfiguration
@@ -183,14 +184,15 @@ def validate_ldap_filter(value: Any, with_user: bool = False) -> None:
183184

184185
dn_value = value.replace(user_search_string, 'USER')
185186

186-
if re.match(r'^\([A-Za-z0-9-]+?=[^()]+?\)$', dn_value):
187-
return
188-
elif re.match(r'^\([&|!]\(.*?\)\)$', dn_value):
187+
# Check if this is an and/or filter with multiple subfilters
188+
if re.match(r'^\([&|!]\(.*?\)\)$', dn_value):
189189
for sub_filter in dn_value[3:-2].split(')('):
190190
# We only need to check with_user at the top of the recursion stack
191191
validate_ldap_filter(f'({sub_filter})', with_user=False)
192-
return
193-
raise ValidationError(_('Invalid filter: %s') % value)
192+
try:
193+
Filter.parse(dn_value)
194+
except ParseError:
195+
raise ValidationError(_('Invalid filter: %s') % value)
194196

195197

196198
def get_all_sub_classes(cls):

requirements/requirements_all.txt

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ jsonschema-specifications==2024.10.1
7171
# via jsonschema
7272
jwcrypto==1.5.6
7373
# via django-oauth-toolkit
74+
ldap-filter==1.0.1
75+
# via -r requirements/requirements_authentication.in
7476
lxml==5.3.0
7577
# via
7678
# python3-saml
@@ -145,7 +147,9 @@ six==1.17.0
145147
social-auth-app-django==5.4.1
146148
# via -r requirements/requirements_authentication.in
147149
social-auth-core==4.5.4
148-
# via social-auth-app-django
150+
# via
151+
# -r requirements/requirements_authentication.in
152+
# social-auth-app-django
149153
sqlparse==0.5.2
150154
# via
151155
# -r requirements/requirements.in

requirements/requirements_authentication.in

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ tabulate
77
# LDAP Authenticator Plugins
88
django-auth-ldap
99
python-ldap
10+
ldap-filter
1011

1112
# Social Authenticator Plugins
1213
python3-saml

test_app/tests/authentication/authenticator_plugins/test_ldap.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,10 @@ def test_ldap_validate_ldap_filter(ldap_configuration, ldap_settings):
479479
validate_ldap_filter(invalid_filter, True)
480480
assert e.value.args[0] == 'Invalid filter: (invalid)'
481481

482+
# From AAP-36738
483+
customer_filter = "(&(sAMAccountName=%(user)s)(memberOf:1.3.850.114256.1.4.1241:=CN=ravioli,CN=Users,DC=username2024,DC=local))"
484+
validate_ldap_filter(customer_filter, True)
485+
482486

483487
@pytest.mark.django_db
484488
@mock.patch("ansible_base.authentication.authenticator_plugins.ldap.logger")

0 commit comments

Comments
 (0)