Page MenuHomePhabricator

Unable to create an account via "Special:ConfirmAccount" in 1.27 - requires porting to AuthManager
Closed, ResolvedPublic

Description

When trying to confirm an account request I am getting "There was either an authentication database error or you are not allowed to update your external account." (Externaldberror).

Probably this is what item 4 on T110451 is describing as an expected side effect.

Setup

  • MediaWiki 1.27.0-rc.1 (5462877) // <s>1.27.0-rc.0 (f502a0c)</s>
  • PHP 5.6.22-0+deb8u1 (apache2handler) // <s>5.6.20-0+deb8u1 (apache2handler)</s>
  • ConfirmAccount – (071b0df)

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Restricted Application added subscribers: Zppix, Aklapper. · View Herald TranscriptJun 14 2016, 7:55 PM
Kghbln updated the task description. (Show Details)Jun 14 2016, 7:56 PM
Kghbln updated the task description. (Show Details)Jun 14 2016, 8:15 PM
Tgr added a subscriber: Tgr.Jun 15 2016, 4:30 PM

Probably the extension is using User::setPassword (which is deprecated) and some other extension is preventing the password change. Look at the authentication log channel for the details.

@Tgr Thanks for your comment.

So MediaWiki believes that the username is already in use. That's not really a wrong statement since it was created for the account request as well as confirmed via e-mail to trigger the process of reviewing the request:

2016-06-15 17:23:14 phalaris 0210020150926-02100_: MediaWiki\Auth\AuthManager::beginAccountCreation: MyTestUserName cannot be created: Username entered already in use.
Please choose a different name.
2016-06-15 17:23:14 phalaris 0210020150926-02100_: MediaWiki\Auth\AuthManagerAuthPlugin::addUser: Authentication failed: Username entered already in use.
Please choose a different name.

Apart from that the authentication log is empty.

Kghbln updated the task description. (Show Details)Jun 15 2016, 5:33 PM
Tgr added a subscriber: Anomie.Jun 16 2016, 12:05 PM

Looking at AccountConfirmSubmission::acceptRequest, it calls User::createNew (which saves the user in the DB) and then $wgAuth->addUser. Pre-AuthManager addUser was supposed to be called after the user was already saved locally. With AuthManger it's just a wrapper for the normal account creation process (which will fail if the user exists).

I don't see any easy way to fix this. AuthManagerAuthPlugin could call providers directly with the existing user instead of using AuthManager's normal account creation logic, but that would be a horrible and volatile hack just to be compatible with old extensions which would probably break somewhere else anyway. @Anomie, thoughts?

Pre-AuthManager addUser was supposed to be called after the user was already saved locally.

That doesn't seem to be the case: LoginForm calls $wgAuth->addUser() before $user->addToDatabase(). OTOH, in that case the $user->addToDatabase() call is going to fail with the "user already exists" error, so things are still broken anyway.

With these constraints, we'd need to manually call PrimaryAuthenticationProviders, then if the user doesn't exist yet hook LocalUserCreated to be able to call finishAccountCreation and SecondaryAuthenticationProviders once it is added, and hope whatever's doing the calling actually does that in the same request. Ugh.

It looks like the only extensions in Gerrit calling addUser are ConfirmAccount and SemanticSignup. The simplest solution might be to just throw an exception, or just log a warning that things are creating accounts in what's probably a broken manner.

Tgr added a comment.Jun 16 2016, 3:04 PM

For the SemanticSignup test site (if I understand correctly what was written on GitHub, the original problem is signup breaking on that site) you can probably fix things by adding an AuthPluginSetup hook which sets $wgAuth to new AuthPlugin. That will disable any authentication provider extension, but you probably don't have any anyway.

@Tgr Thanks for the note I now referenced accordingly. Well the test site is already suffering from ConfirmAccount failure so SemanticSignup is currently not installed for better debugging.

@Tgr trying to login with this extension installed I get this error

There seems to be a problem with your login session; this action has been canceled as a precaution against session hijacking. Go back to the previous page, reload that page and then try again.

I never got that before.

I also have centralauth installed.

Disabling auth manager I now get this error

Login error
Random Wikisaur uses cookies to log in users. You have cookies disabled. Please enable them and try again.

Tgr added a comment.Jun 19 2016, 11:42 AM

@Paladox: that's given on an invalid CSRF token (typically a session timeout). If you have reproduction steps, can you file a separate bug about it?

@Tgr Ok, I will file a separate bug.

Paladox added a comment.EditedJun 19 2016, 12:10 PM

I filled T138168 @Tgr

@Tgr this function getFakeTemplate does not work since it dosent show the link on the login page that confirmaccount add's.

Paladox triaged this task as Unbreak Now! priority.Jun 19 2016, 3:34 PM

Setting unbreak priority since ConfirmAccount really needs to be updated otherwise it is broken if it is not updated.

Restricted Application added subscribers: Luke081515, TerraCodes, Urbanecm. · View Herald TranscriptJun 19 2016, 3:34 PM
Tgr added a comment.Jun 19 2016, 3:35 PM

As the name suggests, it's a fake template; it's not going to be displayed. Changes to it might or might not work depending on whether there is B/C logic for that specific template field.

Aklapper lowered the priority of this task from Unbreak Now! to High.Jun 19 2016, 4:55 PM
Eburcat raised the priority of this task from High to Unbreak Now!.Jul 9 2016, 6:15 PM
Eburcat added a subscriber: Eburcat.

We started to have problems with confirming users, and we're stuck. Now considering to rollback to 1.26 because of this - which will probably be a headache by its own.

Kghbln added a comment.EditedJul 9 2016, 6:29 PM

@Eburcat What I currently do is create requested accounts as an admin via "Special:UserLogin&type=signup" using the e-mail provided and sending out a temporary password. You have to slightly change the user name though probably best by appending a suffix. Once this extension is fixed I will move the accounts to the originally requested names. It is an extra effort and can probably only be done up to a certain request load but you may still board people

@Tgr hi, would you be able to fix this please. Since it seems to be caused by by the new auth manager. Or could you add backword compatibility in mediawiki for extensions that doint yet support auth manager please.

Change 298181 had a related patch set uploaded (by Anomie):
AuthManager: Break AuthPlugin::addUser more explicitly

https://gerrit.wikimedia.org/r/298181

@Paladox: T110451 describes what needs to be done. Whoever maintains this extension should do it, it's not Wikimedia Foundation engineers' job to rewrite every extension whenever something changes, particularly considering items #4 and #5 there seem likely to need significant reworking. Backwards compatibility was already maintained to the extent practical.

That said, when someone does step up to rewrite the extension I'll be happy to review the code, much as we did for T110294.

Paladox added a subscriber: aaron.Jul 10 2016, 3:59 PM

@aaron would you be able to do this please. Since your on the author list.

Aklapper renamed this task from Unable to create an account via "Special:ConfirmAccount" to Unable to create an account via "Special:ConfirmAccount" in 1.27 - requires porting to AuthManager.Jul 11 2016, 3:09 PM

Change 298181 merged by jenkins-bot:
AuthManager: Break AuthPlugin::addUser more explicitly

https://gerrit.wikimedia.org/r/298181

Change 299175 had a related patch set uploaded (by Anomie):
AuthManager: Break AuthPlugin::addUser more explicitly

https://gerrit.wikimedia.org/r/299175

Change 299175 merged by jenkins-bot:
AuthManager: Break AuthPlugin::addUser more explicitly

https://gerrit.wikimedia.org/r/299175

Paladox added a comment.EditedJul 20 2016, 9:53 AM

@Anomie is there a way we can support Special:CreateAccount as the new way to do it by what I mean is that users register as it was before and are added to a queue which then a user looks through and approves the accounts which then get put through the CreateAccount special page but one thing though we will also need password generated and sent to the email please. (Just wondering if there is a way to support that if there is then that will fix our issues please)

See discussion on T110451.

Kghbln added a comment.EditedJul 20 2016, 1:12 PM

@Paladox Until a new developer is found for this extension we have to assume that it is an abandoned extension. That's obviously the way it is. Since it is used on 4000+ wikis I hope that somebody comes along to give it a go at fixing it.

T110451 suggests that this extension needs a complete rewrite or even better needs to be replaced by an extension probably called "CreateAccount" which basically just collects the registration data and creates the actual account after somebody pressed the confirmation data. That's at least what I have understood from the discussion there.

Basically this is what I do now. I manhandle "ConfirmAccount" to collect the registration data, create a second account with them and decline the original request afterwards.

aaron added a comment.Jul 20 2016, 4:49 PM

Instead of admins creating the account and a password getting emailed the email should have a link to a pre-filled CreateAccount page with a token, which lets the user create the account. The user page and groups would have to be added by hooks there instead.

Tgr lowered the priority of this task from Unbreak Now! to High.Jul 21 2016, 8:13 PM

As in T110451, let's not mark tasks about abandoned code as UBN.

It is unmaintained yes but is used by a lot of users. And is also on the top 15 list in ExtensionDistributor, so it may be unmaintained dosent necessary mean it is abandoned by it's users.

Tgr added a comment.Jul 21 2016, 9:17 PM

The point is that marking something as UBN will not magically make it maintained but it might interfere with emergency response for components which do have maintainers, by creating extra noise. A warning template on the extension description page is probably a more effective way of advertising that it needs to be updated for 1.27.

I must note that an account creation process is a dramatic feature in wikis. We have looked extensively for alternatives to this plugin, and eventually came to realize it was our best option. Until a good alternative comes up this is a serious setback.

aaron added a comment.Jul 22 2016, 1:24 AM

It's not quite "unmaintained" as I make fixes here and there. This just happens to be a really invasive breaking change, which I haven't yet had the time to fix.

My wiki is also suffering from this, don't really want to roll back to 1.26 but this extension is the easiest way to fight spam. I'd be happy to help update it however I have almost no mediawiki development experience

Tuxxic added a subscriber: Tuxxic.Aug 1 2016, 1:02 PM
Kghbln added a comment.EditedAug 1 2016, 1:14 PM

It's not quite "unmaintained" as I make fixes here and there. This just happens to be a really invasive breaking change, which I haven't yet had the time to fix.

Thanks for clarifying. I amended the docu page accordingly.

robkam added a subscriber: robkam.Aug 1 2016, 1:28 PM
Albert added a subscriber: Albert.Aug 5 2016, 9:10 PM
Paladox added a comment.EditedAug 11 2016, 5:06 PM

@Tgr and @aaron I guess what we could do is use sql to store users requesting accounts being created. And then a autherised users who looks through the list and confirms they can be created, that can then go through Special:CreateAccount including it randomaly creating the password and then it sends it to the users email for confirming.

A autherised users can also reject meaning it wont be processed.

But I think this may require a change in core to support this please?

It would just change how it does things underneath but instead of a user confirming his / her account can be created, it will be created, just will need the user to confirm.

Tgr added a comment.Aug 12 2016, 8:30 PM

Instead of admins creating the account and a password getting emailed the email should have a link to a pre-filled CreateAccount page with a token, which lets the user create the account. The user page and groups would have to be added by hooks there instead.

That sounds a bit like InviteSignup (T134885).

@Tgr yes, but it is different since ConfirmAccount is a user requests an account and a approved user can confirm the user and the account is created.

Tgr added a comment.Aug 12 2016, 8:57 PM

Instead of admins creating the account and a password getting emailed the email should have a link to a pre-filled CreateAccount page with a token, which lets the user create the account. The user page and groups would have to be added by hooks there instead.

That sounds a bit like InviteSignup (T134885).

On secund thought, not really. I outlined some possible steps to do it in T110451#2549359.

Paladox closed this task as Resolved.Aug 21 2016, 11:49 AM
Paladox assigned this task to aaron.

This is really awesome news that this extension now works with MW 1.27. Thanks a lot @aaron for helping wikis using this out of their misery!!!

aaron added a comment.Aug 24 2016, 3:36 PM

There is still an annoying bug in core that makes the returnto link to CreateAccount instead of ConfirmAccount. I have a patch in https://gerrit.wikimedia.org/r/#/c/305834/.