Page MenuHomePhabricator

Can't validate username against blocking extensions AntiSpoof & TitleBlacklist
Closed, ResolvedPublic

Description

The AntiSpoof extension blocks usernames that use certain Unicode ranges, are all letters or punctuation, usernames that "l0ok l1ke" existing usernames, etc. But MediaWiki only hooks into it *while* it is creating a new user account (with the AbortNewAccount hook). Because MW does not run a hook on User::isValidUserName() or variants like User::getCanonicalName('creatable'), there is no way for a validating account creation form to check if a username will be rejected by AntiSpoof before the user submits it. This reduces the effectiveness of the improved account creation that the E3 team is exploring.

The simplest fix is to introduce a new isUsableUsername hook that User.php runs from User::isUsableName(), and adapt AntiSpoof (and similar extensions like Minimum Name Length extension, etc.) to respond to it with similar code to its AbortNewAccount hook.


Version: 1.20.x
Severity: normal
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=34447

Details

Reference
bz40648

Event Timeline

bzimport raised the priority of this task from to Normal.Nov 22 2014, 12:52 AM
bzimport set Reference to bz40648.
bzimport added a subscriber: Unknown Object (MLST).
Spage created this task.Oct 1 2012, 1:50 AM

I thought this area of code had been improved somewhat recently. There was a similar issue with the Titleblacklist extension (which is the successor to the Username blacklist extension). There's now an API module that can check a particular username (title) against the title blacklists. I guess something similar for the AntiSpoof extension could be implemented?

Though I think you're right that a more abstract solution is needed.

Yes, TitleBlacklist is another one. This bug affects our A/B test of Account Creation with client-side validation: users may receive username errors when they submit, despite an earlier green checkmark implying username is OK (bug 41849).

I tried to work around the problem by running AbortNewAccount myself when validating username. But many other extensions hook into AbortNewAccount, including ConfirmEdit which fails because the (non-existent) CAPTCHA in the request doesn't match. The next hack would be to invoke particular extensions' AbortNewAccount handlers myself, but that's about 150 lines of code.

Yes, if each extension had its own API I could invoke the APIs I know about. It's better for extensions to hook into a CheckValidUsername hook. And for inter-dependent fields in account creation, we need an AboutToCreateAccount function that lets each extension consider multiple fields in the form much like AbortNewAccount and return error information, but which doesn't actually create the account.

tl;dr: Lots to do to fix this properly.

ori added a comment.Jan 13 2013, 3:28 AM
  • Bug 41849 has been marked as a duplicate of this bug. ***
  • Bug 46308 has been marked as a duplicate of this bug. ***

Related change: https://gerrit.wikimedia.org/r/#/c/87546/ (Add an API module for querying AntiSpoof results)

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 28 2015, 11:02 PM
Tgr added a subscriber: Tgr.May 17 2016, 9:04 AM

AuthManager providers can implement testUserForCreation( $user, $isAutocreation ) now. Patches for extensions used by WMF:

Change 283847 had a related patch set uploaded (by Gergő Tisza):
[WIP] Update signup form validation to use AuthManager

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

Anomie closed this task as Resolved.Jun 16 2016, 7:48 PM
Anomie claimed this task.
Anomie added a subscriber: Anomie.

AntiSpoof and TitleBlacklist can now be validated against via API action=query&list=users&usprop=cancreate, so this bug is done.

Other things not done but that aren't part of this task:

  • SpamBlacklist doesn't matter, since it checks the email address being used rather than the username.
  • AbuseFilter would be nice, but I'm told would need some major refactoring to avoid applying throttles, logging, or taking any other action when a username is just being checked, and anyway isn't actually requested here. The ability is there, it just has to do something sensible for PreAuthenticationProvider::testUserForCreation() in the non-autocreate case.
  • Actually making use of the ability seems to be T19544: Client-side validation of the username availability (done) and that password meets requirements rather than this task.