Page MenuHomePhabricator

AntiSpoof should return more than one result
Closed, ResolvedPublic

Description

Author: sxwiki

Description:
Per the reasons here:

http://en.wikipedia.org/wiki/WT:ACC#We_may_have_a_.28minor.29_problem

I really think, it would be helpful, if AntiSpoof could be configured to return multiple results, (say, maybe 3-5?) when user registration is denied due to a similar username.

(It said I had to select a component, or, just guess, so, since AntiSpoof isn't listed here, I just picked something at random near the top -- I hope this isn't a problem)


Version: unspecified
Severity: enhancement
URL: http://www.mediawiki.org/wiki/Extension:AntiSpoof

Details

Reference
bz12232

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 9:56 PM
bzimport added a project: AntiSpoof.
bzimport set Reference to bz12232.
bzimport added a subscriber: Unknown Object (MLST).

(Added component to file bug correctly.)

sxwiki wrote:

Perhaps, as well, it should return any and all Admin/Crat accounts, that match, and, mark them as such.

soxred93 wrote:

Second this request. Very good idea.

This can also return a match which has no contribs, but another close match may be an active account. This in my opinion is important, as it will eliminate this risk.

sxwiki wrote:

First patch - Displays up to 5 matching usernames.

Added a rough patch to accomplish this. I wasn't quite sure how multiple names should be displayed, in the mediawiki: message, however, so that bit is a little clumsy yet. Tested it against HEAD.

attachment antispoof.patch ignored as obsolete

sxwiki wrote:

Comment on attachment 5015
First patch - Displays up to 5 matching usernames.

Patch was against REL1.12, not head, would not work.

Created attachment 5024
Patch against HEAD - Also puts a nice "and more" message if there are more than five.

This is essentially the same as the last patch, except it is against HEAD and it has a useful "and more" message at the end of the warning if there are more conflicts than the script is made to handle.

attachment AntiSpoof.diff ignored as obsolete

Comment on attachment 5024
Patch against HEAD - Also puts a nice "and more" message if there are more than five.

Index: extensions/AntiSpoof/AntiSpoof.i18n.php

  • extensions/AntiSpoof/AntiSpoof.i18n.php (revision 36517)

+++ extensions/AntiSpoof/AntiSpoof.i18n.php (working copy)
@@ -1,4 +1,4 @@
-<?php
+<?php
/**

  • Internationalisation file for extension AntiSpoof. *

@@ -9,7 +9,7 @@

$messages['en'] = array(

	'antispoof-desc'          => 'Blocks the creation of accounts with mixed-script, confusing and similar usernames',
  • 'antispoof-name-conflict' => 'The name "$1" is too similar to the existing account "$2".

+ 'antispoof-name-conflict' => ''The name "$1" is too similar to existing accounts: $2 $3 $4 $5 $6 $7.
Please choose another name.',

	'antispoof-name-illegal'  => 'The name "$1" is not allowed to prevent confusing or spoofed usernames: $2.

Please choose another name.',

Index: extensions/AntiSpoof/SpoofUser.php

  • extensions/AntiSpoof/SpoofUser.php (revision 36517)

+++ extensions/AntiSpoof/SpoofUser.php (working copy)
@@ -47,17 +47,26 @@

			// Join against the user table to ensure that we skip stray
			// entries left after an account is renamed or otherwise munged.
  • $row = $dbr->selectRow(
  • array( 'spoofuser', 'user' ),
  • array( 'user_name' ),
  • array(

+ $spoofedUsers = $dbr->selectRow(
+ array( 'spoofuser', 'user' ),
+ array( 'user_name' ),
+ array(

					'su_normalized' => $this->mNormalized,
					'su_name=user_name',
  • ),
  • METHOD );

+ ),
+ METHOD,
+ array( 'LIMIT' => 6 ) );

  • if( $row ) {
  • return $row->user_name;

+ $spoofs = array();
+ while( $row = $dbr->fetchObject( $spoofedUsers ) {
+ $spoofs[] = $row->user_name;
+ }
+ if( count( $spoofs ) > 0 && < 6 ) {
+ $spoofs[] = false;
+ return $spoofs;
+ } elseif( count( $spoofs > 6 ) {
+ $spoofs[6] = true;
+ return $spoofs;

			} else {
				return false;
			}

Index: extensions/AntiSpoof/AntiSpoof.php

  • extensions/AntiSpoof/AntiSpoof.php (revision 36517)

+++ extensions/AntiSpoof/AntiSpoof.php (working copy)
@@ -83,10 +83,23 @@

		if( $conflict === false ) {
			wfDebugLog( 'antispoof', "{$mode}PASS new account '$name' [$normalized]" );
		} else {
  • wfDebugLog( 'antispoof', "{$mode}CONFLICT new account '$name' [$normalized] spoofs '$conflict'" );
  • if( $active ) {
  • $message = wfMsg( 'antispoof-name-conflict', $name, $conflict );
  • return false;

+ $spoofNumber = count( $conflict ) - 1;
+ $DebugLog = "{$mode}CONFLICT new account '$name' [$normalized] spoofs";
+ for( $i = 0; $i < $spoofNumber; $i++ ) {
+ $DebugLog .= " $conflict[$i];";
+ }
+ if( !$conflict[ $spoofNumber ] ) {
+ wfDebugLog( 'antispoof', $DebugLog );
+ if( $active ) {
+ $message = wfMsg( 'antispoof-name-conflict', $name, $conflict[0], $conflict[1], $conflict[2], $conflict[3], $conflict[4], $conflict[5], '' );
+ return false;
+ }
+ } else {
+ wfDebugLog( 'antispoof', $DebugLog . "; MORE;" );
+ if( $active ) {
+ $message = wfMsg( 'antispoof-name-conflict', $name, $conflict[0], $conflict[1], $conflict[2], $conflict[3], $conflict[4], $conflict[5], 'and more' );
+ return false;
+ }

			}
		}
	} else {

overlordq wrote:

Correct patch against HEAD

Previous patch against HEAD is dirty and creates PHP errors.

This one follows the original patch, but is against HEAD.

attachment AntiSpoof.patch.txt ignored as obsolete

sxwiki wrote:

Patch against HEAD - Minor cleanup

Cleaned up OQ's patch (spacing, counting $conflict is no longer needed, was an experiment in formatting from a while ago)

attachment antispoof2.patch ignored as obsolete

Latest patch applied in r36851.

sxwiki wrote:

Resolved again in r37932 .

overlordq wrote:

Patch that should address brions concerns

Just in case brion reverts again, (which I think he will since r37932 still breaks the customized message stuff), I've created this patch which should address what brion wants.

Example output:
http://i36.tinypic.com/2md4aba.jpg

Default Single Conflict
Customized Single Conflict
Default Multi Conflict
Customized Multi Conflict

Patch is against the version brion reverted to.

attachment AntiSpoof.patch ignored as obsolete

overlordq wrote:

Yet another Patch

Horrible hack to use the suggested plural parser func and a listish style format which will also allow using the variables to create links/etc

attachment AntiSpoof.txt ignored as obsolete

overlordq wrote:

reverted in r37938

Couple notes on the patch:

  • I recommend renaming $conflict to $conflicts to indicate it's an array now
  • 'antispoof-conflict-top' message cannot end with a space, as that will cause troubles when customizing it (trailing space is always trimmed from wiki pages)
  • You only need one {{PLURAL:...}} here; two in a row is unnecessary :)
  • 'antispoof-conflict-body' is an unclear message name; I'd recommend 'antispoof-conflict-item' instead
  • The hack with $wgMessageCache->transform() is unnecessary -- use wfMsgExt() with 'parsemag' option
  • Just return an array consistently (empty is ok) rather than false when no matches found.

overlordq wrote:

Yet another Patch

Alright, same as before but hopefully addressing the critiques.

attachment AntiSpoof.patch ignored as obsolete

overlordq wrote:

Comment on attachment 5109
Yet another Patch

*facepalm* forgot to mark it as patch.

Looks good -- one minor nit, we seem to have lost the "Please choose another name" bit off the end. Perhaps split that also from antispoof-name-illegal and have it as a separate paragraph in both cases.

Probably not necessary to do separate formatting for the single-item and multiple-item list cases. A bullet list is fine even if it's just one -- it'll be nicely set off, and ensures that formatting is consistent and predictable.

And while we're at it -- rename getConflict() to getConflicts() just to make things pretty. :D

overlordq wrote:

Last one?

Hopefully address all the remaining concerns and also remove an unneeded (I think) call to isLegal.

Attached: