Last modified: 2014-10-11 00:59:10 UTC
BACKGROUND: ----------- User accounts on Wikimedia are case-sensitive. After switching to the LdapAuthentication extension, existing users who authenticated against the ldap server are munged to be "Propercase". Even if there is an identical account in the wiki user table, LdapAuthentication extension creates a new account anyway, so that there are now two in the system: MyUserName Myusername PROBLEM: -------- When attempting to merge two accounts with the same text, but different casing, this extension selects the same ID for BOTH, causing no merge to take place and the selected account to be LOST if the delete option is checked. This is because User::newFromName selects the incorrect user id. Note I am using the latest GIT master of UserMerge. (extension version reports 1.7.0, even though the extension download page shows 1.8.0) WORK AROUND: ------------ I corrected this in my local build by changing: FROM: $objOldUser = User::newFromName( $olduser_text ); $olduserID = $objOldUser->idForName(); $objNewUser = User::newFromName( $newuser_text ); $newuserID = $objNewUser->idForName(); TO: $olduserID = User::idFromName( $olduser_text ); $objOldUser = User::newFromId( $olduserID ); $newuserID = User::idFromName( $newuser_text ); $objNewUser = User::newFromId( $newuserID ); I also added the following as another check, but could use an i18n: //check if attempting to merge into same user } elseif($olduserID == $newuserID) { $validNewUser = false; $out->addHTML("new user cannot be the same as the old one"); } else { // newuser looks good $validNewUser = true; }
Seems to be a valid point, thanks for reporting this. Are you able (have you ever tried?) to formally submit a commit to Gerrit? Otherwise I will do, don't be worry, it's not worth that you install the requirements for submitting to gerrit. But can you add at least a formal "diff -bru <original-directory> <patched-code-directory>" as attachment to this bugzilla, based on the latest code, please read the following: Regarding this: > Note I am using the latest GIT master of UserMerge. (extension version reports 1.7.0, even though the extension download page shows 1.8.0) This is not true, Special:Version reports 1.8.0 for me. Can you check your latest version or make in any case a fresh checkout from git -- I cannot see "1.7.0" elsewhere in the current code!
Created attachment 13063 [details] quick patch of workaround Note that this includes an error condition if both id's match. The text in this patch is hard-coded.
My apologies -- the latest snapshot is 1.7; git master is indeed 1.8 I merged my changes into 1.8 and attached the patch as requested.
Change 77814 had a related patch set (by Wikinaut) published: Fixed problem of not merging two case-different usernames https://gerrit.wikimedia.org/r/77814
Carl, you can checkout the last patch set and try. Please comment in gerrit whether it works as designed. Here is the command you can simply apply in the UserMerge directory: git fetch https://wikinaut@gerrit.wikimedia.org/r/mediawiki/extensions/UserMerge refs/changes/14/77814/3 && git checkout FETCH_HEAD
["patch-need-review" keyword refers to Bugzilla only, not Gerrit - removing]
Changing this to LdapAuthentication. The bug is that when you switch from normal database authentication over to LDAP authentication, the extension starts creating new accounts in the database using "canonical username" form.
but the test for equality of old and new names (=> error text) is an improvement in UserMerge.
I'm not so sure about that. The idea behind User::getCanonicalName() is that the returned string is the unique name by which the user is identified. If two names are passed into that function and the same canonical value is returned, then those two users should be considered identical. To bypass that functionality would mean treating identical users differently. The fix for this should be for LdapAuthentication to provide some sort of migration script that fixes the case of usernames in the database.
I only mean the check, if same names were entered into the input fields. Haven't studied the code if this is already checked previously.
Ah yes true. Does your current patch do that? If so then we can merge that (although this bug will still be open since it's still a problem with LdapAuthentication).
see lines 94-96 and the new message text 'usermerge-same-old-and-new-user' in the i18n file: } elseif( $olduserID == $newuserID ) { $validNewUser = false; $out->wrapWikiMsg( "<div class='error'>$1</div>", array( 'usermerge-same-old-and-new-user' ) ); } else { I think you will ask me quickly to make a separate commit for this ;-)
(In reply to comment #12) > see lines 94-96 and the new message text 'usermerge-same-old-and-new-user' in > the i18n file: > > > } elseif( $olduserID == $newuserID ) { > $validNewUser = false; > $out->wrapWikiMsg( "<div class='error'>$1</div>", array( > 'usermerge-same-old-and-new-user' ) ); > } else { > > > I think you will ask me quickly to make a separate commit for this ;-) separate commit is => https://gerrit.wikimedia.org/r/#/c/77980/
While it is true that Ldap is the original culprit, how does one clean up the aftermath? Commit 77980 will only block future merge attempts.
(In reply to comment #14) > While it is true that Ldap is the original culprit, how does one clean up the > aftermath? Commit 77980 will only block future merge attempts. No, this does not block further merge attemps, check the code, it is the same as in https://gerrit.wikimedia.org/r/77814 < this one should be abandoned, perhaps.
Change 77814 abandoned by Legoktm: Fixed problem of not merging two case-different usernames https://gerrit.wikimedia.org/r/77814
