diff --git a/app/assets/javascripts/discourse/components/user-selector.js.es6 b/app/assets/javascripts/discourse/components/user-selector.js.es6 index d3bed034e29..5b0be280c10 100644 --- a/app/assets/javascripts/discourse/components/user-selector.js.es6 +++ b/app/assets/javascripts/discourse/components/user-selector.js.es6 @@ -25,7 +25,7 @@ export default TextField.extend({ dataSource: function(term) { return userSearch({ - term: term.replace(/[^a-zA-Z0-9_]/, ''), + term: term.replace(/[^a-zA-Z0-9_\-\.]/, ''), topicId: self.get('topicId'), exclude: excludedUsernames(), includeGroups, diff --git a/app/assets/javascripts/discourse/dialects/mention_dialect.js b/app/assets/javascripts/discourse/dialects/mention_dialect.js index 4832d436b85..4f1b8b8da17 100644 --- a/app/assets/javascripts/discourse/dialects/mention_dialect.js +++ b/app/assets/javascripts/discourse/dialects/mention_dialect.js @@ -7,7 +7,7 @@ Discourse.Dialect.inlineRegexp({ start: '@', // 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 - 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, emitter: function(matches) { diff --git a/app/assets/javascripts/discourse/lib/user-search.js.es6 b/app/assets/javascripts/discourse/lib/user-search.js.es6 index 790a00ff091..3600814bb8e 100644 --- a/app/assets/javascripts/discourse/lib/user-search.js.es6 +++ b/app/assets/javascripts/discourse/lib/user-search.js.es6 @@ -89,7 +89,7 @@ export default function userSearch(options) { return new Ember.RSVP.Promise(function(resolve) { // TODO site setting for allowed regex in username - if (term.match(/[^a-zA-Z0-9_\.]/)) { + if (term.match(/[^a-zA-Z0-9_\.\-]/)) { resolve([]); return; } diff --git a/app/models/username_validator.rb b/app/models/username_validator.rb index 3b350d289ea..b93ef78a03e 100644 --- a/app/models/username_validator.rb +++ b/app/models/username_validator.rb @@ -30,6 +30,9 @@ class UsernameValidator username_length_max? username_char_valid? username_first_char_valid? + username_last_char_valid? + username_no_double_special? + username_does_not_end_with_confusing_suffix? errors.empty? end @@ -58,15 +61,36 @@ class UsernameValidator def username_char_valid? return unless errors.empty? - if username =~ /[^A-Za-z0-9_]/ + if username =~ /[^A-Za-z0-9_\.\-]/ self.errors << I18n.t(:'user.username.characters') end end def username_first_char_valid? 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') 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 diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 0b7c47b3aff..77a8754d889 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1343,7 +1343,10 @@ en: characters: "must only include numbers, letters and underscores" unique: "must be unique" 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: not_allowed: "is not allowed from that email provider. Please use another email address." blocked: "is not allowed." diff --git a/config/routes.rb b/config/routes.rb index 3675a5e769a..c3164a2ff81 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -7,7 +7,7 @@ require_dependency "permalink_constraint" # This used to be User#username_format, but that causes a preload of the User object # 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 Discourse::Application.routes.draw do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e2bd09f772d..5eaa7f64196 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -338,29 +338,57 @@ describe User do end describe 'username format' do - it "should be #{SiteSetting.min_username_length} chars or longer" do - @user = Fabricate.build(:user) - @user.username = 'ss' - expect(@user.save).to eq(false) + def assert_bad(username) + user = Fabricate.build(:user) + user.username = username + expect(user.valid?).to eq(false) end - it "should never end with a ." do - @user = Fabricate.build(:user) - @user.username = 'sam.' - expect(@user.save).to eq(false) + def assert_good(username) + user = Fabricate.build(:user) + user.username = username + expect(user.valid?).to eq(true) end - it "should never contain spaces" do - @user = Fabricate.build(:user) - @user.username = 'sam s' - expect(@user.save).to eq(false) + it "should be SiteSetting.min_username_length chars or longer" do + SiteSetting.min_username_length = 5 + assert_bad("abcd") + assert_good("abcde") end - ['Bad One', 'Giraf%fe', 'Hello!', '@twitter', 'me@example.com', 'no.dots', 'purple.', '.bilbo', '_nope', 'sa$sy'].each do |bad_nickname| - it "should not allow username '#{bad_nickname}'" do - @user = Fabricate.build(:user) - @user.username = bad_nickname - expect(@user.save).to eq(false) + %w{ first.last + first first-last + _name first_last + mc.hammer_nose + 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 diff --git a/spec/services/username_checker_service_spec.rb b/spec/services/username_checker_service_spec.rb index 258003b3fed..7b9a4094101 100644 --- a/spec/services/username_checker_service_spec.rb +++ b/spec/services/username_checker_service_spec.rb @@ -26,7 +26,7 @@ describe UsernameCheckerService do end 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) end end