Use the Net/LDAP3::domain_root_dn() function if it is available

[kanarip]

Discovering the root dn used for a domain is a routine available in Net/LDAP3.

[alecpl]

I catch the idea, but the change would need to be more complicated considering the body of this if statement https://github.com/roundcube/roundcubemail/blob/master/program/lib/Roundcube/rcube_ldap.php#L296. At least I'm sure we don't need to search for %dc two times.

[kanarip]

The $this->prop['search_bind_dn'] however is not correctly substituted any %dc of, if the $replaces does not receive the correct substitution value -- which needs to be discovered.

[thomascube]

Hmm, but $this->prop['search_bind_dn'] is used for the initial bind to get the root_dn according to your change. It therefore cannot contain any placeholder. Or we need yet another option for that initial bind.

And we don't want to execute this extra bind to resolve the root_dn every time but only when it's actually necessary. Like when there are actually %dc placeholders to be replaced.

[kanarip]

I'm not sure why we are circling a $this->prop['search base dn'] which I'm only using, not modifying. It is $dc that (later used in substitution) bears the relevancy.

[thomascube]

Apparently I misunderstood your previous comment about "The $this->prop['search_bind_dn'] however is not correctly substituted any %dc".

Anyway, I'm trying to avoid implicit calls to the rather costly domain_root_dn() routine unless necessary. Maybe the config should get another option to "hint" the code that a root_dn lookup is required. I see in Net_LDAP3 there are additional options like domain_base_dn, domain_filter, and domain_name_attribute involved. Maybe these can be used to check whether a bind + domain_root_dn() call needs to be done?

[kanarip]

The session may already have a bind dn associated with it, in which case this lookup isn't necessary either (for kolab_auth at least).

[thomascube]

I guess would consider to cache this lookup in session data later on. But first, here's my proposition to do this lookup in the first place:

if (!empty($this->prop['domain_filter']) && !empty($this->prop['search_bind_dn'])
     && method_exists($this->ldap, 'domain_root_dn')) {
    $this->ldap->bind($this->prop['search_bind_dn'], $this->prop['search_bind_pw']);
    $dc = $this->ldap->domain_root_dn($d);
}

That would at least limit the lookup to hosted configurations (domain_filter not being part of the default Roundcube/Kolab config).

[thomascube]

FWIW: Net_LDAP3::find_domain() already uses caching. Although that doesn't omit the extra bind() we're doing before the domain_root_dn() call.

[kanarip]

IIRC, it uses caching only for subsequent repeated calls to the function in the same run, though, and nothing like a cross-request session, right?

[thomascube]

LDAP3.php Line 3052 uses whatever cache is configured and in Roundcube this is controlled by the ldap_cache config option and defaults to database caching. And the kolab_auth plugin stores the resolved username in session and would not repeat this lookup on every request.

[thomascube]

Modified patch committed in 0f63418. Closing.