From 1bcb835446bdb351a1737ae9f5144e7367ce1f96 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Thu, 9 Feb 2017 15:00:22 -0500 Subject: [PATCH] FEATURE: passwords must have a minimum number of unique characters, configurable with a new setting --- config/locales/server.en.yml | 2 ++ config/site_settings.yml | 3 ++ lib/validators/password_validator.rb | 2 ++ .../validators/password_validator_spec.rb | 36 ++++++++++++++++--- spec/controllers/users_controller_spec.rb | 4 +-- 5 files changed, 41 insertions(+), 6 deletions(-) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 0cb40c04045..090b3bca008 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -372,6 +372,7 @@ en: same_as_username: "is the same as your username. Please use a more secure password." same_as_email: "is the same as your email. Please use a more secure password." same_as_current: "is the same as your current password." + unique_characters: "has too few unique characters. Please use a more secure password." ip_address: signup_not_allowed: "Signup is not allowed from this account." color_scheme_color: @@ -1044,6 +1045,7 @@ en: min_password_length: "Minimum password length." min_admin_password_length: "Minimum password length for Admin." + password_unique_characters: "Minimum number of unique characters that a password must have." block_common_passwords: "Don't allow passwords that are in the 10,000 most common passwords." enable_sso: "Enable single sign on via an external site (WARNING: USERS' EMAIL ADDRESSES *MUST* BE VALIDATED BY THE EXTERNAL SITE!)" diff --git a/config/site_settings.yml b/config/site_settings.yml index 777efa87b32..cedc9c277ea 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -350,6 +350,9 @@ users: client: true default: 15 min: 1 + password_unique_characters: + default: 5 + min: 1 block_common_passwords: true enforce_global_nicknames: default: false diff --git a/lib/validators/password_validator.rb b/lib/validators/password_validator.rb index 2ce97f26a8a..a3a0fd3b5c3 100644 --- a/lib/validators/password_validator.rb +++ b/lib/validators/password_validator.rb @@ -18,6 +18,8 @@ class PasswordValidator < ActiveModel::EachValidator record.errors.add(attribute, :same_as_current) elsif SiteSetting.block_common_passwords && CommonPasswords.common_password?(value) record.errors.add(attribute, :common) + elsif value.chars.uniq.size < SiteSetting.password_unique_characters + record.errors.add(attribute, :unique_characters) end end diff --git a/spec/components/validators/password_validator_spec.rb b/spec/components/validators/password_validator_spec.rb index 664fc7b114c..53e4db45537 100644 --- a/spec/components/validators/password_validator_spec.rb +++ b/spec/components/validators/password_validator_spec.rb @@ -3,6 +3,10 @@ require_dependency "common_passwords/common_passwords" describe PasswordValidator do + def password_error_message(key) + I18n.t("activerecord.errors.models.user.attributes.password.#{key.to_s}") + end + let(:validator) { described_class.new({attributes: :password}) } subject(:validate) { validator.validate_each(record,:password,@password) } @@ -72,7 +76,7 @@ describe PasswordValidator do SiteSetting.stubs(:block_common_passwords).returns(true) @password = "password" validate - expect(record.errors[:password]).to be_present + expect(record.errors[:password]).to include(password_error_message(:common)) end it "doesn't add an error when block_common_passwords is disabled" do @@ -83,18 +87,42 @@ describe PasswordValidator do end end + context "password_unique_characters is 5" do + before do + SiteSetting.password_unique_characters = 5 + end + + it "adds an error when there are too few unique characters" do + @password = "cheeeeeeeese" + validate + expect(record.errors[:password]).to include(password_error_message(:unique_characters)) + end + + it "doesn't add an error when there are enough unique characters" do + @password = "spooooooorts" + validate + expect(record.errors[:password]).not_to be_present + end + + it "counts capital letters as unique" do + @password = "cHeEeeeeesE" + validate + expect(record.errors[:password]).not_to be_present + end + end + it "adds an error when password is the same as the username" do @password = "porkchops1234" record.username = @password validate - expect(record.errors[:password]).to be_present + expect(record.errors[:password]).to include(password_error_message(:same_as_username)) end it "adds an error when password is the same as the email" do @password = "pork@chops.com" record.email = @password validate - expect(record.errors[:password]).to be_present + expect(record.errors[:password]).to include(password_error_message(:same_as_email)) end it "adds an error when new password is same as current password" do @@ -103,7 +131,7 @@ describe PasswordValidator do record.reload record.password = @password validate - expect(record.errors[:password]).to be_present + expect(record.errors[:password]).to include(password_error_message(:same_as_current)) end end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 496a502165b..9b735b577bc 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -752,7 +752,7 @@ describe UsersController do context "with values for the fields" do let(:create_params) { { name: @user.name, - password: 'watwatwatwat', + password: 'suChS3cuRi7y', username: @user.username, email: @user.email, user_fields: { @@ -802,7 +802,7 @@ describe UsersController do context "without values for the fields" do let(:create_params) { { name: @user.name, - password: 'watwatwatwat', + password: 'suChS3cuRi7y', username: @user.username, email: @user.email, } }