Skip to content

Commit b6068b3

Browse files
bhavenstBryan Havensteinjohn-westcott-iv
authored
AAP-51591 Remove LDAP composite filter splitter (ansible#803)
## Description Update to LDAP filter validation to remove splitting function and let python-ldap-filter do the entire validation. Some composite filters were failing to validate due to improper splitting. ## Type of Change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Documentation update - [ ] Test update - [ ] Refactoring (no functional changes) - [ ] Development environment change - [ ] Configuration change ## Self-Review Checklist - [x] I have performed a self-review of my code - [x] I have added relevant comments to complex code sections - [ ] I have updated documentation where needed - [ ] I have considered the security impact of these changes - [ ] I have considered performance implications - [x] I have thought about error handling and edge cases - [x] I have tested the changes in my local environment ## Testing Instructions ### Prerequisites ### Steps to Test 1. UT has the two new filters that were failing to be validated, those can be used to test or any valid LDAP filter. 2. 3. ### Expected Results Previously improperly rejected valid filters now treated as valid. Co-authored-by: Bryan Havenstein <[email protected]> Co-authored-by: John Westcott IV <[email protected]>
1 parent 9c7453b commit b6068b3

File tree

2 files changed

+12
-9
lines changed
  • ansible_base/authentication/authenticator_plugins
  • test_app/tests/authentication/authenticator_plugins

2 files changed

+12
-9
lines changed

ansible_base/authentication/authenticator_plugins/ldap.py

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import inspect
22
import logging
3-
import re
43
from collections import OrderedDict
54
from typing import Any
65

@@ -248,11 +247,6 @@ def validate_ldap_filter(value: Any, with_user: bool = False) -> None:
248247

249248
dn_value = value.replace(user_search_string, 'USER')
250249

251-
# Check if this is an and/or filter with multiple subfilters
252-
if re.match(r'^\([&|!]\(.*?\)\)$', dn_value):
253-
for sub_filter in dn_value[3:-2].split(')('):
254-
# We only need to check with_user at the top of the recursion stack
255-
validate_ldap_filter(f'({sub_filter})', with_user=False)
256250
try:
257251
Filter.parse(dn_value)
258252
except ParseError:

test_app/tests/authentication/authenticator_plugins/test_ldap.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -481,11 +481,20 @@ def test_ldap_validate_ldap_filter(ldap_configuration, ldap_settings):
481481
invalid_filter = "(&(cn=%(user)s)(objectClass=posixAccount)(invalid))"
482482
with pytest.raises(ValidationError) as e:
483483
validate_ldap_filter(invalid_filter, True)
484-
assert e.value.args[0] == 'Invalid filter: (invalid)'
484+
assert "(invalid)" in str(e.value.args[0])
485485

486+
487+
@pytest.mark.parametrize(
488+
"filter,with_user",
489+
[
490+
("(&(sAMAccountName=%(user)s)(memberOf:1.3.850.114256.1.4.1241:=CN=ravioli,CN=Users,DC=username2024,DC=local))", True),
491+
("(&(|(objectclass=userproxy)(objectclass=user))(|(employeeID=%(user)s)(hsbc-ad-SAMAccountName=%(user)s)))", True),
492+
("(|(objectclass=userproxy)(objectclass=user))", False),
493+
],
494+
)
495+
def test_ldap_customer_filter_validation(filter, with_user, ldap_configuration, ldap_settings):
486496
# From AAP-36738
487-
customer_filter = "(&(sAMAccountName=%(user)s)(memberOf:1.3.850.114256.1.4.1241:=CN=ravioli,CN=Users,DC=username2024,DC=local))"
488-
validate_ldap_filter(customer_filter, True)
497+
validate_ldap_filter(filter, with_user)
489498

490499

491500
@pytest.mark.django_db

0 commit comments

Comments
 (0)