Moodle
  1. Moodle
  2. MDL-15039

Apostrophes preventing any users in course being unenrolled via LDAP

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major 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
    • Rank:
      24425

      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)).'\')';

        Activity

        Hide
        Michael Woods added a comment -

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

        Show
        Michael Woods added a comment - Update - we tested on our 1.9 test instance and the bug is there also.
        Hide
        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
        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
        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
        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
        Michael Blake added a comment -

        test

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

        should not have been resolved.

        Show
        Michael Blake added a comment - should not have been resolved.
        Hide
        Petr Škoda added a comment -

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

        Show
        Petr Škoda added a comment - Hi, what is the status? should we resolve this?
        Hide
        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
        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
        Petr Škoda added a comment -

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

        Show
        Petr Škoda added a comment - thanks, my +1 to Resolve it now and wait with Closing a bit
        Hide
        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
        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
        Iñaki Arenaza added a comment -

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

        Saludos. Iñaki.

        Show
        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: