Skip to content

Commit a74944a

Browse files
AAP-43543 Force LDAP created usernames to be lowercase (#714)
## Description <!-- Mandatory: Provide a clear, concise description of the changes and their purpose --> - What is being changed? Modify the LDAP authenticator to always use a lowercase username. - Why is this change needed? This is how it worked in 2.4, we accidentally overrode a function in the 2.5 implementation that broke this. - How does this change address the issue? This function was overridden in 2.5 and is responsible for converting the username given to django into lowercase. ## Type of Change <!-- Mandatory: Check one or more boxes that apply --> - [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 <!-- These items help ensure quality - they complement our automated CI checks --> - [X] I have performed a self-review of my code - [X] I have added relevant comments to complex code sections - [X] I have updated documentation where needed - [X] I have considered the security impact of these changes - [X] 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 <!-- Optional for test-only changes. Mandatory for all other changes --> <!-- Must be detailed enough for reviewers to reproduce --> ### Prerequisites <!-- List any specific setup required --> ### Steps to Test 1. With the new version of the authenticator attempt to login through LDAP with usernames in different cases. 2. You should always have a lowercase username in django ### Expected Results <!-- Describe what should happen after following the steps --> ## Additional Context <!-- Optional but helpful information --> ### Required Actions <!-- Check if changes require work in other areas --> <!-- Remove section if no external actions needed --> - [ ] Requires documentation updates <!-- API docs, feature docs, deployment guides --> - [ ] Requires downstream repository changes <!-- Specify repos: django-ansible-base, eda-server, etc. --> - [ ] Requires infrastructure/deployment changes <!-- CI/CD, installer updates, new services --> - [ ] Requires coordination with other teams <!-- UI team, platform services, infrastructure --> - [ ] Blocked by PR/MR: #XXX <!-- Reference blocking PRs/MRs with brief context --> ### Screenshots/Logs <!-- Add if relevant to demonstrate the changes --> --------- Co-authored-by: Rick Elrod <[email protected]>
1 parent 5732c6c commit a74944a

File tree

2 files changed

+27
-1
lines changed
  • ansible_base/authentication/authenticator_plugins
  • test_app/tests/authentication/authenticator_plugins

2 files changed

+27
-1
lines changed

ansible_base/authentication/authenticator_plugins/ldap.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ def validate_ldap_filter(value: Any, with_user: bool = False) -> None:
180180
if with_user:
181181
if user_search_string not in value:
182182
raise ValidationError(_('DN must include "{}" placeholder for username: {}').format(user_search_string, value))
183+
183184
dn_value = value.replace(user_search_string, 'USER')
184185

185186
if re.match(r'^\([A-Za-z0-9-]+?=[^()]+?\)$', dn_value):
@@ -515,7 +516,7 @@ def get_or_build_user(self, username, ldap_user):
515516
This gets called by _LDAPUser to create the user in the database.
516517
"""
517518
user, _authenticator_user, created = get_or_create_authenticator_user(
518-
username,
519+
username.lower(),
519520
self.database_instance,
520521
user_details={
521522
"username": username,

test_app/tests/authentication/authenticator_plugins/test_ldap.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import importlib
12
from collections import OrderedDict
23
from unittest import mock
34
from unittest.mock import MagicMock
@@ -679,3 +680,27 @@ def test_ldap_user_search_validation(
679680
)
680681
def test_ldap_search_field_is_single_search(value, expected_result):
681682
assert LDAPSearchField.is_single_search(value) is expected_result
683+
684+
685+
@pytest.mark.django_db
686+
@pytest.mark.parametrize(
687+
"username",
688+
[
689+
("Timmy"),
690+
("TIMMY"),
691+
("TiMmY"),
692+
],
693+
)
694+
def test_get_or_build_user(username, ldap_authenticator):
695+
from ansible_base.authentication.authenticator_plugins import ldap
696+
697+
with mock.patch(
698+
'ansible_base.authentication.utils.authentication.get_or_create_authenticator_user', return_value=(None, None, None)
699+
) as get_or_create_authenticator_user:
700+
importlib.reload(ldap)
701+
plugin = AuthenticatorPlugin(database_instance=ldap_authenticator)
702+
ldap_object = MagicMock()
703+
plugin.get_or_build_user(username, ldap_object)
704+
assert get_or_create_authenticator_user.called
705+
assert username.lower() in get_or_create_authenticator_user.call_args[0]
706+
assert username not in get_or_create_authenticator_user.call_args[0]

0 commit comments

Comments
 (0)