From: ArthurHoaro <arthur@hoa.ro>
Date: Thu, 25 Jun 2020 14:53:18 +0000 (+0200)
Subject: Merge pull request #1428 from pipoprods/feat/ldap-auth
X-Git-Tag: v0.12.0-beta~7
X-Git-Url: https://git.immae.eu/?a=commitdiff_plain;h=78c2f122e067f8bab62deb7ef758708721f4a9ba;hp=e1231265bc46b070a4edd573c417aa030fe83426;p=github%2Fshaarli%2FShaarli.git

Merge pull request #1428 from pipoprods/feat/ldap-auth
---

diff --git a/application/security/LoginManager.php b/application/security/LoginManager.php
index 0b0ce0b1..39ec9b2e 100644
--- a/application/security/LoginManager.php
+++ b/application/security/LoginManager.php
@@ -1,6 +1,7 @@
 <?php
 namespace Shaarli\Security;
 
+use Exception;
 use Shaarli\Config\ConfigManager;
 
 /**
@@ -139,26 +140,86 @@ class LoginManager
      */
     public function checkCredentials($remoteIp, $clientIpId, $login, $password)
     {
-        $hash = sha1($password . $login . $this->configManager->get('credentials.salt'));
+        // Check login matches config
+        if ($login !== $this->configManager->get('credentials.login')) {
+            return false;
+        }
 
-        if ($login != $this->configManager->get('credentials.login')
-            || $hash != $this->configManager->get('credentials.hash')
-        ) {
+        // Check credentials
+        try {
+            $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'),
+                        $remoteIp,
+                        'Login successful'
+                    );
+                    return true;
+            }
+        }
+        catch(Exception $exception) {
             logm(
                 $this->configManager->get('resource.log'),
                 $remoteIp,
-                'Login failed for user ' . $login
+                'Exception while checking credentials: ' . $exception
             );
-            return false;
         }
 
-        $this->sessionManager->storeLoginInfo($clientIpId);
         logm(
             $this->configManager->get('resource.log'),
             $remoteIp,
-            'Login successful'
+            'Login failed for user ' . $login
+        );
+        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) {
+            $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),
+            $password
         );
-        return true;
     }
 
     /**
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..8fd1698c 100644
--- a/tests/security/LoginManagerTest.php
+++ b/tests/security/LoginManagerTest.php
@@ -78,6 +78,7 @@ class LoginManagerTest extends TestCase
             'security.ban_after' => 2,
             'security.ban_duration' => 3600,
             'security.trusted_proxies' => [$this->trustedProxy],
+            'ldap.host' => '',
         ]);
 
         $this->cookie = [];
@@ -296,4 +297,37 @@ class LoginManagerTest extends TestCase
             $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->configManager->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; })
+        );
+    }
 }