Skip to content

Commit

Permalink
auth/ldap MDL-23652 Error in auth_ldap_sync_users.php
Browse files Browse the repository at this point in the history
We need to specify a valid user id in the call to role_assign(). And we only
have to make the call if the user has been added successfully, not
otherwise.

Also make sure we lowercase the memberuser and group distinguished names
before comparing them. Depending on the LDAP server we can get mixed case
values for the DNs, and the user may have specified the creators group/ou
name in a different case.

By the way, this has been broken for ages (since the auth cleanup in 1.8, in
2007!). It's a bit strange nobody noticed before :-O

Credit goes to Joe Chryst.
  • Loading branch information
iarenaza committed Aug 7, 2010
1 parent a789ac6 commit 3e5f4b8
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 5 deletions.
9 changes: 5 additions & 4 deletions auth/ldap/auth.php
Original file line number Diff line number Diff line change
Expand Up @@ -866,14 +866,15 @@ function sync_users($do_updates=true) {
if (!empty($this->config->forcechangepassword)) {
set_user_preference('auth_forcepasswordchange', 1, $id);
}

// Add course creators if needed
if ($creatorrole !== false and $this->iscreator($user->username)) {
role_assign($creatorrole->id, $id, $sitecontext->id, $this->roleauth);
}
} else {
echo "\t"; print_string('auth_dbinsertusererror', 'auth_db', $user->username); echo "\n";
}

// Add course creators if needed
if ($creatorrole !== false and $this->iscreator($user->username)) {
role_assign($creatorrole->id, $user->id, $sitecontext->id, $this->roleauth);
}
}
$transaction->allow_commit();
unset($add_users); // free mem
Expand Down
2 changes: 1 addition & 1 deletion lib/ldaplib.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ function ldap_isgroupmember($ldapconnection, $userid, $group_dns, $member_attrib
// Check cheaply if the user's DN sits in a subtree of the
// "group" DN provided. Granted, this isn't a proper LDAP
// group, but it's a popular usage.
if (stripos(strrev($userid), strrev($group)) === 0) {
if (stripos(strrev(strtolower($userid)), strrev(strtolower($group))) === 0) {
$result = true;
break;
}
Expand Down

0 comments on commit 3e5f4b8

Please sign in to comment.