Skip to content

Commit 28ef3a6

Browse files
thenetsphessjohn-westcott-iv
authored
[AAP-48822] Add default LDAP connection options to match documentation (#785)
## Description This PR extends the work done in PR #756 by improving how LDAP connection options are handled in the UI while maintaining the default `OPT_REFERRALS` connection option functionality. https://issues.redhat.com/browse/AAP-48822 **Building on PR #756:** PR #756 added the default LDAP connection options that our documentation says we have, specifically setting `OPT_REFERRALS` that was previously missing. This PR continues that work by addressing the UI experience. - What is being changed? - Changed the default value of CONNECTION_OPTIONS field from `default_connection_options` to an empty dict `{}` - Updated backend logic to merge the default `OPT_REFERRALS` setting with user-provided options when CONNECTION_OPTIONS is empty or not a dict - Builds upon the default `OPT_REFERRALS` connection option established in PR #756 - Why is this change needed? - While PR #756 correctly established the default `OPT_REFERRALS` option, users should not be forced to see this default pre-populated in the UI configuration form - How does this change address the issue? - UI now shows empty CONNECTION_OPTIONS field by default (cleaner user experience) - Backend properly merges the default `OPT_REFERRALS` setting with any user-provided options - Maintains backward compatibility with existing configurations - Preserves the `OPT_REFERRALS` default connection option functionality from PR #756 ## 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 - [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 ### Prerequisites - LDAP authenticator configuration environment ### Steps to Test 1. Add some debugger or logger to monitor the final value of the `connection_options` variable 2. Run the `test_ldap_config_defaults` ATF test and put a breakpoint before its teardown, this will setup a valid LDAP server into your AAP 3. Check the value of the `connection_options`, which must contain the expected defaults ### Expected Results - The `connection_options` must always have the `OPT_REFERRALS` key - The default values must be used if the user didn't provide a different one ## Additional Context This PR is a continuation of the LDAP connection options work: 1. **PR #756**: Established the missing default LDAP connection options that match our documentation 2. **This PR (#785)**: Improves the UI experience by not forcing the `OPT_REFERRALS` default to be visible in the form while maintaining all the backend functionality The combination of both PRs provides the complete solution: proper `OPT_REFERRALS` default that matches documentation + clean UI experience. ### Required Actions - [ ] Requires documentation updates - [ ] Requires downstream repository changes - [ ] Requires infrastructure/deployment changes - [ ] Requires coordination with other teams --------- Co-authored-by: Pablo Hess <[email protected]> Co-authored-by: John Westcott IV <[email protected]>
1 parent cb33897 commit 28ef3a6

File tree

2 files changed

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

2 files changed

+125
-1
lines changed

ansible_base/authentication/authenticator_plugins/ldap.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828

2929
user_search_string = '%(user)s'
30+
default_connection_options = {'OPT_REFERRALS': 0}
3031

3132

3233
class PosixUIDGroupType(LDAPGroupType):
@@ -434,7 +435,14 @@ def __init__(self, prefix: str = 'AUTH_LDAP_', defaults: dict = {}):
434435
setattr(self, 'SERVER_URI', ','.join(defaults['SERVER_URI']))
435436

436437
# Connection options need to be set as {"integer": "value"} but our configuration has {"friendly_name": "value"} so we need to convert them
437-
connection_options = defaults.get('CONNECTION_OPTIONS', {})
438+
connection_options = defaults.get('CONNECTION_OPTIONS')
439+
if not isinstance(connection_options, dict):
440+
logger.warning(f"Invalid CONNECTION_OPTIONS (not a dict): {connection_options}")
441+
connection_options = {}
442+
_tmp_connection_options = default_connection_options.copy()
443+
_tmp_connection_options.update(connection_options)
444+
connection_options = _tmp_connection_options
445+
del _tmp_connection_options
438446
valid_options = dict([(v, k) for k, v in ldap.OPT_NAMES_DICT.items()])
439447
internal_data = {}
440448
for key in connection_options:

test_app/tests/authentication/authenticator_plugins/test_ldap.py

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
LDAPSearchField,
1515
LDAPSettings,
1616
PosixUIDGroupType,
17+
default_connection_options,
1718
find_class_in_modules,
1819
validate_ldap_filter,
1920
)
@@ -771,3 +772,118 @@ def test_is_member_missing_uid(group_type, ldap_user):
771772
ldap_user.attrs = {"gidNumber": ["1000"]}
772773
result = group_type.is_member(ldap_user, "cn=group,dc=example,dc=com")
773774
assert result is False
775+
776+
777+
def test_ldap_config_defaults():
778+
from ansible_base.authentication.authenticator_plugins.ldap import LDAPConfiguration, LDAPSettings
779+
780+
config = LDAPConfiguration()
781+
errors = []
782+
783+
# Verify basic field defaults
784+
if config['START_TLS'].default is not False:
785+
errors.append(f"START_TLS did not default to false, got {config['START_TLS'].default}")
786+
787+
# Verify CONNECTION_OPTIONS field default is empty (for clean UI)
788+
if config['CONNECTION_OPTIONS'].default != {}:
789+
errors.append(f"CONNECTION_OPTIONS field did not default to empty dict, got {config['CONNECTION_OPTIONS'].default}")
790+
791+
# Verify that LDAPSettings properly applies defaults when CONNECTION_OPTIONS is empty
792+
test_config = {
793+
'SERVER_URI': ['ldap://example.com'],
794+
'CONNECTION_OPTIONS': {}, # Empty, should get merged with defaults
795+
'GROUP_TYPE': 'PosixGroupType',
796+
'GROUP_TYPE_PARAMS': {"name_attr": "cn"},
797+
}
798+
settings = LDAPSettings(defaults=test_config)
799+
800+
# Check that the defaults were applied in the settings object
801+
import ldap
802+
803+
expected_referrals = ldap.OPT_REFERRALS in settings.CONNECTION_OPTIONS and settings.CONNECTION_OPTIONS[ldap.OPT_REFERRALS] == 0
804+
805+
if not expected_referrals:
806+
errors.append("LDAPSettings did not apply OPT_REFERRALS default when CONNECTION_OPTIONS was empty")
807+
808+
assert errors == []
809+
810+
811+
def test_ldap_connection_options_user_override():
812+
import ldap
813+
814+
from ansible_base.authentication.authenticator_plugins.ldap import LDAPSettings
815+
816+
errors = []
817+
818+
# Test scenario 1: User overrides default values
819+
test_config_override = {
820+
'SERVER_URI': ['ldap://example.com'],
821+
'CONNECTION_OPTIONS': {
822+
'OPT_REFERRALS': 1, # Override default value of default_connection_options['OPT_REFERRALS']
823+
},
824+
'GROUP_TYPE': 'PosixGroupType',
825+
'GROUP_TYPE_PARAMS': {"name_attr": "cn"},
826+
}
827+
settings = LDAPSettings(defaults=test_config_override)
828+
829+
# Verify user values override defaults
830+
if settings.CONNECTION_OPTIONS[ldap.OPT_REFERRALS] != 1:
831+
errors.append(f"Expected OPT_REFERRALS to be overridden to 1, got {settings.CONNECTION_OPTIONS[ldap.OPT_REFERRALS]}")
832+
833+
# Test scenario 2: User provides additional options not in defaults
834+
test_config_additional = {
835+
'SERVER_URI': ['ldap://example.com'],
836+
'CONNECTION_OPTIONS': {
837+
'OPT_PROTOCOL_VERSION': 3, # New option not in defaults
838+
},
839+
'GROUP_TYPE': 'PosixGroupType',
840+
'GROUP_TYPE_PARAMS': {"name_attr": "cn"},
841+
}
842+
settings = LDAPSettings(defaults=test_config_additional)
843+
844+
# Verify defaults are still applied
845+
if settings.CONNECTION_OPTIONS[ldap.OPT_REFERRALS] != default_connection_options['OPT_REFERRALS']:
846+
errors.append(
847+
f"Expected OPT_REFERRALS default ({default_connection_options['OPT_REFERRALS']}) "
848+
f"to be preserved, got {settings.CONNECTION_OPTIONS[ldap.OPT_REFERRALS]}"
849+
)
850+
# Verify additional option is included
851+
if settings.CONNECTION_OPTIONS[ldap.OPT_PROTOCOL_VERSION] != 3:
852+
errors.append(f"Expected OPT_PROTOCOL_VERSION to be set to 3, got {settings.CONNECTION_OPTIONS.get(ldap.OPT_PROTOCOL_VERSION)}")
853+
854+
# Test scenario 3: Mixed scenario - some overrides, some defaults, some new
855+
test_config_mixed = {
856+
'SERVER_URI': ['ldap://example.com'],
857+
'CONNECTION_OPTIONS': {
858+
'OPT_REFERRALS': 1, # Override default
859+
'OPT_PROTOCOL_VERSION': 3, # New option
860+
},
861+
'GROUP_TYPE': 'PosixGroupType',
862+
'GROUP_TYPE_PARAMS': {"name_attr": "cn"},
863+
}
864+
settings = LDAPSettings(defaults=test_config_mixed)
865+
866+
# Verify override
867+
if settings.CONNECTION_OPTIONS[ldap.OPT_REFERRALS] != 1:
868+
errors.append(f"Expected OPT_REFERRALS to be overridden to 1, got {settings.CONNECTION_OPTIONS[ldap.OPT_REFERRALS]}")
869+
# Verify new option
870+
if settings.CONNECTION_OPTIONS[ldap.OPT_PROTOCOL_VERSION] != 3:
871+
errors.append(f"Expected OPT_PROTOCOL_VERSION to be set to 3, got {settings.CONNECTION_OPTIONS.get(ldap.OPT_PROTOCOL_VERSION)}")
872+
873+
# Test scenario 4: CONNECTION_OPTIONS is not a dict (edge case)
874+
test_config_non_dict = {
875+
'SERVER_URI': ['ldap://example.com'],
876+
'CONNECTION_OPTIONS': "invaaalid", # Not a dict
877+
'GROUP_TYPE': 'PosixGroupType',
878+
'GROUP_TYPE_PARAMS': {"name_attr": "cn"},
879+
}
880+
settings = LDAPSettings(defaults=test_config_non_dict)
881+
882+
# Should fall back to defaults only
883+
if settings.CONNECTION_OPTIONS[ldap.OPT_REFERRALS] != default_connection_options['OPT_REFERRALS']:
884+
errors.append(
885+
f"Expected OPT_REFERRALS default ({default_connection_options['OPT_REFERRALS']}) "
886+
f"when CONNECTION_OPTIONS is invalid, got {settings.CONNECTION_OPTIONS[ldap.OPT_REFERRALS]}"
887+
)
888+
889+
assert errors == []

0 commit comments

Comments
 (0)