Add new option: ldap_class

[dlundgren]

The purpose of this would be to allow developers to specify a different class other than SimpleSAML\Module\ldap\Auth\Ldap to be instantiated as the LDAP connection.

[tvdijen]

Hi @dlundgren ! I've completely rewriten this module and I'm wondering how the current situation would suit your needs. We now implement symfony/ldap which allows you to use a different LDAP backend.. Is that what you were after with this issue/feature request?

[dlundgren]

During the course of the Active Directory integration, I ended up with a full copy of the ConfigHelper, and then extended the Auth\Ldap class and set it that way to get what I needed.

I ended up being able to detect:

• expired passwords
• expired accounts
• logon restrictions

I also set sAMAccountName from the UPN, due to the limited length of sAMAccountName in AD.

We haven't upgraded SSP yet, but need to do so, so I can look at the changes in the LDAP module, and go from there.

[tvdijen]

This makes me curious :) I thought you were after a custom LDAP-backend, but that's not the case after reading your last message.. The rewrite of the module got rid of both the ConfigHelper, and the Auth\Ldap class.. What's left is a new Utils\Ldap class with a few generic methods (where we can add more if necessary)..

PS: Sorry for taking so long to get back. I hope you see this rewrite as an improvement

[dlundgren]

In a sense, it is a custom LDAP backend, in so far as it understands Active Directory bind failures, and other errors that AD can return (expired account/password, logon restriction). These may be different in OpenLDAP, I'm not familiar to really know.

We are also returning custom error messages to the users so that we can make it more straight forward on what they need to do.

If these changes could be useful to others I can look at pulling them from our custom module and push them up as a separate connector.

[dlundgren]

I took a quick look at the updated code and it looks like I would still need an extension point to handle the additional processing/errors we are currently utilizing.

https://github.com/simplesamlphp/simplesamlphp-module-ldap/blob/c249eb8ae38f4016086a07346be67acea2f79a01/lib/Auth/Source/Ldap.php#L76

I'd like to suggest refactoring this to a function protected function resolveLdapConnector(): Utils\Ldap. This would be a good extension point for those looking to customize the LDAP connection itself. It may also lend itself to mocking the Utils\Ldap in tests.

Then the ldap_class option could still be utilized, and an ActiveDirectory connector could be introduced that understands how this works. I'll see if I can find time to send a PR for these changes, at the time I had deadlines to get the system operating as expected so I didn't have time to push up a PR.

[tvdijen]

You can already pass any object to the methods on Utils\Ldap as long as it implements Symfony's LdapInterface.. I think that's what you're after?

[dlundgren]

https://github.com/dlundgren/simplesamlphp-module-ldap/tree/dl/ad-support

I refactored the Utils\Ldap to Connector\Ldap, and also added Connector\ActiveDirectory based on how I am utilizing the older code base.

Primarily the ActiveDirectory class detects various errors coming from the AD.

I did add a test for the Source\Ldap showing how this could be utilized.

[tvdijen]

Very nice @dlundgren ! Is it okay if I import this with a couple of minor changes? I'd like to add a load of typehints and use LdapInterface and AdapterInterface in places like here...

[tvdijen]

Was closed in #32