#21 PATCH: Small fixes

[Stanislav German-Evtushenko]

Hello,

Thank you for the nice application you have made public. I like this one.

Here are three small patches:
info.php.patch - DN check has to be case insensitive.
suggest.js.patch - relative path for suggest.php (otherwise would not work when run from custom location and not root)
utils.php.patch - disable referrals (can not work with AD if referrals are enabled)

Best regards,
Stanislav

[James Turner]

Regarding info.php.patch:

Bug confirmed - DNs that differ only in case where considered to not match each other. The correct matching rules for each attribute (as per the directory schema) are not used.

Most common attributes that appear in a DNs (e.g. "DC", "OU", "CN") should be compared case insensitively, but to apply the correct matching rules in all cases the comparison should be done server-side (taking into account the schema):

if(ldap_compare($ldap_link,$ldap_base_dn,"DN",
        substr($dn,-strlen($ldap_base_dn))))
{
        // record is inside the address book - display it
}
else
        echo "This record is outside the part of the "
                . "directory which stores the address book.";

Ref: https://tools.ietf.org/html/rfc4517#section-4.2.15

[Stanislav German-Evtushenko]

In this case strlen should be also replaced by something else as non-reliable. For example, we can count compoentns instead.

~~~~~~
$base_component_count=ldap_explode_dn($ldap_base_dn,0)["count"];
$dn_base=join(array_slice(ldap_explode_dn($dn,0),-$base_component_count),",");
if(ldap_compare($ldap_link,$ldap_base_dn,"DN",$dn_base))
    ...
else
    ...
~~~~~~

[James Turner]

I've been trying to think what you mean by unreliable. Are you referring to special cases such as the following?

Base DN: dc=example,dc=org
DN to be tested: cn=Jimbo,acdc=example,dc=org

This would be incorrectly accepted, as the substring at the end does match the base DN. The attribute name of "example" must be checked to ensure it's "dc", not just something ending in the letters "dc". (could just look for a comma immediately before?)

[Stanislav German-Evtushenko]

Another example is having spaces in $ldap_base_dn, something like "dc=example, dc=com". While this would be intepreted as valid base DN our code with strlen would not work properly. Or we can think about hypothetical issues with multibyte characters.

[James Turner]

Using a method as per your last code snippet, utilising ldap_explode_dn(), array_slice(), etc, seems best then.

[James Turner]

Regarding suggest.js.patch:

Bug and fix confirmed; patch to be included in next version.

[James Turner]

Regarding utils.php.patch:

Everything seems to work OK (for me) in single domain AD forests, but multi-domain configurations have yet to be fully considered.

Do you have a multi-domain forest? Does your address book need to return records that span several domains?

If so then you may find that connecting to the global catalog port will work better for you, e.g.:

$ldap_link = ldap_connect("dc1.example.com",3268);

If you're connecting to the standard LDAP port (389) and having problems with referrals you could turn them off as per your patch. Unfortunately this limits the ability to get data from non-local domains.

In the short term I'm inclined to apply your patch (or perhaps make it a configurable option). Longer term, it would be nice to have referrals working, perhaps using an approach like the one described here:

http://php.net/manual/en/function.ldap-set-rebind-proc.php

[Stanislav German-Evtushenko]

Hi James,

I have checked the suggested way and found it working however some of important fields are missing when requesting records from global catalog. Here are some records which are missing:
title - missing
physicalDeliveryOfficeName - missing
company - missing
jpegPhoto - missing
thumbnailPhoto - missing

Best regards,
Stanislav

[James Turner]

You could add the attributes to the global catalog, although I appreciate that making schema changes may not be desirable or even allowed in many organisations. This page describes how to do it:

https://technet.microsoft.com/en-gb/library/cc737521%28v=ws.10%29.aspx

I've implemented the behaviour you suggested for version 0.18 - to disable referrals by default. They can be enabled by setting the following config option:

$follow_ldap_referrals = true;

At this time I don't have a multi-domain setup to test this on, so use of this setting should be be considered "experimental".

[James Turner]

These three patches now incorporated into version 0.18.