Closed Bug 574276 Opened 11 years ago Closed 11 years ago
Add support for a nickname blacklist
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
Assignee: nobody → chenba
Target Milestone: --- → 5.11.5
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?
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.
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. :-/
Made some comments on IRC
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.
Made changes based on feedback on github. New squashed commit @ http://github.com/chenba/zamboni/commit/29315d6671fbe5ef7c13b6baa922fc5f9bc48985 Still no tests yet :(
Once more, with feeling, and tests: http://github.com/chenba/zamboni/commit/aea0555b023bbe44254887e9dd6d655789791f50
Changes based on clouserw and jbalogh's comments are at http://github.com/chenba/zamboni/commit/5af469aa77ed3a2ee2d52cae3a96c86d0f4cd772
I think this is looking great. Can you squash those two commits?
Rebased against jbalogh's master and squashed: http://github.com/chenba/zamboni/commit/26fd705c61be72063a47cceea7a61dc9578d52a3
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
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [Q32010] → [Q32010][qa-]
(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
@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.
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.
Stripping whitespaces, case insensitivity, and more: http://github.com/chenba/zamboni/commit/2531ea6a8bd8b015280789d418b21450d04044c9
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.
btw, your latest change is http://github.com/jbalogh/zamboni/commit/4d713534fcd8e40a524594579f3941124db10cc5
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.