Merge pull request #2924 from Icinga/bugfix/ldap-membership-resolution

Problems in LDAP membership resolution

.travis.yml

@@ -13,6 +13,8 @@ matrix:
   include:
   - php: '5.3'
     dist: precise
+    env:
+    - ENABLE_LDAP=1
 
 services:
   - mysql
@@ -32,7 +34,9 @@ notifications:
 
 # also see: test/setup_vendor.sh
 before_script:
+  - php -m
   - sudo locale-gen en_US.UTF-8 de_DE.UTF-8 fr_FR.UTF-8
+  - sh -c '[ -z $ENABLE_LDAP ] || phpenv config-add test/travis-ldap.ini'
   - test/travis_database.sh
   - test/setup_vendor.sh
 

library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php

@@ -10,6 +10,7 @@ use Icinga\Data\ConfigObject;
 use Icinga\Exception\ConfigurationError;
 use Icinga\Exception\ProgrammingError;
 use Icinga\Protocol\Ldap\LdapException;
+use Icinga\Protocol\Ldap\LdapUtils;
 use Icinga\Repository\LdapRepository;
 use Icinga\Repository\RepositoryQuery;
 use Icinga\User;
@@ -438,6 +439,11 @@ class LdapUserGroupBackend extends LdapRepository implements UserGroupBackendInt
     /**
      * Return whether the attribute name where to find a group's member holds ambiguous values
      *
+     * This tries to detect if the member attribute of groups contain:
+     *
+     *  full DN -> distinguished name of another object
+     *  other   -> ambiguous field referencing the member by userNameAttribute
+     *
      * @return  bool
      *
      * @throws  ProgrammingError    In case either $this->groupClass or $this->groupMemberAttribute
@@ -463,7 +469,8 @@ class LdapUserGroupBackend extends LdapRepository implements UserGroupBackendInt
                 ->setUnfoldAttribute($this->groupMemberAttribute)
                 ->setBase($this->groupBaseDn)
                 ->fetchOne();
-            $this->ambiguousMemberAttribute = !$this->isRelatedDn($sampleValue);
+
+            $this->ambiguousMemberAttribute = ! LdapUtils::isDn($sampleValue);
         }
 
         return $this->ambiguousMemberAttribute;

library/Icinga/Protocol/Ldap/LdapUtils.php

@@ -19,8 +19,9 @@ class LdapUtils
      * UTF-8 chars like German umlauts would otherwise be escaped and shown
      * as backslash-prefixed hexcode-sequenzes.
      *
-     * @param  string  DN
-     * @param  boolean Returns 'type=value' when true and 'value' when false
+     * @param  string  $dn        DN
+     * @param  boolean $with_type Returns 'type=value' when true and 'value' when false
+     *
      * @return string
      */
     public static function explodeDN($dn, $with_type = true)
@@ -45,7 +46,8 @@ class LdapUtils
      *
      * TODO: throw away, this is not how it shall be done
      *
-     * @param  string DN-component
+     * @param  array $parts DN-component
+     *
      * @return string
      */
     public static function implodeDN($parts)
@@ -61,12 +63,28 @@ class LdapUtils
         return $str;
     }
 
+    /**
+     * Test if supplied value looks like a DN
+     *
+     * @param  mixed $value
+     *
+     * @return bool
+     */
+    public static function isDn($value)
+    {
+        if (is_string($value)) {
+            return ldap_dn2ufn($value) !== false;
+        }
+        return false;
+    }
+
     /**
      * Quote a string that should be used in a DN
      *
      * Special characters will be escaped
      *
-     * @param  string DN-component
+     * @param  string $str DN-component
+     *
      * @return string
      */
     public static function quoteForDN($str)

library/Icinga/Repository/LdapRepository.php

@@ -74,6 +74,9 @@ abstract class LdapRepository extends Repository
      *
      * Will use the current connection's root DN if $baseDn is not given.
      *
+     * @deprecated This was only used by LdapUserGroupBackend::isMemberAttributeAmbiguous
+     *             It will be removed with 2.6.0!
+     *
      * @param   string  $dn         The object DN to check
      * @param   string  $baseDn     The base DN to compare the object DN with
      *

test/php/library/Icinga/Protocol/Ldap/LdapUtilsTest.php

@@ -0,0 +1,41 @@
+<?php
+/* Icinga Web 2 | (c) 2013 Icinga Development Team | GPLv2+ */
+
+namespace Tests\Icinga\Protocol\Ldap;
+
+use Icinga\Protocol\Ldap\LdapUtils;
+use Icinga\Test\BaseTestCase;
+
+class LdapUtilsTest extends BaseTestCase
+{
+    protected static $validDn = array(
+        'dc=example,dc=com',
+        'dc=example, dc=com',
+        'dc = example , dc = com',
+        'DC=EXAMPLE,DC=COM',
+        '0.9.2342.19200300.100.1.25=Example,0.9.2342.19200300.100.1.25=Com',
+        'CN=host,OU=Datacenter Servers,DC=example,DC=com',
+        'CN=Doe\, John,OU=Admin Users,DC=example,DC=com'
+    );
+
+    protected static $invalidDn = array(
+        'testuser',
+        'heinzimüller',
+        'test.user@example.com',
+        'test,user@example.com',
+    );
+
+    public function testIsDnForValidValues()
+    {
+        foreach (static::$validDn as $dn) {
+            $this->assertTrue(LdapUtils::isDn($dn), 'DN should be tested as valid value: ' . $dn);
+        }
+    }
+
+    public function testIsDnForInvalidValues()
+    {
+        foreach (static::$invalidDn as $dn) {
+            $this->assertFalse(LdapUtils::isDn($dn), 'DN should be tested as invalid value: ' . $dn);
+        }
+    }
+}

test/travis-ldap.ini

@@ -0,0 +1 @@
+extension=ldap.so