Moodle

Hi,

I would treat this bug with a much higher priority than "Minor" - IMO it should be a Blocker for 1.7.1 since all Moodle < 1.7 installations using an external DB for user authentication will no longer work at all when updated to 1.7 - logins will no more be possible and the only solution (without the fix) is to revert back (you will lock out yourself).

rob.

I hit this same problem with Moodle 1.7+ (2006101009).

This bug arose in November 2006 when somebody added the line
$authdb->SetFetchMode(ADODB_FETCH_ASSOC); ///Set Assoc mode always after DB connection
to the function auth_get_userinfo()
see:
http://moodle.cvs.sourceforge.net/moodle/moodle/auth/db/lib.php?hideattic=0&r1=1.26&r2=1.27

The code which gets the data out of the result of the query is clearly expecting the query to have been done in none-assoc mode:
$result["$field"] = addslashes(stripslashes($rs->fields[0]));

This bug persists in the new auth/db/auth.php file which replaces lib.php.

I think that the best fix is to remove the $authdb->SetFetchMode(ADODB_FETCH_ASSOC) line. The code will then work even if the configured fieldnames contain SQL statements. I suspect that the following sort of thing may be in common use as a field name for those of us authenticating against mail server databases:

concat(pw_name,char(64),pw_domain) as email

The fix mentioned in the first comment on this issue will only work where the fieldnames are real fieldnames in the database, whereas returning a non associative array would allow administrators to be a little more creative.

Phil

Hi Phil! We don't want to go back on our ADODB_FETCH_ASSOC to ADODB_FETCH_BOTH really. The fix is to name the field and stop relying on the position of things. I've committed a fix to MOODLE_17_STABLE and to HEAD, and I'd appreciate your help testing it and confirming whether it's resolved the problem for you.

With this fix you can do a bit of SQL within the field, but I don't recommend that. Instead, create a view on the SQL server, and select from that view.

Martin,

thanks for the fix - I can confirm that the MOODLE_17_STABLE commit for 'auth/db/lib.php' (v1.25.2.2) solved the problem that no logins have been possible.

Great to hear that! Resolved then.

Sorry Martin, this breaks four of my six Moodle installations. Surely we should be attempting to fix this in a way that provides backwards compatibility. Whilst your suggestion about creating a view on the SQL server would work if I was in full control of the external database, I, like many others I would imagine, have only got read access to the external database, and only managed to get that after much wrangling!

How about this for a ADODB_FETCH_ASSOC compatible fix which at least makes it possible to do some SQL in the fieldname:

if ($rs = $authdb->Execute("SELECT (".$pcfg["field_map_$field"].") AS moodle_user_field FROM $CFG->auth_dbtable
WHERE $CFG->auth_dbfielduser = '$username'")) {
if ( $rs->RecordCount() == 1 ) {
if (!empty($CFG->unicodedb)) { $result["$field"] = addslashes(stripslashes($rs->fields['moodle_user_field'])); } else { $result["$field"] = addslashes(stripslashes(utf8_decode($rs->fields[;'moodle_user_field']))); }
}
}

Excellent that this fix is now also available within the moodle-latest-17.tgz package, as of moodle-20070116.tgz. I am just wondering why the Resolved status of this ticket says to fix Moodle 1.7.2. Does this mean that once 1.7.1 is released it will still include the buggy versions, and if we update our 1.7+ installations we will downgrade already fixed working files to unfixed non-working ones?

Phil, I think you might have been testing older code. I crafted the patch to still support funny SQL tricks in there – and it looks almost exactly like yours (yours has a typo however! there's a stray semicolon...).

If in doubt, fetch it directly from the latest CVS file here: http://moodle.cvs.sourceforge.net/moodle/moodle/auth/db/lib.php?hideattic=0&view=log

Robert, I may have mistagged it. It is part of 1.7+ and will be in 1.7.1 and later releases. Also fixed on HEAD, but now superceded by other improvements. No worries.

Brilliant! Thanks Martin - and apologies for the typo in my example code.