[Bug] Catching reason for failed authentication

Environment (please complete the following information):

LDAP Server Type: [e.g. ActiveDirectory]
PHP Version: [7.4]

Describe the bug: This is potentially a bug or potentially a deficiency in the documentation depending on interpretation. While using the example code for attempting authentication of a user and catching reasons for an unsuccessful auth, including password is wrong, password expired etc, I found that getLdapConnection()->getDiagnosticMessage() was always returning a blank string.

I tried this on AWS managed Microsoft AD, a genuine AD running on a VM, using Adlapd2 and LdapDirectory, both executing in two different environments (OSX and Linux).

However I went back to using pure php and it worked. I then on a hunch tried the third parameter (remaining bound as the user) and it worked! It seems that whatever code is changing the binding back to the main user is also clearing out the ability to get the diagnostic message (that's just a hunch).

It may be 'nice' to change up this API so that it throws exceptions based on these LDAP codes regardless of whether a user stays bound. That seems like a nicer API compared to using strpos on the error code.

Cheers (me again!)

Hey @longshadowdev!

Apologies for the late response on this.

I think you're right on the documentation issue. Is this the example you're talking about?

https://ldaprecord.com/docs/authentication/#determining-bind-failure

The diagnostic message is cleared after rebinding as the configured user, so you will not actually be able to retrieve the diagnostic message from an $connection->auth()->attempt() unless you use an event listener on the Failed auth event:

use LdapRecord\Container;
use LdapRecord\Auth\Events\Failed;

$connection = new Connection(['...']);

Container::addConnection($connection);

$dispatcher = Container::getEventDispatcher();

$dispatcher->listen(Failed::class, function (Failed $event) {
    $ldap = $event->connection->getLdapConnection();

    // The diagnostic message will be available here.
    $diag = $ldap->getDiagnosticMessage();
});

$connection->auth()->attempt('username', 'password');

It may be 'nice' to change up this API so that it throws exceptions based on these LDAP codes regardless of whether a user stays bound. That seems like a nicer API compared to using strpos on the error code

You can use $connection->bind() if you prefer an exception based API:

try {
    $connection->auth()->bind($username, $password);

    // Further bound operations...
} catch (\LdapRecord\Auth\BindException $e) {
    $error = $e->getDetailedError()->getDiagnosticMessage();

    if (strpos($error, '532') !== false) {
        return "Your password has expired.";
    } elseif (strpos($error, '533') !== false) {
        return "Your account is disabled.";
    } elseif (strpos($error, '701') !== false) {
        return "Your account has expired.";
    } elseif (strpos($error, '775') !== false) {
        return "Your account is locked.";
    }

    return "Your account is locked.";
}

This is exactly why the attempt() method exists -- so you can use whichever you prefer. I'm also unsure if I want to include exceptions for every Active Directory error code, since they are Active Directory specific. I'm trying to keep LdapRecord as open as possible with all directories.

If we can somehow keep the exception logic contained to Active Directory servers and not have them in the Connection core -- I'd gladly accept a PR!

I'll have the docs fixed up tonight so there's no confusion. Thanks!

DirectoryTree / LdapRecord

[Bug] Catching reason for failed authentication #167