Skip to content
This repository has been archived by the owner on Jul 24, 2023. It is now read-only.

Lazy connection before searching for user while trying to authenticate #855

Merged
merged 2 commits into from
May 28, 2020

Conversation

KenoKokoro
Copy link
Contributor

  • Laravel Version: 7.1.3
  • Adldap2-Laravel Version: v6.0.9
  • PHP Version: 7.4.3
  • LDAP Type: OpenLDAP

Description:

When trying to authenticate users using the NoDatabaseUserProvider the package is not binding to the LDAP server but only trying to search for the specific record:

[2020-03-14 15:47:42] local.INFO: LDAP (ldap://openldap:389) - Connection: default - Operation: Search - Base DN: dc=domain,dc=com - Filter: (&(objectclass=inetorgperson)(objectclass=person)(uid=testuser)) - Selected: (*,entryuuid) - Time Elapsed: 0.55

However if I use the facade or inject the AdldapInterface the package seems to work just fine. The difference that I noticed is that the facade and the injected interface, when calling the search() method using the magic __call method on the Adldap instance, is trying to connect if the connection is not bound:

    /**
     * {@inheritdoc}
     */
    public function __call($method, $parameters)
    {
        $provider = $this->getDefaultProvider();

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

        return call_user_func_array([$provider, $method], $parameters);
    }

So right now if I try to search for a user, I see also on the log that the connection is bound before searching:

[2020-03-14 17:52:18] local.INFO: LDAP (ldap://openldap:389) - Connection: default - Operation: Binding - Username: cn=admin,dc=domain,dc=com  
[2020-03-14 17:52:18] local.INFO: LDAP (ldap://openldap:389) - Connection: default - Operation: Bound - Username: cn=admin,dc=domain,dc=com  
[2020-03-14 17:52:18] local.INFO: LDAP (ldap://openldap:389) - Connection: default - Operation: Search - Base DN: dc=domain,dc=com - Filter: (&(objectclass=inetorgperson)(objectclass=person)(uid=testuser)) - Selected: (*) - Time Elapsed: 0.32

But this is not the case with the UserResolver query method. Basically is just calling to get the provider, and executing the call without trying to establish the connection to the LDAP before checking if the user exists on the LDAP server, and then to authenticate with it. So after I did the change from the PR, I got in log:

[2020-03-14 15:49:09] local.INFO: LDAP (ldap://openldap:389) - Connection: default - Operation: Binding - Username: cn=admin,dc=domain,dc=com  
[2020-03-14 15:49:09] local.INFO: LDAP (ldap://openldap:389) - Connection: default - Operation: Bound - Username: cn=admin,dc=domain,dc=com  
[2020-03-14 15:49:09] local.INFO: LDAP (ldap://openldap:389) - Connection: default - Operation: Search - Base DN: dc=domain,dc=com - Filter: (&(objectclass=inetorgperson)(objectclass=person)(uid=testuser)) - Selected: (*,entryuuid) - Time Elapsed: 0.28  
[2020-03-14 15:49:09] local.INFO: User 'Test User has been successfully found for authentication.  
[2020-03-14 15:49:09] local.INFO: User 'Test User'' is authenticating with username: 'uid=testuser,ou=People,dc=domain,dc=com'  
[2020-03-14 15:49:09] local.INFO: LDAP (ldap://openldap:389) - Connection: default - Operation: Attempting - Username: uid=testuser,ou=People,dc=domain,dc=com  
[2020-03-14 15:49:09] local.INFO: LDAP (ldap://openldap:389) - Connection: default - Operation: Binding - Username: uid=testuser,ou=People,dc=domain,dc=com  
[2020-03-14 15:49:09] local.INFO: LDAP (ldap://openldap:389) - Connection: default - Operation: Bound - Username: uid=testuser,ou=People,dc=domain,dc=com  
[2020-03-14 15:49:09] local.INFO: LDAP (ldap://openldap:389) - Connection: default - Operation: Passed - Username: uid=testuser,ou=People,dc=domain,dc=com  
[2020-03-14 15:49:09] local.INFO: LDAP (ldap://openldap:389) - Connection: default - Operation: Binding - Username: cn=admin,dc=domain,dc=com  
[2020-03-14 15:49:09] local.INFO: LDAP (ldap://openldap:389) - Connection: default - Operation: Bound - Username: cn=admin,dc=domain,dc=com  
[2020-03-14 15:49:09] local.INFO: User 'Test User' has successfully passed LDAP authentication.  
[2020-03-14 15:49:09] local.INFO: User 'Test User' has been successfully logged in.  
[2020-03-14 15:49:09] local.INFO: LDAP (ldap://openldap:389) - Connection: default - Operation: Binding - Username: cn=admin,dc=domain,dc=com  
[2020-03-14 15:49:09] local.INFO: LDAP (ldap://openldap:389) - Connection: default - Operation: Bound - Username: cn=admin,dc=domain,dc=com  
[2020-03-14 15:49:09] local.INFO: LDAP (ldap://openldap:389) - Connection: default - Operation: Search - Base DN: dc=domain,dc=com - Filter: (&(objectclass=inetorgperson)(objectclass=person)(entryuuid=1e32ea48-fa48-1039-9595-2b51f8d2ca00)) - Selected: (*,entryuuid) - Time Elapsed: 0.43

So, am I missing something here in the configuration, or this is the way to go?

@stevebauman stevebauman merged commit 9fe6006 into Adldap2:master May 28, 2020
@stevebauman
Copy link
Member

Hi @KenoKokoro, apologies for the massive delay on this.

I totally agree with this PR. We should ensure the connection is bound before we attempt searching for records, regardless of the auto_connect configuration option.

I appreciate your work here! I'll make a new release shortly with this included.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants