Fix LDAP authentication

[lucs7]

originally by @christiankreidl, just rebased it so we can proceeed

Hi!

This pull request fixes:

logging to file app.log not working due to comparison of upper- and lower-case strings
LDAP authentication not working due to problems with new config file format
ConfigKeys and config file missing prefix
hard-coded string instead of ConfigKey used
ActiveDirectory authentication also fixed, but could not be tested ( #856 )
LDAP log message not an error thus changed to debug

Closes 782, 824, 856

[Copilot]

Pull request overview

This pull request aims to fix LDAP and Active Directory authentication issues by restructuring plugin configuration files to use proper namespacing. The changes introduce a new standardized format where plugin settings are wrapped under their respective section keys (e.g., ['settings' => ['ldap' => [...]]), along with comprehensive tests to validate this structure.

Key changes:

• All plugin config.dist.php files now nest settings under plugin-specific section keys
• New comprehensive test suite to validate plugin configuration structure and loading
• Updated ConfigKeys classes to remove redundant prefixes from key definitions
• Fixed LDAP debug logging to use Debug level instead of Error level

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/Plugins/Authentication/PluginConfigLoadingTest.php	Adds comprehensive test suite to validate plugin config structure, key naming, and loading behavior
plugins/Authentication/WordPress/WordPress.config.dist.php	Wraps config settings under 'wordpress' section key
plugins/Authentication/Shibboleth/ShibbolethConfigKeys.php	Removes redundant 'shibboleth.' prefix from key definitions
plugins/Authentication/Shibboleth/Shibboleth.config.dist.php	Wraps config settings under 'shibboleth' section key
plugins/Authentication/Saml/Saml.config.dist.php	Wraps config settings under 'saml' section key
plugins/Authentication/MoodleAdv/MoodleAdvConfigKeys.php	Removes redundant 'moodleadv.' prefix from key definitions
plugins/Authentication/MoodleAdv/MoodleAdv.config.dist.php	Wraps config settings under 'moodleadv' section key
plugins/Authentication/Moodle/MoodleConfigKeys.php	Removes redundant 'moodle.' prefix from key definitions
plugins/Authentication/Moodle/Moodle.config.dist.php	Wraps config settings under 'moodle' section key
plugins/Authentication/Mellon/Mellon.config.dist.php	Wraps config settings under 'mellon' section key
plugins/Authentication/Ldap/LdapOptions.php	Replaces hard-coded string with LdapConfigKeys constant
plugins/Authentication/Ldap/LdapConfigKeys.php	Incorrectly adds 'ldap.' prefix to most keys (should be bare keys like DEBUG_ENABLED)
plugins/Authentication/Ldap/Ldap2Wrapper.php	Changes LDAP uid attribute logging from Error to Debug level
plugins/Authentication/Ldap/Ldap.config.dist.php	Wraps config settings under 'ldap' section key
plugins/Authentication/Drupal/DrupalConfigKeys.php	Removes redundant 'drupal.' prefix from key definitions
plugins/Authentication/Drupal/Drupal.config.dist.php	Wraps config settings under 'drupal' section key
plugins/Authentication/CAS/CASConfigKeys.php	Removes redundant 'cas.' prefix from key definitions
plugins/Authentication/CAS/CAS.config.dist.php	Wraps config settings under 'cas' section key
plugins/Authentication/ActiveDirectory/ActiveDirectoryConfigKeys.php	Incorrectly adds 'activedirectory.' prefix to all keys (should be bare keys)
plugins/Authentication/ActiveDirectory/ActiveDirectory.config.dist.php	Wraps config settings under 'activedirectory' section key
lib/Config/PluginConfigKeys.php	Updates findByKey to handle section-prefixed key lookups
lib/Config/Configuration.php	Fixes config rewriting to preserve unknown subkeys for validation

Comments suppressed due to low confidence (2)

plugins/Authentication/ActiveDirectory/ActiveDirectoryConfigKeys.php:142

• The key should be 'domain.controllers' not 'activedirectory.domain.controllers'. Since this config has 'section' => 'activedirectory', the configuration system will automatically prefix it. With the current setup, the full key would be 'activedirectory.activedirectory.domain.controllers' which is incorrect. This same issue affects all keys in this file (lines 9, 18, 27, 36, 46, 55, 64, 73, 82, 91, 100, 109, 118, 127, and 136) - none should include the 'activedirectory.' prefix in the 'key' field.

 public const DOMAIN_CONTROLLERS = [ 'key' => 'activedirectory.domain.controllers', 'type' => 'string', 'default' => '', 'label' => 'Domain Controllers', 'description' => 'Comma separated list of domain controllers', 'section' => 'activedirectory' ]; public const PORT = [ 'key' => 'activedirectory.port', 'type' => 'integer', 'default' => 389, 'label' => 'LDAP Port', 'description' => 'Default LDAP port (389 or 636 for SSL)', 'section' => 'activedirectory' ]; public const USERNAME = [ 'key' => 'activedirectory.username', 'type' => 'string', 'default' => '', 'label' => 'Admin Username', 'description' => 'Username for binding to LDAP service. Will have account.suffix appended automatically', 'section' => 'activedirectory' ]; public const PASSWORD = [ 'key' => 'activedirectory.password', 'type' => 'string', 'default' => '', 'label' => 'Admin Password', 'description' => 'Password for binding to LDAP service', 'section' => 'activedirectory', 'is_protected' => true ]; public const BASEDN = [ 'key' => 'activedirectory.basedn', 'type' => 'string', 'default' => '', 'label' => 'Base DN', 'description' => 'Base DN for your domain', 'section' => 'activedirectory' ]; public const USE_SSL = [ 'key' => 'activedirectory.use.ssl', 'type' => 'boolean', 'default' => false, 'label' => 'Use SSL', 'description' => 'Connect using SSL (LDAPS)', 'section' => 'activedirectory' ]; public const VERSION = [ 'key' => 'activedirectory.version', 'type' => 'integer', 'default' => 3, 'label' => 'LDAP Protocol Version', 'description' => 'LDAP protocol version (usually 3)', 'section' => 'activedirectory' ]; public const ACCOUNT_SUFFIX = [ 'key' => 'activedirectory.account.suffix', 'type' => 'string', 'default' => '', 'label' => 'Account Suffix', 'description' => 'Account suffix for your domain (e.g. @domain.com)', 'section' => 'activedirectory' ]; public const SECTION_AD = [ 'key' => 'activedirectory.ad', 'type' => 'string', 'default' => '', 'label' => 'Active Directory Section', 'description' => 'Configuration section for Active Directory settings', 'section' => 'activedirectory', 'is_hidden' => true ]; public const RETRY_AGAINST_DATABASE = [ 'key' => 'activedirectory.database.auth.when.ldap.user.not.found', 'type' => 'boolean', 'default' => false, 'label' => 'Retry Against Database', 'description' => 'Retry authentication against database if LDAP authentication fails', 'section' => 'activedirectory' ]; public const ATTRIBUTE_MAPPING = [ 'key' => 'activedirectory.attribute.mapping', 'type' => 'string', 'default' => 'sn=sn,givenname=givenname,mail=mail,telephonenumber=telephonenumber,physicaldeliveryofficename=physicaldeliveryofficename,title=title', 'label' => 'Attribute Mapping', 'description' => 'Mapping of required attributes to LDAP attributes', 'section' => 'activedirectory' ]; public const REQUIRED_GROUPS = [ 'key' => 'activedirectory.required.groups', 'type' => 'string', 'default' => '', 'label' => 'Required Groups', 'description' => 'Groups that users must belong to (comma separated)', 'section' => 'activedirectory' ]; public const SYNC_GROUPS = [ 'key' => 'activedirectory.sync.groups', 'type' => 'boolean', 'default' => false, 'label' => 'Sync Groups', 'description' => 'Synchronize AD groups with LibreBooking', 'section' => 'activedirectory' ]; public const USE_SSO = [ 'key' => 'activedirectory.use.sso', 'type' => 'boolean', 'default' => false, 'label' => 'Use SSO', 'description' => 'Use single sign-on with AD', 'section' => 'activedirectory' ]; public const PREVENT_CLEAN_USERNAME = [ 'key' => 'activedirectory.prevent.clean.username', 'type' => 'boolean', 'default' => false, 'label' => 'Prevent Clean Username', 'description' => 'Do not clean username from domain or email format', 'section' => 'activedirectory' ]; 

plugins/Authentication/Ldap/LdapConfigKeys.php:143

• Similar to the HOST key, all keys in LdapConfigKeys should not include the 'ldap.' prefix. The keys at lines 19, 28, 37, 46, 55, 65, 74, 83, 92, 101, 110, 119, 128, and 137 all incorrectly include 'ldap.' prefix. For example, line 19 should be 'key' => 'port' not 'key' => 'ldap.port'. The section field already specifies 'ldap' which will be prefixed automatically.

 public const PORT = [ 'key' => 'ldap.port', 'type' => 'integer', 'default' => 389, 'label' => 'LDAP Port', 'description' => 'Port of LDAP server (usually 389 or 636 for SSL)', 'section' => 'ldap' ]; public const VERSION = [ 'key' => 'ldap.version', 'type' => 'integer', 'default' => 3, 'label' => 'LDAP Version', 'description' => 'LDAP protocol version (usually 3)', 'section' => 'ldap' ]; public const STARTTLS = [ 'key' => 'ldap.starttls', 'type' => 'boolean', 'default' => false, 'label' => 'Use StartTLS', 'description' => 'Whether to use StartTLS encryption', 'section' => 'ldap' ]; public const BINDDN = [ 'key' => 'ldap.binddn', 'type' => 'string', 'default' => '', 'label' => 'Bind DN', 'description' => 'DN to bind to LDAP server', 'section' => 'ldap' ]; public const BINDPW = [ 'key' => 'ldap.bindpw', 'type' => 'string', 'default' => '', 'label' => 'Bind Password', 'description' => 'Password to bind to LDAP server', 'section' => 'ldap', 'is_protected' => true ]; public const BASEDN = [ 'key' => 'ldap.basedn', 'type' => 'string', 'default' => '', 'label' => 'Base DN', 'description' => 'Base DN for LDAP searches', 'section' => 'ldap' ]; public const FILTER = [ 'key' => 'ldap.filter', 'type' => 'string', 'default' => '', 'label' => 'Search Filter', 'description' => 'Optional LDAP search filter for additional constraints (leave empty for default)', 'section' => 'ldap' ]; public const SCOPE = [ 'key' => 'ldap.scope', 'type' => 'string', 'default' => 'sub', 'label' => 'Search Scope', 'description' => 'LDAP search scope (base, one, or sub)', 'section' => 'ldap' ]; public const RETRY_AGAINST_DATABASE = [ 'key' => 'ldap.database.auth.when.ldap.user.not.found', 'type' => 'boolean', 'default' => false, 'label' => 'Retry Against Database', 'description' => 'Try to authenticate against the database if LDAP authentication fails', 'section' => 'ldap' ]; public const ATTRIBUTE_MAPPING = [ 'key' => 'ldap.attribute.mapping', 'type' => 'string', 'default' => 'sn=sn,givenname=givenname,mail=mail,telephonenumber=telephonenumber,physicaldeliveryofficename=physicaldeliveryofficename,title=title', 'label' => 'Attribute Mapping', 'description' => 'Mapping of LibreBooking attributes to LDAP attributes', 'section' => 'ldap' ]; public const USER_ID_ATTRIBUTE = [ 'key' => 'ldap.user.id.attribute', 'type' => 'string', 'default' => 'uid', 'label' => 'User ID Attribute', 'description' => 'LDAP attribute to use as user ID', 'section' => 'ldap' ]; public const REQUIRED_GROUP = [ 'key' => 'ldap.required.group', 'type' => 'string', 'default' => '', 'label' => 'Required Group', 'description' => 'LDAP group required for authentication', 'section' => 'ldap' ]; public const SYNC_GROUPS = [ 'key' => 'ldap.sync.groups', 'type' => 'boolean', 'default' => false, 'label' => 'Sync Groups', 'description' => 'Synchronize LDAP groups with LibreBooking groups', 'section' => 'ldap' ]; public const PREVENT_CLEAN_USERNAME = [ 'key' => 'ldap.prevent.clean.username', 'type' => 'boolean', 'default' => false, 'label' => 'Prevent Clean Username', 'description' => 'Do not clean username from domain or email format', 'section' => 'ldap' ]; 

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[lucs7]

Sorry but i noticed an issue in the phpunit failing silently again, hope this fixes it now

[LeSteph34]

Hello,
I confirm that this fix resolves LDAP authentication problem on my server.
Thanks to @lucs7 and @christiankreidl !