Migrate to v2 of the ldap-module #12

tvdijen · 2024-03-25T07:22:58Z

** Untested **

I need someone to verify this for me, because I'm unable to get certificate authentication to work in my sandbox.

kiokuj · 2024-04-07T10:36:22Z

src/Auth/Source/X509userCert.php

+     */
+    public function findUserByAttribute(string $attr, string $value): ?Entry
+    {
+        $searchBase = $this->ldapConfig->getString('search.base');


Using getString here results in error: "The option 'search.base' is not a valid string value."

That's because option search.base for the LDAP module has changed from string to array.

The right method to call should be getArray; however, that creates more troubles on line 283 (see next comment).

kiokuj · 2024-04-07T10:40:16Z

src/Auth/Source/X509userCert.php

+        Assert::nullOrnotWhitespaceOnly($searchPassword);
+
+        $ldap = ConnectorFactory::fromAuthSource($this->backend);
+        $ldapUserProvider = new LdapUserProvider($ldap, $searchBase, $searchUsername, $searchPassword, [], $attr);


Option search.base should be retrieved with getArray. That makes $searchBase an array of strings, while LdapUserProvider expects $searchBase to be a string.

A quick and dirty workaround, just for the sake of further testing, would be to pass $searchBase[0] (other base DNs to be searched are temporarily ignored).

kiokuj · 2024-04-07T11:13:14Z

I'm trying to setup this module on latest SimpleSAMLphp version (2.2.1) but I'm experiencing issues.

Modules installed:
simplesamlphp-module-ldap (instead of simplesamlphp-module-ldap:dev-master as per README)
simplesamlphp-module-authx509:dev-ldapv2

Code fixes:
authsources.php: in custom x509 source, add option 'backend' => 'ldap' (required by X509userCert constructor)
X509userCert.php: see comments above (line 274: getArray instead of getString; line 283: $searchBase[0] instead of $searchBase)

Issues found:
On X509 authentication, when calling LdapUserProvider (X509userCert.php:283):

[5f2c24a6c8] SimpleSAML\Error\Error: UNHANDLEDEXCEPTION
[5f2c24a6c8] Backtrace:
[5f2c24a6c8] 2 ./src/SimpleSAML/Error/ExceptionHandler.php:35 (SimpleSAML\Error\ExceptionHandler::customExceptionHandler)
[5f2c24a6c8] 1 ./vendor/symfony/error-handler/ErrorHandler.php:535 (Symfony\Component\ErrorHandler\ErrorHandler::handleException)
[5f2c24a6c8] 0 [builtin] (N/A)
[5f2c24a6c8] Caused by: Symfony\Component\ErrorHandler\Error\ClassNotFoundError: Attempted to load interface "UserProviderInterface" from namespace "Symfony\Component\Security\Core\User". Did you forget a "use" statement for another namespace?
[5f2c24a6c8] Backtrace:
[5f2c24a6c8] 10 ./vendor/symfony/ldap/Security/LdapUserProvider.php:36 (include)
[5f2c24a6c8] 9 ./vendor/composer/ClassLoader.php:576 (Composer\Autoload\{closure})
[5f2c24a6c8] 8 ./vendor/composer/ClassLoader.php:427 (Composer\Autoload\ClassLoader::loadClass)
[5f2c24a6c8] 7 ./modules/authX509/src/Auth/Source/X509userCert.php:283 (SimpleSAML\Module\authX509\Auth\Source\X509userCert::findUserByAttribute)
[5f2c24a6c8] 6 ./modules/authX509/src/Auth/Source/X509userCert.php:176 (SimpleSAML\Module\authX509\Auth\Source\X509userCert::authenticate)
[5f2c24a6c8] 5 ./src/SimpleSAML/Auth/Source.php:193 (SimpleSAML\Auth\Source::initLogin)
[5f2c24a6c8] 4 ./src/SimpleSAML/Auth/Simple.php:165 (SimpleSAML\Auth\Simple::login)
[5f2c24a6c8] 3 [builtin] (call_user_func_array)
[5f2c24a6c8] 2 ./src/SimpleSAML/HTTP/RunnableResponse.php:68 (SimpleSAML\HTTP\RunnableResponse::sendContent)
[5f2c24a6c8] 1 ./vendor/symfony/http-foundation/Response.php:423 (Symfony\Component\HttpFoundation\Response::send)
[5f2c24a6c8] 0 ./public/module.php:24 (N/A)
[5f2c24a6c8] Error report with id 7ca8d0a8 generated.

codecov · 2024-04-07T14:29:02Z

Codecov Report

Attention: Patch coverage is 2.38095% with 41 lines in your changes are missing coverage. Please review.

Project coverage is 12.04%. Comparing base (295442a) to head (f4805e0).
Report is 28 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master      #12      +/-   ##
============================================
- Coverage     14.86%   12.04%   -2.82%     
- Complexity       40       43       +3     
============================================
  Files             3        3              
  Lines           148      166      +18     
============================================
- Hits             22       20       -2     
- Misses          126      146      +20

tvdijen · 2024-04-07T14:35:49Z

Can you give it another try @kiokuj ?

kiokuj · 2024-04-07T15:05:13Z

I confirm that the fix for search.base now works out of the box, thank you.

There's still the main issue I reported, which boils down to this exception at X509userCert.php:284, when LdapUserProvider is instantiated:
Caused by: Symfony\Component\ErrorHandler\Error\ClassNotFoundError: Attempted to load interface "UserProviderInterface" from namespace "Symfony\Component\Security\Core\User". Did you forget a "use" statement for another namespace?

Here's the full error log:

[9fd090fc9e] SimpleSAML\Error\Error: UNHANDLEDEXCEPTION
[9fd090fc9e] Backtrace:
[9fd090fc9e] 2 ./src/SimpleSAML/Error/ExceptionHandler.php:35 (SimpleSAML\Error\ExceptionHandler::customExceptionHandler)
[9fd090fc9e] 1 ./vendor/symfony/error-handler/ErrorHandler.php:535 (Symfony\Component\ErrorHandler\ErrorHandler::handleException)
[9fd090fc9e] 0 [builtin] (N/A)
[9fd090fc9e] Caused by: Symfony\Component\ErrorHandler\Error\ClassNotFoundError: Attempted to load interface "UserProviderInterface" from namespace "Symfony\Component\Security\Core\User". Did you forget a "use" statement for another namespace?
[9fd090fc9e] Backtrace:
[9fd090fc9e] 10 ./vendor/symfony/ldap/Security/LdapUserProvider.php:36 (include)
[9fd090fc9e] 9 ./vendor/composer/ClassLoader.php:576 (Composer\Autoload\{closure})
[9fd090fc9e] 8 ./vendor/composer/ClassLoader.php:427 (Composer\Autoload\ClassLoader::loadClass)
[9fd090fc9e] 7 ./modules/authX509/src/Auth/Source/X509userCert.php:284 (SimpleSAML\Module\authX509\Auth\Source\X509userCert::findUserByAttribute)
[9fd090fc9e] 6 ./modules/authX509/src/Auth/Source/X509userCert.php:175 (SimpleSAML\Module\authX509\Auth\Source\X509userCert::authenticate)
[9fd090fc9e] 5 ./src/SimpleSAML/Auth/Source.php:193 (SimpleSAML\Auth\Source::initLogin)
[9fd090fc9e] 4 ./src/SimpleSAML/Auth/Simple.php:165 (SimpleSAML\Auth\Simple::login)
[9fd090fc9e] 3 [builtin] (call_user_func_array)
[9fd090fc9e] 2 ./src/SimpleSAML/HTTP/RunnableResponse.php:68 (SimpleSAML\HTTP\RunnableResponse::sendContent)
[9fd090fc9e] 1 ./vendor/symfony/http-foundation/Response.php:423 (Symfony\Component\HttpFoundation\Response::send)
[9fd090fc9e] 0 ./public/module.php:24 (N/A)

tvdijen · 2024-04-07T15:18:30Z

That one should also be fixed now!

kiokuj · 2024-04-07T15:36:42Z

Thanks! Definitely some progress (no more Symfony errors), but I'm still testing right now.

I'm getting TypeError: array_merge(): Argument #2 must be of type array, null given at X509userCert.php:217.

I'll try and see if it's related to my authsource or LDAP configuration.

tvdijen · 2024-04-07T16:05:36Z

It would be good to know what the value of the $ldap_certs and $attr variables is, at that point.
We know it's not empty at that point

kiokuj · 2024-04-07T16:12:27Z

I agree, $ldap_certs isn't definitely empty, otherwise I'd have seen the UNKNOWNCERT error ("authX509: no certificate found in LDAP"), but that's not supposed to be raised.

Full debug log follows. Certificate attribute dnQualifier is mapped to uidNumber in authsource configuration, and the corresponding user seems to be found indeed in LDAP. I'll check the values of those variables and report back shortly.

[d3c478759d] Session: 'x509' not valid because we are not authenticated.
[d3c478759d] Setting up LDAP connection: host='ldap://***', encryption=tls, version=3, debug=false, timeout=60, referrals=0.
[d3c478759d] authX509: cert dnQualifier = 10000
[d3c478759d] Setting up LDAP connection: host='ldap://***', encryption=tls, version=3, debug=false, timeout=60, referrals=0.
[d3c478759d] SimpleSAML\Error\Exception: Warning - Undefined array key "userCertificate;binary" at ./modules/authX509/src/Auth/Source/X509userCert.php:217
[d3c478759d] Backtrace:
[d3c478759d] 7 ./src/SimpleSAML/Error/ErrorHandler.php:64 (SimpleSAML\Error\ErrorHandler::customErrorHandler)
[d3c478759d] 6 ./modules/authX509/src/Auth/Source/X509userCert.php:217 (SimpleSAML\Module\authX509\Auth\Source\X509userCert::authenticate)
[d3c478759d] 5 ./src/SimpleSAML/Auth/Source.php:193 (SimpleSAML\Auth\Source::initLogin)
[d3c478759d] 4 ./src/SimpleSAML/Auth/Simple.php:165 (SimpleSAML\Auth\Simple::login)
[d3c478759d] 3 [builtin] (call_user_func_array)
[d3c478759d] 2 ./src/SimpleSAML/HTTP/RunnableResponse.php:68 (SimpleSAML\HTTP\RunnableResponse::sendContent)
[d3c478759d] 1 ./vendor/symfony/http-foundation/Response.php:423 (Symfony\Component\HttpFoundation\Response::send)
[d3c478759d] 0 ./public/module.php:24 (N/A)
[d3c478759d] SimpleSAML\Error\Error: UNHANDLEDEXCEPTION
[d3c478759d] Backtrace:
[d3c478759d] 2 ./src/SimpleSAML/Error/ExceptionHandler.php:35 (SimpleSAML\Error\ExceptionHandler::customExceptionHandler)
[d3c478759d] 1 ./vendor/symfony/error-handler/ErrorHandler.php:535 (Symfony\Component\ErrorHandler\ErrorHandler::handleException)
[d3c478759d] 0 [builtin] (N/A)
[d3c478759d] Caused by: TypeError: array_merge(): Argument #2 must be of type array, null given
[d3c478759d] Backtrace:
[d3c478759d] 7 ./modules/authX509/src/Auth/Source/X509userCert.php:217 (array_merge)
[d3c478759d] 6 ./modules/authX509/src/Auth/Source/X509userCert.php:217 (SimpleSAML\Module\authX509\Auth\Source\X509userCert::authenticate)
[d3c478759d] 5 ./src/SimpleSAML/Auth/Source.php:193 (SimpleSAML\Auth\Source::initLogin)
[d3c478759d] 4 ./src/SimpleSAML/Auth/Simple.php:165 (SimpleSAML\Auth\Simple::login)
[d3c478759d] 3 [builtin] (call_user_func_array)
[d3c478759d] 2 ./src/SimpleSAML/HTTP/RunnableResponse.php:68 (SimpleSAML\HTTP\RunnableResponse::sendContent)
[d3c478759d] 1 ./vendor/symfony/http-foundation/Response.php:423 (Symfony\Component\HttpFoundation\Response::send)
[d3c478759d] 0 ./public/module.php:24 (N/A)

kiokuj · 2024-04-07T16:39:55Z

OK, I'm back with some results.

First of all, I believe there's a typo in X509userCert.php:205: the function call should be getAttributes (final 's'):
$ldap_certs = array_map([$entry, 'getAttributes'], $this->ldapusercert);

Moving on, $ldap_certs is actually a twofold array, that is every key-value pair (including userCertificate) is child of the same 0-indexed element. So the usual quick-and-dirty fix for X509userCert.php:217 is the following:
$merged_ldapcerts = array_merge($merged_ldapcerts, $ldap_certs[0][$attr]);

This seems to finally work (by the way, for my setup in authsources.php I had to set authX509:ldapusercert to userCertificate instead of the default userCertificate;binary).

tvdijen · 2024-04-07T17:11:32Z

What kind of LDAP-backend are you using? MS Active Directory?

kiokuj · 2024-04-07T17:28:54Z

Samba4 on Debian 12.

tvdijen · 2024-04-07T17:37:02Z

The getAttribute/getAttributes seems correct to me.. I only want to fetch the specific ldap-attributes that contain a certificate.. getAttributes would fetch all attributes.

The array access was fixed

kiokuj · 2024-04-07T18:04:28Z

You're right, getAttribute is more efficient. I was misled by the $attr key used in array_merge, so I thought (wrongly) that getAttributes was needed.

However, I noticed that the array returned by getAttribute is no longer associative. Therefore X509userCert.php:217 should become $merged_ldapcerts = array_merge($merged_ldapcerts, $ldap_certs[0]); and there's no need for $attr anymore.

tvdijen · 2024-04-07T18:27:47Z

That doesn't make sense, because that would make the foreach-statement useless..
Shouldn't it be $ldap_certs[$attr] then? The way it was.

kiokuj · 2024-04-07T19:14:49Z

Good point, but the array structure is different.

Array $ldap_certs returned by getAttributes in array_map (showing just the relevant item, you actually get all attributes):

Array
(
    [0] => Array
        (
            [userCertificate] => Array
                (
                    [0] => <binary_certificate>
                )
	)
)

Array $ldap_certs returned by getAttribute in array_map:

Array
(
    [0] => Array
        (
            [0] => <binary_certificate>
        )

)

As there is just one attribute, it seems that its name is not retained, and you just get its value(s). So there's apparently no place for $attr anymore.

I'm not sure what happens in the (rather exotic) case of multi-valued LDAP attributes (that is, if you have two or more certificate attributes, like userCertificate and otherUserCertificate, each of them with more than one binary value). The first structure is obviously strong enough to encode that scenario; I don't know what the second will become (not being an associative array).

tvdijen · 2024-04-07T19:45:13Z

Could you please test my latest fix?

kiokuj · 2024-04-07T20:14:44Z

It works, thank you!

tvdijen · 2024-04-07T20:24:09Z

Thanks a lot for your help! I've just released v2.0.0

kiokuj · 2024-04-07T20:41:26Z

It was my pleasure! But there's one more issue... X509 authentication is completed successfully, however when I test that method from the admin page I only get the certificate attribute, and not also the other attributes taken from LDAP. In authsources.php I used to add the 'attributes' array, but now it seems ignored.

Here's the relevant part of my authsources.php:

    'x509' => [
        'authX509:X509userCert',
        'backend' => 'ldap',
        'attributes' => ['sn', 'givenName', 'displayName', 'mail', 'cn', 'memberOf'],
        'authX509:x509attributes' => ['dnQualifier' => 'uidNumber'],
        'authX509:ldapusercert' => ['userCertificate'],
    ],

In addition, the documentation for the module should be updated...

tvdijen · 2024-04-07T21:08:59Z

Can you verify that this solves your issue?
cac692a

btw, the attributes-array should go in the ldap-authsource, not in the x509 source. That should indeed be fixed in the docs.

kiokuj · 2024-04-07T21:35:20Z

Yes, commit cac692a solves my issue.

Indeed, I already put the attributes-array in the ldap-authsource. The duplicate attributes-array in the x509-authsource was probably a leftover from the previous version of authsources.php.

tvdijen · 2024-04-07T21:41:48Z

Thanks again! Updated the docs and tagged v2.0.1

kiokuj · 2024-04-07T21:55:35Z

Glad to have helped. Keep up the good work!

tvdijen added 5 commits January 15, 2024 22:06

Bump ldap-module

c54a8b9

First rough migration to ldapv2

e65b92b

Bump nosborn

2582691

Lock conversations of closed issues/prs after 90 days of inactivity

188d628

Fix composer file

e9ba41f

tvdijen added enhancement New feature or request help wanted Extra attention is needed labels Mar 25, 2024

Merge branch 'master' into ldapv2

4421401

kiokuj reviewed Apr 7, 2024

View reviewed changes

Fix reported issues

876139b

tvdijen force-pushed the ldapv2 branch 4 times, most recently from b37852f to 0aaf849 Compare April 7, 2024 14:26

tvdijen force-pushed the ldapv2 branch from 0aaf849 to da55c7e Compare April 7, 2024 14:35

tvdijen force-pushed the ldapv2 branch from da55c7e to f0f9d10 Compare April 7, 2024 14:41

Fix unit tests

7e700e5

tvdijen force-pushed the ldapv2 branch from f0f9d10 to 7e700e5 Compare April 7, 2024 14:45

tvdijen added 2 commits April 7, 2024 17:05

Fix unit tests

a46ec9a

Fix deprecations

83b5198

Fix

c69ead1

Fix

f4805e0

tvdijen merged commit 4f83765 into master Apr 7, 2024
29 checks passed

tvdijen deleted the ldapv2 branch April 7, 2024 20:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to v2 of the ldap-module #12

Migrate to v2 of the ldap-module #12

tvdijen commented Mar 25, 2024

kiokuj Apr 7, 2024

kiokuj Apr 7, 2024

kiokuj commented Apr 7, 2024

codecov bot commented Apr 7, 2024 •

edited

Loading

tvdijen commented Apr 7, 2024

kiokuj commented Apr 7, 2024 •

edited

Loading

tvdijen commented Apr 7, 2024

kiokuj commented Apr 7, 2024

tvdijen commented Apr 7, 2024

kiokuj commented Apr 7, 2024

kiokuj commented Apr 7, 2024 •

edited

Loading

tvdijen commented Apr 7, 2024

kiokuj commented Apr 7, 2024

tvdijen commented Apr 7, 2024

kiokuj commented Apr 7, 2024

tvdijen commented Apr 7, 2024

kiokuj commented Apr 7, 2024 •

edited

Loading

tvdijen commented Apr 7, 2024

kiokuj commented Apr 7, 2024

tvdijen commented Apr 7, 2024

kiokuj commented Apr 7, 2024

tvdijen commented Apr 7, 2024 •

edited

Loading

kiokuj commented Apr 7, 2024

tvdijen commented Apr 7, 2024

kiokuj commented Apr 7, 2024

Migrate to v2 of the ldap-module #12

Migrate to v2 of the ldap-module #12

Conversation

tvdijen commented Mar 25, 2024

kiokuj Apr 7, 2024

Choose a reason for hiding this comment

kiokuj Apr 7, 2024

Choose a reason for hiding this comment

kiokuj commented Apr 7, 2024

codecov bot commented Apr 7, 2024 • edited Loading

Codecov Report

tvdijen commented Apr 7, 2024

kiokuj commented Apr 7, 2024 • edited Loading

tvdijen commented Apr 7, 2024

kiokuj commented Apr 7, 2024

tvdijen commented Apr 7, 2024

kiokuj commented Apr 7, 2024

kiokuj commented Apr 7, 2024 • edited Loading

tvdijen commented Apr 7, 2024

kiokuj commented Apr 7, 2024

tvdijen commented Apr 7, 2024

kiokuj commented Apr 7, 2024

tvdijen commented Apr 7, 2024

kiokuj commented Apr 7, 2024 • edited Loading

tvdijen commented Apr 7, 2024

kiokuj commented Apr 7, 2024

tvdijen commented Apr 7, 2024

kiokuj commented Apr 7, 2024

tvdijen commented Apr 7, 2024 • edited Loading

kiokuj commented Apr 7, 2024

tvdijen commented Apr 7, 2024

kiokuj commented Apr 7, 2024

codecov bot commented Apr 7, 2024 •

edited

Loading

kiokuj commented Apr 7, 2024 •

edited

Loading

kiokuj commented Apr 7, 2024 •

edited

Loading

kiokuj commented Apr 7, 2024 •

edited

Loading

tvdijen commented Apr 7, 2024 •

edited

Loading