Login fallback when there is no connection does not work

[dionysiosarvanitis]

• Laravel Version: 7.0
• Adldap2-Laravel Version: latest
• PHP Version: 7.4
• LDAP Type: ActiveDirectory

Description:

Login fallback when there is no connection does not work.

The problem is caused by the line $user = Resolver::byCredentials($credentials); inside DatabaseUserProvider::retrieveByCredentials.

It tries to connect with an unbounded connection without handling the exception thrown. Changing if statement like below fixes the problem.

- if (!$provider->getConnection()->isBound())
+ if ($provider->getConnection()->isBound())

    protected function getLdapAuthProvider(): ProviderInterface
    {
        $provider = $this->ldap->getProvider($this->connection ?? $this->getLdapAuthConnectionName());

        if (!$provider->getConnection()->isBound()) {
            // We'll make sure we have a bound connection before
            // allowing dynamic calls on the default provider.
            $provider->connect();
        }

        return $provider;
    }

Can make a PR if you want.

[ivanwitzke]

fixed on #898

[stevebauman]

Fixed thanks to @ivanwitzke - v6.1.3 is now released with his patch 👍

[dionysiosarvanitis]

Thanks a lot!

Altough, it still needs a small fix for cases when exception is thrown. $user is undefined and therefore instanceof check throws an other error Undefined variable: user. Maybe the block below should be moved inside try.

if ($user instanceof User) {
    return $this->setAndImportAuthenticatingUser($user);
}

It also still bugs me a lot the block below. It says We'll make sure we have a bound connection but it checks for the opposite! Comment or condition should change. Isn't it?

if (!$provider->getConnection()->isBound()) {
    // We'll make sure we have a bound connection before
    // allowing dynamic calls on the default provider.
    $provider->connect();
}

[stevebauman]

Hi @dionysiosarvanitis!

Altough, it still needs a small fix for cases when exception is thrown. $user is undefined and therefore instanceof check throws an other error Undefined variable: user. Maybe the block below should be moved inside try.

That's completely my mistake, I should have triple checked the PR before merging. It's now fixed and will be released shortly.

It also still bugs me a lot the block below. It says We'll make sure we have a bound connection but it checks for the opposite! Comment or condition should change. Isn't it?

You're missing the outer context of that snippet, as it's saying "If dynamic calls are being made to the default provider, then make sure we bind to that provider before we pass the call along if it is not already connected".

I'll update the comment so it's more clear 👍