Username validation in signup and username change forms

app/assets/javascripts/discourse/controllers/preferences_username_controller.js.coffee

@@ -1,6 +1,7 @@
 Discourse.PreferencesUsernameController = Ember.ObjectController.extend Discourse.Presence,
 
   taken: false
+  invalid: false
   saving: false
   error: false
 
@@ -8,8 +9,10 @@ Discourse.PreferencesUsernameController = Ember.ObjectController.extend Discours
     return true if @get('saving')
     return true if @blank('newUsername')
     return true if @get('taken')
+    return true if @get('invalid')
     return true if @get('unchanged')
-  ).property('newUsername', 'taken', 'unchanged', 'saving')
+    false
+  ).property('newUsername', 'taken', 'invalid', 'unchanged', 'saving')
 
   unchanged: (->
     @get('newUsername') == @get('content.username')
@@ -17,10 +20,14 @@ Discourse.PreferencesUsernameController = Ember.ObjectController.extend Discours
 
   checkTaken: (->
     @set('taken', false)
+    @set('invalid', false)
     return if @blank('newUsername')
     return if @get('unchanged')
     Discourse.User.checkUsername(@get('newUsername')).then (result) =>
-      @set('taken', true) unless result.available
+      if result.errors
+        @set('invalid', true)
+      else if result.available == false
+        @set('taken', true)
   ).observes('newUsername')
 
   saveButtonText: (->

app/assets/javascripts/discourse/templates/user/username.js.handlebars

@@ -23,6 +23,9 @@
       {{#if controller.taken}}
         {{i18n user.change_username.taken}}
       {{/if}}
+      {{#if controller.invalid}}
+        {{i18n user.change_username.invalid}}
+      {{/if}}
     </div>
   </div>
 

app/assets/javascripts/discourse/views/user/preferences_username_view.js.coffee

@@ -2,6 +2,13 @@ window.Discourse.PreferencesUsernameView = Ember.View.extend
   templateName: 'user/username'
   classNames: ['user-preferences']
 
-
   didInsertElement: ->
     $('#change_username').focus()
+
+  keyDown: (e) ->
+    if e.keyCode is 13
+      unless @get('controller').get('saveDisabled')
+        @get('controller').changeUsername()
+      else
+        e.preventDefault()
+        return false

app/controllers/users_controller.rb

@@ -76,7 +76,9 @@ class UsersController < ApplicationController
   def check_username
     requires_parameter(:username)
 
-    if !SiteSetting.call_mothership?
+    if !User.username_valid?(params[:username])
+      render json: {errors: [I18n.t("user.username.characters")]}
+    elsif !SiteSetting.call_mothership?
       if User.username_available?(params[:username])
         render json: {available: true}
       else

app/models/user.rb

@@ -112,6 +112,12 @@ class User < ActiveRecord::Base
     !User.where(username_lower: lower).exists?
   end
 
+  def self.username_valid?(username)
+    u = User.new(username: username)
+    u.username_format_validator
+    !u.errors[:username].present?
+  end
+
   def enqueue_welcome_message(message_type)
     return unless SiteSetting.send_welcome_message?
     Jobs.enqueue(:send_system_message, user_id: self.id, message_type: message_type)
@@ -367,6 +373,30 @@ class User < ActiveRecord::Base
     Guardian.new(self)
   end
 
+  def username_format_validator
+    unless username
+      return errors.add(:username, I18n.t(:'user.username.blank'))
+    end
+
+    if username.length < User.username_length.begin
+      return errors.add(:username, I18n.t(:'user.username.short', min: User.username_length.begin))
+    end
+
+    if username.length > User.username_length.end
+      return errors.add(:username, I18n.t(:'user.username.long', max: User.username_length.end))
+    end
+
+    if username =~ /[^A-Za-z0-9_]/
+      return errors.add(:username, I18n.t(:'user.username.characters'))
+    end
+
+    if username[0,1] =~ /[^A-Za-z0-9]/
+      return errors.add(:username, I18n.t(:'user.username.must_begin_with_alphanumeric'))
+    end
+    nil
+  end
+
+
   protected
 
     def cook
@@ -426,38 +456,19 @@ class User < ActiveRecord::Base
       self.username_lower = username.downcase
     end
 
+    def username_validator
+      username_format_validator || begin
+        lower = username.downcase
+        if username_changed? && User.where(username_lower: lower).exists?
+          return errors.add(:username, I18n.t(:'user.username.unique'))
+        end
+      end
+    end
+
     def password_validator
       if @raw_password
         return errors.add(:password, "must be 6 letters or longer") if @raw_password.length < 6
       end
     end
 
-    def username_validator
-      unless username
-        return errors.add(:username, I18n.t(:'user.username.blank'))
-      end
-
-      if username.length < User.username_length.begin
-        return errors.add(:username, I18n.t(:'user.username.short', min: User.username_length.begin))
-      end
-
-      if username.length > User.username_length.end
-        return errors.add(:username, I18n.t(:'user.username.long', max: User.username_length.end))
-      end
-
-      if username =~ /[^A-Za-z0-9_]/
-        return errors.add(:username, I18n.t(:'user.username.characters'))
-      end
-
-      if username[0,1] =~ /[^A-Za-z0-9]/
-        return errors.add(:username, I18n.t(:'user.username.must_begin_with_alphanumeric'))
-      end
-
-      lower = username.downcase
-      if username_changed? && User.where(username_lower: lower).exists?
-        return errors.add(:username, I18n.t(:'user.username.unique'))
-      end
-
-    end
-
 end

config/locales/en.yml

@@ -474,6 +474,7 @@ en:
         confirm: "There could be consequences to changing your username. Are you absolutely sure you want to?"
         taken: "Sorry that username is taken."
         error: "There was an error changing your username."
+        invalid: "That username is invalid. It must only include numbers and letters"
       change_email:
         action: 'change email'
         title: "Change Email"

spec/controllers/users_controller_spec.rb

@@ -483,6 +483,24 @@ describe UsersController do
         end
         it_should_behave_like 'when username is unavailable locally'
       end
+
+      context 'has invalid characters' do
+        before do
+          xhr :get, :check_username, username: 'bad username'
+        end
+
+        it 'should return success' do
+          response.should be_success
+        end
+
+        it 'should not return an available key' do
+          ::JSON.parse(response.body)['available'].should be_nil
+        end
+
+        it 'should return an error message' do
+          ::JSON.parse(response.body)['errors'].should_not be_empty
+        end
+      end
     end
 
     context 'when call_mothership is enabled' do

spec/models/user_spec.rb

@@ -394,6 +394,20 @@ describe User do
     end
   end
 
+  context '.username_valid?' do
+    it 'returns true when username is both valid and available' do
+      User.username_valid?('Available').should be_true
+    end
+
+    it 'returns true when the username is valid but not available' do
+      User.username_valid?(Fabricate(:user).username).should be_true
+    end
+
+    it 'returns false when the username is not valid' do
+      User.username_valid?('not valid.name').should be_false
+    end
+  end
+
   describe '.suggest_username' do
     it 'corrects weird characters' do
       User.suggest_username("Darth%^Vadar").should == "Darth_Vadar"