Moodle

LDAP sync_users fails on usernames with uppercase characters

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.6, 1.8, 1.9
  • Fix Version/s: 1.8.4, 1.9, 2.0
  • Component/s: Authentication
  • Labels:
    None
  • Database:
    MySQL
  • Affected Branches:
    MOODLE_16_STABLE, MOODLE_18_STABLE, MOODLE_19_STABLE
  • Fixed Branches:
    MOODLE_18_STABLE, MOODLE_19_STABLE, MOODLE_20_STABLE

Description

auth_ldap_sync_users.php fails with "Cannot update non-existent user" when updating usernames containing capital letters.
In auth/ldap/auth.php, in function update_user_record(), the following line is converting the username to lowercase, causing the get_record() lookup to fail with no match on the username:
$username = trim(moodle_strtolower($username));

Here is an example of the output from auth_ldap_sync_users.php:

Got 1151 records from LDAP

User entries to update: 1332
Updating user ldap.test id 792
Updating user ann.bolton id 488
Updating user olivian.roddenby id 1538
Updating user SBCS - ASP id 590Cannot update non-existent user: sbcs - asp

Commenting out the strtolower line fixes this.

Also as a subsequent bug, the update_user_record function in auth/ldap/auth.php calls print_error which prints out a HTML error page, however auth_ldap_sync_users.php is not intended to be called from a web server so should have plain text output suitable for cron emails.

  1. MDL-10509-165.diff
    05/Nov/07 12:11 AM
    0.7 kB
    Iñaki Arenaza
  2. MDL-10509-173.diff
    05/Nov/07 12:11 AM
    0.7 kB
    Iñaki Arenaza
  3. MDL-10509-183.diff
    05/Nov/07 12:11 AM
    0.8 kB
    Iñaki Arenaza
  4. MDL-10509-19beta2.diff
    05/Nov/07 12:11 AM
    0.8 kB
    Iñaki Arenaza

Activity

Hide
Martin Dougiamas added a comment -

As far as I remember, usernames in Moodle were all stored in lower case all the time, but things have changed there a lot more recently.

Yu, can you check what the current rule is?

ie

1) usernames should always be lowercased in the database or
2) usernames are case-sensitive

Show
Martin Dougiamas added a comment - As far as I remember, usernames in Moodle were all stored in lower case all the time, but things have changed there a lot more recently. Yu, can you check what the current rule is? ie 1) usernames should always be lowercased in the database or 2) usernames are case-sensitive
Hide
Ashley Holman added a comment - - edited

I think you are right about usernames being lowercase. When an LDAP user logs in and their account does not exist in the local database, they get created with create_user_record() which converts the username into lowercase. However when synching with auth_ldap_sync_users.php the "User Additions" section does not appear to do this conversion before adding them, so I think that's why I have usernames with capitals in the database.

Show
Ashley Holman added a comment - - edited I think you are right about usernames being lowercase. When an LDAP user logs in and their account does not exist in the local database, they get created with create_user_record() which converts the username into lowercase. However when synching with auth_ldap_sync_users.php the "User Additions" section does not appear to do this conversion before adding them, so I think that's why I have usernames with capitals in the database.
Hide
Petr Škoda (skodak) added a comment -

My +1 for keeping all usernames lowercase and fixing the sync scripts and external auths to work with this limitation.
I do not think that we should treat user,User and USER as three different users.

Show
Petr Škoda (skodak) added a comment - My +1 for keeping all usernames lowercase and fixing the sync scripts and external auths to work with this limitation. I do not think that we should treat user,User and USER as three different users.
Hide
Yu Zhang added a comment -

Hi, sorry I am not very familiar with LDAP. The User Additions section does a join on username with the user table. That means new users are only created if the usernames are already in lower case, isn't it?

Are we supposed to convert usernames to lowercases before ldap_bulk_insert() calls?

Show
Yu Zhang added a comment - Hi, sorry I am not very familiar with LDAP. The User Additions section does a join on username with the user table. That means new users are only created if the usernames are already in lower case, isn't it? Are we supposed to convert usernames to lowercases before ldap_bulk_insert() calls?
Hide
Iñaki Arenaza added a comment -

Yep, we should convert username to lowercase before calling ldap_bulk_insert. Something like the attached patches would do it.

Saludos. Iñaki.

Show
Iñaki Arenaza added a comment - Yep, we should convert username to lowercase before calling ldap_bulk_insert. Something like the attached patches would do it. Saludos. Iñaki.
Hide
Yu Zhang added a comment -

Hi Iñaki,

Thanks for the patch. Does $value[0] always correspond to username, or could it be used for other user attributes?

Cheers,

Yu

Show
Yu Zhang added a comment - Hi Iñaki, Thanks for the patch. Does $value[0] always correspond to username, or could it be used for other user attributes? Cheers, Yu
Hide
Iñaki Arenaza added a comment -

It always corresponds to the username, as we specifically ask for that attribute in the call to ldap_get_values_len() a couple of lines above.

Saludos. Iñaki.

Show
Iñaki Arenaza added a comment - It always corresponds to the username, as we specifically ask for that attribute in the call to ldap_get_values_len() a couple of lines above. Saludos. Iñaki.
Hide
Yu Zhang added a comment -

Thanks, patch in 1.8, 1.9 and HEAD

Show
Yu Zhang added a comment - Thanks, patch in 1.8, 1.9 and HEAD

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: