Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-15039

Apostrophes preventing any users in course being unenrolled via LDAP

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.8.2, 1.9
    • Fix Version/s: 1.8.6, 1.9.2
    • Component/s: Enrolments
    • Labels:
      None
    • Environment:
      MS Active Directory
    • Database:
      MySQL
    • Affected Branches:
      MOODLE_18_STABLE, MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_18_STABLE, MOODLE_19_STABLE

      Description

      If any member of a course has an ID number containing an apostrophe (eg. CN=Paddy O'Brien,OU=Students,DC=etc...), no other members will ever be unenrolled via LDAP.

      In enrol/ldap/enrol.php, this is due to the array $ldapmembers not accounting for apostrophes and therefore get_records_sql($sql) fails to return any members due to the resultant syntax error.

      It doesn't just affect the name with an apostrophe - it affects any members of the same course.

      We fixed by replacing the following code around line 264 (v1.8.2)

      $sql .= 'AND usr.idnumber NOT IN (\''. join('\',\'', $ldapmembers).'\')';

      with...

      $sql .= 'AND usr.idnumber NOT IN (\''. join('\',\'', str_replace("'", "\'",$ldapmembers)).'\')';

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            mcwoods Michael Woods added a comment -

            Update - we tested on our 1.9 test instance and the bug is there also.

            Show
            mcwoods Michael Woods added a comment - Update - we tested on our 1.9 test instance and the bug is there also.
            Hide
            mcwoods Michael Woods added a comment -

            Forgot to mention an earlier change we made which should probably come under the same bug report.

            Around line 285 of enrol/ldap/enrol.php (v1.8.2), current enrolments are inserted, but this fails for anyone with an apostrophe.

            We replaced:

            ." WHERE idnumber='$ldapmember'";

            with

            ." WHERE idnumber='".str_replace("'", "\'",$ldapmember)."'";

            I think this is still outstanding in 1.9.

            Show
            mcwoods Michael Woods added a comment - Forgot to mention an earlier change we made which should probably come under the same bug report. Around line 285 of enrol/ldap/enrol.php (v1.8.2), current enrolments are inserted, but this fails for anyone with an apostrophe. We replaced: ." WHERE idnumber='$ldapmember'"; with ." WHERE idnumber='".str_replace("'", "\'",$ldapmember)."'"; I think this is still outstanding in 1.9.
            Hide
            iarenaza Iñaki Arenaza added a comment -

            Hi Michael,

            this should be fixed in the latest CVS versions of 1.8, 1.9 and HEAD.

            Could you confirm it, please?

            Saludos. Iñaki.

            Show
            iarenaza Iñaki Arenaza added a comment - Hi Michael, this should be fixed in the latest CVS versions of 1.8, 1.9 and HEAD. Could you confirm it, please? Saludos. Iñaki.
            Hide
            mblake Michael Blake added a comment -

            test

            Show
            mblake Michael Blake added a comment - test
            Hide
            mblake Michael Blake added a comment -

            should not have been resolved.

            Show
            mblake Michael Blake added a comment - should not have been resolved.
            Hide
            skodak Petr Skoda added a comment -

            Hi,
            what is the status? should we resolve this?

            Show
            skodak Petr Skoda added a comment - Hi, what is the status? should we resolve this?
            Hide
            iarenaza Iñaki Arenaza added a comment -

            All my tests show that this should be fixed in 1.8, 1.9 and 2.0 (with your patch from MDL-14679, in revision 1.25), but I was waiting for comfirmation from Michael Woods.

            Saludos. Iñaki.

            Show
            iarenaza Iñaki Arenaza added a comment - All my tests show that this should be fixed in 1.8, 1.9 and 2.0 (with your patch from MDL-14679 , in revision 1.25), but I was waiting for comfirmation from Michael Woods. Saludos. Iñaki.
            Hide
            skodak Petr Skoda added a comment -

            thanks, my +1 to Resolve it now and wait with Closing a bit

            Show
            skodak Petr Skoda added a comment - thanks, my +1 to Resolve it now and wait with Closing a bit
            Hide
            mcwoods Michael Woods added a comment -

            Hi all,

            Inaki - thank you for the fix. Unfortunately, I still haven't had a chance to test it. If your tests show that this is resolved, feel free to close it. When I eventually get around to testing it, I can let you know if there any problems (which I doubt).

            Thanks again,
            Michael

            Show
            mcwoods Michael Woods added a comment - Hi all, Inaki - thank you for the fix. Unfortunately, I still haven't had a chance to test it. If your tests show that this is resolved, feel free to close it. When I eventually get around to testing it, I can let you know if there any problems (which I doubt). Thanks again, Michael
            Hide
            iarenaza Iñaki Arenaza added a comment -

            Resolving now. Michael, please reopen if it doesn't fix your case

            Saludos. Iñaki.

            Show
            iarenaza Iñaki Arenaza added a comment - Resolving now. Michael, please reopen if it doesn't fix your case Saludos. Iñaki.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  11/Jul/08