[#MDL-15039] Apostrophes preventing any users in course being unenrolled via LDAP - Moodle Tracker

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

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Michael Woods added a comment - 30/May/08 9:42 AM

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

Show

Michael Woods added a comment - 30/May/08 9:42 AM Update - we tested on our 1.9 test instance and the bug is there also.

Hide

Permalink

Michael Woods added a comment - 02/Jun/08 7:27 AM

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 - 02/Jun/08 7:27 AM 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

Permalink

Iñaki Arenaza added a comment - 03/Jun/08 6:40 AM

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 - 03/Jun/08 6:40 AM 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

Permalink

Michael Blake added a comment - 10/Jun/08 4:45 PM

test

Show

Michael Blake added a comment - 10/Jun/08 4:45 PM test

Hide

Permalink

Michael Blake added a comment - 10/Jun/08 4:46 PM

should not have been resolved.

Show

Michael Blake added a comment - 10/Jun/08 4:46 PM should not have been resolved.

Hide

Permalink

Petr Škoda (skodak) added a comment - 10/Jun/08 11:15 PM

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

Show

Petr Škoda (skodak) added a comment - 10/Jun/08 11:15 PM Hi, what is the status? should we resolve this?

Hide

Permalink

Iñaki Arenaza added a comment - 10/Jun/08 11:34 PM

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 - 10/Jun/08 11:34 PM 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

Permalink

Petr Škoda (skodak) added a comment - 10/Jun/08 11:37 PM

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

Show

Petr Škoda (skodak) added a comment - 10/Jun/08 11:37 PM thanks, my +1 to Resolve it now and wait with Closing a bit

Hide

Permalink

Michael Woods added a comment - 11/Jun/08 6:54 AM

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 - 11/Jun/08 6:54 AM 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

Permalink

Iñaki Arenaza added a comment - 14/Jun/08 8:37 PM

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

Saludos. Iñaki.

Show

Iñaki Arenaza added a comment - 14/Jun/08 8:37 PM Resolving now. Michael, please reopen if it doesn't fix your case

Saludos. Iñaki.

People

Assignee:

Iñaki Arenaza

Reporter:

Michael Woods

Participants:

Iñaki Arenaza, Michael Blake, Michael Woods, Petr Škoda (skodak)

Vote (1)

Watch (1)

Dates

Created:

29/May/08 2:09 PM

Updated:

17/Dec/10 1:02 PM

Resolved:

14/Jun/08 8:37 PM