Skip to content

Commit

Permalink
Filter ldap attributes correctly; fixes patrickpollet#9
Browse files Browse the repository at this point in the history
  • Loading branch information
@mackensen
mackensen committed May 4, 2018
1 parent be94591 commit 9f451db
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 12 deletions.
6 changes: 3 additions & 3 deletions locallib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down
32 changes: 23 additions & 9 deletions tests/sync_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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",
Expand All @@ -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');
Expand Down

0 comments on commit 9f451db

Please sign in to comment.