574276 - Add support for a nickname blacklist

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Code Quality, defect)

Description

[Wil Clouser [:clouserw]]

We need a database table that holds values we don't want to allow as nicknames.  This should be editable via the admin tools.  This bug is for:

- modifying the db
- implementing the admin tools
- modifying the zamboni user registration page to check the blacklist

Comment 1

[Barry Chen]

What type of input(s) should the admin editing form accept?  Are the blacklisted nicknames going to be managed one at a time, or does it need some sort of mass editing form?

Comment 2

[Wil Clouser [:clouserw]]

I think a text area accepting commas, newlines or spaces as separators is good for creation.  They should be listed in a table above that with a delete link.  We don't need editing.  Add pagination for bonus points - the code already exists for all our add-on listing pages.

Comment 3

[Barry Chen]

Implementation up on github (http://github.com/chenba/zamboni.git).

It adds a new model to the users app.  The add form uses newlines as separators because the registration form accepts any non-empty values as nicknames, so commas and other characters can't be used as separators.

No tests yet. :-/

Comment 4

[Wil Clouser [:clouserw]]

Made some comments on IRC

Comment 5

[Barry Chen]

I was a doofus.  Then I leveled up to n00b and created a branch.  The changes are in the 574276-nickname-blacklist branch.  Diff @ http://github.com/chenba/zamboni/compare/master...574276-nickname-blacklist.

Comment 6

[Barry Chen]

Made changes based on feedback on github.  New squashed commit @ http://github.com/chenba/zamboni/commit/29315d6671fbe5ef7c13b6baa922fc5f9bc48985

Still no tests yet :(

Comment 7

[Barry Chen]

Once more, with feeling, and tests: http://github.com/chenba/zamboni/commit/aea0555b023bbe44254887e9dd6d655789791f50

Comment 8

[Barry Chen]

Changes based on clouserw and jbalogh's comments are at http://github.com/chenba/zamboni/commit/5af469aa77ed3a2ee2d52cae3a96c86d0f4cd772

Comment 9

[Wil Clouser [:clouserw]]

I think this is looking great.  Can you squash those two commits?

Comment 10

[Barry Chen]

Rebased against jbalogh's master and squashed: http://github.com/chenba/zamboni/commit/26fd705c61be72063a47cceea7a61dc9578d52a3

Comment 11

[Wil Clouser [:clouserw]]

Merged your branch to master: http://github.com/jbalogh/zamboni/commit/26fd705c61be72063a47cceea7a61dc9578d52a3

Thanks!  Marking this qa- since the registration form isn't ready for qa yet.

Comment 12

[David Chan [:dchan]]

(In reply to comment #3)
> Implementation up on github (http://github.com/chenba/zamboni.git).
> 
> It adds a new model to the users app.  The add form uses newlines as separators
> because the registration form accepts any non-empty values as nicknames, so
> commas and other characters can't be used as separators.
> 
> No tests yet. :-/

Are there any protections in place to stop "similar" nicknames. I'm specifically thinking about trailing/leading whitespace. I couldn't find code in forms.py that performed whitespace-stripping during clean() . 

Assuming "IE6Fan" is blacklisted, a user could potentially input " IE6Fan" as their nickname which would not be in the blacklist and is rendered the same in the browser. A leading whitespace may also get around the 
"* User profile with this Nickname already exists." message when registering/editing the user profile

Comment 13

[Barry Chen]

@David Yeah good point. I think trimming whitespace on form inputs in general is out of the scope of this bug.  Preferably the nickname's already stripped of extra whitespace by the time it's being checked against the blacklist.  The blacklist admin add form should trim its input though.

I guess one step further would be doing case insensitive checks (e.g. ie6Fan).

Let's see what clouserw thinks.

Comment 14

[Wil Clouser [:clouserw]]

nickname blacklist has actually become "username blacklist" in one of my branches because of the migration (http://blog.mozilla.com/addons/2010/07/26/upcoming-changes-to-amo-accounts/).  When that goes live, usernames are going through a slugify() anyway which will trim whitespace.

As for this bug, please trim whitespace on the admin form and make any validation against it insensitive.  Thanks.

Comment 15

[Barry Chen]

Stripping whitespaces, case insensitivity, and more: http://github.com/chenba/zamboni/commit/2531ea6a8bd8b015280789d418b21450d04044c9

Comment 16

[Wil Clouser [:clouserw]]

Thanks, it's looking good, although UTF-8 still fails for me.  I filed bug 585494 so we can stop talking in this closed bug and you can rebase stuff and make a new patch.

Comment 17

[Wil Clouser [:clouserw]]

btw, your latest change is http://github.com/jbalogh/zamboni/commit/4d713534fcd8e40a524594579f3941124db10cc5