From 9f451dbcfc92f803b360a63d88cdc48fca835c20 Mon Sep 17 00:00:00 2001 From: Charles Fulton Date: Thu, 3 May 2018 16:11:34 -0400 Subject: [PATCH] Filter ldap attributes correctly; fixes #9 --- locallib.php | 6 +++--- tests/sync_test.php | 32 +++++++++++++++++++++++--------- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/locallib.php b/locallib.php index 27c6a62..c80396f 100644 --- a/locallib.php +++ b/locallib.php @@ -178,7 +178,7 @@ private function ldap_get_group_members_rfc($group) { return $ret; } - $queryg = "(&({$this->config->group_attribute}=" . trim($group) . ")(objectClass={$this->config->group_class}))"; + $queryg = "(&({$this->config->group_attribute}=" . ldap_filter_addslashes(trim($group)) . ")(objectClass={$this->config->group_class}))"; if (!empty($CFG->cohort_synching_ldap_groups_contexts)) { $contexts = explode(';', $CFG->cohort_synching_ldap_groups_contexts); @@ -270,7 +270,7 @@ private function ldap_get_group_members_ad($group) { $group = core_text::convert($group, 'utf-8', $this->config->ldapencoding); - $queryg = "(&({$this->config->group_attribute}=" . trim($group) . ")(objectClass={$this->config->group_class}))"; + $queryg = "(&({$this->config->group_attribute}=" . ldap_filter_addslashes(trim($group)) . ")(objectClass={$this->config->group_class}))"; $size = 999; @@ -513,7 +513,7 @@ public function get_users_having_attribute_value($attributevalue) { // Build a filter. $filter = '(&('.$this->config->user_attribute.'=*)'.$this->config->objectclass.')'; $filter = '(&'.$filter.'('.$this->config->cohort_synching_ldap_attribute_attribute. - '='.ldap_addslashes($attributevalue).'))'; + '='.ldap_filter_addslashes($attributevalue).'))'; // Call Moodle ldap_get_userlist that return it as an array with Moodle user attributes names. $matchings = $this->ldap_get_userlist($filter); diff --git a/tests/sync_test.php b/tests/sync_test.php index 02c8b2f..82c259b 100644 --- a/tests/sync_test.php +++ b/tests/sync_test.php @@ -79,7 +79,7 @@ public function test_cohort_group_sync() { $o['objectClass'] = array('organizationalUnit'); $o['ou'] = 'groups'; ldap_add($connection, 'ou='.$o['ou'].','.$topdn, $o); - $departments = array('english', 'history'); + $departments = array('english', 'history', 'english(bis)'); foreach ($departments as $department) { $o = array(); $o['objectClass'] = array('groupOfNames'); @@ -144,14 +144,21 @@ public function test_cohort_group_sync() { $cohort->name = "English Department"; $cohort->idnumber = 'english'; $englishid = cohort_add_cohort($cohort); + $cohort = new stdClass(); + $cohort->contextid = context_system::instance()->id; + $cohort->name = "English Department (bis)"; + $cohort->idnumber = 'english(bis)'; + $englishbisid = cohort_add_cohort($cohort); - // Both cohorts should have three members. + // All three cohorts should have three members. $plugin = new local_ldap(); $plugin->sync_cohorts_by_group(); $members = $DB->count_records('cohort_members', array('cohortid' => $historyid)); $this->assertEquals(3, $members); $members = $DB->count_records('cohort_members', array('cohortid' => $englishid)); $this->assertEquals(3, $members); + $members = $DB->count_records('cohort_members', array('cohortid' => $englishbisid)); + $this->assertEquals(3, $members); // Remove a user and then ensure he's re-added. $members = $plugin->get_cohort_members($englishid); @@ -243,7 +250,7 @@ public function test_cohort_attribute_sync() { ldap_mod_add($connection, "cn=username4,ou=users,$topdn", array('eduPersonAffiliation' => 'staff')); ldap_mod_add($connection, "cn=username5,ou=users,$topdn", - array('eduPersonAffiliation' => 'staff')); + array('eduPersonAffiliation' => 'staff(pt)')); // Configure the authentication plugin a bit. set_config('host_url', TEST_AUTH_LDAP_HOST_URL, 'auth_ldap'); @@ -300,23 +307,30 @@ public function test_cohort_attribute_sync() { $cohort->name = "All staff"; $cohort->idnumber = 'staff'; $staffid = cohort_add_cohort($cohort); + $cohort = new stdClass(); + $cohort->contextid = context_system::instance()->id; + $cohort->name = "All staff (pt)"; + $cohort->idnumber = 'staff(pt)'; + $staffptid = cohort_add_cohort($cohort); - // Faculty should have two members and staff should have three. + // Faculty and staff should have two members and staff(pt) should have one. $plugin = new local_ldap(); $plugin->sync_cohorts_by_attribute(); $members = $DB->count_records('cohort_members', array('cohortid' => $facultyid)); $this->assertEquals(2, $members); $members = $DB->count_records('cohort_members', array('cohortid' => $staffid)); - $this->assertEquals(3, $members); + $this->assertEquals(2, $members); + $members = $DB->count_records('cohort_members', array('cohortid' => $staffptid)); + $this->assertEquals(1, $members); // Remove a user and then ensure he's re-added. $members = $plugin->get_cohort_members($staffid); cohort_remove_member($staffid, current($members)->id); $members = $DB->count_records('cohort_members', array('cohortid' => $staffid)); - $this->assertEquals(2, $members); + $this->assertEquals(1, $members); $plugin->sync_cohorts_by_attribute(); $members = $DB->count_records('cohort_members', array('cohortid' => $staffid)); - $this->assertEquals(3, $members); + $this->assertEquals(2, $members); // Add an affiliation in LDAP and ensure he'd added. ldap_mod_add($connection, "cn=username3,ou=users,$topdn", @@ -331,10 +345,10 @@ public function test_cohort_attribute_sync() { ldap_mod_del($connection, "cn=username3,ou=users,$topdn", array('eduPersonAffiliation' => 'staff')); $members = $DB->count_records('cohort_members', array('cohortid' => $staffid)); - $this->assertEquals(3, $members); + $this->assertEquals(2, $members); $plugin->sync_cohorts_by_attribute(); $members = $DB->count_records('cohort_members', array('cohortid' => $staffid)); - $this->assertEquals(2, $members); + $this->assertEquals(1, $members); // Cleanup. $this->recursive_delete($connection, TEST_AUTH_LDAP_DOMAIN, 'dc=moodletest');