diff --git a/docs/source/LDAP-Authentication.rst b/docs/source/LDAP-Authentication.rst index a93482674..cf0ff904d 100644 --- a/docs/source/LDAP-Authentication.rst +++ b/docs/source/LDAP-Authentication.rst @@ -47,7 +47,7 @@ If not existing already, copy the template and edit with your LDAP settings: The configuration file at ``/config/Ldap.config.php`` contains all available options with detailed comments explaining each setting. You can also view and modify these settings through the web admin interface at **Application Management > Configuration**. Key settings include: -- **host**: LDAP server URL(s) +- **uri**: LDAP URI string. For multiple servers, use a space-separated list of URIs. - **binddn/bindpw**: Service account credentials for directory searches - **basedn**: Base DN where users are located - **user.id.attribute**: LDAP attribute for username lookup (typically ``uid``) @@ -55,6 +55,38 @@ The configuration file at ``/config/Ldap.config.php`` contains all available opt - **sync.groups**: Enable group membership synchronization - **database.auth.when.ldap.user.not.found**: Fallback to database authentication +URI examples: + +.. code-block:: php + + // single LDAP server (unencrypted LDAP, explicit port) + 'uri' => 'ldap://ldap1.example.com:389', + + // single LDAP server (unencrypted LDAP, default port 389) + 'uri' => 'ldap://ldap1.example.com', + + // single LDAP server over LDAPS (TLS, explicit port) + 'uri' => 'ldaps://ldap1.example.com:636', + + // single LDAP server over LDAPS (TLS, default port 636) + 'uri' => 'ldaps://ldap1.example.com', + + // multiple LDAP servers (space-separated URIs in one string) + 'uri' => 'ldap://ldap1.example.com:389 ldap://ldap2.example.com:389', + + // multiple LDAPS servers + 'uri' => 'ldaps://ldap1.example.com:636 ldaps://ldap2.example.com:636', + +Port defaults: + +- ``ldap://`` uses port ``389`` by default when no port is specified. +- ``ldaps://`` uses port ``636`` by default when no port is specified. + +Breaking change: + +- ``host`` and ``port`` are no longer supported. +- Configure LDAP endpoints only through ``uri``. + Alternatively, configure the plugin through the web admin interface at **Application Configuration** (``/Web/admin/manage_configuration.php``) and select **Authentification-Ldap**. Refer to ``/plugins/Authentication/Ldap/Ldap.config.dist.php`` for complete documentation of all options. @@ -75,7 +107,7 @@ Common Issues ~~~~~~~~~~~~~ **Connection failures** - - Verify server hostname and port accessibility + - Verify LDAP URI hostname and port accessibility - Check firewall rules - Test with ``telnet ldap.example.com 389`` diff --git a/plugins/Authentication/Ldap/Ldap.config.dist.php b/plugins/Authentication/Ldap/Ldap.config.dist.php index dfed1dccc..2dfee73dc 100644 --- a/plugins/Authentication/Ldap/Ldap.config.dist.php +++ b/plugins/Authentication/Ldap/Ldap.config.dist.php @@ -6,11 +6,16 @@ return [ 'settings' => [ 'ldap' => [ - // comma separated list of ldap servers such as ldap://mydomain1,ldap://localhost - 'host' => 'ldap://localhost', - - // default ldap port 389 or 636 for ssl. - 'port' => 389, + // LDAP URI(s). For multiple servers, separate each URI with spaces. + // If no port is specified, defaults are: ldap:// = 389, ldaps:// = 636. + // examples: + // 'ldap://ldap1.example.com' + // 'ldap://ldap1.example.com:389' + // 'ldaps://ldap1.example.com' + // 'ldaps://ldap1.example.com:636' + // 'ldap://ldap1.example.com:389 ldap://ldap2.example.com:389' + // 'ldaps://ldap1.example.com:636 ldaps://ldap2.example.com:636' + 'uri' => 'ldap://localhost:389', // LDAP protocol version 'version' => 3, diff --git a/plugins/Authentication/Ldap/LdapConfigKeys.php b/plugins/Authentication/Ldap/LdapConfigKeys.php index 1571fb839..f9ab0b268 100644 --- a/plugins/Authentication/Ldap/LdapConfigKeys.php +++ b/plugins/Authentication/Ldap/LdapConfigKeys.php @@ -6,21 +6,12 @@ class LdapConfigKeys extends PluginConfigKeys { public const CONFIG_ID = 'ldap'; - public const HOST = [ - 'key' => 'host', + public const URI = [ + 'key' => 'uri', 'type' => 'string', 'default' => '', - 'label' => 'LDAP Host', - 'description' => 'Hostname or IP address of LDAP server. Should start with ldap:// or ldaps://', - 'section' => 'ldap' - ]; - - public const PORT = [ - 'key' => 'port', - 'type' => 'integer', - 'default' => 389, - 'label' => 'LDAP Port', - 'description' => 'Port of LDAP server (usually 389 or 636 for SSL)', + 'label' => 'LDAP server URI(s)', + 'description' => 'LDAP server URI(s). Use ldap:// or ldaps://. For multiple servers, separate URIs with spaces.', 'section' => 'ldap' ]; diff --git a/plugins/Authentication/Ldap/LdapOptions.php b/plugins/Authentication/Ldap/LdapOptions.php index a89a4a392..45eda2423 100644 --- a/plugins/Authentication/Ldap/LdapOptions.php +++ b/plugins/Authentication/Ldap/LdapOptions.php @@ -5,6 +5,10 @@ class LdapOptions { private $_options = []; + /** + * @var array + */ + private $rawLdapSettings = []; public function __construct() { @@ -13,7 +17,10 @@ public function __construct() $configPath = dirname(__FILE__) . '/Ldap.config.dist.php'; } - require_once($configPath); + $loadedConfig = @require $configPath; + if (is_array($loadedConfig) && isset($loadedConfig['settings']['ldap']) && is_array($loadedConfig['settings']['ldap'])) { + $this->rawLdapSettings = $loadedConfig['settings']['ldap']; + } Configuration::Instance()->Register( $configPath, @@ -26,9 +33,8 @@ public function __construct() public function Ldap2Config() { - $hosts = $this->GetHosts(); + $hosts = $this->GetUris(); $this->SetOption('host', $hosts); - $this->SetOption('port', $this->GetConfig(LdapConfigKeys::PORT, new IntConverter())); $this->SetOption('starttls', $this->GetConfig(LdapConfigKeys::STARTTLS, new BooleanConverter())); $this->SetOption('version', $this->GetConfig(LdapConfigKeys::VERSION, new IntConverter())); $this->SetOption('binddn', $this->GetConfig(LdapConfigKeys::BINDDN)); @@ -47,7 +53,7 @@ public function RetryAgainstDatabase() public function Controllers() { - return $this->GetHosts(); + return $this->GetUris(); } private function SetOption($key, $value) @@ -64,15 +70,51 @@ private function GetConfig($configDef, $converter = null) return Configuration::Instance()->File(LdapConfigKeys::CONFIG_ID)->GetKey($configDef, $converter); } - private function GetHosts() + private function GetUris() { - $hosts = explode(',', $this->GetConfig(LdapConfigKeys::HOST)); + $this->AssertLegacyHostPortNotConfigured(); - for ($i = 0; $i < count($hosts); $i++) { - $hosts[$i] = trim($hosts[$i]); + $uriConfig = trim($this->GetConfig(LdapConfigKeys::URI)); + if (empty($uriConfig)) { + throw new RuntimeException("LDAP setting 'uri' is required and must contain at least one ldap:// or ldaps:// URI."); + } + + $uris = preg_split('/\s+/', $uriConfig) ?: []; + foreach ($uris as $uri) { + $scheme = parse_url($uri, PHP_URL_SCHEME); + $host = parse_url($uri, PHP_URL_HOST); + + if (!in_array($scheme, ['ldap', 'ldaps'], true) || empty($host)) { + throw new RuntimeException( + sprintf("Invalid LDAP URI '%s'. Use ldap:// or ldaps:// with a hostname. For multiple servers, separate URIs with spaces.", $uri) + ); + } } - return $hosts; + return $uris; + } + + private function AssertLegacyHostPortNotConfigured() + { + if (array_key_exists('host', $this->rawLdapSettings) || array_key_exists('port', $this->rawLdapSettings)) { + throw new RuntimeException("LDAP settings 'host' and 'port' have been removed. Use only the 'uri' setting."); + } + + // Also enforce legacy key removal when values are provided via env vars or tests. + $legacyHost = trim($this->GetConfig([ + 'key' => 'host', + 'section' => 'ldap', + 'type' => 'string' + ])); + $legacyPort = trim($this->GetConfig([ + 'key' => 'port', + 'section' => 'ldap', + 'type' => 'string' + ])); + + if (!empty($legacyHost) || !empty($legacyPort)) { + throw new RuntimeException("LDAP settings 'host' and 'port' have been removed. Use only the 'uri' setting."); + } } public function BaseDn() diff --git a/tests/Plugins/Authentication/Ldap/LdapTest.php b/tests/Plugins/Authentication/Ldap/LdapTest.php index ce8378841..67a7c093c 100644 --- a/tests/Plugins/Authentication/Ldap/LdapTest.php +++ b/tests/Plugins/Authentication/Ldap/LdapTest.php @@ -184,8 +184,7 @@ public function testDoesNotSyncIfUserWasNotFoundInLdap() public function testConstructsOptionsCorrectly() { - $hosts = 'localhost, localhost.2'; - $port = '389'; + $uris = 'ldap://localhost:389 ldap://localhost.2:389'; $binddn = 'cn=admin,ou=users,dc=example,dc=org'; $password = 'pw'; $base = 'dc=example,dc=org'; @@ -193,8 +192,7 @@ public function testConstructsOptionsCorrectly() $version = '3'; $configFile = new FakeConfigFile(); - $configFile->SetKey(LdapConfigKeys::HOST, $hosts); - $configFile->SetKey(LdapConfigKeys::PORT, $port); + $configFile->SetKey(LdapConfigKeys::URI, $uris); $configFile->SetKey(LdapConfigKeys::BINDDN, $binddn); $configFile->SetKey(LdapConfigKeys::BINDPW, $password); $configFile->SetKey(LdapConfigKeys::BASEDN, $base); @@ -207,8 +205,7 @@ public function testConstructsOptionsCorrectly() $options = $ldapOptions->Ldap2Config(); $this->assertNotNull($this->fakeConfig->_RegisteredFiles[LdapConfigKeys::CONFIG_ID]); - $this->assertEquals('localhost', $options['host'][0], 'domain_controllers must be an array'); - $this->assertEquals(intval($port), $options['port'], 'port should be int'); + $this->assertEquals('ldap://localhost:389', $options['host'][0], 'controllers must be an array of URIs'); $this->assertEquals($binddn, $options['binddn']); $this->assertEquals($password, $options['bindpw']); $this->assertEquals($base, $options['basedn']); @@ -218,15 +215,49 @@ public function testConstructsOptionsCorrectly() public function testGetAllHosts() { - $controllers = 'localhost, localhost.2'; + $controllers = 'ldap://localhost:389 ldap://localhost.2:389'; $configFile = new FakeConfigFile(); - $configFile->SetKey(LdapConfigKeys::HOST, $controllers); + $configFile->SetKey(LdapConfigKeys::URI, $controllers); $this->fakeConfig->SetFile(LdapConfigKeys::CONFIG_ID, $configFile); $options = new LdapOptions(); - $this->assertEquals(['localhost', 'localhost.2'], $options->Controllers(), "comma separated values should become array"); + $this->assertEquals(['ldap://localhost:389', 'ldap://localhost.2:389'], $options->Controllers(), "space separated uris should become array"); + } + + public function testThrowsIfLegacyHostIsConfigured() + { + $configFile = new FakeConfigFile(); + $configFile->SetKey([ + 'key' => 'host', + 'section' => 'ldap', + 'type' => 'string', + ], 'ldap://localhost'); + $this->fakeConfig->SetFile(LdapConfigKeys::CONFIG_ID, $configFile); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage("LDAP settings 'host' and 'port' have been removed"); + + $options = new LdapOptions(); + $options->Ldap2Config(); + } + + public function testThrowsIfLegacyPortIsConfigured() + { + $configFile = new FakeConfigFile(); + $configFile->SetKey([ + 'key' => 'port', + 'section' => 'ldap', + 'type' => 'string', + ], '389'); + $this->fakeConfig->SetFile(LdapConfigKeys::CONFIG_ID, $configFile); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage("LDAP settings 'host' and 'port' have been removed"); + + $options = new LdapOptions(); + $options->Ldap2Config(); } public function testUserHandlesArraysAsAttribute()