Page MenuHomePhabricator

Core patches still block account creation when extension is not enabled
Closed, ResolvedPublic

Description

MediaWikiAuth blocks normal account creation for an account when it can be imported, because otherwise the account could be taken over by a troll, the contributions could get lost and weird, and/or possibly other things. Unfortunately this is done via a horrible hack in the specialloginform or some such, and if the horrible hack is present, it still activates even when the extension itself is not enabled.

Immediate fix would be to make the speciallogin hack less stupid, but really the base problem is because it needs a core hack in the first place. The hack/patch exists, apparently, because there is/was no hook or something in the file in order to block account creation or enable it or something along those lines.

Jack Phoenix, please say more about this or something. You would actually know what you're talking about BUT THIS IS IMPORTANT DAMMIT AND I ALREADY COMPLETELY FORGOT ABOUT IT ONCE.

Er.


Version: master
Severity: normal

Details

Reference
bz66823

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 3:24 AM
bzimport set Reference to bz66823.
bzimport added a subscriber: Unknown Object (MLST).

There are at least two sorta-but-not-quite-separate issues here:

  • the "don't allow account creation if such an account existed on the remote wiki" feature; this should be an optional setting that can be turned on or off; right now it's on and the only way to get around that is to mess with the code
  • MediaWikiAuth requires a core patch (hack) and there might not be such a patch available for the latest version(s) of MediaWiki, because people are lazy (trust me, I'm an expert on this field!)

When we upgraded ShoutWiki to 1.23, I applied the 1.21 patch, but two hunks failed, so I had to do those manually, and frankly, I'm not sure if I got it correct at all (this is r2545 on the ShoutWiki SVN). In any case, this whole patching scenario is just outright awful and something we should fix.

The biggest code portion -- new static function checkImportableUser(), which happens to be the thing that oughta be hidden behind a global -- can be moved to MediaWikiAuth as-is, probably.
LoginForm::checkImportableUser() is called in LoginForm::addNewAccountInternal(), right after the "is this an invalid domain?" check, at the very beginning of the function. Much later in the same function we have the AbortNewAccount hook (and also the ExemptFromAccountCreationThrottle hook, though that's of no relevance to us for this issue); this is way too late for our use case, hence the core patch. Should be solvable by slapping a new hook at the beginning of LoginForm::checkImportableUser() and calling it a day.

Then there's the weird error message support patch for LoginForm::attemptAutoCreate(); a $wgAuth->authenticate() call normally called with two params is given a third one (which is null by default) *and* to display a possible error message set by the AuthPlugin, a call to $this->mainLoginForm() is made right before returning the status code (self::WRONG_PLUGIN_PASS). This is very messy and needs some serious rethinking.

The above-described mess has a little cousin, too, which is the patch to LoginForm::processLogin(). The actual logic of case self::WRONG_PLUGIN_PASS is commented out as it's implemented above, in attemptAutoCreate().

My explanation probably made no sense, but once you look at the available patch files, it should be somewhat clearer.

I think this was fixed in a patch I forgot to link because I forgot I filed a bug. Seems to be, though.