Last modified: 2014-09-24 04:08:35 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T68823, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 66823 - Core patches still block account creation when extension is not enabled
Core patches still block account creation when extension is not enabled
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
MediaWikiAuth (Other open bugs)
master
All All
: Unprioritized normal (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2014-06-19 04:23 UTC by Isarra
Modified: 2014-09-24 04:08 UTC (History)
4 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Isarra 2014-06-19 04:23:23 UTC
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.
Comment 1 Jack Phoenix 2014-06-19 04:45:22 UTC
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.
Comment 2 Isarra 2014-09-24 04:08:35 UTC
I think this was fixed in a patch I forgot to link because I forgot I filed a bug. Seems to be, though.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links