Merge pull request #12 from simplesamlphp/ldapv2

Migrate to v2 of the ldap-module

.github/workflows/documentation.yml

@@ -4,11 +4,11 @@ name: Documentation
 
 on:  # yamllint disable-line rule:truthy
   push:
-    branches: [ master, simplesamlphp-* ]
+    branches: [master, simplesamlphp-*]
     paths:
       - '**.md'
   pull_request:
-    branches: [ master, simplesamlphp-* ]
+    branches: [master, simplesamlphp-*]
     paths:
       - '**.md'
   workflow_dispatch:

.github/workflows/php.yml

@@ -55,7 +55,7 @@ jobs:
           php-version: '8.3'
           tools: composer, composer-require-checker, composer-unused, phpcs, psalm
           # optional performance gain for psalm: opcache
-          extensions: ctype, date, dom, fileinfo, filter, hash, intl, mbstring, opcache, openssl, pcre, spl, xml
+          extensions: ctype, date, dom, fileinfo, filter, hash, intl, ldap, mbstring, opcache, openssl, pcre, spl, xml
 
       - name: Setup problem matchers for PHP
         run: echo "::add-matcher::${{ runner.tool_cache }}/php.json"
@@ -119,7 +119,7 @@ jobs:
         with:
           # Should be the lowest supported version
           php-version: '8.1'
-          extensions: ctype, date, dom, fileinfo, filter, hash, intl, mbstring, openssl, pcre, spl, xml
+          extensions: ctype, date, dom, fileinfo, filter, hash, intl, ldap, mbstring, openssl, pcre, spl, xml
           tools: composer
           coverage: none
 
@@ -166,7 +166,7 @@ jobs:
         uses: shivammathur/setup-php@v2
         with:
           php-version: ${{ matrix.php-versions }}
-          extensions: ctype, date, dom, fileinfo, filter, hash, intl, mbstring, openssl, pcre, spl, xml
+          extensions: ctype, date, dom, fileinfo, filter, hash, intl, ldap, mbstring, openssl, pcre, spl, xml
           tools: composer
           ini-values: error_reporting=E_ALL
           coverage: pcov
@@ -228,7 +228,7 @@ jobs:
         uses: shivammathur/setup-php@v2
         with:
           php-version: ${{ matrix.php-versions }}
-          extensions: ctype, date, dom, fileinfo, filter, hash, intl, mbstring, openssl, pcre, spl, xml
+          extensions: ctype, date, dom, fileinfo, filter, hash, intl, ldap, mbstring, openssl, pcre, spl, xml
           tools: composer
           ini-values: error_reporting=E_ALL
           coverage: none
@@ -287,8 +287,8 @@ jobs:
     runs-on: [ubuntu-latest]
     if: |
       always() &&
-      needs.coverage.result == 'success' &&
-      (needs.unit-tests-linux == 'success' || needs.coverage == 'skipped')
+      needs.coverage.result == 'success' ||
+      (needs.unit-tests-linux == 'success' && needs.coverage == 'skipped')
 
     steps:
       - uses: geekyeggo/delete-artifact@v5

composer.json

@@ -45,12 +45,14 @@
 
         "simplesamlphp/assert": "^1.0",
         "simplesamlphp/composer-module-installer": "^1.3.2",
-        "simplesamlphp/simplesamlphp": "^2.1",
-        "simplesamlphp/simplesamlphp-module-ldap": "^1.2",
-        "symfony/http-foundation": "^6.4"
+        "simplesamlphp/simplesamlphp": "^2.2",
+        "simplesamlphp/simplesamlphp-module-ldap": "^2.2",
+        "symfony/http-foundation": "^6.4",
+        "symfony/ldap": "^6.4",
+        "symfony/security-core": "^6.4"
     },
     "require-dev": {
-        "simplesamlphp/simplesamlphp-test-framework": "^1.5.5"
+        "simplesamlphp/simplesamlphp-test-framework": "^1.6.0"
     },
     "extra": {
         "ssp-mixedcase-module-name": "authX509"

psalm-dev.xml

@@ -0,0 +1,27 @@
+<?xml version="1.0"?>
+<psalm
+    name="SimpleSAMLphp testsuite"
+    useDocblockTypes="true"
+    errorLevel="4"
+    reportMixedIssues="false"
+    hideExternalErrors="true"
+    allowStringToStandInForClass="true"
+>
+    <projectFiles>
+        <directory name="tests" />
+
+        <!-- Ignore certain directories -->
+        <ignoreFiles>
+            <directory name="vendor" />
+        </ignoreFiles>
+    </projectFiles>
+
+    <issueHandlers>
+        <!-- Ignore UnresolvableInclude on CLI-scripts -->
+        <UnresolvableInclude>
+            <errorLevel type="suppress">
+                <file name="tests/bootstrap.php" />
+            </errorLevel>
+        </UnresolvableInclude>
+    </issueHandlers>
+</psalm>

src/Auth/Source/X509userCert.php

@@ -5,13 +5,27 @@
 namespace SimpleSAML\Module\authX509\Auth\Source;
 
 use Exception;
+use SimpleSAML\Assert\Assert;
 use SimpleSAML\Auth;
 use SimpleSAML\Configuration;
 use SimpleSAML\Error;
 use SimpleSAML\Logger;
-use SimpleSAML\Module\ldap\ConfigHelper;
+use SimpleSAML\Module\ldap\ConnectorFactory;
+use SimpleSAML\Module\ldap\ConnectorInterface;
 use SimpleSAML\Utils;
 use SimpleSAML\XHTML\Template;
+use Symfony\Component\Ldap\Entry;
+use Symfony\Component\Ldap\Ldap;
+use Symfony\Component\Ldap\Security\LdapUserProvider;
+use Symfony\Component\Security\Core\Exception\UserNotFoundException;
+
+use function array_fill_keys;
+use function array_key_exists;
+use function array_merge;
+use function array_values;
+use function current;
+use function openssl_x509_parse;
+use function sprintf;
 
 /**
  * This class implements x509 certificate authentication with certificate validation against an LDAP directory.
@@ -21,18 +35,26 @@
 
 class X509userCert extends Auth\Source
 {
+    /** @var \SimpleSAML\Module\ldap\ConnectorInterface */
+    protected ConnectorInterface $connector;
+
     /**
-     * x509 attributes to use from the certificate for searching the user in the LDAP directory.
-     * @var array<string, string>
+     * The ldap-authsource to use
+     * @var string
      */
-    private array $x509attributes = ['UID' => 'uid'];
+    private string $backend;
 
     /**
-     * A pattern from configuration to construct a ldap dn from a username
-     * @var string|null
+     * The ldap-authsource config to use
+     * @var \SimpleSAML\Configuration
      */
-    private ?string $dnpattern;
+    private Configuration $ldapConfig;
 
+    /**
+     * x509 attributes to use from the certificate for searching the user in the LDAP directory.
+     * @var array<string, string>
+     */
+    private array $x509attributes = ['UID' => 'uid'];
 
     /**
      * LDAP attribute containing the user certificate.
@@ -42,12 +64,6 @@ class X509userCert extends Auth\Source
     private ?array $ldapusercert = ['userCertificate;binary'];
 
 
-    /**
-     * @var \SimpleSAML\Module\ldap\ConfigHelper
-     */
-    private ConfigHelper $ldapcf;
-
-
     /**
      * Constructor for this authentication source.
      *
@@ -58,6 +74,8 @@ class X509userCert extends Auth\Source
      */
     public function __construct(array $info, array &$config)
     {
+        parent::__construct($info, $config);
+
         if (isset($config['authX509:x509attributes'])) {
             $this->x509attributes = $config['authX509:x509attributes'];
         }
@@ -66,16 +84,25 @@ public function __construct(array $info, array &$config)
             $this->ldapusercert = $config['authX509:ldapusercert'];
         }
 
-        if (isset($config['dnpattern'])) {
-            $this->dnpattern = $config['dnpattern'];
+        Assert::keyExists($config, 'backend');
+        $this->backend = $config['backend'];
+
+        // Get the authsources file, which should contain the backend-config
+        $authSources = Configuration::getConfig('authsources.php');
+
+        // Verify that the authsource config exists
+        if (!$authSources->hasValue($this->backend)) {
+            throw new Error\Exception(
+                sprintf('Authsource [%s] not found in authsources.php', $this->backend)
+            );
         }
 
-        parent::__construct($info, $config);
+        // Get just the specified authsource config values
+        $this->ldapConfig = $authSources->getConfigItem($this->backend);
+        $type = current($this->ldapConfig->toArray());
+        Assert::oneOf($type, ['ldap:Ldap']);
 
-        $this->ldapcf = new ConfigHelper(
-            $config,
-            'Authentication source ' . var_export($this->authId, true)
-        );
+        $this->connector = ConnectorFactory::fromAuthSource($this->backend);
     }
 
 
@@ -90,7 +117,7 @@ public function authFailed(&$state): void
     {
         $config = Configuration::getInstance();
         $errorcode = $state['authX509.error'];
-        $errorcodes = Error\ErrorCodes::getAllErrorCodeMessages();
+        $errorcodes = (new Error\ErrorCodes())->getAllMessages();
 
         $t = new Template($config, 'authX509:X509error.twig');
         $httpUtils = new Utils\HTTP();
@@ -120,8 +147,6 @@ public function authFailed(&$state): void
      */
     public function authenticate(array &$state): void
     {
-        $ldapcf = $this->ldapcf;
-
         if (
             !isset($_SERVER['SSL_CLIENT_CERT']) ||
             ($_SERVER['SSL_CLIENT_CERT'] == '')
@@ -142,25 +167,21 @@ public function authenticate(array &$state): void
             throw new Exception("Should never be reached");
         }
 
-        $dn = null;
-        foreach ($this->x509attributes as $x509_attr => $ldap_attr) {
+        $entry = $dn = null;
+        foreach ($this->x509attributes as $x509_attr => $attr) {
             // value is scalar
             if (array_key_exists($x509_attr, $client_cert_data['subject'])) {
                 $value = $client_cert_data['subject'][$x509_attr];
                 Logger::info('authX509: cert ' . $x509_attr . ' = ' . $value);
-
-                if (isset($this->dnpattern)) {
-                    $dn = str_replace('%username%', $value, $this->dnpattern);
-                } else {
-                    $dn = $ldapcf->searchfordn($ldap_attr, $value, true);
-                }
-                if ($dn !== null) {
+                $entry = $this->findUserByAttribute($attr, $value);
+                if ($entry !== null) {
+                    $dn = $attr;
                     break;
                 }
             }
         }
 
-        if ($dn === null) {
+        if ($entry === null) {
             Logger::error('authX509: cert has no matching user in LDAP.');
             $state['authX509.error'] = "UNKNOWNCERT";
             $this->authFailed($state);
@@ -170,15 +191,21 @@ public function authenticate(array &$state): void
 
         if ($this->ldapusercert === null) {
             // do not check for certificate match
-            $attributes = $ldapcf->getAttributes($dn);
+            $attributes = array_intersect_key(
+                $entry->getAttributes(),
+                array_fill_keys(array_values($this->x509attributes), null),
+            );
 
             $state['Attributes'] = $attributes;
             $this->authSuccesful($state);
 
             throw new Exception("Should never be reached");
         }
 
-        $ldap_certs = $ldapcf->getAttributes($dn, $this->ldapusercert);
+        $ldap_certs = [];
+        foreach ($this->ldapusercert as $attr) {
+            $ldap_certs[$attr] = $entry->getAttribute($attr);
+        }
 
         if (empty($ldap_certs)) {
             Logger::error('authX509: no certificate found in LDAP for dn=' . $dn);
@@ -205,7 +232,10 @@ public function authenticate(array &$state): void
             }
 
             if ($ldap_cert_data === $client_cert_data) {
-                $attributes = $ldapcf->getAttributes($dn);
+                $attributes = array_intersect_key(
+                    $entry->getAttributes(),
+                    array_fill_keys(array_values($this->x509attributes), null)
+                );
                 $state['Attributes'] = $attributes;
                 $this->authSuccesful($state);
 
@@ -234,4 +264,38 @@ public function authSuccesful(array &$state): void
 
         throw new Exception("Should never be reached");
     }
+
+
+    /**
+     * Find user in LDAP-store
+     *
+     * @param string $attr
+     * @param string $value
+     * @return \Symfony\Component\Ldap\Entry|null
+     */
+    public function findUserByAttribute(string $attr, string $value): ?Entry
+    {
+        $searchBase = $this->ldapConfig->getArray('search.base');
+
+        $searchUsername = $this->ldapConfig->getString('search.username');
+        Assert::notWhitespaceOnly($searchUsername);
+
+        $searchPassword = $this->ldapConfig->getOptionalString('search.password', null);
+        Assert::nullOrnotWhitespaceOnly($searchPassword);
+
+        $ldap = ConnectorFactory::fromAuthSource($this->backend);
+        $connection = new Ldap($ldap->getAdapter());
+
+        foreach ($searchBase as $base) {
+            $ldapUserProvider = new LdapUserProvider($connection, $base, $searchUsername, $searchPassword, [], $attr);
+            try {
+                return $ldapUserProvider->loadUserByIdentifier($value)->getEntry();
+            } catch (UserNotFoundException $e) {
+                continue;
+            }
+        }
+
+        // We haven't found the user
+        return null;
+    }
 }

src/Controller/ExpiryWarning.php

@@ -89,7 +89,7 @@ public function main(Request $request): Response
         $t->data['data'] = ['StateId' => $id];
         $t->data['daysleft'] = $state['daysleft'];
         $t->data['renewurl'] = $state['renewurl'];
-        $t->data['errorcodes'] = Error\ErrorCodes::getAllErrorCodeMessages();
+        $t->data['errorcodes'] = (new Error\ErrorCodes())->getAllMessages();
         return $t;
     }
 }

tools/composer-require-checker.json

@@ -1,5 +1,7 @@
 {
   "symbol-whitelist": [
-    "SimpleSAML\\Module\\ldap\\ConfigHelper"
+    "SimpleSAML\\Module\\ldap\\ConfigHelper",
+    "SimpleSAML\\Module\\ldap\\ConnectorFactory",
+    "SimpleSAML\\Module\\ldap\\ConnectorInterface"
   ]
 }