From bcf73493710963a340e638269ffb09fbbca0237e Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Tue, 26 Nov 2019 23:43:18 +0100 Subject: [PATCH 1/4] MDL-67118 auth_ldap: paged results functions deprecated php74 and up Starting with php74 the following functions are deprecated: - ldap_control_paged_result() - ldap_control_paged_result_response() Starting with php73, ldap servercontrols were included. One of those servercontrols, LDAP_CONTROL_PAGEDRESULTS, is the one in charge of controlling paged results. So, we are going to add some conditional code here: 1) if php < 7.3, use old paged result functions. 2) if php >= 7.3, switch to LDAP_CONTROL_PAGEDRESULTS servercontrol. With a TODO about removing 1) in Moodle 4.1, once php73 becomes required. --- auth/ldap/auth.php | 94 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 82 insertions(+), 12 deletions(-) diff --git a/auth/ldap/auth.php b/auth/ldap/auth.php index 1052f3a582b..f03372f3154 100644 --- a/auth/ldap/auth.php +++ b/auth/ldap/auth.php @@ -691,6 +691,7 @@ class auth_plugin_ldap extends auth_plugin_base { //// // prepare some data we'll need $filter = '(&('.$this->config->user_attribute.'=*)'.$this->config->objectclass.')'; + $servercontrols = array(); $contexts = explode(';', $this->config->contexts); @@ -708,24 +709,58 @@ class auth_plugin_ldap extends auth_plugin_base { do { if ($ldappagedresults) { - ldap_control_paged_result($ldapconnection, $this->config->pagesize, true, $ldapcookie); + // TODO: Remove the old branch of code once PHP 7.3.0 becomes required (Moodle 4.1). + if (version_compare(PHP_VERSION, '7.3.0', '<')) { + // Before 7.3, use this function that was deprecated in PHP 7.4. + ldap_control_paged_result($ldapconnection, $this->config->pagesize, true, $ldapcookie); + } else { + // PHP 7.3 and up, use server controls. + $servercontrols = array(array( + 'oid' => LDAP_CONTROL_PAGEDRESULTS, 'value' => array( + 'size' => $this->config->pagesize, 'cookie' => $ldapcookie))); + } } if ($this->config->search_sub) { // Use ldap_search to find first user from subtree. - $ldapresult = ldap_search($ldapconnection, $context, $filter, array($this->config->user_attribute)); + // TODO: Remove the old branch of code once PHP 7.3.0 becomes required (Moodle 4.1). + if (version_compare(PHP_VERSION, '7.3.0', '<')) { + $ldapresult = ldap_search($ldapconnection, $context, $filter, array($this->config->user_attribute)); + } else { + $ldapresult = ldap_search($ldapconnection, $context, $filter, array($this->config->user_attribute), + 0, -1, -1, LDAP_DEREF_NEVER, $servercontrols); + } } else { // Search only in this context. - $ldapresult = ldap_list($ldapconnection, $context, $filter, array($this->config->user_attribute)); + // TODO: Remove the old branch of code once PHP 7.3.0 becomes required (Moodle 4.1). + if (version_compare(PHP_VERSION, '7.3.0', '<')) { + $ldapresult = ldap_list($ldapconnection, $context, $filter, array($this->config->user_attribute)); + } else { + $ldapresult = ldap_list($ldapconnection, $context, $filter, array($this->config->user_attribute), + 0, -1, -1, LDAP_DEREF_NEVER, $servercontrols); + } } if (!$ldapresult) { continue; } if ($ldappagedresults) { - $pagedresp = ldap_control_paged_result_response($ldapconnection, $ldapresult, $ldapcookie); - // Function ldap_control_paged_result_response() does not overwrite $ldapcookie if it fails, by - // setting this to null we avoid an infinite loop. - if ($pagedresp === false) { - $ldapcookie = null; + // Get next server cookie to know if we'll need to continue searching. + $ldapcookie = ''; + // TODO: Remove the old branch of code once PHP 7.3.0 becomes required (Moodle 4.1). + if (version_compare(PHP_VERSION, '7.3.0', '<')) { + // Before 7.3, use this function that was deprecated in PHP 7.4. + $pagedresp = ldap_control_paged_result_response($ldapconnection, $ldapresult, $ldapcookie); + // Function ldap_control_paged_result_response() does not overwrite $ldapcookie if it fails, by + // setting this to null we avoid an infinite loop. + if ($pagedresp === false) { + $ldapcookie = null; + } + } else { + // Get next cookie from controls. + ldap_parse_result($ldapconnection, $ldapresult, $errcode, $matcheddn, + $errmsg, $referrals, $controls); + if (isset($controls[LDAP_CONTROL_PAGEDRESULTS]['value']['cookie'])) { + $ldapcookie = $controls[LDAP_CONTROL_PAGEDRESULTS]['value']['cookie']; + } } } if ($entry = @ldap_first_entry($ldapconnection, $ldapresult)) { @@ -1504,6 +1539,7 @@ class auth_plugin_ldap extends auth_plugin_base { if ($filter == '*') { $filter = '(&('.$this->config->user_attribute.'=*)'.$this->config->objectclass.')'; } + $servercontrols = array(); $contexts = explode(';', $this->config->contexts); if (!empty($this->config->create_context)) { @@ -1520,20 +1556,54 @@ class auth_plugin_ldap extends auth_plugin_base { do { if ($ldap_pagedresults) { - ldap_control_paged_result($ldapconnection, $this->config->pagesize, true, $ldap_cookie); + // TODO: Remove the old branch of code once PHP 7.3.0 becomes required (Moodle 4.1). + if (version_compare(PHP_VERSION, '7.3.0', '<')) { + // Before 7.3, use this function that was deprecated in PHP 7.4. + ldap_control_paged_result($ldapconnection, $this->config->pagesize, true, $ldap_cookie); + } else { + // PHP 7.3 and up, use server controls. + $servercontrols = array(array( + 'oid' => LDAP_CONTROL_PAGEDRESULTS, 'value' => array( + 'size' => $this->config->pagesize, 'cookie' => $ldap_cookie))); + } } if ($this->config->search_sub) { // Use ldap_search to find first user from subtree. - $ldap_result = ldap_search($ldapconnection, $context, $filter, array($this->config->user_attribute)); + // TODO: Remove the old branch of code once PHP 7.3.0 becomes required (Moodle 4.1). + if (version_compare(PHP_VERSION, '7.3.0', '<')) { + $ldap_result = ldap_search($ldapconnection, $context, $filter, array($this->config->user_attribute)); + } else { + $ldap_result = ldap_search($ldapconnection, $context, $filter, array($this->config->user_attribute), + 0, -1, -1, LDAP_DEREF_NEVER, $servercontrols); + } } else { // Search only in this context. - $ldap_result = ldap_list($ldapconnection, $context, $filter, array($this->config->user_attribute)); + // TODO: Remove the old branch of code once PHP 7.3.0 becomes required (Moodle 4.1). + if (version_compare(PHP_VERSION, '7.3.0', '<')) { + $ldap_result = ldap_list($ldapconnection, $context, $filter, array($this->config->user_attribute)); + } else { + $ldap_result = ldap_list($ldapconnection, $context, $filter, array($this->config->user_attribute), + 0, -1, -1, LDAP_DEREF_NEVER, $servercontrols); + } } if(!$ldap_result) { continue; } if ($ldap_pagedresults) { - ldap_control_paged_result_response($ldapconnection, $ldap_result, $ldap_cookie); + // Get next server cookie to know if we'll need to continue searching. + $ldap_cookie = ''; + // TODO: Remove the old branch of code once PHP 7.3.0 becomes required (Moodle 4.1). + if (version_compare(PHP_VERSION, '7.3.0', '<')) { + // Before 7.3, use this function that was deprecated in PHP 7.4. + ldap_control_paged_result_response($ldapconnection, $ldap_result, $ldap_cookie); + } else { + // Get next cookie from controls. + ldap_parse_result($ldapconnection, $ldap_result, $errcode, $matcheddn, + $errmsg, $referrals, $controls); + if (isset($controls[LDAP_CONTROL_PAGEDRESULTS]['value']['cookie'])) { + $ldap_cookie = $controls[LDAP_CONTROL_PAGEDRESULTS]['value']['cookie']; + } + } } $users = ldap_get_entries_moodle($ldapconnection, $ldap_result); // Add found users to list. From 19e3cdbad9bc85c8f286188b26b362f5138c62f0 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Wed, 27 Nov 2019 10:51:16 +0100 Subject: [PATCH 2/4] MDL-67118 auth_ldap: add pagesize/subcontexts test provider With that provider we'll be explicitly covering that any pagesize and subcontexts search is returning results consistently so the internal paging/servercontrols work as expected. --- auth/ldap/tests/plugin_test.php | 35 ++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/auth/ldap/tests/plugin_test.php b/auth/ldap/tests/plugin_test.php index f746583e6b1..17ee9dd9b26 100644 --- a/auth/ldap/tests/plugin_test.php +++ b/auth/ldap/tests/plugin_test.php @@ -36,7 +36,36 @@ defined('MOODLE_INTERNAL') || die(); class auth_ldap_plugin_testcase extends advanced_testcase { - public function test_auth_ldap() { + /** + * Data provider for auth_ldap tests + * + * Used to ensure that all the paged stuff works properly, irrespectively + * of the pagesize configured (that implies all the chunking and paging + * built in the plugis is doing its work consistently). Both searching and + * not searching within subcontexts. + * + * @return array[] + */ + public function auth_ldap_provider() { + $pagesizes = [1, 3, 5, 1000]; + $subcontexts = [0, 1]; + $combinations = []; + foreach ($pagesizes as $pagesize) { + foreach ($subcontexts as $subcontext) { + $combinations["pagesize {$pagesize}, subcontexts {$subcontext}"] = [$pagesize, $subcontext]; + } + } + return $combinations; + } + + /** + * General auth_ldap testcase + * + * @dataProvider auth_ldap_provider + * @param int $pagesize Value to be configured in settings controlling page size. + * @param int $subcontext Value to be configured in settings controlling searching in subcontexts. + */ + public function test_auth_ldap(int $pagesize, int $subcontext) { global $CFG, $DB; if (!extension_loaded('ldap')) { @@ -100,12 +129,12 @@ class auth_ldap_plugin_testcase extends advanced_testcase { set_config('start_tls', 0, 'auth_ldap'); set_config('ldap_version', 3, 'auth_ldap'); set_config('ldapencoding', 'utf-8', 'auth_ldap'); - set_config('pagesize', '2', 'auth_ldap'); + set_config('pagesize', $pagesize, 'auth_ldap'); set_config('bind_dn', TEST_AUTH_LDAP_BIND_DN, 'auth_ldap'); set_config('bind_pw', TEST_AUTH_LDAP_BIND_PW, 'auth_ldap'); set_config('user_type', 'rfc2307', 'auth_ldap'); set_config('contexts', 'ou=users,'.$topdn, 'auth_ldap'); - set_config('search_sub', 0, 'auth_ldap'); + set_config('search_sub', $subcontext, 'auth_ldap'); set_config('opt_deref', LDAP_DEREF_NEVER, 'auth_ldap'); set_config('user_attribute', 'cn', 'auth_ldap'); set_config('memberattribute', 'memberuid', 'auth_ldap'); From f8b9cc01c3e92aa3fe7f2b227ec18b4e5f5c9577 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Wed, 27 Nov 2019 23:17:20 +0100 Subject: [PATCH 3/4] MDL-67118 enrol_ldap: paged results functions deprecated php74 and up Starting with php74 the following functions are deprecated: - ldap_control_paged_result() - ldap_control_paged_result_response() Starting with php73, ldap servercontrols were included. One of those servercontrols, LDAP_CONTROL_PAGEDRESULTS, is the one in charge of controlling paged results. So, we are going to add some conditional code here: 1) if php < 7.3, use old paged result functions. 2) if php >= 7.3, switch to LDAP_CONTROL_PAGEDRESULTS servercontrol. With a TODO about removing 1) in Moodle 4.1, once php73 becomes required. --- enrol/ldap/lib.php | 106 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 86 insertions(+), 20 deletions(-) diff --git a/enrol/ldap/lib.php b/enrol/ldap/lib.php index 4d37457bfbd..8c4672dc865 100644 --- a/enrol/ldap/lib.php +++ b/enrol/ldap/lib.php @@ -379,6 +379,7 @@ class enrol_ldap_plugin extends enrol_plugin { } $ldap_cookie = ''; + $servercontrols = array(); foreach ($ldap_contexts as $ldap_context) { $ldap_context = trim($ldap_context); if (empty($ldap_context)) { @@ -388,28 +389,60 @@ class enrol_ldap_plugin extends enrol_plugin { $flat_records = array(); do { if ($ldap_pagedresults) { - ldap_control_paged_result($this->ldapconnection, $this->config->pagesize, true, $ldap_cookie); + // TODO: Remove the old branch of code once PHP 7.3.0 becomes required (Moodle 4.1). + if (version_compare(PHP_VERSION, '7.3.0', '<')) { + // Before 7.3, use this function that was deprecated in PHP 7.4. + ldap_control_paged_result($this->ldapconnection, $this->config->pagesize, true, $ldap_cookie); + } else { + // PHP 7.3 and up, use server controls. + $servercontrols = array(array( + 'oid' => LDAP_CONTROL_PAGEDRESULTS, 'value' => array( + 'size' => $this->config->pagesize, 'cookie' => $ldap_cookie))); + } } if ($this->config->course_search_sub) { // Use ldap_search to find first user from subtree - $ldap_result = @ldap_search($this->ldapconnection, - $ldap_context, - $ldap_search_pattern, - $ldap_fields_wanted); + // TODO: Remove the old branch of code once PHP 7.3.0 becomes required (Moodle 4.1). + if (version_compare(PHP_VERSION, '7.3.0', '<')) { + $ldap_result = @ldap_search($this->ldapconnection, $ldap_context, + $ldap_search_pattern, $ldap_fields_wanted); + } else { + $ldap_result = @ldap_search($this->ldapconnection, $ldap_context, + $ldap_search_pattern, $ldap_fields_wanted, + 0, -1, -1, LDAP_DEREF_NEVER, $servercontrols); + } } else { // Search only in this context - $ldap_result = @ldap_list($this->ldapconnection, - $ldap_context, - $ldap_search_pattern, - $ldap_fields_wanted); + // TODO: Remove the old branch of code once PHP 7.3.0 becomes required (Moodle 4.1). + if (version_compare(PHP_VERSION, '7.3.0', '<')) { + $ldap_result = @ldap_list($this->ldapconnection, $ldap_context, + $ldap_search_pattern, $ldap_fields_wanted); + } else { + $ldap_result = @ldap_list($this->ldapconnection, $ldap_context, + $ldap_search_pattern, $ldap_fields_wanted, + 0, -1, -1, LDAP_DEREF_NEVER, $servercontrols); + } } if (!$ldap_result) { continue; // Next } if ($ldap_pagedresults) { - ldap_control_paged_result_response($this->ldapconnection, $ldap_result, $ldap_cookie); + // Get next server cookie to know if we'll need to continue searching. + $ldap_cookie = ''; + // TODO: Remove the old branch of code once PHP 7.3.0 becomes required (Moodle 4.1). + if (version_compare(PHP_VERSION, '7.3.0', '<')) { + // Before 7.3, use this function that was deprecated in PHP 7.4. + ldap_control_paged_result_response($this->ldapconnection, $ldap_result, $ldap_cookie); + } else { + // Get next cookie from controls. + ldap_parse_result($this->ldapconnection, $ldap_result, $errcode, $matcheddn, + $errmsg, $referrals, $controls); + if (isset($controls[LDAP_CONTROL_PAGEDRESULTS]['value']['cookie'])) { + $ldap_cookie = $controls[LDAP_CONTROL_PAGEDRESULTS]['value']['cookie']; + } + } } // Check and push results @@ -769,24 +802,44 @@ class enrol_ldap_plugin extends enrol_plugin { } $ldap_cookie = ''; + $servercontrols = array(); $flat_records = array(); do { if ($ldap_pagedresults) { - ldap_control_paged_result($this->ldapconnection, $this->config->pagesize, true, $ldap_cookie); + // TODO: Remove the old branch of code once PHP 7.3.0 becomes required (Moodle 4.1). + if (version_compare(PHP_VERSION, '7.3.0', '<')) { + // Before 7.3, use this function that was deprecated in PHP 7.4. + ldap_control_paged_result($this->ldapconnection, $this->config->pagesize, true, $ldap_cookie); + } else { + // PHP 7.3 and up, use server controls. + $servercontrols = array(array( + 'oid' => LDAP_CONTROL_PAGEDRESULTS, 'value' => array( + 'size' => $this->config->pagesize, 'cookie' => $ldap_cookie))); + } } if ($this->get_config('course_search_sub')) { // Use ldap_search to find first user from subtree - $ldap_result = @ldap_search($this->ldapconnection, - $context, - $ldap_search_pattern, - $ldap_fields_wanted); + // TODO: Remove the old branch of code once PHP 7.3.0 becomes required (Moodle 4.1). + if (version_compare(PHP_VERSION, '7.3.0', '<')) { + $ldap_result = @ldap_search($this->ldapconnection, $context, + $ldap_search_pattern, $ldap_fields_wanted); + } else { + $ldap_result = @ldap_search($this->ldapconnection, $context, + $ldap_search_pattern, $ldap_fields_wanted, + 0, -1, -1, LDAP_DEREF_NEVER, $servercontrols); + } } else { // Search only in this context - $ldap_result = @ldap_list($this->ldapconnection, - $context, - $ldap_search_pattern, - $ldap_fields_wanted); + // TODO: Remove the old branch of code once PHP 7.3.0 becomes required (Moodle 4.1). + if (version_compare(PHP_VERSION, '7.3.0', '<')) { + $ldap_result = @ldap_list($this->ldapconnection, $context, + $ldap_search_pattern, $ldap_fields_wanted); + } else { + $ldap_result = @ldap_list($this->ldapconnection, $context, + $ldap_search_pattern, $ldap_fields_wanted, + 0, -1, -1, LDAP_DEREF_NEVER, $servercontrols); + } } if (!$ldap_result) { @@ -794,7 +847,20 @@ class enrol_ldap_plugin extends enrol_plugin { } if ($ldap_pagedresults) { - ldap_control_paged_result_response($this->ldapconnection, $ldap_result, $ldap_cookie); + // Get next server cookie to know if we'll need to continue searching. + $ldap_cookie = ''; + // TODO: Remove the old branch of code once PHP 7.3.0 becomes required (Moodle 4.1). + if (version_compare(PHP_VERSION, '7.3.0', '<')) { + // Before 7.3, use this function that was deprecated in PHP 7.4. + ldap_control_paged_result_response($this->ldapconnection, $ldap_result, $ldap_cookie); + } else { + // Get next cookie from controls. + ldap_parse_result($this->ldapconnection, $ldap_result, $errcode, $matcheddn, + $errmsg, $referrals, $controls); + if (isset($controls[LDAP_CONTROL_PAGEDRESULTS]['value']['cookie'])) { + $ldap_cookie = $controls[LDAP_CONTROL_PAGEDRESULTS]['value']['cookie']; + } + } } // Check and push results. ldap_get_entries() already From 86362a7fb2faa1f370058d84d92eef92fbdafb42 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Wed, 27 Nov 2019 23:40:09 +0100 Subject: [PATCH 4/4] MDL-67118 enrol_ldap: add pagesize/subcontexts provider With that provider we'll be explicitly checking that any pagesize and subcontexts search is returning results consistently so the internal paging/servercontrols work as expected. --- enrol/ldap/tests/ldap_test.php | 35 +++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/enrol/ldap/tests/ldap_test.php b/enrol/ldap/tests/ldap_test.php index 9ac6277ab4d..9893146415f 100644 --- a/enrol/ldap/tests/ldap_test.php +++ b/enrol/ldap/tests/ldap_test.php @@ -39,7 +39,36 @@ global $CFG; class enrol_ldap_testcase extends advanced_testcase { - public function test_enrol_ldap() { + /** + * Data provider for enrol_ldap tests + * + * Used to ensure that all the paged stuff works properly, irrespectively + * of the pagesize configured (that implies all the chunking and paging + * built in the plugis is doing its work consistently). Both searching and + * not searching within subcontexts. + * + * @return array[] + */ + public function enrol_ldap_provider() { + $pagesizes = [1, 3, 5, 1000]; + $subcontexts = [0, 1]; + $combinations = []; + foreach ($pagesizes as $pagesize) { + foreach ($subcontexts as $subcontext) { + $combinations["pagesize {$pagesize}, subcontexts {$subcontext}"] = [$pagesize, $subcontext]; + } + } + return $combinations; + } + + /** + * General enrol_ldap testcase + * + * @dataProvider enrol_ldap_provider + * @param int $pagesize Value to be configured in settings controlling page size. + * @param int $subcontext Value to be configured in settings controlling searching in subcontexts. + */ + public function test_enrol_ldap(int $pagesize, int $subcontext) { global $CFG, $DB; if (!extension_loaded('ldap')) { @@ -83,10 +112,10 @@ class enrol_ldap_testcase extends advanced_testcase { $enrol->set_config('start_tls', 0); $enrol->set_config('ldap_version', 3); $enrol->set_config('ldapencoding', 'utf-8'); - $enrol->set_config('pagesize', '2'); + $enrol->set_config('pagesize', $pagesize); $enrol->set_config('bind_dn', TEST_ENROL_LDAP_BIND_DN); $enrol->set_config('bind_pw', TEST_ENROL_LDAP_BIND_PW); - $enrol->set_config('course_search_sub', 0); + $enrol->set_config('course_search_sub', $subcontext); $enrol->set_config('memberattribute_isdn', 0); $enrol->set_config('user_contexts', ''); $enrol->set_config('user_search_sub', 0);