Moodle

Also, this error occurs for ANY user with authentication set to LDAP, not just the users who belong to the course creators group.

debuging this now - I'm off for the day, but will look at it again tomorrow sometime - if someone else gets to this b4 me - these errors on login might be useful also:

Notice: Undefined property: stdClass::$auth_ntlm_stdchangepassword in E:\learn\moodleweb\learn17\login\index.php on line 147
Notice: Undefined property: stdClass::$enrol_ldap_contexts_role1 in E:\learn\moodleweb\learn17\enrol\ldap\enrol.php on line 495
Notice: Undefined property: stdClass::$enrol_ldap_memberattribute_role1 in E:\learn\moodleweb\learn17\enrol\ldap\enrol.php on line 511
Notice: Undefined property: stdClass::$enrol_ldap_contexts_role2 in E:\learn\moodleweb\learn17\enrol\ldap\enrol.php on line 495
Notice: Undefined property: stdClass::$enrol_ldap_memberattribute_role2 in E:\learn\moodleweb\learn17\enrol\ldap\enrol.php on line 511
Notice: Undefined property: stdClass::$enrol_ldap_contexts_role3 in E:\learn\moodleweb\learn17\enrol\ldap\enrol.php on line 495
Notice: Undefined property: stdClass::$enrol_ldap_memberattribute_role3 in E:\learn\moodleweb\learn17\enrol\ldap\enrol.php on line 511
Warning: ldap_list() [function.ldap-list]: Search: Bad search filter in E:\learn\moodleweb\learn17\enrol\ldap\enrol.php on line 534
Warning: ldap_get_entries(): supplied argument is not a valid ldap result resource in E:\learn\moodleweb\learn17\enrol\ldap\enrol.php on line 538

+ a few others - comment out line 213 in login\index.php to view them:
//redirect($urltogo);

problem Matt mentions above is with user_coursecreators not existing and still being used in Core code - moodlelib.php lines 2385-2403

if (function_exists('auth_iscreator')) { // Check if the user is a creator
$useriscreator = auth_iscreator($username);
if (!is_null($useriscreator)) {
if ($useriscreator) {
if (! record_exists('user_coursecreators', 'userid', $user->id)) {
$cdata->userid = $user->id;
if (! insert_record('user_coursecreators', $cdata)) { error('Cannot add user to course creators.'); }
}
} else {
if (record_exists('user_coursecreators', 'userid', $user->id)) {
if (! delete_records('user_coursecreators', 'userid', $user->id)) { error('Cannot remove user from course creators.'); }
}
}
}
}

my first hack at replacing the above function - NEEDS TESTING!!!

$sitecontext = get_context_instance(CONTEXT_SYSTEM);

if ($creatorroles = get_roles_with_capability('moodle/legacy:coursecreator', CAP_ALLOW)) {

$creatorrole = array_shift($creatorroles); // We can only use one, let's use the first one

if (function_exists('auth_iscreator')) { // Check if the user is a creator
if (auth_iscreator($username)) { // Following calls will not create duplicates
if ($useriscreator) { role_assign($creatorrole->id, $user->id, 0, $sitecontext->id, 0, 0, 0, 'ldap'); } else { role_unassign($creatorrole->id, $user->id, 0, $sitecontext->id); }
}
}
}

Testing it now. I'll get back to you later.

Hi,

I have committed your fix (the ordering of if clauses are a bit different) into HEAD and 1.7, so that you can test this easier =)

Yu

Hi,

Can anyone confirm this patch is working / not working? =)

Thanks,

Yu

Hello,

This patch resolves the issue for me.

Thanks,

Mark

I'm not able to assign the course creators role - will you comfirm if the patch properly creates course creators if ldap_creators is filled in?

Looks like the patch in CVS is crashing Moodle - at least on my end! Lines 2385-2422 in /lib/moodlelib.php -

If you look at the comment marks, I think it needs to be changed to this:

/*<<<<<<< moodlelib.php
$sitecontext = get_context_instance(CONTEXT_SYSTEM);

if ($creatorroles = get_roles_with_capability('moodle/legacy:coursecreator', CAP_ALLOW)) {

$creatorrole = array_shift($creatorroles); // We can only use one, let's use the first one

if (function_exists('auth_iscreator')) { // Check if the user is a creator
if (auth_iscreator($username)) { // Following calls will not create duplicates
if ($useriscreator) { role_assign($creatorrole->id, $user->id, 0, $sitecontext->id, 0, 0, 0, 'ldap'); } else {
role_unassign($creatorrole->id, $user->id, 0, $sitecontext->id);
=======
*/
if (function_exists('auth_iscreator')) { // Check if the user is a creator
$useriscreator = auth_iscreator($username);
if (!is_null($useriscreator)) {
if ($useriscreator) {
if (! record_exists('user_coursecreators', 'userid', $user->id)) {
$cdata->userid = $user->id;
if (! insert_record('user_coursecreators', $cdata)) { error('Cannot add user to course creators.'); }
}
} else {
if (record_exists('user_coursecreators', 'userid', $user->id)) {
if (! delete_records('user_coursecreators', 'userid', $user->id)) { error('Cannot remove user from course creators.'); }
}
}
}
}

// fix for MDL-6928

Hi Matt - If I'm reading your above comment - you're suggesting reverting back to the old code that caused this initial problem
user_coursecreators does not exist in 1.7 due to the new roles.

as to the patch - it removes the critical error of not being able to log in when that field is filled in, however I don't think the patch adds/removes course creators correctly - I'm also unsure about this line:
role_assign($creatorrole->id, $user->id, 0, $sitecontext->id, 0, 0, 0, 'ldap');

inserting the value 'ldap' at the end - is this valid? - this function 'could' be called by any auth plugin right? - I don't know enough about the roles to know whether that value matters or not.....

-I've tried testing the course creators addition/removal on my install and I don't think it works, but I've never used the course creators field before, so I might be doing something strange......

Dan

No, not reverting back at all, just cleaning up where the comment marks are. I just wasn't reading it well at all. The patch as it was placed into CVS had some of the patch information (<<<<<<< moodlelib.php and ======= that weren't commented out and it was throwing an error. That's what needs to be cleaned up and we need to work with the patch you submitted. I think I've got it right below but it would be best if you'd double-check it.

I agree that course creators are not being added or removed correctly, but I don't have the answer for it.

In /lib/moodlelib.php, lines 2385-2421 should read:

/* if (function_exists('auth_iscreator')) { // Check if the user is a creator
$useriscreator = auth_iscreator($username);
if (!is_null($useriscreator)) {
if ($useriscreator) {
if (! record_exists('user_coursecreators', 'userid', $user->id)) {
$cdata->userid = $user->id;
if (! insert_record('user_coursecreators', $cdata)) { error('Cannot add user to course creators.'); }
}
} else {
if (record_exists('user_coursecreators', 'userid', $user->id)) {
if (! delete_records('user_coursecreators', 'userid', $user->id)) { error('Cannot remove user from course creators.'); }
}
}
}
}
*/
// fix for MDL-6928

if (function_exists('auth_iscreator')) {
$sitecontext = get_context_instance(CONTEXT_SYSTEM);
if ($creatorroles = get_roles_with_capability('moodle/legacy:coursecreator', CAP_ALLOW)) {
$creatorrole = array_shift($creatorroles); // We can only use one, let's use the first one
// Check if the user is a creator
if (auth_iscreator($username)) { // Following calls will not create duplicates
if ($useriscreator) { role_assign($creatorrole->id, $user->id, 0, $sitecontext->id, 0, 0, 0, 'ldap'); } else { role_unassign($creatorrole->id, $user->id, 0, $sitecontext->id); }
}
}
}

Hi matt,

It seems like you have some conflict markers (the <<<<< ===== >>>>>), probably because you modified moodlelib.php youself before you did the cvs update. If you delete the copy of moodlelib.php you have and do a cvs update, then you should have the correct lines.

Cheers,

Yu

Hi,

I am resolving this bug as fixed. Please reopen if the problem persists.

Yu

Hi Yu,

I don't think the patch is complete! - did you see my comments above:

***
as to the patch - it removes the critical error of not being able to log in when that field is filled in, however I don't think the patch adds/removes course creators correctly - I'm also unsure about this line:
role_assign($creatorrole->id, $user->id, 0, $sitecontext->id, 0, 0, 0, 'ldap');

inserting the value 'ldap' at the end - is this valid? - this function 'could' be called by any auth plugin right? - I don't know enough about the roles to know whether that value matters or not.....
****

looks like someone else has also posted a partial fix for the existing patch:

(I haven't tested/read it yet)

see here:
http://moodle.org/mod/forum/discuss.php?d=57590

Hi Dan,

Thanks for poiting that out. The old code was buggy indeed. Fixed according to the Gisela Hillenbrand's post, please let me know if it works.

role_assign() could be called by anything, that's right. The last parameter is the enrolment type.

Cheers,

Yu

Hi Yu - still not fixed completely!

• 'ldap' is still hardcoded in the role_assign()

Dan

Hi Dan,

Sorry I don't know this part too well, but isn't auth_iscreator() LDAP only?

Yu

Hi Yu,

At this stage, I think you're right, but there might be third party plugins that make use of the same code.... - might be one to flag as won't fix at this stage if it's too difficult to get the Auth Type being used in context.

Dan

Hi Dan,

Changed to $auth just in case, thanks!!

Yu

This code segment still is not working correctly. With 1.7, we have added permission for "moodle/course:create" to the standard the "editing teacher" role, but this code always deletes the permission. The same user has no other role which would cause a duplication, which is what this code is supposed to remove.

if ($useriscreator) { role_assign($creatorrole->id, $user->id, 0, $sitecontext->id, 0, 0, 0, 'ldap'); } else { role_unassign($creatorrole->id, $user->id, 0, $sitecontext->id); }

Furthermore, I don't see the relevance of the authentication method to this function. Why is it necessary?