From 854d9c8fc6f94f6c3c5bdcd17ab09ce2324411a4 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Thu, 19 Dec 2013 16:15:36 -0500 Subject: [PATCH] Minimum password length is configurable with the min_password_length site setting. FIX: reset password needs to validate password length. --- .../controllers/create_account_controller.js | 4 +- app/controllers/users_controller.rb | 1 + config/locales/server.en.yml | 1 + config/site_settings.yml | 3 ++ lib/validators/password_validator.rb | 4 +- .../validators/password_validator_spec.rb | 52 ++++++++++++------- .../fixtures/site_settings_fixtures.js | 2 +- 7 files changed, 43 insertions(+), 24 deletions(-) diff --git a/app/assets/javascripts/discourse/controllers/create_account_controller.js b/app/assets/javascripts/discourse/controllers/create_account_controller.js index d409757d621..6babd49d80a 100644 --- a/app/assets/javascripts/discourse/controllers/create_account_controller.js +++ b/app/assets/javascripts/discourse/controllers/create_account_controller.js @@ -31,7 +31,7 @@ Discourse.CreateAccountController = Discourse.Controller.extend(Discourse.ModalF }.property('authOptions.auth_provider'), passwordInstructions: function() { - return I18n.t('user.password.instructions', {count: 6}); // TODO: soon to be configurable + return I18n.t('user.password.instructions', {count: Discourse.SiteSettings.min_password_length}); }.property(), // Validate the name @@ -273,7 +273,7 @@ Discourse.CreateAccountController = Discourse.Controller.extend(Discourse.ModalF } // If too short - if (password.length < 6) { + if (password.length < Discourse.SiteSettings.min_password_length) { return Discourse.InputValidation.create({ failed: true, reason: I18n.t('user.password.too_short') diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 29591269ca0..6174652d2d7 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -172,6 +172,7 @@ class UsersController < ApplicationController elsif request.put? raise Discourse::InvalidParameters.new(:password) unless params[:password].present? @user.password = params[:password] + @user.password_required! logon_after_password_reset if @user.save end render layout: 'no_js' diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 1c54ddbf0a2..da14b65a514 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -603,6 +603,7 @@ en: login_required: "Require authentication to read posts" + min_password_length: "Minimum password length." 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" diff --git a/config/site_settings.yml b/config/site_settings.yml index 286d6e57526..280879454b3 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -76,6 +76,9 @@ users: must_approve_users: client: true default: false + min_password_length: + client: true + default: 8 enable_google_logins: client: true default: true diff --git a/lib/validators/password_validator.rb b/lib/validators/password_validator.rb index 9f87fdecd68..b396cdf3f77 100644 --- a/lib/validators/password_validator.rb +++ b/lib/validators/password_validator.rb @@ -4,8 +4,8 @@ class PasswordValidator < ActiveModel::EachValidator return unless record.password_required? if value.nil? record.errors.add(attribute, :blank) - elsif value.length < 6 - record.errors.add(attribute, :too_short, count: 6) + elsif value.length < SiteSetting.min_password_length + record.errors.add(attribute, :too_short, count: SiteSetting.min_password_length) end end diff --git a/spec/components/validators/password_validator_spec.rb b/spec/components/validators/password_validator_spec.rb index cdaa6a38d0e..6d23ce1be9a 100644 --- a/spec/components/validators/password_validator_spec.rb +++ b/spec/components/validators/password_validator_spec.rb @@ -8,28 +8,42 @@ describe PasswordValidator do context "password required" do let(:record) { u = Fabricate.build(:user, password: @password); u.password_required!; u } - it "doesn't add an error when password is good" do - @password = "weron235alsfn234" - validate - record.errors[:password].should_not 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 too short" do - @password = "p" - 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 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 + it "adds an error when password length is 11" do + @password = "gt38sdt92bv" + validate + record.errors[:password].should be_present + end end end diff --git a/test/javascripts/fixtures/site_settings_fixtures.js b/test/javascripts/fixtures/site_settings_fixtures.js index 718c65dafb5..d32a26ba707 100644 --- a/test/javascripts/fixtures/site_settings_fixtures.js +++ b/test/javascripts/fixtures/site_settings_fixtures.js @@ -1,3 +1,3 @@ /*jshint maxlen:10000000 */ -Discourse.SiteSettingsOriginal = {"title":"Discourse Meta","logo_url":"/assets/logo.png","logo_small_url":"/assets/logo-single.png","traditional_markdown_linebreaks":false,"top_menu":"latest|new|unread|read|favorited|categories","post_menu":"like|edit|flag|delete|share|bookmark|reply","share_links":"twitter|facebook|google+|email","track_external_right_clicks":false,"must_approve_users":false,"ga_tracking_code":"UA-33736483-2","ga_domain_name":"","enable_long_polling":true,"polling_interval":3000,"anon_polling_interval":30000,"min_post_length":20,"max_post_length":16000,"min_topic_title_length":15,"max_topic_title_length":255,"min_private_message_title_length":2,"allow_uncategorized_topics":true,"min_search_term_length":3,"flush_timings_secs":5,"suppress_reply_directly_below":true,"email_domains_blacklist":"mailinator.com","email_domains_whitelist":null,"version_checks":true,"min_title_similar_length":10,"min_body_similar_length":15,"category_colors":"BF1E2E|F1592A|F7941D|9EB83B|3AB54A|12A89D|25AAE2|0E76BD|652D90|92278F|ED207B|8C6238|231F20|808281|B3B5B4|283890","max_upload_size_kb":1024,"category_featured_topics":6,"favicon_url":"/assets/favicon.ico","dynamic_favicon":false,"uncategorized_name":"uncategorized","uncategorized_color":"AB9364","uncategorized_text_color":"FFFFFF","invite_only":false,"login_required":false,"enable_local_logins":true,"enable_local_account_create":true,"enable_google_logins":true,"enable_yahoo_logins":true,"enable_twitter_logins":true,"enable_facebook_logins":true,"enable_cas_logins":false,"enable_github_logins":true,"enable_persona_logins":true,"educate_until_posts":2,"topic_views_heat_low":1000,"topic_views_heat_medium":2000,"topic_views_heat_high":5000,"min_private_message_post_length":5,"faq_url":"","tos_url":"","privacy_policy_url":"","authorized_extensions":".jpg|.jpeg|.png|.gif|.txt","relative_date_duration":14,"delete_removed_posts_after":24,"delete_user_max_age":7, "default_code_lang": "lang-auto"}; +Discourse.SiteSettingsOriginal = {"title":"Discourse Meta","logo_url":"/assets/logo.png","logo_small_url":"/assets/logo-single.png","traditional_markdown_linebreaks":false,"top_menu":"latest|new|unread|read|favorited|categories","post_menu":"like|edit|flag|delete|share|bookmark|reply","share_links":"twitter|facebook|google+|email","track_external_right_clicks":false,"must_approve_users":false,"ga_tracking_code":"UA-33736483-2","ga_domain_name":"","enable_long_polling":true,"polling_interval":3000,"anon_polling_interval":30000,"min_post_length":20,"max_post_length":16000,"min_topic_title_length":15,"max_topic_title_length":255,"min_private_message_title_length":2,"allow_uncategorized_topics":true,"min_search_term_length":3,"flush_timings_secs":5,"suppress_reply_directly_below":true,"email_domains_blacklist":"mailinator.com","email_domains_whitelist":null,"version_checks":true,"min_title_similar_length":10,"min_body_similar_length":15,"category_colors":"BF1E2E|F1592A|F7941D|9EB83B|3AB54A|12A89D|25AAE2|0E76BD|652D90|92278F|ED207B|8C6238|231F20|808281|B3B5B4|283890","max_upload_size_kb":1024,"category_featured_topics":6,"favicon_url":"/assets/favicon.ico","dynamic_favicon":false,"uncategorized_name":"uncategorized","uncategorized_color":"AB9364","uncategorized_text_color":"FFFFFF","invite_only":false,"login_required":false,"min_password_length":8,"enable_local_logins":true,"enable_local_account_create":true,"enable_google_logins":true,"enable_yahoo_logins":true,"enable_twitter_logins":true,"enable_facebook_logins":true,"enable_cas_logins":false,"enable_github_logins":true,"enable_persona_logins":true,"educate_until_posts":2,"topic_views_heat_low":1000,"topic_views_heat_medium":2000,"topic_views_heat_high":5000,"min_private_message_post_length":5,"faq_url":"","tos_url":"","privacy_policy_url":"","authorized_extensions":".jpg|.jpeg|.png|.gif|.txt","relative_date_duration":14,"delete_removed_posts_after":24,"delete_user_max_age":7, "default_code_lang": "lang-auto"}; Discourse.SiteSettings = jQuery.extend(true, {}, Discourse.SiteSettingsOriginal);