From cc2ded54e12e3f3140b895067af086cd71cc5dc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20NOBILI?= Date: Mon, 2 Mar 2020 17:08:19 +0100 Subject: [PATCH 1/6] ldap authentication, fixes shaarli/Shaarli#1343 --- application/security/LoginManager.php | 72 ++++++++++++++++++++++----- doc/md/Shaarli-configuration.md | 9 ++++ tests/security/LoginManagerTest.php | 34 +++++++++++++ 3 files changed, 102 insertions(+), 13 deletions(-) diff --git a/application/security/LoginManager.php b/application/security/LoginManager.php index 0b0ce0b1..2cea3f10 100644 --- a/application/security/LoginManager.php +++ b/application/security/LoginManager.php @@ -1,6 +1,7 @@ configManager->get('credentials.salt')); - - if ($login != $this->configManager->get('credentials.login') - || $hash != $this->configManager->get('credentials.hash') - ) { - logm( - $this->configManager->get('resource.log'), - $remoteIp, - 'Login failed for user ' . $login - ); + // Check login matches config + if ($login != $this->configManager->get('credentials.login')) { return false; } - $this->sessionManager->storeLoginInfo($clientIpId); + // Check credentials + try { + if (($this->configManager->get('ldap.host') != "" && $this->checkCredentialsFromLdap($login, $password)) + || ($this->configManager->get('ldap.host') == "" && $this->checkCredentialsFromLocalConfig($login, $password))) { + $this->sessionManager->storeLoginInfo($clientIpId); + logm( + $this->configManager->get('resource.log'), + $remoteIp, + 'Login successful' + ); + return true; + } + } + catch(Exception $exception) { + logm( + $this->configManager->get('resource.log'), + $remoteIp, + 'Exception while checking credentials: ' . $exception + ); + } + logm( $this->configManager->get('resource.log'), $remoteIp, - 'Login successful' + 'Login failed for user ' . $login ); - return true; + return false; + } + + + /** + * Check user credentials from local config + * + * @param string $login Username + * @param string $password Password + * + * @return bool true if the provided credentials are valid, false otherwise + */ + public function checkCredentialsFromLocalConfig($login, $password) { + $hash = sha1($password . $login . $this->configManager->get('credentials.salt')); + + return $login == $this->configManager->get('credentials.login') + && $hash == $this->configManager->get('credentials.hash'); + } + + /** + * Check user credentials are valid through LDAP bind + * + * @param string $remoteIp Remote client IP address + * @param string $clientIpId Client IP address identifier + * @param string $login Username + * @param string $password Password + * + * @return bool true if the provided credentials are valid, false otherwise + */ + public function checkCredentialsFromLdap($login, $password, $connect = null, $bind = null) + { + $connect = $connect ?? function($host) { return ldap_connect($host); }; + $bind = $bind ?? function($handle, $dn, $password) { return ldap_bind($handle, $dn, $password); }; + return $bind($connect($this->configManager->get('ldap.host')), sprintf($this->configManager->get('ldap.dn'), $login), $password); } /** diff --git a/doc/md/Shaarli-configuration.md b/doc/md/Shaarli-configuration.md index 664e36dd..2462e20e 100644 --- a/doc/md/Shaarli-configuration.md +++ b/doc/md/Shaarli-configuration.md @@ -122,6 +122,11 @@ Must be an associative array: `translation domain => translation path`. - **enable_thumbnails**: Enable or disable thumbnail display. - **enable_localcache**: Enable or disable local cache. +### LDAP + +- **host**: LDAP host used for user authentication +- **dn**: user DN template (`sprintf` format, `%s` being replaced by user login) + ## Configuration file example ```json @@ -223,6 +228,10 @@ Must be an associative array: `translation domain => translation path`. "extensions": { "demo": "plugins/demo_plugin/languages/" } + }, + "ldap": { + "host": "ldap://localhost", + "dn": "uid=%s,ou=people,dc=example,dc=org" } } ?> ``` diff --git a/tests/security/LoginManagerTest.php b/tests/security/LoginManagerTest.php index eef0f22a..f2d78802 100644 --- a/tests/security/LoginManagerTest.php +++ b/tests/security/LoginManagerTest.php @@ -78,6 +78,7 @@ public function setUp() 'security.ban_after' => 2, 'security.ban_duration' => 3600, 'security.trusted_proxies' => [$this->trustedProxy], + 'ldap.host' => '', ]); $this->cookie = []; @@ -296,4 +297,37 @@ public function testCheckCredentialsGoodLoginAndPassword() $this->loginManager->checkCredentials('', '', $this->login, $this->password) ); } + + /** + * Check user credentials through LDAP - server unreachable + */ + public function testCheckCredentialsFromUnreachableLdap() + { + $this->configManager->set('ldap.host', 'dummy'); + $this->assertFalse( + $this->loginManager->checkCredentials('', '', $this->login, $this->password) + ); + } + + /** + * Check user credentials through LDAP - wrong login and password supplied + */ + public function testCheckCredentialsFromLdapWrongLoginAndPassword() + { + $this->coddnfigManager->set('ldap.host', 'dummy'); + $this->assertFalse( + $this->loginManager->checkCredentialsFromLdap($this->login, $this->password, function() { return null; }, function() { return false; }) + ); + } + + /** + * Check user credentials through LDAP - correct login and password supplied + */ + public function testCheckCredentialsFromLdapGoodLoginAndPassword() + { + $this->configManager->set('ldap.host', 'dummy'); + $this->assertTrue( + $this->loginManager->checkCredentialsFromLdap($this->login, $this->password, function() { return null; }, function() { return true; }) + ); + } } From 46846fd4fcc391f46f17037d69d0699567ae769e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20NOBILI?= Date: Mon, 2 Mar 2020 18:23:55 +0100 Subject: [PATCH 2/6] fixed typo --- tests/security/LoginManagerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/security/LoginManagerTest.php b/tests/security/LoginManagerTest.php index f2d78802..8fd1698c 100644 --- a/tests/security/LoginManagerTest.php +++ b/tests/security/LoginManagerTest.php @@ -314,7 +314,7 @@ public function testCheckCredentialsFromUnreachableLdap() */ public function testCheckCredentialsFromLdapWrongLoginAndPassword() { - $this->coddnfigManager->set('ldap.host', 'dummy'); + $this->configManager->set('ldap.host', 'dummy'); $this->assertFalse( $this->loginManager->checkCredentialsFromLdap($this->login, $this->password, function() { return null; }, function() { return false; }) ); From 21e5df5ee8f302ab96d4ca46ac3070405dd9aafb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20NOBILI?= Date: Wed, 3 Jun 2020 10:34:32 +0200 Subject: [PATCH 3/6] Update application/security/LoginManager.php Co-authored-by: ArthurHoaro --- application/security/LoginManager.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/application/security/LoginManager.php b/application/security/LoginManager.php index 2cea3f10..16ef3878 100644 --- a/application/security/LoginManager.php +++ b/application/security/LoginManager.php @@ -147,8 +147,10 @@ public function checkCredentials($remoteIp, $clientIpId, $login, $password) // Check credentials try { - if (($this->configManager->get('ldap.host') != "" && $this->checkCredentialsFromLdap($login, $password)) - || ($this->configManager->get('ldap.host') == "" && $this->checkCredentialsFromLocalConfig($login, $password))) { + $useLdapLogin = !empty($this->configManager->get('ldap.host')); + if ((false === $useLdapLogin && $this->checkCredentialsFromLocalConfig($login, $password)) + || (true === $useLdapLogin && $this->checkCredentialsFromLdap($login, $password)) + ) { $this->sessionManager->storeLoginInfo($clientIpId); logm( $this->configManager->get('resource.log'), From 9ba6982ea312c49a5c2b2cc5ad09d350e46fd87f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20NOBILI?= Date: Wed, 3 Jun 2020 10:35:41 +0200 Subject: [PATCH 4/6] Update application/security/LoginManager.php Co-authored-by: ArthurHoaro --- application/security/LoginManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/application/security/LoginManager.php b/application/security/LoginManager.php index 16ef3878..e34e0efa 100644 --- a/application/security/LoginManager.php +++ b/application/security/LoginManager.php @@ -141,7 +141,7 @@ public function isLoggedIn() public function checkCredentials($remoteIp, $clientIpId, $login, $password) { // Check login matches config - if ($login != $this->configManager->get('credentials.login')) { + if ($login !== $this->configManager->get('credentials.login')) { return false; } From a69cfe0dd23fbd2e25c07ec92717998585a9560d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20NOBILI?= Date: Wed, 3 Jun 2020 10:36:04 +0200 Subject: [PATCH 5/6] Update application/security/LoginManager.php Co-authored-by: ArthurHoaro --- application/security/LoginManager.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/application/security/LoginManager.php b/application/security/LoginManager.php index e34e0efa..5f395a87 100644 --- a/application/security/LoginManager.php +++ b/application/security/LoginManager.php @@ -206,7 +206,12 @@ public function checkCredentialsFromLdap($login, $password, $connect = null, $bi { $connect = $connect ?? function($host) { return ldap_connect($host); }; $bind = $bind ?? function($handle, $dn, $password) { return ldap_bind($handle, $dn, $password); }; - return $bind($connect($this->configManager->get('ldap.host')), sprintf($this->configManager->get('ldap.dn'), $login), $password); + + return $bind( + $connect($this->configManager->get('ldap.host')), + sprintf($this->configManager->get('ldap.dn'), $login), + $password + ); } /** From 8694e8411b19d499ff58d8168fba448c63a5e443 Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Thu, 25 Jun 2020 16:18:25 +0200 Subject: [PATCH 6/6] LDAP - Force protocol LDAPv3 On Linux, php-ldap seems to rely on a library which still uses deprecated LDAPv2 as default version, causing authentication issues. See: https://stackoverflow.com/a/48238224/1484919 --- application/security/LoginManager.php | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/application/security/LoginManager.php b/application/security/LoginManager.php index 5f395a87..39ec9b2e 100644 --- a/application/security/LoginManager.php +++ b/application/security/LoginManager.php @@ -204,12 +204,20 @@ public function checkCredentialsFromLocalConfig($login, $password) { */ public function checkCredentialsFromLdap($login, $password, $connect = null, $bind = null) { - $connect = $connect ?? function($host) { return ldap_connect($host); }; - $bind = $bind ?? function($handle, $dn, $password) { return ldap_bind($handle, $dn, $password); }; + $connect = $connect ?? function($host) { + $resource = ldap_connect($host); + + ldap_set_option($resource, LDAP_OPT_PROTOCOL_VERSION, 3); + + return $resource; + }; + $bind = $bind ?? function($handle, $dn, $password) { + return ldap_bind($handle, $dn, $password); + }; return $bind( $connect($this->configManager->get('ldap.host')), - sprintf($this->configManager->get('ldap.dn'), $login), + sprintf($this->configManager->get('ldap.dn'), $login), $password ); }