Last modified: 2014-08-17 07:04:59 UTC
In MassMessage::getMessengerUser(), we do some evil things to create a system account that cannot be logged into: $user = User::newFromName( $wgMassMessageAccountUsername ); $user->load(); if ( $user->getId() && $user->mPassword == '' && $user->mNewpassword == '' ) { // We've already stolen the account return $user; } Problems here are: a) User::load() no longer loads the password members, so we can't check if they are equal to empty string. b) null == '', should have been using triple equals. Also, AbuseFilter has a very similar function (I got the idea from it), so we'll need to patch this there too.
From my reading of User::setInternalPassword(), an InvalidPassword-type password is saved for the user. But I don't see any non-hacky way for me to check whether a user has that password type. Am I missing something?
:/ is there a reason an actual account is being created and registered (as opposed to just adding the username to $wgReservedUsernames)?
(In reply to Tyler Romeo from comment #2) > :/ is there a reason an actual account is being created and registered (as > opposed to just adding the username to $wgReservedUsernames)? Yes. We do this so local wikis can adjust the accounts' user groups if they wish to (some remove it from the "bot" group it auto-adds itself to), or if they want to block the account for whatever reason.
The password API changes in question were https://gerrit.wikimedia.org/r/#/c/77645/ The breaking test has been temporarily commented out in https://gerrit.wikimedia.org/r/#/c/150770/
Ideas on how to fix... * Have User::load() call loadPasswords() - fixes all back-compat issues * Make loadPasswords() public * Add getPassword()/getTemporaryPassword() accessors and make the member variables private * Have MM call checkPassword() with a dummy value and then check ->mPassword directly (ewwww)
(In reply to Kunal Mehta (Legoktm) from comment #5) > Ideas on how to fix... > > * Have User::load() call loadPasswords() - fixes all back-compat issues Would rather not, for the same reason load() does not call loadOptions() > * Make loadPasswords() public This makes sense, and I'm not even sure why I didn't make it public in the first place. > * Add getPassword()/getTemporaryPassword() accessors and make the member > variables private This also makes sense. And while we're at it, also make $mId, $mName, $mRealName, etc. private since you shouldn't be accessing those directly either. > * Have MM call checkPassword() with a dummy value and then check ->mPassword > directly (ewwww) > ewwww Agreed. Stick with above.
(In reply to Tyler Romeo from comment #6) > (In reply to Kunal Mehta (Legoktm) from comment #5) > > * Make loadPasswords() public > > This makes sense, and I'm not even sure why I didn't make it public in the > first place. If we add the accessors, there should be no reason to do this I think. > > > * Add getPassword()/getTemporaryPassword() accessors and make the member > > variables private > > This also makes sense. And while we're at it, also make $mId, $mName, > $mRealName, etc. private since you shouldn't be accessing those directly > either. Ib79ce01a47f90af681e376ce918eda559b4b94a6. Will do a second patch once extensions accessing directly are fixed (MassMessage, AbuseFilter, OpenID).
Change 151570 had a related patch set uploaded by Legoktm: Fix MassMessage::getMessengerUser() after Password API changes https://gerrit.wikimedia.org/r/151570
Change 151570 merged by jenkins-bot: Fix MassMessage::getMessengerUser() after Password API changes https://gerrit.wikimedia.org/r/151570
Change 153042 had a related patch set uploaded by Legoktm: Fix MassMessage::getMessengerUser() after Password API changes https://gerrit.wikimedia.org/r/153042
Change 153042 merged by jenkins-bot: Fix MassMessage::getMessengerUser() after Password API changes https://gerrit.wikimedia.org/r/153042
Deployed by Ori, thanks!
Change 154449 had a related patch set uploaded by Wctaiwan: Fix MassMessage::getMessengerUser() after Password API changes https://gerrit.wikimedia.org/r/154449
Change 154449 merged by jenkins-bot: Fix MassMessage::getMessengerUser() after Password API changes https://gerrit.wikimedia.org/r/154449
