FEATURE: relax username rules to allow - and . and leading _

This relaxes our very strict username rules to allow for some long asked for requests

- leading _ is now allowed
- . is allowed except for trailing char and confusing extensions like .gif .json
- dash (-) is now permitted
This commit is contained in:
Sam 2015-09-02 12:13:44 +10:00
parent 0a46ec9c50
commit 262f561a87
8 changed files with 80 additions and 25 deletions

View File

@ -25,7 +25,7 @@ export default TextField.extend({
dataSource: function(term) { dataSource: function(term) {
return userSearch({ return userSearch({
term: term.replace(/[^a-zA-Z0-9_]/, ''), term: term.replace(/[^a-zA-Z0-9_\-\.]/, ''),
topicId: self.get('topicId'), topicId: self.get('topicId'),
exclude: excludedUsernames(), exclude: excludedUsernames(),
includeGroups, includeGroups,

View File

@ -7,7 +7,7 @@ Discourse.Dialect.inlineRegexp({
start: '@', start: '@',
// NOTE: we really should be using SiteSettings here, but it loads later in process // NOTE: we really should be using SiteSettings here, but it loads later in process
// also, if we do, we must ensure serverside version works as well // also, if we do, we must ensure serverside version works as well
matcher: /^(@[A-Za-z0-9][A-Za-z0-9_]{0,40})/, matcher: /^(@[A-Za-z0-9][A-Za-z0-9_\.\-]{0,40}[A-Za-z0-9])/,
wordBoundary: true, wordBoundary: true,
emitter: function(matches) { emitter: function(matches) {

View File

@ -89,7 +89,7 @@ export default function userSearch(options) {
return new Ember.RSVP.Promise(function(resolve) { return new Ember.RSVP.Promise(function(resolve) {
// TODO site setting for allowed regex in username // TODO site setting for allowed regex in username
if (term.match(/[^a-zA-Z0-9_\.]/)) { if (term.match(/[^a-zA-Z0-9_\.\-]/)) {
resolve([]); resolve([]);
return; return;
} }

View File

@ -30,6 +30,9 @@ class UsernameValidator
username_length_max? username_length_max?
username_char_valid? username_char_valid?
username_first_char_valid? username_first_char_valid?
username_last_char_valid?
username_no_double_special?
username_does_not_end_with_confusing_suffix?
errors.empty? errors.empty?
end end
@ -58,15 +61,36 @@ class UsernameValidator
def username_char_valid? def username_char_valid?
return unless errors.empty? return unless errors.empty?
if username =~ /[^A-Za-z0-9_]/ if username =~ /[^A-Za-z0-9_\.\-]/
self.errors << I18n.t(:'user.username.characters') self.errors << I18n.t(:'user.username.characters')
end end
end end
def username_first_char_valid? def username_first_char_valid?
return unless errors.empty? return unless errors.empty?
if username[0] =~ /[^A-Za-z0-9]/ if username[0] =~ /[^A-Za-z0-9_]/
self.errors << I18n.t(:'user.username.must_begin_with_alphanumeric') self.errors << I18n.t(:'user.username.must_begin_with_alphanumeric')
end end
end end
def username_last_char_valid?
return unless errors.empty?
if username[-1] =~ /[^A-Za-z0-9]/
self.errors << I18n.t(:'user.username.must_end_with_alphanumeric')
end
end
def username_no_double_special?
return unless errors.empty?
if username =~ /[\-_\.]{2,}/
self.errors << I18n.t(:'user.username.must_not_contain_two_special_chars_in_seq')
end
end
def username_does_not_end_with_confusing_suffix?
return unless errors.empty?
if username =~ /\.(json|gif|jpeg|png|htm|js|json|xml|woff|tif|html)/i
self.errors << I18n.t(:'user.username.must_not_contain_confusing_suffix')
end
end
end end

View File

@ -1343,7 +1343,10 @@ en:
characters: "must only include numbers, letters and underscores" characters: "must only include numbers, letters and underscores"
unique: "must be unique" unique: "must be unique"
blank: "must be present" blank: "must be present"
must_begin_with_alphanumeric: "must begin with a letter or number" must_begin_with_alphanumeric: "must begin with a letter or number or an underscore"
must_end_with_alphanumeric: "must end with a letter or number"
must_not_contain_two_special_chars_in_seq: "must not contain a sequence of 2 or more special chars (.-_)"
must_not_contain_confusing_suffix: "must not contain a confusing suffix like .json or .png etc."
email: email:
not_allowed: "is not allowed from that email provider. Please use another email address." not_allowed: "is not allowed from that email provider. Please use another email address."
blocked: "is not allowed." blocked: "is not allowed."

View File

@ -7,7 +7,7 @@ require_dependency "permalink_constraint"
# This used to be User#username_format, but that causes a preload of the User object # This used to be User#username_format, but that causes a preload of the User object
# and makes Guard not work properly. # and makes Guard not work properly.
USERNAME_ROUTE_FORMAT = /[A-Za-z0-9\_]+/ unless defined? USERNAME_ROUTE_FORMAT USERNAME_ROUTE_FORMAT = /[A-Za-z0-9\_.\-]+/ unless defined? USERNAME_ROUTE_FORMAT
BACKUP_ROUTE_FORMAT = /[a-zA-Z0-9\-_]*\d{4}(-\d{2}){2}-\d{6}\.(tar\.gz|t?gz)/i unless defined? BACKUP_ROUTE_FORMAT BACKUP_ROUTE_FORMAT = /[a-zA-Z0-9\-_]*\d{4}(-\d{2}){2}-\d{6}\.(tar\.gz|t?gz)/i unless defined? BACKUP_ROUTE_FORMAT
Discourse::Application.routes.draw do Discourse::Application.routes.draw do

View File

@ -338,29 +338,57 @@ describe User do
end end
describe 'username format' do describe 'username format' do
it "should be #{SiteSetting.min_username_length} chars or longer" do def assert_bad(username)
@user = Fabricate.build(:user) user = Fabricate.build(:user)
@user.username = 'ss' user.username = username
expect(@user.save).to eq(false) expect(user.valid?).to eq(false)
end end
it "should never end with a ." do def assert_good(username)
@user = Fabricate.build(:user) user = Fabricate.build(:user)
@user.username = 'sam.' user.username = username
expect(@user.save).to eq(false) expect(user.valid?).to eq(true)
end end
it "should never contain spaces" do it "should be SiteSetting.min_username_length chars or longer" do
@user = Fabricate.build(:user) SiteSetting.min_username_length = 5
@user.username = 'sam s' assert_bad("abcd")
expect(@user.save).to eq(false) assert_good("abcde")
end end
['Bad One', 'Giraf%fe', 'Hello!', '@twitter', 'me@example.com', 'no.dots', 'purple.', '.bilbo', '_nope', 'sa$sy'].each do |bad_nickname| %w{ first.last
it "should not allow username '#{bad_nickname}'" do first first-last
@user = Fabricate.build(:user) _name first_last
@user.username = bad_nickname mc.hammer_nose
expect(@user.save).to eq(false) UPPERCASE
sgif
}.each do |username|
it "allows #{username}" do
assert_good(username)
end
end
%w{
traildot.
has\ space
double__underscore
with%symbol
Exclamation!
@twitter
my@email.com
.tester
sa$sy
sam.json
sam.xml
sam.html
sam.htm
sam.js
sam.woff
sam.Png
sam.gif
}.each do |username|
it "disallows #{username}" do
assert_bad(username)
end end
end end
end end

View File

@ -26,7 +26,7 @@ describe UsernameCheckerService do
end end
it 'rejects usernames that do not start with an alphanumeric character' do it 'rejects usernames that do not start with an alphanumeric character' do
result = @service.check_username('_vincent', @nil_email) result = @service.check_username('.vincent', @nil_email)
expect(result).to have_key(:errors) expect(result).to have_key(:errors)
end end
end end