Add support for a nickname blacklist

RESOLVED FIXED in 5.11.6

Status

RESOLVED FIXED
8 years ago
3 years ago

People

(Reporter: clouserw, Assigned: chenba)

Tracking

unspecified
5.11.6

Details

(Whiteboard: [Q32010][qa-])

(Reporter)

Description

8 years ago
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
(Reporter)

Updated

8 years ago
Assignee: nobody → chenba
Target Milestone: --- → 5.11.5
(Assignee)

Comment 1

8 years ago
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?
(Reporter)

Comment 2

8 years ago
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.
(Assignee)

Comment 3

8 years ago
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. :-/
(Reporter)

Comment 4

8 years ago
Made some comments on IRC
(Assignee)

Comment 5

8 years ago
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.
(Assignee)

Comment 6

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

Still no tests yet :(
(Reporter)

Updated

8 years ago
Target Milestone: 5.11.5 → 5.11.6
(Assignee)

Comment 7

8 years ago
Once more, with feeling, and tests: http://github.com/chenba/zamboni/commit/aea0555b023bbe44254887e9dd6d655789791f50
(Assignee)

Comment 8

8 years ago
Changes based on clouserw and jbalogh's comments are at http://github.com/chenba/zamboni/commit/5af469aa77ed3a2ee2d52cae3a96c86d0f4cd772
(Reporter)

Comment 9

8 years ago
I think this is looking great.  Can you squash those two commits?
(Assignee)

Comment 10

8 years ago
Rebased against jbalogh's master and squashed: http://github.com/chenba/zamboni/commit/26fd705c61be72063a47cceea7a61dc9578d52a3
(Reporter)

Comment 11

8 years ago
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.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Whiteboard: [Q32010] → [Q32010][qa-]

Comment 12

8 years ago
(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
(Assignee)

Comment 13

8 years ago
@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.
(Reporter)

Comment 14

8 years ago
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.
(Assignee)

Comment 15

8 years ago
Stripping whitespaces, case insensitivity, and more: http://github.com/chenba/zamboni/commit/2531ea6a8bd8b015280789d418b21450d04044c9
(Reporter)

Comment 16

8 years ago
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.
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.