Moodle
  1. Moodle
  2. MDL-7712

Fields not read from external database

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.7
    • Fix Version/s: 1.7.1, 1.7.2, 1.8
    • Component/s: Authentication, Enrolments
    • Labels:
      None
    • Environment:
      Windows XP, MySQL 5.0, MSSQL2000, Moodle 1.7
    • Database:
      MySQL, Microsoft SQL
    • Affected Branches:
      MOODLE_17_STABLE
    • Fixed Branches:
      MOODLE_17_STABLE, MOODLE_18_STABLE
    • Rank:
      27703

      Description

      When using external authentication I found out that the authentication module did not retrieve the database fields from the server.

      I had to change line 110 in auth/db/lib.php from:
      $result["$field"] = addslashes(stripslashes($rs->fields[0]));

      to:
      $result["$field"] = addslashes(stripslashes($rs->fields[$pcfg["field_map_$field"]]));

      After that authentication with the external database worked fine.

      I also had a similar problem in the database enrolment module (enrol/database/enrol.php) where I also had to change the field access from index to id.

      http://moodle.org/mod/forum/discuss.php?d=59540

        Issue Links

          Activity

          Hide
          Robert Allerstorfer added a comment -

          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.

          Show
          Robert Allerstorfer added a comment - 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.
          Hide
          Phil Driscoll added a comment -

          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

          Show
          Phil Driscoll added a comment - 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
          Hide
          Martín Langhoff added a comment -

          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.

          Show
          Martín Langhoff added a comment - 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.
          Hide
          Robert Allerstorfer added a comment -

          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.

          Show
          Robert Allerstorfer added a comment - 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.
          Hide
          Martín Langhoff added a comment -

          Great to hear that! Resolved then.

          Show
          Martín Langhoff added a comment - Great to hear that! Resolved then.
          Hide
          Phil Driscoll added a comment -

          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']))); }

          }
          }

          Show
          Phil Driscoll added a comment - 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']))); } } }
          Hide
          Robert Allerstorfer added a comment -

          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?

          Show
          Robert Allerstorfer added a comment - 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?
          Hide
          Martín Langhoff added a comment -

          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.

          Show
          Martín Langhoff added a comment - 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.
          Hide
          Phil Driscoll added a comment -

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

          Show
          Phil Driscoll added a comment - Brilliant! Thanks Martin - and apologies for the typo in my example code.

            People

            • Votes:
              4 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: