Moodle
  1. Moodle
  2. MDL-5823

LDAP authentication uses a mix of username and idnumber

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.6
    • Fix Version/s: 1.8, 1.9
    • Component/s: Authentication
    • Labels:
      None
    • Environment:
      All
    • Database:
      Any
    • Affected Branches:
      MOODLE_16_STABLE
    • Fixed Branches:
      MOODLE_18_STABLE, MOODLE_19_STABLE
    • Rank:
      28138

      Description

      when auth_ldap_sync_users is called it fails if you are not using the username as the idnumber. Thus the temporary table {$CFG->prefix}ext_user gets the idnumber field populated with usernames. Then when the subsequent 'LEFT JOIN' is called the idnumber field returned is NULL for all users. This results in all of the users being deleted and then readded. This causes lots of extraneous work and problems in certain cases.

      To resolve the issue the code either needs to be switched to use only idnumber as the keying attribute or username. Even though the other external authentication methods use idnumber as the outside keying entry I think it is correct to use username in the case of LDAP.

      One solution is to change the LEFT JOINS, there are two queries, to use the username field from {$CFG->prefix}user rather than idnumber.

        Issue Links

          Activity

          Hide
          Martin Dougiamas added a comment -

          From Jeff Graham (jjg7 at humboldt.edu) Tuesday, 20 June 2006, 02:33 AM:

          attached is a diff from that should fix this bug. I chose to go with username as the rest of the authentication code is keying off of username rather than idnumber.

          Show
          Martin Dougiamas added a comment - From Jeff Graham (jjg7 at humboldt.edu) Tuesday, 20 June 2006, 02:33 AM: attached is a diff from that should fix this bug. I chose to go with username as the rest of the authentication code is keying off of username rather than idnumber.
          Hide
          Dan Marsden added a comment -

          Hi Martin, - any chance on gettting this patched? - I could commit it to head if you have no objections!

          this is the change I'd suggest

          $sql = 'SELECT u.id, u.username
          FROM ' . $CFG->prefix .'user u LEFT JOIN ' . $CFG->prefix .'extuser e
          ON u.username = e.idnumber
          WHERE u.auth=\'' . AUTH_LDAP_NAME . '\' AND u.deleted=\'0\' AND e.idnumber IS NULL';

          Instead of:

          $sql = 'SELECT u.id, u.username
          FROM ' . $CFG->prefix .'user u LEFT JOIN ' . $CFG->prefix .'extuser e
          ON u.idnumber = e.idnumber
          WHERE u.auth=\'' . AUTH_LDAP_NAME . '\' AND u.deleted=\'0\' AND e.idnumber IS NULL';

          Show
          Dan Marsden added a comment - Hi Martin, - any chance on gettting this patched? - I could commit it to head if you have no objections! this is the change I'd suggest $sql = 'SELECT u.id, u.username FROM ' . $CFG->prefix .'user u LEFT JOIN ' . $CFG->prefix .'extuser e ON u.username = e.idnumber WHERE u.auth=\'' . AUTH_LDAP_NAME . '\' AND u.deleted=\'0\' AND e.idnumber IS NULL'; Instead of: $sql = 'SELECT u.id, u.username FROM ' . $CFG->prefix .'user u LEFT JOIN ' . $CFG->prefix .'extuser e ON u.idnumber = e.idnumber WHERE u.auth=\'' . AUTH_LDAP_NAME . '\' AND u.deleted=\'0\' AND e.idnumber IS NULL';
          Hide
          Martín Langhoff added a comment -

          Dan - I agree with the idea but... your "patch" looks backwards?

          Show
          Martín Langhoff added a comment - Dan - I agree with the idea but... your "patch" looks backwards?
          Hide
          Jeff Graham added a comment -

          Bringing attached diff from old tracker over.

          Show
          Jeff Graham added a comment - Bringing attached diff from old tracker over.
          Hide
          Jeff Graham added a comment -

          Martin,

          I believe Dan's patch is correct, but there are 2 queries that need to be adjusted. I have brought over the diff from the old tracker.

          The other option would be to populate prefix_extuser.idnumber with the same value as user.idnumber or change that fieldname in extuser to username.

          Show
          Jeff Graham added a comment - Martin, I believe Dan's patch is correct, but there are 2 queries that need to be adjusted. I have brought over the diff from the old tracker. The other option would be to populate prefix_extuser.idnumber with the same value as user.idnumber or change that fieldname in extuser to username.
          Hide
          Dan Marsden added a comment -

          Hi Martin,

          Nope - not backwards! - it makes the thing work in our environment! - what looks wrong - will this not work everywhere?

          Dan

          Show
          Dan Marsden added a comment - Hi Martin, Nope - not backwards! - it makes the thing work in our environment! - what looks wrong - will this not work everywhere? Dan
          Hide
          Petr Škoda added a comment -

          should be fixed as part of the auth cleanup, only usernames are now used for sync, synchronization of removed users is now fully configurable - allows full delete/suspend/nothing actions.

          please report any problems with the new code, closing now - thanks!

          Show
          Petr Škoda added a comment - should be fixed as part of the auth cleanup, only usernames are now used for sync, synchronization of removed users is now fully configurable - allows full delete/suspend/nothing actions. please report any problems with the new code, closing now - thanks!
          Hide
          Patrick Pollet added a comment -

          Although revamped code in 1.82 DO fix the previous issue (it is a pity it was not backported in CVS in 1.6 and 1.7 in Novembre 2006 , it would have saved us a lot of headache ;-(() , use of username as the Ldap unique user's information to sync Moodle's user database brings new problems.

          See http://moodle.org/mod/forum/discuss.php?d=80652#p358210

          Show
          Patrick Pollet added a comment - Although revamped code in 1.82 DO fix the previous issue (it is a pity it was not backported in CVS in 1.6 and 1.7 in Novembre 2006 , it would have saved us a lot of headache ;-(() , use of username as the Ldap unique user's information to sync Moodle's user database brings new problems. See http://moodle.org/mod/forum/discuss.php?d=80652#p358210

            People

            • Votes:
              1 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: