Moodle
  1. Moodle
  2. MDL-19672

update_user_record() usage of stripslashes() breaks LDAP authentication/enrolment with certain configurations

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.9.5
    • Fix Version/s: 1.9.6
    • Component/s: Authentication, Enrolments
    • Labels:
      None
    • Environment:
      MS Active Directory as the LDAP backend (mainly, but not only)
    • Database:
      Any
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE
    • Rank:
      31431

      Description

      When using LDAP as the authentication source and we map the distinguished name on one of the user attributes (usually the ID Number) and this mapping is configured to update the value on each login, if the distinguished name cotains a '\' character (the quote character in LDAP ditinguished names), update_user_record() mangles the value as it calls stripslashed() before calling addslashes() to store the value.

      If this mapped value is later used for enrolment (a typical scenario) Moodle unenrols the user from the courses, as it doesn't match what comes back from the LDAP enrolment server.

      So we need to keep update_user_record() from mangling the value. All the values processed by update_user_record() come from the external authentication sub-systems (external db, ldap server, cas server, etc.) via get_userinfo(), and the convention is that this function returns the data with no magic quotes.

      I discussed this issue with Petr and Eloy in the developer chat and given that no contrib plugin provides get_userinfo(), we agreed that we can simply remove the call to stripslahes().

      But this is just the first step. In order to be able to used a distiguished name with '\' (and other LDAP special characters) in a LDAP filter to find the user enrolments, we need to quote it using the LDAP filter quoting rules. Now that we are at it, I've also updated ldap_addslashes() to deal with all the special characters specified in the latest RFC.

      If this was not enough, MS Active Directory doesn't return the distiguished name as the 'dn' attribute, but as 'distinguishedName', so we need to take that into account too.

      The patch is only for 1.9, as I plan to refactor LDAP authentication and enrolment in 2.0 (I have it nearly ready, and it includes the same functionnality of this patch)

        Issue Links

          Activity

          Hide
          Iñaki Arenaza added a comment -

          Attaching patch so Petr can review it.

          Show
          Iñaki Arenaza added a comment - Attaching patch so Petr can review it.
          Hide
          Petr Škoda added a comment -

          looks ok, my +1 for commit tomorrow - after the review day

          Show
          Petr Škoda added a comment - looks ok, my +1 for commit tomorrow - after the review day
          Hide
          Iñaki Arenaza added a comment -

          Fixed in CVS for 1.9 (LDAP auth and enrolment is going to be refactored in HEAD as stated in the bug report)

          Thanks a lot to Gordon Falk for testing the bugfix before comitting it into CVS

          Saludos.
          Iñaki.

          Show
          Iñaki Arenaza added a comment - Fixed in CVS for 1.9 (LDAP auth and enrolment is going to be refactored in HEAD as stated in the bug report) Thanks a lot to Gordon Falk for testing the bugfix before comitting it into CVS Saludos. Iñaki.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: