Description of problem: The "Loaned To" field allows space before user name but user not able to access system Version-Release number of selected component (if applicable): .17 How reproducible: Always Steps to Reproduce: 1. Add system loan with space before the users name 2. Assign Access Policy that user belongs to with "Loan to self" ACL 3. Have user attempt to return the loan or run a job. Actual results: Job will not start and or user can't return the system. Expected results: Beaker should not allow space character before names or it should strip out blank spaces. Additional info:
Okay it turns out this one is quite nasty... The problem is Beaker tries looking up a username like " dcallagh" in the database, which doesn't exist, but then it tries looking it up in LDAP. A mistake with quoting in the LDAP filter means that this actually matches "dcallagh", so Beaker creates a *new* user account with the username " dcallagh" based on the LDAP lookup for "dcallagh". This is particularly bad because now there will be two user accounts with different user names that look the same in Beaker's UI. So those will need to be cleaned up. Prior to 0.17 the UNIQUE constraint on e-mail address would have ultimately prevented Beaker from creating the duplicate whitespace username (the whole request would have just exploded with a 500). To fix this we should: * Add SQLAlchemy validation to reject usernames with leading or trailing whitespace (or possibly, usernames with any whitespace at all?) * Fix the LDAP look-up code in User.by_user_name to correctly quote the uid value * Optionally, in the web layer trim leading/trailing whitespace from usernames that are submitted by the user * Figure out a strategy to deal with the duplicate user accounts (ideally some SQL which will merge them back together)
For this bug, let's just track the "keep it from happening again" fix. I agree we also need to write and publish the SQL to fix installations that were already affected by the bug, but we can track that separately.
(In reply to Dan Callaghan from comment #2) > * Fix the LDAP look-up code in User.by_user_name to correctly quote the uid > value Actually it turns out there is no mistake in the quoting, it's done correctly using the filter_format() function. The actual problem is that LDAP's rules for whitespace normalisation [1] mean that usernames which are distinct from Beaker's point of view will be normalised to the same value in LDAP. Unfortunately it seems like python-ldap doesn't have any code to do the normalisation, although it probably should since OpenStack have written their own version [2] and we will have to do the same... [1] http://tools.ietf.org/html/rfc4518#section-2.6.1 [2] http://docs.openstack.org/developer/keystone/api/keystone.common.ldap.html#keystone.common.ldap.core.prep_case_insensitive
On Gerrit: http://gerrit.beaker-project.org/3230
The following query can be used to check if your Beaker database is affected by this issue: SELECT old_user.user_name, old_user.user_id, new_user.user_name, new_user.user_id FROM tg_user old_user INNER JOIN tg_user new_user ON TRIM(old_user.user_name) = new_user.user_name AND old_user.user_name != new_user.user_name; If any rows are returned, they are duplicates with leading/trailing whitespace. (Note that it doesn't cover cases where a username has multiple consecutive inner whitespace characters, which would be normalized to a single space. I'm not covering that case because I think whitespace in usernames is uncommon since it doesn't meet the constraints of posixAccount.)
The following UPDATE statements may be useful for fixing foreign keys currently pointing at a duplicate tg_user row, to make them refer to the original tg_user row instead. Beware that these statements *will not* work correctly in cases where there is already a row referencing the original tg_user row as well. UPDATE submission_delegate INNER JOIN tg_user old_user ON submission_delegate.delegate_id = old_user.user_id INNER JOIN tg_user new_user ON TRIM(old_user.user_name) = new_user.user_name AND old_user.user_name != new_user.user_name SET submission_delegate.delegate_id = new_user.user_id; UPDATE system INNER JOIN tg_user old_user ON system.loan_id = old_user.user_id INNER JOIN tg_user new_user ON TRIM(old_user.user_name) = new_user.user_name AND old_user.user_name != new_user.user_name SET system.loan_id = new_user.user_id; UPDATE system_access_policy_rule INNER JOIN tg_user old_user ON system_access_policy_rule.user_id = old_user.user_id INNER JOIN tg_user new_user ON TRIM(old_user.user_name) = new_user.user_name AND old_user.user_name != new_user.user_name SET system_access_policy_rule.user_id = new_user.user_id; UPDATE user_group INNER JOIN tg_user old_user ON user_group.user_id = old_user.user_id INNER JOIN tg_user new_user ON TRIM(old_user.user_name) = new_user.user_name AND old_user.user_name != new_user.user_name SET user_group.user_id = new_user.user_id;
(In reply to Dan Callaghan from comment #10) > The following UPDATE statements may be useful for fixing foreign keys > currently pointing at a duplicate tg_user row, to make them refer to the > original tg_user row instead. Another one, for system.owner_id: UPDATE system INNER JOIN tg_user old_user ON system.owner_id = old_user.user_id INNER JOIN tg_user new_user ON TRIM(old_user.user_name) = new_user.user_name AND old_user.user_name != new_user.user_name SET system.owner_id = new_user.user_id;
Beaker 0.17.3 has been released (https://beaker-project.org/docs/whats-new/release-0.17.html#beaker-0-17-3)