diff --git a/auth/ldap/auth.php b/auth/ldap/auth.php index b23c9ec99be..6c671ee7aaf 100644 --- a/auth/ldap/auth.php +++ b/auth/ldap/auth.php @@ -276,7 +276,7 @@ class auth_plugin_ldap extends auth_plugin_base { } $ldapval = NULL; foreach ($values as $value) { - $entry = array_change_key_case($user_entry[0], CASE_LOWER); + $entry = $user_entry[0]; if (($value == 'dn') || ($value == 'distinguishedname')) { $result[$key] = $user_dn; continue; @@ -634,7 +634,7 @@ class auth_plugin_ldap extends auth_plugin_base { if ($sr) { $info = ldap_get_entries_moodle($ldapconnection, $sr); if (!empty ($info)) { - $info = array_change_key_case($info[0], CASE_LOWER); + $info = $info[0]; if (isset($info[$this->config->expireattr][0])) { $expiretime = $this->ldap_expirationtime2unix($info[$this->config->expireattr][0], $ldapconnection, $user_dn); if ($expiretime != 0) { @@ -1201,7 +1201,7 @@ class auth_plugin_ldap extends auth_plugin_base { return false; } - $user_entry = array_change_key_case($user_entry[0], CASE_LOWER); + $user_entry = $user_entry[0]; foreach ($attrmap as $key => $ldapkeys) { $profilefield = ''; @@ -1369,7 +1369,7 @@ class auth_plugin_ldap extends auth_plugin_base { $sr = ldap_read($ldapconnection, $user_dn, '(objectClass=*)', $search_attribs); if ($sr) { $entry = ldap_get_entries_moodle($ldapconnection, $sr); - $info = array_change_key_case($entry[0], CASE_LOWER); + $info = $entry[0]; $newattrs = array(); if (!empty($info[$this->config->expireattr][0])) { // Set expiration time only if passwordExpirationInterval is defined @@ -2024,7 +2024,7 @@ class auth_plugin_ldap extends auth_plugin_base { } $entry = ldap_get_entries_moodle($ldapconn, $sr); - $info = array_change_key_case($entry[0], CASE_LOWER); + $info = $entry[0]; $useraccountcontrol = $info['useraccountcontrol'][0]; if ($useraccountcontrol & UF_DONT_EXPIRE_PASSWD) { // Password doesn't expire. @@ -2069,17 +2069,17 @@ class auth_plugin_ldap extends auth_plugin_base { } $entry = ldap_get_entries_moodle($ldapconn, $sr); - $info = array_change_key_case($entry[0], CASE_LOWER); + $info = $entry[0]; $domaindn = $info['defaultnamingcontext'][0]; $sr = ldap_read ($ldapconn, $domaindn, '(objectClass=*)', array('maxPwdAge')); $entry = ldap_get_entries_moodle($ldapconn, $sr); - $info = array_change_key_case($entry[0], CASE_LOWER); + $info = $entry[0]; $maxpwdage = $info['maxpwdage'][0]; if ($sr = ldap_read($ldapconn, $user_dn, '(objectClass=*)', array('msDS-ResultantPSO'))) { if ($entry = ldap_get_entries_moodle($ldapconn, $sr)) { - $info = array_change_key_case($entry[0], CASE_LOWER); + $info = $entry[0]; $userpso = $info['msds-resultantpso'][0]; // If a PSO exists, FGPP is being utilized. @@ -2087,7 +2087,7 @@ class auth_plugin_ldap extends auth_plugin_base { if (!empty($userpso)) { $sr = ldap_read($ldapconn, $userpso, '(objectClass=*)', array('msDS-MaximumPasswordAge')); if ($entry = ldap_get_entries_moodle($ldapconn, $sr)) { - $info = array_change_key_case($entry[0], CASE_LOWER); + $info = $entry[0]; // Default value of msds-maximumpasswordage is 42 and is always set. $maxpwdage = $info['msds-maximumpasswordage'][0]; } diff --git a/lib/ldaplib.php b/lib/ldaplib.php index 8a5c9652b9b..78923c7f131 100644 --- a/lib/ldaplib.php +++ b/lib/ldaplib.php @@ -339,13 +339,18 @@ function ldap_get_entries_moodle($ldapconnection, $searchresult) { return array(); } do { - $attributes = array_change_key_case(ldap_get_attributes($ldapconnection, $entry), CASE_LOWER); - for ($j = 0; $j < $attributes['count']; $j++) { - $values = ldap_get_values_len($ldapconnection, $entry, $attributes[$j]); + $attributes = array(); + $attribute = ldap_first_attribute($ldapconnection, $entry); + while ($attribute !== false) { + $attributes[] = strtolower($attribute); // Attribute names don't usually contain non-ASCII characters. + $attribute = ldap_next_attribute($ldapconnection, $entry); + } + foreach ($attributes as $attribute) { + $values = ldap_get_values_len($ldapconnection, $entry, $attribute); if (is_array($values)) { - $result[$i][$attributes[$j]] = $values; + $result[$i][$attribute] = $values; } else { - $result[$i][$attributes[$j]] = array($values); + $result[$i][$attribute] = array($values); } } $i++; @@ -488,7 +493,7 @@ function ldap_paged_results_supported($ldapversion, $ldapconnection = null) { if (empty($entries)) { return false; } - $info = array_change_key_case($entries[0], CASE_LOWER); + $info = $entries[0]; if (isset($info['supportedcontrol']) && in_array(LDAP_PAGED_RESULTS_CONTROL, $info['supportedcontrol'])) { return true; } diff --git a/lib/tests/ldaplib_test.php b/lib/tests/ldaplib_test.php index 909c3f71140..4656b114e62 100644 --- a/lib/tests/ldaplib_test.php +++ b/lib/tests/ldaplib_test.php @@ -207,4 +207,268 @@ class core_ldaplib_testcase extends advanced_testcase { ), ); } + + /** + * Tests for ldap_get_entries_moodle. + * + * NOTE: in order to execute this test you need to set up OpenLDAP server with core, + * cosine, nis and internet schemas and add configuration constants to + * config.php or phpunit.xml configuration file. The bind users *needs* + * permissions to create objects in the LDAP server, under the bind domain. + * + * define('TEST_LDAPLIB_HOST_URL', 'ldap://127.0.0.1'); + * define('TEST_LDAPLIB_BIND_DN', 'cn=someuser,dc=example,dc=local'); + * define('TEST_LDAPLIB_BIND_PW', 'somepassword'); + * define('TEST_LDAPLIB_DOMAIN', 'dc=example,dc=local'); + * + */ + public function test_ldap_get_entries_moodle() { + $this->resetAfterTest(); + + if (!defined('TEST_LDAPLIB_HOST_URL') or !defined('TEST_LDAPLIB_BIND_DN') or + !defined('TEST_LDAPLIB_BIND_PW') or !defined('TEST_LDAPLIB_DOMAIN')) { + $this->markTestSkipped('External LDAP test server not configured.'); + } + + // Make sure we can connect the server. + $debuginfo = ''; + if (!$connection = ldap_connect_moodle(TEST_LDAPLIB_HOST_URL, 3, 'rfc2307', TEST_LDAPLIB_BIND_DN, + TEST_LDAPLIB_BIND_PW, LDAP_DEREF_NEVER, $debuginfo, false)) { + $this->markTestSkipped('Cannot connect to LDAP test server: '.$debuginfo); + } + + // Create new empty test container. + if (!($containerdn = $this->create_test_container($connection, 'moodletest'))) { + $this->markTestSkipped('Can not create test LDAP container.'); + } + + // Add all the test objects. + $testobjects = $this->get_ldap_get_entries_moodle_test_objects(); + if (!$this->add_test_objects($connection, $containerdn, $testobjects)) { + $this->markTestSkipped('Can not create LDAP test objects.'); + } + + // Now query about them and compare results. + foreach ($testobjects as $object) { + $dn = $this->get_object_dn($object, $containerdn); + $filter = $object['query']['filter']; + $attributes = $object['query']['attributes']; + + $sr = ldap_read($connection, $dn, $filter, $attributes); + if (!$sr) { + $this->markTestSkipped('Cannot retrieve test objects from LDAP test server.'); + } + + $entries = ldap_get_entries_moodle($connection, $sr); + $actual = array_keys($entries[0]); + $expected = $object['expected']; + + // We need to sort both arrays to be able to compare them, as the LDAP server + // might return attributes in any order. + sort($expected); + sort($actual); + $this->assertEquals($expected, $actual); + } + + // Clean up test objects and container. + $this->remove_test_objects($connection, $containerdn, $testobjects); + $this->remove_test_container($connection, $containerdn); + } + + /** + * Provide the array of test objects for the ldap_get_entries_moodle test case. + * + * @return array of test objects + */ + protected function get_ldap_get_entries_moodle_test_objects() { + $testobjects = array( + // Test object 1. + array( + // Add/remove this object to LDAP directory? There are existing standard LDAP + // objects that we might want to test, but that we should'nt add/remove ourselves. + 'addremove' => true, + // Relative (to test container) or absolute distinguished name (DN). + 'relativedn' => true, + // Distinguished name for this object (interpretation depends on 'relativedn'). + 'dn' => 'cn=test1', + // Values to add to LDAP directory. + 'values' => array( + 'objectClass' => array('inetOrgPerson', 'organizationalPerson', 'person', 'posixAccount'), + 'cn' => 'test1', // We don't care about the actual values, as long as they are unique. + 'sn' => 'test1', + 'givenName' => 'test1', + 'uid' => 'test1', + 'uidNumber' => '20001', // Start from 20000, then add test number. + 'gidNumber' => '20001', // Start from 20000, then add test number. + 'homeDirectory' => '/', + 'userPassword' => '*', + ), + // Attributes to query the object for. + 'query' => array( + 'filter' => '(objectClass=posixAccount)', + 'attributes' => array( + 'cn', + 'sn', + 'givenName', + 'uid', + 'uidNumber', + 'gidNumber', + 'homeDirectory', + 'userPassword' + ), + ), + // Expected values for the queried attributes' names. + 'expected' => array( + 'cn', + 'sn', + 'givenname', + 'uid', + 'uidnumber', + 'gidnumber', + 'homedirectory', + 'userpassword' + ), + ), + // Test object 2. + array( + 'addremove' => true, + 'relativedn' => true, + 'dn' => 'cn=group2', + 'values' => array( + 'objectClass' => array('top', 'posixGroup'), + 'cn' => 'group2', // We don't care about the actual values, as long as they are unique. + 'gidNumber' => '20002', // Start from 20000, then add test number. + 'memberUid' => '20002', // Start from 20000, then add test number. + ), + 'query' => array( + 'filter' => '(objectClass=posixGroup)', + 'attributes' => array( + 'cn', + 'gidNumber', + 'memberUid' + ), + ), + 'expected' => array( + 'cn', + 'gidnumber', + 'memberuid' + ), + ), + // Test object 3. + array( + 'addremove' => false, + 'relativedn' => false, + 'dn' => '', // To query the RootDSE, we must specify the empty string as the absolute DN. + 'values' => array( + ), + 'query' => array( + 'filter' => '(objectClass=*)', + 'attributes' => array( + 'supportedControl', + 'namingContexts' + ), + ), + 'expected' => array( + 'supportedcontrol', + 'namingcontexts' + ), + ), + ); + + return $testobjects; + } + + /** + * Create a new container in the LDAP domain, to hold the test objects. The + * container is created as a domain component (dc) + organizational unit (ou) object. + * + * @param object $connection Valid LDAP connection + * @param string $container Name of the test container to create. + * + * @return string or false Distinguished name for the created container, or false on error. + */ + protected function create_test_container($connection, $container) { + $object = array(); + $object['objectClass'] = array('dcObject', 'organizationalUnit'); + $object['dc'] = $container; + $object['ou'] = $container; + $containerdn = 'dc='.$container.','.TEST_LDAPLIB_DOMAIN; + if (!ldap_add($connection, $containerdn, $object)) { + return false; + } + return $containerdn; + } + + /** + * Remove the container in the LDAP domain root that holds the test objects. The container + * *must* be empty before trying to remove it. Otherwise this function fails. + * + * @param object $connection Valid LDAP connection + * @param string $containerdn The distinguished of the container to remove. + */ + protected function remove_test_container($connection, $containerdn) { + ldap_delete($connection, $containerdn); + } + + /** + * Add the test objects to the test container. + * + * @param resource $connection Valid LDAP connection + * @param string $containerdn The distinguished name of the container for the created objects. + * @param array $testobjects Array of the tests objects to create. The structure of + * the array elements *must* follow the structure of the value returned + * by ldap_get_entries_moodle_test_objects() member function. + * + * @return boolean True on success, false otherwise. + */ + protected function add_test_objects($connection, $containerdn, $testobjects) { + foreach ($testobjects as $object) { + if ($object['addremove'] !== true) { + continue; + } + $dn = $this->get_object_dn($object, $containerdn); + $entry = $object['values']; + if (!ldap_add($connection, $dn, $entry)) { + return false; + } + } + return true; + } + + /** + * Remove the test objects from the test container. + * + * @param resource $connection Valid LDAP connection + * @param string $containerdn The distinguished name of the container for the objects to remove. + * @param array $testobjects Array of the tests objects to create. The structure of + * the array elements *must* follow the structure of the value returned + * by ldap_get_entries_moodle_test_objects() member function. + * + */ + protected function remove_test_objects($connection, $containerdn, $testobjects) { + foreach ($testobjects as $object) { + if ($object['addremove'] !== true) { + continue; + } + $dn = $this->get_object_dn($object, $containerdn); + ldap_delete($connection, $dn); + } + } + + /** + * Get the distinguished name (DN) for a given object. + * + * @param object $object The LDAP object to calculate the DN for. + * @param string $containerdn The DN of the container to use for objects with relative DNs. + * + * @return string The calculated DN. + */ + protected function get_object_dn($object, $containerdn) { + if ($object['relativedn']) { + $dn = $object['dn'].','.$containerdn; + } else { + $dn = $object['dn']; + } + return $dn; + } } diff --git a/lib/upgrade.txt b/lib/upgrade.txt index bc87ba9a6e0..ba7874fc69e 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -5,6 +5,8 @@ information provided here is intended especially for developers. * Behat compatibility changes are now being documented at https://docs.moodle.org/dev/Acceptance_testing/Compatibility_changes +* ldap_get_entries_moodle() now always returns lower-cased attribute names in the returned entries. + It was suppposed to do so before, but it actually didn't. === 3.2.2 ===