Block passwords that are in the top 5000 most common passwords. Site setting block_common_passwords can disable this feature.

app/assets/javascripts/discourse/controllers/create_account_controller.js

@@ -15,6 +15,7 @@ Discourse.CreateAccountController = Discourse.Controller.extend(Discourse.ModalF
   accountChallenge: 0,
   formSubmitted: false,
   rejectedEmails: Em.A([]),
+  rejectedPasswords: Em.A([]),
   prefilledUsername: null,
 
   submitDisabled: function() {
@@ -280,12 +281,19 @@ Discourse.CreateAccountController = Discourse.Controller.extend(Discourse.ModalF
       });
     }
 
+    if (this.get('rejectedPasswords').contains(password)) {
+      return Discourse.InputValidation.create({
+        failed: true,
+        reason: I18n.t('user.password.common')
+      });
+    }
+
     // Looks good!
     return Discourse.InputValidation.create({
       ok: true,
       reason: I18n.t('user.password.ok')
     });
-  }.property('accountPassword'),
+  }.property('accountPassword', 'rejectedPasswords.@each'),
 
   fetchConfirmationValue: function() {
     var createAccountController = this;
@@ -297,7 +305,7 @@ Discourse.CreateAccountController = Discourse.Controller.extend(Discourse.ModalF
 
   actions: {
     createAccount: function() {
-      var createAccountController = this;
+      var self = this;
       this.set('formSubmitted', true);
       var name = this.get('accountName');
       var email = this.get('accountEmail');
@@ -307,21 +315,24 @@ Discourse.CreateAccountController = Discourse.Controller.extend(Discourse.ModalF
       var challenge = this.get('accountChallenge');
       return Discourse.User.createAccount(name, email, password, username, passwordConfirm, challenge).then(function(result) {
         if (result.success) {
-          createAccountController.flash(result.message);
-          createAccountController.set('complete', true);
+          self.flash(result.message);
+          self.set('complete', true);
         } else {
-          createAccountController.flash(result.message || I18n.t('create_account.failed'), 'error');
-          if (result.errors && result.errors.email && result.values) {
-            createAccountController.get('rejectedEmails').pushObject(result.values.email);
+          self.flash(result.message || I18n.t('create_account.failed'), 'error');
+          if (result.errors && result.errors.email && result.errors.email.length > 0 && result.values) {
+            self.get('rejectedEmails').pushObject(result.values.email);
           }
-          createAccountController.set('formSubmitted', false);
+          if (result.errors && result.errors.password && result.errors.password.length > 0) {
+            self.get('rejectedPasswords').pushObject(password);
+          }
+          self.set('formSubmitted', false);
         }
         if (result.active) {
           return window.location.reload();
         }
       }, function() {
-        createAccountController.set('formSubmitted', false);
-        return createAccountController.flash(I18n.t('create_account.failed'), 'error');
+        self.set('formSubmitted', false);
+        return self.flash(I18n.t('create_account.failed'), 'error');
       });
     }
   }

app/assets/javascripts/discourse/views/modal/create_account_view.js

@@ -21,7 +21,7 @@ Discourse.CreateAccountView = Discourse.ModalBodyView.extend({
     Em.run.schedule('afterRender', function() {
       $("input[type='text'], input[type='password']").keydown(function(e) {
         if (createAccountController.get('submitDisabled') === false && e.keyCode === 13) {
-          createAccountController.createAccount();
+          createAccountController.send('createAccount');
         }
       });
     });

config/locales/client.en.yml

@@ -356,6 +356,7 @@ en:
       password:
         title: "Password"
         too_short: "Your password is too short."
+        common: "That password is too common."
         ok: "Your password looks good."
         instructions: "Must be at least %{count} characters."
 

config/locales/server.en.yml

@@ -160,6 +160,8 @@ en:
               cant_send_pm: "Sorry, you cannot send a private message to that user."
         user:
           attributes:
+            password:
+              common: "is one of the top 5000 most common passwords. Please use a more secure password."
             ip_address:
               signup_not_allowed: "Signup is not allowed from this account."
 
@@ -606,6 +608,7 @@ en:
     login_required: "Require authentication to read posts"
 
     min_password_length: "Minimum password length."
+    block_common_passwords: "Don't allow passwords that are in the top 5000 most common passwords."
     enable_local_logins: "Enable traditional, local username and password authentication"
     enable_local_account_create: "Enable creating new local accounts"
     enable_google_logins: "Enable Google authentication"

config/site_settings.yml

@@ -81,6 +81,7 @@ users:
   min_password_length:
     client: true
     default: 8
+  block_common_passwords: true
   enable_google_logins:
     client: true
     default: true

lib/common_passwords/common_passwords.rb

@@ -0,0 +1,50 @@
+# CommonPasswords will check a given password against a list of the most commonly used passwords.
+# The list comes from https://xato.net/passwords/more-top-worst-passwords/#.UrR1AHmpxs4
+# The list is stored in Redis at a key that is shared by all sites in a multisite config.
+#
+# If the password file is changed, you need to add a migration that deletes the list from redis
+# so it gets re-populated:
+#
+#   $redis.without_namespace.del CommonPasswords::LIST_KEY
+
+class CommonPasswords
+
+  PASSWORD_FILE = File.join(Rails.root, 'lib', 'common_passwords', '10k-common-passwords.txt')
+  LIST_KEY = 'discourse-common-passwords'
+
+  @mutex = Mutex.new
+
+  def self.common_password?(password)
+    return false unless password.present?
+    password_list.include?(password)
+  end
+
+
+  private
+
+    class RedisPasswordList
+      def include?(password)
+        CommonPasswords.redis.sismember CommonPasswords::LIST_KEY, password
+      end
+    end
+
+    def self.password_list
+      @mutex.synchronize do
+        load_passwords unless redis.exists(LIST_KEY)
+      end
+      RedisPasswordList.new
+    end
+
+    def self.redis
+      $redis.without_namespace
+    end
+
+    def self.load_passwords
+      passwords = File.readlines(PASSWORD_FILE)
+      redis.sadd LIST_KEY, passwords[0,5000].map!(&:chomp)
+    rescue Errno::ENOENT
+      # tolerate this so we don't block signups
+      Rails.logger.error "Common passwords file #{PASSWORD_FILE} is not found! Common password checking is skipped."
+    end
+
+end

lib/discourse_redis.rb

@@ -24,6 +24,11 @@ class DiscourseRedis
     @redis = DiscourseRedis.raw_connection(@config)
   end
 
+  def without_namespace
+    # Only use this if you want to store and fetch data that's shared between sites
+    @redis
+  end
+
   def url
     self.class.url(@config)
   end

lib/validators/password_validator.rb

@@ -1,3 +1,5 @@
+require_dependency "common_passwords/common_passwords"
+
 class PasswordValidator < ActiveModel::EachValidator
 
   def validate_each(record, attribute, value)
@@ -6,6 +8,8 @@ class PasswordValidator < ActiveModel::EachValidator
       record.errors.add(attribute, :blank)
     elsif value.length < SiteSetting.min_password_length
       record.errors.add(attribute, :too_short, count: SiteSetting.min_password_length)
+    elsif SiteSetting.block_common_passwords && CommonPasswords.common_password?(value)
+      record.errors.add(attribute, :common)
     end
   end
 

spec/components/common_passwords/common_passwords_spec.rb

@@ -0,0 +1,67 @@
+require "spec_helper"
+require_dependency "common_passwords/common_passwords"
+
+describe CommonPasswords do
+
+  it "the passwords file should exist" do
+    File.exists?(described_class::PASSWORD_FILE).should eq(true)
+  end
+
+  describe "#common_password?" do
+    before { described_class.stubs(:redis).returns(stub_everything) }
+
+    subject { described_class.common_password? @password }
+
+    it "returns false if password isn't in the common passwords list" do
+      described_class.stubs(:password_list).returns(stub_everything(:include? => false))
+      @password = 'uncommonPassword'
+      subject.should eq(false)
+    end
+
+    it "returns false if password is nil" do
+      described_class.expects(:password_list).never
+      @password = nil
+      subject.should eq(false)
+    end
+
+    it "returns false if password is blank" do
+      described_class.expects(:password_list).never
+      @password = ""
+      subject.should eq(false)
+    end
+
+    it "returns true if password is in the common passwords list" do
+      described_class.stubs(:password_list).returns(stub_everything(:include? => true))
+      @password = "password"
+      subject.should eq(true)
+    end
+  end
+
+  describe '#password_list' do
+    it "loads the passwords file if redis doesn't have it" do
+      mock_redis = mock("redis")
+      mock_redis.stubs(:exists).returns(false)
+      described_class.stubs(:redis).returns(mock_redis)
+      described_class.expects(:load_passwords).returns([])
+      list = described_class.password_list
+      list.should respond_to(:include?)
+    end
+
+    it "doesn't load the passwords file if redis has it" do
+      mock_redis = mock("redis")
+      mock_redis.stubs(:exists).returns(true)
+      described_class.stubs(:redis).returns(mock_redis)
+      described_class.expects(:load_passwords).never
+      list = described_class.password_list
+      list.should respond_to(:include?)
+    end
+  end
+
+  context "missing password file" do
+    it "tolerates it" do
+      described_class.stubs(:redis).returns(stub_everything(sismember: false))
+      File.stubs(:readlines).with(described_class::PASSWORD_FILE).raises(Errno::ENOENT)
+      described_class.common_password?("password").should eq(false)
+    end
+  end
+end

spec/components/validators/password_validator_spec.rb

@@ -1,4 +1,5 @@
 require 'spec_helper'
+require_dependency "common_passwords/common_passwords"
 
 describe PasswordValidator do
 
@@ -8,42 +9,68 @@ describe PasswordValidator do
   context "password required" do
     let(:record) { u = Fabricate.build(:user, password: @password); u.password_required!; u }
 
-    context "min password length is 8" do
-      before { SiteSetting.stubs(:min_password_length).returns(8) }
-
-      it "doesn't add an error when password is good" do
-        @password = "weron235alsfn234"
-        validate
-        record.errors[:password].should_not be_present
+    context "password is not common" do
+      before do
+        CommonPasswords.any_instance.stubs(:common_password?).returns(false)
       end
 
-      it "adds an error when password is too short" do
-        @password = "p"
-        validate
-        record.errors[:password].should be_present
+      context "min password length is 8" do
+        before { SiteSetting.stubs(:min_password_length).returns(8) }
+
+        it "doesn't add an error when password is good" do
+          @password = "weron235alsfn234"
+          validate
+          record.errors[:password].should_not be_present
+        end
+
+        it "adds an error when password is too short" do
+          @password = "p"
+          validate
+          record.errors[:password].should be_present
+        end
+
+        it "adds an error when password is blank" do
+          @password = ''
+          validate
+          record.errors[:password].should be_present
+        end
+
+        it "adds an error when password is nil" do
+          @password = nil
+          validate
+          record.errors[:password].should be_present
+        end
       end
 
-      it "adds an error when password is blank" do
-        @password = ''
-        validate
-        record.errors[:password].should be_present
-      end
+      context "min password length is 12" do
+        before { SiteSetting.stubs(:min_password_length).returns(12) }
 
-      it "adds an error when password is nil" do
-        @password = nil
-        validate
-        record.errors[:password].should be_present
+        it "adds an error when password length is 11" do
+          @password = "gt38sdt92bv"
+          validate
+          record.errors[:password].should be_present
+        end
       end
     end
 
-    context "min password length is 12" do
-      before { SiteSetting.stubs(:min_password_length).returns(12) }
+    context "password is commonly used" do
+      before do
+        CommonPasswords.any_instance.stubs(:common_password?).returns(true)
+      end
 
-      it "adds an error when password length is 11" do
-        @password = "gt38sdt92bv"
+      it "adds an error when block_common_passwords is enabled" do
+        SiteSetting.stubs(:block_common_passwords).returns(true)
+        @password = "password"
         validate
         record.errors[:password].should be_present
       end
+
+      it "doesn't add an error when block_common_passwords is disabled" do
+        SiteSetting.stubs(:block_common_passwords).returns(false)
+        @password = "password"
+        validate
+        record.errors[:password].should_not be_present
+      end
     end
   end