Introduce an Ldap factory in the authentication plugin

plugins/authentication/ldap/src/Extension/Ldap.php

@@ -13,11 +13,12 @@
 use Joomla\CMS\Authentication\Authentication;
 use Joomla\CMS\Log\Log;
 use Joomla\CMS\Plugin\CMSPlugin;
-use Joomla\Database\DatabaseAwareTrait;
+use Joomla\Event\Dispatcher;
+use Joomla\Plugin\Authentication\Ldap\Factory\LdapFactoryInterface;
 use Symfony\Component\Ldap\Entry;
 use Symfony\Component\Ldap\Exception\ConnectionException;
 use Symfony\Component\Ldap\Exception\LdapException;
-use Symfony\Component\Ldap\Ldap as LdapProvider;
+use Symfony\Component\Ldap\LdapInterface;
 
 // phpcs:disable PSR1.Files.SideEffects
 \defined('_JEXEC') or die;
@@ -30,7 +31,31 @@
  */
 final class Ldap extends CMSPlugin
 {
-    use DatabaseAwareTrait;
+    /**
+     * The ldap factory
+     *
+     * @var    LdapFactoryInterface
+     * @since  __DEPLOY_VERSION__
+     */
+    private $factory;
+
+    /**
+     * Constructor
+     *
+     * @param   LdapFactoryInterface  $factory     The Ldap factory
+     * @param   DispatcherInterface   $dispatcher  The object to observe
+     * @param   array                 $config      An optional associative array of configuration settings.
+     *                                             Recognized key values include 'name', 'group', 'params', 'language'
+     *                                             (this list is not meant to be comprehensive).
+     *
+     * @since   __DEPLOY_VERSION__
+     */
+    public function __construct(LdapFactoryInterface $factory, Dispatcher $dispatcher, $config = [])
+    {
+        parent::__construct($dispatcher, $config);
+
+        $this->factory = $factory;
+    }
 
     /**
      * This method should handle any authentication and report back to the subject
@@ -46,12 +71,12 @@ final class Ldap extends CMSPlugin
     public function onUserAuthenticate($credentials, $options, &$response)
     {
         // If LDAP not correctly configured then bail early.
-        if (!$this->params->get('host')) {
+        if (!$this->params->get('host', '')) {
             return false;
         }
 
         // For JLog
-        $logcategory = "ldap";
+        $logcategory = 'ldap';
         $response->type = $logcategory;
 
         // Strip null bytes from the password
@@ -66,22 +91,22 @@ public function onUserAuthenticate($credentials, $options, &$response)
         }
 
         // Load plugin params info
-        $ldap_email    = $this->params->get('ldap_email');
-        $ldap_fullname = $this->params->get('ldap_fullname');
-        $ldap_uid      = $this->params->get('ldap_uid');
-        $auth_method   = $this->params->get('auth_method');
+        $ldap_email    = $this->params->get('ldap_email', '');
+        $ldap_fullname = $this->params->get('ldap_fullname', '');
+        $ldap_uid      = $this->params->get('ldap_uid', '');
+        $auth_method   = $this->params->get('auth_method', '');
 
         $options = [
-                'host'       => $this->params->get('host'),
-                'port'       => (int) $this->params->get('port'),
-                'version'    => $this->params->get('use_ldapV3', '0') == '1' ? 3 : 2,
-                'referrals'  => (bool) $this->params->get('no_referrals', '0'),
-                'encryption' => $this->params->get('negotiate_tls', '0') == '1' ? 'tls' : 'none',
-            ];
+            'host'       => $this->params->get('host', ''),
+            'port'       => (int) $this->params->get('port', ''),
+            'version'    => $this->params->get('use_ldapV3', '0') == '1' ? 3 : 2,
+            'referrals'  => (bool) $this->params->get('no_referrals', '0'),
+            'encryption' => $this->params->get('negotiate_tls', '0') == '1' ? 'tls' : 'none',
+        ];
         Log::add(sprintf('Creating LDAP session with options: %s', json_encode($options)), Log::DEBUG, $logcategory);
         $connection_string = sprintf('ldap%s://%s:%s', 'ssl' === $options['encryption'] ? 's' : '', $options['host'], $options['port']);
         Log::add(sprintf('Creating LDAP session to connect to "%s" while binding', $connection_string), Log::DEBUG, $logcategory);
-        $ldap = LdapProvider::create('ext_ldap', $options);
+        $ldap = $this->factory->createLdap($options);
 
         switch ($auth_method) {
             case 'search':
@@ -102,13 +127,10 @@ public function onUserAuthenticate($credentials, $options, &$response)
                     $searchstring = str_replace(
                         '[search]',
                         str_replace(';', '\3b', $ldap->escape($credentials['username'], '', LDAP_ESCAPE_FILTER)),
-                        $this->params->get('search_string')
+                        $this->params->get('search_string', '')
                     );
                     Log::add(sprintf('Searching LDAP entry with filter: "%s"', $searchstring), Log::DEBUG, $logcategory);
-                    $entry = $this->searchByString(
-                        $searchstring,
-                        $ldap
-                    );
+                    $entry = $this->searchByString($searchstring, $ldap);
                 } catch (LdapException $exception) {
                     $response->status = Authentication::STATUS_FAILURE;
                     $response->error_message = $this->getApplication()->getLanguage()->_('JGLOBAL_AUTH_UNKNOWN_ACCESS_DENIED');
@@ -169,13 +191,10 @@ public function onUserAuthenticate($credentials, $options, &$response)
                     $searchstring = str_replace(
                         '[search]',
                         str_replace(';', '\3b', $ldap->escape($credentials['username'], '', LDAP_ESCAPE_FILTER)),
-                        $this->params->get('search_string')
+                        $this->params->get('search_string', '')
                     );
                     Log::add(sprintf('Searching LDAP entry with filter: "%s"', $searchstring), Log::DEBUG, $logcategory);
-                    $entry = $this->searchByString(
-                        $searchstring,
-                        $ldap
-                    );
+                    $entry = $this->searchByString($searchstring, $ldap);
                 } catch (LdapException $exception) {
                     $response->status = Authentication::STATUS_FAILURE;
                     $response->error_message = $this->getApplication()->getLanguage()->_('JGLOBAL_AUTH_UNKNOWN_ACCESS_DENIED');
@@ -184,6 +203,17 @@ public function onUserAuthenticate($credentials, $options, &$response)
                     return;
                 }
 
+                if (!$entry) {
+                    // we did not find the login in LDAP
+                    $response->status = Authentication::STATUS_FAILURE;
+                    $response->error_message = $this->getApplication()->getLanguage()->_('JGLOBAL_AUTH_NO_USER');
+                    Log::add($this->getApplication()->getLanguage()->_('JGLOBAL_AUTH_USER_NOT_FOUND'), Log::ERROR, $logcategory);
+
+                    return;
+                } else {
+                    Log::add(sprintf('LDAP entry found at "%s"', $entry->getDn()), Log::DEBUG, $logcategory);
+                }
+
                 break;
 
             default:
@@ -198,7 +228,7 @@ public function onUserAuthenticate($credentials, $options, &$response)
         // Grab some details from LDAP and return them
         $response->username = $entry->getAttribute($ldap_uid)[0] ?? false;
         $response->email    = $entry->getAttribute($ldap_email)[0] ?? false;
-        $response->fullname = $entry->getAttribute($ldap_fullname)[0] ?? trim($entry->getAttribute($ldap_fullname)[0]) ?: $credentials['username'];
+        $response->fullname = $entry->getAttribute($ldap_fullname)[0] ?? $credentials['username'];
 
         // Were good - So say so.
         Log::add(sprintf('LDAP login succeeded; username: "%s", email: "%s", fullname: "%s"', $response->username, $response->email, $response->fullname), Log::DEBUG, $logcategory);
@@ -215,16 +245,16 @@ public function onUserAuthenticate($credentials, $options, &$response)
      * Note that this method requires that semicolons which should be part of the search term to be escaped
      * to correctly split the search string into separate lookups
      *
-     * @param   string        $search  search string of search values
-     * @param   LdapProvider  $ldap    The LDAP client
+     * @param   string         $search  search string of search values
+     * @param   LdapInterface  $ldap    The LDAP client
      *
      * @return  Entry|null The search result entry if a matching record was found
      *
      * @since   3.8.2
      */
-    private function searchByString($search, LdapProvider $ldap)
+    private function searchByString(string $search, LdapInterface $ldap)
     {
-        $dn = $this->params->get('base_dn');
+        $dn = $this->params->get('base_dn', '');
 
         // We return the first entry from the first search result which contains data
         foreach (explode(';', $search) as $key => $result) {

plugins/authentication/ldap/src/Factory/LdapFactory.php

@@ -0,0 +1,41 @@
+<?php
+
+/**
+ * Joomla! Content Management System
+ *
+ * @copyright  (C) 2023 Open Source Matters, Inc. <https://www.joomla.org>
+ * @license    GNU General Public License version 2 or later; see LICENSE
+ */
+
+namespace Joomla\Plugin\Authentication\Ldap\Factory;
+
+use Symfony\Component\Ldap\Ldap;
+use Symfony\Component\Ldap\LdapInterface;
+
+// phpcs:disable PSR1.Files.SideEffects
+\defined('JPATH_PLATFORM') or die;
+// phpcs:enable PSR1.Files.SideEffects
+
+/**
+ * Factory to create Ldap clients.
+ *
+ * @since  __DEPLOY_VERSION__
+ */
+class LdapFactory implements LdapFactoryInterface
+{
+    /**
+     * Method to load and return an Ldap client.
+     *
+     * @param   array  $config  The configuration array for the ldap client
+     *
+     * @return  LdapInterface
+     *
+     * @since   __DEPLOY_VERSION__
+     *
+     * @throws  \Exception
+     */
+    public function createLdap(array $config): LdapInterface
+    {
+        return Ldap::create('ext_ldap', $config);
+    }
+}

plugins/authentication/ldap/src/Factory/LdapFactoryInterface.php

@@ -0,0 +1,36 @@
+<?php
+
+/**
+ * Joomla! Content Management System
+ *
+ * @copyright  (C) 2023 Open Source Matters, Inc. <https://www.joomla.org>
+ * @license    GNU General Public License version 2 or later; see LICENSE
+ */
+
+namespace Joomla\Plugin\Authentication\Ldap\Factory;
+
+use Symfony\Component\Ldap\LdapInterface;
+
+// phpcs:disable PSR1.Files.SideEffects
+\defined('JPATH_PLATFORM') or die;
+// phpcs:enable PSR1.Files.SideEffects
+
+/**
+ * Factory to create Ldap clients.
+ *
+ * @since  __DEPLOY_VERSION__
+ */
+interface LdapFactoryInterface
+{
+    /**
+     * Method to load and return an Ldap client.
+     *
+     * @param   array  $config  The configuration array for the ldap client
+     *
+     * @return  LdapInterface
+     *
+     * @since   __DEPLOY_VERSION__
+     * @throws  \Exception
+     */
+    public function createLdap(array $config): LdapInterface;
+}

tests/Integration/IntegrationTestCase.php

@@ -11,7 +11,7 @@
 namespace Joomla\Tests\Integration;
 
 /**
- * Base Integration Test case for common behaviour across integration tests
+ * Base Integration Test case for common behavior across integration tests
  *
  * @since   4.0.0
  */

tests/Integration/Plugin/Authentication/Ldap/LdapPluginTest.php

@@ -1,38 +1,39 @@
 <?php
 
 /**
- * @package     Joomla.UnitTest
+ * @package     Joomla.IntegrationTest
  * @subpackage  Authentication
  *
  * @copyright   (C) 2022 Open Source Matters, Inc. <https://www.joomla.org>
  * @license     GNU General Public License version 2 or later; see LICENSE.txt
  */
 
-namespace Joomla\Tests\Unit\Plugin\Authentication\Ldap;
+namespace Joomla\Tests\Integration\Plugin\Authentication\Ldap;
 
 use Joomla\CMS\Application\CMSApplicationInterface;
 use Joomla\CMS\Authentication\Authentication;
 use Joomla\CMS\Authentication\AuthenticationResponse;
 use Joomla\CMS\Language\Language;
 use Joomla\Event\Dispatcher;
 use Joomla\Plugin\Authentication\Ldap\Extension\Ldap as LdapPlugin;
-use Joomla\Tests\Unit\UnitTestCase;
+use Joomla\Plugin\Authentication\Ldap\Factory\LdapFactory;
+use Joomla\Tests\Integration\IntegrationTestCase;
 use Symfony\Component\Ldap\Ldap;
 
 /**
  * Test class for Ldap plugin
  *
- * @package     Joomla.UnitTest
+ * @package     Joomla.IntegrationTest
  * @subpackage  Ldap
  *
  * @testdox     The Ldap plugin
  *
  * @since       4.3.0
  */
-class LdapPluginTest extends UnitTestCase
+class LdapPluginTest extends IntegrationTestCase
 {
     public const LDAPPORT = JTEST_LDAP_PORT;
-    public const SSLPORT = JTEST_LDAP_PORT_SSL;
+    public const SSLPORT  = JTEST_LDAP_PORT_SSL;
 
     /**
      * The default options
@@ -58,16 +59,14 @@ private function getPlugin($options): LdapPlugin
         $app = $this->createStub(CMSApplicationInterface::class);
         $app->method('getLanguage')->willReturn($language);
 
-        $dispatcher = new Dispatcher();
-
         // plugin object: result from DB using PluginHelper::getPlugin
         $pluginObject = [
             'name'   => 'ldap',
             'params' => json_encode($options),
             'type'   => 'authentication'
         ];
 
-        $plugin = new LdapPlugin($dispatcher, $pluginObject);
+        $plugin = new LdapPlugin(new LdapFactory(), new Dispatcher(), $pluginObject);
         $plugin->setApplication($app);
 
         return $plugin;